[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D52835#1263032, @xbolva00 wrote:

> As noted, this in under LiteralConversion (-Wconversion). GCC has it too 
> under -Wconversion, so I think it is fine as is, or? 
>  @rsmith


It's not so much about "which flag group do i need to enable to get the 
warning";
it's about "which flag do i need to disable to silence it", and "how 
fine-grained that flag is, how many warnings will get disabled".
The flags should be pretty fine-grained.


https://reviews.llvm.org/D52835



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

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

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

xbolva00 added a comment.

So any suggestions?


https://reviews.llvm.org/D52835



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

[PATCH] D53186: [clangd] Support hover on "aut^o *".

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: arphaman, jkorous, MaskRay, ioeric, ilya-biryukov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53186

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -755,6 +755,15 @@
   )cpp",
   "int",
   },
+  {
+  R"cpp(// Simple initialization with auto*
+void foo() {
+  int a = 1;
+  ^auto* i = &a;
+}
+  )cpp",
+  "int",
+  },
   {
   R"cpp(// Auto with initializer list.
 namespace std
@@ -862,6 +871,16 @@
   )cpp",
   "struct Bar",
   },
+  {
+  R"cpp(// auto* in function return
+struct Bar {};
+^auto* test() {
+  Bar* bar;
+  return bar;
+}
+  )cpp",
+  "struct Bar",
+  },
   {
   R"cpp(// const auto& in function return
 struct Bar {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -579,20 +579,28 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
+  // Remove the surrounding Reference or Pointer type of the given type T.
+  QualType UnwrapReferenceOrPointer(QualType T) {
+// "auto &" is represented as a ReferenceType containing an AutoType
+if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
+  return RT->getPointeeType();
+// "auto *" is represented as a PointerType containing an AutoType
+if (const PointerType *PT = dyn_cast(T.getTypePtr()))
+  return PT->getPointeeType();
+return T;
+  }
+
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
   //- auto& i = 1;
+  //- auto* i = &a;
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
 if (!D->getTypeSourceInfo() ||
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = D->getType();
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(DeclT.getTypePtr()))
-  DeclT = RT->getPointeeType();
-
+auto DeclT = UnwrapReferenceOrPointer(D->getType());
 const AutoType *AT = dyn_cast(DeclT.getTypePtr());
 if (AT && !AT->getDeducedType().isNull()) {
   // For auto, use the underlying type because the const& would be
@@ -626,11 +634,7 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = D->getReturnType();
-// "auto &" is represented as a ReferenceType containing an AutoType.
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  T = RT->getPointeeType();
-
+auto T = UnwrapReferenceOrPointer(D->getReturnType());
 const AutoType *AT = dyn_cast(T.getTypePtr());
 if (AT && !AT->getDeducedType().isNull()) {
   DeducedType = T.getUnqualifiedType();


Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -755,6 +755,15 @@
   )cpp",
   "int",
   },
+  {
+  R"cpp(// Simple initialization with auto*
+void foo() {
+  int a = 1;
+  ^auto* i = &a;
+}
+  )cpp",
+  "int",
+  },
   {
   R"cpp(// Auto with initializer list.
 namespace std
@@ -862,6 +871,16 @@
   )cpp",
   "struct Bar",
   },
