[clang-tools-extra] b2eaac3 - [clangd] Replace shortenNamespace with getQualification

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-01-03T09:05:30+01:00
New Revision: b2eaac3e3e0a6177f16b3e5c2a4c7c6a85104ff5

URL: 
https://github.com/llvm/llvm-project/commit/b2eaac3e3e0a6177f16b3e5c2a4c7c6a85104ff5
DIFF: 
https://github.com/llvm/llvm-project/commit/b2eaac3e3e0a6177f16b3e5c2a4c7c6a85104ff5.diff

LOG: [clangd] Replace shortenNamespace with getQualification

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D71652

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/AST.h
clang-tools-extra/clangd/unittests/ASTTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index d06245a53977..c800ee870dc9 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -8,8 +8,10 @@
 
 #include "AST.h"
 
+#include "FindTarget.h"
 #include "SourceCode.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
@@ -299,32 +301,21 @@ llvm::Optional getSymbolID(const 
llvm::StringRef MacroName,
   return SymbolID(USR);
 }
 
-std::string shortenNamespace(const llvm::StringRef OriginalName,
- const llvm::StringRef CurrentNamespace) {
-  llvm::SmallVector OriginalParts;
-  llvm::SmallVector CurrentParts;
-  llvm::SmallVector Result;
-  OriginalName.split(OriginalParts, "::");
-  CurrentNamespace.split(CurrentParts, "::");
-  auto MinLength = std::min(CurrentParts.size(), OriginalParts.size());
-
-  unsigned DifferentAt = 0;
-  while (DifferentAt < MinLength &&
- CurrentParts[DifferentAt] == OriginalParts[DifferentAt]) {
-DifferentAt++;
-  }
-
-  for (unsigned i = DifferentAt; i < OriginalParts.size(); ++i) {
-Result.push_back(OriginalParts[i]);
-  }
-  return join(Result, "::");
-}
-
-std::string printType(const QualType QT, const DeclContext &Context) {
-  PrintingPolicy PP(Context.getParentASTContext().getPrintingPolicy());
-  PP.SuppressUnwrittenScope = 1;
-  PP.SuppressTagKeyword = 1;
-  return shortenNamespace(QT.getAsString(PP), printNamespaceScope(Context));
+// FIXME: This should be handled while printing underlying decls instead.
+std::string printType(const QualType QT, const DeclContext &CurContext) {
+  std::string Result;
+  llvm::raw_string_ostream OS(Result);
+  auto Decls = explicitReferenceTargets(
+  ast_type_traits::DynTypedNode::create(QT), DeclRelation::Alias);
+  if (!Decls.empty())
+OS << getQualification(CurContext.getParentASTContext(), &CurContext,
+   Decls.front(),
+   
/*VisibleNamespaces=*/llvm::ArrayRef{});
+  PrintingPolicy PP(CurContext.getParentASTContext().getPrintingPolicy());
+  PP.SuppressScope = true;
+  PP.SuppressTagKeyword = true;
+  QT.print(OS, PP);
+  return OS.str();
 }
 
 QualType declaredType(const TypeDecl *D) {
@@ -464,7 +455,7 @@ std::string getQualification(ASTContext &Context,
 
 std::string getQualification(ASTContext &Context,
  const DeclContext *DestContext,
- SourceLocation InsertionPoint, const NamedDecl 
*ND,
+ const NamedDecl *ND,
  llvm::ArrayRef VisibleNamespaces) {
   for (llvm::StringRef NS : VisibleNamespaces) {
 assert(NS.endswith("::"));

diff  --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index a823365e7053..f65c42869b74 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -80,19 +80,7 @@ llvm::Optional getSymbolID(const llvm::StringRef 
MacroName,
 
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like annoymous/inline namespace.
-std::string printType(const QualType QT, const DeclContext &Context);
-
-/// Try to shorten the OriginalName by removing namespaces from the left of
-/// the string that are redundant in the CurrentNamespace. This way the type
-/// idenfier become shorter and easier to read.
-/// Limitation: It only handles the qualifier of the type itself, not that of
-/// templates.
-/// FIXME: change type of parameter CurrentNamespace to DeclContext ,
-/// take in to account using directives etc
-/// Example: shortenNamespace("ns1::MyClass", "ns1")
-///--> "MyClass"
-std::string shortenNamespace(const llvm::StringRef OriginalName,
- const llvm::StringRef CurrentNamespace);
+std::string printType(const QualType QT, const DeclContext &CurContext);
 
 /// Indicates if \p D is a template instantiation implicitly generated by the
 /// compiler, e.g.
@@ -157,7 +145,7 @@ std::string getQualification(ASTContext &Context,
 /// present in \p VisibleNamespaces, no matter whether it is from ns1:: or 
ns2::
 s

[PATCH] D72085: [clangd] Fix hover for functions inside templated classes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet abandoned this revision.
kadircet added a comment.

nope i haven't, thanks for taking care !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72085/new/

https://reviews.llvm.org/D72085



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71652: [clangd] Replace shortenNamespace with getQualification

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
kadircet marked an inline comment as done.
Closed by commit rGb2eaac3e3e0a: [clangd] Replace shortenNamespace with 
getQualification (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D71652?vs=234564&id=235996#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71652/new/

https://reviews.llvm.org/D71652

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp

Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/DeclBase.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -25,29 +26,6 @@
 namespace clangd {
 namespace {
 
-TEST(ShortenNamespace, All) {
-  ASSERT_EQ("TestClass", shortenNamespace("TestClass", ""));
-
-  ASSERT_EQ("TestClass", shortenNamespace(
-  "testnamespace::TestClass", "testnamespace"));
-
-  ASSERT_EQ(
-  "namespace1::TestClass",
-  shortenNamespace("namespace1::TestClass", "namespace2"));
-
-  ASSERT_EQ("TestClass",
-shortenNamespace("testns1::testns2::TestClass",
- "testns1::testns2"));
-
-  ASSERT_EQ(
-  "testns2::TestClass",
-  shortenNamespace("testns1::testns2::TestClass", "testns1"));
-
-  ASSERT_EQ("TestClass",
-shortenNamespace(
-"testns1::TestClass", "testns1"));
-}
-
 TEST(GetDeducedType, KwAutoExpansion) {
   struct Test {
 StringRef AnnotatedCode;
@@ -166,14 +144,70 @@
   Case.Qualifications[I]);
   } else {
 EXPECT_EQ(getQualification(AST.getASTContext(),
-   D->getLexicalDeclContext(), D->getBeginLoc(),
-   TargetDecl, Case.VisibleNamespaces),
+   D->getLexicalDeclContext(), TargetDecl,
+   Case.VisibleNamespaces),
   Case.Qualifications[I]);
   }
 }
   }
 }
 
+TEST(ClangdAST, PrintType) {
+  const struct {
+llvm::StringRef Test;
+std::vector Types;
+  } Cases[] = {
+  {
+  R"cpp(
+namespace ns1 { namespace ns2 { class Foo {}; } }
+void insert(); // ns1::ns2::Foo
+namespace ns1 {
+  void insert(); // ns2::Foo
+  namespace ns2 {
+void insert(); // Foo
+  }
+}
+  )cpp",
+  {"ns1::ns2::Foo", "ns2::Foo", "Foo"},
+  },
+  {
+  R"cpp(
+namespace ns1 {
+  typedef int Foo;
+}
+void insert(); // ns1::Foo
+namespace ns1 {
+  void insert(); // Foo
+}
+  )cpp",
+  {"ns1::Foo", "Foo"},
+  },
+  };
+  for (const auto &Case : Cases) {
+Annotations Test(Case.Test);
+TestTU TU = TestTU::withCode(Test.code());
+ParsedAST AST = TU.build();
+std::vector InsertionPoints;
+const TypeDecl *TargetDecl = nullptr;
+findDecl(AST, [&](const NamedDecl &ND) {
+  if (ND.getNameAsString() == "Foo") {
+if (const auto *TD = llvm::dyn_cast(&ND)) {
+  TargetDecl = TD;
+  return true;
+}
+  } else if (ND.getNameAsString() == "insert")
+InsertionPoints.push_back(ND.getDeclContext());
+  return false;
+});
+
+ASSERT_EQ(InsertionPoints.size(), Case.Types.size());
+for (size_t I = 0, E = InsertionPoints.size(); I != E; ++I) {
+  const auto *DC = InsertionPoints[I];
+  EXPECT_EQ(printType(AST.getASTContext().getTypeDeclType(TargetDecl), *DC),
+Case.Types[I]);
+}
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/AST.h
===
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -80,19 +80,7 @@
 
 /// Returns a QualType as string. The result doesn't contain unwritten scopes
 /// like annoymous/inline namespace.
-std::string printType(const QualType QT, const DeclContext &Context);
-
-/// Try to shorten the OriginalName by removing namespaces from the left of
-/// the string that are redundant in the CurrentNamespace. This way the type
-/// idenfier become shorter and easier to read.
-/// Limitation: It only handles the qualifier of the type itself, not that of
-/// templates.
-/// FIXME: change type of parameter CurrentNamespace to DeclContext ,
-/// take in to account using directives etc
-/// Example: shortenNamespace("ns1::MyClass", "ns1")
-///--> "MyClass"
-std::string shortenNamespace(const llvm::Stri

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a reviewer: sammccall.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:499
+  )cpp",
+
+  R"cpp(// Class template argument deduction

could you rather add this test into `FindTargetTests.cpp` ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72119/new/

https://reviews.llvm.org/D72119



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72066: [clangd] Assert that the testcases in LocateSymbol.All have no diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

Thanks!




Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:519
+ASSERT_TRUE(AST.getDiagnostics().empty())
+<< AST.getDiagnostics().begin()->Message << Test;
+

nit: I would rather keep the loop and just use `ADD_FAILURE()` instead of 
`llvm::errs()`.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:573
+// parsing.
+TU.ExtraArgs.push_back("-fno-delayed-template-parsing");
+

nit: this one can be dropped.



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:577
+for (auto &Diag : AST.getDiagnostics()) {
+  ASSERT_TRUE(Diag.Severity <= DiagnosticsEngine::Warning)
+  << Diag.Message << Test;

nit: this looks like an overkill to me, in the end we are trying to test hover 
behavior, not diagnostics. I would drop the whole loop here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72066/new/

https://reviews.llvm.org/D72066



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[Awanish Pandey via Phabricator via cfe-commits]

awpandey updated this revision to Diff 235998.
awpandey added a comment.
Herald added a subscriber: hiraditya.

@dblaikie Thanks a lot for the suggestions. I have made the changes such that 
this will become consistent with your suggestions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70524/new/

https://reviews.llvm.org/D70524

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/test/CodeGenCXX/debug-info-auto-return.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1165,6 +1165,14 @@
   DIE *DeclDie = nullptr;
   StringRef DeclLinkageName;
   if (auto *SPDecl = SP->getDeclaration()) {
+DITypeRefArray DeclArgs, DefinitionArgs;
+DeclArgs = SPDecl->getType()->getTypeArray();
+DefinitionArgs = SP->getType()->getTypeArray();
+
+if (DeclArgs.size() && DefinitionArgs.size())
+  if (DeclArgs[0] != DefinitionArgs[0])
+addType(SPDie, DefinitionArgs[0]);
+
 DeclDie = getDIE(SPDecl);
 assert(DeclDie && "This DIE should've already been constructed when the "
   "definition DIE was created in "
Index: clang/test/CodeGenCXX/debug-info-auto-return.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/debug-info-auto-return.cpp
@@ -0,0 +1,22 @@
+//  Test for debug info for C++11 auto return member functions
+// RUN: %clang_cc1 -dwarf-version=5  -emit-llvm -triple x86_64-linux-gnu %s -o - \
+// RUN:   -O0 -disable-llvm-passes \
+// RUN:   -debug-info-kind=standalone \
+// RUN: | FileCheck %s
+
+// CHECK: !DISubprogram(name: "findMax",{{.*}}, type: ![[t:[0-9]+]],{{.*}}
+
+// CHECK: ![[t:[0-9]+]] = !DISubroutineType(types: ![[t1:[0-9]+]])
+// CHECK-NEXT: ![[t1:[0-9]+]] = !{![[t2:[0-9]+]], {{.*}}
+// CHECK-NEXT: ![[t2:[0-9]+]] = !DIBasicType(name: "double", {{.*}})
+
+// CHECK: ![[t:[0-9]+]] = !DISubroutineType(types: ![[t1:[0-9]+]])
+// CHECK-NEXT: ![[t1:[0-9]+]] = !{![[t2:[0-9]+]], {{.*}}
+// CHECK-NEXT: ![[t2:[0-9]+]] = !DIBasicType(tag: DW_TAG_unspecified_type, name: "auto")
+struct myClass {
+  auto findMax();
+};
+
+auto myClass::findMax() {
+  return 0.0;
+}
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -165,6 +165,7 @@
   /// ivars and property accessors.
   llvm::DIType *CreateType(const BuiltinType *Ty);
   llvm::DIType *CreateType(const ComplexType *Ty);
+  llvm::DIType *CreateType(const AutoType *Ty);
   llvm::DIType *CreateQualifiedType(QualType Ty, llvm::DIFile *Fg);
   llvm::DIType *CreateType(const TypedefType *Ty, llvm::DIFile *Fg);
   llvm::DIType *CreateType(const TemplateSpecializationType *Ty,
@@ -214,10 +215,10 @@
   /// not updated to include implicit \c this pointer. Use this routine
   /// to get a method type which includes \c this pointer.
   llvm::DISubroutineType *getOrCreateMethodType(const CXXMethodDecl *Method,
-llvm::DIFile *F);
+llvm::DIFile *F, bool decl);
   llvm::DISubroutineType *
   getOrCreateInstanceMethodType(QualType ThisPtr, const FunctionProtoType *Func,
-llvm::DIFile *Unit);
+llvm::DIFile *Unit, bool decl);
   llvm::DISubroutineType *
   getOrCreateFunctionType(const Decl *D, QualType FnType, llvm::DIFile *F);
   /// \return debug info descriptor for vtable.
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -810,6 +810,10 @@
   return DBuilder.createBasicType(BTName, Size, Encoding);
 }
 
+llvm::DIType *CGDebugInfo::CreateType(const AutoType *Ty) {
+  return DBuilder.createUnspecifiedType("auto");
+}
+
 llvm::DIType *CGDebugInfo::CreateType(const ComplexType *Ty) {
   // Bit size and offset of the type.
   llvm::dwarf::TypeKind Encoding = llvm::dwarf::DW_ATE_complex_float;
@@ -1457,16 +1461,18 @@
 
 llvm::DISubroutineType *
 CGDebugInfo::getOrCreateMethodType(const CXXMethodDecl *Method,
-   llvm::DIFile *Unit) {
+   llvm::DIFile *Unit, bool decl) {
   const FunctionProtoType *Func = Method->getType()->getAs();
   if (Method->isStatic())
 return cast_or_null(
 getOrCreateType(QualType(Func, 0), Unit));
-  return getOrCreateInstanceMethodType(Method->getThisType(), Func, Unit);
+  return getOrCreateInstanceMethodType(Method->getThisType(), Func, Unit, decl);
 }
 
-llvm::DISubroutineType *CGDebugInfo::getOrCreateInstanceMethodType(
-QualType ThisPtr, const FunctionProtoType *Func, llvm::DIFile *Unit) {
+llvm::DISu

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

Please add a test for `findExplicitReferences` too


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72119/new/

https://reviews.llvm.org/D72119



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

sammccall wrote:
> ilya-biryukov wrote:
> > No need to fix this.
> > 
> > The name could probably be better, but we can fix this later. Don't have 
> > any good ideas.
> I think as a public API this should be clearer on how/when to use and its 
> relation to other things (planning to send a patch, but wanted to discuss a 
> bit here first).
> 
> - This is essentially a filter of allTargetDecls (as is targetDecl), but the 
> relationship between the two isn't clear. They should have closely related 
> names (variant) or maybe better be more orthogonal and composed at the 
> callsite.
> - The most distinctive word in the name is `explicit`, but this function 
> doesn't actually have anything to do with explicit vs non-explicit references 
> (just history?)
> - It's not clear to me whether we actually want to encapsulate/hide the 
> preference for instantiations over templates here, or whether it should be 
> expressed at the callsite because the policy is decided feature-by-feature. 
> `chooseBest(...)` vs `prefer(TemplateInstantiation, TemplatePattern, ...)`. 
> The latter seems safer to me, based on experience with targetDecl so far.
> 
> A couple of ideas:
>  - caller invokes allTargetDecls() and then this is a helper function 
> `prefer(DeclRelation, DeclRelation, Results)` that mutates Results
>  - targetDecl() becomes the swiss-army filter and accepts a list of 
> "prefer-X-over-Y", which it applies before returning the results with flags 
> dropped
I don't like the idea of `targetDecl` becoming the swiss-army knife, it's 
complicated to use as is.
The contract of `explicitReferenceTargets` is to expose only the decls written 
in the source code and nothing else, it's much simpler to explain and to use.

It's useful for many things, essentially for anything that needs to answer the 
question of "what does this name in the source code refers to".

The name could be clearer, the comments might need to be improved.
The only crucial improvement that I'd attempt is getting rid of the `Mask` 
parameter, I think there was just one or two cases where it was useful.




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71596/new/

https://reviews.llvm.org/D71596



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71533: [clangd] Show template arguments in type hierarchy when possible

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:140
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
-auto Decls = targetDecl(N->ASTNode, Relations);
+auto Decls = explicitReferenceTargets(N->ASTNode, Relations);
 Result.assign(Decls.begin(), Decls.end());

this will result in changes in behavior for other functionality (it is 
unfortunate if no tests regressed), for example `findReferences` did work on 
`template patterns` rather than `instantiations` now it won't receive pattern 
results when triggered on `instantiations`. whether we want that or not is up 
for debate, but definitely not part of this patch.



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:418
+  EXPECT_THAT(*Result,
+  AllOf(WithName("S<0>"), WithKind(SymbolKind::Struct), 
Parents()));
 }

nridge wrote:
> kadircet wrote:
> > what about making use of template pattern in case of invalid instantiations?
> > 
> > as type hierarchy tries to provide information regarding `bases` and 
> > `children`, I think it is more important to show those instead of template 
> > instantiation arguments.
> Is there a way to query a `ClassTemplateSpecializationDecl` for whether 
> instantiating its base specifier failed?
> 
> Or, should we fall back on the template pattern any time `bases()` is empty?
you can check for `CTSD->isInvalidDecl()` and fallback to pattern in such cases.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71533/new/

https://reviews.llvm.org/D71533



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > No need to fix this.
> > > 
> > > The name could probably be better, but we can fix this later. Don't have 
> > > any good ideas.
> > I think as a public API this should be clearer on how/when to use and its 
> > relation to other things (planning to send a patch, but wanted to discuss a 
> > bit here first).
> > 
> > - This is essentially a filter of allTargetDecls (as is targetDecl), but 
> > the relationship between the two isn't clear. They should have closely 
> > related names (variant) or maybe better be more orthogonal and composed at 
> > the callsite.
> > - The most distinctive word in the name is `explicit`, but this function 
> > doesn't actually have anything to do with explicit vs non-explicit 
> > references (just history?)
> > - It's not clear to me whether we actually want to encapsulate/hide the 
> > preference for instantiations over templates here, or whether it should be 
> > expressed at the callsite because the policy is decided feature-by-feature. 
> > `chooseBest(...)` vs `prefer(TemplateInstantiation, TemplatePattern, ...)`. 
> > The latter seems safer to me, based on experience with targetDecl so far.
> > 
> > A couple of ideas:
> >  - caller invokes allTargetDecls() and then this is a helper function 
> > `prefer(DeclRelation, DeclRelation, Results)` that mutates Results
> >  - targetDecl() becomes the swiss-army filter and accepts a list of 
> > "prefer-X-over-Y", which it applies before returning the results with flags 
> > dropped
> I don't like the idea of `targetDecl` becoming the swiss-army knife, it's 
> complicated to use as is.
> The contract of `explicitReferenceTargets` is to expose only the decls 
> written in the source code and nothing else, it's much simpler to explain and 
> to use.
> 
> It's useful for many things, essentially for anything that needs to answer 
> the question of "what does this name in the source code refers to".
> 
> The name could be clearer, the comments might need to be improved.
> The only crucial improvement that I'd attempt is getting rid of the `Mask` 
> parameter, I think there was just one or two cases where it was useful.
> 
> 
> I don't like the idea of targetDecl becoming the swiss-army knife
Fair enough. I'll poke more in the direction of making it easier to compose 
allTargetDecls then.

> The contract of explicitReferenceTargets is to expose only the decls written 
> in the source code and nothing else, it's much simpler to explain and to use.
So I agree that the interface of targetDecl() is complicated, but I don't think 
this one is simpler to explain or use - or at least it hasn't been 
well-explained so far.

For example,
 - "to expose only the decls written in the source code" - `vector` isn't 
a decl written in the source code.
 - "find declarations explicitly referenced in the source code" - `return 
[[{foo}]];` - the class/constructor isn't explicitly referenced, but this 
function returns it.
 - "what does this name in the source code refers to" - a template name refers 
to the primary template, the (possible) partial specialization, and 
specialization all at once. Features like find refs/highlight show the 
equivalence class of names that refer to the same thing, but they don't prefer 
the specialization for that.
 - none of these explanations explains why the function is opinionated about 
template vs expansion but not about alias vs underlying.

> No need to fix this.
> The name could probably be better, but we can fix this later. Don't have any 
> good ideas.

I think it's fine (and good!) that we this got added to unblock the hover work 
and to understand more use cases for this API. But I do think it's time to fix 
it now, and I'm not convinced a better name will do it (I can't think of a good 
name to describe the current functionality, either). Let me put a patch 
together and take a look?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71596/new/

https://reviews.llvm.org/D71596



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72119: [clangd] Handle DeducedTemplateSpecializationType in TargetFinder

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks!




Comment at: clang-tools-extra/clangd/FindTarget.cpp:318
   }
+  void VisitDeducedTemplateSpecializationType(
+  const DeducedTemplateSpecializationType *DTST) {

This needs a comment referencing the bug, and describing the consequence (we're 
going to miss the instantiation even when it's in principle known)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72119/new/

https://reviews.llvm.org/D72119



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70765: LTOVisibility.rst: fix up syntax in example

[Nick Black via Phabricator via cfe-commits]

dankamongmen added a comment.

I hate to bother anyone, but can this go ahead and get merged? :) thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70765/new/

https://reviews.llvm.org/D70765



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > No need to fix this.
> > > > 
> > > > The name could probably be better, but we can fix this later. Don't 
> > > > have any good ideas.
> > > I think as a public API this should be clearer on how/when to use and its 
> > > relation to other things (planning to send a patch, but wanted to discuss 
> > > a bit here first).
> > > 
> > > - This is essentially a filter of allTargetDecls (as is targetDecl), but 
> > > the relationship between the two isn't clear. They should have closely 
> > > related names (variant) or maybe better be more orthogonal and composed 
> > > at the callsite.
> > > - The most distinctive word in the name is `explicit`, but this function 
> > > doesn't actually have anything to do with explicit vs non-explicit 
> > > references (just history?)
> > > - It's not clear to me whether we actually want to encapsulate/hide the 
> > > preference for instantiations over templates here, or whether it should 
> > > be expressed at the callsite because the policy is decided 
> > > feature-by-feature. `chooseBest(...)` vs `prefer(TemplateInstantiation, 
> > > TemplatePattern, ...)`. The latter seems safer to me, based on experience 
> > > with targetDecl so far.
> > > 
> > > A couple of ideas:
> > >  - caller invokes allTargetDecls() and then this is a helper function 
> > > `prefer(DeclRelation, DeclRelation, Results)` that mutates Results
> > >  - targetDecl() becomes the swiss-army filter and accepts a list of 
> > > "prefer-X-over-Y", which it applies before returning the results with 
> > > flags dropped
> > I don't like the idea of `targetDecl` becoming the swiss-army knife, it's 
> > complicated to use as is.
> > The contract of `explicitReferenceTargets` is to expose only the decls 
> > written in the source code and nothing else, it's much simpler to explain 
> > and to use.
> > 
> > It's useful for many things, essentially for anything that needs to answer 
> > the question of "what does this name in the source code refers to".
> > 
> > The name could be clearer, the comments might need to be improved.
> > The only crucial improvement that I'd attempt is getting rid of the `Mask` 
> > parameter, I think there was just one or two cases where it was useful.
> > 
> > 
> > I don't like the idea of targetDecl becoming the swiss-army knife
> Fair enough. I'll poke more in the direction of making it easier to compose 
> allTargetDecls then.
> 
> > The contract of explicitReferenceTargets is to expose only the decls 
> > written in the source code and nothing else, it's much simpler to explain 
> > and to use.
> So I agree that the interface of targetDecl() is complicated, but I don't 
> think this one is simpler to explain or use - or at least it hasn't been 
> well-explained so far.
> 
> For example,
>  - "to expose only the decls written in the source code" - `vector` 
> isn't a decl written in the source code.
>  - "find declarations explicitly referenced in the source code" - `return 
> [[{foo}]];` - the class/constructor isn't explicitly referenced, but this 
> function returns it.
>  - "what does this name in the source code refers to" - a template name 
> refers to the primary template, the (possible) partial specialization, and 
> specialization all at once. Features like find refs/highlight show the 
> equivalence class of names that refer to the same thing, but they don't 
> prefer the specialization for that.
>  - none of these explanations explains why the function is opinionated about 
> template vs expansion but not about alias vs underlying.
> 
> > No need to fix this.
> > The name could probably be better, but we can fix this later. Don't have 
> > any good ideas.
> 
> I think it's fine (and good!) that we this got added to unblock the hover 
> work and to understand more use cases for this API. But I do think it's time 
> to fix it now, and I'm not convinced a better name will do it (I can't think 
> of a good name to describe the current functionality, either). Let me put a 
> patch together and take a look?
What's the proposed fix?

I might miss some context with the examples you provided, but here's my view on 
how each of those should be handled:
- `vector`. This function **can** return decls, not written in the source 
code (e.g. template instantiations). References to those decls should be 
written in the source code explicitly, not the decls themselves.
- `return {foo}`. I agree this one is a corner case that we should describe. It 
still provides sensible results that could be used by hover, etc.
- "what does this name in the source code refers to". That's exactly what this 
function is about. We want to pick the most sp

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 236012.
njames93 added a comment.

More edge cases handled to do with field decls and initializers


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72121/new/

https://reviews.llvm.org/D72121

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp
@@ -0,0 +1,142 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.MemberCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase} \
+// RUN:  ]}'
+
+int set_up(int);
+int clear(int);
+
+class Foo {
+public:
+  const int bar_baz; // comment-0
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for member 'bar_baz'
+  // CHECK-FIXES: {{^}}  const int BarBaz; // comment-0
+
+  Foo(int Val) : bar_baz(Val) { // comment-1
+// CHECK-FIXES: {{^}}  Foo(int Val) : BarBaz(Val) { // comment-1
+set_up(bar_baz); // comment-2
+// CHECK-FIXES: {{^}}set_up(BarBaz); // comment-2
+  }
+
+  Foo() : Foo(0) {}
+
+  ~Foo() {
+clear(bar_baz); // comment-3
+// CHECK-FIXES: {{^}}clear(BarBaz); // comment-3
+  }
+
+  int getBar() const { return bar_baz; } // comment-4
+  // CHECK-FIXES: {{^}}  int getBar() const { return BarBaz; } // comment-4
+};
+
+class FooBar : public Foo {
+public:
+  int getFancyBar() const {
+return this->bar_baz; // comment-5
+// CHECK-FIXES: {{^}}return this->BarBaz; // comment-5
+  }
+};
+
+int getBar(const Foo &Foo) {
+  return Foo.bar_baz; // comment-6
+  // CHECK-FIXES: {{^}}  return Foo.BarBaz; // comment-6
+}
+
+int getBar(const FooBar &Foobar) {
+  return Foobar.bar_baz; // comment-7
+  // CHECK-FIXES: {{^}}  return Foobar.BarBaz; // comment-7
+}
+
+int getFancyBar(const FooBar &Foobar) {
+  return Foobar.getFancyBar();
+}
+
+template 
+class TempTest : public Foo {
+public:
+  TempTest() = default;
+  TempTest(int Val) : Foo(Val) {}
+  int getBar() const { return Foo::bar_baz; } // comment-8
+  // CHECK-FIXES: {{^}}  int getBar() const { return Foo::BarBaz; } // comment-8
+  int getBar2() const { return this->bar_baz; } // comment-9
+  // CHECK-FIXES: {{^}}  int getBar2() const { return this->BarBaz; } // comment-9
+};
+
+TempTest x; //force an instantiation
+
+int blah() {
+  return x.getBar2(); // gotta have a reference to the getBar2 so that the
+  // compiler will generate the function and resolve
+  // the DependantScopeMemberExpr
+}
+
+namespace Bug41122 {
+namespace std {
+
+// for this example we aren't bothered about how std::vector is treated
+template  //NOLINT
+class vector { //NOLINT
+public:
+  void push_back(bool); //NOLINT
+  void pop_back(); //NOLINT
+}; //NOLINT
+}; // namespace std
+
+class Foo { 
+
+  // bug report has Stack swapped to stack, this is stack swapped to Stack
+  // probably not going to cause any issue its just how
+  // people prefer their style
+
+  std::vector &stack;
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for member 'stack' [readability-identifier-naming]
+public:
+  Foo(std::vector &stack)
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for parameter 'stack' [readability-identifier-naming]
+  // CHECK-FIXES: {{^}}  Foo(std::vector &Stack)
+  : stack(stack) {
+// CHECK-FIXES: {{^}}  : Stack(Stack) {
+stack.push_back(true);
+// CHECK-FIXES: {{^}}Stack.push_back(true);
+  }
+  ~Foo() {
+stack.pop_back();
+// CHECK-FIXES: {{^}}Stack.pop_back();
+  }
+};
+}; // namespace Bug41122
+
+namespace Bug29005 {
+class Foo {
+public:
+  int a_member_of_foo = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for member 'a_member_of_foo'
+  // CHECK-FIXES: {{^}}  int AMemberOfFoo = 0;
+};
+
+int main() {
+  Foo foo;
+  return foo.a_member_of_foo;
+  // CHECK-FIXES: {{^}}  return foo.AMemberOfFoo;
+}
+}; // namespace Bug29005
+
+namespace CtorInits {
+template 
+class Container {
+  T storage[N];
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for member 'storage'
+  // CHECK-FIXES: {{^}}  T Storage[N];
+  T *pointer = &storage[0];
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for member 'pointer'
+  // CHECK-FIXES: {{^}}  T *Pointer = &Storage[0];
+public:
+  Container() : pointer(&storage[0]) {}
+  // CHECK-FIXES: {{^}}  Container() : Pointer(&Storage[0]) {}
+};
+
+void foo() {
+  Container container;
+}
+}; // namespace CtorInits
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cp

[clang] 04f627f - [Syntax] Build spanning SimpleDecalration for classes, structs, etc

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2020-01-03T12:33:11+01:00
New Revision: 04f627f6b9aeda924a83e75d281ab27a546d3515

URL: 
https://github.com/llvm/llvm-project/commit/04f627f6b9aeda924a83e75d281ab27a546d3515
DIFF: 
https://github.com/llvm/llvm-project/commit/04f627f6b9aeda924a83e75d281ab27a546d3515.diff

LOG: [Syntax] Build spanning SimpleDecalration for classes, structs, etc

When they are free-standing, e.g. `struct X;` or `struct X {};`.
Although this complicates the common case (of free-standing class
declarations), this ensures the less common case (e.g. `struct X {} a;`)
are handled uniformly and produce similar syntax trees.

Added: 


Modified: 
clang/lib/Tooling/Syntax/BuildTree.cpp
clang/unittests/Tooling/Syntax/TreeTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/Syntax/BuildTree.cpp 
b/clang/lib/Tooling/Syntax/BuildTree.cpp
index f61c5ff39e1c..7357cab6f976 100644
--- a/clang/lib/Tooling/Syntax/BuildTree.cpp
+++ b/clang/lib/Tooling/Syntax/BuildTree.cpp
@@ -343,9 +343,13 @@ class BuildTreeVisitor : public 
RecursiveASTVisitor {
   }
 
   bool WalkUpFromTagDecl(TagDecl *C) {
-// Avoid building UnknownDeclaration here, syntatically 'struct X {}' and
-// similar are part of declaration specifiers and do not introduce a new
-// top-level declaration.
+// FIXME: build the ClassSpecifier node.
+if (C->isFreeStanding()) {
+  // Class is a declaration specifier and needs a spanning declaration 
node.
+  Builder.foldNode(Builder.getRange(C),
+   new (allocator()) syntax::SimpleDeclaration);
+  return true;
+}
 return true;
   }
 

diff  --git a/clang/unittests/Tooling/Syntax/TreeTest.cpp 
b/clang/unittests/Tooling/Syntax/TreeTest.cpp
index 4de353091cdc..b54d06319e83 100644
--- a/clang/unittests/Tooling/Syntax/TreeTest.cpp
+++ b/clang/unittests/Tooling/Syntax/TreeTest.cpp
@@ -590,6 +590,50 @@ namespace foo = a;
   |-=
   |-a
   `-;
+)txt"},
+  // Free-standing classes, must live inside a SimpleDeclaration.
+  {R"cpp(
+sturct X;
+struct X {};
+
+struct Y *y1;
+struct Y {} *y2;
+
+struct {} *a1;
+)cpp",
+   R"txt(
+*: TranslationUnit
+|-SimpleDeclaration
+| |-sturct
+| |-X
+| `-;
+|-SimpleDeclaration
+| |-struct
+| |-X
+| |-{
+| |-}
+| `-;
+|-SimpleDeclaration
+| |-struct
+| |-Y
+| |-*
+| |-y1
+| `-;
+|-SimpleDeclaration
+| |-struct
+| |-Y
+| |-{
+| |-}
+| |-*
+| |-y2
+| `-;
+`-SimpleDeclaration
+  |-struct
+  |-{
+  |-}
+  |-*
+  |-a1
+  `-;
 )txt"},
   {R"cpp(
 namespace ns {}
@@ -646,24 +690,25 @@ template  struct X {
   | |-class
   | `-T
   |->
-  |-struct
-  |-X
-  |-{
-  |-UsingDeclaration
-  | |-using
-  | |-T
-  | |-::
-  | |-foo
-  | `-;
-  |-UsingDeclaration
-  | |-using
-  | |-typename
-  | |-T
-  | |-::
-  | |-bar
-  | `-;
-  |-}
-  `-;
+  `-SimpleDeclaration
+|-struct
+|-X
+|-{
+|-UsingDeclaration
+| |-using
+| |-T
+| |-::
+| |-foo
+| `-;
+|-UsingDeclaration
+| |-using
+| |-typename
+| |-T
+| |-::
+| |-bar
+| `-;
+|-}
+`-;
)txt"},
   {R"cpp(
 using type = int;



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Would changing the literal in the attribute have the same effect? I.e., 
`acquire_handle("Fuchsia_But_Please_Ignore_Me")`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72018/new/

https://reviews.llvm.org/D72018



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > No need to fix this.
> > > > > 
> > > > > The name could probably be better, but we can fix this later. Don't 
> > > > > have any good ideas.
> > > > I think as a public API this should be clearer on how/when to use and 
> > > > its relation to other things (planning to send a patch, but wanted to 
> > > > discuss a bit here first).
> > > > 
> > > > - This is essentially a filter of allTargetDecls (as is targetDecl), 
> > > > but the relationship between the two isn't clear. They should have 
> > > > closely related names (variant) or maybe better be more orthogonal and 
> > > > composed at the callsite.
> > > > - The most distinctive word in the name is `explicit`, but this 
> > > > function doesn't actually have anything to do with explicit vs 
> > > > non-explicit references (just history?)
> > > > - It's not clear to me whether we actually want to encapsulate/hide the 
> > > > preference for instantiations over templates here, or whether it should 
> > > > be expressed at the callsite because the policy is decided 
> > > > feature-by-feature. `chooseBest(...)` vs `prefer(TemplateInstantiation, 
> > > > TemplatePattern, ...)`. The latter seems safer to me, based on 
> > > > experience with targetDecl so far.
> > > > 
> > > > A couple of ideas:
> > > >  - caller invokes allTargetDecls() and then this is a helper function 
> > > > `prefer(DeclRelation, DeclRelation, Results)` that mutates Results
> > > >  - targetDecl() becomes the swiss-army filter and accepts a list of 
> > > > "prefer-X-over-Y", which it applies before returning the results with 
> > > > flags dropped
> > > I don't like the idea of `targetDecl` becoming the swiss-army knife, it's 
> > > complicated to use as is.
> > > The contract of `explicitReferenceTargets` is to expose only the decls 
> > > written in the source code and nothing else, it's much simpler to explain 
> > > and to use.
> > > 
> > > It's useful for many things, essentially for anything that needs to 
> > > answer the question of "what does this name in the source code refers to".
> > > 
> > > The name could be clearer, the comments might need to be improved.
> > > The only crucial improvement that I'd attempt is getting rid of the 
> > > `Mask` parameter, I think there was just one or two cases where it was 
> > > useful.
> > > 
> > > 
> > > I don't like the idea of targetDecl becoming the swiss-army knife
> > Fair enough. I'll poke more in the direction of making it easier to compose 
> > allTargetDecls then.
> > 
> > > The contract of explicitReferenceTargets is to expose only the decls 
> > > written in the source code and nothing else, it's much simpler to explain 
> > > and to use.
> > So I agree that the interface of targetDecl() is complicated, but I don't 
> > think this one is simpler to explain or use - or at least it hasn't been 
> > well-explained so far.
> > 
> > For example,
> >  - "to expose only the decls written in the source code" - `vector` 
> > isn't a decl written in the source code.
> >  - "find declarations explicitly referenced in the source code" - `return 
> > [[{foo}]];` - the class/constructor isn't explicitly referenced, but this 
> > function returns it.
> >  - "what does this name in the source code refers to" - a template name 
> > refers to the primary template, the (possible) partial specialization, and 
> > specialization all at once. Features like find refs/highlight show the 
> > equivalence class of names that refer to the same thing, but they don't 
> > prefer the specialization for that.
> >  - none of these explanations explains why the function is opinionated 
> > about template vs expansion but not about alias vs underlying.
> > 
> > > No need to fix this.
> > > The name could probably be better, but we can fix this later. Don't have 
> > > any good ideas.
> > 
> > I think it's fine (and good!) that we this got added to unblock the hover 
> > work and to understand more use cases for this API. But I do think it's 
> > time to fix it now, and I'm not convinced a better name will do it (I can't 
> > think of a good name to describe the current functionality, either). Let me 
> > put a patch together and take a look?
> What's the proposed fix?
> 
> I might miss some context with the examples you provided, but here's my view 
> on how each of those should be handled:
> - `vector`. This function **can** return decls, not written in the 
> source code (e.g. template instantiations). References to those decls should 
> be written in the source code explicitly, not the decls themselves.
> - `return {foo}`. I agree this one is a corner case that we should descri

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > sammccall wrote:
> > > > > ilya-biryukov wrote:
> > > > > > No need to fix this.
> > > > > > 
> > > > > > The name could probably be better, but we can fix this later. Don't 
> > > > > > have any good ideas.
> > > > > I think as a public API this should be clearer on how/when to use and 
> > > > > its relation to other things (planning to send a patch, but wanted to 
> > > > > discuss a bit here first).
> > > > > 
> > > > > - This is essentially a filter of allTargetDecls (as is targetDecl), 
> > > > > but the relationship between the two isn't clear. They should have 
> > > > > closely related names (variant) or maybe better be more orthogonal 
> > > > > and composed at the callsite.
> > > > > - The most distinctive word in the name is `explicit`, but this 
> > > > > function doesn't actually have anything to do with explicit vs 
> > > > > non-explicit references (just history?)
> > > > > - It's not clear to me whether we actually want to encapsulate/hide 
> > > > > the preference for instantiations over templates here, or whether it 
> > > > > should be expressed at the callsite because the policy is decided 
> > > > > feature-by-feature. `chooseBest(...)` vs 
> > > > > `prefer(TemplateInstantiation, TemplatePattern, ...)`. The latter 
> > > > > seems safer to me, based on experience with targetDecl so far.
> > > > > 
> > > > > A couple of ideas:
> > > > >  - caller invokes allTargetDecls() and then this is a helper function 
> > > > > `prefer(DeclRelation, DeclRelation, Results)` that mutates Results
> > > > >  - targetDecl() becomes the swiss-army filter and accepts a list of 
> > > > > "prefer-X-over-Y", which it applies before returning the results with 
> > > > > flags dropped
> > > > I don't like the idea of `targetDecl` becoming the swiss-army knife, 
> > > > it's complicated to use as is.
> > > > The contract of `explicitReferenceTargets` is to expose only the decls 
> > > > written in the source code and nothing else, it's much simpler to 
> > > > explain and to use.
> > > > 
> > > > It's useful for many things, essentially for anything that needs to 
> > > > answer the question of "what does this name in the source code refers 
> > > > to".
> > > > 
> > > > The name could be clearer, the comments might need to be improved.
> > > > The only crucial improvement that I'd attempt is getting rid of the 
> > > > `Mask` parameter, I think there was just one or two cases where it was 
> > > > useful.
> > > > 
> > > > 
> > > > I don't like the idea of targetDecl becoming the swiss-army knife
> > > Fair enough. I'll poke more in the direction of making it easier to 
> > > compose allTargetDecls then.
> > > 
> > > > The contract of explicitReferenceTargets is to expose only the decls 
> > > > written in the source code and nothing else, it's much simpler to 
> > > > explain and to use.
> > > So I agree that the interface of targetDecl() is complicated, but I don't 
> > > think this one is simpler to explain or use - or at least it hasn't been 
> > > well-explained so far.
> > > 
> > > For example,
> > >  - "to expose only the decls written in the source code" - `vector` 
> > > isn't a decl written in the source code.
> > >  - "find declarations explicitly referenced in the source code" - `return 
> > > [[{foo}]];` - the class/constructor isn't explicitly referenced, but this 
> > > function returns it.
> > >  - "what does this name in the source code refers to" - a template name 
> > > refers to the primary template, the (possible) partial specialization, 
> > > and specialization all at once. Features like find refs/highlight show 
> > > the equivalence class of names that refer to the same thing, but they 
> > > don't prefer the specialization for that.
> > >  - none of these explanations explains why the function is opinionated 
> > > about template vs expansion but not about alias vs underlying.
> > > 
> > > > No need to fix this.
> > > > The name could probably be better, but we can fix this later. Don't 
> > > > have any good ideas.
> > > 
> > > I think it's fine (and good!) that we this got added to unblock the hover 
> > > work and to understand more use cases for this API. But I do think it's 
> > > time to fix it now, and I'm not convinced a better name will do it (I 
> > > can't think of a good name to describe the current functionality, 
> > > either). Let me put a patch together and take a look?
> > What's the proposed fix?
> > 
> > I might miss some context with the examples you provided, but here's my 
> > view on how each of those should be handled:
> > - `vector`. This function **can** return decls, not written in the 
> > source code (e.g.

[clang] e456165 - [OpenCL] Add link to C++ for OpenCL documentation

[Anastasia Stulova via cfe-commits]

Author: Anastasia Stulova
Date: 2020-01-03T12:01:03Z
New Revision: e456165f9fec9148566849f21bc4f7dda2fea034

URL: 
https://github.com/llvm/llvm-project/commit/e456165f9fec9148566849f21bc4f7dda2fea034
DIFF: 
https://github.com/llvm/llvm-project/commit/e456165f9fec9148566849f21bc4f7dda2fea034.diff

LOG: [OpenCL] Add link to C++ for OpenCL documentation

Remove description of language mode from the language
extensions and add a link to pdf document.

Tags: #clang

Differential Revision: https://reviews.llvm.org/D72076

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/UsersManual.rst

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index d9a4862dbe80..b0f57202e079 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -1631,285 +1631,6 @@ parameters of protocol-qualified type.
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
-
-OpenCL Features
-===
-
-C++ for OpenCL
---
-
-This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
-regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
-is inherited. This section describes minor 
diff erences to OpenCL C and any
-limitations related to C++ support as well as interactions between OpenCL and
-C++ features that are not documented elsewhere.
-
-Restrictions to C++17
-^
-
-The following features are not supported:
-
-- Virtual functions
-- Exceptions
-- ``dynamic_cast`` operator
-- Non-placement ``new``/``delete`` operators
-- Standard C++ libraries. Currently there is no solution for alternative C++
-  libraries provided. Future release will feature library support.
-
-
-Interplay of OpenCL and C++ features
-
-
-Address space behavior
-""
-
-Address spaces are part of the type qualifiers; many rules are just inherited
-from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
-extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally, Clang extends the existing concept
-from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using notation of sets and
-overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
-s3.1.3). For OpenCL it means that implicit conversions are allowed from
-a named address space (except for ``__constant``) to ``__generic`` (OpenCL C
-v2.0 6.5.5). Reverse conversion is only allowed explicitly. The ``__constant``
-address space does not overlap with any other and therefore no valid conversion
-between ``__constant`` and other address spaces exists. Most of the rules
-follow this logic.
-
-**Casts**
-
-C-style casts follow OpenCL C v2.0 rules (s6.5.5). All cast operators
-permit conversion to ``__generic`` implicitly. However converting from
-``__generic`` to named address spaces can only be done using 
``addrspace_cast``.
-Note that conversions between ``__constant`` and any other address space
-are disallowed.
-
-.. _opencl_cpp_addrsp_deduction:
-
-**Deduction**
-
-Address spaces are not deduced for:
-
-- non-pointer/non-reference template parameters or any dependent types except
-  for template specializations.
-- non-pointer/non-reference class members except for static data members that 
are
-  deduced to ``__global`` address space.
-- non-pointer/non-reference alias declarations.
-- ``decltype`` expressions.
-
-.. code-block:: c++
-
-  template 
-  void foo() {
-T m; // address space of m will be known at template instantiation time.
-T * ptr; // ptr points to __generic address space object.
-T & ref = ...; // ref references an object in __generic address space.
-  };
-
-  template 
-  struct S {
-int i; // i has no address space
-static int ii; // ii is in global address space
-int * ptr; // ptr points to __generic address space int.
-int & ref = ...; // ref references int in __generic address space.
-  };
-
-  template 
-  void bar()
-  {
-S s; // s is in __private address space
-  }
-
-TODO: Add example for type alias and decltype!
-
-**References**
-
-Reference types can be qualified with an address space.
-
-.. code-block:: c++
-
-  __private int & ref = ...; // references int in __private address space
-
-By default references will refer to ``__generic`` address space objects, except
-for dependent types that are not template specializations
-(see :ref:`Deduction `). Address space 
compatibility
-checks are performed when references are bound to values. The logic follows the
-rules from address space pointer conversion (OpenCL v2.0 s6.5.5).
-
-**Default address space**
-
-All non-static member functions take an implicit object parameter ``thi

[PATCH] D72076: [OpenCL] Add link to C++ for OpenCL documentation

[Anastasia Stulova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe456165f9fec: [OpenCL] Add link to C++ for OpenCL 
documentation (authored by Anastasia).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72076/new/

https://reviews.llvm.org/D72076

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/UsersManual.rst

Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -2856,6 +2856,10 @@
 
 Declaring the same types in different vendor extensions is disallowed.
 
+Clang also supports language extensions documented in `The OpenCL C Language
+Extensions Documentation
+`_.
+
 OpenCL Metadata
 ---
 
@@ -3031,8 +3035,9 @@
 `_ and
 there is no plan to support it in clang in any new releases in the near future.
 
-For detailed information about restrictions to allowed C++ features please
-refer to :doc:`LanguageExtensions`.
+For detailed information about this language refer to `The C++ for OpenCL
+Programming Language Documentation
+`_.
 
 Since C++ features are to be used on top of OpenCL C functionality, all existing
 restrictions from OpenCL C v2.0 will inherently apply. All OpenCL C builtin types
@@ -3060,6 +3065,35 @@
 
  clang -cl-std=clc++ test.cl
 
+Constructing and destroying global objects
+^^
+
+Global objects must be constructed before the first kernel using the global objects
+is executed and destroyed just after the last kernel using the program objects is
+executed. In OpenCL v2.0 drivers there is no specific API for invoking global
+constructors. However, an easy workaround would be to enqueue a constructor
+initialization kernel that has a name ``@_GLOBAL__sub_I_``.
+This kernel is only present if there are any global objects to be initialized in
+the compiled binary. One way to check this is by passing ``CL_PROGRAM_KERNEL_NAMES``
+to ``clGetProgramInfo`` (OpenCL v2.0 s5.8.7).
+
+Note that if multiple files are compiled and linked into libraries, multiple kernels
+that initialize global objects for multiple modules would have to be invoked.
+
+Applications are currently required to run initialization of global objects manually
+before running any kernels in which the objects are used.
+
+   .. code-block:: console
+
+ clang -cl-std=clc++ test.cl
+
+If there are any global objects to be initialized, the final binary will contain
+the ``@_GLOBAL__sub_I_test.cl`` kernel to be enqueued.
+
+Global destructors can not be invoked in OpenCL v2.0 drivers. However, all memory used
+for program scope objects is released on ``clReleaseProgram``.
+
+
 .. _target_features:
 
 Target-Specific Features and Limitations
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1631,285 +1631,6 @@
 Query the presence of this new mangling with
 ``__has_feature(objc_protocol_qualifier_mangling)``.
 
-
-OpenCL Features
-===
-
-C++ for OpenCL
---
-
-This functionality is built on top of OpenCL C v2.0 and C++17 enabling most of
-regular C++ features in OpenCL kernel code. Most functionality from OpenCL C
-is inherited. This section describes minor differences to OpenCL C and any
-limitations related to C++ support as well as interactions between OpenCL and
-C++ features that are not documented elsewhere.
-
-Restrictions to C++17
-^
-
-The following features are not supported:
-
-- Virtual functions
-- Exceptions
-- ``dynamic_cast`` operator
-- Non-placement ``new``/``delete`` operators
-- Standard C++ libraries. Currently there is no solution for alternative C++
-  libraries provided. Future release will feature library support.
-
-
-Interplay of OpenCL and C++ features
-
-
-Address space behavior
-""
-
-Address spaces are part of the type qualifiers; many rules are just inherited
-from the qualifier behavior documented in OpenCL C v2.0 s6.5 and Embedded C
-extension ISO/IEC JTC1 SC22 WG14 N1021 s3.1. Note that since the address space
-behavior in C++ is not documented formally, Clang extends the existing concept
-from C and OpenCL. For example conversion rules are extended from qualification
-conversion but the compatibility is determined using notation of sets and
-overlapping of address spaces from Embedded C (ISO/IEC JTC1 SC22 WG14 N1021
-s3.1.3). For OpenCL it means that implicit conversions are allowed from
-a named address space (except for ``__constant

[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 2 inline comments as done.
kadircet added inline comments.



Comment at: clang/lib/Format/Format.cpp:2381
+// if so adds a Replacement to NewReplacements that removes the entire line.
+llvm::Error MaybeRemoveBlankLine(tooling::Replacements &NewReplaces,
+ StringRef FilePath, StringRef Code,

can we rather make this function return a `tooling::Replacement` ?



Comment at: clang/lib/Format/Format.cpp:2381
+// if so adds a Replacement to NewReplacements that removes the entire line.
+llvm::Error MaybeRemoveBlankLine(tooling::Replacements &NewReplaces,
+ StringRef FilePath, StringRef Code,

kadircet wrote:
> can we rather make this function return a `tooling::Replacement` ?
name should be `maybeRemoveBlankLine`



Comment at: clang/lib/Format/Format.cpp:2383
+ StringRef FilePath, StringRef Code,
+ bool LineBlankSoFar, int LineStartPos,
+ int CheckedThroughPos) {

instead of passing `LineBlankSoFar` as a parameter, maybe don't call the 
function at all if it doesn't hold?



Comment at: clang/lib/Format/Format.cpp:2394
+assert(LineEndPos >= CheckedThroughPos);
+StringRef TrailingText(FileText + CheckedThroughPos,
+   LineEndPos - CheckedThroughPos);

```
if(!isAllWhiteSpace(Code.substr(CheckedThroughPos, LineEndPos - 
CheckedThroughPos))
  return llvm::Error::success();
```



Comment at: clang/lib/Format/Format.cpp:2397
+assert(LineEndPos >= LineStartPos);
+StringRef OriginalLine(FileText + LineStartPos, LineEndPos - LineStartPos);
+if (isAllWhitespace(TrailingText) && !isAllWhitespace(OriginalLine)) {

```
if(isAllWhiteSpace(Code.substr(LineStartPos, CheckedThroughPos - LineStartPos))
  return llvm::Error::success();
```

Also move this to the top, as it doesn't actually require any of the 
computations you performed.

I would actually keep a `HasDeleted` flag in the caller, updating it by looking 
at length of replacements, and call this function iff we've deleted something 
in that line, but up to you.



Comment at: clang/lib/Format/Format.cpp:2403
+  (CheckedThroughPos > 0 &&
+   isVerticalWhitespace(FileText[CheckedThroughPos - 1]))
+  ? (FileText + LineEndPos)

i don't follow what this check is about.

the comment talks about a replacement removing a trailing newline, but check is 
for a preceding whitespace, and it is not even at the `LineEndPos` ?
how come we get a vertical whitespace, in the middle of a line?



Comment at: clang/lib/Format/Format.cpp:2416
+// Find the beginning of the line containing StartPos.
+int FindLineStart(const char *FileText, int StartPos) {
+  int LineStartPos = StartPos;

name should be `findLineStart`



Comment at: clang/lib/Format/Format.cpp:2417
+int FindLineStart(const char *FileText, int StartPos) {
+  int LineStartPos = StartPos;
+  // When preceding character is '\n' or '\r', LineStartPos is start of line.

nit: no need to copy the parameter(i.e. creating a new variable)



Comment at: clang/lib/Format/Format.cpp:2453
+  MaybeRemoveBlankLine(NewReplaces, FilePath, Code, LineBlankSoFar,
+   LineStartPos, LineCheckedThroughPos))
+return std::move(Err);

both `LineStartPos` and `LineCheckedThroughPos` would be `-1` for the first 
replacement in the file. and you will use them to index into `Code`.

I believe it makes sense for those to be initialized to `0`.



Comment at: clang/lib/Format/Format.cpp:2466
+LineBlankSoFar = LineBlankSoFar && isAllWhitespace(PriorTextToCheck) &&
+ ReplacementText.empty();
+LineCheckedThroughPos = R.getOffset() + R.getLength();

nit: inline the variable, i.e `R.getReplacementText().empty()`



Comment at: clang/lib/Format/Format.cpp:2467
+ ReplacementText.empty();
+LineCheckedThroughPos = R.getOffset() + R.getLength();
+  }

nit: `RStartPos + R.getLength();`



Comment at: clang/lib/Format/Format.cpp:2391
+
+int CurrentRLineStartPos = RStartPos;
+while (CurrentRLineStartPos > 0 &&

poelmanc wrote:
> kadircet wrote:
> > it seems like this loop is trying to find line start, would be nice to 
> > clarify with a comment.
> > 
> > If so seems like the logic is a little buggy, for example:
> > 
> > `int a^;` if the offset is at `^`(namely at `a`) then this will return 
> > current position as line start, since `a` is preceded by a whitespace.
> > Maybe change the logic to find the first `\n` before current offse

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > sammccall wrote:
> > > > > > ilya-biryukov wrote:
> > > > > > > No need to fix this.
> > > > > > > 
> > > > > > > The name could probably be better, but we can fix this later. 
> > > > > > > Don't have any good ideas.
> > > > > > I think as a public API this should be clearer on how/when to use 
> > > > > > and its relation to other things (planning to send a patch, but 
> > > > > > wanted to discuss a bit here first).
> > > > > > 
> > > > > > - This is essentially a filter of allTargetDecls (as is 
> > > > > > targetDecl), but the relationship between the two isn't clear. They 
> > > > > > should have closely related names (variant) or maybe better be more 
> > > > > > orthogonal and composed at the callsite.
> > > > > > - The most distinctive word in the name is `explicit`, but this 
> > > > > > function doesn't actually have anything to do with explicit vs 
> > > > > > non-explicit references (just history?)
> > > > > > - It's not clear to me whether we actually want to encapsulate/hide 
> > > > > > the preference for instantiations over templates here, or whether 
> > > > > > it should be expressed at the callsite because the policy is 
> > > > > > decided feature-by-feature. `chooseBest(...)` vs 
> > > > > > `prefer(TemplateInstantiation, TemplatePattern, ...)`. The latter 
> > > > > > seems safer to me, based on experience with targetDecl so far.
> > > > > > 
> > > > > > A couple of ideas:
> > > > > >  - caller invokes allTargetDecls() and then this is a helper 
> > > > > > function `prefer(DeclRelation, DeclRelation, Results)` that mutates 
> > > > > > Results
> > > > > >  - targetDecl() becomes the swiss-army filter and accepts a list of 
> > > > > > "prefer-X-over-Y", which it applies before returning the results 
> > > > > > with flags dropped
> > > > > I don't like the idea of `targetDecl` becoming the swiss-army knife, 
> > > > > it's complicated to use as is.
> > > > > The contract of `explicitReferenceTargets` is to expose only the 
> > > > > decls written in the source code and nothing else, it's much simpler 
> > > > > to explain and to use.
> > > > > 
> > > > > It's useful for many things, essentially for anything that needs to 
> > > > > answer the question of "what does this name in the source code refers 
> > > > > to".
> > > > > 
> > > > > The name could be clearer, the comments might need to be improved.
> > > > > The only crucial improvement that I'd attempt is getting rid of the 
> > > > > `Mask` parameter, I think there was just one or two cases where it 
> > > > > was useful.
> > > > > 
> > > > > 
> > > > > I don't like the idea of targetDecl becoming the swiss-army knife
> > > > Fair enough. I'll poke more in the direction of making it easier to 
> > > > compose allTargetDecls then.
> > > > 
> > > > > The contract of explicitReferenceTargets is to expose only the decls 
> > > > > written in the source code and nothing else, it's much simpler to 
> > > > > explain and to use.
> > > > So I agree that the interface of targetDecl() is complicated, but I 
> > > > don't think this one is simpler to explain or use - or at least it 
> > > > hasn't been well-explained so far.
> > > > 
> > > > For example,
> > > >  - "to expose only the decls written in the source code" - 
> > > > `vector` isn't a decl written in the source code.
> > > >  - "find declarations explicitly referenced in the source code" - 
> > > > `return [[{foo}]];` - the class/constructor isn't explicitly 
> > > > referenced, but this function returns it.
> > > >  - "what does this name in the source code refers to" - a template name 
> > > > refers to the primary template, the (possible) partial specialization, 
> > > > and specialization all at once. Features like find refs/highlight show 
> > > > the equivalence class of names that refer to the same thing, but they 
> > > > don't prefer the specialization for that.
> > > >  - none of these explanations explains why the function is opinionated 
> > > > about template vs expansion but not about alias vs underlying.
> > > > 
> > > > > No need to fix this.
> > > > > The name could probably be better, but we can fix this later. Don't 
> > > > > have any good ideas.
> > > > 
> > > > I think it's fine (and good!) that we this got added to unblock the 
> > > > hover work and to understand more use cases for this API. But I do 
> > > > think it's time to fix it now, and I'm not convinced a better name will 
> > > > do it (I can't think of a good name to describe the current 
> > > > functionality, either). Let me put a patch together and take a look?
> > > What's the proposed fix?
> > > 
> > > I might mis

[PATCH] D72140: [clang-tools-extra] NFC: Fix trivial typos in comments

[Kazuaki Ishizaki via Phabricator via cfe-commits]

kiszk created this revision.
Herald added subscribers: cfe-commits, usaxena95, jsji, kadircet, arphaman, 
jkorous, javed.absar, kbarton, nemanjai.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72140

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-doc/BitcodeReader.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/clang-include-fixer/IncludeFixerContext.cpp
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
  clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.h
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
  clang-tools-extra/clang-tidy/utils/NamespaceAliaser.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Context.h
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/Trace.h
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/index/Symbol.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone-branch-clone.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-undelegated-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  clang-tools-extra/modularize/Modularize.cpp
  clang-tools-extra/modularize/PreprocessorTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-no-malloc-custom.cpp
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-no-malloc.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory-containers.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-arc.m
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-arc.mm
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
  
clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Index: clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
===
--- clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
+++ clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
@@ -198,8 +198,8 @@
 TEST_F(FindAllSymbolsTest, CXXRecordSymbols) {
   static const char Header[] = R"(
   struct Glob {};
-  struct A; // Not a defintion, ignored.
-  class NOP; // Not a defintion, ignored
+  struct A; // Not a definition, ignored.
+  cl

[PATCH] D72140: [clang-tools-extra] NFC: Fix trivial typos in comments

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.
Herald added a subscriber: wuzish.

{icon check-circle color=green} Unit tests: pass. 61173 tests passed, 0 failed 
and 729 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72140/new/

https://reviews.llvm.org/D72140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thank you for fixing these bugs!
Did you try running the check of clang/llvm again, to see if there are still 
missing cases of this?




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:575
 
-if (Decl->isFileVarDecl() && Type.getTypePtr()->isAnyPointerType() && 
NamingStyles[SK_GlobalPointer])
+if (Decl->isFileVarDecl() && Type.getTypePtr()->isAnyPointerType() &&
+NamingStyles[SK_GlobalPointer])

these all seem to be an artifact of running clang-format?
Such a change can be done, but should happen in a separate patch (just 
formatting does not require review).



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:742
 continue;
-  if (const auto *FD = Init->getAnyMember())
+  if (const auto *FD = Init->getAnyMember()) {
 addUsage(NamingCheckFailures, FD,

this change violates the coding guideline.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:828
 
+  if (const auto MemberRef = Result.Nodes.getNodeAs("memberExpr")) 
{
+SourceRange Range = MemberRef->getMemberNameInfo().getSourceRange();

`const auto *` to show that this variable is a pointer.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp:88
+
+  // bug report has Stack swapped to stack, this is stack swapped to Stack
+  // probably not going to cause any issue its just how

I dont understand this comment. What do you mean?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72121/new/

https://reviews.llvm.org/D72121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > sammccall wrote:
> > > > > ilya-biryukov wrote:
> > > > > > sammccall wrote:
> > > > > > > ilya-biryukov wrote:
> > > > > > > > No need to fix this.
> > > > > > > > 
> > > > > > > > The name could probably be better, but we can fix this later. 
> > > > > > > > Don't have any good ideas.
> > > > > > > I think as a public API this should be clearer on how/when to use 
> > > > > > > and its relation to other things (planning to send a patch, but 
> > > > > > > wanted to discuss a bit here first).
> > > > > > > 
> > > > > > > - This is essentially a filter of allTargetDecls (as is 
> > > > > > > targetDecl), but the relationship between the two isn't clear. 
> > > > > > > They should have closely related names (variant) or maybe better 
> > > > > > > be more orthogonal and composed at the callsite.
> > > > > > > - The most distinctive word in the name is `explicit`, but this 
> > > > > > > function doesn't actually have anything to do with explicit vs 
> > > > > > > non-explicit references (just history?)
> > > > > > > - It's not clear to me whether we actually want to 
> > > > > > > encapsulate/hide the preference for instantiations over templates 
> > > > > > > here, or whether it should be expressed at the callsite because 
> > > > > > > the policy is decided feature-by-feature. `chooseBest(...)` vs 
> > > > > > > `prefer(TemplateInstantiation, TemplatePattern, ...)`. The latter 
> > > > > > > seems safer to me, based on experience with targetDecl so far.
> > > > > > > 
> > > > > > > A couple of ideas:
> > > > > > >  - caller invokes allTargetDecls() and then this is a helper 
> > > > > > > function `prefer(DeclRelation, DeclRelation, Results)` that 
> > > > > > > mutates Results
> > > > > > >  - targetDecl() becomes the swiss-army filter and accepts a list 
> > > > > > > of "prefer-X-over-Y", which it applies before returning the 
> > > > > > > results with flags dropped
> > > > > > I don't like the idea of `targetDecl` becoming the swiss-army 
> > > > > > knife, it's complicated to use as is.
> > > > > > The contract of `explicitReferenceTargets` is to expose only the 
> > > > > > decls written in the source code and nothing else, it's much 
> > > > > > simpler to explain and to use.
> > > > > > 
> > > > > > It's useful for many things, essentially for anything that needs to 
> > > > > > answer the question of "what does this name in the source code 
> > > > > > refers to".
> > > > > > 
> > > > > > The name could be clearer, the comments might need to be improved.
> > > > > > The only crucial improvement that I'd attempt is getting rid of the 
> > > > > > `Mask` parameter, I think there was just one or two cases where it 
> > > > > > was useful.
> > > > > > 
> > > > > > 
> > > > > > I don't like the idea of targetDecl becoming the swiss-army knife
> > > > > Fair enough. I'll poke more in the direction of making it easier to 
> > > > > compose allTargetDecls then.
> > > > > 
> > > > > > The contract of explicitReferenceTargets is to expose only the 
> > > > > > decls written in the source code and nothing else, it's much 
> > > > > > simpler to explain and to use.
> > > > > So I agree that the interface of targetDecl() is complicated, but I 
> > > > > don't think this one is simpler to explain or use - or at least it 
> > > > > hasn't been well-explained so far.
> > > > > 
> > > > > For example,
> > > > >  - "to expose only the decls written in the source code" - 
> > > > > `vector` isn't a decl written in the source code.
> > > > >  - "find declarations explicitly referenced in the source code" - 
> > > > > `return [[{foo}]];` - the class/constructor isn't explicitly 
> > > > > referenced, but this function returns it.
> > > > >  - "what does this name in the source code refers to" - a template 
> > > > > name refers to the primary template, the (possible) partial 
> > > > > specialization, and specialization all at once. Features like find 
> > > > > refs/highlight show the equivalence class of names that refer to the 
> > > > > same thing, but they don't prefer the specialization for that.
> > > > >  - none of these explanations explains why the function is 
> > > > > opinionated about template vs expansion but not about alias vs 
> > > > > underlying.
> > > > > 
> > > > > > No need to fix this.
> > > > > > The name could probably be better, but we can fix this later. Don't 
> > > > > > have any good ideas.
> > > > > 
> > > > > I think it's fine (and good!) that we this got added to unblock the 
> > > > > hover work and to understand more use cases for this API. But I do 
> > > > > think it's time to fix it now, and I'm not convinced a better name 
> > > 

[PATCH] D72144: Treat C# `using` as a control statement

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe created this revision.
jbcoe added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Unless SpaceBeforeParensOptions is set to SBPO_Never, a space will be put 
between `using` and `(` in C# code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72144

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -249,6 +249,15 @@
 
   verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}",
Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter(filenameA)) {}\n"
+   "}",
+   Style);
+
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filenameB)) {}",
+   Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpRegions) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2763,7 +2763,7 @@
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
+  tok::kw_using, TT_ObjCForIn) ||
  Left.isIf(Line.Type != LT_PreprocessorDirective) ||
  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
@@ -2861,7 +2861,7 @@
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
   if (Left.is(tok::kw_using))
-return spaceRequiredBeforeParens(Left);
+return Style.SpaceBeforeParens != FormatStyle::SBPO_Never;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 if (Left.is(TT_JsFatArrow))
   return true;


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -249,6 +249,15 @@
 
   verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}",
Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter(filenameA)) {}\n"
+   "}",
+   Style);
+
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filenameB)) {}",
+   Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpRegions) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2763,7 +2763,7 @@
(Style.SpaceBeforeParens != FormatStyle::SBPO_Never &&
 (Left.isOneOf(tok::pp_elif, tok::kw_for, tok::kw_while,
   tok::kw_switch, tok::kw_case, TT_ForEachMacro,
-  TT_ObjCForIn) ||
+  tok::kw_using, TT_ObjCForIn) ||
  Left.isIf(Line.Type != LT_PreprocessorDirective) ||
  (Left.isOneOf(tok::kw_try, Keywords.kw___except, tok::kw_catch,
tok::kw_new, tok::kw_delete) &&
@@ -2861,7 +2861,7 @@
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
   if (Left.is(tok::kw_using))
-return spaceRequiredBeforeParens(Left);
+return Style.SpaceBeforeParens != FormatStyle::SBPO_Never;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 if (Left.is(TT_JsFatArrow))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72144: Treat C# `using` as a control statement

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe updated this revision to Diff 236035.
jbcoe added a reviewer: klimek.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72144/new/

https://reviews.llvm.org/D72144

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -249,6 +249,15 @@
 
   verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}",
Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter(filenameA)) {}\n"
+   "}",
+   Style);
+
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filenameB)) {}",
+   Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpRegions) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2861,7 +2861,7 @@
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
   if (Left.is(tok::kw_using))