+  {
+  R"cpp(// auto* in function return
+struct Bar {};
+^auto* test() {
+  Bar* bar;
+  return bar;
+}
+  )cpp",
+  "struct Bar",
+  },
   {
   R"cpp(// const auto& in function return
 struct Bar {};
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -579,20 +579,28 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
+  // Remove the surrounding Reference or Pointer type of the given type T.
+  QualType UnwrapReferenceOrPointer(QualType T) {
+// "auto &" is represented as a ReferenceType containing an AutoType
+if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
+  return RT->getPointeeType();
+// "auto *" is represented as a PointerType containing an AutoType
+if (const PointerType *PT = dyn_cast(T.getTypePtr()))
+  return PT->getPointeeType();
+return T;
+  }
+
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
   //- auto& i = 1;
+  //- auto* i = &a;
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
 if (!D->getTypeSourceInfo() ||
  

[PATCH] D53024: [analyzer][www] Add more open projects

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

In https://reviews.llvm.org/D53024#1262976, @george.karpenkov wrote:

> @Szelethus I take it this is mostly formed from @NoQ email? Language could 
> use polishing in quite a few places, could I just commandeer this revision 
> and try to fix it?


Yes it is. Though the other item you mentioned should be on this list -- I just 
simply forgot to put it there before updating the diff O:). Feel free to 
commandeer it, this patch is barely my work.


https://reviews.llvm.org/D53024



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

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added inline comments.



Comment at: test/Driver/arm-mfpu.c:410
+
+// RUN: %clang -target armv7-linux-androideabi23 %s -mfpu=vfp3-d16 -### -c 
2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ARM-ANDROID-M-FP-D16 %s

danalbert wrote:
> >>! In D53121#1261602, @kristof.beyls wrote:
> > Seems fine to me too. I'd maybe just add an additional test case to verify 
> > that things still work as expected when users explicitly specify that they 
> > want to target a different FPU (e.g. "-mfpu=none").
> 
> Is this test (and it's counterpart in `CHECK-ARM-ANDROID-L-FP-NEON`) not 
> sufficient? It shows that `-mfpu` is honored regardless of the default. Is 
> there something special about `-mfpu=none` that this doesn't exercise?
you're right - this test does what I was asking for. Apologies, I should've 
looked more closely - I hadn't picked up this mfpu isn't the default one...


Repository:
  rC Clang

https://reviews.llvm.org/D53121



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

[PATCH] D53121: [Driver] Add defaults for Android ARM FPUs.

[Peter Smith via Phabricator via cfe-commits]

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

Looks good to me. By generic target I just meant 
--target=arm-linux-androideabi21 with a -march to specify the revision, which 
you've got in the test.


Repository:
  rC Clang

https://reviews.llvm.org/D53121



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

[PATCH] D53187: [clang-tidy] Optimize query in bugprone-exception-escape

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware created this revision.
baloghadamsoftware added reviewers: JonasToth, alexfh, aaron.ballman.
baloghadamsoftware added a project: clang-tools-extra.
Herald added subscribers: Szelethus, dkrupp, rnkovacs, xazax.hun, whisperity.

Checking whether a functions throws indirectly may be very expensive because it 
needs to visit its whole call graph. Therefore we should first check whether 
the function is forbidden to throw and only check whether it throws afterward. 
This also seems to solve bug bug 39167 
 where the execution time is so 
long that it seems to hang.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53187

Files:
  clang-tidy/bugprone/ExceptionEscapeCheck.cpp
  test/clang-tidy/bugprone-exception-escape.cpp


Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- test/clang-tidy/bugprone-exception-escape.cpp
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -258,6 +258,31 @@
   throw ignored1();
 }
 
+void thrower(int n) {
+  throw n;
+}
+
+int directly_recursive(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in 
funcion 'directly_recursive' which should not throw exceptions
+  if (n == 0)
+thrower(n);
+  return directly_recursive(n);
+}
+
+int indirectly_recursive(int n) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in 
functin 'indirectly_recursive' which should not throw exceptions
+
+int recursion_helper(int n) {
+  indirectly_recursive(n);
+}
+
+int indirectly_recursive(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in 
funcion 'indirectly_recursive' which should not throw exceptions
+  if (n == 0)
+thrower(n);
+  return recursion_helper(n);
+}
+
 int main() {
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in 
function 'main' which should not throw exceptions
   throw 1;
Index: clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -190,12 +190,12 @@
 return;
 
   Finder->addMatcher(
-  functionDecl(allOf(throws(unless(isIgnored(IgnoredExceptions))),
- anyOf(isNoThrow(), cxxDestructorDecl(),
+  functionDecl(allOf(anyOf(isNoThrow(), cxxDestructorDecl(),
cxxConstructorDecl(isMoveConstructor()),
cxxMethodDecl(isMoveAssignmentOperator()),
hasName("main"), hasName("swap"),
-   isEnabled(FunctionsThatShouldNotThrow
+   isEnabled(FunctionsThatShouldNotThrow)),
+ throws(unless(isIgnored(IgnoredExceptions)
   .bind("thrower"),
   this);
 }


Index: test/clang-tidy/bugprone-exception-escape.cpp
===
--- test/clang-tidy/bugprone-exception-escape.cpp
+++ test/clang-tidy/bugprone-exception-escape.cpp
@@ -258,6 +258,31 @@
   throw ignored1();
 }
 
+void thrower(int n) {
+  throw n;
+}
+
+int directly_recursive(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in funcion 'directly_recursive' which should not throw exceptions
+  if (n == 0)
+thrower(n);
+  return directly_recursive(n);
+}
+
+int indirectly_recursive(int n) noexcept;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in functin 'indirectly_recursive' which should not throw exceptions
+
+int recursion_helper(int n) {
+  indirectly_recursive(n);
+}
+
+int indirectly_recursive(int n) noexcept {
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in funcion 'indirectly_recursive' which should not throw exceptions
+  if (n == 0)
+thrower(n);
+  return recursion_helper(n);
+}
+
 int main() {
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: an exception may be thrown in function 'main' which should not throw exceptions
   throw 1;
Index: clang-tidy/bugprone/ExceptionEscapeCheck.cpp
===
--- clang-tidy/bugprone/ExceptionEscapeCheck.cpp
+++ clang-tidy/bugprone/ExceptionEscapeCheck.cpp
@@ -190,12 +190,12 @@
 return;
 
   Finder->addMatcher(
-  functionDecl(allOf(throws(unless(isIgnored(IgnoredExceptions))),
- anyOf(isNoThrow(), cxxDestructorDecl(),
+  functionDecl(allOf(anyOf(isNoThrow(), cxxDestructorDecl(),
cxxConstructorDecl(isMoveConstructor()),
cxxMethodDecl(isMoveAssignmentOperator()),
hasName("main"), hasName("swap"),
-   isEnabled(FunctionsThatShouldNotThrow
+

[PATCH] D52971: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py to support multiple prefixes

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

In https://reviews.llvm.org/D52971#1262200, @ymandel wrote:

> Hi,
>
> It looks like this change has disabled FileCheck for all clang-tidy lit tests 
> that don't use check-prefixes.  So, they all trivially pass even if their 
> CHECK... lines are wrong.  An easy repro is just to randomly modify any CHECK 
> line in a lit file (e.g. 
> llvm/tools/clang/tools/extra/test/clang-tidy/readability-avoid-const-params-in-decls.cpp)
>  and run ninja check-clang-tools.
>
> In check_clang_tidy.py, if you add back (slightly modified) lines 93-95 to 
> the else branch (line 132), it seems to fix the problem.  For example, add:
>
>   has_check_fixes = check_fixes_prefixes[0] in input_text
>   has_check_messages = check_messages_prefixes[0] in input_text
>   has_check_notes = check_notes_prefixes[0] in input_text


Oh, thanks. I'll fix it a bit later today. Sorry.


Repository:
  rL LLVM

https://reviews.llvm.org/D52971



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

[PATCH] D53186: [clangd] Support hover on "aut^o *".

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53186



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

[PATCH] D53188: [clangd] XRef C++ API returns structure results, NFC

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.

The representation of references in LSP is limitted (just location
information).  This patch makes our xref C++ API return structured results,
which allows other clients get more data for references.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53188

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/XRefs.cpp
  clangd/XRefs.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -1184,7 +1184,7 @@
 std::vector> ExpectedLocations;
 for (const auto &R : T.ranges())
   ExpectedLocations.push_back(RangeIs(R));
-EXPECT_THAT(findReferences(AST, T.point()),
+EXPECT_THAT(findReferences(AST, T.point()).render(""),
 ElementsAreArray(ExpectedLocations))
 << Test;
   }
@@ -1199,7 +1199,7 @@
   auto AST = TU.build();
 
   // References in main file are returned without index.
-  EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr),
+  EXPECT_THAT(findReferences(AST, Main.point(), /*Index=*/nullptr).render(""),
   ElementsAre(RangeIs(Main.range(;
   Annotations IndexedMain(R"cpp(
 int main() { [[f^oo]](); }
@@ -1210,13 +1210,14 @@
   IndexedTU.Code = IndexedMain.code();
   IndexedTU.Filename = "Indexed.cpp";
   IndexedTU.HeaderCode = Header;
-  EXPECT_THAT(findReferences(AST, Main.point(), IndexedTU.index().get()),
-  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
+  EXPECT_THAT(
+  findReferences(AST, Main.point(), IndexedTU.index().get()).render(""),
+  ElementsAre(RangeIs(Main.range()), RangeIs(IndexedMain.range(;
 
   // If the main file is in the index, we don't return duplicates.
   // (even if the references are in a different location)
   TU.Code = ("\n\n" + Main.code()).str();
-  EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()),
+  EXPECT_THAT(findReferences(AST, Main.point(), TU.index().get()).render(""),
   ElementsAre(RangeIs(Main.range(;
 }
 
Index: clangd/XRefs.h
===
--- clangd/XRefs.h
+++ clangd/XRefs.h
@@ -34,9 +34,20 @@
 /// Get the hover information when hovering at \p Pos.
 llvm::Optional getHover(ParsedAST &AST, Position Pos);
 
-/// Returns reference locations of the symbol at a specified \p Pos.
-std::vector findReferences(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index = nullptr);
+/// Represents a list of references for clangd C++ API.
+/// We don't use the LSP structures here as LSP ref representation is limitted,
+/// and we want other C++ API users (unlike ClangdLSPServer) to get more
+/// information about references.
+struct RefResult {
+  std::vector Refs;
+  // Storage for the underlying data.
+  llvm::StringSet<> Storage;
+  // Serialize to a list of LSP locations.
+  std::vector render(llvm::StringRef HintPath) const;
+};
+/// Returns symbol references at a specified \p Pos.
+RefResult findReferences(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index = nullptr);
 
 } // namespace clangd
 } // namespace clang
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -705,9 +705,18 @@
   return None;
 }
 
-std::vector findReferences(ParsedAST &AST, Position Pos,
- const SymbolIndex *Index) {
+std::vector RefResult::render(llvm::StringRef HintPath) const {
   std::vector Results;
+  for (auto &R : Refs) {
+if (auto LSPLoc = toLSPLocation(R.Location, HintPath))
+  Results.push_back(std::move(*LSPLoc));
+  }
+  return Results;
+}
+
+RefResult findReferences(ParsedAST &AST, Position Pos,
+ const SymbolIndex *Index) {
+  RefResult Results;
   const SourceManager &SM = AST.getASTContext().getSourceManager();
   auto MainFilePath = getRealPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
   if (!MainFilePath) {
@@ -723,14 +732,26 @@
   TargetDecls.push_back(DI.D);
   }
 
+  auto Intern = [&Results](llvm::StringRef S) {
+auto It = Results.Storage.find(S);
+if (It == Results.Storage.end())
+  It = Results.Storage.insert(S).first;
+return It->first();
+  };
+
   // We traverse the AST to find references in the main file.
   // TODO: should we handle macros, too?
   auto MainFileRefs = findRefs(TargetDecls, AST);
   for (const auto &Ref : MainFileRefs) {
-Location Result;
-Result.range = getTokenRange(AST, Ref.Loc);
-Result.uri = URIForFile(*MainFilePath);
-Results.push_back(std::move(Result));
+clang::clangd::Ref R;
+auto Range = getTokenRange(AST,

[clang-tools-extra] r344330 - [clangd] Support hover on "aut^o *".

[Haojian Wu via cfe-commits]

Author: hokein
Date: Fri Oct 12 03:11:02 2018
New Revision: 344330

URL: http://llvm.org/viewvc/llvm-project?rev=344330&view=rev
Log:
[clangd] Support hover on "aut^o *".

Reviewers: kadircet

Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp

Modified: clang-tools-extra/trunk/clangd/XRefs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/XRefs.cpp?rev=344330&r1=344329&r2=344330&view=diff
==
--- clang-tools-extra/trunk/clangd/XRefs.cpp (original)
+++ clang-tools-extra/trunk/clangd/XRefs.cpp Fri Oct 12 03:11:02 2018
@@ -579,20 +579,28 @@ public:
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
+  // Remove the surrounding Reference or Pointer type of the given type T.
+  QualType UnwrapReferenceOrPointer(QualType T) {
+// "auto &" is represented as a ReferenceType containing an AutoType
+if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
+  return RT->getPointeeType();
+// "auto *" is represented as a PointerType containing an AutoType
+if (const PointerType *PT = dyn_cast(T.getTypePtr()))
+  return PT->getPointeeType();
+return T;
+  }
+
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
   //- auto& i = 1;
+  //- auto* i = &a;
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
 if (!D->getTypeSourceInfo() ||
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = D->getType();
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(DeclT.getTypePtr()))
-  DeclT = RT->getPointeeType();
-
+auto DeclT = UnwrapReferenceOrPointer(D->getType());
 const AutoType *AT = dyn_cast(DeclT.getTypePtr());
 if (AT && !AT->getDeducedType().isNull()) {
   // For auto, use the underlying type because the const& would be
@@ -626,11 +634,7 @@ public:
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = D->getReturnType();
-// "auto &" is represented as a ReferenceType containing an AutoType.
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  T = RT->getPointeeType();
-
+auto T = UnwrapReferenceOrPointer(D->getReturnType());
 const AutoType *AT = dyn_cast(T.getTypePtr());
 if (AT && !AT->getDeducedType().isNull()) {
   DeducedType = T.getUnqualifiedType();

Modified: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp?rev=344330&r1=344329&r2=344330&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp Fri Oct 12 03:11:02 
2018
@@ -756,6 +756,15 @@ TEST(Hover, All) {
   "int",
   },
   {
+  R"cpp(// Simple initialization with auto*
+void foo() {
+  int a = 1;
+  ^auto* i = &a;
+}
+  )cpp",
+  "int",
+  },
+  {
   R"cpp(// Auto with initializer list.
 namespace std
 {
@@ -861,6 +870,16 @@ TEST(Hover, All) {
 }
   )cpp",
   "struct Bar",
+  },
+  {
+  R"cpp(// auto* in function return
+struct Bar {};
+^auto* test() {
+  Bar* bar;
+  return bar;
+}
+  )cpp",
+  "struct Bar",
   },
   {
   R"cpp(// const auto& in function return


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

[PATCH] D53186: [clangd] Support hover on "aut^o *".

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344330: [clangd] Support hover on "aut^o *". 
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53186?vs=169357&id=169363#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53186

Files:
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp


Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -756,6 +756,15 @@
   "int",
   },
   {
+  R"cpp(// Simple initialization with auto*
+void foo() {
+  int a = 1;
+  ^auto* i = &a;
+}
+  )cpp",
+  "int",
+  },
+  {
   R"cpp(// Auto with initializer list.
 namespace std
 {
@@ -863,6 +872,16 @@
   "struct Bar",
   },
   {
+  R"cpp(// auto* in function return
+struct Bar {};
+^auto* test() {
+  Bar* bar;
+  return bar;
+}
+  )cpp",
+  "struct Bar",
+  },
+  {
   R"cpp(// const auto& in function return
 struct Bar {};
 const ^auto& test() {
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -579,20 +579,28 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
+  // Remove the surrounding Reference or Pointer type of the given type T.
+  QualType UnwrapReferenceOrPointer(QualType T) {
+// "auto &" is represented as a ReferenceType containing an AutoType
+if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
+  return RT->getPointeeType();
+// "auto *" is represented as a PointerType containing an AutoType
+if (const PointerType *PT = dyn_cast(T.getTypePtr()))
+  return PT->getPointeeType();
+return T;
+  }
+
   // Handle auto initializers:
   //- auto i = 1;
   //- decltype(auto) i = 1;
   //- auto& i = 1;
+  //- auto* i = &a;
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
 if (!D->getTypeSourceInfo() ||
 D->getTypeSourceInfo()->getTypeLoc().getBeginLoc() != SearchedLocation)
   return true;
 
-auto DeclT = D->getType();
-// "auto &" is represented as a ReferenceType containing an AutoType
-if (const ReferenceType *RT = dyn_cast(DeclT.getTypePtr()))
-  DeclT = RT->getPointeeType();
-
+auto DeclT = UnwrapReferenceOrPointer(D->getType());
 const AutoType *AT = dyn_cast(DeclT.getTypePtr());
 if (AT && !AT->getDeducedType().isNull()) {
   // For auto, use the underlying type because the const& would be
@@ -626,11 +634,7 @@
 if (CurLoc != SearchedLocation)
   return true;
 
-auto T = D->getReturnType();
-// "auto &" is represented as a ReferenceType containing an AutoType.
-if (const ReferenceType *RT = dyn_cast(T.getTypePtr()))
-  T = RT->getPointeeType();
-
+auto T = UnwrapReferenceOrPointer(D->getReturnType());
 const AutoType *AT = dyn_cast(T.getTypePtr());
 if (AT && !AT->getDeducedType().isNull()) {
   DeducedType = T.getUnqualifiedType();


Index: clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/XRefsTests.cpp
@@ -756,6 +756,15 @@
   "int",
   },
   {
+  R"cpp(// Simple initialization with auto*
+void foo() {
+  int a = 1;
+  ^auto* i = &a;
+}
+  )cpp",
+  "int",
+  },
+  {
   R"cpp(// Auto with initializer list.
 namespace std
 {
@@ -863,6 +872,16 @@
   "struct Bar",
   },
   {
+  R"cpp(// auto* in function return
+struct Bar {};
+^auto* test() {
+  Bar* bar;
+  return bar;
+}
+  )cpp",
+  "struct Bar",
+  },
+  {
   R"cpp(// const auto& in function return
 struct Bar {};
 const ^auto& test() {
Index: clang-tools-extra/trunk/clangd/XRefs.cpp
===
--- clang-tools-extra/trunk/clangd/XRefs.cpp
+++ clang-tools-extra/trunk/clangd/XRefs.cpp
@@ -579,20 +579,28 @@
 
   llvm::Optional getDeducedType() { return DeducedType; }
 
+  // Remove the surrounding Reference or Pointer type of the given type T.
+  QualType UnwrapReferenceOrPointer(QualType T) {
+// "auto &" is represented as a ReferenceType containin

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Whisperity via Phabricator via cfe-commits]

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

Have been looked at this far and wide too many times now. I say we roll this 
out and fix and improve later on, lest this only going into Clang 72. Results 
look promising, let's hope users start reporting good findings too.

Considering Tidy has no `alpha` group the name `bugprone` seems like a 
double-edged sword, as what is bugprone, the checker or what it finds? 😜


https://reviews.llvm.org/D45050



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:11976
+
+while (FD && !getDLLAttr(FD) &&
+   !FD->hasAttr() &&

hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > takuto.ikuta wrote:
> > > > hans wrote:
> > > > > Why does this need to be a loop? I don't think FunctionDecl's can be 
> > > > > nested?
> > > > This is for static local var in lambda function.
> > > > static_x's ParentFunction does not become f().
> > > > ```
> > > > class __declspec(dllexport) C {
> > > >   int f() {
> > > > return ([]() { static int static_x; return ++static_x; })();
> > > >   }
> > > > };
> > > > ```
> > > I don't think the lambda (or its static local) should be exported in this 
> > > case.
> > If we don't export static_x, dllimport library cannot find static_x when 
> > linking?
> > 
> > 
> I believe even if C is marked dllimport, the lambda will not be dllimport. 
> The inline definition will be used.
Do you say importing/implementation library should have different instance of 
static_x here?
Current clang does not generate such obj.

I think static_x should be dllimported. But without this loop, static_x cannot 
find FD of C::f() having DLLImport/ExportStaticLocalAttr and static_x won't be 
treated as imported/exported variable.

And if static_x is not exported, importing library and implementation library 
will not have the same instance of static_x when C::f() is inlined.



Comment at: clang/lib/Sema/SemaDecl.cpp:11995
+
+// Function having static local variables should be exported.
+auto *ExportAttr = 
cast(NewAttr->clone(getASTContext()));

takuto.ikuta wrote:
> hans wrote:
> > takuto.ikuta wrote:
> > > hans wrote:
> > > > Isn't it enough that the static local is exported, does the function 
> > > > itself need to be exported too?
> > > Adding dllexport only to variable does not export variable when the 
> > > function is not used.
> > > But dllimport is not necessary for function, removed.
> > Hmm, I wonder if we could fix that instead? That is, export the variable in 
> > that case even if the function is not used?
> I see. I'll investigate which code path emits static variables
static local variable seems to be exported in
https://github.com/llvm-project/llvm-project-20170507/blob/f54514c50350743d3d1a3c5aa80cbe4bb449eb71/clang/lib/CodeGen/CGDecl.cpp#L378

But emitting static local var only by bypassing function emission seems 
difficult.
Let me leave as-is here.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:5719
+  TSK != TSK_ExplicitInstantiationDeclaration &&
+  TSK != TSK_ExplicitInstantiationDefinition) {
+if (ClassExported) {

hans wrote:
> takuto.ikuta wrote:
> > hans wrote:
> > > I still don't understand why we need these checks for template 
> > > instantiations. Why does it matter whether the functions get inlined or 
> > > not?
> > When function of dllimported class is not inlined, such funtion needs to be 
> > dllexported from implementation library.
> > 
> > c.h
> > ```
> > template
> > class EXPORT C {
> >  public:
> >   void f() {}
> > };
> > ```
> > 
> > cuser.cc
> > ```
> > #include "c.h"
> > 
> > void cuser() {
> >   C c;
> >   c.f();  // This function may not be inlined when EXPORT is 
> > __declspec(dllimport), so link may fail.
> > }
> > ```
> > 
> > When cuser.cc and c.h are built to different library, cuser.cc needs to be 
> > able to see dllexported C::f() when C::f() is not inlined.
> > This is my understanding.
> Your example doesn't use explicit instantiation definition or declaration, so 
> doesn't apply here I think.
> 
> As long as the dllexport and dllimport attributes matches it's fine. It 
> doesn't matter whether the function gets inlined or not, the only thing that 
> matters is that if it's marked dllimport on the "consumer side", it must be 
> dllexport when building the dll.
Hmm, I changed the linkage for functions having DLLExport/ImportStaticLocal 
Attr.



https://reviews.llvm.org/D51340



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

[PATCH] D51340: Add /Zc:DllexportInlines option to clang-cl

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta updated this revision to Diff 169365.
takuto.ikuta added a comment.

Address comment


https://reviews.llvm.org/D51340

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/CLCompatOptions.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp

Index: clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/dllexport-no-dllexport-inlines.cpp
@@ -0,0 +1,164 @@
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -fno-dllexport-inlines -emit-llvm -O0 -o - |\
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=NOINLINE %s
+
+// RUN: %clang_cc1 %s -fms-extensions -triple x86_64-windows-msvc   \
+// RUN: -emit-llvm -O0 -o - |   \
+// RUN: FileCheck --check-prefix=DEFAULT --check-prefix=INLINE %s
+
+// Function
+
+// DEFAULT-DAG: define dso_local dllexport void @"?NormalFunction@@YAXXZ"()
+void __declspec(dllexport) NormalFunction() {}
+
+
+// DEFAULT-DAG: define weak_odr dso_local dllexport void @"?AlwaysInlineFunction@@YAXXZ"
+__forceinline void __declspec(dllexport) AlwaysInlineFunction() {}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableExported@@YAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+__forceinline int __declspec(dllexport) AlwaysInlineWithStaticVariableExported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+// DEFAULT-DAG: @"?static_variable@?1??AlwaysInlineWithStaticVariableImported@@YAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+__forceinline int __declspec(dllimport) AlwaysInlineWithStaticVariableImported() {
+  static int static_variable = 0;
+  ++static_variable;
+  return static_variable;
+}
+
+int ImportedFunctionUser() {
+  return AlwaysInlineWithStaticVariableImported();
+}
+
+// Class member function
+
+// check for local static variables
+// NOINLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// INLINE-DAG: @"?static_variable@?1??InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ@4HA" = weak_odr dso_local dllexport global i32 0, comdat, align 4
+
+// NOINLINE-DAG: @"?static_variable@?1??InClassDefFuncWithStaticVariable@ImportedClass@@QEAAHXZ@4HA" = available_externally dllimport global i32 0, align 4
+
+class __declspec(dllexport) NoTemplateExportedClass {
+ public:
+  // DEFAULT-NOT: NoTemplateExportedClass@NoTemplateExportedClass@@
+  NoTemplateExportedClass() = default;
+
+  // NOINLINE-NOT: InclassDefFunc@NoTemplateExportedClass
+  // INLINE-DAG: define weak_odr dso_local dllexport void @"?InclassDefFunc@NoTemplateExportedClass@@
+  void InclassDefFunc() {}
+
+  int f();
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFuncWithStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFuncWithStaticVariable() {
+static int static_variable = 0;
+++static_variable;
+return static_variable;
+  }
+
+  // DEFAULT-DAG: define weak_odr dso_local dllexport i32 @"?InclassDefFunctWithLambdaStaticVariable@NoTemplateExportedClass@@QEAAHXZ"
+  int InclassDefFunctWithLambdaStaticVariable() {
+return ([]() { static int static_x; return ++static_x; })();
+  }
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWihtoutDefinition
+  __forceinline void InlineOutclassDefFuncWihtoutDefinition();
+
+  // DEFAULT-NOT: InlineOutclassDefFunc@NoTemplateExportedClass@@
+  __forceinline void InlineOutclassDefFunc();
+
+  // DEFAULT-NOT: InlineOutclassDefFuncWithStaticVariable@NoTemplateExportedClass@@
+  __forceinline int InlineOutclassDefFuncWithStaticVariable();
+
+  // DEFAULT-DAG: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@QEAAXXZ"
+  void OutclassDefFunc();
+};
+
+void NoTemplateExportedClass::OutclassDefFunc() {}
+
+__forceinline void NoTemplateExportedClass::InlineOutclassDefFunc() {}
+
+__forceinline int NoTemplateExportedClass::InlineOutclassDefFuncWithStaticVariable() {
+  static int static_variable = 0;
+  return ++static_variable;
+}
+
+void __declspec(dllexport) NoTemplateExportedClassUser() {
+  NoTemplateExportedClass a;
+  a.InlineOutclassDefFunc();
+}
+
+template
+class __declspec(dllexport) TemplateExportedClass {
+  void InclassDefFunc() {}
+  void OutclassDefFunc();
+
+  T templateValue;
+};
+
+// DEFAULT-NOT: define dso_local dllexport void @"?OutclassDefFunc@NoTemplateExportedClass@@
+template void TemplateExportedClass::OutclassDefFunc() {}
+
+class A11{};
+class 

r344333 - Fix Wdocumentation warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Fri Oct 12 03:34:03 2018
New Revision: 344333

URL: http://llvm.org/viewvc/llvm-project?rev=344333&view=rev
Log:
Fix Wdocumentation warning. NFCI.

Modified:
cfe/trunk/include/clang/AST/DeclTemplate.h

Modified: cfe/trunk/include/clang/AST/DeclTemplate.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=344333&r1=344332&r2=344333&view=diff
==
--- cfe/trunk/include/clang/AST/DeclTemplate.h (original)
+++ cfe/trunk/include/clang/AST/DeclTemplate.h Fri Oct 12 03:34:03 2018
@@ -1093,7 +1093,7 @@ public:
   /// template.
   ArrayRef getInjectedTemplateArgs();
 
-  /// Merge our RedeclarableTemplateDecl::Common with \param Prev.
+  /// Merge \param Prev with our RedeclarableTemplateDecl::Common.
   void mergePrevDecl(FunctionTemplateDecl *Prev);
 
   /// Create a function template node.


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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.
Herald added subscribers: donat.nagy, Szelethus, mikhail.ramalho, rnkovacs, 
szepet.
Herald added a reviewer: george.karpenkov.

//Soft ping.//


https://reviews.llvm.org/D33672



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

[PATCH] D53135: Remove top-level using declaration from header files, as these aliases leak.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: include/clang/Driver/Job.h:33
 // Re-export this as clang::driver::ArgStringList.
-using llvm::opt::ArgStringList;
+using ArgStringList = llvm::opt::ArgStringList;
 

ilya-biryukov wrote:
> Both approaches seem equivalent in that case, since we do want the re-export.
> So not sure we want to change this in the first place. But feel free to keep 
> it too, don't have a strong opinion on this one.
The intent here is to make this look less like the typical "bring this name 
into the namespace" ane more like "give this a new name", even if the 
unqualified name is the same.

In particular: this no longer matches `rg -t h "using llvm::"` which is a good 
way to find such bad decls :-) It's not a big deal, but I think this is a 
marginal improment.


Repository:
  rC Clang

https://reviews.llvm.org/D53135



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

[PATCH] D53188: [clangd] XRef C++ API returns structure results, NFC

[Haojian Wu via Phabricator via cfe-commits]

hokein planned changes to this revision.
hokein added a comment.

As discussed offline, we don't have a clear motivation to do it now, and LSP 
will likely introduce a new `References` type of xrefs 
(https://github.com/Microsoft/language-server-protocol/issues/396), so I'll 
leave this patch opened.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53188



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

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov updated this revision to Diff 169368.
AlexeySachkov marked an inline comment as done.
AlexeySachkov added a comment.

Updated tests. Reverted usage of `#pragma OPENCL EXTENSION my_ext : begin` due 
to a failed LIT tests: `test/Headers/opencl-c-header.cl`

Test failed on the following RUN-line:

  // Compile for OpenCL 2.0 for the first time. The module should change.   
  
  // RUN: %clang_cc1 -triple spir-unknown-unknown -O0 -emit-llvm -o - 
-cl-std=CL2.0 -finclude-default-header -fmodules -fimplicit-modu le-maps 
-fmodules-cache-path=%t -fdisable-module-hash -ftime-report %s 2>&1 | FileCheck 
--check-prefix=CHECK20 --check-prefix=CHECK- MOD %s 

 

Clang crashes at the assertion in `ASTReader::getGlobalSubmoduleID()`:

  assert(I != M.SubmoduleRemap.end() && "Invalid index into submodule index 
remap");


https://reviews.llvm.org/D51484

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/OperationKinds.def
  include/clang/AST/Type.h
  include/clang/Basic/OpenCLExtensionTypes.def
  include/clang/Basic/OpenCLExtensions.def
  include/clang/Sema/Initialization.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/module.modulemap
  lib/AST/ASTContext.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Edit/RewriteObjCFoundationAPI.cpp
  lib/Headers/opencl-c.h
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  test/Headers/opencl-c-header.cl
  test/Index/opencl-types.cl
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
  tools/libclang/CIndex.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -70,6 +70,8 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) BTCASE(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) BTCASE(Id);
+#include "clang/Basic/OpenCLExtensionTypes.def"
 BTCASE(OCLSampler);
 BTCASE(OCLEvent);
 BTCASE(OCLQueue);
@@ -605,6 +607,8 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
+#define EXT_OPAQUE_TYPE(ExtTYpe, Id, Ext) TKIND(Id);
+#include "clang/Basic/OpenCLExtensionTypes.def"
 TKIND(OCLSampler);
 TKIND(OCLEvent);
 TKIND(OCLQueue);
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1519,6 +1519,9 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   case BuiltinType::Id:
 #include "clang/Basic/OpenCLImageTypes.def"
+#define EXT_OPAQUE_TYPE(ExtTYpe, Id, Ext) \
+  case BuiltinType::Id:
+#include "clang/Basic/OpenCLExtensionTypes.def"
   case BuiltinType::OCLSampler:
   case BuiltinType::OCLEvent:
   case BuiltinType::OCLClkEvent:
Index: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
===
--- /dev/null
+++ test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
@@ -0,0 +1,111 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify %s
+
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+
+// Having the initializers defined in the test allows us not to include default
+// opencl header, and thus reducing test execution time.
+// The initializers' values must match with ones defined in the default opencl
+// header (clang/lib/Headers/opencl-c.h)
+
+#define CLK_AVC_ME_INITIALIZE_INTEL 0x0
+
+#define CLK_AVC_IME_PAYLOAD_INITIALIZE_INTEL 0x0
+#define CLK_AVC_REF_PAYLOAD_INITIALIZE_INTEL 0x0
+#define CLK_AVC_SIC_PAYLOAD_INITIALIZE_INTEL 0x0
+
+#define CLK_AVC_IME_RESULT_INITIALIZE_INTEL 0x0
+#define CLK_AVC_REF_RESULT_INITIALIZE_INTEL 0x0
+#define CLK_AVC_SIC_RESULT_INITIALIZE_INTEL 0x0
+
+#define CLK_AVC_IME_RESULT_SINGLE_REFERENCE_STREAMOUT_INITIALIZE_INTEL 0x0
+#define CLK_AVC_IME_RESULT_SINGLE_REFERENCE_STREAMIN

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov added inline comments.



Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:26
+ char4 c4, event_t e, struct st ss) {
+  intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer 
for mce types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with 
an expression of incompatible type 'int'}}

Anastasia wrote:
> AlexeySachkov wrote:
> > Anastasia wrote:
> > > Would it make sense to add a check for non-zero constant?
> > > 
> > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type 
> > > from the same type? Any other restrictions on assignment (i.e. w integer 
> > > literals) and operations over these types?
> > > Also can you assign variables of intel_sub_group_avc_mce_payload_t type 
> > > from the same type?
> > 
> > Yes, such assignment is allowed.
> > 
> > > Any other restrictions on assignment (i.e. w integer literals)
> > 
> > All of these types can only be initialized using call to a special 
> > built-ins or using predefined macros like 
> > `CLK_AVC_REF_RESULT_INITIALIZE_INTEL`. Any other assignment should lead to 
> > an error. 
> > 
> > I found that I'm able to assign variable of type 
> > `intel_sub_group_avc_imc_payload_t` to variable of type 
> > `intel_sub_group_avc_mce_payload_t`, so I will update the patch when I 
> > implement such diagnostic message.
> > 
> > > and operations over these types?
> > Variables of these types can only be used as return values or arguments for 
> > built-in functions described in the specification. All other operations are 
> > restricted
> W/o the spec change it's really difficult to review properly. So are you 
> testing 2 groups of types:
> 1. Init by zero in `bar`?
> 2. Init by builtin function in `foo`?
> 
I would like to test:
1. foo: init by literal or variable, negative tests
2. far: init by other type or assignment between different types, negative tests
3. bar: init by special initializers from spec, positive tests



Comment at: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl:34
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with 
an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ime_payload_t payload_ime = b;
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_ime_payload_t' with 
an expression of incompatible type 'bool'}}

Anastasia wrote:
> I am not sure it makes sense to iterate through all different types. You 
> don't enumerate all of them and we don't do exhaustive testing in Clang tests 
> anyway. I would just check integer literal and one other builtin type.
Simplified test a little bit.


https://reviews.llvm.org/D51484



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

r344335 - [Tooling] Expose ExecutorName option.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Fri Oct 12 04:47:36 2018
New Revision: 344335

URL: http://llvm.org/viewvc/llvm-project?rev=344335&view=rev
Log:
[Tooling] Expose ExecutorName option.

Modified:
cfe/trunk/include/clang/Tooling/Execution.h
cfe/trunk/lib/Tooling/Execution.cpp

Modified: cfe/trunk/include/clang/Tooling/Execution.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Execution.h?rev=344335&r1=344334&r2=344335&view=diff
==
--- cfe/trunk/include/clang/Tooling/Execution.h (original)
+++ cfe/trunk/include/clang/Tooling/Execution.h Fri Oct 12 04:47:36 2018
@@ -37,6 +37,8 @@
 namespace clang {
 namespace tooling {
 
+extern llvm::cl::opt ExecutorName;
+
 /// An abstraction for the result of a tool execution. For example, the
 /// underlying result can be in-memory or on-disk.
 ///

Modified: cfe/trunk/lib/Tooling/Execution.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Execution.cpp?rev=344335&r1=344334&r2=344335&view=diff
==
--- cfe/trunk/lib/Tooling/Execution.cpp (original)
+++ cfe/trunk/lib/Tooling/Execution.cpp Fri Oct 12 04:47:36 2018
@@ -16,7 +16,7 @@ LLVM_INSTANTIATE_REGISTRY(clang::tooling
 namespace clang {
 namespace tooling {
 
-static llvm::cl::opt
+llvm::cl::opt
 ExecutorName("executor", llvm::cl::desc("The name of the executor to 
use."),
  llvm::cl::init("standalone"));
 


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

[PATCH] D53131: [clangd] Support scope proximity in code completion.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(BTW I wouldn't expect to see improvements on eval here, as




Comment at: clangd/CodeComplete.cpp:558
+if (const auto *NS = dyn_cast(Ctx))
+  return NS->getQualifiedNameAsString() + "::";
+  return llvm::None;

ioeric wrote:
> sammccall wrote:
> > does this do the right thing if it's anonymous or inline, or a parent is?
> > 
> > For indexing, we call a function in AST.h, we should probably do something 
> > similar.
> > 
> > The catch is that function relies on NamedDecl::printQualifiedName, maybe 
> > we need to factor out the relevant part so we can call it on a DeclContext.
> Good catch.
> 
> `NamedDecl::printQualifiedName` doesn't skip the `(anonymous)` part if the 
> decl itself is anonymous, even with `SuppressUnwrittenScope` set. So I think 
> we should explicitly filter those out here and then call `printQualifiedName`.
can you move this function to AST.h? e.g. `printScope`? It seems like an 
obvious peer to `printQualifiedName`.



Comment at: clangd/Quality.cpp:246
 
+float QueryScopeProximity::proximity(llvm::StringRef Scope) const {
+  if (QueryScopes.empty())

ioeric wrote:
> sammccall wrote:
> > If you're going to decide these numbers directly...
> > 
> > a) I think you should just return a multiplier here, rather than a 0-1 score
> > b) we should more aggressively downrank non-preferred symbols: currently by 
> > only ~1/3 vs symbols from a preferred scopes
> > 
> > e.g. I'd suggest returning 1, 2, 1.5, 1, 1, 1.5, 0.3 or similar
> > a) I think you should just return a multiplier here, rather than a 0-1 
> > score.
> I tried this. Unfortunately, allowing <1 multiplier would make the default 
> value (for sema proximity) tricky. And [0,1] proximity score seems to be less 
> confusing as it's consistent with the file proximity scale. If [0,1] isn't 
> enough, we could still increase the upper bound?
> 
> > b) we should more aggressively downrank non-preferred symbols: currently by 
> > only ~1/3 vs symbols from a preferred scopes
> It's ~1/3 only for the global scope vs non-preferred symbols, which seems 
> reasonable to me. I worry penalizing non-preferred scopes too much can make 
> cross-namespace completion less useable.
> Unfortunately, allowing <1 multiplier would make the default value (for sema 
> proximity) tricky. 

There are three types of symbols:
 - those we get from sema only. Here your patch decides to give max boost (I 
think?) because we don't trust scope comparison for all these cases.
 - those we get from index only. We choose a boost by matching the stated scope 
to the query scopes.
 - those we get from sema and index. We should *probably* treat these like 
Sema-only and give max boost.

Note the category of results which we have neither sema nor index for doesn't 
actually exist. The framework allows for it and we should respect that as best 
we can, but it can't be the top priority.

So can't we just change the SemaScopeProximityScore to `bool SemaSaysInScope` 
or something and evaluate as `SemaSaysInScope ? MaxBoost : QueryScopeProximity 
? evaluateUsingQueryScopeProximity() : 1`?

> It's ~1/3 only for the global scope vs non-preferred symbols
Queried scopes (not most-preferred, and not the global scope!) get a boost of 
1.6, while completely irrelevant scopes get a boost of 1. This is a relative 
penalty of only 37.5%.
(I think a more reasonable penalty would be in the range of 75% or 80%, i.e. a 
3-4x boost for symbols that are in-scope)



Comment at: clangd/Quality.h:92
+  private:
+std::vector QueryScopes;
+};

ioeric wrote:
> sammccall wrote:
> > hmm, can/should we use FileProximity for this, and just transform the 
> > strings?
> > 
> > This isn't going to cache for symbols sharing a namespace, isn't going to 
> > handle "symbol is in a child namespace of an included namespace", and some 
> > other combinations.
> It seems to me that it'd be less trivial to associate scopes with up/down 
> traverses. Currently, the query scopes contain all enclosing namespaces, so 
> the distance seems less important here. In general, I think the 
> characteristics of scope proximity (e.g. all enclosing namespaces are already 
> in query scopes) allow us to get away with something more trivial than file 
> proximity? 
> 
> > This isn't going to cache for symbols sharing a namespace.
> This seems to be less a concern if we are not calculating up/down distance.
> > isn't going to handle "symbol is in a child namespace of an included 
> > namespace"
> This can be covered by `SymbolScope.startswith(QueryScope)`? It might not 
> work well for `using-namespace`s, but it's unclear how important that is.
> 
> Still happy to consider FileDistance-approach as I might be missing something.
> Currently, the query scopes contain all enclosing namespaces, so the distance 
> seems less important here.

Sure. But there cases like `using namespace std` and then 

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

[Eric Liu via Phabricator via cfe-commits]

ioeric requested changes to this revision.
ioeric added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:203
+llvm::Expected>
+createClangDocExecutor(int &argc, const char **argv,
+   llvm::cl::OptionCategory &Category) {

This is exposing too much implementation details.I landed a patch to expose the 
`ExecutorName` option from the library (rL344335) so you could simply do 
`ExecutorName.setInitialValue("all-TUs")` before calling 
`createExecutorFromCommandLineArgs()`.


https://reviews.llvm.org/D53170



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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Also needs an entry to `www/analyzer/alpha_checks.html`.




Comment at: test/Analysis/enum-cast-out-of-range.cpp:1
+// RUN: %clang_cc1 -std=c++11 -analyze 
-analyzer-checker=alpha.cplusplus.EnumCastOutOfRange -verify %s
+

1. Prefer `%clang_analyze_cc1`
2. Always run core checkers too. 
`-analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange`
3. Organize the run line to fit into 80 cols (if possible)
```
// RUN: %clang_analyze_cc1 \
// RUN:   -analyzer-checker=core,alpha.cplusplus.EnumCastOutOfRange \
// RUN:   -std=c++11 -verify %s


https://reviews.llvm.org/D33672



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

[PATCH] D53191: [clang] Use Statement and Namespace instead of Name and PotentiallyQualifiedName

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a subscriber: cfe-commits.

New name suggestion contexts were being used in cases they were not
suitable. This patch changes those contexts into more suitable ones.


Repository:
  rC Clang

https://reviews.llvm.org/D53191

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/ordinary-name.c


Index: test/CodeCompletion/ordinary-name.c
===
--- test/CodeCompletion/ordinary-name.c
+++ test/CodeCompletion/ordinary-name.c
@@ -3,8 +3,9 @@
 typedef struct t TYPEDEF;
 typedef struct t _TYPEDEF;
 void foo() {
+  int yar;
   int y;
-  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:6:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:6:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1-NOT: __INTEGER_TYPE
   // CHECK-CC1: _Imaginary
   // CHECK-CC1: _MyPrivateType
@@ -16,3 +17,6 @@
 
   // PR8744
   // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:1:11 %s
+
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:7:8 %s -o - | FileCheck -allow-empty %s
+  // CHECK-NOT: yar
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4822,7 +4822,7 @@
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CodeCompletionContext CC(CodeCompletionContext::CCC_Statement);
 CC.setCXXScopeSpecifier(SS);
 HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
 return;
@@ -4840,7 +4840,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Name);
+CodeCompletionContext::CCC_Statement);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
@@ -4880,7 +4880,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
+CodeCompletionContext::CCC_Namespace,
 &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 


Index: test/CodeCompletion/ordinary-name.c
===
--- test/CodeCompletion/ordinary-name.c
+++ test/CodeCompletion/ordinary-name.c
@@ -3,8 +3,9 @@
 typedef struct t TYPEDEF;
 typedef struct t _TYPEDEF;
 void foo() {
+  int yar;
   int y;
-  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:6:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:6:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1-NOT: __INTEGER_TYPE
   // CHECK-CC1: _Imaginary
   // CHECK-CC1: _MyPrivateType
@@ -16,3 +17,6 @@
 
   // PR8744
   // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:1:11 %s
+
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:7:8 %s -o - | FileCheck -allow-empty %s
+  // CHECK-NOT: yar
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4822,7 +4822,7 @@
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CodeCompletionContext CC(CodeCompletionContext::CCC_Statement);
 CC.setCXXScopeSpecifier(SS);
 HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
 return;
@@ -4840,7 +4840,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Name);
+CodeCompletionContext::CCC_Statement);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
@@ -4880,7 +4880,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
+CodeCompletionContext::CCC_Namespace,
 &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 
___
cfe-com

[PATCH] D53153: [OpenCL] Mark namespace scope variables and kernel functions with default visibility

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: lib/AST/Decl.cpp:565
+return FD->hasAttr();
+  return dyn_cast(D);
+}

isa


Repository:
  rC Clang

https://reviews.llvm.org/D53153



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

[PATCH] D33672: [analyzer] INT50-CPP. Do not cast to an out-of-range enumeration checker

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:296-299
+def EnumCastOutOfRangeChecker : Checker<"EnumCastOutOfRange">,
+  HelpText<"Check integer to enumeration casts for out of range values">,
+  DescFile<"EnumCastOutOfRange.cpp">;
+

Sort alphabetically.


https://reviews.llvm.org/D33672



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

[PATCH] D53192: [clangd] Do not query index for new name completions.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53192

Files:
  clangd/CodeComplete.cpp
  unittests/clangd/CodeCompleteTests.cpp


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2154,6 +2154,15 @@
AllOf(Qualifier("nx::"), 
Named("Clangd2";
 }
 
+TEST(CompletionTest, NoCompletionsForNewNames) {
+  clangd::CodeCompleteOptions Opts;
+  Opts.AllScopes = true;
+  auto Results = completions(R"cpp(
+  void f() { int n^ }
+)cpp",
+ {cls("naber"), cls("nx::foo")}, Opts);
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -620,8 +620,6 @@
   case CodeCompletionContext::CCC_ObjCProtocolName:
   case CodeCompletionContext::CCC_Namespace:
   case CodeCompletionContext::CCC_Type:
-  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
-  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
   case CodeCompletionContext::CCC_ParenthesizedExpression:
   case CodeCompletionContext::CCC_ObjCInterfaceName:
   case CodeCompletionContext::CCC_ObjCCategoryName:
@@ -642,6 +640,9 @@
   case CodeCompletionContext::CCC_ObjCClassMessage:
   case CodeCompletionContext::CCC_IncludedFile:
   case CodeCompletionContext::CCC_Recovery:
+  // TODO: Provide identifier based completions for the following two contexts:
+  case CodeCompletionContext::CCC_Name:
+  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
 return false;
   }
   llvm_unreachable("unknown code completion context");


Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2154,6 +2154,15 @@
AllOf(Qualifier("nx::"), Named("Clangd2";
 }
 
+TEST(CompletionTest, NoCompletionsForNewNames) {
+  clangd::CodeCompleteOptions Opts;
+  Opts.AllScopes = true;
+  auto Results = completions(R"cpp(
+  void f() { int n^ }
+)cpp",
+ {cls("naber"), cls("nx::foo")}, Opts);
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre());
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -620,8 +620,6 @@
   case CodeCompletionContext::CCC_ObjCProtocolName:
   case CodeCompletionContext::CCC_Namespace:
   case CodeCompletionContext::CCC_Type:
-  case CodeCompletionContext::CCC_Name: // FIXME: why does ns::^ give this?
-  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
   case CodeCompletionContext::CCC_ParenthesizedExpression:
   case CodeCompletionContext::CCC_ObjCInterfaceName:
   case CodeCompletionContext::CCC_ObjCCategoryName:
@@ -642,6 +640,9 @@
   case CodeCompletionContext::CCC_ObjCClassMessage:
   case CodeCompletionContext::CCC_IncludedFile:
   case CodeCompletionContext::CCC_Recovery:
+  // TODO: Provide identifier based completions for the following two contexts:
+  case CodeCompletionContext::CCC_Name:
+  case CodeCompletionContext::CCC_PotentiallyQualifiedName:
 return false;
   }
   llvm_unreachable("unknown code completion context");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53191: [clang] Use Statement and Namespace instead of Name and PotentiallyQualifiedName

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 169374.
kadircet added a comment.

- Use statement as context for using decls.


Repository:
  rC Clang

https://reviews.llvm.org/D53191

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/ordinary-name.c


Index: test/CodeCompletion/ordinary-name.c
===
--- test/CodeCompletion/ordinary-name.c
+++ test/CodeCompletion/ordinary-name.c
@@ -3,8 +3,9 @@
 typedef struct t TYPEDEF;
 typedef struct t _TYPEDEF;
 void foo() {
+  int yar;
   int y;
-  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:6:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:6:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1-NOT: __INTEGER_TYPE
   // CHECK-CC1: _Imaginary
   // CHECK-CC1: _MyPrivateType
@@ -16,3 +17,6 @@
 
   // PR8744
   // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:1:11 %s
+
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only 
-code-completion-at=%s:7:8 %s -o - | FileCheck -allow-empty %s
+  // CHECK-NOT: yar
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4822,7 +4822,7 @@
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CodeCompletionContext CC(CodeCompletionContext::CCC_Statement);
 CC.setCXXScopeSpecifier(SS);
 HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
 return;
@@ -4840,7 +4840,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Name);
+CodeCompletionContext::CCC_Statement);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
@@ -4880,7 +4880,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
+CodeCompletionContext::CCC_Statement,
 &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 


Index: test/CodeCompletion/ordinary-name.c
===
--- test/CodeCompletion/ordinary-name.c
+++ test/CodeCompletion/ordinary-name.c
@@ -3,8 +3,9 @@
 typedef struct t TYPEDEF;
 typedef struct t _TYPEDEF;
 void foo() {
+  int yar;
   int y;
-  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:6:9 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:6:11 %s -o - | FileCheck -check-prefix=CHECK-CC1 %s
   // CHECK-CC1-NOT: __INTEGER_TYPE
   // CHECK-CC1: _Imaginary
   // CHECK-CC1: _MyPrivateType
@@ -16,3 +17,6 @@
 
   // PR8744
   // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:1:11 %s
+
+  // RUN: %clang_cc1 -isystem %S/Inputs -fsyntax-only -code-completion-at=%s:7:8 %s -o - | FileCheck -allow-empty %s
+  // CHECK-NOT: yar
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4822,7 +4822,7 @@
   // it can be useful for global code completion which have information about
   // contexts/symbols that are not in the AST.
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CodeCompletionContext CC(CodeCompletionContext::CCC_Statement);
 CC.setCXXScopeSpecifier(SS);
 HandleCodeCompleteResults(this, CodeCompleter, CC, nullptr, 0);
 return;
@@ -4840,7 +4840,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_Name);
+CodeCompletionContext::CCC_Statement);
   Results.EnterNewScope();
 
   // The "template" keyword can follow "::" in the grammar, but only
@@ -4880,7 +4880,7 @@
 
   ResultBuilder Results(*this, CodeCompleter->getAllocator(),
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
+CodeCompletionContext::CCC_Statement,
 &ResultBuilder::IsNestedNameSpecifier);
   Results.EnterNewScope();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53191: [clang] Use Statement and Namespace instead of Name and PotentiallyQualifiedName

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: lib/Sema/SemaCodeComplete.cpp:4825
   if (SS.isInvalid()) {
-CodeCompletionContext CC(CodeCompletionContext::CCC_Name);
+CodeCompletionContext CC(CodeCompletionContext::CCC_Statement);
 CC.setCXXScopeSpecifier(SS);

I'm not sure statement is correct here.
First, `CCC_Expression` seems strictly better: this can't be `ns::while()` or 
such.

Secondly, ISTM there are other cases we might have a qualified ID in a 
non-expression context, like implementing a function `void foo::bar()` or so? 
Are we sure none of those hit this codepath?

No need to fix this now (this code is clearly an improvement) but maybe this is 
worth a comment like `// Usually qualified-IDs occur in expression context` or 
so.



Comment at: lib/Sema/SemaCodeComplete.cpp:4883
 CodeCompleter->getCodeCompletionTUInfo(),
-CodeCompletionContext::CCC_PotentiallyQualifiedName,
+CodeCompletionContext::CCC_Namespace,
 &ResultBuilder::IsNestedNameSpecifier);

Hmm, I don't think `CCC_Namespace` is right either. This is a using 
declaration, not a using directive.
So ultimately we're completing a namespace member, not a namespace itself. But 
there's no enum value for that: `Expression` would be strings like `vector` 
or `foo()`, and we want `vector` or `foo`.

I'd suggest either adding a new CCC enum value for this 
(PotentiallyQualifiedName is actually the best name for this! we'd need to come 
up with a new one), or use Expression with a comment that this isn't really 
accurate and someone should fix it :-)


Repository:
  rC Clang

https://reviews.llvm.org/D53191



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

[PATCH] D53135: Remove top-level using declaration from header files, as these aliases leak.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344337: Remove top-level using declaration from header 
files, as these aliases leak. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D53135

Files:
  cfe/trunk/include/clang/Driver/Job.h
  cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
  cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h
  cfe/trunk/lib/Frontend/FrontendAction.cpp
  cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
  cfe/trunk/utils/TableGen/TableGenBackends.h

Index: cfe/trunk/utils/TableGen/TableGenBackends.h
===
--- cfe/trunk/utils/TableGen/TableGenBackends.h
+++ cfe/trunk/utils/TableGen/TableGenBackends.h
@@ -23,63 +23,60 @@
   class RecordKeeper;
 }
 
-using llvm::raw_ostream;
-using llvm::RecordKeeper;
-
 namespace clang {
 
-void EmitClangDeclContext(RecordKeeper &RK, raw_ostream &OS);
-void EmitClangASTNodes(RecordKeeper &RK, raw_ostream &OS,
+void EmitClangDeclContext(llvm::RecordKeeper &RK, llvm::raw_ostream &OS);
+void EmitClangASTNodes(llvm::RecordKeeper &RK, llvm::raw_ostream &OS,
const std::string &N, const std::string &S);
 
-void EmitClangAttrParserStringSwitches(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrSubjectMatchRulesParserStringSwitches(RecordKeeper &Records,
-raw_ostream &OS);
-void EmitClangAttrClass(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrImpl(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrList(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrSubjectMatchRuleList(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrPCHRead(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrPCHWrite(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrHasAttrImpl(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrSpellingListIndex(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrASTVisitor(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrTemplateInstantiate(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrParsedAttrList(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrParsedAttrImpl(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrParsedAttrKinds(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangAttrDump(RecordKeeper &Records, raw_ostream &OS);
+void EmitClangAttrParserStringSwitches(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrSubjectMatchRulesParserStringSwitches(llvm::RecordKeeper &Records,
+llvm::raw_ostream &OS);
+void EmitClangAttrClass(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrImpl(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrList(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrSubjectMatchRuleList(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrPCHRead(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrPCHWrite(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrHasAttrImpl(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrSpellingListIndex(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrASTVisitor(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrTemplateInstantiate(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrParsedAttrList(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrParsedAttrImpl(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrParsedAttrKinds(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangAttrDump(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
-void EmitClangDiagsDefs(RecordKeeper &Records, raw_ostream &OS,
+void EmitClangDiagsDefs(llvm::RecordKeeper &Records, llvm::raw_ostream &OS,
 const std::string &Component);
-void EmitClangDiagGroups(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangDiagsIndexName(RecordKeeper &Records, raw_ostream &OS);
+void EmitClangDiagGroups(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
+void EmitClangDiagsIndexName(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
-void EmitClangSACheckers(RecordKeeper &Records, raw_ostream &OS);
+void EmitClangSACheckers(llvm::RecordKeeper &Records, llvm::raw_ostream &OS);
 
-void EmitClangCommentHTMLTags(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangCommentHTMLTagsProperties(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangCommentHTMLNamedCharacterReferences(RecordKeeper &Records, raw_ostream &OS);
-
-void EmitClangCommentCommandInfo(RecordKeeper &Records, raw_ostream &OS);
-void EmitClangCom

r344337 - Remove top-level using declaration from header files, as these aliases leak.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Fri Oct 12 05:21:29 2018
New Revision: 344337

URL: http://llvm.org/viewvc/llvm-project?rev=344337&view=rev
Log:
Remove top-level using declaration from header files, as these aliases leak.

Reviewers: ilya-biryukov

Subscribers: arphaman, cfe-commits

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

Modified:
cfe/trunk/include/clang/Driver/Job.h
cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h
cfe/trunk/lib/Frontend/FrontendAction.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
cfe/trunk/utils/TableGen/TableGenBackends.h

Modified: cfe/trunk/include/clang/Driver/Job.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Job.h?rev=344337&r1=344336&r2=344337&view=diff
==
--- cfe/trunk/include/clang/Driver/Job.h (original)
+++ cfe/trunk/include/clang/Driver/Job.h Fri Oct 12 05:21:29 2018
@@ -30,7 +30,7 @@ class InputInfo;
 class Tool;
 
 // Re-export this as clang::driver::ArgStringList.
-using llvm::opt::ArgStringList;
+using ArgStringList = llvm::opt::ArgStringList;
 
 struct CrashReportInfo {
   StringRef Filename;

Modified: cfe/trunk/include/clang/Frontend/PCHContainerOperations.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/PCHContainerOperations.h?rev=344337&r1=344336&r2=344337&view=diff
==
--- cfe/trunk/include/clang/Frontend/PCHContainerOperations.h (original)
+++ cfe/trunk/include/clang/Frontend/PCHContainerOperations.h Fri Oct 12 
05:21:29 2018
@@ -20,8 +20,6 @@ namespace llvm {
 class raw_pwrite_stream;
 }
 
-using llvm::StringRef;
-
 namespace clang {
 
 class ASTConsumer;
@@ -41,7 +39,7 @@ struct PCHBuffer {
 class PCHContainerWriter {
 public:
   virtual ~PCHContainerWriter() = 0;
-  virtual StringRef getFormat() const = 0;
+  virtual llvm::StringRef getFormat() const = 0;
 
   /// Return an ASTConsumer that can be chained with a
   /// PCHGenerator that produces a wrapper file format containing a
@@ -61,15 +59,15 @@ class PCHContainerReader {
 public:
   virtual ~PCHContainerReader() = 0;
   /// Equivalent to the format passed to -fmodule-format=
-  virtual StringRef getFormat() const = 0;
+  virtual llvm::StringRef getFormat() const = 0;
 
   /// Returns the serialized AST inside the PCH container Buffer.
-  virtual StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const = 0;
+  virtual llvm::StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const = 0;
 };
 
 /// Implements write operations for a raw pass-through PCH container.
 class RawPCHContainerWriter : public PCHContainerWriter {
-  StringRef getFormat() const override { return "raw"; }
+  llvm::StringRef getFormat() const override { return "raw"; }
 
   /// Return an ASTConsumer that can be chained with a
   /// PCHGenerator that writes the module to a flat file.
@@ -83,10 +81,10 @@ class RawPCHContainerWriter : public PCH
 
 /// Implements read operations for a raw pass-through PCH container.
 class RawPCHContainerReader : public PCHContainerReader {
-  StringRef getFormat() const override { return "raw"; }
+  llvm::StringRef getFormat() const override { return "raw"; }
 
   /// Simply returns the buffer contained in Buffer.
-  StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
+  llvm::StringRef ExtractPCH(llvm::MemoryBufferRef Buffer) const override;
 };
 
 /// A registry of PCHContainerWriter and -Reader objects for different formats.
@@ -103,10 +101,10 @@ public:
   void registerReader(std::unique_ptr Reader) {
 Readers[Reader->getFormat()] = std::move(Reader);
   }
-  const PCHContainerWriter *getWriterOrNull(StringRef Format) {
+  const PCHContainerWriter *getWriterOrNull(llvm::StringRef Format) {
 return Writers[Format].get();
   }
-  const PCHContainerReader *getReaderOrNull(StringRef Format) {
+  const PCHContainerReader *getReaderOrNull(llvm::StringRef Format) {
 return Readers[Format].get();
   }
   const PCHContainerReader &getRawReader() {

Modified: cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h?rev=344337&r1=344336&r2=344337&view=diff
==
--- cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h (original)
+++ cfe/trunk/include/clang/Serialization/GlobalModuleIndex.h Fri Oct 12 
05:21:29 2018
@@ -42,11 +42,6 @@ namespace serialization {
   class ModuleFile;
 }
 
-using llvm::SmallVector;
-using llvm::SmallVectorImpl;
-using llvm::StringRef;
-using serialization::ModuleFile;
-
 /// A global index for a set of module files, providing information about
 /// the identifiers within those module files.
 ///
@@ -59,6 +54,8 @@ using serialization::ModuleFile;

[PATCH] D53194: [clang-tidy] Fix check_clang_tidy.py trivially passing default CHECK

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis created this revision.
zinovy.nis added reviewers: ymandel, alexfh.
zinovy.nis added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.

In https://reviews.llvm.org/D52971#1262200, @ymandel wrote:

> Hi,
> 
> It looks like this change has disabled FileCheck for all clang-tidy lit tests 
> that don't use check-prefixes.  So, they all trivially pass even if their 
> CHECK... lines are wrong.  An easy repro is just to randomly modify any CHECK 
> line in a lit file (e.g. 
> llvm/tools/clang/tools/extra/test/clang-tidy/readability-avoid-const-params-in-decls.cpp)
>  and run ninja check-clang-tools.
> 
> In check_clang_tidy.py, if you add back (slightly modified) lines 93-95 to 
> the else branch (line 132), it seems to fix the problem.  For example, add:
> 
>   has_check_fixes = check_fixes_prefixes[0] in input_text
>   has_check_messages = check_messages_prefixes[0] in input_text
>   has_check_notes = check_notes_prefixes[0] in input_text


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53194

Files:
  test/clang-tidy/check_clang_tidy.py


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -51,7 +51,7 @@
   parser.add_argument('check_name')
   parser.add_argument('temp_file_name')
   parser.add_argument('-check-suffix', '-check-suffixes',
-  default=[], type=csv,
+  default=[''], type=csv,
   help="comma-separated list of FileCheck suffixes")
 
   args, extra_args = parser.parse_known_args()
@@ -96,41 +96,37 @@
   has_check_messages = False
   has_check_notes = False
 
-  if any(args.check_suffix):
-for check in args.check_suffix:
-  if not re.match('^[A-Z0-9\-]+$', check):
-sys.exit('Only A..Z, 0..9 and "-" are ' +
-  'allowed in check suffixes list, but "%s" was given' % (check))
-
-  file_check_suffix = '-' + check
-  check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
-  check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
-  check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
-
-  has_check_fix = check_fixes_prefix in input_text 
-  has_check_message = check_messages_prefix in input_text
-  has_check_note = check_notes_prefix in input_text 
-
-  if has_check_note and has_check_message:
-sys.exit('Please use either %s or %s but not both' %
-  (check_notes_prefix, check_messages_prefix))
-
-  if not has_check_fix and not has_check_message and not has_check_note:
-sys.exit('%s, %s or %s not found in the input' %
-  (check_fixes_prefix, check_messages_prefix, check_notes_prefix))
-
-  has_check_fixes = has_check_fixes or has_check_fix
-  has_check_messages = has_check_messages or has_check_message
-  has_check_notes = has_check_notes or has_check_note
-
-  check_fixes_prefixes.append(check_fixes_prefix)
-  check_messages_prefixes.append(check_messages_prefix)
-  check_notes_prefixes.append(check_notes_prefix)
-  else:
-check_fixes_prefixes = ['CHECK-FIXES']
-check_messages_prefixes = ['CHECK-MESSAGES']
-check_notes_prefixes = ['CHECK-NOTES']
+  for check in args.check_suffix:
+if check and not re.match('^[A-Z0-9\-]+$', check):
+  sys.exit('Only A..Z, 0..9 and "-" are ' +
+'allowed in check suffixes list, but "%s" was given' % (check))
 
+file_check_suffix = ('-' + check) if check else ''
+check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
+check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
+check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
+
+has_check_fix = check_fixes_prefix in input_text
+has_check_message = check_messages_prefix in input_text
+has_check_note = check_notes_prefix in input_text
+
+if has_check_note and has_check_message:
+  sys.exit('Please use either %s or %s but not both' %
+(check_notes_prefix, check_messages_prefix))
+
+if not has_check_fix and not has_check_message and not has_check_note:
+  sys.exit('%s, %s or %s not found in the input' %
+(check_fixes_prefix, check_messages_prefix, check_notes_prefix))
+
+has_check_fixes = has_check_fixes or has_check_fix
+has_check_messages = has_check_messages or has_check_message
+has_check_notes = has_check_notes or has_check_note
+
+check_fixes_prefixes.append(check_fixes_prefix)
+check_messages_prefixes.append(check_messages_prefix)
+check_notes_prefixes.append(check_notes_prefix)
+
+  assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related


Index: test/clang-tidy/check_clang_tidy.py
=

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Looks good in general.




Comment at: clangd/ClangdLSPServer.h:117
+llvm::Optional CompileCommandsDir,
+std::function);

please document what the callback is for and how often it's called.



Comment at: clangd/index/Background.cpp:63
+ const tooling::CompilationDatabase &CDB) {
+  // TODO: this function may be slow. Perhaps enqueue a task to re-read the CDB
+  // from disk and enqueue the commands asynchronously?

Maybe add a trace?



Comment at: clangd/index/Background.cpp:98
+  }
+  llvm::sys::path::native(AbsolutePath);
+

Why is this needed?



Comment at: clangd/index/Background.cpp:163
+  vlog("Rebuilding automatic index");
+  reset(IndexedSymbols.buildMemIndex());
+  return Error::success();

Add a FIXME to use Dex?



Comment at: clangd/index/Background.h:1
+//===--- Background.h - Build an index in a background thread *- 
C++-*-===//
+//

Is it possible to add test for this?



Comment at: clangd/index/Background.h:28
+// all commands in a compilation database. Indexing happens in the background.
+// TODO: it should also persist its state on disk for fast start.
+class BackgroundIndex : public SwapIndex {

s/TODO/FIXME/?



Comment at: clangd/index/Background.h:39
+  // The indexing happens in a background thread, so after enqueueing files
+  // for indexing their symbols will be available sometime later.
+  void enqueueAll(llvm::StringRef Directory,

Maybe add a FIXME for files not in CDB e.g. headers? 



Comment at: clangd/tool/ClangdMain.cpp:170
 
+static llvm::cl::opt AutoIndex(
+"auto-index",

Should we ignore index-file/auto-index if one is set?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D52301: [clang] Set TypeSourceInfo for vardecl's in addition to type when we can deduce.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.

Can you give a bit more background on what problem you're trying to solve with 
this patch?




Comment at: lib/Sema/SemaChecking.cpp:852
 
-  if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
-S.Diag(Call->getArg(0)->getBeginLoc(),

I'm not certain I understand why this code has been removed?



Comment at: lib/Sema/SemaDecl.cpp:10766
 
-QualType Sema::deduceVarTypeFromInitializer(VarDecl *VDecl,
+std::pair 
Sema::deduceVarTypeFromInitializer(VarDecl *VDecl,
 DeclarationName Name, QualType 
Type,

Why does this need to return a pair? It seems like the returned 
`TypeSourceInfo*` carries all the information needed by the caller, or is there 
ever a case where the `QualType` will be different than what is returned by 
`TypeSourceInfo::getType()`?


Repository:
  rC Clang

https://reviews.llvm.org/D52301



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

[PATCH] D53187: [clang-tidy] Optimize query in bugprone-exception-escape

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

This looks reasonable to me but could you please explain a bit what the issue 
was in the bugreport? So a very deep hierarchy causing the problem makes sense, 
but why was "ignoring first" the difference maker?
Would it make sense to add a warning in the documentation that this check can 
be very expensive? And are there measures to speed the process up somehow?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53187



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

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D52727#1261901, @JonasToth wrote:

> LG in principle, just the SmallVec thing could be done if you agree. I don't 
> insist on it, but it looks like a performance benefit to me.


I principally agree, but then I also have to duplicate the string list parser 
function in the options to make it work on `SmallVector` as well.


https://reviews.llvm.org/D52727



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

[PATCH] D53194: [clang-tidy] Fix check_clang_tidy.py trivially passing default CHECK

[Jonas Toth via Phabricator via cfe-commits]

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

Did you verify it actually works?
Otherwise LGTM and because its a bug-fix you can commit and other concerns can 
be done post-commit.




Comment at: test/clang-tidy/check_clang_tidy.py:129
+
+  assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on

Nit: Could you please move this assert three lines up? It looks that its 
actually checking the conditions established there.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53194



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

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I see, probably not worth it. Its one-time effort anyway right?
LGTM


https://reviews.llvm.org/D52727



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

[PATCH] D53187: [clang-tidy] Optimize query in bugprone-exception-escape

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

I think that with this optimization it is not so expensive anymore. I do not 
think it was an endless loop in the bugreport but it was insufferable execution 
time. Maybe we could speed it up a little more by changing it totally to a 
width-first CFG visitor. Then we could apply your solution as well (not 
removing visited function from the call stack) so the algorithm would visit 
every called function (for every function that should not throw) only once 
along the shortest path. This would not introduce new false positives neither 
would it lose true positives.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53187



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

[clang-tools-extra] r344340 - [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Adam Balogh via cfe-commits]

Author: baloghadamsoftware
Date: Fri Oct 12 06:05:21 2018
New Revision: 344340

URL: http://llvm.org/viewvc/llvm-project?rev=344340&view=rev
Log:
[clang-tidy] White List Option for performance-unnecessary-value-param, 
performance-unnecessary-copy-initialization and performance-for-range-copy

New option added to these three checks to be able to silence false positives on
types that are intentionally passed by value or copied. Such types are e.g.
intrusive reference counting pointer types like llvm::IntrusiveRefCntPtr. The
new option is named WhiteListTypes and can contain a semicolon-separated list of
names of these types. Regular expressions are allowed. Default is empty.

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


Added:

clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp

clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp

clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h

clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h
clang-tools-extra/trunk/clang-tidy/utils/Matchers.h

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-for-range-copy.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst

clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst

Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp?rev=344340&r1=344339&r2=344340&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp Fri 
Oct 12 06:05:21 2018
@@ -10,6 +10,8 @@
 #include "ForRangeCopyCheck.h"
 #include "../utils/DeclRefExprUtils.h"
 #include "../utils/FixItHintUtils.h"
+#include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include "../utils/TypeTraits.h"
 #include "clang/Analysis/Analyses/ExprMutationAnalyzer.h"
 
@@ -21,10 +23,14 @@ namespace performance {
 
 ForRangeCopyCheck::ForRangeCopyCheck(StringRef Name, ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)) {}
+  WarnOnAllAutoCopies(Options.get("WarnOnAllAutoCopies", 0)),
+  AllowedTypes(
+  utils::options::parseStringList(Options.get("AllowedTypes", ""))) {}
 
 void ForRangeCopyCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "WarnOnAllAutoCopies", WarnOnAllAutoCopies);
+  Options.store(Opts, "AllowedTypes",
+utils::options::serializeStringList(AllowedTypes));
 }
 
 void ForRangeCopyCheck::registerMatchers(MatchFinder *Finder) {
@@ -32,7 +38,10 @@ void ForRangeCopyCheck::registerMatchers
   // initialized through MaterializeTemporaryExpr which indicates a type
   // conversion.
   auto LoopVar = varDecl(
-  hasType(hasCanonicalType(unless(anyOf(referenceType(), pointerType(),
+  hasType(qualType(
+  unless(anyOf(hasCanonicalType(anyOf(referenceType(), pointerType())),
+   hasDeclaration(namedDecl(
+   matchers::matchesAnyListedName(AllowedTypes))),
   unless(hasInitializer(expr(hasDescendant(materializeTemporaryExpr());
   Finder->addMatcher(cxxForRangeStmt(hasLoopVariable(LoopVar.bind("loopVar")))
  .bind("forRange"),
@@ -41,6 +50,7 @@ void ForRangeCopyCheck::registerMatchers
 
 void ForRangeCopyCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Var = Result.Nodes.getNodeAs("loopVar");
+
   // Ignore code in macros since we can't place the fixes correctly.
   if (Var->getBeginLoc().isMacroID())
 return;

Modified: clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h?rev=344340&r1=344339&r2=344340&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h Fri Oct 
12 06:05:21 2018
@@ -40,6 +40,7 @@ private:
ASTContext &Context);
 
   const bool WarnOnAllAutoCopie

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Balogh , Ádám via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344340: [clang-tidy] White List Option for 
performance-unnecessary-value-param… (authored by baloghadamsoftware, committed 
by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52727?vs=168993&id=169383#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52727

Files:
  clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.cpp
  clang-tools-extra/trunk/clang-tidy/performance/ForRangeCopyCheck.h
  
clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryCopyInitialization.h
  clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
  clang-tools-extra/trunk/clang-tidy/performance/UnnecessaryValueParamCheck.h
  clang-tools-extra/trunk/clang-tidy/utils/Matchers.h
  clang-tools-extra/trunk/docs/clang-tidy/checks/performance-for-range-copy.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-copy-initialization.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/performance-unnecessary-value-param.rst
  
clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp
  
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-copy-initialization-allowed-types.cpp
  
clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/performance-for-range-copy-allowed-types.cpp
@@ -0,0 +1,124 @@
+// RUN: %check_clang_tidy %s performance-for-range-copy %t -- -config="{CheckOptions: [{key: performance-for-range-copy.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" -- -std=c++11 -fno-delayed-template-parsing
+
+template 
+struct Iterator {
+  void operator++() {}
+  const T& operator*() {
+static T* TT = new T();
+return *TT;
+  }
+  bool operator!=(const Iterator &) { return false; }
+  typedef const T& const_reference;
+};
+template 
+struct View {
+  T begin() { return T(); }
+  T begin() const { return T(); }
+  T end() { return T(); }
+  T end() const { return T(); }
+  typedef typename T::const_reference const_reference;
+};
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_reference {
+  ~smart_reference();
+};
+
+struct SmartRef {
+  ~SmartRef();
+};
+
+struct smart_ref {
+  ~smart_ref();
+};
+
+struct OtherType {
+  ~OtherType();
+};
+
+template  struct SomeComplexTemplate {
+  ~SomeComplexTemplate();
+};
+
+typedef SomeComplexTemplate NotTooComplexRef;
+
+void negativeSmartPointer() {
+  for (auto P : View>()) {
+auto P2 = P;
+  }
+}
+
+void negative_smart_pointer() {
+  for (auto p : View>()) {
+auto p2 = p;
+  }
+}
+
+void negativeSmartPtr() {
+  for (auto P : View>()) {
+auto P2 = P;
+  }
+}
+
+void negative_smart_ptr() {
+  for (auto p : View>()) {
+auto p2 = p;
+  }
+}
+
+void negativeSmartReference() {
+  for (auto R : View>()) {
+auto R2 = R;
+  }
+}
+
+void negative_smart_reference() {
+  for (auto r : View>()) {
+auto r2 = r;
+  }
+}
+
+void negativeSmartRef() {
+  for (auto R : View>()) {
+auto R2 = R;
+  }
+}
+
+void negative_smart_ref() {
+  for (auto r : View>()) {
+auto r2 = r;
+  }
+}
+
+void positiveOtherType() {
+  for (auto O : View>()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
+  // CHECK-FIXES: for (const auto& O : View>()) {
+auto O2 = O;
+  }
+}
+
+void negativeNotTooComplexRef() {
+  for (NotTooComplexRef R : View>()) {
+auto R2 = R;
+  }
+}
Index: clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/performance-unnecessary-value-param-allowed-types.cpp
@@ -0,0 +1,75 @@
+// RUN: %check_clang_tidy %s performance-unnecessary-value-param %t -- -config="{CheckOptions: [{key: performance-unnecessary-value-param.AllowedTypes, value: '[Pp]ointer$;[Pp]tr$;[Rr]ef(erence)?$'}]}" --
+
+struct SmartPointer {
+  ~SmartPointer();
+};
+
+struct smart_pointer {
+  ~smart_pointer();
+};
+
+struct SmartPtr {
+  ~SmartPtr();
+};
+
+struct smart_ptr {
+  ~smart_ptr();
+};
+
+struct SmartReference {
+  ~SmartReference();
+};
+
+struct smart_ref

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Maybe it should be done in a separate patch together for all checks using that 
utility function. So there would be no need for duplication but the original 
functions should be changed instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D52727



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D41648#1253647, @JonasToth wrote:

> @aaron.ballman What do you think of the current version of this check?
>  As migration strategy the option for `CAPS_ONLY` is provided, otherwise the 
> filtering is provided to silence necessary macros (which kind of enforces a 
> common prefix for macros). This does implement both the cppcoreguidelines and 
> allows migration.


I think that's a reasonable approach.




Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:78
+"function-like macro used; consider a 'constexpr' template function";
+  if (Info->isVariadic())
+DiagnosticMessage = "variadic macro used; consider using a 'constexpr' "

Aren't all vararg macros also function-like macros, so this will trigger two 
diagnostics for all variadic macro definitions?



Comment at: docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst:26
+
+Boolean flag to warn all macros except those with CAPS_ONLY names.
+This option is intended to ease introduction of this check into older

warn all macros -> warn on all macros



Comment at: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp:14
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using 
a 'constexpr' variadic template function

Please add a test like:
```
#define PROBLEMATIC_VARIADIC_TWO(x, ...) (__VA_ARGS__)
```
To ensure that you don't get two diagnostics (one for function-like and one for 
variadic).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[PATCH] D53195: [MinGW] Allow using LTO when lld is used as linker

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: rnk, ruiu, smeenai, compnerd.
Herald added subscribers: dexonsmith, inglorion.

Repository:
  rC Clang

https://reviews.llvm.org/D53195

Files:
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/MinGW.h


Index: lib/Driver/ToolChains/MinGW.h
===
--- lib/Driver/ToolChains/MinGW.h
+++ lib/Driver/ToolChains/MinGW.h
@@ -59,6 +59,8 @@
   MinGW(const Driver &D, const llvm::Triple &Triple,
 const llvm::opt::ArgList &Args);
 
+  bool HasNativeLLVMSupport() const override;
+
   bool IsIntegratedAssemblerDefault() const override;
   bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
@@ -99,6 +101,8 @@
   void findGccLibDir();
   llvm::ErrorOr findGcc();
   llvm::ErrorOr findClangRelativeSysroot();
+
+  bool NativeLLVMSupport;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -10,6 +10,7 @@
 #include "MinGW.h"
 #include "InputInfo.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
@@ -376,6 +377,10 @@
   getFilePaths().push_back(Base + "lib");
   // openSUSE
   getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+
+  NativeLLVMSupport =
+  Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
+  .equals_lower("lld");
 }
 
 bool toolchains::MinGW::IsIntegratedAssemblerDefault() const { return true; }
@@ -403,6 +408,10 @@
   return new tools::MinGW::Linker(*this);
 }
 
+bool toolchains::MinGW::HasNativeLLVMSupport() const {
+  return NativeLLVMSupport;
+}
+
 bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args) const {
   return getArch() == llvm::Triple::x86_64;
 }


Index: lib/Driver/ToolChains/MinGW.h
===
--- lib/Driver/ToolChains/MinGW.h
+++ lib/Driver/ToolChains/MinGW.h
@@ -59,6 +59,8 @@
   MinGW(const Driver &D, const llvm::Triple &Triple,
 const llvm::opt::ArgList &Args);
 
+  bool HasNativeLLVMSupport() const override;
+
   bool IsIntegratedAssemblerDefault() const override;
   bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
@@ -99,6 +101,8 @@
   void findGccLibDir();
   llvm::ErrorOr findGcc();
   llvm::ErrorOr findClangRelativeSysroot();
+
+  bool NativeLLVMSupport;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -10,6 +10,7 @@
 #include "MinGW.h"
 #include "InputInfo.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
@@ -376,6 +377,10 @@
   getFilePaths().push_back(Base + "lib");
   // openSUSE
   getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+
+  NativeLLVMSupport =
+  Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
+  .equals_lower("lld");
 }
 
 bool toolchains::MinGW::IsIntegratedAssemblerDefault() const { return true; }
@@ -403,6 +408,10 @@
   return new tools::MinGW::Linker(*this);
 }
 
+bool toolchains::MinGW::HasNativeLLVMSupport() const {
+  return NativeLLVMSupport;
+}
+
 bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args) const {
   return getArch() == llvm::Triple::x86_64;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52784: [ARM][AArch64] Pass through endianness flags to the GNU assembler and linker

[Peter Smith via Phabricator via cfe-commits]

peter.smith updated this revision to Diff 169386.
peter.smith added a comment.

I've decided to roll the linker changes in with the assembler ones as the 
linker use case affects the design. It turns out that only Arm needs to check 
to see if the -mbig-endian and -mlittle-endian flags override the triple as 
computeTargetTriple() accounts for them for AArch64.

I renamed arm::appendEBLinkFlags to arm::appendBE8LinkFlag as it exclusively 
adds "--be8" for Armv7 and above targets.


https://reviews.llvm.org/D52784

Files:
  lib/Driver/ToolChains/Arch/ARM.cpp
  lib/Driver/ToolChains/Arch/ARM.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/NetBSD.cpp
  test/Driver/linux-as.c
  test/Driver/linux-ld.c

Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -1759,6 +1759,7 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARMEB %s
 // CHECK-ARMEB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ARMEB-NOT: "--be8"
+// CHECK-ARMEB: "-EB"
 // CHECK-ARMEB: "-m" "armelfb_linux_eabi"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
@@ -1768,16 +1769,64 @@
 // RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
 // CHECK-ARMV7EB: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
 // CHECK-ARMV7EB: "--be8"
+// CHECK-ARMV7EB: "-EB"
 // CHECK-ARMV7EB: "-m" "armelfb_linux_eabi"
 
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EB %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armv7-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+// CHECK-ARMV7EL: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-ARMV7EL: "-EL"
+// CHECK-ARMV7EL: "-m" "armelf_linux_eabi"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=armebv7-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-ARMV7EL %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=aarch64_be-unknown-linux \
 // RUN: --gcc-toolchain="" \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
 // CHECK-AARCH64BE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64BE: "-EB"
 // CHECK-AARCH64BE: "-m" "aarch64linuxb"
 
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: -mbig-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64BE %s
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64-unknown-linux \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64LE %s
+// CHECK-AARCH64LE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-AARCH64LE: "-EL"
+// CHECK-AARCH64LE: "-m" "aarch64linux"
+
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=aarch64_be-unknown-linux \
+// RUN: -mlittle-endian \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-AARCH64LE %s
+
 // Check dynamic-linker for musl-libc
 // RUN: %clang %s -### -o %t.o 2>&1 \
 // RUN: --target=i386-pc-linux-musl \
Index: test/Driver/linux-as.c
===
--- test/Driver/linux-as.c
+++ test/Driver/linux-as.c
@@ -3,129 +3,160 @@
 // RUN: %clang -target arm-linux -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM %s
-// CHECK-ARM: as{{(.exe)?}}" "-mfloat-abi=soft"
+// CHECK-ARM: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft"
 //
 // RUN: %clang -target arm-linux -mcpu=cortex-a8 -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM-MCPU %s
-// CHECK-ARM-MCPU: as{{(.exe)?}}" "-mfloat-abi=soft" "-mcpu=cortex-a8"
+// CHECK-ARM-MCPU: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft" "-mcpu=cortex-a8"
 //
 // RUN: %clang -target arm-linux -mfpu=neon -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ARM-MFPU %s
-// CHECK-ARM-MFPU: as{{(.exe)?}}" "-mfloat-abi=soft" "-mfpu=neon"
+// CHECK-ARM-MFPU: as{{(.exe)?}}" "-EL" "-mfloat-abi=soft" "-mfpu=neon"
 //
 // RUN: %clang -target arm-linux -march=armv7-a -### \
 // RUN:   -no-integrated-as -c %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=

[PATCH] D53187: [clang-tidy] Optimize query in bugprone-exception-escape

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D53187#1263294, @baloghadamsoftware wrote:

> I think that with this optimization it is not so expensive anymore. I do not 
> think it was an endless loop in the bugreport but it was insufferable 
> execution time. Maybe we could speed it up a little more by changing it 
> totally to a width-first CFG visitor. Then we could apply your solution as 
> well (not removing visited function from the call stack) so the algorithm 
> would visit every called function (for every function that should not throw) 
> only once along the shortest path. This would not introduce new false 
> positives neither would it lose true positives.


In the debugging if have seen some functions analyzed thousands of times so I 
think this would really make a difference. Caching the result might work too?
If you have time for this I would really appreciate if you took a look into!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53187



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

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I like the thrust of this check, but we already have the 
readability-magic-numbers check and I think that this functionality would fit 
naturally there. That check finds numerical constants and recommends turning 
them into named constants, but for special values it seems reasonable to 
recommend using `numeric_limits` instead. What do others think of that, from an 
organizational perspective?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52892



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

[PATCH] D53187: [clang-tidy] Optimize query in bugprone-exception-escape

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

In https://reviews.llvm.org/D53187#1263323, @JonasToth wrote:

> In https://reviews.llvm.org/D53187#1263294, @baloghadamsoftware wrote:
>
> > I think that with this optimization it is not so expensive anymore. I do 
> > not think it was an endless loop in the bugreport but it was insufferable 
> > execution time. Maybe we could speed it up a little more by changing it 
> > totally to a width-first CFG visitor. Then we could apply your solution as 
> > well (not removing visited function from the call stack) so the algorithm 
> > would visit every called function (for every function that should not 
> > throw) only once along the shortest path. This would not introduce new 
> > false positives neither would it lose true positives.
>
>
> In the debugging if have seen some functions analyzed thousands of times so I 
> think this would really make a difference. Caching the result might work too?


Thousands? After the query optimization the max was 173, and that only for a 
single function. The next number was 64.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53187



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

[PATCH] D53195: [MinGW] Allow using LTO when lld is used as linker

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 169387.
mstorsjo added a comment.
Herald added subscribers: steven_wu, mehdi_amini.

Added a minimal testcase.


https://reviews.llvm.org/D53195

Files:
  lib/Driver/ToolChains/MinGW.cpp
  lib/Driver/ToolChains/MinGW.h
  test/Driver/mingw-lto.c


Index: test/Driver/mingw-lto.c
===
--- /dev/null
+++ test/Driver/mingw-lto.c
@@ -0,0 +1,4 @@
+// The default linker doesn't support LLVM bitcode
+// RUN: not %clang -target i686-pc-windows-gnu %s -flto -fuse-ld=bfd
+// When using lld, this is allowed though.
+// RUN: %clang -target i686-pc-windows-gnu -### %s -flto -fuse-ld=lld
Index: lib/Driver/ToolChains/MinGW.h
===
--- lib/Driver/ToolChains/MinGW.h
+++ lib/Driver/ToolChains/MinGW.h
@@ -59,6 +59,8 @@
   MinGW(const Driver &D, const llvm::Triple &Triple,
 const llvm::opt::ArgList &Args);
 
+  bool HasNativeLLVMSupport() const override;
+
   bool IsIntegratedAssemblerDefault() const override;
   bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
@@ -99,6 +101,8 @@
   void findGccLibDir();
   llvm::ErrorOr findGcc();
   llvm::ErrorOr findClangRelativeSysroot();
+
+  bool NativeLLVMSupport;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -10,6 +10,7 @@
 #include "MinGW.h"
 #include "InputInfo.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
@@ -376,6 +377,10 @@
   getFilePaths().push_back(Base + "lib");
   // openSUSE
   getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+
+  NativeLLVMSupport =
+  Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
+  .equals_lower("lld");
 }
 
 bool toolchains::MinGW::IsIntegratedAssemblerDefault() const { return true; }
@@ -403,6 +408,10 @@
   return new tools::MinGW::Linker(*this);
 }
 
+bool toolchains::MinGW::HasNativeLLVMSupport() const {
+  return NativeLLVMSupport;
+}
+
 bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args) const {
   return getArch() == llvm::Triple::x86_64;
 }


Index: test/Driver/mingw-lto.c
===
--- /dev/null
+++ test/Driver/mingw-lto.c
@@ -0,0 +1,4 @@
+// The default linker doesn't support LLVM bitcode
+// RUN: not %clang -target i686-pc-windows-gnu %s -flto -fuse-ld=bfd
+// When using lld, this is allowed though.
+// RUN: %clang -target i686-pc-windows-gnu -### %s -flto -fuse-ld=lld
Index: lib/Driver/ToolChains/MinGW.h
===
--- lib/Driver/ToolChains/MinGW.h
+++ lib/Driver/ToolChains/MinGW.h
@@ -59,6 +59,8 @@
   MinGW(const Driver &D, const llvm::Triple &Triple,
 const llvm::opt::ArgList &Args);
 
+  bool HasNativeLLVMSupport() const override;
+
   bool IsIntegratedAssemblerDefault() const override;
   bool IsUnwindTablesDefault(const llvm::opt::ArgList &Args) const override;
   bool isPICDefault() const override;
@@ -99,6 +101,8 @@
   void findGccLibDir();
   llvm::ErrorOr findGcc();
   llvm::ErrorOr findClangRelativeSysroot();
+
+  bool NativeLLVMSupport;
 };
 
 } // end namespace toolchains
Index: lib/Driver/ToolChains/MinGW.cpp
===
--- lib/Driver/ToolChains/MinGW.cpp
+++ lib/Driver/ToolChains/MinGW.cpp
@@ -10,6 +10,7 @@
 #include "MinGW.h"
 #include "InputInfo.h"
 #include "CommonArgs.h"
+#include "clang/Config/config.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
@@ -376,6 +377,10 @@
   getFilePaths().push_back(Base + "lib");
   // openSUSE
   getFilePaths().push_back(Base + Arch + "/sys-root/mingw/lib");
+
+  NativeLLVMSupport =
+  Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
+  .equals_lower("lld");
 }
 
 bool toolchains::MinGW::IsIntegratedAssemblerDefault() const { return true; }
@@ -403,6 +408,10 @@
   return new tools::MinGW::Linker(*this);
 }
 
+bool toolchains::MinGW::HasNativeLLVMSupport() const {
+  return NativeLLVMSupport;
+}
+
 bool toolchains::MinGW::IsUnwindTablesDefault(const ArgList &Args) const {
   return getArch() == llvm::Triple::x86_64;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> Unfortunately, I won't be able to review the code in the upcoming few weeks 
> as I caught a cold and I'm trying to get better before the LLVM Meeting, so 
> if there is anyone else interested in reviewing proposed changes please feel 
> free to jump in.

Thank you very much for the review so far! I think this patch is already close 
to finish so anyone can continue with the review :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D53194: [clang-tidy] Fix check_clang_tidy.py trivially passing default CHECK

[Zinovy Nis via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL344343: [clang-tidy] Fix check_clang_tidy.py trivially 
passing default CHECK (authored by zinovy.nis, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D53194?vs=169379&id=169388#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D53194

Files:
  clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py


Index: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
@@ -51,7 +51,7 @@
   parser.add_argument('check_name')
   parser.add_argument('temp_file_name')
   parser.add_argument('-check-suffix', '-check-suffixes',
-  default=[], type=csv,
+  default=[''], type=csv,
   help="comma-separated list of FileCheck suffixes")
 
   args, extra_args = parser.parse_known_args()
@@ -96,41 +96,37 @@
   has_check_messages = False
   has_check_notes = False
 
-  if any(args.check_suffix):
-for check in args.check_suffix:
-  if not re.match('^[A-Z0-9\-]+$', check):
-sys.exit('Only A..Z, 0..9 and "-" are ' +
-  'allowed in check suffixes list, but "%s" was given' % (check))
-
-  file_check_suffix = '-' + check
-  check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
-  check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
-  check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
-
-  has_check_fix = check_fixes_prefix in input_text 
-  has_check_message = check_messages_prefix in input_text
-  has_check_note = check_notes_prefix in input_text 
-
-  if has_check_note and has_check_message:
-sys.exit('Please use either %s or %s but not both' %
-  (check_notes_prefix, check_messages_prefix))
-
-  if not has_check_fix and not has_check_message and not has_check_note:
-sys.exit('%s, %s or %s not found in the input' %
-  (check_fixes_prefix, check_messages_prefix, check_notes_prefix))
-
-  has_check_fixes = has_check_fixes or has_check_fix
-  has_check_messages = has_check_messages or has_check_message
-  has_check_notes = has_check_notes or has_check_note
-
-  check_fixes_prefixes.append(check_fixes_prefix)
-  check_messages_prefixes.append(check_messages_prefix)
-  check_notes_prefixes.append(check_notes_prefix)
-  else:
-check_fixes_prefixes = ['CHECK-FIXES']
-check_messages_prefixes = ['CHECK-MESSAGES']
-check_notes_prefixes = ['CHECK-NOTES']
+  for check in args.check_suffix:
+if check and not re.match('^[A-Z0-9\-]+$', check):
+  sys.exit('Only A..Z, 0..9 and "-" are ' +
+'allowed in check suffixes list, but "%s" was given' % (check))
+
+file_check_suffix = ('-' + check) if check else ''
+check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
+check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
+check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
+
+has_check_fix = check_fixes_prefix in input_text
+has_check_message = check_messages_prefix in input_text
+has_check_note = check_notes_prefix in input_text
+
+if has_check_note and has_check_message:
+  sys.exit('Please use either %s or %s but not both' %
+(check_notes_prefix, check_messages_prefix))
+
+if not has_check_fix and not has_check_message and not has_check_note:
+  sys.exit('%s, %s or %s not found in the input' %
+(check_fixes_prefix, check_messages_prefix, check_notes_prefix))
+
+has_check_fixes = has_check_fixes or has_check_fix
+has_check_messages = has_check_messages or has_check_message
+has_check_notes = has_check_notes or has_check_note
+
+check_fixes_prefixes.append(check_fixes_prefix)
+check_messages_prefixes.append(check_messages_prefix)
+check_notes_prefixes.append(check_notes_prefix)
 
+  assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related


Index: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
@@ -51,7 +51,7 @@
   parser.add_argument('check_name')
   parser.add_argument('temp_file_name')
   parser.add_argument('-check-suffix', '-check-suffixes',
-  default=[], type=csv,
+  default=[''], type=csv,
   help="comma-separated list of FileCheck suffixes")
 
   args, extra_args = parser.parse_known_args()

[clang-tools-extra] r344343 - [clang-tidy] Fix check_clang_tidy.py trivially passing default CHECK

[Zinovy Nis via cfe-commits]

Author: zinovy.nis
Date: Fri Oct 12 06:35:47 2018
New Revision: 344343

URL: http://llvm.org/viewvc/llvm-project?rev=344343&view=rev
Log:
[clang-tidy] Fix check_clang_tidy.py trivially passing default CHECK

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

Modified:
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py

Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py?rev=344343&r1=344342&r2=344343&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py (original)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Fri Oct 12 
06:35:47 2018
@@ -51,7 +51,7 @@ def main():
   parser.add_argument('check_name')
   parser.add_argument('temp_file_name')
   parser.add_argument('-check-suffix', '-check-suffixes',
-  default=[], type=csv,
+  default=[''], type=csv,
   help="comma-separated list of FileCheck suffixes")
 
   args, extra_args = parser.parse_known_args()
@@ -96,41 +96,37 @@ def main():
   has_check_messages = False
   has_check_notes = False
 
-  if any(args.check_suffix):
-for check in args.check_suffix:
-  if not re.match('^[A-Z0-9\-]+$', check):
-sys.exit('Only A..Z, 0..9 and "-" are ' +
-  'allowed in check suffixes list, but "%s" was given' % (check))
-
-  file_check_suffix = '-' + check
-  check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
-  check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
-  check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
-
-  has_check_fix = check_fixes_prefix in input_text 
-  has_check_message = check_messages_prefix in input_text
-  has_check_note = check_notes_prefix in input_text 
-
-  if has_check_note and has_check_message:
-sys.exit('Please use either %s or %s but not both' %
-  (check_notes_prefix, check_messages_prefix))
-
-  if not has_check_fix and not has_check_message and not has_check_note:
-sys.exit('%s, %s or %s not found in the input' %
-  (check_fixes_prefix, check_messages_prefix, check_notes_prefix))
-
-  has_check_fixes = has_check_fixes or has_check_fix
-  has_check_messages = has_check_messages or has_check_message
-  has_check_notes = has_check_notes or has_check_note
-
-  check_fixes_prefixes.append(check_fixes_prefix)
-  check_messages_prefixes.append(check_messages_prefix)
-  check_notes_prefixes.append(check_notes_prefix)
-  else:
-check_fixes_prefixes = ['CHECK-FIXES']
-check_messages_prefixes = ['CHECK-MESSAGES']
-check_notes_prefixes = ['CHECK-NOTES']
+  for check in args.check_suffix:
+if check and not re.match('^[A-Z0-9\-]+$', check):
+  sys.exit('Only A..Z, 0..9 and "-" are ' +
+'allowed in check suffixes list, but "%s" was given' % (check))
+
+file_check_suffix = ('-' + check) if check else ''
+check_fixes_prefix = 'CHECK-FIXES' + file_check_suffix
+check_messages_prefix = 'CHECK-MESSAGES' + file_check_suffix
+check_notes_prefix = 'CHECK-NOTES' + file_check_suffix
+
+has_check_fix = check_fixes_prefix in input_text
+has_check_message = check_messages_prefix in input_text
+has_check_note = check_notes_prefix in input_text
+
+if has_check_note and has_check_message:
+  sys.exit('Please use either %s or %s but not both' %
+(check_notes_prefix, check_messages_prefix))
+
+if not has_check_fix and not has_check_message and not has_check_note:
+  sys.exit('%s, %s or %s not found in the input' %
+(check_fixes_prefix, check_messages_prefix, check_notes_prefix))
+
+has_check_fixes = has_check_fixes or has_check_fix
+has_check_messages = has_check_messages or has_check_message
+has_check_notes = has_check_notes or has_check_note
+
+check_fixes_prefixes.append(check_fixes_prefix)
+check_messages_prefixes.append(check_messages_prefix)
+check_notes_prefixes.append(check_notes_prefix)
 
+  assert has_check_fixes or has_check_messages or has_check_notes
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related


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

[PATCH] D53187: [clang-tidy] Optimize query in bugprone-exception-escape

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

> Thousands? After the query optimization the max was 173, and that only for a 
> single function. The next number was 64.

See https://bugs.llvm.org/attachment.cgi?id=20963
I did not run again with your patch. But even 173 and 64 seem like a lot and 
might be worth optimizing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53187



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

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

[Jonas Toth via Phabricator via cfe-commits]

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

- fix headline in doc


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/IsolateDeclarationCheck.cpp
  clang-tidy/readability/IsolateDeclarationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tidy/utils/LexerUtils.cpp
  clang-tidy/utils/LexerUtils.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-isolate-declaration.rst
  test/clang-tidy/readability-isolate-declaration-cxx17.cpp
  test/clang-tidy/readability-isolate-declaration-fixing.cpp
  test/clang-tidy/readability-isolate-declaration.c
  test/clang-tidy/readability-isolate-declaration.cpp

Index: test/clang-tidy/readability-isolate-declaration.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-isolate-declaration.cpp
@@ -0,0 +1,412 @@
+// RUN: %check_clang_tidy %s readability-isolate-declaration %t
+
+void f() {
+  int i;
+}
+
+void f2() {
+  int i, j, *k, lala = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int j;
+  // CHECK-FIXES: {{^  }}int *k;
+  // CHECK-FIXES: {{^  }}int lala = 42;
+
+  int normal, weird = /* comment */ 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int normal;
+  // CHECK-FIXES: {{^  }}int weird = /* comment */ 42;
+
+  int /* here is a comment */ v1,
+  // another comment
+  v2 = 42 // Ok, more comments
+  ;
+  // CHECK-MESSAGES: [[@LINE-4]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int /* here is a comment */ v1;
+  // CHECK-FIXES: {{^  }}int /* here is a comment */ // another comment
+  // CHECK-FIXES: {{^  }}v2 = 42 // Ok, more comments
+  // CHECK-FIXES: {{^  }};
+
+  auto int1 = 42, int2 = 0, int3 = 43;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: auto int1 = 42;
+  // CHECK-FIXES: {{^  }}auto int2 = 0;
+  // CHECK-FIXES: {{^  }}auto int3 = 43;
+
+  decltype(auto) ptr1 = &int1, ptr2 = &int1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: decltype(auto) ptr1 = &int1;
+  // CHECK-FIXES: {{^  }}decltype(auto) ptr2 = &int1;
+
+  decltype(k) ptr3 = &int1, ptr4 = &int1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: decltype(k) ptr3 = &int1;
+  // CHECK-FIXES: {{^  }}decltype(k) ptr4 = &int1;
+}
+
+void f3() {
+  int i, *pointer1;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int i;
+  // CHECK-FIXES: {{^  }}int *pointer1;
+  //
+  int *pointer2 = nullptr, *pointer3 = &i;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *pointer2 = nullptr;
+  // CHECK-FIXES: {{^  }}int *pointer3 = &i;
+
+  int *(i_ptr) = nullptr, *((i_ptr2));
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: int *(i_ptr) = nullptr;
+  // CHECK-FIXES: {{^  }}int *((i_ptr2));
+
+  float(*f_ptr)[42], (((f_value))) = 42;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (*f_ptr)[42];
+  // CHECK-FIXES: {{^  }}float (((f_value))) = 42;
+
+  float(((*f_ptr2)))[42], ((*f_ptr3)), f_value2 = 42.f;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (((*f_ptr2)))[42];
+  // CHECK-FIXES: {{^  }}float ((*f_ptr3));
+  // CHECK-FIXES: {{^  }}float f_value2 = 42.f;
+
+  float(((*f_ptr4)))[42], *f_ptr5, ((f_value3));
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (((*f_ptr4)))[42];
+  // CHECK-FIXES: {{^  }}float *f_ptr5;
+  // CHECK-FIXES: {{^  }}float ((f_value3));
+
+  void(((*f2))(int)), (*g2)(int, float);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: void (((*f2))(int));
+  // CHECK-FIXES: {{^  }}void (*g2)(int, float);
+
+  float(*(*(*f_ptr6)))[42], (*f_ptr7);
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: multiple declarations in a single statement reduces readability
+  // CHECK-FIXES: float (*(*(*f_ptr6)))[42];
+  // CHECK-FIXES: {{^  }}float (*f_ptr7);
+}
+
+void f4() {
+  double d = 42. /* foo */, z = 43., /* hi */ y, c /* */ /*  */, l = 2.;
+  // CHECK-MESSAGES: [[@LINE-1]]:3: 

[PATCH] D53025: [clang-tidy] implement new check for const return types.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:25
+
+// Finds the location of the relevant "const" token in `Def`.
+llvm::Optional findConstToRemove(

ymandel wrote:
> JonasToth wrote:
> > s/"const"/`const`
> here and throughout.  All comments mention const without quotes.
I meant that you use the  \` thingie to mark const :)
Its better to mark language constructs in the comments as well for better 
clarity whats meant. Comments need to be full sentences and super clear, 
otherwise the comment does more harm then helping :)



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:79
+   Decl = Decl->getPreviousDecl()) {
+if (const llvm::Optional PrevLoc =
+findConstToRemove(Decl, Result)) {

ymandel wrote:
> JonasToth wrote:
> > The `const` is not common in llvm-code. Please use it only for references 
> > and pointers.
> > 
> > What do you think about emitting a `note` if this transformation can not be 
> > done? It is relevant for the user as he might need to do manual fix-up. It 
> > would complicate the code as there can only be one(?) diagnostic at the 
> > same time (90% sure only tbh).
> Not the most elegant, but I don't see any other way to display multiple 
> diagnoses. Let me know what you think.
What do you want to achieve? I think you just want to append the `FixItHint` do 
you?

you can do this with saving the diagnostic object in a variable.

```
auto Diag = diag(Loc, "Warning Message");

// Foo

if (HaveFix)
Diag << FixItHint::CreateRemove();
```

When `Diag` goes out of scope the diagnostic is actually issued, calling just 
`diag` produces a temporary that gets immediately destroyed. 



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:57
+  if (!Def->getReturnType().isLocalConstQualified()) {
+// From the matcher, we know this is const qualified.  But it is not done
+// locally, so we can't offer a fix -- just a warning.

Please make the second sentence a gramatically correct english sentence, right 
now it's a little off.



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:60
+diag(Def->getInnerLocStart(),
+ "return type is 'const'-qualifed (possibly behind a type alias), "
+ "which hinders compiler optimizations");

This diagnostic is nice, but including the type would make it even better. This 
will resolve typedefs as well (like "'uint32_t (a.k.a. 'unsigned int')").

Necessary code (more info in the cfe-internals manual)
```
diag(Location, "return type %0 is 'const'-qualified hindering compiler 
optimizations") << Def->getReturnType();
```
The value passed in with `<<` must be a `QualType`.
Also diagnostics don't have punctuation and are not full sentences and should 
be as short as possible (but still clear).



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:69
+// a macro. So, warn the user, but offer no fix.
+diag(Def->getInnerLocStart(),
+ "return type is 'const'-qualifed (possibly behind a macro), which "

This duplicates the diagnostic above. The diagnostics engine would resolve 
macro expansion and emit the `note: expanded from XXX`. I think this does not 
add value and introduces duplication.



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:83
+// warning at the definition.
+DiagnosticBuilder Diagnostics =
+diag(*Loc,

You don't need this diagnostic again, you can just use the one and first 
diagnostic from the beginning. Right now you are creating to many warnings 
which are all redundant.
Please refactor the logic to

1. Create warning for everything because the matcher found an offending 
definition
2. If(!NotFixable) return
3. CreateFixOtherwiseForAllDefintions

This looks simpler to me.



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:93
+Diagnostics << FixItHint::CreateRemoval(
+CharSourceRange::getTokenRange(*PrevLoc, *PrevLoc));
+  } else {

Twice `*PrevLoc`?



Comment at: clang-tidy/readability/ConstValueReturnCheck.cpp:95
+  } else {
+UnfixabledDecls.emplace_back(
+Decl->getInnerLocStart(),

Did you consider `diag(Loc, "could not transform this declaration", 
DiagnosticIDs::Note)` which emits a `note` instead of `warning`.
You dont need to store these in between.



Comment at: docs/clang-tidy/checks/list.rst:12
abseil-redundant-strcat-calls
-   abseil-string-find-startswith
abseil-str-cat-append

ymandel wrote:
> JonasToth wrote:
> > spurious change
> right. this was done by the script, so I wonder why it reordered these.
It reads all checks, orders and inserts the new one and prints them out again, 
thats why the change ha

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D52892#1263324, @aaron.ballman wrote:

> I like the thrust of this check, but we already have the 
> readability-magic-numbers check and I think that this functionality would fit 
> naturally there. That check finds numerical constants and recommends turning 
> them into named constants, but for special values it seems reasonable to 
> recommend using `numeric_limits` instead. What do others think of that, from 
> an organizational perspective?


+1, totally forgot that one. I feel that the bugprone is more strict though, so 
adding an `readability`-mode for it that only warns for the `numeric_limits` 
constants seems desirable.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52892



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

[PATCH] D53038: [Hexagon] Use GetLinkerPath method instead of hard-coded linker name.

[David Zarzycki via Phabricator via cfe-commits]

davezarzycki added a comment.

A bisect revealed this change as the source of test failures on my Red Hat 
Fedora 28 Linux box. How was/is this tested?

Command Output (stderr):


/home/dave/s/lc/tools/clang/test/Driver/linux-ld.c:882:19: error: 
CHECK-HEXAGON: expected string not found in input
// CHECK-HEXAGON: "{{.*}}hexagon-link{{(.exe)?}}"

  ^

:1:1: note: scanning from here
clang version 8.0.0 (https://git.llvm.org/git/clang.git 
efe41bf98e6011bb413a5056f0025a4509de860d) (https://git.llvm.org/git/llvm.git 
638941f488dd9d1b1b9d49913240fdc2beec56a0)
^
:2:3: note: possible intended match here
Target: hexagon-unknown-linux-gnu

  ^

-



Testing Time: 43.34s



Failing Tests (2):

Clang :: Driver/hexagon-toolchain-elf.c
Clang :: Driver/linux-ld.c
  
  Expected Passes: 34506
  Expected Failures  : 82
  Unsupported Tests  : 8838
  Unexpected Failures: 2

FAILED: CMakeFiles/check-all


Repository:
  rC Clang

https://reviews.llvm.org/D53038



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

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[Marcus Hultman via Phabricator via cfe-commits]

hultman created this revision.
Herald added a subscriber: cfe-commits.

> clang-format --version
>  clang-format version 7.0.0 (tags/RELEASE_700/final)
>  echo "@implementation Foo\n- (void)foo:(id)bar\n{\n}\n@end\n" |clang-format 
> -style='{BreakBeforeBraces: Custom, BraceWrapping: {AfterFunction: true}}'

  @implementation Foo
  - (void)foo:(id)bar {
  }
  @end

with patch:

> bin/clang-format --version
>  clang-format version 8.0.0 (trunk 344285)
>  echo "@implementation Foo\n- (void)foo:(id)bar\n{\n}\n@end\n" 
> |bin/clang-format -style='{BreakBeforeBraces: Custom, BraceWrapping: 
> {AfterFunction: true}}'

  @implementation Foo
  - (void)foo:(id)bar
  {
  }
  @end


Repository:
  rC Clang

https://reviews.llvm.org/D53197

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -584,6 +584,16 @@
" a:(a)yyy\n"
"   bbb:(d);");
   verifyFormat("- (void)drawRectOn:(id)surface 
ofSize:(aaa)height:(bbb)width;");
+
+  // BraceWrapping AfterFunction is respected for ObjC methods 
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("@implementation Foo\n"
+   "- (void)foo:(id)bar\n"
+   "{\n"
+   "}\n"
+   "@end\n");
 }
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2164,6 +2164,8 @@
   addUnwrappedLine();
   return;
 } else if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction)
+addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
   addUnwrappedLine();
   return;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -584,6 +584,16 @@
" a:(a)yyy\n"
"   bbb:(d);");
   verifyFormat("- (void)drawRectOn:(id)surface ofSize:(aaa)height:(bbb)width;");
+
+  // BraceWrapping AfterFunction is respected for ObjC methods 
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("@implementation Foo\n"
+   "- (void)foo:(id)bar\n"
+   "{\n"
+   "}\n"
+   "@end\n");
 }
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2164,6 +2164,8 @@
   addUnwrappedLine();
   return;
 } else if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction)
+addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
   addUnwrappedLine();
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D45050



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

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 6 inline comments as done.
sammccall added inline comments.



Comment at: clangd/index/Background.cpp:98
+  }
+  llvm::sys::path::native(AbsolutePath);
+

ioeric wrote:
> Why is this needed?
Removed. I copied it from the related code in JSONCompilationDatabase, but I'm 
not sure what the right behavior is, as JSONCompilationDatabase doesn't specify 
the format of paths.



Comment at: clangd/index/Background.cpp:163
+  vlog("Rebuilding automatic index");
+  reset(IndexedSymbols.buildMemIndex());
+  return Error::success();

ioeric wrote:
> Add a FIXME to use Dex?
Added it to the existing TODO. They're coupled: dex is expensive to build, so 
rebuilding it after every file wouldnt' make sense.



Comment at: clangd/index/Background.h:1
+//===--- Background.h - Build an index in a background thread *- 
C++-*-===//
+//

ioeric wrote:
> Is it possible to add test for this?
Added a test that indexes a file, waits for the queue to be empty, and queries.

That's pretty basic, do you have suggestions for things you'd like to see 
tested?



Comment at: clangd/index/Background.h:39
+  // The indexing happens in a background thread, so after enqueueing files
+  // for indexing their symbols will be available sometime later.
+  void enqueueAll(llvm::StringRef Directory,

ioeric wrote:
> Maybe add a FIXME for files not in CDB e.g. headers? 
There's nothing missing for headers at this point.
Headers will already be indexed (as part of any TU that includes them).
Later, we'll partition the outputs by file, but these headers will still be 
indexed.
When it comes time to watch files on disk for changes, then we'll need to 
consider headers specially.

Indexing headers that are not included by any file is out of scope (it's not 
clear this is a good idea, e.g. consider files under test/ in llvm).



Comment at: clangd/tool/ClangdMain.cpp:170
 
+static llvm::cl::opt AutoIndex(
+"auto-index",

ioeric wrote:
> Should we ignore index-file/auto-index if one is set?
Since this is hidden/experimental, I think it's not worth adding explicit code 
so they interact nicely, I just documented the behavior instead instead.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 169399.
sammccall marked 4 inline comments as done.
sammccall added a comment.

Address comments, add tests.
Some changes (enqueue(), blockUntilIdleForTest()) to support testing.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/Compiler.cpp
  clangd/Compiler.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SyncAPI.cpp
  unittests/clangd/SyncAPI.h

Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -17,7 +17,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 
 #include "ClangdServer.h"
-#include 
+#include "index/Index.h"
 
 namespace clang {
 namespace clangd {
@@ -50,6 +50,9 @@
 llvm::Expected>
 runDocumentSymbols(ClangdServer &Server, PathRef File);
 
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
+
 } // namespace clangd
 } // namespace clang
 
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -125,5 +125,19 @@
   return std::move(*Result);
 }
 
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query) {
+  FuzzyFindRequest Req;
+  Req.Query = Query;
+  return runFuzzyFind(Index, Req);
+}
+
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req) {
+  SymbolSlab::Builder Builder;
+  Index.fuzzyFind(Req, [&](const Symbol& Sym) {
+Builder.insert(Sym);
+  });
+  return std::move(Builder).build();
+}
+
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
 
 add_extra_unittest(ClangdTests
   Annotations.cpp
+  BackgroundIndexTests.cpp
   CancellationTests.cpp
   ClangdTests.cpp
   ClangdUnitTests.cpp
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- /dev/null
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -0,0 +1,30 @@
+#include "index/Background.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using testing::ElementsAre;
+
+namespace clang {
+namespace clangd {
+
+MATCHER_P(Named, N, "") { return arg.Name == N; }
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.h")] = "void NAME(){}";
+  FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
+  BackgroundIndex Idx(Context::empty(), "", FS);
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", "-DNAME=foo", testPath("root/A.cc")};
+  Idx.enqueue(testPath("root"), std::move(Cmd));
+
+  Idx.blockUntilIdleForTest();
+  EXPECT_THAT(runFuzzyFind(Idx, "f"), ElementsAre(Named("foo")));
+}
+
+} // namespace clangd
+} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -167,6 +167,15 @@
 "eventually. Don't rely on it."),
 llvm::cl::init(""), llvm::cl::Hidden);
 
+static llvm::cl::opt AutoIndex(
+"auto-index",
+llvm::cl::desc(
+"Build a full index for the codebase containing edited files. "
+"Indexing will occur in the background. "
+"This option is still experimental, as the indexing is inefficient."
+"Takes precedence over -index-file."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
 enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
 static llvm::cl::opt CompileArgsFrom(
 "compile_args_from", llvm::cl::desc("The source of compile commands"),
@@ -316,9 +325,10 @@
   CCOpts.AllScopes = AllScopesCompletion;
 
   // Initialize and run ClangdLSPServer.
-  ClangdLSPServer LSPServer(
-  Out, CCOpts, CompileCommandsDirPath,
-  /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
+  ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath,
+/*ShouldUseInMemoryCDB=*/CompileArgsFrom ==
+LSPCompileArgs,
+AutoIndex, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   // Change stdin to binary to not lose \r\n on windows.
Index: clangd/index/Background.h
==

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 169400.
sammccall added a comment.

Oops, forgot one comment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/Compiler.cpp
  clangd/Compiler.h
  clangd/GlobalCompilationDatabase.cpp
  clangd/GlobalCompilationDatabase.h
  clangd/index/Background.cpp
  clangd/index/Background.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/CMakeLists.txt
  unittests/clangd/SyncAPI.cpp
  unittests/clangd/SyncAPI.h

Index: unittests/clangd/SyncAPI.h
===
--- unittests/clangd/SyncAPI.h
+++ unittests/clangd/SyncAPI.h
@@ -17,7 +17,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_UNITTESTS_CLANGD_SYNCAPI_H
 
 #include "ClangdServer.h"
-#include 
+#include "index/Index.h"
 
 namespace clang {
 namespace clangd {
@@ -50,6 +50,9 @@
 llvm::Expected>
 runDocumentSymbols(ClangdServer &Server, PathRef File);
 
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query);
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req);
+
 } // namespace clangd
 } // namespace clang
 
Index: unittests/clangd/SyncAPI.cpp
===
--- unittests/clangd/SyncAPI.cpp
+++ unittests/clangd/SyncAPI.cpp
@@ -125,5 +125,19 @@
   return std::move(*Result);
 }
 
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, StringRef Query) {
+  FuzzyFindRequest Req;
+  Req.Query = Query;
+  return runFuzzyFind(Index, Req);
+}
+
+SymbolSlab runFuzzyFind(const SymbolIndex &Index, const FuzzyFindRequest &Req) {
+  SymbolSlab::Builder Builder;
+  Index.fuzzyFind(Req, [&](const Symbol& Sym) {
+Builder.insert(Sym);
+  });
+  return std::move(Builder).build();
+}
+
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
 
 add_extra_unittest(ClangdTests
   Annotations.cpp
+  BackgroundIndexTests.cpp
   CancellationTests.cpp
   ClangdTests.cpp
   ClangdUnitTests.cpp
Index: unittests/clangd/BackgroundIndexTests.cpp
===
--- /dev/null
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -0,0 +1,30 @@
+#include "index/Background.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using testing::ElementsAre;
+
+namespace clang {
+namespace clangd {
+
+MATCHER_P(Named, N, "") { return arg.Name == N; }
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;
+  FS.Files[testPath("root/A.h")] = "void NAME(){}";
+  FS.Files[testPath("root/A.cc")] = "#include \"A.h\"";
+  BackgroundIndex Idx(Context::empty(), "", FS);
+  tooling::CompileCommand Cmd;
+  Cmd.Filename = testPath("root/A.cc");
+  Cmd.Directory = testPath("root");
+  Cmd.CommandLine = {"clang++", "-DNAME=foo", testPath("root/A.cc")};
+  Idx.enqueue(testPath("root"), std::move(Cmd));
+
+  Idx.blockUntilIdleForTest();
+  EXPECT_THAT(runFuzzyFind(Idx, "f"), ElementsAre(Named("foo")));
+}
+
+} // namespace clangd
+} // namespace clang
Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -167,6 +167,15 @@
 "eventually. Don't rely on it."),
 llvm::cl::init(""), llvm::cl::Hidden);
 
+static llvm::cl::opt AutoIndex(
+"auto-index",
+llvm::cl::desc(
+"Build a full index for the codebase containing edited files. "
+"Indexing will occur in the background. "
+"This option is still experimental, as the indexing is inefficient."
+"Takes precedence over -index-file."),
+llvm::cl::init(false), llvm::cl::Hidden);
+
 enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
 static llvm::cl::opt CompileArgsFrom(
 "compile_args_from", llvm::cl::desc("The source of compile commands"),
@@ -316,9 +325,10 @@
   CCOpts.AllScopes = AllScopesCompletion;
 
   // Initialize and run ClangdLSPServer.
-  ClangdLSPServer LSPServer(
-  Out, CCOpts, CompileCommandsDirPath,
-  /*ShouldUseInMemoryCDB=*/CompileArgsFrom == LSPCompileArgs, Opts);
+  ClangdLSPServer LSPServer(Out, CCOpts, CompileCommandsDirPath,
+/*ShouldUseInMemoryCDB=*/CompileArgsFrom ==
+LSPCompileArgs,
+AutoIndex, Opts);
   constexpr int NoShutdownRequestErrorCode = 1;
   llvm::set_thread_name("clangd.main");
   // Change stdin to binary to not lose \r\n on windows.
Index: clangd/index/Background.h
===
--- /dev/null
+++ clangd/index/Background.h
@@ -0,0 +1,79 @@
+//===--- Background.h - Build

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/ClangdLSPServer.h:117
+llvm::Optional CompileCommandsDir,
+std::function);

ioeric wrote:
> please document what the callback is for and how often it's called.
Documented at the callsite and in GlobalCompilationDatabase.h.
This class here is really just plumbing (it shouldn't be in the header at all)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 169401.
lebedev.ri marked 2 inline comments as done.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

Let's try to wrap this up.

- Dropped HICPP alias. I/we really don't understand what those guidelines 
require.
- Don't touch unions. https://github.com/isocpp/CppCoreGuidelines/issues/1281
- Deduplicate check lines, yay.
- Some code cleanups.

FIXME: `IgnoreClassesWithAllMemberVariablesBeingPublic` needs to be somehow 
enabled for cppcoreguidelines check.
I don't know if it is possible, and how.

Anything i missed?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
  clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
  docs/ReleaseNotes.rst
  
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
  test/clang-tidy/misc-non-private-member-variables-in-classes.cpp

Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,373 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+  int S0_v0;
+
+public:
+  int S0_v1;
+
+protected:
+  int S0_v2;
+
+private:
+  int S0_v3;
+};
+
+class S1 {
+  int S1_v0;
+
+public:
+  int S1_v1;
+
+protected:
+  int S1_v2;
+
+private:
+  int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+  static void S2_m0();
+  int S2_v0;
+
+public:
+  static void S2_m1();
+  int S2_v1;
+
+protected:
+  static void S2_m3();
+  int S2_v2;
+
+private:
+  static void S2_m4();
+  int S2_v3;
+};
+
+class S3 {
+  static void S3_m0();
+  int S3_v0;
+
+public:
+  static void S3_m1();
+  int S3_v1;
+
+protected:
+  static void S3_m3();
+  int S3_v2;
+
+private:
+  static void S3_m4();
+  int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+  void U0_m0();
+  int U0_v0;
+
+public:
+  void U0_m1();
+  int U0_v1;
+
+protected:
+  void U0_m2();
+  int U0_v2;
+
+private:
+  void U0_m3();
+  int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+  void S4_m0();
+
+  int S4_v0;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' of struct 'S4' has public visibility
+public:
+  int S4_v1;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' of struct 'S4' has public visibility
+protected:
+  int S4_v2;
+  // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' of struct 'S4' has protected visibility
+private:
+  int S4_v3;
+};
+
+class S5 {
+  void S5_m0();
+
+  int S5_v0;
+
+public:
+  int S5_v1;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' of class 'S5' has public visibility
+protected:
+  int S5_v2;
+  // CHECK-MESSAGES-PROTECTED:

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:56
+  auto RecordIsInteresting =
+  allOf(anyOf(isStruct(), isClass()), hasMethods(), hasNonStaticMethod(),
+IgnoreClassesWithAllMemberVariablesBeingPublic

aaron.ballman wrote:
> Neither the C++ Core Guidelines nor HIC++ carve out an exception for union 
> types. I think you should talk to the guideline authors to see what they 
> think. Perhaps first try to diagnose unions the same as structs and classes 
> and see how the results look to you -- if they look bad, you can incorporate 
> those as examples when discussing with the authors.
As discussed, let's just drop HICPP alias,
and CPPCG only talks about struct/class, not union.
https://github.com/isocpp/CppCoreGuidelines/issues/1281



Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:82
+   "member variable '%0' of %1 '%2' has %3 visibility")
+  << Field->getName() << Record->getKindName() << Record->getName()
+  << Field->getAccess();

aaron.ballman wrote:
> Drop the single quotes above and just pass in `Field` and `Record` directly 
> -- the diagnostic printer will do the right thing.
Nice, but that does not seem to dump `getKindName()`.



Comment at: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp:1
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: 
[{key: 
misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, 
value: 0}, {key: 
misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic,
 value: 0}]}' --

zinovy.nis wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > JonasToth wrote:
> > > > I would prefer multiple test-files instead of mixing them all together.
> > > Hmm, no. Then the tests will have to be duplicated in N files,
> > > because i really do want to see what each of these 4 configurations do on 
> > > the *same* input.
> > > 
> > > It only looks ugly because the script only supports `-check-suffix=`,
> > > and not `-check-suffixes=`, which would significantly cut down on 
> > > check-lines.
> > Ok, if you prefer this.
> Not an issue anymore. You can use `-check-suffixes` since now.
Yay, look at this beauty :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 169402.
JonasToth marked 3 inline comments as done.
JonasToth added a comment.

- add tests and adjust doc
- one more test case


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+#define TEST_VARIADIC2(x, ...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+//
+#define problematic_variadic2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define OKISH_CONSTANT 42
+#define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define OKISH_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -88,6 +88,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcor

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Updated




Comment at: clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp:78
+"function-like macro used; consider a 'constexpr' template function";
+  if (Info->isVariadic())
+DiagnosticMessage = "variadic macro used; consider using a 'constexpr' "

aaron.ballman wrote:
> Aren't all vararg macros also function-like macros, so this will trigger two 
> diagnostics for all variadic macro definitions?
`MacroInfo` contains bits about function-like and beeing variadic separatly 
(gnu variadic as well, but thats included in `isVariadic`). There are no double 
warnings created.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

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

- remove unused enum in header file, no idea what i intended to do with it :D


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+#define TEST_VARIADIC2(x, ...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+//
+#define problematic_variadic2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define OKISH_CONSTANT 42
+#define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define OKISH_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -88,6 +88,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-ow

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[Marcus Hultman via Phabricator via cfe-commits]

hultman added reviewers: benhamilton, jolesiak, klimek, Wizard.
hultman added a comment.

This bug was introduced in revision 333553, authored by benhamilton, reviewed 
by jolesiak, klimek.


Repository:
  rC Clang

https://reviews.llvm.org/D53197



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

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 169405.
lebedev.ri added a comment.

Dead code elimination.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp
  clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.h
  docs/ReleaseNotes.rst
  
docs/clang-tidy/checks/cppcoreguidelines-non-private-member-variables-in-classes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-non-private-member-variables-in-classes.rst
  test/clang-tidy/misc-non-private-member-variables-in-classes.cpp

Index: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-non-private-member-variables-in-classes.cpp
@@ -0,0 +1,373 @@
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,NONPRIVATE,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PUBLIC,PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 0}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 0}]}' --
+// RUN: %check_clang_tidy -check-suffixes=PROTECTED %s misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: [{key: misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, value: 1}, {key: misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic, value: 1}]}' --
+
+////
+
+// Only data, do not warn
+
+struct S0 {
+  int S0_v0;
+
+public:
+  int S0_v1;
+
+protected:
+  int S0_v2;
+
+private:
+  int S0_v3;
+};
+
+class S1 {
+  int S1_v0;
+
+public:
+  int S1_v1;
+
+protected:
+  int S1_v2;
+
+private:
+  int S1_v3;
+};
+
+////
+
+// All functions are static, do not warn.
+
+struct S2 {
+  static void S2_m0();
+  int S2_v0;
+
+public:
+  static void S2_m1();
+  int S2_v1;
+
+protected:
+  static void S2_m3();
+  int S2_v2;
+
+private:
+  static void S2_m4();
+  int S2_v3;
+};
+
+class S3 {
+  static void S3_m0();
+  int S3_v0;
+
+public:
+  static void S3_m1();
+  int S3_v1;
+
+protected:
+  static void S3_m3();
+  int S3_v2;
+
+private:
+  static void S3_m4();
+  int S3_v3;
+};
+
+////
+
+// union != struct/class. do not diagnose.
+
+union U0 {
+  void U0_m0();
+  int U0_v0;
+
+public:
+  void U0_m1();
+  int U0_v1;
+
+protected:
+  void U0_m2();
+  int U0_v2;
+
+private:
+  void U0_m3();
+  int U0_v3;
+};
+
+////
+
+// Has non-static method with default visibility.
+
+struct S4 {
+  void S4_m0();
+
+  int S4_v0;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v0' of struct 'S4' has public visibility
+public:
+  int S4_v1;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S4_v1' of struct 'S4' has public visibility
+protected:
+  int S4_v2;
+  // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S4_v2' of struct 'S4' has protected visibility
+private:
+  int S4_v3;
+};
+
+class S5 {
+  void S5_m0();
+
+  int S5_v0;
+
+public:
+  int S5_v1;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S5_v1' of class 'S5' has public visibility
+protected:
+  int S5_v2;
+  // CHECK-MESSAGES-PROTECTED: :[[@LINE-1]]:7: warning: member variable 'S5_v2' of class 'S5' has protected visibility
+private:
+  int S5_v3;
+};
+
+////
+
+// Has non-static method with public visibility.
+
+struct S6 {
+  int S6_v0;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning: member variable 'S6_v0' of struct 'S6' has public visibility
+public:
+  void S6_m0();
+  int S6_v1;
+  // CHECK-MESSAGES-PUBLIC: :[[@LINE-1]]:7: warning:

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: test/clang-tidy/misc-non-private-member-variables-in-classes.cpp:1
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t
+// RUN: %check_clang_tidy -check-suffix=NONPRIVATE %s 
misc-non-private-member-variables-in-classes %t -- -config='{CheckOptions: 
[{key: 
misc-non-private-member-variables-in-classes.IgnorePublicMemberVariables, 
value: 0}, {key: 
misc-non-private-member-variables-in-classes.IgnoreClassesWithAllMemberVariablesBeingPublic,
 value: 0}]}' --

lebedev.ri wrote:
> zinovy.nis wrote:
> > JonasToth wrote:
> > > lebedev.ri wrote:
> > > > JonasToth wrote:
> > > > > I would prefer multiple test-files instead of mixing them all 
> > > > > together.
> > > > Hmm, no. Then the tests will have to be duplicated in N files,
> > > > because i really do want to see what each of these 4 configurations do 
> > > > on the *same* input.
> > > > 
> > > > It only looks ugly because the script only supports `-check-suffix=`,
> > > > and not `-check-suffixes=`, which would significantly cut down on 
> > > > check-lines.
> > > Ok, if you prefer this.
> > Not an issue anymore. You can use `-check-suffixes` since now.
> Yay, look at this beauty :)
[[ 
https://i.pinimg.com/736x/ef/16/be/ef16bea67542d3628e95a307114d2be8--gaslighting-trump-tower.jpg
 | ;) ]]


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52771



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

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[Ben Hamilton via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D53197



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

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

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

xbolva00 updated this revision to Diff 169407.
xbolva00 added a comment.

- check for overflow when evaluating


https://reviews.llvm.org/D52750

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/integer-overflow.c


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 
'int'}}
   (void)f2(0, f0(4608 * 1024 * 1024));
 }
+void check_integer_overflows_in_array_size() {
+  int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; 
result is 536870912 with type 'int'}}
+}
 
 struct s {
   unsigned x;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14052,8 +14052,9 @@
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
-if (Result)
-  *Result = E->EvaluateKnownConstInt(Context);
+if (Result) {
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+}
 return E;
   }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10866,6 +10866,19 @@
   return EvalResult.Val.getInt();
 }
 
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+  EvalResult EvalResult;
+  EvalResult.Diag = Diag;
+  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  (void)Result;
+  assert(Result && "Could not evaluate expression");
+  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+  return EvalResult.Val.getInt();
+}
+
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -631,9 +631,14 @@
   /// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
   /// integer. This must be called on an expression that constant folds to an
   /// integer.
-  llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
-SmallVectorImpl *Diag = nullptr) 
const;
+  llvm::APSInt EvaluateKnownConstInt(
+  const ASTContext &Ctx,
+  SmallVectorImpl *Diag = nullptr) const;
 
+  llvm::APSInt EvaluateKnownConstIntCheckOverflow(
+  const ASTContext &Ctx,
+  SmallVectorImpl *Diag = nullptr) const;
+
   void EvaluateForOverflow(const ASTContext &Ctx) const;
 
   /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   (void)f2(0, f0(4608 * 1024 * 1024));
 }
+void check_integer_overflows_in_array_size() {
+  int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; result is 536870912 with type 'int'}}
+}
 
 struct s {
   unsigned x;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14052,8 +14052,9 @@
   // Circumvent ICE checking in C++11 to avoid evaluating the expression twice
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
-if (Result)
-  *Result = E->EvaluateKnownConstInt(Context);
+if (Result) {
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+}
 return E;
   }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10866,6 +10866,19 @@
   return EvalResult.Val.getInt();
 }
 
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+  EvalResult EvalResult;
+  EvalResult.Diag = Diag;
+  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  (void)Result;
+  assert(Result && "Could not evaluate expression");
+  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+  return EvalResult.Val.getInt();
+}
+
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Eric Liu via Phabricator via cfe-commits]

ioeric accepted this revision.
ioeric added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/index/Background.cpp:51
+  std::unique_lock Lock(QueueMu);
+  assert(!HasActiveTask);
+  QueueCV.wait(Lock, [&] { return ShouldStop || !Queue.empty(); });

Maybe generalize this to `NumActiveTasks`? Currently, this is single-threaded 
and safe, but it could be missed when we add more threads.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;

Also add a test for `enqueueAll` with multiple TUs ?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

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

xbolva00 updated this revision to Diff 169408.

https://reviews.llvm.org/D52750

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/integer-overflow.c


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 
'int'}}
   (void)f2(0, f0(4608 * 1024 * 1024));
 }
+void check_integer_overflows_in_array_size() {
+  int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; 
result is 536870912 with type 'int'}}
+}
 
 struct s {
   unsigned x;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14053,7 +14053,8 @@
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
 if (Result)
-  *Result = E->EvaluateKnownConstInt(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+
 return E;
   }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10866,6 +10866,19 @@
   return EvalResult.Val.getInt();
 }
 
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+  EvalResult EvalResult;
+  EvalResult.Diag = Diag;
+  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  (void)Result;
+  assert(Result && "Could not evaluate expression");
+  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+  return EvalResult.Val.getInt();
+}
+
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -631,9 +631,14 @@
   /// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
   /// integer. This must be called on an expression that constant folds to an
   /// integer.
-  llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
-SmallVectorImpl *Diag = nullptr) 
const;
+  llvm::APSInt EvaluateKnownConstInt(
+  const ASTContext &Ctx,
+  SmallVectorImpl *Diag = nullptr) const;
 
+  llvm::APSInt EvaluateKnownConstIntCheckOverflow(
+  const ASTContext &Ctx,
+  SmallVectorImpl *Diag = nullptr) const;
+
   void EvaluateForOverflow(const ASTContext &Ctx) const;
 
   /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   (void)f2(0, f0(4608 * 1024 * 1024));
 }
+void check_integer_overflows_in_array_size() {
+  int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; result is 536870912 with type 'int'}}
+}
 
 struct s {
   unsigned x;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14053,7 +14053,8 @@
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
 if (Result)
-  *Result = E->EvaluateKnownConstInt(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
+
 return E;
   }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10866,6 +10866,19 @@
   return EvalResult.Val.getInt();
 }
 
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+  EvalResult EvalResult;
+  EvalResult.Diag = Diag;
+  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  (void)Result;
+  assert(Result && "Could not evaluate expression");
+  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+  return EvalResult.Val.getInt();
+}
+
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -631,9 +631,14 @@
   /// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
   /// integer. This must be called on an expression that constant folds to an
   /// integer.
-  llvm::APSInt EvaluateKnownConstInt(const ASTContext

r344352 - Fix MSVC 2015 ambiguous symbol warning introduced by rL344337. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Fri Oct 12 08:16:25 2018
New Revision: 344352

URL: http://llvm.org/viewvc/llvm-project?rev=344352&view=rev
Log:
Fix MSVC 2015 ambiguous symbol warning introduced by rL344337. NFCI.

Modified:
cfe/trunk/lib/Driver/Compilation.cpp

Modified: cfe/trunk/lib/Driver/Compilation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Compilation.cpp?rev=344352&r1=344351&r2=344352&view=diff
==
--- cfe/trunk/lib/Driver/Compilation.cpp (original)
+++ cfe/trunk/lib/Driver/Compilation.cpp Fri Oct 12 08:16:25 2018
@@ -127,7 +127,7 @@ bool Compilation::CleanupFile(const char
   return true;
 }
 
-bool Compilation::CleanupFileList(const ArgStringList &Files,
+bool Compilation::CleanupFileList(const llvm::opt::ArgStringList &Files,
   bool IssueErrors) const {
   bool Success = true;
   for (const auto &File: Files)


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

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

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

xbolva00 updated this revision to Diff 169409.
xbolva00 added a comment.

- Undo extra newline


https://reviews.llvm.org/D52750

Files:
  include/clang/AST/Expr.h
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaExpr.cpp
  test/Sema/integer-overflow.c


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 
'int'}}
   (void)f2(0, f0(4608 * 1024 * 1024));
 }
+void check_integer_overflows_in_array_size() {
+  int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; 
result is 536870912 with type 'int'}}
+}
 
 struct s {
   unsigned x;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14053,7 +14053,7 @@
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
 if (Result)
-  *Result = E->EvaluateKnownConstInt(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
 return E;
   }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10866,6 +10866,19 @@
   return EvalResult.Val.getInt();
 }
 
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+  EvalResult EvalResult;
+  EvalResult.Diag = Diag;
+  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  (void)Result;
+  assert(Result && "Could not evaluate expression");
+  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+  return EvalResult.Val.getInt();
+}
+
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -631,9 +631,14 @@
   /// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
   /// integer. This must be called on an expression that constant folds to an
   /// integer.
-  llvm::APSInt EvaluateKnownConstInt(const ASTContext &Ctx,
-SmallVectorImpl *Diag = nullptr) 
const;
+  llvm::APSInt EvaluateKnownConstInt(
+  const ASTContext &Ctx,
+  SmallVectorImpl *Diag = nullptr) const;
 
+  llvm::APSInt EvaluateKnownConstIntCheckOverflow(
+  const ASTContext &Ctx,
+  SmallVectorImpl *Diag = nullptr) const;
+
   void EvaluateForOverflow(const ASTContext &Ctx) const;
 
   /// EvaluateAsLValue - Evaluate an expression to see if we can fold it to an


Index: test/Sema/integer-overflow.c
===
--- test/Sema/integer-overflow.c
+++ test/Sema/integer-overflow.c
@@ -172,6 +172,9 @@
 // expected-warning@+1 {{overflow in expression; result is 536870912 with type 'int'}}
   (void)f2(0, f0(4608 * 1024 * 1024));
 }
+void check_integer_overflows_in_array_size() {
+  int arr[4608 * 1024 * 1024]; // expected-warning {{overflow in expression; result is 536870912 with type 'int'}}
+}
 
 struct s {
   unsigned x;
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14053,7 +14053,7 @@
   // in the non-ICE case.
   if (!getLangOpts().CPlusPlus11 && E->isIntegerConstantExpr(Context)) {
 if (Result)
-  *Result = E->EvaluateKnownConstInt(Context);
+  *Result = E->EvaluateKnownConstIntCheckOverflow(Context);
 return E;
   }
 
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10866,6 +10866,19 @@
   return EvalResult.Val.getInt();
 }
 
+APSInt Expr::EvaluateKnownConstIntCheckOverflow(
+const ASTContext &Ctx, SmallVectorImpl *Diag) const {
+  EvalResult EvalResult;
+  EvalResult.Diag = Diag;
+  EvalInfo Info(Ctx, EvalResult, EvalInfo::EM_EvaluateForOverflow);
+  bool Result = ::EvaluateAsRValue(Info, this, EvalResult.Val);
+  (void)Result;
+  assert(Result && "Could not evaluate expression");
+  assert(EvalResult.Val.isInt() && "Expression did not evaluate to integer");
+
+  return EvalResult.Val.getInt();
+}
+
 void Expr::EvaluateForOverflow(const ASTContext &Ctx) const {
   bool IsConst;
   EvalResult EvalResult;
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -631,9 +631,14 @@
   /// EvaluateKnownConstInt - Call EvaluateAsRValue and return the folded
   /// integer. This must be called on an expression that constant folds to an
   /// integer.
-  llvm::

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;

ioeric wrote:
> Also add a test for `enqueueAll` with multiple TUs ?
Is it important to call `enqueueAll` specifically vs `enqueue` multiple times?

We don't have a good test fixture for a compilation database, and `enqueueAll` 
is trivial...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D45050: [clang-tidy] New checker for not null-terminated result caused by strlen(), size() or equal length

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

@aaron.ballman Neither I nor @Charusso has commit rights, could you please 
commit this in our stead?


https://reviews.llvm.org/D45050



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

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;

sammccall wrote:
> ioeric wrote:
> > Also add a test for `enqueueAll` with multiple TUs ?
> Is it important to call `enqueueAll` specifically vs `enqueue` multiple times?
> 
> We don't have a good test fixture for a compilation database, and 
> `enqueueAll` is trivial...
I think the randomization code worths a test. 

How about adding a test in ClangdServer with the auto index enabled? I think 
we'd also want coverage in ClangdServer anyway.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[Marcus Hultman via Phabricator via cfe-commits]

hultman added a comment.

@benhamilton Could you land this patch?


Repository:
  rC Clang

https://reviews.llvm.org/D53197



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

[PATCH] D41648: [clang-tidy] implement cppcoreguidelines macro rules

[Jonas Toth via Phabricator via cfe-commits]

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

- make the logic with variadic clear


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D41648

Files:
  clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  docs/clang-tidy/checks/cppcoreguidelines-macro-usage.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
  test/clang-tidy/cppcoreguidelines-macro-usage.cpp

Index: test/clang-tidy/cppcoreguidelines-macro-usage.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-custom.cpp
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.AllowedRegexp, value: "DEBUG_*|TEST_*"}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define PROBLEMATIC_CONSTANT 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro used to declare a constant; consider using a 'constexpr' constant
+
+#define PROBLEMATIC_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: function-like macro used; consider a 'constexpr' template function
+
+#define PROBLEMATIC_VARIADIC(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define PROBLEMATIC_VARIADIC2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: variadic macro used; consider using a 'constexpr' variadic template function
+
+#define DEBUG_CONSTANT 0
+#define DEBUG_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define DEBUG_VARIADIC(...) (__VA_ARGS__)
+#define TEST_CONSTANT 0
+#define TEST_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define TEST_VARIADIC(...) (__VA_ARGS__)
+#define TEST_VARIADIC2(x, ...) (__VA_ARGS__)
+
+#endif
Index: test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
===
--- /dev/null
+++ test/clang-tidy/cppcoreguidelines-macro-usage-caps-only.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-macro-usage %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: cppcoreguidelines-macro-usage.CheckCapsOnly, value: 1}]}' --
+
+#ifndef INCLUDE_GUARD
+#define INCLUDE_GUARD
+
+#define problematic_constant 0
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_function(x, y) ((a) > (b) ? (a) : (b))
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define problematic_variadic(...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+//
+#define problematic_variadic2(x, ...) (__VA_ARGS__)
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: macro definition does not define the macro name using all uppercase characters
+
+#define OKISH_CONSTANT 42
+#define OKISH_FUNCTION(x, y) ((a) > (b) ? (a) : (b))
+#define OKISH_VARIADIC(...) (__VA_ARGS__)
+
+#endif
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -88,6 +88,7 @@
cppcoreguidelines-avoid-magic-numbers (redirects to readability-magic-numbers) 
cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) 
cppcoreguidelines-interfaces-global-init
+   cppcoreguidelines-macro-usage
cppcoreguidelines-narrowing-conversions
cppcoreguidelines-no-malloc
cppcoreguidelines-owning-memory
Index: docs/clang-tidy/checks

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added a comment.

This doesn't produce a warning in C++11 and up.


https://reviews.llvm.org/D52750



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

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 169411.

https://reviews.llvm.org/D53170

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -31,7 +31,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
-#include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/CommandLine.h"
@@ -88,6 +87,11 @@
 llvm::cl::desc("Use only doxygen-style comments to generate docs."),
 llvm::cl::init(false), llvm::cl::cat(ClangDocCategory));
 
+static llvm::cl::opt ClangDocExecutorName(
+"doc-executor",
+llvm::cl::desc("The name of the executor to use in clang-doc."),
+llvm::cl::init("all-TUs"));
+
 bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) {
   std::error_code OK;
   llvm::SmallString<128> DocsRootPath;
@@ -199,6 +203,7 @@
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
 
+  ExecutorName.setInitialValue("all-TUs");
   auto Exec = clang::tooling::createExecutorFromCommandLineArgs(
   argc, argv, ClangDocCategory);
 


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -31,7 +31,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
-#include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/CommandLine.h"
@@ -88,6 +87,11 @@
 llvm::cl::desc("Use only doxygen-style comments to generate docs."),
 llvm::cl::init(false), llvm::cl::cat(ClangDocCategory));
 
+static llvm::cl::opt ClangDocExecutorName(
+"doc-executor",
+llvm::cl::desc("The name of the executor to use in clang-doc."),
+llvm::cl::init("all-TUs"));
+
 bool CreateDirectory(const Twine &DirName, bool ClearDirectory = false) {
   std::error_code OK;
   llvm::SmallString<128> DocsRootPath;
@@ -199,6 +203,7 @@
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
 
+  ExecutorName.setInitialValue("all-TUs");
   auto Exec = clang::tooling::createExecutorFromCommandLineArgs(
   argc, argv, ClangDocCategory);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

[Julie Hockett via Phabricator via cfe-commits]

juliehockett marked 3 inline comments as done.
juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:203
+llvm::Expected>
+createClangDocExecutor(int &argc, const char **argv,
+   llvm::cl::OptionCategory &Category) {

ioeric wrote:
> This is exposing too much implementation details.I landed a patch to expose 
> the `ExecutorName` option from the library (rL344335) so you could simply do 
> `ExecutorName.setInitialValue("all-TUs")` before calling 
> `createExecutorFromCommandLineArgs()`.
Perfect. Thanks!


https://reviews.llvm.org/D53170



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

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

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

xbolva00 added a comment.

In https://reviews.llvm.org/D52750#1263461, @Rakete wrote:

> This doesn't produce a warning in C++11 and up.


But see Richard's comment: https://reviews.llvm.org/D52750#1251759


https://reviews.llvm.org/D52750



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

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:89
 llvm::cl::init(false), llvm::cl::cat(ClangDocCategory));
 
+static llvm::cl::opt ClangDocExecutorName(

This flag can also be removed. Users can still use `--executor` to override the 
default.


https://reviews.llvm.org/D53170



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

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added a comment.

In https://reviews.llvm.org/D52750#1263466, @xbolva00 wrote:

> In https://reviews.llvm.org/D52750#1263461, @Rakete wrote:
>
> > This doesn't produce a warning in C++11 and up.
>
>
> But see Richard's comment: https://reviews.llvm.org/D52750#125175 so I am not 
> sure :/


I guess I can see why it makes sense to suppress the warning in those cases. 
Sorry.


https://reviews.llvm.org/D52750



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

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;

ioeric wrote:
> sammccall wrote:
> > ioeric wrote:
> > > Also add a test for `enqueueAll` with multiple TUs ?
> > Is it important to call `enqueueAll` specifically vs `enqueue` multiple 
> > times?
> > 
> > We don't have a good test fixture for a compilation database, and 
> > `enqueueAll` is trivial...
> I think the randomization code worths a test. 
> 
> How about adding a test in ClangdServer with the auto index enabled? I think 
> we'd also want coverage in ClangdServer anyway.
How would you suggest testing the randomization :-)

The problem with a ClangdServer test is that it doesn't know anything about 
autoindex.
AutoIndex lives in ClangdLSPServer, because that's where the compilation 
database lives (because people keep cramming LSP extensions in to manipulate 
it, and I haven't been able to remove them).
Nobody has worked out how to test ClangdLSPServer yet. It's a worthy project, 
but...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

[PATCH] D53199: Fix the behavior of clang's -w flag.

[James Y Knight via Phabricator via cfe-commits]

jyknight created this revision.
jyknight added a reviewer: rsmith.

It is intended to disable _all_ warnings, even those upgraded to
errors via `-Werror=warningname` or `#pragma clang diagnostic error'


https://reviews.llvm.org/D53199

Files:
  clang/lib/Basic/DiagnosticIDs.cpp
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Frontend/warning-mapping-4.c
  clang/test/Frontend/warning-mapping-5.c
  clang/test/Frontend/warning-mapping-6.c


Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 // RUN: %clang_cc1 -verify -Wno-error=sign-compare %s
 // RUN: %clang_cc1 -verify -Wsign-compare -w -Wno-error=sign-compare %s
+// RUN: %clang_cc1 -verify -w -Werror=sign-compare %s
 // expected-no-diagnostics
 
 int f0(int x, unsigned y) {
Index: clang/test/Frontend/warning-mapping-2.c
===
--- clang/test/Frontend/warning-mapping-2.c
+++ clang/test/Frontend/warning-mapping-2.c
@@ -1,5 +1,7 @@
-// Check that -w has lower priority than -pedantic-errors.
+// Check that -w takes precedence over -pedantic-errors.
 // RUN: %clang_cc1 -verify -pedantic-errors -w %s
 
-void f0() { f1(); } // expected-error {{implicit declaration of function}}
+// Expect *not* to see a diagnostic for "implicit declaration of function"
+// expected-no-diagnostics
 
+void f0() { f1(); }
Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -457,12 +457,15 @@
   if (Result == diag::Severity::Ignored)
 return Result;
 
-  // Honor -w, which is lower in priority than pedantic-errors, but higher than
-  // -Werror.
-  // FIXME: Under GCC, this also suppresses warnings that have been mapped to
-  // errors by -W flags and #pragma diagnostic.
-  if (Result == diag::Severity::Warning && State->IgnoreAllWarnings)
-return diag::Severity::Ignored;
+  // Honor -w: this disables all messages mapped to Warning severity, and 
*also*
+  // any diagnostics which are not Error/Fatal by default (that is, they were
+  // upgraded by any of the mechanisms available: -Werror, -pedantic, or 
#pragma
+  // diagnostic)
+  if (State->IgnoreAllWarnings) {
+if (Result == diag::Severity::Warning ||
+!isDefaultMappingAsError((diag::kind)DiagID))
+  return diag::Severity::Ignored;
+  }
 
   // If -Werror is enabled, map warnings to errors unless explicitly disabled.
   if (Result == diag::Severity::Warning) {


Index: clang/test/Frontend/warning-mapping-6.c
===
--- /dev/null
+++ clang/test/Frontend/warning-mapping-6.c
@@ -0,0 +1,9 @@
+// Check that "#pragma diagnostic error" is suppressed by -w.
+//
+// RUN: %clang_cc1 -verify -Werror -w %s
+
+// expected-no-diagnostics
+#pragma gcc diagnostic error "-Wsign-compare"
+int f0(int x, unsigned y) {
+  return x < y;
+}
Index: clang/test/Frontend/warning-mapping-5.c
===
--- clang/test/Frontend/warning-mapping-5.c
+++ clang/test/Frontend/warning-mapping-5.c
@@ -1,6 +1,5 @@
-// Check that #pragma diagnostic warning overrides -Werror. This matches GCC's
-// original documentation, but not its earlier implementations.
-// 
+// Check that #pragma diagnostic warning overrides -Werror.
+//
 // RUN: %clang_cc1 -verify -Werror %s
 
 #pragma clang diagnostic warning "-Wsign-compare"
Index: clang/test/Frontend/warning-mapping-4.c
===
--- clang/test/Frontend/warning-mapping-4.c
+++ clang/test/Frontend/warning-mapping-4.c
@@ -1,5 +1,9 @@
+// Verify that various combinations of flags properly keep the sign-compare
+// warning disabled.
+
 

[PATCH] D52750: [Diagnostics] Check for integer overflow in array size expressions

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

xbolva00 added a comment.

That's fine :) btw, thanks for review comments!

Hopefully, this solution would be now acceptable :)


https://reviews.llvm.org/D52750



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

[PATCH] D53170: [clang-doc] Switch to default to all-TUs executor

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 169414.
juliehockett marked 3 inline comments as done.

https://reviews.llvm.org/D53170

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -31,7 +31,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
-#include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/CommandLine.h"
@@ -199,6 +198,7 @@
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
 
+  ExecutorName.setInitialValue("all-TUs");
   auto Exec = clang::tooling::createExecutorFromCommandLineArgs(
   argc, argv, ClangDocCategory);
 


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -31,7 +31,6 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
-#include "clang/Tooling/StandaloneExecution.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/Support/CommandLine.h"
@@ -199,6 +198,7 @@
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
 
+  ExecutorName.setInitialValue("all-TUs");
   auto Exec = clang::tooling::createExecutorFromCommandLineArgs(
   argc, argv, ClangDocCategory);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53153: [OpenCL] Mark namespace scope variables and kernel functions with default visibility

[Scott Linder via Phabricator via cfe-commits]

scott.linder updated this revision to Diff 169417.
scott.linder added a comment.

Address feedback


https://reviews.llvm.org/D53153

Files:
  lib/AST/Decl.cpp
  test/CodeGenOpenCL/visibility.cl


Index: test/CodeGenOpenCL/visibility.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/visibility.cl
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fvisibility default -triple amdgcn-unknown-unknown -S 
-emit-llvm -o - %s | FileCheck --check-prefix=FVIS-DEFAULT %s
+// RUN: %clang_cc1 -fvisibility protected -triple amdgcn-unknown-unknown -S 
-emit-llvm -o - %s | FileCheck --check-prefix=FVIS-PROTECTED %s
+// RUN: %clang_cc1 -fvisibility hidden -triple amdgcn-unknown-unknown -S 
-emit-llvm -o - %s | FileCheck --check-prefix=FVIS-HIDDEN %s
+
+// REQUIRES: amdgpu-registered-target
+
+// FVIS-DEFAULT: @globl = local_unnamed_addr
+// FVIS-PROTECTED: @globl = local_unnamed_addr
+// FVIS-HIDDEN: @globl = local_unnamed_addr
+__constant int globl = 0;
+// FVIS-DEFAULT: @default_globl = local_unnamed_addr
+// FVIS-PROTECTED: @default_globl = local_unnamed_addr
+// FVIS-HIDDEN: @default_globl = local_unnamed_addr
+__attribute__((visibility("default"))) __constant int default_globl = 0;
+// FVIS-DEFAULT: @protected_globl = protected local_unnamed_addr
+// FVIS-PROTECTED: @protected_globl = protected local_unnamed_addr
+// FVIS-HIDDEN: @protected_globl = protected local_unnamed_addr
+__attribute__((visibility("protected"))) __constant int protected_globl = 0;
+// FVIS-DEFAULT: @hidden_globl = hidden local_unnamed_addr
+// FVIS-PROTECTED: @hidden_globl = hidden local_unnamed_addr
+// FVIS-HIDDEN: @hidden_globl = hidden local_unnamed_addr
+__attribute__((visibility("hidden"))) __constant int hidden_globl = 0;
+
+// FVIS-DEFAULT: define amdgpu_kernel void @kern()
+// FVIS-PROTECTED: define amdgpu_kernel void @kern()
+// FVIS-HIDDEN: define amdgpu_kernel void @kern()
+kernel void kern() {}
+// FVIS-DEFAULT: define amdgpu_kernel void @default_kern()
+// FVIS-PROTECTED: define amdgpu_kernel void @default_kern()
+// FVIS-HIDDEN: define amdgpu_kernel void @default_kern()
+__attribute__((visibility("default"))) kernel void default_kern() {}
+// FVIS-DEFAULT: define protected amdgpu_kernel void @protected_kern()
+// FVIS-PROTECTED: define protected amdgpu_kernel void @protected_kern()
+// FVIS-HIDDEN: define protected amdgpu_kernel void @protected_kern()
+__attribute__((visibility("protected"))) kernel void protected_kern() {}
+// FVIS-DEFAULT: define hidden amdgpu_kernel void @hidden_kern()
+// FVIS-PROTECTED: define hidden amdgpu_kernel void @hidden_kern()
+// FVIS-HIDDEN: define hidden amdgpu_kernel void @hidden_kern()
+__attribute__((visibility("hidden"))) kernel void hidden_kern() {}
+
+// FVIS-DEFAULT: define void @func()
+// FVIS-PROTECTED: define protected void @func()
+// FVIS-HIDDEN: define hidden void @func()
+void func() {}
+// FVIS-DEFAULT: define void @default_func()
+// FVIS-PROTECTED: define void @default_func()
+// FVIS-HIDDEN: define void @default_func()
+__attribute__((visibility("default"))) void default_func() {}
+// FVIS-DEFAULT: define protected void @protected_func()
+// FVIS-PROTECTED: define protected void @protected_func()
+// FVIS-HIDDEN: define protected void @protected_func()
+__attribute__((visibility("protected"))) void protected_func() {}
+// FVIS-DEFAULT: define hidden void @hidden_func()
+// FVIS-PROTECTED: define hidden void @hidden_func()
+// FVIS-HIDDEN: define hidden void @hidden_func()
+__attribute__((visibility("hidden"))) void hidden_func() {}
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -556,6 +556,15 @@
 FD->hasBody(Def) && Def->isInlined() && !Def->hasAttr();
 }
 
+static bool useOpenCLVisibilityDefault(const NamedDecl *D) {
+  const LangOptions &Opts = D->getASTContext().getLangOpts();
+  if (!Opts.OpenCL)
+return false;
+  if (const auto *FD = dyn_cast(D))
+return FD->hasAttr();
+  return isa(D);
+}
+
 template  static bool isFirstInExternCContext(T *D) {
   const T *First = D->getFirstDecl();
   return First->isInExternCContext();
@@ -713,6 +722,9 @@
   }
 }
 
+if (!LV.isVisibilityExplicit() && useOpenCLVisibilityDefault(D))
+  LV.mergeVisibility(DefaultVisibility, true);
+
 // Add in global settings if the above didn't give us direct visibility.
 if (!LV.isVisibilityExplicit()) {
   // Use global type/value visibility as appropriate.


Index: test/CodeGenOpenCL/visibility.cl
===
--- /dev/null
+++ test/CodeGenOpenCL/visibility.cl
@@ -0,0 +1,56 @@
+// RUN: %clang_cc1 -fvisibility default -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | FileCheck --check-prefix=FVIS-DEFAULT %s
+// RUN: %clang_cc1 -fvisibility protected -triple amdgcn-unknown-unknown -S -emit-llvm -o - %s | FileCheck --check-prefix=FVIS-PROTECTED %s
+// RUN: %clang_cc1 -fvisibili

[PATCH] D53200: [OpenCL] Fix serialization of OpenCLExtensionDecls

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov created this revision.
AlexeySachkov added reviewers: Anastasia, yaxunl.

I recently discovered that adding the following code into `opencl-c.h` causes
failure of `test/Headers/opencl-c-header.cl`:

  #pragma OPENCL EXTENSION cl_my_ext : begin
  void cl_my_ext_foobarbaz();
  #pragma OPENCL EXTENSIOn cl_my_ext : end

Clang crashes at the assertion is `ASTReader::getGlobalSubmoduleID()`:

  assert(I != M.SubmoduleRemap.end() && "Invalid index into submodule index 
remap");

The root cause of the problem that to deserialize `OPENCL_EXTENSION_DECLS`
section `ASTReader` needs to deserialize a Decl contained in it. In turn,
deserializing a Decl requires information about whether this declaration is
part of a (sub)module, but this information is not read yet because it is
located further in a module file.


Repository:
  rC Clang

https://reviews.llvm.org/D53200

Files:
  lib/Serialization/ASTWriter.cpp


Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5014,13 +5014,16 @@
   WriteFPPragmaOptions(SemaRef.getFPOptions());
   WriteOpenCLExtensions(SemaRef);
   WriteOpenCLExtensionTypes(SemaRef);
-  WriteOpenCLExtensionDecls(SemaRef);
   WriteCUDAPragmas(SemaRef);
 
   // If we're emitting a module, write out the submodule information.
   if (WritingModule)
 WriteSubmodules(WritingModule);
 
+  // We need to have information about submodules to correctly deserialize
+  // decls from OpenCLExtensionDecls block
+  WriteOpenCLExtensionDecls(SemaRef);
+
   Stream.EmitRecord(SPECIAL_TYPES, SpecialTypes);
 
   // Write the record containing external, unnamed definitions.


Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -5014,13 +5014,16 @@
   WriteFPPragmaOptions(SemaRef.getFPOptions());
   WriteOpenCLExtensions(SemaRef);
   WriteOpenCLExtensionTypes(SemaRef);
-  WriteOpenCLExtensionDecls(SemaRef);
   WriteCUDAPragmas(SemaRef);
 
   // If we're emitting a module, write out the submodule information.
   if (WritingModule)
 WriteSubmodules(WritingModule);
 
+  // We need to have information about submodules to correctly deserialize
+  // decls from OpenCLExtensionDecls block
+  WriteOpenCLExtensionDecls(SemaRef);
+
   Stream.EmitRecord(SPECIAL_TYPES, SpecialTypes);
 
   // Write the record containing external, unnamed definitions.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53032: [clangd] Minimal implementation of automatic static index, behind a flag.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: unittests/clangd/BackgroundIndexTests.cpp:14
+
+TEST(BackgroundIndexTest, IndexesOneFile) {
+  MockFSProvider FS;

sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > ioeric wrote:
> > > > Also add a test for `enqueueAll` with multiple TUs ?
> > > Is it important to call `enqueueAll` specifically vs `enqueue` multiple 
> > > times?
> > > 
> > > We don't have a good test fixture for a compilation database, and 
> > > `enqueueAll` is trivial...
> > I think the randomization code worths a test. 
> > 
> > How about adding a test in ClangdServer with the auto index enabled? I 
> > think we'd also want coverage in ClangdServer anyway.
> How would you suggest testing the randomization :-)
> 
> The problem with a ClangdServer test is that it doesn't know anything about 
> autoindex.
> AutoIndex lives in ClangdLSPServer, because that's where the compilation 
> database lives (because people keep cramming LSP extensions in to manipulate 
> it, and I haven't been able to remove them).
> Nobody has worked out how to test ClangdLSPServer yet. It's a worthy project, 
> but...
> How would you suggest testing the randomization :-)
Not suggesting testing the behavior; just test that it works really. I guess a 
single file would also do it.

> The problem with a ClangdServer test is that it doesn't know anything about 
> autoindex.
Ah, I see. That's a bit unfortunate. ClangdServer seems to be a better place 
for the auto index. I wonder if we could move AutoIndex into ClangdServer? I'm 
not very familiar with CDB APIs, but intuitively this seems doable if we tweak 
the interface of CDB a bit e.g. inject the callback into 
`GlobalCompilationDatabase` without going through `CompilationDB`? Not sure if 
how well this would play with the CDB design though.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53032



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

r344356 - [OPENMP][NVPTX]Reduce memory usage in orphaned functions.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Oct 12 09:04:20 2018
New Revision: 344356

URL: http://llvm.org/viewvc/llvm-project?rev=344356&view=rev
Log:
[OPENMP][NVPTX]Reduce memory usage in orphaned functions.

if the function has globalized variables and called in context of
target/teams/distribute regions, it does not need to globalize 32
copies of the same variables for memory coalescing, it is enough to
have just one copy, because there is parallel region.
Patch does this by adding call for `__kmpc_parallel_level` function and
checking its return value. If the code sees that the parallel level is
0, then only one variable is allocated, not 32.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
cfe/trunk/test/OpenMP/nvptx_target_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=344356&r1=344355&r2=344356&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Fri Oct 12 09:04:20 2018
@@ -1972,6 +1972,7 @@ void CGOpenMPRuntimeNVPTX::emitGenericVa
 return;
   if (const RecordDecl *GlobalizedVarsRecord = I->getSecond().GlobalRecord) {
 QualType GlobalRecTy = 
CGM.getContext().getRecordType(GlobalizedVarsRecord);
+QualType SecGlobalRecTy;
 
 // Recover pointer to this function's global record. The runtime will
 // handle the specifics of the allocation of the memory.
@@ -1986,11 +1987,20 @@ void CGOpenMPRuntimeNVPTX::emitGenericVa
 llvm::PointerType *GlobalRecPtrTy =
 CGF.ConvertTypeForMem(GlobalRecTy)->getPointerTo();
 llvm::Value *GlobalRecCastAddr;
+llvm::Value *IsTTD = nullptr;
 if (WithSPMDCheck ||
 getExecutionMode() == CGOpenMPRuntimeNVPTX::EM_Unknown) {
   llvm::BasicBlock *ExitBB = CGF.createBasicBlock(".exit");
   llvm::BasicBlock *SPMDBB = CGF.createBasicBlock(".spmd");
   llvm::BasicBlock *NonSPMDBB = CGF.createBasicBlock(".non-spmd");
+  if (I->getSecond().SecondaryGlobalRecord.hasValue()) {
+llvm::Value *RTLoc = emitUpdateLocation(CGF, Loc);
+llvm::Value *ThreadID = getThreadID(CGF, Loc);
+llvm::Value *PL = CGF.EmitRuntimeCall(
+createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_parallel_level),
+{RTLoc, ThreadID});
+IsTTD = Bld.CreateIsNull(PL);
+  }
   llvm::Value *IsSPMD = Bld.CreateIsNotNull(CGF.EmitNounwindRuntimeCall(
   createNVPTXRuntimeFunction(OMPRTL_NVPTX__kmpc_is_spmd_exec_mode)));
   Bld.CreateCondBr(IsSPMD, SPMDBB, NonSPMDBB);
@@ -2003,11 +2013,28 @@ void CGOpenMPRuntimeNVPTX::emitGenericVa
   // There is no need to emit line number for unconditional branch.
   (void)ApplyDebugLocation::CreateEmpty(CGF);
   CGF.EmitBlock(NonSPMDBB);
+  llvm::Value *Size = llvm::ConstantInt::get(CGM.SizeTy, GlobalRecordSize);
+  if (const RecordDecl *SecGlobalizedVarsRecord =
+  I->getSecond().SecondaryGlobalRecord.getValueOr(nullptr)) {
+SecGlobalRecTy =
+CGM.getContext().getRecordType(SecGlobalizedVarsRecord);
+
+// Recover pointer to this function's global record. The runtime will
+// handle the specifics of the allocation of the memory.
+// Use actual memory size of the record including the padding
+// for alignment purposes.
+unsigned Alignment =
+CGM.getContext().getTypeAlignInChars(SecGlobalRecTy).getQuantity();
+unsigned GlobalRecordSize =
+CGM.getContext().getTypeSizeInChars(SecGlobalRecTy).getQuantity();
+GlobalRecordSize = llvm::alignTo(GlobalRecordSize, Alignment);
+Size = Bld.CreateSelect(
+IsTTD, llvm::ConstantInt::get(CGM.SizeTy, GlobalRecordSize), Size);
+  }
   // TODO: allow the usage of shared memory to be controlled by
   // the user, for now, default to global.
   llvm::Value *GlobalRecordSizeArg[] = {
-  llvm::ConstantInt::get(CGM.SizeTy, GlobalRecordSize),
-  CGF.Builder.getInt16(/*UseSharedMemory=*/0)};
+  Size, CGF.Builder.getInt16(/*UseSharedMemory=*/0)};
   llvm::Value *GlobalRecValue =
   CGF.EmitRuntimeCall(createNVPTXRuntimeFunction(
   OMPRTL_NVPTX__kmpc_data_sharing_push_stack),
@@ -2042,6 +2069,17 @@ void CGOpenMPRuntimeNVPTX::emitGenericVa
 
 // Emit the "global alloca" which is a GEP from the global declaration
 // record using the pointer returned by the runtime.
+LValue SecBase;
+decltype(I->getSecond().LocalVarData)::const_iterator SecIt;
+if (IsTTD) {
+  SecIt = I->getSecond().SecondaryLocalVarData->begin();
+  llvm::PointerType *SecGlobalRecPtrTy =
+  CGF.ConvertTypeForMem(SecGlobalRecTy)->getPointerTo();
+  SecBase = CGF.MakeNaturalAlignPointeeAddrLV

[PATCH] D49244: Always search sysroot for GCC installs

[David Greene via Phabricator via cfe-commits]

greened added a comment.

Ping.  This is impacting our ability to get clang functioning.


Repository:
  rC Clang

https://reviews.llvm.org/D49244



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

r344357 - Make YAML quote forward slashes.

[Zachary Turner via cfe-commits]

Author: zturner
Date: Fri Oct 12 09:24:09 2018
New Revision: 344357

URL: http://llvm.org/viewvc/llvm-project?rev=344357&view=rev
Log:
Make YAML quote forward slashes.

Modified:
cfe/trunk/unittests/Tooling/DiagnosticsYamlTest.cpp
cfe/trunk/unittests/Tooling/ReplacementsYamlTest.cpp

Modified: cfe/trunk/unittests/Tooling/DiagnosticsYamlTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/DiagnosticsYamlTest.cpp?rev=344357&r1=344356&r2=344357&view=diff
==
--- cfe/trunk/unittests/Tooling/DiagnosticsYamlTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/DiagnosticsYamlTest.cpp Fri Oct 12 09:24:09 2018
@@ -58,30 +58,30 @@ TEST(DiagnosticsYamlTest, serializesDiag
   YAML << TUD;
 
   EXPECT_EQ("---\n"
-"MainSourceFile:  path/to/source.cpp\n"
+"MainSourceFile:  'path/to/source.cpp'\n"
 "Diagnostics: \n"
 "  - DiagnosticName:  'diagnostic#1\'\n"
 "Message: 'message #1'\n"
 "FileOffset:  55\n"
-"FilePath:path/to/source.cpp\n"
+"FilePath:'path/to/source.cpp'\n"
 "Replacements:\n"
-"  - FilePath:path/to/source.cpp\n"
+"  - FilePath:'path/to/source.cpp'\n"
 "Offset:  100\n"
 "Length:  12\n"
 "ReplacementText: 'replacement #1'\n"
 "  - DiagnosticName:  'diagnostic#2'\n"
 "Message: 'message #2'\n"
 "FileOffset:  60\n"
-"FilePath:path/to/header.h\n"
+"FilePath:'path/to/header.h'\n"
 "Replacements:\n"
-"  - FilePath:path/to/header.h\n"
+"  - FilePath:'path/to/header.h'\n"
 "Offset:  62\n"
 "Length:  2\n"
 "ReplacementText: 'replacement #2'\n"
 "  - DiagnosticName:  'diagnostic#3'\n"
 "Message: 'message #3'\n"
 "FileOffset:  72\n"
-"FilePath:path/to/source2.cpp\n"
+"FilePath:'path/to/source2.cpp'\n"
 "Replacements:\n"
 "...\n",
 YamlContentStream.str());

Modified: cfe/trunk/unittests/Tooling/ReplacementsYamlTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Tooling/ReplacementsYamlTest.cpp?rev=344357&r1=344356&r2=344357&view=diff
==
--- cfe/trunk/unittests/Tooling/ReplacementsYamlTest.cpp (original)
+++ cfe/trunk/unittests/Tooling/ReplacementsYamlTest.cpp Fri Oct 12 09:24:09 
2018
@@ -33,13 +33,13 @@ TEST(ReplacementsYamlTest, serializesRep
 
   // NOTE: If this test starts to fail for no obvious reason, check whitespace.
   ASSERT_STREQ("---\n"
-   "MainSourceFile:  /path/to/source.cpp\n"
+   "MainSourceFile:  '/path/to/source.cpp'\n"
"Replacements:\n" // Extra whitespace here!
-   "  - FilePath:/path/to/file1.h\n"
+   "  - FilePath:'/path/to/file1.h'\n"
"Offset:  232\n"
"Length:  56\n"
"ReplacementText: 'replacement #1'\n"
-   "  - FilePath:/path/to/file2.h\n"
+   "  - FilePath:'/path/to/file2.h'\n"
"Offset:  301\n"
"Length:  2\n"
"ReplacementText: 'replacement #2'\n"


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