-return spaceRequiredBeforeParens(Left);
+return Style.SpaceBeforeParens != FormatStyle::SBPO_Never;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 if (Left.is(TT_JsFatArrow))
   return true;


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -249,6 +249,15 @@
 
   verifyFormat("using(StreamWriter sw = new StreamWriter(filenameB)) {}",
Style);
+
+  Style.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  verifyFormat("public void foo() {\n"
+   "  using (StreamWriter sw = new StreamWriter(filenameA)) {}\n"
+   "}",
+   Style);
+
+  verifyFormat("using (StreamWriter sw = new StreamWriter(filenameB)) {}",
+   Style);
 }
 
 TEST_F(FormatTestCSharp, CSharpRegions) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2861,7 +2861,7 @@
 // space between keywords and paren e.g. "using ("
 if (Right.is(tok::l_paren))
   if (Left.is(tok::kw_using))
-return spaceRequiredBeforeParens(Left);
+return Style.SpaceBeforeParens != FormatStyle::SBPO_Never;
   } else if (Style.Language == FormatStyle::LK_JavaScript) {
 if (Left.is(TT_JsFatArrow))
   return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:86
+// Add macro references.
+for (const auto &IDToRefs : MacroRefsToIndex->MacroRefs) {
+  for (const auto &Range : IDToRefs.second) {

ilya-biryukov wrote:
> hokein wrote:
> > ilya-biryukov wrote:
> > > This is trying to emulate existing logic in `SymbolCollector::finish`. Is 
> > > there a way we could share this?
> > > Would avoid creating extra copies of reference slabs and allow to keep 
> > > the code in one place, rather than scattering it between `FileIndex.cpp` 
> > > and `SymbolCollector.cpp`. Would also allow to keep `toURI` private, 
> > > meaning we don't have to worry about naming it and the fact it's exposed 
> > > in the public interface.
> > > 
> > > One potential way to do this is to have an alternative version of 
> > > `handleMacroOccurence`, which would fill `SymbolCollector::MacroRefs` 
> > > directly and call this right after `indexTopLevelDecls`.
> > > Would that work?
> > +1 on the concern about performance (this is a hot function, we are paying 
> > an extra cost of copying all refs, which should be avoided), and the 
> > layering of `toURI`.
> > 
> > > One potential way to do this is to have an alternative version of 
> > > handleMacroOccurence, which would fill SymbolCollector::MacroRefs 
> > > directly and call this right after indexTopLevelDecls.
> > > Would that work?
> > 
> > the main problem is that at this point (parsing is finished), preprocessor 
> > callback is not available, so we won't see macro references 
> > (`SymbolCollector::handleMacroOccurence` will only receive macro 
> > definitions).
> > 
> > I think two options:
> > - as mentioned above, add alternative version of `handleMacroOccurence` in 
> > SymbolCollector, calling it **before** `indexTopLevelDecls` (because 
> > `finish` is called in `indexTopLevelDecls` which we have no way to 
> > customize);
> > - or we could add a finish callback to the `SymbolCollector` which is 
> > called in `SymbolCollector::finish`, and put this logic to the callback.
> > 
> Thanks, good point, in my proposal we should populate macro references before 
> calling `indexTopLevelDecls`.
> 
> I would still suggest going with the first option, it seems simpler to me. 
> But up to you, @usaxena95.
This looks much better now. Thanks for your inputs Haojian and Ilya.
I have added another version of `handleMacroOccurence` to add the 
`MainFileMacros` to the references. 

> add alternative version of handleMacroOccurence in SymbolCollector, calling 
> it before `indexTopLevelDecls`

@hokein 
SymbolCollector doesn't build the slabs in the `finish` so I think it is still 
fine to call it after `indexTopLevelDecls`. But doing this before `finish` 
makes more sense semantically and reasonable.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71406/new/

https://reviews.llvm.org/D71406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 236037.
usaxena95 marked 5 inline comments as done.
usaxena95 added a comment.

Added another version of `handleMacroOccurence` to handle MacroReferences from 
main file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71406/new/

https://reviews.llvm.org/D71406

Files:
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -345,6 +345,41 @@
  FileURI("unittest:///test2.cc"))}));
 }
 
+TEST(FileIndexTest, MacroRefs) {
+  Annotations HeaderCode(R"cpp(
+#define $macro[[MACRO]](X) (X+1)
+  )cpp");
+  Annotations MainCode(R"cpp(
+  void f() {
+int a = $macro[[MACRO]](1);
+  }
+  )cpp");
+
+  auto Macro = findSymbol(
+  TestTU::withHeaderCode(HeaderCode.code()).headerSymbols(), "MACRO");
+  FileIndex Index;
+  // Add test.cc
+  TestTU Test;
+  Test.HeaderCode = HeaderCode.code();
+  Test.Code = MainCode.code();
+  Test.Filename = "test.cc";
+  auto AST = Test.build();
+  Index.updateMain(Test.Filename, AST);
+  // Add test2.cc
+  TestTU Test2;
+  Test2.HeaderCode = HeaderCode.code();
+  Test2.Code = MainCode.code();
+  Test2.Filename = "test2.cc";
+  AST = Test2.build();
+  Index.updateMain(Test2.Filename, AST);
+
+  EXPECT_THAT(getRefs(Index, Macro.ID),
+  RefsAre({AllOf(RefRange(MainCode.range("macro")),
+ FileURI("unittest:///test.cc")),
+   AllOf(RefRange(MainCode.range("macro")),
+ FileURI("unittest:///test2.cc"))}));
+}
+
 TEST(FileIndexTest, CollectMacros) {
   FileIndex M;
   update(M, "f", "#define CLANGD 1");
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -9,6 +9,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOL_COLLECTOR_H
 
 #include "CanonicalIncludes.h"
+#include "CollectMacros.h"
 #include "Index.h"
 #include "SymbolOrigin.h"
 #include "clang/AST/ASTContext.h"
@@ -104,6 +105,8 @@
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override;
 
+  void handleMacroOccurence(const MainFileMacros &MacroRefsToIndex);
+
   bool handleMacroOccurence(const IdentifierInfo *Name, const MacroInfo *MI,
 index::SymbolRoleSet Roles,
 SourceLocation Loc) override;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -342,6 +342,31 @@
   return true;
 }
 
+void SymbolCollector::handleMacroOccurence(
+const MainFileMacros &MacroRefsToIndex) {
+  assert(PP.get());
+  const auto &SM = PP->getSourceManager();
+  const auto *MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
+  if (!MainFileEntry)
+return;
+
+  const auto MainFileURI = toURI(SM, MainFileEntry->getName(), Opts);
+  // Add macro references.
+  for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) {
+for (const auto &Range : IDToRefs.second) {
+  Ref R;
+  R.Location.Start.setLine(Range.start.line);
+  R.Location.Start.setColumn(Range.start.character);
+  R.Location.End.setLine(Range.end.line);
+  R.Location.End.setColumn(Range.end.character);
+  R.Location.FileURI = MainFileURI.c_str();
+  // FIXME: Add correct RefKind information to MainFileMacros.
+  R.Kind = RefKind::Reference;
+  Refs.insert(IDToRefs.first, R);
+}
+  }
+}
+
 bool SymbolCollector::handleMacroOccurence(const IdentifierInfo *Name,
const MacroInfo *MI,
index::SymbolRoleSet Roles,
Index: clang-tools-extra/clangd/index/FileIndex.h
===
--- clang-tools-extra/clangd/index/FileIndex.h
+++ clang-tools-extra/clangd/index/FileIndex.h
@@ -140,7 +140,7 @@
 /// Exposed to assist in unit tests.
 SlabTuple indexMainDecls(ParsedAST &AST);
 
-/// Idex declarations from \p AST and macros from \p PP that are declared in
+/// Index declarations from \p AST and macros from \p PP that are declared in
 /// included headers.
 SlabTuple indexHeaderSymbols(ASTContext &AST, std::shared_ptr PP,
  const CanonicalIncludes &Includes);
Index: cla

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > sammccall wrote:
> > > > > > ilya-biryukov wrote:
> > > > > > > sammccall wrote:
> > > > > > > > ilya-biryukov wrote:
> > > > > > > > > No need to fix this.
> > > > > > > > > 
> > > > > > > > > The name could probably be better, but we can fix this later. 
> > > > > > > > > Don't have any good ideas.
> > > > > > > > I think as a public API this should be clearer on how/when to 
> > > > > > > > use and its relation to other things (planning to send a patch, 
> > > > > > > > but wanted to discuss a bit here first).
> > > > > > > > 
> > > > > > > > - This is essentially a filter of allTargetDecls (as is 
> > > > > > > > targetDecl), but the relationship between the two isn't clear. 
> > > > > > > > They should have closely related names (variant) or maybe 
> > > > > > > > better be more orthogonal and composed at the callsite.
> > > > > > > > - The most distinctive word in the name is `explicit`, but this 
> > > > > > > > function doesn't actually have anything to do with explicit vs 
> > > > > > > > non-explicit references (just history?)
> > > > > > > > - It's not clear to me whether we actually want to 
> > > > > > > > encapsulate/hide the preference for instantiations over 
> > > > > > > > templates here, or whether it should be expressed at the 
> > > > > > > > callsite because the policy is decided feature-by-feature. 
> > > > > > > > `chooseBest(...)` vs `prefer(TemplateInstantiation, 
> > > > > > > > TemplatePattern, ...)`. The latter seems safer to me, based on 
> > > > > > > > experience with targetDecl so far.
> > > > > > > > 
> > > > > > > > A couple of ideas:
> > > > > > > >  - caller invokes allTargetDecls() and then this is a helper 
> > > > > > > > function `prefer(DeclRelation, DeclRelation, Results)` that 
> > > > > > > > mutates Results
> > > > > > > >  - targetDecl() becomes the swiss-army filter and accepts a 
> > > > > > > > list of "prefer-X-over-Y", which it applies before returning 
> > > > > > > > the results with flags dropped
> > > > > > > I don't like the idea of `targetDecl` becoming the swiss-army 
> > > > > > > knife, it's complicated to use as is.
> > > > > > > The contract of `explicitReferenceTargets` is to expose only the 
> > > > > > > decls written in the source code and nothing else, it's much 
> > > > > > > simpler to explain and to use.
> > > > > > > 
> > > > > > > It's useful for many things, essentially for anything that needs 
> > > > > > > to answer the question of "what does this name in the source code 
> > > > > > > refers to".
> > > > > > > 
> > > > > > > The name could be clearer, the comments might need to be improved.
> > > > > > > The only crucial improvement that I'd attempt is getting rid of 
> > > > > > > the `Mask` parameter, I think there was just one or two cases 
> > > > > > > where it was useful.
> > > > > > > 
> > > > > > > 
> > > > > > > I don't like the idea of targetDecl becoming the swiss-army knife
> > > > > > Fair enough. I'll poke more in the direction of making it easier to 
> > > > > > compose allTargetDecls then.
> > > > > > 
> > > > > > > The contract of explicitReferenceTargets is to expose only the 
> > > > > > > decls written in the source code and nothing else, it's much 
> > > > > > > simpler to explain and to use.
> > > > > > So I agree that the interface of targetDecl() is complicated, but I 
> > > > > > don't think this one is simpler to explain or use - or at least it 
> > > > > > hasn't been well-explained so far.
> > > > > > 
> > > > > > For example,
> > > > > >  - "to expose only the decls written in the source code" - 
> > > > > > `vector` isn't a decl written in the source code.
> > > > > >  - "find declarations explicitly referenced in the source code" - 
> > > > > > `return [[{foo}]];` - the class/constructor isn't explicitly 
> > > > > > referenced, but this function returns it.
> > > > > >  - "what does this name in the source code refers to" - a template 
> > > > > > name refers to the primary template, the (possible) partial 
> > > > > > specialization, and specialization all at once. Features like find 
> > > > > > refs/highlight show the equivalence class of names that refer to 
> > > > > > the same thing, but they don't prefer the specialization for that.
> > > > > >  - none of these explanations explains why the function is 
> > > > > > opinionated about template vs expansion but not about alias vs 
> > > > > > underlying.
> > > > > > 
> > > > > > > No need to fix this.
> > > > > > > The name could probably be better, but we can fix this later. 
> > > > > > > Don't have any good ideas.
> > > > > > 
> > > > > > I think it's fine (and good!)

[PATCH] D72150: Allow space after C-style cast in C# code

[Jonathan B Coe via Phabricator via cfe-commits]

jbcoe created this revision.
jbcoe added a reviewer: klimek.
jbcoe added a project: clang-format.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72150

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTestCSharp.cpp


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -374,5 +374,14 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, CSharpSpaceAfterCStyleCast) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  
+  verifyFormat("(int)x / y;", Style);
+  
+  Style.SpaceAfterCStyleCast = true; 
+  verifyFormat("(int) x / y;", Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1640,8 +1640,9 @@
 
   /// Determine whether ')' is ending a cast.
   bool rParenEndsCast(const FormatToken &Tok) {
-// C-style casts are only used in C++ and Java.
-if (!Style.isCpp() && Style.Language != FormatStyle::LK_Java)
+// C-style casts are only used in C++, C# and Java.
+if (!Style.isCSharp() && !Style.isCpp() && 
+Style.Language != FormatStyle::LK_Java)
   return false;
 
 // Empty parens aren't casts and there are no casts at the end of the line.


Index: clang/unittests/Format/FormatTestCSharp.cpp
===
--- clang/unittests/Format/FormatTestCSharp.cpp
+++ clang/unittests/Format/FormatTestCSharp.cpp
@@ -374,5 +374,14 @@
Style);
 }
 
+TEST_F(FormatTestCSharp, CSharpSpaceAfterCStyleCast) {
+  FormatStyle Style = getGoogleStyle(FormatStyle::LK_CSharp);
+  
+  verifyFormat("(int)x / y;", Style);
+  
+  Style.SpaceAfterCStyleCast = true; 
+  verifyFormat("(int) x / y;", Style);
+}
+
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1640,8 +1640,9 @@
 
   /// Determine whether ')' is ending a cast.
   bool rParenEndsCast(const FormatToken &Tok) {
-// C-style casts are only used in C++ and Java.
-if (!Style.isCpp() && Style.Language != FormatStyle::LK_Java)
+// C-style casts are only used in C++, C# and Java.
+if (!Style.isCSharp() && !Style.isCpp() && 
+Style.Language != FormatStyle::LK_Java)
   return false;
 
 // Empty parens aren't casts and there are no casts at the end of the line.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60496 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71406/new/

https://reviews.llvm.org/D71406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think this is alright, I want @ctopper to take a look before I approve it 
though.  Additionally, do you know if this modifies the regcall calling 
convention at all?  Should it?




Comment at: clang/test/CodeGenCXX/inalloca-vector.cpp:71
+// CHECK-LABEL: define dso_local x86_vectorcallcc void 
@"?vectorcall_receive_vec@@Y{{[^"]*}}"
+// CHECKX-SAME: (double inreg %xmm0,
+// CHECKX-SAME: double inreg %xmm1,

Are all the checks hre on out disabled for a reason?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72114/new/

https://reviews.llvm.org/D72114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I don't see anything I have a problem with, but still want @ctopper to have a 
bit of time to take a look.  Ping me monday on both if he doesn't respond.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72110/new/

https://reviews.llvm.org/D72110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr.
Herald added a project: clang.

This patch adds `getAssociatedRange` which, for a given decl, computes preceding
and trailing text that would conceptually be associated with the decl by the
reader. This includes comments, whitespace, and separators like ';'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72153

Files:
  clang/include/clang/Tooling/Transformer/SourceCode.h
  clang/lib/Tooling/Transformer/SourceCode.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -17,6 +17,8 @@
 using namespace clang;
 
 using llvm::ValueIs;
+using testing::Eq;
+using tooling::getAssociatedRange;
 using tooling::getExtendedText;
 using tooling::getRangeForEdit;
 using tooling::getText;
@@ -41,12 +43,43 @@
   std::function OnCall;
 };
 
+struct DeclaratorDeclsVisitor : TestVisitor {
+  bool VisitDeclaratorDecl(DeclaratorDecl *Decl) {
+OnDecl(Decl, Context);
+return true;
+  }
+
+  std::function OnDecl;
+};
+
 // Equality matcher for `clang::CharSourceRange`, which lacks `operator==`.
 MATCHER_P(EqualsRange, R, "") {
   return arg.isTokenRange() == R.isTokenRange() &&
  arg.getBegin() == R.getBegin() && arg.getEnd() == R.getEnd();
 }
 
+MATCHER_P2(EqualsAnnotatedRange, SM, R, "") {
+  if (arg.getBegin().isMacroID()) {
+*result_listener << "which starts in a macro";
+return false;
+  }
+  if (arg.getEnd().isMacroID()) {
+*result_listener << "which ends in a macro";
+return false;
+  }
+
+  unsigned Begin = SM->getFileOffset(arg.getBegin());
+  unsigned End = SM->getFileOffset(arg.getEnd());
+
+  *result_listener << "which is [" << Begin << ",";
+  if (arg.isTokenRange()) {
+*result_listener << End << "]";
+return Begin == R.Begin && End + 1 == R.End;
+  }
+  *result_listener << End << ")";
+  return Begin == R.Begin && End == R.End;
+}
+
 static ::testing::Matcher AsRange(const SourceManager &SM,
llvm::Annotations::Range R) {
   return EqualsRange(CharSourceRange::getCharRange(
@@ -122,6 +155,95 @@
   Visitor.runOver("int foo() { return foo() + 3; }");
 }
 
+TEST(SourceCodeTest, getAssociatedRange) {
+  struct DeclaratorDeclsVisitor : TestVisitor {
+llvm::Annotations Code;
+
+DeclaratorDeclsVisitor() : Code("$r[[]]") {}
+bool VisitDeclaratorDecl(DeclaratorDecl *Decl) {
+  EXPECT_THAT(
+  getAssociatedRange(*Decl, *Context),
+  EqualsAnnotatedRange(&Context->getSourceManager(), Code.range("r")))
+  << Code.code();
+  return true;
+}
+bool runOverWithComments(StringRef Code) {
+  std::vector Args = {"-std=c++11", "-fparse-all-comments"};
+  return tooling::runToolOnCodeWithArgs(CreateTestAction(), Code, Args);
+}
+  };
+
+  DeclaratorDeclsVisitor Visitor;
+
+  // Includes newline.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4;]]");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Includes newline and semicolon.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4;\n]]");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Includes trailing comments.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4; // Comment\n]]");
+  Visitor.runOver(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations("$r[[int x = 4; /* Comment */\n]]");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Does *not* include trailing comments when another entity appears between
+  // the decl and the comment.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4;]] class C {}; // Comment\n");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Includes leading comments.
+  Visitor.Code = llvm::Annotations("$r[[// Comment.\nint x = 4;]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations("$r[[// Comment.\nint x = 4;\n]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations("$r[[/* Comment.*/\nint x = 4;\n]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  // ... even when separated by multiple empty lines.
+  Visitor.Code = llvm::Annotations("$r[[// Comment.\n\n\nint x = 4;\n]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+
+  // Includes  multi-line comments.
+  Visitor.Code = llvm::Annotations(R"cpp(
+  $r[[/* multi
+   * line
+   * comment
+   */
+  int x;]])cpp");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations(R"cpp(
+  $r[[// multi
+  // line
+  // comment
+  int x;]])cpp");
+  Visitor.runOverWithComments(Visitor.Code.code());
+
+  // Does not include comments before a *series* of declarations.
+  Visitor.Code = llvm::Annotations("// Comment.\n$r[[int x = 4;\n]]class foo {};\n");
+  Visitor.runOverWithComments(Visitor.Code.code());
+

[PATCH] D72153: [libTooling] Add function to determine associated text of a declaration.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 236057.
ymandel added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72153/new/

https://reviews.llvm.org/D72153

Files:
  clang/include/clang/Tooling/Transformer/SourceCode.h
  clang/lib/Tooling/Transformer/SourceCode.cpp
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -17,6 +17,8 @@
 using namespace clang;
 
 using llvm::ValueIs;
+using testing::Eq;
+using tooling::getAssociatedRange;
 using tooling::getExtendedText;
 using tooling::getRangeForEdit;
 using tooling::getText;
@@ -41,12 +43,43 @@
   std::function OnCall;
 };
 
+struct DeclaratorDeclsVisitor : TestVisitor {
+  bool VisitDeclaratorDecl(DeclaratorDecl *Decl) {
+OnDecl(Decl, Context);
+return true;
+  }
+
+  std::function OnDecl;
+};
+
 // Equality matcher for `clang::CharSourceRange`, which lacks `operator==`.
 MATCHER_P(EqualsRange, R, "") {
   return arg.isTokenRange() == R.isTokenRange() &&
  arg.getBegin() == R.getBegin() && arg.getEnd() == R.getEnd();
 }
 
+MATCHER_P2(EqualsAnnotatedRange, SM, R, "") {
+  if (arg.getBegin().isMacroID()) {
+*result_listener << "which starts in a macro";
+return false;
+  }
+  if (arg.getEnd().isMacroID()) {
+*result_listener << "which ends in a macro";
+return false;
+  }
+
+  unsigned Begin = SM->getFileOffset(arg.getBegin());
+  unsigned End = SM->getFileOffset(arg.getEnd());
+
+  *result_listener << "which is [" << Begin << ",";
+  if (arg.isTokenRange()) {
+*result_listener << End << "]";
+return Begin == R.Begin && End + 1 == R.End;
+  }
+  *result_listener << End << ")";
+  return Begin == R.Begin && End == R.End;
+}
+
 static ::testing::Matcher AsRange(const SourceManager &SM,
llvm::Annotations::Range R) {
   return EqualsRange(CharSourceRange::getCharRange(
@@ -122,6 +155,96 @@
   Visitor.runOver("int foo() { return foo() + 3; }");
 }
 
+TEST(SourceCodeTest, getAssociatedRange) {
+  struct DeclaratorDeclsVisitor : TestVisitor {
+llvm::Annotations Code;
+
+DeclaratorDeclsVisitor() : Code("$r[[]]") {}
+bool VisitDeclaratorDecl(DeclaratorDecl *Decl) {
+  EXPECT_THAT(
+  getAssociatedRange(*Decl, *Context),
+  EqualsAnnotatedRange(&Context->getSourceManager(), Code.range("r")))
+  << Code.code();
+  return true;
+}
+bool runOverWithComments(StringRef Code) {
+  std::vector Args = {"-std=c++11", "-fparse-all-comments"};
+  return tooling::runToolOnCodeWithArgs(CreateTestAction(), Code, Args);
+}
+  };
+
+  DeclaratorDeclsVisitor Visitor;
+
+  // Includes newline.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4;]]");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Includes newline and semicolon.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4;\n]]");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Includes trailing comments.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4; // Comment\n]]");
+  Visitor.runOver(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations("$r[[int x = 4; /* Comment */\n]]");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Does *not* include trailing comments when another entity appears between
+  // the decl and the comment.
+  Visitor.Code = llvm::Annotations("$r[[int x = 4;]] class C {}; // Comment\n");
+  Visitor.runOver(Visitor.Code.code());
+
+  // Includes leading comments.
+  Visitor.Code = llvm::Annotations("$r[[// Comment.\nint x = 4;]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations("$r[[// Comment.\nint x = 4;\n]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations("$r[[/* Comment.*/\nint x = 4;\n]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  // ... even when separated by multiple empty lines.
+  Visitor.Code = llvm::Annotations("$r[[// Comment.\n\n\nint x = 4;\n]]");
+  Visitor.runOverWithComments(Visitor.Code.code());
+
+  // Includes  multi-line comments.
+  Visitor.Code = llvm::Annotations(R"cpp(
+  $r[[/* multi
+   * line
+   * comment
+   */
+  int x;]])cpp");
+  Visitor.runOverWithComments(Visitor.Code.code());
+  Visitor.Code = llvm::Annotations(R"cpp(
+  $r[[// multi
+  // line
+  // comment
+  int x;]])cpp");
+  Visitor.runOverWithComments(Visitor.Code.code());
+
+  // Does not include comments before a *series* of declarations.
+  Visitor.Code =
+  llvm::Annotations("// Comment.\n$r[[int x = 4;\n]]class foo {};\n");
+  Visitor.runOverWithComments(Visitor.Code.code());
+
+  // Includes attributes.
+  Visitor.Code = llvm::Annotations(R"cpp(
+  #define ATTR __attribute__((deprecated("message")))
+  $r[[ATTR
+  int x;]])cpp");
+

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D71499#1802134 , @lebedev.ri wrote:

> For future reference, if anyone needs here's the C versions of these builtins:
>  https://godbolt.org/z/oHeAh-
>
> ^ looks like we are missing some backend-level folds for round-down variant, 
> filed https://bugs.llvm.org/show_bug.cgi?id=8


Appled some backend codegen fixes (8dab0a4a7d691f2704f1079538e0ef29548db159 
, 
3d492d7503d197246115eb38e7b1b61143d0c99f 
, 
0727e2b90c7b11d5c6be55919c443628d8e2bc6e 
),
`__builtin_align_down()` codegen should now be as optimal as 
`__builtin_align_up()` is. (at least on X86)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71499/new/

https://reviews.llvm.org/D71499



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Still LG with a small nit in one of the FIXME tests.




Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1006
+}
+
+} // namespace test

JonasToth wrote:
> aaron.ballman wrote:
> > JonasToth wrote:
> > > aaron.ballman wrote:
> > > > Can you also add some ObjC pointer tests?
> > > i added the most basic test, do you think more is necessary? I don't know 
> > > a lot about the objc languages.
> > Ah, those are regular C pointers, not ObjC pointers. I was thinking more 
> > along the lines of:
> > ```
> > @class Object;
> > Object *g; // Try adding const to this
> > 
> > @interface Inter
> > - (void) foo1: (int *)arg1; // Try adding const to this parameter
> > @end
> > ```
> Well, those don't work well.
> Can i still commit? :) FIXMEs are there.
> Can i still commit? :) FIXMEs are there.

Yes, I think you've still made good progress with the patch as-is. We can 
correct the ObjC stuff in a follow-up.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1055
+  EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),
+Cat("Object  const*target;"));
+  EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),

The whitespace looks incorrect here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 236058.
Tyker added a comment.

This update should fixes the issue.
the assert was incorrect. because file offsets are 0-based where as column 
numbers are 1-based.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71037/new/

https://reviews.llvm.org/D71037

Files:
  clang/lib/Parse/ParseStmt.cpp
  clang/test/Parser/warn-misleading-indentation.cpp

Index: clang/test/Parser/warn-misleading-indentation.cpp
===
--- clang/test/Parser/warn-misleading-indentation.cpp
+++ clang/test/Parser/warn-misleading-indentation.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -x c -fsyntax-only -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation -DWITH_WARN %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN -DCXX17 %s
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -Wno-misleading-indentation -DCXX17 %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation -DWITH_WARN -ftabstop 8 -DTAB_SIZE=8 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN  -ftabstop 4 -DTAB_SIZE=4 -DCXX17 %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN -ftabstop 1 -DTAB_SIZE=1 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -Wmisleading-indentation -DCXX17 -DWITH_WARN -ftabstop 2 -DTAB_SIZE=2 %s
 
 #ifndef WITH_WARN
 // expected-no-diagnostics
@@ -225,3 +227,81 @@
 // expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
 #endif
 }
+int a4()
+{
+	if (0)
+		return 1;
+ 	return 0;
+#if (TAB_SIZE == 1)
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+// expected-note@-5 {{here}}
+#endif 
+}
+
+int a5()
+{
+	if (0)
+		return 1;
+		return 0;
+#if WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+// expected-note@-5 {{here}}
+#endif
+}
+
+int a6()
+{
+	if (0)
+		return 1;
+  		return 0;
+#if (TAB_SIZE == 8)
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+// expected-note@-5 {{here}}
+#endif
+}
+
+#define FOO \
+ goto fail
+
+int main(int argc, char* argv[]) {
+  if (5 != 0)
+goto fail;
+  else
+goto fail;
+
+  if (1) {
+if (1)
+  goto fail;
+else if (1)
+  goto fail;
+else if (1)
+  goto fail;
+else
+  goto fail;
+  } else if (1) {
+if (1)
+  goto fail;
+  }
+
+  if (1) {
+if (1)
+  goto fail;
+  } else if (1)
+goto fail;
+
+
+  if (1) goto fail; goto fail;
+
+if (0)
+goto fail;
+
+goto fail;
+
+if (0)
+FOO;
+
+goto fail;
+
+fail:;
+}
+
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1214,6 +1214,42 @@
 if (Kind == MSK_else && !ShouldSkip)
   P.MisleadingIndentationElseLoc = SL;
   }
+
+  /// Compute the column number will aligning tabs on TabStop (-ftabstop), this
+  /// gives the visual indentation of the SourceLocation.
+  static unsigned getVisualIndentation(SourceManager &SM, SourceLocation Loc) {
+unsigned TabStop = SM.getDiagnostics().getDiagnosticOptions().TabStop;
+
+unsigned ColNo = SM.getSpellingColumnNumber(Loc);
+if (ColNo == 0 || TabStop == 1)
+  return ColNo;
+
+std::pair FIDAndOffset = SM.getDecomposedLoc(Loc);
+
+bool Invalid;
+StringRef BufData = SM.getBufferData(FIDAndOffset.first, &Invalid);
+if (Invalid)
+  return 0;
+
+const char *EndPos = BufData.data() + FIDAndOffset.second;
+// FileOffset are 0-based and Column numbers are 1-based
+assert(FIDAndOffset.second + 1 >= ColNo &&
+   "Column number smaller than file offset?");
+
+unsigned VisualColumn = 0; // Stored as 0-based column, here.
+// Loop from beginning of line up to Loc's file position, counting columns,
+// expanding tabs.
+for (const char *CurPos = EndPos - (ColNo - 1); CurPos != EndPos;
+ ++CurPos) {
+  if (*CurPos == '\t')
+// Advance visual column to next tabstop.
+VisualColumn += (TabStop - VisualColumn % TabStop);
+  else
+VisualColumn++;
+}
+return VisualColumn + 1;
+  }
+
   void Check() {
 Token Tok = P.getCurToken();
 if (P.getActions().getDiagnostics().isIgnored(
@@ -1230,9 +1266,9 @@
   P.MisleadingIndentationElseLoc = SourceLocation();
 
 SourceManager &SM = P.getPreprocessor().getSourceManager();
-unsigned PrevColNum = SM.getSpellingColumnNumber(PrevLoc);
-unsigned CurColNum = SM.getSpellingColumnNumber(Tok.getLocation());
-unsigned StmtColNum = SM.getSpellingColumnNumber(StmtLoc);
+unsigned PrevColNum = getVisualIndentation(SM, PrevLoc);
+unsigned CurColNum = getVisualIndentation(SM, Tok.getLocation());
+

[PATCH] D72057: [docs] Remove outdated svn/git information from hacking page

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with a small typo fix. Do you need someone to commit this on your behalf?




Comment at: clang/www/hacking.html:279
+  To contribute changes to Clang see
+https://llvm.org/docs/GettingStarted.html#sending-patches";>LLVM's 
Getting started page
 

Getting started -> Getting Started (or, alternatively, getting started)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72057/new/

https://reviews.llvm.org/D72057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D72018#1802636 , @NoQ wrote:

> Would changing the literal in the attribute have the same effect? I.e., 
> `acquire_handle("Fuchsia_But_Please_Ignore_Me")`.


It should, but doesn't currently because we don't have any checking that the 
string literal matches a name in the static analyzer or clang-tidy for that 
attribute.

In D72018#1801739 , @xazax.hun wrote:

> I think if there are many problems with this concept, I will fall back to 
> hard code this information in the checker instead of using an annotation.


How much hard coded information would this remove? How often do you find that 
users want to add to the list of hard coded functions themselves but can't? 
Maybe the benefits to the attribute outweigh the downside and it is worth 
exploring ways to resolve some of these concerns -- I don't have a good feeling 
for the problem space though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72018/new/

https://reviews.llvm.org/D72018



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72140: [clang-tools-extra] NFC: Fix trivial typos in comments

[Kazuaki Ishizaki via Phabricator via cfe-commits]

kiszk updated this revision to Diff 236069.
kiszk added a comment.

Rebase with master


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72140/new/

https://reviews.llvm.org/D72140

Files:
  clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
  clang-tools-extra/clang-doc/BitcodeReader.h
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/clang-include-fixer/IncludeFixerContext.cpp
  clang-tools-extra/clang-include-fixer/tool/clang-include-fixer.el
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
  clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ForwardDeclarationNamespaceCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringLiteralWithEmbeddedNulCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SlicingCheck.cpp
  clang-tools-extra/clang-tidy/google/GlobalNamesInHeadersCheck.cpp
  clang-tools-extra/clang-tidy/google/IntegerTypesCheck.h
  clang-tools-extra/clang-tidy/modernize/AvoidCArraysCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang-tools-extra/clang-tidy/performance/InefficientAlgorithmCheck.h
  clang-tools-extra/clang-tidy/performance/InefficientVectorOperationCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.cpp
  clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.h
  clang-tools-extra/clang-tidy/utils/NamespaceAliaser.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Context.h
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/Trace.h
  clang-tools-extra/clangd/clients/clangd-vscode/src/extension.ts
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/index/Symbol.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang-tools-extra/clangd/unittests/SymbolInfoTests.cpp
  clang-tools-extra/clangd/unittests/SyncAPI.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/TweakTests.cpp
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang-tools-extra/docs/clang-tidy/checks/bugprone-branch-clone.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-mem57-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/hicpp-undelegated-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  clang-tools-extra/modularize/Modularize.cpp
  clang-tools-extra/modularize/PreprocessorTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-no-malloc-custom.cpp
  clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-no-malloc.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory-containers.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-make-unique.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-nullptr-basic.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-arc.m
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-value-param-arc.mm
  
clang-tools-extra/test/clang-tidy/checkers/readability-redundant-declaration.cpp
  
clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp

Index: clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
===
--- clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
+++ clang-tools-extra/unittests/clang-include-fixer/find-all-symbols/FindAllSymbolsTests.cpp
@@ -198,8 +198,8 @@
 TEST_F(FindAllSymbolsTest, CXXRecordSymbols) {
   static const char Header[] = R"(
   struct Glob {};
-  struct A; // Not a defintion, ignored.
-  class NOP; // Not a defintion, ignored
+  struct A; // Not a definition, ignored.
+  class NOP; // Not a defini

[PATCH] D71510: [clang][checkers] Added new checker 'error-return-checker'. (WIP)

[Balázs Kéri via Phabricator via cfe-commits]

balazske updated this revision to Diff 236070.
balazske added a comment.

- Implemented all (now possible) functions.
- Moved ParmVal value to own state map.
- Improved state data and bug reporting.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71510/new/

https://reviews.llvm.org/D71510

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/error-return.c

Index: clang/test/Analysis/error-return.c
===
--- /dev/null
+++ clang/test/Analysis/error-return.c
@@ -0,0 +1,578 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.unix.ErrorReturn -verify %s
+
+#include "Inputs/system-header-simulator.h"
+
+/*
+Functions from CERT ERR33-C that should be checked for error:
+
+void *aligned_alloc( size_t alignment, size_t size );
+errno_t asctime_s(char *buf, rsize_t bufsz, const struct tm *time_ptr);
+int at_quick_exit( void (*func)(void) );
+int atexit( void (*func)(void) );
+void* bsearch( const void *key, const void *ptr, size_t count, size_t size,
+   int (*comp)(const void*, const void*) );
+void* bsearch_s( const void *key, const void *ptr, rsize_t count, rsize_t size,
+ int (*comp)(const void *, const void *, void *),
+ void *context );
+wint_t btowc( int c );
+size_t c16rtomb( char * restrict s, char16_t c16, mbstate_t * restrict ps );
+size_t c32rtomb( char * restrict s, char32_t c32, mbstate_t * restrict ps );
+void* calloc( size_t num, size_t size );
+clock_t clock(void);
+int cnd_broadcast( cnd_t *cond );
+int cnd_init( cnd_t* cond );
+int cnd_signal( cnd_t *cond );
+int cnd_timedwait( cnd_t* restrict cond, mtx_t* restrict mutex,
+   const struct timespec* restrict time_point );
+int cnd_wait( cnd_t* cond, mtx_t* mutex );
+errno_t ctime_s(char *buffer, rsize_t bufsz, const time_t *time);
+int fclose( FILE *stream );
+int fflush( FILE *stream );
+int fgetc( FILE *stream );
+int fgetpos( FILE *restrict stream, fpos_t *restrict pos );
+char *fgets( char *restrict str, int count, FILE *restrict stream );
+wint_t fgetwc( FILE *stream );
+FILE *fopen( const char *restrict filename, const char *restrict mode );
+errno_t fopen_s(FILE *restrict *restrict streamptr,
+const char *restrict filename,
+const char *restrict mode);
+int fprintf( FILE *restrict stream, const char *restrict format, ... );
+int fprintf_s(FILE *restrict stream, const char *restrict format, ...);
+int fputc( int ch, FILE *stream );
+int fputs( const char *restrict str, FILE *restrict stream );
+wint_t fputwc( wchar_t ch, FILE *stream );
+int fputws( const wchar_t * restrict str, FILE * restrict stream );
+size_t fread( void *restrict buffer, size_t size, size_t count,
+  FILE *restrict stream );
+FILE *freopen( const char *restrict filename, const char *restrict mode,
+   FILE *restrict stream );
+errno_t freopen_s(FILE *restrict *restrict newstreamptr,
+  const char *restrict filename, const char *restrict mode,
+  FILE *restrict stream);
+int fscanf( FILE *restrict stream, const char *restrict format, ... );
+int fscanf_s(FILE *restrict stream, const char *restrict format, ...);
+int fseek( FILE *stream, long offset, int origin );
+int fsetpos( FILE *stream, const fpos_t *pos );
+long ftell( FILE *stream );
+int fwprintf( FILE *restrict stream,
+  const wchar_t *restrict format, ... );
+int fwprintf_s( FILE *restrict stream,
+const wchar_t *restrict format, ...);
+size_t fwrite( const void *restrict buffer, size_t size, size_t count,
+   FILE *restrict stream ); // more exact error return: < count
+int fwscanf( FILE *restrict stream,
+ const wchar_t *restrict format, ... );
+int fwscanf_s( FILE *restrict stream,
+   const wchar_t *restrict format, ...);
+int getc( FILE *stream );
+int getchar(void);
+char *getenv( const char *name );
+errno_t getenv_s( size_t *restrict len, char *restrict value,
+  rsize_t valuesz, const char *restrict name );
+char *gets_s( char *str, rsize_t n );
+wint_t getwc( FILE *stream );
+wint_t getwchar(void);
+struct tm *gmtime( const time_t *time );
+struct tm *gmtime_s(const time_t *restrict time, struct tm *restrict result);
+struct tm *localtime( const time_t *time );
+struct tm *localtime_s(const time_t *restrict time, struct tm *restrict result);
+void* malloc( size_t size );
+int mblen( const char* s, size_t n );
+size_t mbrlen( const char *restrict s, size_t n, mbstate_t *restrict ps );
+size_t mbrtoc16( char16_t * restrict pc16, const char * restrict s,
+ size_t n, mbstate_t * restrict ps );
+size_t mbrtoc32( char32_t restr

[clang] b4b904e - [Diagnostic] Fixed add ftabstop to -Wmisleading-indentation

[via cfe-commits]

Author: Tyker
Date: 2020-01-03T17:22:24+01:00
New Revision: b4b904e19bb356724b2c6aea0199ce05c6f15cdb

URL: 
https://github.com/llvm/llvm-project/commit/b4b904e19bb356724b2c6aea0199ce05c6f15cdb
DIFF: 
https://github.com/llvm/llvm-project/commit/b4b904e19bb356724b2c6aea0199ce05c6f15cdb.diff

LOG: [Diagnostic] Fixed add ftabstop to -Wmisleading-indentation

Summary:
this allow much better support of codebases like the linux kernel that mix tabs 
and spaces.

-ftabstop=//Width// allow specifying how large tabs are considered to be.

Reviewers: xbolva00, aaron.ballman, rsmith

Reviewed By: aaron.ballman

Subscribers: mstorsjo, cfe-commits, jyknight, riccibruno, rsmith, nathanchance

Tags: #clang

Differential Revision: https://reviews.llvm.org/D71037

Added: 


Modified: 
clang/lib/Parse/ParseStmt.cpp
clang/test/Parser/warn-misleading-indentation.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 203f30610ce7..b79654015653 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1214,6 +1214,42 @@ struct MisleadingIndentationChecker {
 if (Kind == MSK_else && !ShouldSkip)
   P.MisleadingIndentationElseLoc = SL;
   }
+
+  /// Compute the column number will aligning tabs on TabStop (-ftabstop), this
+  /// gives the visual indentation of the SourceLocation.
+  static unsigned getVisualIndentation(SourceManager &SM, SourceLocation Loc) {
+unsigned TabStop = SM.getDiagnostics().getDiagnosticOptions().TabStop;
+
+unsigned ColNo = SM.getSpellingColumnNumber(Loc);
+if (ColNo == 0 || TabStop == 1)
+  return ColNo;
+
+std::pair FIDAndOffset = SM.getDecomposedLoc(Loc);
+
+bool Invalid;
+StringRef BufData = SM.getBufferData(FIDAndOffset.first, &Invalid);
+if (Invalid)
+  return 0;
+
+const char *EndPos = BufData.data() + FIDAndOffset.second;
+// FileOffset are 0-based and Column numbers are 1-based
+assert(FIDAndOffset.second + 1 >= ColNo &&
+   "Column number smaller than file offset?");
+
+unsigned VisualColumn = 0; // Stored as 0-based column, here.
+// Loop from beginning of line up to Loc's file position, counting columns,
+// expanding tabs.
+for (const char *CurPos = EndPos - (ColNo - 1); CurPos != EndPos;
+ ++CurPos) {
+  if (*CurPos == '\t')
+// Advance visual column to next tabstop.
+VisualColumn += (TabStop - VisualColumn % TabStop);
+  else
+VisualColumn++;
+}
+return VisualColumn + 1;
+  }
+
   void Check() {
 Token Tok = P.getCurToken();
 if (P.getActions().getDiagnostics().isIgnored(
@@ -1230,9 +1266,9 @@ struct MisleadingIndentationChecker {
   P.MisleadingIndentationElseLoc = SourceLocation();
 
 SourceManager &SM = P.getPreprocessor().getSourceManager();
-unsigned PrevColNum = SM.getSpellingColumnNumber(PrevLoc);
-unsigned CurColNum = SM.getSpellingColumnNumber(Tok.getLocation());
-unsigned StmtColNum = SM.getSpellingColumnNumber(StmtLoc);
+unsigned PrevColNum = getVisualIndentation(SM, PrevLoc);
+unsigned CurColNum = getVisualIndentation(SM, Tok.getLocation());
+unsigned StmtColNum = getVisualIndentation(SM, StmtLoc);
 
 if (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
 ((PrevColNum > StmtColNum && PrevColNum == CurColNum) ||

diff  --git a/clang/test/Parser/warn-misleading-indentation.cpp 
b/clang/test/Parser/warn-misleading-indentation.cpp
index d366db767e67..4b9d45b19ca6 100644
--- a/clang/test/Parser/warn-misleading-indentation.cpp
+++ b/clang/test/Parser/warn-misleading-indentation.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -x c -fsyntax-only -verify %s
-// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation 
-DWITH_WARN %s
-// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused 
-DWITH_WARN -DCXX17 %s
 // RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused 
-Wno-misleading-indentation -DCXX17 %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation 
-DWITH_WARN -ftabstop 8 -DTAB_SIZE=8 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused 
-DWITH_WARN  -ftabstop 4 -DTAB_SIZE=4 -DCXX17 %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN 
-ftabstop 1 -DTAB_SIZE=1 %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused 
-Wmisleading-indentation -DCXX17 -DWITH_WARN -ftabstop 2 -DTAB_SIZE=2 %s
 
 #ifndef WITH_WARN
 // expected-no-diagnostics
@@ -225,3 +227,81 @@ void s(int num) {
 // expected-warning@-2 {{misleading indentation; statement is not part of the 
previous 'if'}}
 #endif
 }
+int a4()
+{
+   if (0)
+   return 1;
+   return 0;
+#if (TAB_SIZE == 1)
+// expected-warning@-2 {{misleading indentation; statement is not part of the 
previous 'if'}}
+// expected-note@-5 {{here}}
+#endif 
+}
+
+int a5

[PATCH] D71510: [clang][checkers] Added new checker 'error-return-checker'. (WIP)

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Works relatively good now but not perfect. The tests are sometimes too strict 
so there are some false positives, for example this case:

  unsigned long X = strtoul("345", NULL, 10);
  if (X > 100) {
   // handle error
  }

The result is not checked for `ULONG_MAX` but still the code is correct in this 
way. But we can not figure out the intention of the programmer to detect what 
is an "error handling code" to check for error handling branches. Other 
solution is to detect any branch condition that involves the value (returned 
from the function) as a "test for error return value" but this is probably not 
better.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71510/new/

https://reviews.llvm.org/D71510



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72163: [clangd] targetDecl() returns only NamedDecls.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: kadircet, ilya-biryukov.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

While it's perfectly reasonable for non-named decls such as
static_assert to resolve to themselves:

- nothing else ever resolves to them
- features based on references (hover, highlight, find refs etc) tend to be 
uninteresting where only trivial references are possible
- returning NamedDecl is a more convenient API (we cast to it in many places)
- this aligns closer to findExplicitReferences/explicitReferenceTargets

This fixes a crash in explicitReferenceTargets: if the target is a
non-named decl then there's an invalid unchecked cast to NamedDecl.

In practice this means when hovering over e.g. a static_assert:

- before ac3f9e4842 
, we would 
show a (boring) hover card
- after ac3f9e4842 
, we would 
crash
- after this patch, we will show nothing


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72163

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -598,6 +598,9 @@
func<1>();
 }
   )cpp",
+  R"cpp(// non-named decls don't get hover. Don't crash!
+^static_assert(1, "");
+  )cpp",
   };
 
   for (const auto &Test : Tests) {
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -34,7 +34,7 @@
 struct PrintedDecl {
   PrintedDecl(const char *Name, DeclRelationSet Relations = {})
   : Name(Name), Relations(Relations) {}
-  PrintedDecl(const Decl *D, DeclRelationSet Relations = {})
+  PrintedDecl(const NamedDecl *D, DeclRelationSet Relations = {})
   : Relations(Relations) {
 std::string S;
 llvm::raw_string_ostream OS(S);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -86,15 +86,12 @@
 return {};
 
   llvm::DenseSet Result;
-  for (const auto *D :
+  for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-const auto *ND = llvm::dyn_cast(D);
-if (!ND)
-  continue;
 // Get to CXXRecordDecl from constructor or destructor.
-ND = tooling::getCanonicalSymbolDeclaration(ND);
-Result.insert(ND);
+D = tooling::getCanonicalSymbolDeclaration(D);
+Result.insert(D);
   }
   return Result;
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -53,7 +53,7 @@
 // - for kinds that allow multiple definitions (e.g. namespaces), return nullptr
 // Kinds of nodes that always return nullptr here will not have definitions
 // reported by locateSymbolAt().
-const Decl *getDefinition(const Decl *D) {
+const NamedDecl *getDefinition(const NamedDecl *D) {
   assert(D);
   // Decl has one definition that we can find.
   if (const auto *TD = dyn_cast(D))
@@ -129,13 +129,14 @@
   return Merged.CanonicalDeclaration;
 }
 
-std::vector getDeclAtPosition(ParsedAST &AST, SourceLocation Pos,
-DeclRelationSet Relations) {
+std::vector getDeclAtPosition(ParsedAST &AST,
+ SourceLocation Pos,
+ DeclRelationSet Relations) {
   FileID FID;
   unsigned Offset;
   std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
-  std::vector Result;
+  std::vector Result;
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
 auto Decls = targetDecl(N->ASTNode, Relations);
 Result.assign(Decls.begin(), Decls.end());
@@ -251,9 +252,9 @@
   // Emit all symbol locations (declaration or definition) from AST.
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
-const Decl *Def = getDefinition

[PATCH] D72140: [clang-tools-extra] NFC: Fix trivial typos in comments

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61180 tests passed, 0 failed 
and 729 were skipped.

{icon times-circle color=red} clang-tidy: fail. Please fix clang-tidy findings 
.

{icon times-circle color=red} clang-format: fail. Please format your changes 
with clang-format by running `git-clang-format HEAD^` or applying this patch 
.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72140/new/

https://reviews.llvm.org/D72140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72163: [clangd] targetDecl() returns only NamedDecls.

[pre-merge checks [bot] via Phabricator via cfe-commits]

merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 61180 tests passed, 0 failed 
and 729 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72163/new/

https://reviews.llvm.org/D72163



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D72018#1803012 , @aaron.ballman 
wrote:

> In D72018#1802636 , @NoQ wrote:
>
> > Would changing the literal in the attribute have the same effect? I.e., 
> > `acquire_handle("Fuchsia_But_Please_Ignore_Me")`.
>
>
> It should, but doesn't currently because we don't have any checking that the 
> string literal matches a name in the static analyzer or clang-tidy for that 
> attribute.


Actually, the static analyzer checker does check for the literal, so this could 
work.

> 
> 
> In D72018#1801739 , @xazax.hun wrote:
> 
>> I think if there are many problems with this concept, I will fall back to 
>> hard code this information in the checker instead of using an annotation.
> 
> 
> How much hard coded information would this remove? How often do you find that 
> users want to add to the list of hard coded functions themselves but can't? 
> Maybe the benefits to the attribute outweigh the downside and it is worth 
> exploring ways to resolve some of these concerns -- I don't have a good 
> feeling for the problem space though.

I expect not to have too many functions that need such exclusions. It was more 
about making it easier to do some exclusions without touching the analyzer. I 
will ask around what are the preferences of the team, maybe Artem's proposal 
will work for us.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72018/new/

https://reviews.llvm.org/D72018



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72163: [clangd] targetDecl() returns only NamedDecls.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72163/new/

https://reviews.llvm.org/D72163



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 427ffa2 - [OpenMP] diagnose zero-length array section in the depend clause

[Kelvin Li via cfe-commits]

Author: Kelvin Li
Date: 2020-01-03T11:55:37-05:00
New Revision: 427ffa2cdbbc7337d903ba71823a7830fa92568d

URL: 
https://github.com/llvm/llvm-project/commit/427ffa2cdbbc7337d903ba71823a7830fa92568d
DIFF: 
https://github.com/llvm/llvm-project/commit/427ffa2cdbbc7337d903ba71823a7830fa92568d.diff

LOG: [OpenMP] diagnose zero-length array section in the depend clause

The OpenMP specification disallows having zero-length array
sections in the depend clause (OpenMP 5.0 2.17.11).

Differential Revision: https://reviews.llvm.org/D71969

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/target_depend_messages.cpp
clang/test/OpenMP/target_enter_data_depend_messages.cpp
clang/test/OpenMP/target_exit_data_depend_messages.cpp
clang/test/OpenMP/target_parallel_depend_messages.cpp
clang/test/OpenMP/target_parallel_for_depend_messages.cpp
clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp
clang/test/OpenMP/target_simd_depend_messages.cpp
clang/test/OpenMP/target_teams_depend_messages.cpp
clang/test/OpenMP/target_teams_distribute_depend_messages.cpp
clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp

clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp
clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp
clang/test/OpenMP/target_update_depend_messages.cpp
clang/test/OpenMP/task_depend_messages.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 54299a0409fd..99ce42dd7533 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9640,6 +9640,8 @@ def err_omp_depend_sink_expected_plus_minus : Error<
   "expected '+' or '-' operation">;
 def err_omp_depend_sink_source_not_allowed : Error<
   "'depend(%select{source|sink:vec}0)' clause%select{|s}0 cannot be mixed with 
'depend(%select{sink:vec|source}0)' clause%select{s|}0">;
+def err_omp_depend_zero_length_array_section_not_allowed : Error<
+  "zero-length array section is not allowed in 'depend' clause">;
 def err_omp_linear_ordered : Error<
   "'linear' clause cannot be specified along with 'ordered' clause with a 
parameter">;
 def err_omp_unexpected_schedule_modifier : Error<

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 23cd6f04a93d..5a4254f11a8b 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -15020,6 +15020,22 @@ Sema::ActOnOpenMPDependClause(OpenMPDependClauseKind 
DepKind,
   }
   OpsOffs.emplace_back(RHS, OOK);
 } else {
+  // OpenMP 5.0 [2.17.11, Restrictions]
+  // List items used in depend clauses cannot be zero-length array 
sections.
+  const auto *OASE = dyn_cast(SimpleExpr);
+  if (OASE) {
+const Expr *Length = OASE->getLength();
+Expr::EvalResult Result;
+if (Length && !Length->isValueDependent() &&
+Length->EvaluateAsInt(Result, Context) &&
+Result.Val.getInt().isNullValue()) {
+  Diag(ELoc,
+   diag::err_omp_depend_zero_length_array_section_not_allowed)
+  << SimpleExpr->getSourceRange();
+  continue;
+}
+  }
+
   auto *ASE = dyn_cast(SimpleExpr);
   if (!RefExpr->IgnoreParenImpCasts()->isLValue() ||
   (ASE &&

diff  --git a/clang/test/OpenMP/target_depend_messages.cpp 
b/clang/test/OpenMP/target_depend_messages.cpp
index a18bc5d5edcb..df8723e12bee 100644
--- a/clang/test/OpenMP/target_depend_messages.cpp
+++ b/clang/test/OpenMP/target_depend_messages.cpp
@@ -74,7 +74,7 @@ int main(int argc, char **argv, char *env[]) {
   foo();
   #pragma omp target depend (in : argv[0:-1]) // expected-error {{section 
length is evaluated to a negative value -1}}
   foo();
-  #pragma omp target depend (in : argv[-1:0])
+  #pragma omp target depend (in : argv[-1:0]) // expected-error {{zero-length 
array section is not allowed in 'depend' clause}}
   foo();
   #pragma omp target depend (in : argv[:]) // expected-error {{section length 
is unspecified and cannot be inferred because subscripted value is not an 
array}}
   foo();

diff  --git a/clang/test/OpenMP/target_enter_data_depend_messages.cpp 
b/clang/test/OpenMP/target_enter_data_depend_messages.cpp
index 49dddfcc7cba..6d459e8793e6 100644
--- a/clang/test/OpenMP/target_enter_data_depend_messages.cpp
+++ b/clang/test/OpenMP/target_enter_data_depend_messages.cpp
@@ -70,7 +70,7 @@ int tmain(T argc, S **argv, R *env[]) {
   foo();
   #pragma omp target enter data map(to: i) depend (in : argv[0:-1]) // 
expected-error {{section length is evaluated to a negative value -1}}
   foo();
-  #pragma omp target enter data map(to: i) depend (in : argv[-1:0])
+  #pragma omp target

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

sammccall wrote:
> ilya-biryukov wrote:
> > sammccall wrote:
> > > ilya-biryukov wrote:
> > > > sammccall wrote:
> > > > > ilya-biryukov wrote:
> > > > > > sammccall wrote:
> > > > > > > ilya-biryukov wrote:
> > > > > > > > sammccall wrote:
> > > > > > > > > ilya-biryukov wrote:
> > > > > > > > > > No need to fix this.
> > > > > > > > > > 
> > > > > > > > > > The name could probably be better, but we can fix this 
> > > > > > > > > > later. Don't have any good ideas.
> > > > > > > > > I think as a public API this should be clearer on how/when to 
> > > > > > > > > use and its relation to other things (planning to send a 
> > > > > > > > > patch, but wanted to discuss a bit here first).
> > > > > > > > > 
> > > > > > > > > - This is essentially a filter of allTargetDecls (as is 
> > > > > > > > > targetDecl), but the relationship between the two isn't 
> > > > > > > > > clear. They should have closely related names (variant) or 
> > > > > > > > > maybe better be more orthogonal and composed at the callsite.
> > > > > > > > > - The most distinctive word in the name is `explicit`, but 
> > > > > > > > > this function doesn't actually have anything to do with 
> > > > > > > > > explicit vs non-explicit references (just history?)
> > > > > > > > > - It's not clear to me whether we actually want to 
> > > > > > > > > encapsulate/hide the preference for instantiations over 
> > > > > > > > > templates here, or whether it should be expressed at the 
> > > > > > > > > callsite because the policy is decided feature-by-feature. 
> > > > > > > > > `chooseBest(...)` vs `prefer(TemplateInstantiation, 
> > > > > > > > > TemplatePattern, ...)`. The latter seems safer to me, based 
> > > > > > > > > on experience with targetDecl so far.
> > > > > > > > > 
> > > > > > > > > A couple of ideas:
> > > > > > > > >  - caller invokes allTargetDecls() and then this is a helper 
> > > > > > > > > function `prefer(DeclRelation, DeclRelation, Results)` that 
> > > > > > > > > mutates Results
> > > > > > > > >  - targetDecl() becomes the swiss-army filter and accepts a 
> > > > > > > > > list of "prefer-X-over-Y", which it applies before returning 
> > > > > > > > > the results with flags dropped
> > > > > > > > I don't like the idea of `targetDecl` becoming the swiss-army 
> > > > > > > > knife, it's complicated to use as is.
> > > > > > > > The contract of `explicitReferenceTargets` is to expose only 
> > > > > > > > the decls written in the source code and nothing else, it's 
> > > > > > > > much simpler to explain and to use.
> > > > > > > > 
> > > > > > > > It's useful for many things, essentially for anything that 
> > > > > > > > needs to answer the question of "what does this name in the 
> > > > > > > > source code refers to".
> > > > > > > > 
> > > > > > > > The name could be clearer, the comments might need to be 
> > > > > > > > improved.
> > > > > > > > The only crucial improvement that I'd attempt is getting rid of 
> > > > > > > > the `Mask` parameter, I think there was just one or two cases 
> > > > > > > > where it was useful.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > I don't like the idea of targetDecl becoming the swiss-army 
> > > > > > > > knife
> > > > > > > Fair enough. I'll poke more in the direction of making it easier 
> > > > > > > to compose allTargetDecls then.
> > > > > > > 
> > > > > > > > The contract of explicitReferenceTargets is to expose only the 
> > > > > > > > decls written in the source code and nothing else, it's much 
> > > > > > > > simpler to explain and to use.
> > > > > > > So I agree that the interface of targetDecl() is complicated, but 
> > > > > > > I don't think this one is simpler to explain or use - or at least 
> > > > > > > it hasn't been well-explained so far.
> > > > > > > 
> > > > > > > For example,
> > > > > > >  - "to expose only the decls written in the source code" - 
> > > > > > > `vector` isn't a decl written in the source code.
> > > > > > >  - "find declarations explicitly referenced in the source code" - 
> > > > > > > `return [[{foo}]];` - the class/constructor isn't explicitly 
> > > > > > > referenced, but this function returns it.
> > > > > > >  - "what does this name in the source code refers to" - a 
> > > > > > > template name refers to the primary template, the (possible) 
> > > > > > > partial specialization, and specialization all at once. Features 
> > > > > > > like find refs/highlight show the equivalence class of names that 
> > > > > > > refer to the same thing, but they don't prefer the specialization 
> > > > > > > for that.
> > > > > > >  - none of these explanations explains why the function is 
> > > > > > > opinionated about template vs expansion but not about alias vs 
> > > > > > > un

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.h:194
+llvm::SmallVector
+explicitReferenceTargets(ast_type_traits::DynTypedNode N,
+ DeclRelationSet Mask = {});

ilya-biryukov wrote:
> sammccall wrote:
> > ilya-biryukov wrote:
> > > sammccall wrote:
> > > > ilya-biryukov wrote:
> > > > > sammccall wrote:
> > > > > > ilya-biryukov wrote:
> > > > > > > sammccall wrote:
> > > > > > > > ilya-biryukov wrote:
> > > > > > > > > sammccall wrote:
> > > > > > > > > > ilya-biryukov wrote:
> > > > > > > > > > > No need to fix this.
> > > > > > > > > > > 
> > > > > > > > > > > The name could probably be better, but we can fix this 
> > > > > > > > > > > later. Don't have any good ideas.
> > > > > > > > > > I think as a public API this should be clearer on how/when 
> > > > > > > > > > to use and its relation to other things (planning to send a 
> > > > > > > > > > patch, but wanted to discuss a bit here first).
> > > > > > > > > > 
> > > > > > > > > > - This is essentially a filter of allTargetDecls (as is 
> > > > > > > > > > targetDecl), but the relationship between the two isn't 
> > > > > > > > > > clear. They should have closely related names (variant) or 
> > > > > > > > > > maybe better be more orthogonal and composed at the 
> > > > > > > > > > callsite.
> > > > > > > > > > - The most distinctive word in the name is `explicit`, but 
> > > > > > > > > > this function doesn't actually have anything to do with 
> > > > > > > > > > explicit vs non-explicit references (just history?)
> > > > > > > > > > - It's not clear to me whether we actually want to 
> > > > > > > > > > encapsulate/hide the preference for instantiations over 
> > > > > > > > > > templates here, or whether it should be expressed at the 
> > > > > > > > > > callsite because the policy is decided feature-by-feature. 
> > > > > > > > > > `chooseBest(...)` vs `prefer(TemplateInstantiation, 
> > > > > > > > > > TemplatePattern, ...)`. The latter seems safer to me, based 
> > > > > > > > > > on experience with targetDecl so far.
> > > > > > > > > > 
> > > > > > > > > > A couple of ideas:
> > > > > > > > > >  - caller invokes allTargetDecls() and then this is a 
> > > > > > > > > > helper function `prefer(DeclRelation, DeclRelation, 
> > > > > > > > > > Results)` that mutates Results
> > > > > > > > > >  - targetDecl() becomes the swiss-army filter and accepts a 
> > > > > > > > > > list of "prefer-X-over-Y", which it applies before 
> > > > > > > > > > returning the results with flags dropped
> > > > > > > > > I don't like the idea of `targetDecl` becoming the swiss-army 
> > > > > > > > > knife, it's complicated to use as is.
> > > > > > > > > The contract of `explicitReferenceTargets` is to expose only 
> > > > > > > > > the decls written in the source code and nothing else, it's 
> > > > > > > > > much simpler to explain and to use.
> > > > > > > > > 
> > > > > > > > > It's useful for many things, essentially for anything that 
> > > > > > > > > needs to answer the question of "what does this name in the 
> > > > > > > > > source code refers to".
> > > > > > > > > 
> > > > > > > > > The name could be clearer, the comments might need to be 
> > > > > > > > > improved.
> > > > > > > > > The only crucial improvement that I'd attempt is getting rid 
> > > > > > > > > of the `Mask` parameter, I think there was just one or two 
> > > > > > > > > cases where it was useful.
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > I don't like the idea of targetDecl becoming the swiss-army 
> > > > > > > > > knife
> > > > > > > > Fair enough. I'll poke more in the direction of making it 
> > > > > > > > easier to compose allTargetDecls then.
> > > > > > > > 
> > > > > > > > > The contract of explicitReferenceTargets is to expose only 
> > > > > > > > > the decls written in the source code and nothing else, it's 
> > > > > > > > > much simpler to explain and to use.
> > > > > > > > So I agree that the interface of targetDecl() is complicated, 
> > > > > > > > but I don't think this one is simpler to explain or use - or at 
> > > > > > > > least it hasn't been well-explained so far.
> > > > > > > > 
> > > > > > > > For example,
> > > > > > > >  - "to expose only the decls written in the source code" - 
> > > > > > > > `vector` isn't a decl written in the source code.
> > > > > > > >  - "find declarations explicitly referenced in the source code" 
> > > > > > > > - `return [[{foo}]];` - the class/constructor isn't explicitly 
> > > > > > > > referenced, but this function returns it.
> > > > > > > >  - "what does this name in the source code refers to" - a 
> > > > > > > > template name refers to the primary template, the (possible) 
> > > > > > > > partial specialization, and specialization all at once. 
> > > > > > > > Features like find refs/highlight show the equivalence class of 
> > > > > > > > names that refer to the same thing, but they don't prefer the 
> > >

[PATCH] D71969: [OpenMP] diagnose zero-length array section in the depend clause

[Kelvin Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG427ffa2cdbbc: [OpenMP] diagnose zero-length array section in 
the depend clause (authored by kkwli0).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71969/new/

https://reviews.llvm.org/D71969

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/target_depend_messages.cpp
  clang/test/OpenMP/target_enter_data_depend_messages.cpp
  clang/test/OpenMP/target_exit_data_depend_messages.cpp
  clang/test/OpenMP/target_parallel_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_depend_messages.cpp
  clang/test/OpenMP/target_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_parallel_for_depend_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_depend_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_depend_messages.cpp
  clang/test/OpenMP/target_update_depend_messages.cpp
  clang/test/OpenMP/task_depend_messages.cpp

Index: clang/test/OpenMP/task_depend_messages.cpp
===
--- clang/test/OpenMP/task_depend_messages.cpp
+++ clang/test/OpenMP/task_depend_messages.cpp
@@ -45,7 +45,7 @@
   #pragma omp task depend (in : argv[argc: // expected-error {{expected expression}} expected-error {{expected ']'}} expected-error {{expected ')'}} expected-note {{to match this '['}} expected-note {{to match this '('}}
   #pragma omp task depend (in : argv[argc:argc] // expected-error {{expected ')'}} expected-note {{to match this '('}}
   #pragma omp task depend (in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
-  #pragma omp task depend (in : argv[-1:0])
+  #pragma omp task depend (in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp task depend (in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
   #pragma omp task depend (in : argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
   #pragma omp task depend(in:a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
Index: clang/test/OpenMP/target_update_depend_messages.cpp
===
--- clang/test/OpenMP/target_update_depend_messages.cpp
+++ clang/test/OpenMP/target_update_depend_messages.cpp
@@ -15,7 +15,6 @@
   public:
 int operator[](int index) { return 0; }
 };
-
 template 
 int tmain(T argc, S **argv, R *env[]) {
   vector vec;
@@ -52,7 +51,7 @@
   #pragma omp target update to(z) depend(in : argv[argc: // expected-error {{expected expression}} expected-error {{expected ']'}} expected-error {{expected ')'}} expected-note {{to match this '['}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[argc:argc] // expected-error {{expected ')'}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
-  #pragma omp target update to(z) depend(in : argv[-1:0])
+  #pragma omp target update to(z) depend(in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp target update to(z) depend(in : argv[:]) // expected-error {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
   #pragma omp target update to(z) depend(in : argv[3:4:1]) // expected-error {{expected ']'}} expected-note {{to match this '['}}
   #pragma omp target update to(z) depend(in:a[0:1]) // expected-error {{subscripted value is not an array or pointer}}
@@ -64,7 +63,6 @@
 
   return 0;
 }
-
 int main(int argc, char **argv, char *env[]) {
   vector vec;
   typedef float V __attribute__((vector_size(16)));
@@ -100,7 +98,7 @@
   #pragma omp target update to(z) depend(in : argv[argc: // expected-error {{expected expression}} expected-error {{expected ']'}} expected-error {{expected ')'}} expected-note {{to match this '['}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[argc:argc] // expected-error {{expected ')'}} expected-note {{to match this '('}}
   #pragma omp target update to(z) depend(in : argv[0:-1]) // expected-error {{section length is evaluated to a negative value -1}}
-  #pragma omp target update to(z) depend(in : argv[-1:0])
+  #pragma omp target update to(z) depend(in : argv[-1:0]) // expected-error {{zero-length array section is not allowed in 'depend' clause}}
   #pragma omp target update to(z) depend(in : argv[:])

[clang] ba3484c - [clang-format/java] format multiple qualified annotations on one declaration better

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2020-01-03T12:11:44-05:00
New Revision: ba3484c051b62a662c555200f4a03b2e8df8e094

URL: 
https://github.com/llvm/llvm-project/commit/ba3484c051b62a662c555200f4a03b2e8df8e094
DIFF: 
https://github.com/llvm/llvm-project/commit/ba3484c051b62a662c555200f4a03b2e8df8e094.diff

LOG: [clang-format/java] format multiple qualified annotations on one 
declaration better

Before:
class Foo {
  @CommandLineFlags
  .Add
  @Features.foo
  public void test() {}
}

Now:
class Foo {
@Features.foo
@CommandLineFlags.Add
public void test() { }
}

See also https://crbug.com/1034115

Added: 


Modified: 
clang/lib/Format/FormatToken.h
clang/unittests/Format/FormatTestJava.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 39498280fb6d..e9cd327754ef 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -407,7 +407,7 @@ struct FormatToken {
   bool isMemberAccess() const {
 return isOneOf(tok::arrow, tok::period, tok::arrowstar) &&
!isOneOf(TT_DesignatedInitializerPeriod, TT_TrailingReturnArrow,
-TT_LambdaArrow);
+TT_LambdaArrow, TT_LeadingJavaAnnotation);
   }
 
   bool isUnaryOperator() const {

diff  --git a/clang/unittests/Format/FormatTestJava.cpp 
b/clang/unittests/Format/FormatTestJava.cpp
index a4936e0e1ccc..5e73e4b4ea4e 100644
--- a/clang/unittests/Format/FormatTestJava.cpp
+++ b/clang/unittests/Format/FormatTestJava.cpp
@@ -335,6 +335,14 @@ TEST_F(FormatTestJava, Annotations) {
   verifyFormat("@Annotation(\"Some\"\n"
"+ \" text\")\n"
"List list;");
+
+  verifyFormat(
+"@Test\n"
+"@Feature({\"Android-TabSwitcher\"})\n"
+"@CommandLineFlags.Add({ChromeSwitches.DISABLE_FIRST_RUN_EXPERIENCE})\n"
+"@Features.EnableFeatures({FEATURE})\n"
+"public void test(@Foo.bar(\"baz\") @Quux.Qoob int theFirstParam,\n"
+"@Foo.bar(\"baz\") @Quux.Qoob int theSecondParm) {}");
 }
 
 TEST_F(FormatTestJava, Generics) {



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] add743b - [OPENMP]Fix crash on error message for declare reduction.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-01-03T12:13:03-05:00
New Revision: add743b4348095c0d2e407c7a2b8a87a5f8194b0

URL: 
https://github.com/llvm/llvm-project/commit/add743b4348095c0d2e407c7a2b8a87a5f8194b0
DIFF: 
https://github.com/llvm/llvm-project/commit/add743b4348095c0d2e407c7a2b8a87a5f8194b0.diff

LOG: [OPENMP]Fix crash on error message for declare reduction.

If the qualified reduction name is specified and not found, the compiler
may crash because of not specified parameter.

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_reduction_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5a4254f11a8b..d2393c17bcc8 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -13498,7 +13498,8 @@ buildDeclareReductionRef(Sema &SemaRef, SourceLocation 
Loc, SourceRange Range,
 }
   }
   if (ReductionIdScopeSpec.isSet()) {
-SemaRef.Diag(Loc, diag::err_omp_not_resolved_reduction_identifier) << 
Range;
+SemaRef.Diag(Loc, diag::err_omp_not_resolved_reduction_identifier)
+<< Ty << Range;
 return ExprError();
   }
   return ExprEmpty();

diff  --git a/clang/test/OpenMP/declare_reduction_messages.cpp 
b/clang/test/OpenMP/declare_reduction_messages.cpp
index 1fc5ec6e683f..eeafe9032e08 100644
--- a/clang/test/OpenMP/declare_reduction_messages.cpp
+++ b/clang/test/OpenMP/declare_reduction_messages.cpp
@@ -146,9 +146,9 @@ struct A {
 };
 
 int A_TEST() {
-  A test;
+  A test, test1;
 #pragma omp declare reduction(+ : A : omp_out) initializer(omp_priv = A()) 
allocate(test) // expected-warning {{extra tokens at the end of '#pragma omp 
declare reduction' are ignored}}
-#pragma omp parallel reduction(+ : test)
+#pragma omp parallel reduction(+ : test) reduction(::operator+: test1) // 
expected-error {{unable to resolve declare reduction construct for type 'A'}}
   {}
   return 0;
 }



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71499: Add builtins for aligning and checking alignment of pointers and integers

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

@lebedev.ri I agree with you that the semantics of these alignment builtins 
should only return a pointer that is of the same object as the one given as 
input.
Otherwise, these builtins would be even worst that ptr2int/int2ptr, since their 
result could alias with any other pointer in the program, not just the escaped 
pointers.

I gave a glance through the patch, especially the documentation section, and 
the semantics look right to me.
Would be nice to see this using ptrmask on a subsequent patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71499/new/

https://reviews.llvm.org/D71499



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] f06f439 - [clangd] targetDecl() returns only NamedDecls.

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-01-03T18:18:40+01:00
New Revision: f06f439fadf740ea9019f2eb7f26ff88198ed375

URL: 
https://github.com/llvm/llvm-project/commit/f06f439fadf740ea9019f2eb7f26ff88198ed375
DIFF: 
https://github.com/llvm/llvm-project/commit/f06f439fadf740ea9019f2eb7f26ff88198ed375.diff

LOG: [clangd] targetDecl() returns only NamedDecls.

Summary:
While it's perfectly reasonable for non-named decls such as
static_assert to resolve to themselves:
 - nothing else ever resolves to them
 - features based on references (hover, highlight, find refs etc) tend
   to be uninteresting where only trivial references are possible
 - returning NamedDecl is a more convenient API (we cast to it in many places)
 - this aligns closer to findExplicitReferences/explicitReferenceTargets

This fixes a crash in explicitReferenceTargets: if the target is a
non-named decl then there's an invalid unchecked cast to NamedDecl.

In practice this means when hovering over e.g. a static_assert:
 - before ac3f9e4842, we would show a (boring) hover card
 - after ac3f9e4842, we would crash
 - after this patch, we will show nothing

Reviewers: kadircet, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D72163

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/FindTarget.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/refactor/Rename.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index f9332ee4f220..f5d7f4a9326b 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -119,10 +119,10 @@ getMembersReferencedViaDependentName(const Type *T, const 
DeclarationName &Name,
 struct TargetFinder {
   using RelSet = DeclRelationSet;
   using Rel = DeclRelation;
-  llvm::SmallDenseMap Decls;
+  llvm::SmallDenseMap Decls;
   RelSet Flags;
 
-  static const Decl *getTemplatePattern(const Decl *D) {
+  static const NamedDecl *getTemplatePattern(const NamedDecl *D) {
 if (const CXXRecordDecl *CRD = dyn_cast(D)) {
   return CRD->getTemplateInstantiationPattern();
 } else if (const FunctionDecl *FD = dyn_cast(D)) {
@@ -134,12 +134,12 @@ struct TargetFinder {
 } else if (const auto *ED = dyn_cast(D)) {
   return ED->getInstantiatedFromMemberEnum();
 } else if (isa(D) || isa(D)) {
-  const auto *ND = cast(D);
-  if (const DeclContext *Parent = dyn_cast_or_null(
-  getTemplatePattern(llvm::cast(ND->getDeclContext()
-for (const NamedDecl *BaseND : Parent->lookup(ND->getDeclName()))
-  if (!BaseND->isImplicit() && BaseND->getKind() == ND->getKind())
-return BaseND;
+  if (const auto *Parent = llvm::dyn_cast(D->getDeclContext()))
+if (const DeclContext *ParentPat =
+dyn_cast_or_null(getTemplatePattern(Parent)))
+  for (const NamedDecl *BaseND : ParentPat->lookup(D->getDeclName()))
+if (!BaseND->isImplicit() && BaseND->getKind() == D->getKind())
+  return BaseND;
 } else if (const auto *ECD = dyn_cast(D)) {
   if (const auto *ED = dyn_cast(ECD->getDeclContext())) {
 if (const EnumDecl *Pattern = ED->getInstantiatedFromMemberEnum()) {
@@ -156,14 +156,15 @@ struct TargetFinder {
  nodeToString(ast_type_traits::DynTypedNode::create(Node)));
   }
 
-  void report(const Decl *D, RelSet Flags) {
+  void report(const NamedDecl *D, RelSet Flags) {
 dlog("--> [{0}] {1}", Flags,
  nodeToString(ast_type_traits::DynTypedNode::create(*D)));
 Decls[D] |= Flags;
   }
 
 public:
-  void add(const Decl *D, RelSet Flags) {
+  void add(const Decl *Dcl, RelSet Flags) {
+const NamedDecl *D = llvm::dyn_cast(Dcl);
 if (!D)
   return;
 debug(*D, Flags);
@@ -405,7 +406,7 @@ struct TargetFinder {
 
 } // namespace
 
-llvm::SmallVector, 1>
+llvm::SmallVector, 1>
 allTargetDecls(const ast_type_traits::DynTypedNode &N) {
   dlog("allTargetDecls({0})", nodeToString(N));
   TargetFinder Finder;
@@ -428,9 +429,9 @@ allTargetDecls(const ast_type_traits::DynTypedNode &N) {
   return {Finder.Decls.begin(), Finder.Decls.end()};
 }
 
-llvm::SmallVector
+llvm::SmallVector
 targetDecl(const ast_type_traits::DynTypedNode &N, DeclRelationSet Mask) {
-  llvm::SmallVector Result;
+  llvm::SmallVector Result;
   for (const auto &Entry : allTargetDecls(N)) {
 if (!(Entry.second & ~Mask))
   Result.push_back(Entry.first);
@@ -456,12 +457,12 @@ explicitReferenceTargets(DynTypedNode N, DeclRelationSet 
Mask) {
 if (D.second & ~Mask)
   continue;
 if (D.second & DeclRelation::TemplatePattern) {
-  TemplatePatterns.push_back(llvm

[PATCH] D72167: Add support for __declspec(guard(nocf))

[Andrew Paverd via Phabricator via cfe-commits]

ajpaverd created this revision.
ajpaverd added reviewers: rnk, dmajor, pcc, hans.
Herald added subscribers: llvm-commits, cfe-commits, hiraditya.
Herald added projects: clang, LLVM.

Avoid using the `nocf_check` attribute with Control Flow Guard. Instead, use a 
new `"guard_nocf"` function attribute to indicate that checks should not be 
added on indirect calls within that function. Add support for 
`__declspec(guard(nocf))` following the same syntax as MSVC.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D72167

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/guard_nocf.c
  clang/test/CodeGenCXX/guard_nocf.cpp
  llvm/docs/LangRef.rst
  llvm/lib/Transforms/CFGuard/CFGuard.cpp
  llvm/test/CodeGen/AArch64/cfguard-checks.ll
  llvm/test/CodeGen/ARM/cfguard-checks.ll
  llvm/test/CodeGen/X86/cfguard-checks.ll

Index: llvm/test/CodeGen/X86/cfguard-checks.ll
===
--- llvm/test/CodeGen/X86/cfguard-checks.ll
+++ llvm/test/CodeGen/X86/cfguard-checks.ll
@@ -8,8 +8,8 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -17,17 +17,17 @@
   %1 = call i32 %0()
   ret i32 %1
 
-  ; X32-LABEL: func_nocf_checks
+  ; X32-LABEL: func_guard_nocf
   ; X32: 	 movl  $_target_func, %eax
   ; X32-NOT: __guard_check_icall_fptr
 	; X32: 	 calll *%eax
 
-  ; X64-LABEL: func_nocf_checks
+  ; X64-LABEL: func_guard_nocf
   ; X64:   leaq	target_func(%rip), %rax
   ; X64-NOT: __guard_dispatch_icall_fptr
   ; X64:   callq	*%rax
 }
-attributes #0 = { nocf_check }
+attributes #0 = { "guard_nocf" }
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/test/CodeGen/ARM/cfguard-checks.ll
===
--- llvm/test/CodeGen/ARM/cfguard-checks.ll
+++ llvm/test/CodeGen/ARM/cfguard-checks.ll
@@ -7,8 +7,8 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -16,13 +16,13 @@
   %1 = call arm_aapcs_vfpcc i32 %0()
   ret i32 %1
 
-  ; CHECK-LABEL: func_nocf_checks
+  ; CHECK-LABEL: func_guard_nocf
   ; CHECK:   movw r0, :lower16:target_func
 	; CHECK:   movt r0, :upper16:target_func
   ; CHECK-NOT:   __guard_check_icall_fptr
 	; CHECK:   blx r0
 }
-attributes #0 = { nocf_check "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
+attributes #0 = { "guard_nocf" "target-cpu"="cortex-a9" "target-features"="+armv7-a,+dsp,+fp16,+neon,+strict-align,+thumb-mode,+vfp3"}
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/test/CodeGen/AArch64/cfguard-checks.ll
===
--- llvm/test/CodeGen/AArch64/cfguard-checks.ll
+++ llvm/test/CodeGen/AArch64/cfguard-checks.ll
@@ -7,8 +7,8 @@
 declare i32 @target_func()
 
 
-; Test that Control Flow Guard checks are not added to functions with nocf_checks attribute.
-define i32 @func_nocf_checks() #0 {
+; Test that Control Flow Guard checks are not added to functions with the "guard_nocf" attribute.
+define i32 @func_guard_nocf() #0 {
 entry:
   %func_ptr = alloca i32 ()*, align 8
   store i32 ()* @target_func, i32 ()** %func_ptr, align 8
@@ -16,13 +16,13 @@
   %1 = call i32 %0()
   ret i32 %1
 
-  ; CHECK-LABEL: func_nocf_checks
+  ; CHECK-LABEL: func_guard_nocf
   ; CHECK:   adrp x8, target_func
 	; CHECK:   add x8, x8, target_func
   ; CHECK-NOT:   __guard_check_icall_fptr
 	; CHECK:   blr x8
 }
-attributes #0 = { nocf_check }
+attributes #0 = { "guard_nocf" }
 
 
 ; Test that Control Flow Guard checks are added even at -O0.
Index: llvm/lib/Transforms/CFGuard/CFGuard.cpp
===
--- llvm/lib/Transforms/CFGuard/CFGuard.cpp
+++ llvm/lib/Transforms/CFGuard/CFGuard.cpp
@@ -255,7 +255,7 @@
 bool CFGuard::runOnFunction(Function &F) {
 
   // Skip modules and functions for which CFGuard checks have been disabled.
-  if (cfguard_module_flag != 2 || F.hasFnAttribute(Attribute::NoCfCheck))
+  if (cfguard_module_flag != 2 || F.hasFnAttribute("guard_nocf"))
 return false;
 
   SmallVector IndirectCalls;
@@ -277,7 +2

[PATCH] D71644: [clangd] Heuristically resolve dependent call through smart pointer type

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

This is great! Thank you!




Comment at: clang-tools-extra/clangd/FindTarget.cpp:116
+  // smart pointer type.
+  ASTContext *Ctx = hackyFindASTContext(T);
+  if (!Ctx)

nridge wrote:
> I don't intend for this function to be in the final patch.
> 
> Rather, I'm wondering: am I missing some obvious general way to recover the 
> `ASTContext` from a `Type` (or a `Stmt`)?
> 
> If not, I think we'll need to modify `targetDecl()`, `allTargetDecls()`, 
> `findExplicitReferences()` etc. to have their callers pass in an `ASTContext`?
Haha, I see what you mean. There isn't a general way, but I think your hack is 
better than changing the public API, and we can improve it a little.

By the time you actually do the lookup, you have a CXXRecordDecl you're going 
to look into, and decls have a pointer to the ASTContext. So instead of a 
DeclarationName, you could pass a function_ref or such into 
`getMembersReferencedViaDependentName`, and have that function lazily call the 
factory to get the DeclarationName to use. Still hacky, but it's an internal 
function, and we don't have the weird parallel codepath...



Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:503
 
   R"cpp(// FIXME: Heuristic resolution of dependent method
 // invoked via smart pointer

you can remove the FIXME


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71644/new/

https://reviews.llvm.org/D71644



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72163: [clangd] targetDecl() returns only NamedDecls.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf06f439fadf7: [clangd] targetDecl() returns only NamedDecls. 
(authored by sammccall).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72163/new/

https://reviews.llvm.org/D72163

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/FindTarget.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -598,6 +598,9 @@
func<1>();
 }
   )cpp",
+  R"cpp(// non-named decls don't get hover. Don't crash!
+^static_assert(1, "");
+  )cpp",
   };
 
   for (const auto &Test : Tests) {
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -34,7 +34,7 @@
 struct PrintedDecl {
   PrintedDecl(const char *Name, DeclRelationSet Relations = {})
   : Name(Name), Relations(Relations) {}
-  PrintedDecl(const Decl *D, DeclRelationSet Relations = {})
+  PrintedDecl(const NamedDecl *D, DeclRelationSet Relations = {})
   : Relations(Relations) {
 std::string S;
 llvm::raw_string_ostream OS(S);
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -86,15 +86,12 @@
 return {};
 
   llvm::DenseSet Result;
-  for (const auto *D :
+  for (const NamedDecl *D :
targetDecl(SelectedNode->ASTNode,
   DeclRelation::Alias | DeclRelation::TemplatePattern)) {
-const auto *ND = llvm::dyn_cast(D);
-if (!ND)
-  continue;
 // Get to CXXRecordDecl from constructor or destructor.
-ND = tooling::getCanonicalSymbolDeclaration(ND);
-Result.insert(ND);
+D = tooling::getCanonicalSymbolDeclaration(D);
+Result.insert(D);
   }
   return Result;
 }
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -53,7 +53,7 @@
 // - for kinds that allow multiple definitions (e.g. namespaces), return nullptr
 // Kinds of nodes that always return nullptr here will not have definitions
 // reported by locateSymbolAt().
-const Decl *getDefinition(const Decl *D) {
+const NamedDecl *getDefinition(const NamedDecl *D) {
   assert(D);
   // Decl has one definition that we can find.
   if (const auto *TD = dyn_cast(D))
@@ -129,13 +129,14 @@
   return Merged.CanonicalDeclaration;
 }
 
-std::vector getDeclAtPosition(ParsedAST &AST, SourceLocation Pos,
-DeclRelationSet Relations) {
+std::vector getDeclAtPosition(ParsedAST &AST,
+ SourceLocation Pos,
+ DeclRelationSet Relations) {
   FileID FID;
   unsigned Offset;
   std::tie(FID, Offset) = AST.getSourceManager().getDecomposedSpellingLoc(Pos);
   SelectionTree Selection(AST.getASTContext(), AST.getTokens(), Offset);
-  std::vector Result;
+  std::vector Result;
   if (const SelectionTree::Node *N = Selection.commonAncestor()) {
 auto Decls = targetDecl(N->ASTNode, Relations);
 Result.assign(Decls.begin(), Decls.end());
@@ -251,9 +252,9 @@
   // Emit all symbol locations (declaration or definition) from AST.
   DeclRelationSet Relations =
   DeclRelation::TemplatePattern | DeclRelation::Alias;
-  for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
-const Decl *Def = getDefinition(D);
-const Decl *Preferred = Def ? Def : D;
+  for (const NamedDecl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
+const NamedDecl *Def = getDefinition(D);
+const NamedDecl *Preferred = Def ? Def : D;
 
 // If we're at the point of declaration of a template specialization,
 // it's more useful to navigate to the template declaration.
@@ -272,8 +273,7 @@
   continue;
 
 Result.emplace_back();
-if (auto *ND = dyn_cast(Preferred))
-  Result.back().Name = printName(AST.getASTContext(), *ND);
+Result.back().Name = printName(AST.getASTContext(), *D);
 Result.back().PreferredDeclaration = *Loc;
 // Preferred is always a definition if possible, so this check works.
 if (Def == Preferred)
@@ -332,9 +332,9 @@
   };
 
   ReferenceFinder(ASTContext &AST, Preprocessor &PP,
-  c

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Ahh clang format script must have run, I'll undo that. I've not tried running 
it against the whole of llvm as I don't know what the easy way to do that is, 
especially given that llvm disregards it's standard naming convention for old 
projects and stl like containers. The comment is just in relation to the bug 
report that this fix is based on, could probably go


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72121/new/

https://reviews.llvm.org/D72121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[Andrew Paverd via Phabricator via cfe-commits]

ajpaverd added a comment.

In D65761#1772076 , @dmajor wrote:

> Well, this patch specifically has code (even a test) that `nocf_check` 
> prevents CFG, so it looks like the intent was there, it's just that there 
> isn't a way to get that attribute onto functions from cpp in a CFG/non-CET 
> world.


Apologies for the delayed reply! This code was indeed reusing `nocf_check`, but 
I now think it would be better to use a separate attribute for Control Flow 
Guard. Please see the proposed improvement at D72167: Add support for 
__declspec(guard(nocf)) 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65761/new/

https://reviews.llvm.org/D65761



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1648
+  // a bit different than the x64 version.  First, all vector types (not HVAs)
+  // are assigned, with the first 6 ending up in the YMM0-5 or XMM0-5 
registers.
+  // This differs from the x64 implementation, where the first 6 by INDEX get

What about ZMM?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72110/new/

https://reviews.llvm.org/D72110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72057: [docs] Remove outdated svn/git information from hacking page

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin updated this revision to Diff 236088.
AlexanderLanin marked an inline comment as done.
AlexanderLanin added a comment.

`Getting Started` since it's `Getting Started with the LLVM System`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72057/new/

https://reviews.llvm.org/D72057

Files:
  clang/www/hacking.html


Index: clang/www/hacking.html
===
--- clang/www/hacking.html
+++ clang/www/hacking.html
@@ -275,30 +275,8 @@
   Creating Patch Files
   
 
-  To return changes to the Clang team, unless you have checkin
-  privileges, the preferred way is to send patch files
-  https://llvm.org/docs/Contributing.html#how-to-submit-a-patch";>using 
LLVM's Phabricator with an explanation of what the patch is for. Clang 
follows https://llvm.org/docs/DeveloperPolicy.html";>LLVM's developer 
policy.
-  If your patch requires a wider discussion (for example, because it is an
-  architectural change), you can use the cfe-dev mailing list.
-
-  To create these patch files, change directory
-  to the llvm/tools/clang root and run:
-
-  svn diff (relative path) >(patch file name)
-
-  For example, for getting the diffs of all of clang:
-
-  svn diff . >~/mypatchfile.patch
-
-  For example, for getting the diffs of a single file:
-
-  svn diff lib/Parse/ParseDeclCXX.cpp >~/ParseDeclCXX.patch
-
-  Note that the paths embedded in the patch depend on where you run it,
-  so changing directory to the llvm/tools/clang directory is recommended.
-
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  To contribute changes to Clang see
+https://llvm.org/docs/GettingStarted.html#sending-patches";>LLVM's 
Getting Started page
 
   
   LLVM IR Generation


Index: clang/www/hacking.html
===
--- clang/www/hacking.html
+++ clang/www/hacking.html
@@ -275,30 +275,8 @@
   Creating Patch Files
   
 
-  To return changes to the Clang team, unless you have checkin
-  privileges, the preferred way is to send patch files
-  https://llvm.org/docs/Contributing.html#how-to-submit-a-patch";>using LLVM's Phabricator with an explanation of what the patch is for. Clang follows https://llvm.org/docs/DeveloperPolicy.html";>LLVM's developer policy.
-  If your patch requires a wider discussion (for example, because it is an
-  architectural change), you can use the cfe-dev mailing list.
-
-  To create these patch files, change directory
-  to the llvm/tools/clang root and run:
-
-  svn diff (relative path) >(patch file name)
-
-  For example, for getting the diffs of all of clang:
-
-  svn diff . >~/mypatchfile.patch
-
-  For example, for getting the diffs of a single file:
-
-  svn diff lib/Parse/ParseDeclCXX.cpp >~/ParseDeclCXX.patch
-
-  Note that the paths embedded in the patch depend on where you run it,
-  so changing directory to the llvm/tools/clang directory is recommended.
-
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use git to contribute to Clang.
+  To contribute changes to Clang see
+https://llvm.org/docs/GettingStarted.html#sending-patches";>LLVM's Getting Started page
 
   
   LLVM IR Generation
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72057: [docs] Remove outdated svn/git information from hacking page

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin added a comment.

In D72057#1802982 , @aaron.ballman 
wrote:

> LGTM with a small typo fix. Do you need someone to commit this on your behalf?


Yes, please. I cannot commit myself.
Alexander Lanin 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72057/new/

https://reviews.llvm.org/D72057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72121: [clang-tidy] Fix readability-identifier-naming missing member variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D72121#1803242 , @njames93 wrote:

> Ahh clang format script must have run, I'll undo that. I've not tried running 
> it against the whole of llvm as I don't know what the easy way to do that is, 
> especially given that llvm disregards it's standard naming convention for old 
> projects and stl like containers. The comment is just in relation to the bug 
> report that this fix is based on, could probably go


yeah clang-format tends to do that, as devs do not consistently apply automatic 
formatting :/

for llvm: https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
You can use `run-clang-tidy.py` in the `tools/` directory of clang-tidy. Then 
specify your custom built clang-tidy binary.

The call should like similar to this (not tested):
`/llvm-project $ run-clang-tidy.py -p 
 -clang-tidy-binary  
-checks=-*,readability-identifier-naming -fix clang/lib`

That will run over clang/libs only. If you choose `lib/` it will run over each 
`lib`-subdirectory of the project.

Maybe you can still try it out and see what diagnostics you get. It will be a 
good practice anyway, if you plan to keep developing :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72121/new/

https://reviews.llvm.org/D72121



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72167: Add support for __declspec(guard(nocf))

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

Thanks for doing this!

When I applied this patch and added `__declspec(guard(nocf))` to the function 
that was giving us trouble 
,
 I still crashed with a CFG failure in the caller, since the `nocf` function 
was inlined. When I additionally marked that function as `noinline`, then the 
CFG checks were omitted, as desired. Is this behavior with respect to inlining 
expected?

You may want to mention "PR44096" in the commit message.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72167/new/

https://reviews.llvm.org/D72167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked an inline comment as done.
JonasToth added inline comments.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1055
+  EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),
+Cat("Object  const*target;"));
+  EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),

aaron.ballman wrote:
> The whitespace looks incorrect here.
It does, but it is actually how it is expected to look.
All pointer transformations create a double-blank if there was a blank before 
the star. My thought was, that clang-format should be used anyway, so it should 
do this cleanup. (especially with the -format option for clang-tidy).
The additional whitespace needs to be inserted to avoid `intconst* target;` for 
`int* target;` variables. Thats why it is always added, if necessary or not.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/test/CodeGen/x86_32-arguments-win32.c:77
+// CHECK-LABEL: define dso_local void @receive_vec_256(<8 x float> inreg %x, 
<8 x float> inreg %y, <8 x float> inreg %z, <8 x float>* %0, <8 x float>* %1)
+// CHECK-LABEL: define dso_local void @receive_vec_512(<16 x float> inreg %x, 
<16 x float> inreg %y, <16 x float> inreg %z, <16 x float>* %0, <16 x float>* 
%1)
+// CHECK-LABEL: define dso_local void @receive_vec_1024(<32 x float>* %0, <32 
x float>* %1, <32 x float>* %2, <32 x float>* %3, <32 x float>* %4)

What happens in the backend with inreg if 512-bit vectors aren't legal?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72114/new/

https://reviews.llvm.org/D72114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp:1055
+  EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),
+Cat("Object  const*target;"));
+  EXPECT_NE(runCheckOnCode(Cat(S), nullptr, "input.m"),

JonasToth wrote:
> aaron.ballman wrote:
> > The whitespace looks incorrect here.
> It does, but it is actually how it is expected to look.
> All pointer transformations create a double-blank if there was a blank before 
> the star. My thought was, that clang-format should be used anyway, so it 
> should do this cleanup. (especially with the -format option for clang-tidy).
> The additional whitespace needs to be inserted to avoid `intconst* target;` 
> for `int* target;` variables. Thats why it is always added, if necessary or 
> not.
> It does, but it is actually how it is expected to look.

Okay, that's fair -- we usually let clang-format handle that sort of thing 
anyway and I cannot think of a case where the transformation would be incorrect.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70073: [ConstExprPreter] Implemented function calls and if statements

[Nandor Licker via Phabricator via cfe-commits]

nand added a comment.

ping


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70073/new/

https://reviews.llvm.org/D70073



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] e5a56f2 - Remove outdated svn/git information from hacking page

[Aaron Ballman via cfe-commits]

Author: Alexander Lanin
Date: 2020-01-03T14:13:40-05:00
New Revision: e5a56f2d50ce1939eba4fddbeb9c8e032db4fc95

URL: 
https://github.com/llvm/llvm-project/commit/e5a56f2d50ce1939eba4fddbeb9c8e032db4fc95
DIFF: 
https://github.com/llvm/llvm-project/commit/e5a56f2d50ce1939eba4fddbeb9c8e032db4fc95.diff

LOG: Remove outdated svn/git information from hacking page

The patch files section is redundant to 
https://llvm.org/docs/GettingStarted.html.
There is nothing clang specific here. We are talking about a monorepo after all.
While it may seem nice to have one single clang page which explains everything,
it's not: It doesn't cover the topics in sufficient depth, it's redundant to
other pages and it's hard to keep it up to date as we see with the svn
instructions.

Added: 


Modified: 
clang/www/hacking.html

Removed: 




diff  --git a/clang/www/hacking.html b/clang/www/hacking.html
index fbcf88b79b3f..3623b904119a 100755
--- a/clang/www/hacking.html
+++ b/clang/www/hacking.html
@@ -275,30 +275,8 @@ Testing on the Command Line
   Creating Patch Files
   
 
-  To return changes to the Clang team, unless you have checkin
-  privileges, the preferred way is to send patch files
-  https://llvm.org/docs/Contributing.html#how-to-submit-a-patch";>using 
LLVM's Phabricator with an explanation of what the patch is for. Clang 
follows https://llvm.org/docs/DeveloperPolicy.html";>LLVM's developer 
policy.
-  If your patch requires a wider discussion (for example, because it is an
-  architectural change), you can use the cfe-dev mailing list.
-
-  To create these patch files, change directory
-  to the llvm/tools/clang root and run:
-
-  svn 
diff  (relative path) >(patch file name)
-
-  For example, for getting the 
diff s of all of clang:
-
-  svn 
diff  . >~/mypatchfile.patch
-
-  For example, for getting the 
diff s of a single file:
-
-  svn 
diff  lib/Parse/ParseDeclCXX.cpp >~/ParseDeclCXX.patch
-
-  Note that the paths embedded in the patch depend on where you run it,
-  so changing directory to the llvm/tools/clang directory is recommended.
-
-  It is also possible to https://llvm.org/docs/GettingStarted.html#sending-patches-with-git";>use 
git to contribute to Clang.
+  To contribute changes to Clang see
+https://llvm.org/docs/GettingStarted.html#sending-patches";>LLVM's 
Getting Started page
 
   
   LLVM IR Generation



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72057: [docs] Remove outdated svn/git information from hacking page

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Thank you for the patch! I've commit on your behalf in 
e5a56f2d50ce1939eba4fddbeb9c8e032db4fc95 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72057/new/

https://reviews.llvm.org/D72057



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D72018#1803144 , @xazax.hun wrote:

> In D72018#1803012 , @aaron.ballman 
> wrote:
>
> > In D72018#1802636 , @NoQ wrote:
> >
> > > Would changing the literal in the attribute have the same effect? I.e., 
> > > `acquire_handle("Fuchsia_But_Please_Ignore_Me")`.
> >
> >
> > It should, but doesn't currently because we don't have any checking that 
> > the string literal matches a name in the static analyzer or clang-tidy for 
> > that attribute.
>
>
> Actually, the static analyzer checker does check for the literal, so this 
> could work.


Huh, that's interesting. I would expect that check to happen in more than just 
the static analyzer though -- for instance, clang-tidy checks may want to use 
those attributes, or even a less-heavy CFG analysis than the static analyzer 
might.

>> 
>> 
>> In D72018#1801739 , @xazax.hun 
>> wrote:
>> 
>>> I think if there are many problems with this concept, I will fall back to 
>>> hard code this information in the checker instead of using an annotation.
>> 
>> 
>> How much hard coded information would this remove? How often do you find 
>> that users want to add to the list of hard coded functions themselves but 
>> can't? Maybe the benefits to the attribute outweigh the downside and it is 
>> worth exploring ways to resolve some of these concerns -- I don't have a 
>> good feeling for the problem space though.
> 
> I expect not to have too many functions that need such exclusions. It was 
> more about making it easier to do some exclusions without touching the 
> analyzer. I will ask around what are the preferences of the team, maybe 
> Artem's proposal will work for us.

Okay, that makes sense to me, thank you!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72018/new/

https://reviews.llvm.org/D72018



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 reopened this revision.
xbolva00 added a comment.
This revision is now accepted and ready to land.

Can you add a test case for that crash? Otherwise OK.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71037/new/

https://reviews.llvm.org/D71037



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72140: [clang-tools-extra] NFC: Fix trivial typos in comments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

This is fantastic, thank you for much for all the typo fixes! This LGTM. Do you 
need someone to commit on your behalf?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72140/new/

https://reviews.llvm.org/D72140



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70638: [Diagnostic] add a warning which warns about misleading indentation

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

(re-ping; I think this false positive for goto label case is important to be 
fixed before 10 release)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70638/new/

https://reviews.llvm.org/D70638



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72018: [attributes] [analyzer] Add an attribute to prevent checkers from modeling a function

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D72018#1803144 , @xazax.hun wrote:

> I expect not to have too many functions that need such exclusions. It was 
> more about making it easier to do some exclusions without touching the 
> analyzer. I will ask around what are the preferences of the team, maybe 
> Artem's proposal will work for us.


I think it ultimately makes sense to introduce analyzer IPA hints ("please 
inline this function", "please evaluate this one conservatively", "please don't 
model effects of this function on checker state", etc.), given that otherwise 
our set of heuristics on this subject is incomprehensible.

But if you have a chance to hardcode a small list of legacy functions with 
weird behavior (that cannot be expressed with annotations; maybe add explicit 
modeling for such functions) and use the analyzer //enforce// the lack of other 
exceptional functions (so that everything else was expressible with 
annotations), that should be the way to go simply because it makes your 
function surfaces much easier to reason about for the programmer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72018/new/

https://reviews.llvm.org/D72018



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] cf48101 - [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via cfe-commits]

Author: Jonas Toth
Date: 2020-01-03T20:37:47+01:00
New Revision: cf48101200ee192dd82e6ed0512ae42e7b3162a9

URL: 
https://github.com/llvm/llvm-project/commit/cf48101200ee192dd82e6ed0512ae42e7b3162a9
DIFF: 
https://github.com/llvm/llvm-project/commit/cf48101200ee192dd82e6ed0512ae42e7b3162a9.diff

LOG: [clang-tidy] implement utility-function to add 'const' to variables

Summary:
This patch extends the already existing facility to add 'const' to variables
to be more flexible and correct. The previous version did not consider pointers
as value AND pointee. For future automatic introduction for const-correctness
this shortcoming needs to be fixed.
It always allows configuration where the 'const' token is inserted, either on
the left side (if possible) or the right side.
It adds many unit-tests to the utility-function that did not exist before, as
the function was implicitly tested through clang-tidy checks. These
tests were not changed, as the API is still compatible.

Reviewers: aaron.ballman, hokein, alexfh, shuaiwang, lebedev.ri

Reviewed By: aaron.ballman

Subscribers: jdoerfert, mgorny, xazax.hun, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D54395

Added: 
clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp

Modified: 
clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
clang-tools-extra/clang-tidy/utils/LexerUtils.h
clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp 
b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
index e0040094259b..67a620227666 100644
--- a/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
+++ b/clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
@@ -13,6 +13,7 @@
 #include "../utils/OptionsUtils.h"
 #include "../utils/TypeTraits.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
+#include "clang/Basic/Diagnostic.h"
 
 using namespace clang::ast_matchers;
 
@@ -77,8 +78,11 @@ bool ForRangeCopyCheck::handleConstValueCopy(const VarDecl 
&LoopVar,
"the loop variable's type is not a reference type; this creates a "
"copy in each iteration; consider making this a reference")
   << utils::fixit::changeVarDeclToReference(LoopVar, Context);
-  if (!LoopVar.getType().isConstQualified())
-Diagnostic << utils::fixit::changeVarDeclToConst(LoopVar);
+  if (!LoopVar.getType().isConstQualified()) {
+if (llvm::Optional Fix = utils::fixit::addQualifierToVarDecl(
+LoopVar, Context, DeclSpec::TQ::TQ_const))
+  Diagnostic << *Fix;
+  }
   return true;
 }
 
@@ -101,11 +105,15 @@ bool ForRangeCopyCheck::handleCopyIsOnlyConstReferenced(
   !utils::decl_ref_expr::allDeclRefExprs(LoopVar, *ForRange.getBody(),
  Context)
.empty()) {
-diag(LoopVar.getLocation(),
- "loop variable is copied but only used as const reference; consider "
- "making it a const reference")
-<< utils::fixit::changeVarDeclToConst(LoopVar)
-<< utils::fixit::changeVarDeclToReference(LoopVar, Context);
+auto Diag = diag(
+LoopVar.getLocation(),
+"loop variable is copied but only used as const reference; consider "
+"making it a const reference");
+
+if (llvm::Optional Fix = utils::fixit::addQualifierToVarDecl(
+LoopVar, Context, DeclSpec::TQ::TQ_const))
+  Diag << *Fix << utils::fixit::changeVarDeclToReference(LoopVar, Context);
+
 return true;
   }
   return false;

diff  --git 
a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp 
b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 7e36b374fbaa..c9ae37d036ca 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -12,6 +12,7 @@
 #include "../utils/FixItHintUtils.h"
 #include "../utils/Matchers.h"
 #include "../utils/OptionsUtils.h"
+#include "clang/Basic/Diagnostic.h"
 
 namespace clang {
 namespace tidy {
@@ -21,8 +22,11 @@ namespace {
 void recordFixes(const VarDecl &Var, ASTContext &Context,
  DiagnosticBuilder &Diagnostic) {
   Diagnostic << utils::fixit::changeVarDeclToReference(Var, Context);
-  if (!Var.getType().isLocalConstQualified())
-Diagnostic << utils::fixit::changeVarDeclToConst(Var);
+  if (!Var.getType().isLocalConstQualified()) {
+if (llvm::Optional Fix = utils::fixit::addQualifier

[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/include/clang/CodeGen/CGFunctionInfo.h:91
   bool InAllocaSRet : 1;// isInAlloca()
+  bool InAllocaIndirect : 1;// isInAlloca()
   bool IndirectByVal : 1;   // isIndirect()

Would it be better to handle `inalloca` differently, maybe as a flag rather 
than as a top-level kind?  I'm concerned about gradually duplicating a 
significant amount of the expressivity of other kinds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72114/new/

https://reviews.llvm.org/D72114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf48101200ee: [clang-tidy] implement utility-function to add 
'const' to variables (authored by JonasToth).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395

Files:
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
  clang-tools-extra/clang-tidy/utils/LexerUtils.h
  clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Index: clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
===
--- clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
 include_directories(${CLANG_LINT_SOURCE_DIR})
 
 add_extra_unittest(ClangTidyTests
+  AddConstTest.cpp
   ClangTidyDiagnosticConsumerTest.cpp
   ClangTidyOptionsTest.cpp
   IncludeInserterTest.cpp
Index: clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/AddConstTest.cpp
@@ -0,0 +1,1081 @@
+#include "../clang-tidy/utils/FixItHintUtils.h"
+#include "ClangTidyDiagnosticConsumer.h"
+#include "ClangTidyTest.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tidy {
+
+namespace {
+using namespace clang::ast_matchers;
+using namespace utils::fixit;
+
+template 
+class ConstTransform : public ClangTidyCheck {
+public:
+  ConstTransform(StringRef CheckName, ClangTidyContext *Context)
+  : ClangTidyCheck(CheckName, Context) {}
+
+  void registerMatchers(MatchFinder *Finder) override {
+Finder->addMatcher(varDecl(hasName("target")).bind("var"), this);
+  }
+
+  void check(const MatchFinder::MatchResult &Result) override {
+const auto *D = Result.Nodes.getNodeAs("var");
+using utils::fixit::addQualifierToVarDecl;
+Optional Fix = addQualifierToVarDecl(
+*D, *Result.Context, DeclSpec::TQ::TQ_const, CT, CP);
+auto Diag = diag(D->getBeginLoc(), "doing const transformation");
+if (Fix)
+  Diag << *Fix;
+  }
+};
+} // namespace
+
+namespace test {
+using PointeeLTransform =
+ConstTransform;
+using PointeeRTransform =
+ConstTransform;
+
+using ValueLTransform =
+ConstTransform;
+using ValueRTransform =
+ConstTransform;
+
+// 
+// Test Value-like types. Everything with indirection is done later.
+// 
+
+TEST(Values, Builtin) {
+  StringRef Snippet = "int target = 0;";
+
+  EXPECT_EQ("const int target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("const int target = 0;",
+runCheckOnCode(Snippet));
+
+  EXPECT_EQ("int const target = 0;", runCheckOnCode(Snippet));
+  EXPECT_EQ("int const target = 0;",
+runCheckOnCode(Snippet));
+}
+TEST(Values, TypedefBuiltin) {
+  StringRef T = "typedef int MyInt;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, TypedefBuiltinPointer) {
+  StringRef T = "typedef int* MyInt;";
+  StringRef S = "MyInt target = nullptr;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = nullptr;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target = nullptr;"),
+runCheckOnCode(Cat(S)));
+}
+TEST(Values, UsingBuiltin) {
+  StringRef T = "using MyInt = int;";
+  StringRef S = "MyInt target = 0;";
+  auto Cat = [&T](StringRef S) { return (T + S).str(); };
+
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("const MyInt target = 0;"),
+runCheckOnCode(Cat(S)));
+
+  EXPECT_EQ(Cat("MyInt const target = 0;"),
+runCheckOnCode(Cat(S)));
+  EXPECT_EQ(Cat("MyInt const target

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Thank you for all the review over the time this took to land! :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72167: Add support for __declspec(guard(nocf))

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you please add some Sema tests that verify the attribute only appertains to 
functions, accepts the right number of arguments, etc? Also, some tests showing 
how this attribute works for things like redelcarations and virtual functions 
would help. e.g.,

  void f();
  __declspec(guard(nocf)) void f() {} // Is this fine?
  
  __declspec(guard(nocf)) void g();
  void g() {} // How about this?
  
  struct S {
__declspec(guard(nocf)) virtual void f();
  };
  
  struct T : S {
void f() override; // Should this be guard(nocf) as well?
  };




Comment at: clang/include/clang/Basic/AttrDocs.td:4568
+where the programmer has manually inserted "CFG-equivalent" protection. The 
+programmer knows that they are calling through some read only function table 
+whose address is obtained through read only memory references and for which 
the 

read only -> read-only



Comment at: clang/include/clang/Basic/AttrDocs.td:4569
+programmer knows that they are calling through some read only function table 
+whose address is obtained through read only memory references and for which 
the 
+index is masked to the function table limit. This approach may also be applied 

read only -> read-only



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6635
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_n_type)
+<< AL << 0 << AANT_ArgumentIdentifier;

I think this should use `err_attribute_argument_type` instead, since there's 
only one argument.



Comment at: clang/test/CodeGen/guard_nocf.c:15
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
\ No newline at end of file


Please add a newline to the end of the file.



Comment at: clang/test/CodeGenCXX/guard_nocf.cpp:15
+// CHECK-NOT: attributes [[CF]] = { {{.*}}"guard_nocf"{{.*}} }
\ No newline at end of file


Please add a newline to the end of the file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72167/new/

https://reviews.llvm.org/D72167



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36051: Move from a long list of checkers to tables

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

This list used to be automatically updated by the 
clang-tools-extra/clang-tidy/add_new_check.py script. I suppose, this is broken 
now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D36051/new/

https://reviews.llvm.org/D36051



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

[Reid Kleckner via Phabricator via cfe-commits]

rnk marked an inline comment as done.
rnk added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:1648
+  // a bit different than the x64 version.  First, all vector types (not HVAs)
+  // are assigned, with the first 6 ending up in the YMM0-5 or XMM0-5 
registers.
+  // This differs from the x64 implementation, where the first 6 by INDEX get

craig.topper wrote:
> What about ZMM?
This comment was pre-existing, but we can say [XYZ]MM0-5.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72110/new/

https://reviews.llvm.org/D72110



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D54943#1800182 , @0x8000- wrote:

> F11163406: 0001-Add-extra-tests.patch  
> shows a couple of false positives.


I added your tests, but some did not show false positives anymore. I think i 
will remove some, that i find redundant.
But could you please recheck with the current version first, if this is 
actually correct and the original false positives are gone?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54943/new/

https://reviews.llvm.org/D54943



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

[Reid Kleckner via Phabricator via cfe-commits]

rnk marked 2 inline comments as done.
rnk added inline comments.



Comment at: clang/test/CodeGen/x86_32-arguments-win32.c:77
+// CHECK-LABEL: define dso_local void @receive_vec_256(<8 x float> inreg %x, 
<8 x float> inreg %y, <8 x float> inreg %z, <8 x float>* %0, <8 x float>* %1)
+// CHECK-LABEL: define dso_local void @receive_vec_512(<16 x float> inreg %x, 
<16 x float> inreg %y, <16 x float> inreg %z, <16 x float>* %0, <16 x float>* 
%1)
+// CHECK-LABEL: define dso_local void @receive_vec_1024(<32 x float>* %0, <32 
x float>* %1, <32 x float>* %2, <32 x float>* %3, <32 x float>* %4)

craig.topper wrote:
> What happens in the backend with inreg if 512-bit vectors aren't legal?
LLVM splits the vector up using the largest legal vector size. As many pieces 
as possible are passed in available XMM/YMM registers, and the rest are passed 
in memory. MSVC, of course, assumes the user wanted the larger vector size, and 
uses whatever vector instructions it needs to move the arguments around.

Previously, I advocated for a model where calling an Intel intrinsic function 
had the effect of implicitly marking the caller with the target attributes of 
the intrinsic. This falls down if the user tries to write a single function 
that conditionally branches between code that uses different instruction set 
extensions. You can imagine the SSE2 codepath accidentally using AVX 
instructions because the compiler thinks they are better. I'm told that ICC 
models CPU micro-architectural features in the CFG, but I don't ever expect 
that LLVM will do that. If we're stuck with per-function CPU feature settings, 
it seems like it would be nice to try to do what the user asked by default, and 
warn the user if we see them doing a cpuid check in a function that has been 
implicitly blessed with some target attributes. You could imagine doing a 
similar thing when large vector type variables are used: if a large vector 
argument or local is used, implicitly enable the appropriate target features to 
move vectors of that size around.

This idea didn't get anywhere, and the current situation has persisted.

---

You know, maybe we should just keep clang the way it is, and just set up a 
warning in the backend that says "hey, I split your large vector. You probably 
didn't want that." And then we just continue doing what we do now. Nobody likes 
backend warnings, but it seems better than the current direction of the 
frontend knowing every detail of x86 vector extensions.



Comment at: clang/test/CodeGenCXX/inalloca-vector.cpp:71
+// CHECK-LABEL: define dso_local x86_vectorcallcc void 
@"?vectorcall_receive_vec@@Y{{[^"]*}}"
+// CHECKX-SAME: (double inreg %xmm0,
+// CHECKX-SAME: double inreg %xmm1,

erichkeane wrote:
> Are all the checks hre on out disabled for a reason?
Yes, this is case 2 in the commit message. I won't close the bug without coming 
back to this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72114/new/

https://reviews.llvm.org/D72114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 236101.
JonasToth added a comment.

- ignore implicit casts in memberExpr(hasObjectExpression())
- implement functionality for function pointers


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54943/new/

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -62,13 +62,18 @@
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
-std::string buffer;
+if (!By)
+  break;
+
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
-Chain.push_back(StringRef(stream.str()).trim().str());
+Chain.emplace_back(StringRef(stream.str()).trim().str());
 E = dyn_cast(By);
+buffer.clear();
   }
   return Chain;
 }
@@ -882,38 +887,36 @@
 }
 
 TEST(ExprMutationAnalyzerTest, CommaExprWithAnAssigment) {
-  const auto AST =
-  buildASTFromCodeWithArgs("void f() { int x; int y; (x, y) = 5; }",
-   {"-Wno-unused-value"});
+  const auto AST = buildASTFromCodeWithArgs(
+  "void f() { int x; int y; (x, y) = 5; }", {"-Wno-unused-value"});
   const auto Results =
   match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, CommaExprWithDecOp) {
-  const auto AST =
-  buildASTFromCodeWithArgs("void f() { int x; int y; (x, y)++; }",
-   {"-Wno-unused-value"});
+  const auto AST = buildASTFromCodeWithArgs(
+  "void f() { int x; int y; (x, y)++; }", {"-Wno-unused-value"});
   const auto Results =
   match(withEnclosingCompound(declRefTo("y")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, CommaExprWithNonConstMemberCall) {
-  const auto AST =
-  buildASTFromCodeWithArgs("class A { public: int mem; void f() { mem ++; } };"
-   "void fn() { A o1, o2; (o1, o2).f(); }",
-   {"-Wno-unused-value"});
+  const auto AST = buildASTFromCodeWithArgs(
+  "class A { public: int mem; void f() { mem ++; } };"
+  "void fn() { A o1, o2; (o1, o2).f(); }",
+  {"-Wno-unused-value"});
   const auto Results =
   match(withEnclosingCompound(declRefTo("o2")), AST->getASTContext());
   EXPECT_TRUE(isMutated(Results, AST.get()));
 }
 
 TEST(ExprMutationAnalyzerTest, CommaExprWithConstMemberCall) {
-  const auto AST =
-  buildASTFromCodeWithArgs("class A { public: int mem; void f() const  { } };"
-   "void fn() { A o1, o2; (o1, o2).f(); }",
-   {"-Wno-unused-value"});
+  const auto AST = buildASTFromCodeWithArgs(
+  "class A { public: int mem; void f() const  { } };"
+  "void fn() { A o1, o2; (o1, o2).f(); }",
+  {"-Wno-unused-value"});
   const auto Results =
   match(withEnclosingCompound(declRefTo("o2")), AST->getASTContext());
   EXPECT_FALSE(isMutated(Results, AST.get()));
@@ -957,11 +960,10 @@
 }
 
 TEST(ExprMutationAnalyzerTest, CommaExprWithAmpersandOp) {
-  const auto AST =
-  buildASTFromCodeWithArgs("class A { public: int mem;};"
-   "void fn () { A o1, o2;"
- 

[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked 2 inline comments as done.
JonasToth added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5224
 
+/// Matches QualType nodes that are of function pointer type.
+///

I will remove those later. They are not used.
Tidying up will be done, once i dont find any false positives/false 
transformations anymore.



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:27
 
-AST_MATCHER_P(Expr, maybeEvalCommaExpr,
- ast_matchers::internal::Matcher, InnerMatcher) {
-  const Expr* Result = &Node;
+AST_MATCHER_P(Expr, maybeEvalCommaExpr, ast_matchers::internal::Matcher,
+  InnerMatcher) {

these are formating changes, i will revert them / apply them as separate commit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54943/new/

https://reviews.llvm.org/D54943



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72114: [MS] Overhaul how clang passes overaligned args on x86_32

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/test/CodeGen/x86_32-arguments-win32.c:77
+// CHECK-LABEL: define dso_local void @receive_vec_256(<8 x float> inreg %x, 
<8 x float> inreg %y, <8 x float> inreg %z, <8 x float>* %0, <8 x float>* %1)
+// CHECK-LABEL: define dso_local void @receive_vec_512(<16 x float> inreg %x, 
<16 x float> inreg %y, <16 x float> inreg %z, <16 x float>* %0, <16 x float>* 
%1)
+// CHECK-LABEL: define dso_local void @receive_vec_1024(<32 x float>* %0, <32 
x float>* %1, <32 x float>* %2, <32 x float>* %3, <32 x float>* %4)

rnk wrote:
> craig.topper wrote:
> > What happens in the backend with inreg if 512-bit vectors aren't legal?
> LLVM splits the vector up using the largest legal vector size. As many pieces 
> as possible are passed in available XMM/YMM registers, and the rest are 
> passed in memory. MSVC, of course, assumes the user wanted the larger vector 
> size, and uses whatever vector instructions it needs to move the arguments 
> around.
> 
> Previously, I advocated for a model where calling an Intel intrinsic function 
> had the effect of implicitly marking the caller with the target attributes of 
> the intrinsic. This falls down if the user tries to write a single function 
> that conditionally branches between code that uses different instruction set 
> extensions. You can imagine the SSE2 codepath accidentally using AVX 
> instructions because the compiler thinks they are better. I'm told that ICC 
> models CPU micro-architectural features in the CFG, but I don't ever expect 
> that LLVM will do that. If we're stuck with per-function CPU feature 
> settings, it seems like it would be nice to try to do what the user asked by 
> default, and warn the user if we see them doing a cpuid check in a function 
> that has been implicitly blessed with some target attributes. You could 
> imagine doing a similar thing when large vector type variables are used: if a 
> large vector argument or local is used, implicitly enable the appropriate 
> target features to move vectors of that size around.
> 
> This idea didn't get anywhere, and the current situation has persisted.
> 
> ---
> 
> You know, maybe we should just keep clang the way it is, and just set up a 
> warning in the backend that says "hey, I split your large vector. You 
> probably didn't want that." And then we just continue doing what we do now. 
> Nobody likes backend warnings, but it seems better than the current direction 
> of the frontend knowing every detail of x86 vector extensions.
If target attributes affect ABI, it seems really dangerous to implicitly set 
attributes based on what intrinsics are called.

The local CPU-testing problem seems similar to the problems with local `#pragma 
STDC FENV_ACCESS` blocks that the constrained-FP people are looking into.  They 
both have a "this operation is normally fully optimizable, but we might need to 
be more careful in specific functions" aspect to them.  I wonder if there's a 
reasonable way to unify the approaches, or at least benefit from lessons 
learned.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72114/new/

https://reviews.llvm.org/D72114



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] c0f1eac - [SystemZ] Don't allow CL option -mpacked-stack with -mbackchain.

[Jonas Paulsson via cfe-commits]

Author: Jonas Paulsson
Date: 2020-01-03T12:26:54-08:00
New Revision: c0f1eac008e61e8345e3f41347cfd191e4ecb215

URL: 
https://github.com/llvm/llvm-project/commit/c0f1eac008e61e8345e3f41347cfd191e4ecb215
DIFF: 
https://github.com/llvm/llvm-project/commit/c0f1eac008e61e8345e3f41347cfd191e4ecb215.diff

LOG: [SystemZ]  Don't allow CL option -mpacked-stack with -mbackchain.

-mpacked-stack is currently not supported with -mbackchain, so this should
result in a compilation error message instead of being silently ignored.

Review: Ulrich Weigand

Added: 
clang/test/Driver/mbackchain.c

Modified: 
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 86aee334436a..808cca76c6be 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2481,6 +2481,7 @@ def mrecord_mcount : Flag<["-"], "mrecord-mcount">, 
HelpText<"Generate a __mcoun
   Flags<[CC1Option]>, Group;
 def mpacked_stack : Flag<["-"], "mpacked-stack">, HelpText<"Use packed stack 
layout (SystemZ only).">,
   Flags<[CC1Option]>, Group;
+def mno_packed_stack : Flag<["-"], "mno-packed-stack">, Flags<[CC1Option]>, 
Group;
 def mips16 : Flag<["-"], "mips16">, Group;
 def mno_mips16 : Flag<["-"], "mno-mips16">, Group;
 def mmicromips : Flag<["-"], "mmicromips">, Group;

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 1b8eca0ea0d7..e75dd6badc14 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -1991,8 +1991,20 @@ void Clang::AddSparcTargetArgs(const ArgList &Args,
 
 void Clang::AddSystemZTargetArgs(const ArgList &Args,
  ArgStringList &CmdArgs) const {
-  if (Args.hasFlag(options::OPT_mbackchain, options::OPT_mno_backchain, false))
+  bool HasBackchain = Args.hasFlag(options::OPT_mbackchain,
+   options::OPT_mno_backchain, false);
+  bool HasPackedStack = Args.hasFlag(options::OPT_mpacked_stack,
+ options::OPT_mno_packed_stack, false);
+  if (HasBackchain && HasPackedStack) {
+const Driver &D = getToolChain().getDriver();
+D.Diag(diag::err_drv_unsupported_opt)
+  << Args.getLastArg(options::OPT_mpacked_stack)->getAsString(Args) +
+  " " + Args.getLastArg(options::OPT_mbackchain)->getAsString(Args);
+  }
+  if (HasBackchain)
 CmdArgs.push_back("-mbackchain");
+  if (HasPackedStack)
+CmdArgs.push_back("-mpacked-stack");
 }
 
 void Clang::AddX86TargetArgs(const ArgList &Args,
@@ -5014,8 +5026,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
 }
   }
 
-  Args.AddLastArg(CmdArgs, options::OPT_mpacked_stack);
-
   if (Args.getLastArg(options::OPT_fapple_kext) ||
   (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
 CmdArgs.push_back("-fapple-kext");

diff  --git a/clang/test/Driver/mbackchain.c b/clang/test/Driver/mbackchain.c
new file mode 100644
index ..33076829ccd7
--- /dev/null
+++ b/clang/test/Driver/mbackchain.c
@@ -0,0 +1,3 @@
+// RUN: %clang -target s390x -c -### %s -mpacked-stack -mbackchain 2>&1 | 
FileCheck %s
+
+// CHECK: error: unsupported option '-mpacked-stack -mbackchain'

diff  --git a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp 
b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
index 3e2b376d3be6..3cdf6bf98ee0 100644
--- a/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
+++ b/llvm/lib/Target/SystemZ/SystemZFrameLowering.cpp
@@ -68,6 +68,8 @@ static bool usePackedStack(MachineFunction &MF) {
   bool CallConv = MF.getFunction().getCallingConv() != CallingConv::GHC;
   bool BackChain = MF.getFunction().hasFnAttribute("backchain");
   bool FramAddressTaken = MF.getFrameInfo().isFrameAddressTaken();
+  if (HasPackedStackAttr && BackChain)
+report_fatal_error("packed-stack with backchain is currently 
unsupported.");
   return HasPackedStackAttr && !IsVarArg && CallConv && !BackChain &&
  !FramAddressTaken;
 }



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70764: build: reduce CMake handling for zlib

[Khem Raj via Phabricator via cfe-commits]

raj.khem added a comment.

this is now in master, and I am seeing build failures in cross-building clang, 
e.g. when building clang for arm on a x86_64 host. its resorting to finding, 
libz from buildhost instead of target sysroot ( using --sysroot) and failing in 
link step. e.g.

FAILED: bin/llvm-config
...
 -o bin/llvm-config  -Wl,-rpath,"\$ORIGIN/../lib"  lib/libLLVMSupport.a  
/usr/lib/libz.so  -lrt  -ldl  -ltinfo  -lm  lib/libLLVMDemangle.a
...

aarch64-yoe-linux-musl/aarch64-yoe-linux-musl-ld: /usr/lib/libz.so: error 
adding symbols: file in wrong format
clang-10: error: linker command failed with exit code 1 (use -v to see 
invocation)

you can see that its adding /usr/lib/libz.so to linker cmdline while cross 
linking.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70764/new/

https://reviews.llvm.org/D70764



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests on Windows: http://45.33.8.238/win/5061/step_8.txt

Please take a look, and if takes a while to investigate, please revert in the 
meantime. (Probably needs the fno-delayed-template-instantiation kludge that's 
in many other tests.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 05ebaa6 - [clang-tidy] fix broken linking for AddConstTest with adding clangSema as dependency (DeclSpec)

[Jonas Toth via cfe-commits]

Author: Jonas Toth
Date: 2020-01-03T21:55:51+01:00
New Revision: 05ebaa62e0db67d7a04d47b2f50eb2faa8597cc8

URL: 
https://github.com/llvm/llvm-project/commit/05ebaa62e0db67d7a04d47b2f50eb2faa8597cc8
DIFF: 
https://github.com/llvm/llvm-project/commit/05ebaa62e0db67d7a04d47b2f50eb2faa8597cc8.diff

LOG: [clang-tidy] fix broken linking for AddConstTest with adding clangSema as 
dependency (DeclSpec)

Added: 


Modified: 
clang-tools-extra/unittests/clang-tidy/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt 
b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
index 297c793855a5..f36d7cb430a6 100644
--- a/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
+++ b/clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
@@ -29,6 +29,7 @@ clang_target_link_libraries(ClangTidyTests
   clangBasic
   clangFrontend
   clangLex
+  clangSema
   clangSerialization
   clangTooling
   clangToolingCore



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71037: [Diagnostic] Add ftabstop to -Wmisleading-indentation

[Tyker via Phabricator via cfe-commits]

Tyker added a comment.

In D71037#1803361 , @xbolva00 wrote:

> Can you add a test case for that crash? Otherwise OK.


as i said. the bug can only occur for one line files. so we can't have a run 
line and a the crashing line. so i wasn't able to make a test for it.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71037/new/

https://reviews.llvm.org/D71037



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D72110: [X86] ABI compat bugfix for MSVC vectorcall

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 236115.
rnk added a comment.

- ZYXMM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72110/new/

https://reviews.llvm.org/D72110

Files:
  clang/include/clang/CodeGen/CGFunctionInfo.h
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/vectorcall.c

Index: clang/test/CodeGen/vectorcall.c
===
--- clang/test/CodeGen/vectorcall.c
+++ clang/test/CodeGen/vectorcall.c
@@ -116,3 +116,24 @@
 // X32: define dso_local x86_vectorcallcc void @"\01HVAAnywhere@@88"(%struct.HFA2 inreg %p1.coerce, i32 inreg %p2, i32 inreg %p3, float %p4, i32 %p5, i32 %p6, %struct.HFA4* %p7, %struct.HFA2 inreg %p8.coerce, float %p9)
 // X64: define dso_local x86_vectorcallcc void @"\01HVAAnywhere@@112"(%struct.HFA2 inreg %p1.coerce, i32 %p2, i32 %p3, float %p4, i32 %p5, i32 %p6, %struct.HFA4* %p7, %struct.HFA2 inreg %p8.coerce, float %p9) 
 
+#ifndef __x86_64__
+// This covers the three ways XMM values can be passed on 32-bit x86:
+// - directly in XMM register (xmm5)
+// - indirectly by address, address in GPR (ecx)
+// - indirectly by address, address on stack
+void __vectorcall vectorcall_indirect_vec(
+double xmm0, double xmm1, double xmm2, double xmm3, double xmm4,
+v4f32 xmm5, v4f32 ecx, int edx, v4f32 mem) {
+}
+
+// X32: define dso_local x86_vectorcallcc void @"\01vectorcall_indirect_vec@@{{[0-9]+}}"
+// X32-SAME: (double %xmm0,
+// X32-SAME: double %xmm1,
+// X32-SAME: double %xmm2,
+// X32-SAME: double %xmm3,
+// X32-SAME: double %xmm4,
+// X32-SAME: <4 x float> %xmm5,
+// X32-SAME: <4 x float>* inreg %0,
+// X32-SAME: i32 inreg %edx,
+// X32-SAME: <4 x float>* %1)
+#endif
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -22,6 +22,7 @@
 #include "clang/Basic/CodeGenOptions.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
+#include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
@@ -996,11 +997,13 @@
 
 /// Similar to llvm::CCState, but for Clang.
 struct CCState {
-  CCState(unsigned CC) : CC(CC), FreeRegs(0), FreeSSERegs(0) {}
+  CCState(CGFunctionInfo &FI)
+  : IsPreassigned(FI.arg_size()), CC(FI.getCallingConvention()) {}
 
-  unsigned CC;
-  unsigned FreeRegs;
-  unsigned FreeSSERegs;
+  llvm::SmallBitVector IsPreassigned;
+  unsigned CC = CallingConv::CC_C;
+  unsigned FreeRegs = 0;
+  unsigned FreeSSERegs = 0;
 };
 
 enum {
@@ -1071,8 +1074,7 @@
   void addFieldToArgStruct(SmallVector &FrameFields,
CharUnits &StackOffset, ABIArgInfo &Info,
QualType Type) const;
-  void computeVectorCallArgs(CGFunctionInfo &FI, CCState &State,
- bool &UsedInAlloca) const;
+  void runVectorCallFirstPass(CGFunctionInfo &FI, CCState &State) const;
 
 public:
 
@@ -1640,9 +1642,38 @@
   return true;
 }
 
+void X86_32ABIInfo::runVectorCallFirstPass(CGFunctionInfo &FI, CCState &State) const {
+  // Vectorcall x86 works subtly different than in x64, so the format is
+  // a bit different than the x64 version.  First, all vector types (not HVAs)
+  // are assigned, with the first 6 ending up in the [XYZ]MM0-5 registers.
+  // This differs from the x64 implementation, where the first 6 by INDEX get
+  // registers.
+  // In the second pass over the arguments, HVAs are passed in the remaining
+  // vector registers if possible, or indirectly by address. The address will be
+  // passed in ECX/EDX if available. Any other arguments are passed according to
+  // the usual fastcall rules.
+  MutableArrayRef Args = FI.arguments();
+  for (int I = 0, E = Args.size(); I < E; ++I) {
+const Type *Base = nullptr;
+uint64_t NumElts = 0;
+const QualType &Ty = Args[I].type;
+if ((Ty->isVectorType() || Ty->isBuiltinType()) &&
+isHomogeneousAggregate(Ty, Base, NumElts)) {
+  if (State.FreeSSERegs >= NumElts) {
+State.FreeSSERegs -= NumElts;
+Args[I].info = ABIArgInfo::getDirect();
+State.IsPreassigned.set(I);
+  }
+}
+  }
+}
+
 ABIArgInfo X86_32ABIInfo::classifyArgumentType(QualType Ty,
CCState &State) const {
   // FIXME: Set alignment on indirect arguments.
+  bool IsFastCall = State.CC == llvm::CallingConv::X86_FastCall;
+  bool IsRegCall = State.CC == llvm::CallingConv::X86_RegCall;
+  bool IsVectorCall = State.CC == llvm::CallingConv::X86_VectorCall;
 
   Ty = useFirstFieldIfTransparentUnion(Ty);
 
@@ -1662,11 +1693,16 @@
   // to other targets.
   const Type *Base = nullptr;
   uint64_t NumElts = 0;
-  if (State.CC == llvm::CallingConv::X86_RegCall &&
+  if ((IsRegCall || IsVectorCall) &&
   isHomogeneousAggregate(Ty, Base, NumElts)) {
-
 if (State.F

[clang-tools-extra] fed2a50 - [clang-tidy] quickfix: add -fno-delayed-template-parsing as default argument for runCheckOnCode unit-tests to unbreak windows

[Jonas Toth via cfe-commits]

Author: Jonas Toth
Date: 2020-01-03T22:02:11+01:00
New Revision: fed2a5033af564af390faa8f8438018fe747126a

URL: 
https://github.com/llvm/llvm-project/commit/fed2a5033af564af390faa8f8438018fe747126a
DIFF: 
https://github.com/llvm/llvm-project/commit/fed2a5033af564af390faa8f8438018fe747126a.diff

LOG: [clang-tidy] quickfix: add -fno-delayed-template-parsing as default 
argument for runCheckOnCode unit-tests to unbreak windows

Added: 


Modified: 
clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h

Removed: 




diff  --git a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h 
b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
index 36727ec7662e..5e62a199b78d 100644
--- a/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
+++ b/clang-tools-extra/unittests/clang-tidy/ClangTidyTest.h
@@ -97,6 +97,7 @@ runCheckOnCode(StringRef Code, std::vector 
*Errors = nullptr,
 
   std::vector Args(1, "clang-tidy");
   Args.push_back("-fsyntax-only");
+  Args.push_back("-fno-delayed-template-parsing");
   std::string extension(llvm::sys::path::extension(Filename.str()));
   if (extension == ".m" || extension == ".mm") {
 Args.push_back("-fobjc-abi-version=2");



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D54395#1803552 , @thakis wrote:

> This breaks tests on Windows: http://45.33.8.238/win/5061/step_8.txt
>
> Please take a look, and if takes a while to investigate, please revert in the 
> meantime. (Probably needs the fno-delayed-template-instantiation kludge 
> that's in many other tests.)


you are right. I belive it is "-fno-delayed-template-parsing" though. I added 
it to the default arguments `runCheckOnCode` for quickfix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54395/new/

https://reviews.llvm.org/D54395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits