[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 378495.
carlosgalvezp added a comment.

Refactored NOLINTBEGIN/END slightly. Now, the requirements for a match are 
stricter (and simpler), making the code easier to reason about: any 
NOLINTBEGIN(X) must be matched by an identical NOLINTEND(X), for any X.

Added additional tests.

@salman-javed-nz let me know what you think!


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

https://reviews.llvm.org/D111208

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
@@ -11,21 +11,25 @@
 class D { D(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
-// NOLINTNEXTLINE(*)
+// NOLINTNEXTLINE()
 class D1 { D1(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 
-// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+// NOLINTNEXTLINE(*)
 class D2 { D2(int i); };
 
-// NOLINTNEXTLINE(google-explicit-constructor)
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
 class D3 { D3(int i); };
 
-// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+// NOLINTNEXTLINE(google-explicit-constructor)
 class D4 { D4(int i); };
 
-// NOLINTNEXTLINE without-brackets-skip-all, another-check
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
 class D5 { D5(int i); };
 
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class D6 { D6(int i); };
+
 // NOLINTNEXTLINE
 
 class E { E(int i); };
@@ -46,6 +50,21 @@
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 9 warnings (9 NOLINT)
+// NOLINTNEXTLINE(google*)
+class I1 { I1(int i); };
+
+// NOLINTNEXTLINE(google*,-google*)
+class I2 { I2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+class I3 { I3(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:38: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+
+// NOLINTNEXTLINE(*,-google*)
+class I4 { I4(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// CHECK-MESSAGES: Suppressed 11 warnings (11 NOLINT)
 
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s google-explicit-constructor %t
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
@@ -34,60 +34,54 @@
 class C1 { C1(int i); };
 // NOLINTEND(google-explicit-constructor)
 
-// NOLINTBEGIN(*)
+// NOLINTBEGIN()
 class C2 { C2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINTEND()
+
+// NOLINTBEGIN(*)
+class C3 { C3(int i); };
 // NOLINTEND(*)
 
 // NOLINTBEGIN(some-other-check)
-class C3 { C3(int i); };
+class C4 { C4(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 // NOLINTEND(some-other-check)
 
 // NOLINTBEGIN(some-other-check, google-explicit-constructor)
-class C4 { C4(int i); };
-// NOLINTEND(some-other-check, google-explicit-constructor)
-
-// NOLINTBEGIN(some-other-check, google-explicit-constructor)
-// NOLINTEND(some-other-check)
 class C5 { C5(int i); };
-// NOLINTEND(google-explicit-constructor)
-
-// NOLINTBEGIN(some-other-check, google-explicit-constructor)
-// NOLINTEND(google-explicit-constructor)
-class C6 { C6(i

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:446
 SuppressionErrors.emplace_back(Error.getValue());
-return false;
   }

I had to remove these "return false", otherwise I would not get errors 
regarding "Found NOLINTBEGIN that was unmatched by a NOLINTEND". Not really 
sure why they were there in the first place.

All unit tests pass with this patch.


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

https://reviews.llvm.org/D111208

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

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

In D111208#3053660 , @carlosgalvezp 
wrote:

> Now, the requirements for a match are stricter (and simpler), making the code 
> easier to reason about: any NOLINTBEGIN(X) must be matched by an identical 
> NOLINTEND(X), for any X.

Thanks! Simplifying it right down looks to me like the right thing to do.

In case you find it hard to follow my suggested changes in Phabricator's 
interface, I have copied these suggestions into this attachment for convenience 
of reading: F19524409: changes.cpp 




Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:325
   StringRef Line, size_t *FoundNolintIndex = nullptr,
-  bool *SuppressionIsSpecific = nullptr) {
+  StringRef *FoundNolintBracket = nullptr) {
   if (FoundNolintIndex)

This feels like a more intuitive argument name to me, plus it aligns with the 
local variable `ChecksStr` below. What do you think?



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:343-357
   if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
+const size_t BracketStartIndex = BracketIndex;
 ++BracketIndex;
 const size_t BracketEndIndex = Line.find(')', BracketIndex);
 if (BracketEndIndex != StringRef::npos) {
+  if (FoundNolintBracket)
+*FoundNolintBracket = Line.substr(

I think this should work the same without the additional index variable and 
substring.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:344
   if (BracketIndex < Line.size() && Line[BracketIndex] == '(') {
+const size_t BracketStartIndex = BracketIndex;
 ++BracketIndex;

This is just a copy of `BracketIndex`. Would it be better to consolidate the 
two? 



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:348-353
+  if (FoundNolintBracket)
+*FoundNolintBracket = Line.substr(
+BracketStartIndex, BracketEndIndex - BracketStartIndex + 1);
+
   StringRef ChecksStr =
   Line.substr(BracketIndex, BracketEndIndex - BracketIndex);

It gets a bit hard to follow with `BracketIndex` and `BracketStartIndex` 
interspersed on these lines, and a `+ 1` in one substring but not in the other 
substring.
Also aren't both substrings pretty much the same thing?



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:397
   size_t NolintIndex;
-  bool SuppressionIsSpecific;
-  auto List = [&]() -> SmallVector * {
-return SuppressionIsSpecific ? &SpecificNolintBegins : &GlobalNolintBegins;
-  };
+  StringRef NolintBracket;
   for (const auto &Line : Lines) {

To align with the name `ChecksStr` used in the other function(s).



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:407-409
+  if (!NolintBegins.empty() &&
+  (NolintBegins.back().second == NolintBracket)) {
+NolintBegins.pop_back();

It's not necessarily the last element of `NolintBegins` that you need to 
`pop_back()`.

In the case where you have overlapping `NOLINTBEGIN` regions...
```
// NOLINTBEGIN(A)  <-- push A
// NOLINTBEGIN(B) <-- push B
// NOLINTEND(A) <-- pop A
// NOLINTEND(B) <-- pop B
```

Therefore you need to search through all of the elements, not just the last one.



Comment at: 
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp:57
 // NOLINTBEGIN(some-other-check)
-class C7 { C7(int i); };
+class C8 { C8(int i); };
 // NOLINTEND(some-other-check)

Since `C6` & `C7` have been removed you could renumber `C8` to `C6`. Once this 
feature is in master I don't see this test file changing again for a long time. 


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

https://reviews.llvm.org/D111208

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

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:446
 SuppressionErrors.emplace_back(Error.getValue());
-return false;
   }

carlosgalvezp wrote:
> I had to remove these "return false", otherwise I would not get errors 
> regarding "Found NOLINTBEGIN that was unmatched by a NOLINTEND". Not really 
> sure why they were there in the first place.
> 
> All unit tests pass with this patch.
The reason for the `return false` was because the file is already determined to 
have a unmatched `NOLINTBEGIN`, so it's FUBAR and probably not worth checking 
the remainder of anyway. I don't mind either way. 


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

https://reviews.llvm.org/D111208

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

[PATCH] D110684: [RISCV] Define _m intrinsics as builtins, instead of macros.

[Hsiangkai Wang via Phabricator via cfe-commits]

HsiangKai added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110684

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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 378501.
mizvekov added a comment.

.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/test/SemaCXX/sugared-auto.cpp

Index: clang/test/SemaCXX/sugared-auto.cpp
===
--- clang/test/SemaCXX/sugared-auto.cpp
+++ clang/test/SemaCXX/sugared-auto.cpp
@@ -1,31 +1,160 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++14
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++20 -fblocks -fenable-matrix -Wno-dynamic-exception-spec
 
 enum class N {};
 
-using T1 = int;
-auto x1 = T1();
-N t1 = x1;
-// expected-error@-1 {{cannot initialize a variable of type 'N' with an lvalue of type 'T1' (aka 'int')}}
-
-using T2 = T1 *;
-auto x2 = T2();
-N t2 = x2;
-// expected-error@-1 {{cannot initialize a variable of type 'N' with an lvalue of type 'T2' (aka 'int *')}}
-
-auto *x3 = T2();
-N t3 = x3;
-// expected-error@-1 {{cannot initialize a variable of type 'N' with an lvalue of type 'T1 *' (aka 'int *')}}
-
-auto f1() { return T1(); }
-auto x4 = f1();
-N t4 = x4;
-// expected-error@-1 {{cannot initialize a variable of type 'N' with an lvalue of type 'T1' (aka 'int')}}
-
-decltype(auto) f2() { return T1(); }
-auto x5 = f2();
-N t5 = x5;
-// expected-error@-1 {{cannot initialize a variable of type 'N' with an lvalue of type 'T1' (aka 'int')}}
-
-auto x6 = [a = T1()] { return a; }();
-N t6 = x6;
-// expected-error@-1 {{cannot initialize a variable of type 'N' with an lvalue of type 'T1' (aka 'int')}}
+using Animal = int;
+
+using AnimalPtr = Animal *;
+
+using Man = Animal;
+using Dog = Animal;
+
+using ManPtr = Man *;
+using DogPtr = Dog *;
+
+using SocratesPtr = ManPtr;
+
+using Virus = void;
+using SARS = Virus;
+using Ebola = Virus;
+
+using Bacteria = float;
+using Bacilli = Bacteria;
+using Vibrio = Bacteria;
+
+struct Plant;
+using Gymnosperm = Plant;
+using Angiosperm = Plant;
+
+namespace variable {
+
+auto x1 = Animal();
+N t1 = x1; // expected-error {{lvalue of type 'Animal' (aka 'int')}}
+
+auto x2 = AnimalPtr();
+N t2 = x2; // expected-error {{lvalue of type 'AnimalPtr' (aka 'int *')}}
+
+auto *x3 = AnimalPtr();
+N t3 = x3; // expected-error {{lvalue of type 'Animal *' (aka 'int *')}}
+
+// Each variable deduces separately.
+auto x4 = Man(), x5 = Dog();
+N t4 = x4; // expected-error {{lvalue of type 'Man' (aka 'int')}}
+N t5 = x5; // expected-error {{lvalue of type 'Dog' (aka 'int')}}
+
+} // namespace variable
+
+namespace function_basic {
+
+auto f1() { return Animal(); }
+auto x1 = f1();
+N t1 = x1; // expected-error {{lvalue of type 'Animal' (aka 'int')}}
+
+decltype(auto) f2() { return Animal(); }
+auto x2 = f2();
+N t2 = x2; // expected-error {{lvalue of type 'Animal' (aka 'int')}}
+
+auto x3 = [a = Animal()] { return a; }();
+N t3 = x3; // expected-error {{lvalue of type 'Animal' (aka 'int')}}
+
+} // namespace function_basic
+
+namespace function_multiple_basic {
+
+N t1 = [] { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Man();
+  return Dog();
+}();
+
+N t2 = []() -> decltype(auto) { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Man();
+  return Dog();
+}();
+
+N t3 = [] { // expected-error {{rvalue of type 'Animal' (aka 'int')}}
+  if (true)
+return Dog();
+  auto x = Man();
+  return x;
+}();
+
+N t4 = [] { // expected-error {{rvalue of type 'int'}}
+  if (true)
+return Dog();
+  return 1;
+}();
+
+N t5 = [] { // expected-error {{rvalue of type 'Virus' (aka 'void')}}
+  if (true)
+return Ebola();
+  return SARS();
+}();
+
+N t6 = [] { // expected-error {{rvalue of type 'void'}}
+  if (true)
+return SARS();
+  return;
+}();
+
+} // namespace function_multiple_basic
+
+#define TEST_AUTO(X, A, B) \
+  auto X(A a, B b) {   \
+if (0) \
+  return a;\
+if (0) \
+  return b;\
+return N();\
+  }
+#define TEST_DAUTO(X, A, B) \
+  decltype(auto) X(A a, B b) {  \
+if (0)  \
+  return static_cast(a); \
+if (0)  \
+  return static_cast(b); \
+return N(); \
+  }
+
+namespace misc {
+
+TEST_AUTO(t1, ManPtr, DogPtr)  // expected-error {{but deduced as 'Animal *' (aka 'int *')}}
+TEST_AUTO(t2, ManPtr, int *)   // expected-error {{but deduced as 'int *'}}
+TEST_AUTO(t3, SocratesPtr, ManPtr) // expected-error {{but deduced as 'ManPtr' (aka 'int *')}}
+
+TEST_AUTO(t4, _Atomic(Man), _Atomic(Dog)) // expected-error {{but deduced as '_Atomic(Animal)

[PATCH] D111490: [DOCS] Update ninja build doc (new: github link, build command, and chmod requirements)

[Samuel Marks via Phabricator via cfe-commits]

SamuelMarks added a comment.

In D111490#3053564 , @xgupta wrote:

> The patch is not applying cleanly so can't be committed. please follow the 
> above phabricator docs I suggested before to upload it.

Oh there was an error in the web interface which told me to retry. Will check 
the docs on how to delete|supersede diffs


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

https://reviews.llvm.org/D111490

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

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 378507.
carlosgalvezp marked 9 inline comments as done.
carlosgalvezp added a comment.

Fixed comments from @salman-javed-nz


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

https://reviews.llvm.org/D111208

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/infrastructure/nolint.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-global-end-specific.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-begin-specific-end-global.cpp
  
clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend-mismatched-check-names.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp

Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintnextline.cpp
@@ -11,21 +11,25 @@
 class D { D(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
 
-// NOLINTNEXTLINE(*)
+// NOLINTNEXTLINE()
 class D1 { D1(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 
-// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
+// NOLINTNEXTLINE(*)
 class D2 { D2(int i); };
 
-// NOLINTNEXTLINE(google-explicit-constructor)
+// NOLINTNEXTLINE(not-closed-bracket-is-treated-as-skip-all
 class D3 { D3(int i); };
 
-// NOLINTNEXTLINE(some-check, google-explicit-constructor)
+// NOLINTNEXTLINE(google-explicit-constructor)
 class D4 { D4(int i); };
 
-// NOLINTNEXTLINE without-brackets-skip-all, another-check
+// NOLINTNEXTLINE(some-check, google-explicit-constructor)
 class D5 { D5(int i); };
 
+// NOLINTNEXTLINE without-brackets-skip-all, another-check
+class D6 { D6(int i); };
+
 // NOLINTNEXTLINE
 
 class E { E(int i); };
@@ -46,6 +50,21 @@
 // NOLINTNEXTLINE
 MACRO_NOARG
 
-// CHECK-MESSAGES: Suppressed 9 warnings (9 NOLINT)
+// NOLINTNEXTLINE(google*)
+class I1 { I1(int i); };
+
+// NOLINTNEXTLINE(google*,-google*)
+class I2 { I2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+class I3 { I3(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// CHECK-MESSAGES: :[[@LINE-2]]:38: warning: unused variable 'i' [clang-diagnostic-unused-variable]
+
+// NOLINTNEXTLINE(*,-google*)
+class I4 { I4(int x); void f() { int i; } };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+
+// CHECK-MESSAGES: Suppressed 11 warnings (11 NOLINT)
 
-// RUN: %check_clang_tidy %s google-explicit-constructor %t --
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
Index: clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/nolintbeginend.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s google-explicit-constructor %t
+// RUN: %check_clang_tidy %s google-explicit-constructor,clang-diagnostic-unused-variable %t -- -extra-arg=-Wunused-variable
 
 class A { A(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
@@ -34,60 +34,54 @@
 class C1 { C1(int i); };
 // NOLINTEND(google-explicit-constructor)
 
-// NOLINTBEGIN(*)
+// NOLINTBEGIN()
 class C2 { C2(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
+// NOLINTEND()
+
+// NOLINTBEGIN(*)
+class C3 { C3(int i); };
 // NOLINTEND(*)
 
 // NOLINTBEGIN(some-other-check)
-class C3 { C3(int i); };
+class C4 { C4(int i); };
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
 // NOLINTEND(some-other-check)
 
 // NOLINTBEGIN(some-other-check, google-explicit-constructor)
-class C4 { C4(int i); };
-// NOLINTEND(some-other-check, google-explicit-constructor)
-
-// NOLINTBEGIN(some-other-check, google-explicit-constructor)
-// NOLINTEND(some-other-check)
 class C5 { C5(int i); };
-// NOLINTEND(google-explicit-constructor)
-
-// NOLINTBEGIN(some-other-check, google-explicit-constructor)
-// NOLINTEND(google-explicit-constructor)
-class C6 { C6(int i); };
-// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: single-argument constructors must be marked explicit
-// NOLINTEND(some-other-check)
+// NOLINTEND(some-other-check, google-explicit-constructor)
 

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:325
   StringRef Line, size_t *FoundNolintIndex = nullptr,
-  bool *SuppressionIsSpecific = nullptr) {
+  StringRef *FoundNolintBracket = nullptr) {
   if (FoundNolintIndex)

salman-javed-nz wrote:
> This feels like a more intuitive argument name to me, plus it aligns with the 
> local variable `ChecksStr` below. What do you think?
Sure. What about `FoundNolintChecksStr`, for consistency with 
`FoundNolintIndex`?



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:348-353
+  if (FoundNolintBracket)
+*FoundNolintBracket = Line.substr(
+BracketStartIndex, BracketEndIndex - BracketStartIndex + 1);
+
   StringRef ChecksStr =
   Line.substr(BracketIndex, BracketEndIndex - BracketIndex);

salman-javed-nz wrote:
> It gets a bit hard to follow with `BracketIndex` and `BracketStartIndex` 
> interspersed on these lines, and a `+ 1` in one substring but not in the 
> other substring.
> Also aren't both substrings pretty much the same thing?
I wanted "FoundCheckStr" to contain also the brackets, so that NOLINT( is 
different than NOLINT(). However I realize now that they are anyway treated 
differently (as per previous discussion) so I'll remove this and keep it simple.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:407-409
+  if (!NolintBegins.empty() &&
+  (NolintBegins.back().second == NolintBracket)) {
+NolintBegins.pop_back();

salman-javed-nz wrote:
> It's not necessarily the last element of `NolintBegins` that you need to 
> `pop_back()`.
> 
> In the case where you have overlapping `NOLINTBEGIN` regions...
> ```
> // NOLINTBEGIN(A)  <-- push A
> // NOLINTBEGIN(B) <-- push B
> // NOLINTEND(A) <-- pop A
> // NOLINTEND(B) <-- pop B
> ```
> 
> Therefore you need to search through all of the elements, not just the last 
> one.
Thanks, that's right. However that's a problem that already exists on master, 
right? It's pushing from the back anyway? We should probably have a unit test 
for this.

Would it make sense to leave that for a separate patch? Seems like the scope is 
growing and I'd like to keep the change as small as possible.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:446
 SuppressionErrors.emplace_back(Error.getValue());
-return false;
   }

salman-javed-nz wrote:
> carlosgalvezp wrote:
> > I had to remove these "return false", otherwise I would not get errors 
> > regarding "Found NOLINTBEGIN that was unmatched by a NOLINTEND". Not really 
> > sure why they were there in the first place.
> > 
> > All unit tests pass with this patch.
> The reason for the `return false` was because the file is already determined 
> to have a unmatched `NOLINTBEGIN`, so it's FUBAR and probably not worth 
> checking the remainder of anyway. I don't mind either way. 
I see. In the test case that I added, I got prints that `NOLINTEND` is 
unmatched by a `BEGIN`, but I didn't get a message that `NOLINTBEGIN` is 
unmatched by an `END`, which I did get in a separate unit test, so I thought 
that's intended behavior. 


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

https://reviews.llvm.org/D111208

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

[PATCH] D111041: [clang-tidy] Remove 'IgnoreDestructors = true' from cppcoreguidelines-explicit-virtual-functions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 378510.
carlosgalvezp added a comment.

Added documentation


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

https://reviews.llvm.org/D111041

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 Changes in existing checks
 ^^
 
+- Removed default setting 
`cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+  to match the current state of the C++ Core Guidelines.
+
+
 Removed checks
 ^^
 
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -106,9 +106,6 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "true";
 
-Opts["cppcoreguidelines-explicit-virtual-functions."
- "IgnoreDestructors"] = "true";
-
 return Options;
   }
 };


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 Changes in existing checks
 ^^
 
+- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+  to match the current state of the C++ Core Guidelines.
+
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -106,9 +106,6 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "true";
 
-Opts["cppcoreguidelines-explicit-virtual-functions."
- "IgnoreDestructors"] = "true";
-
 return Options;
   }
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111041: [clang-tidy] Remove 'IgnoreDestructors = true' from cppcoreguidelines-explicit-virtual-functions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Regarding test coverage, I'm unsure if what you propose is intended. I have 
looked at similar aliases and I can't find any test at all for them either. For 
example `modernize-avoid-c-arrays` is aliased by "cppcoreguidelines" and 
"hicpp" and I cannot find tests for them. So perhaps there's a reason for this?

I had a look at your example with `check_prefix` but I'm not sure it fits well 
here. In the case you mention, it's about running *the same check* with 
different flags. Here we are talking about running *different checks* (though 
aliased) in the same .cpp test file. If we want to treat aliases as "first 
class checks", I think it only makes sense to have a "first class test" also 
for the aliases. But that leads to code duplication, of course. Feels like this 
should be discussed in cfe-dev, but perhaps it's outside the scope of this 
patch? Right now there's really nothing new to test - the check is now a 
perfect alias, and the test strategy is consistent with other aliases.


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

https://reviews.llvm.org/D111041

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

[PATCH] D111041: [clang-tidy] Remove 'IgnoreDestructors = true' from cppcoreguidelines-explicit-virtual-functions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Oh, I found `misc-non-private-member-in-classes` that does what you proposed. 
I'll add another `RUN` line to run the `cppcoreguidelines` check.


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

https://reviews.llvm.org/D111041

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

[clang] 0e9373a - [Basic] Use llvm::is_contained (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2021-10-10T08:52:14-07:00
New Revision: 0e9373a6a638430c67fa8378c774003acfcf5990

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

LOG: [Basic] Use llvm::is_contained (NFC)

Added: 


Modified: 
clang/lib/Basic/Module.cpp
clang/lib/Basic/Targets/AMDGPU.h
clang/lib/Basic/Targets/AVR.cpp
clang/lib/Basic/Targets/BPF.cpp
clang/lib/Basic/Targets/Mips.cpp
clang/lib/Basic/Targets/PPC.cpp
clang/lib/Basic/Targets/Sparc.h
clang/lib/Basic/Targets/WebAssembly.cpp
clang/lib/Basic/Targets/X86.cpp

Removed: 




diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 4ec0699e40d49..09bd3251fea0c 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -121,9 +121,7 @@ static bool hasFeature(StringRef Feature, const LangOptions 
&LangOpts,
 .Default(Target.hasFeature(Feature) ||
  isPlatformEnvironment(Target, Feature));
   if (!HasFeature)
-HasFeature = std::find(LangOpts.ModuleFeatures.begin(),
-   LangOpts.ModuleFeatures.end(),
-   Feature) != LangOpts.ModuleFeatures.end();
+HasFeature = llvm::is_contained(LangOpts.ModuleFeatures, Feature);
   return HasFeature;
 }
 

diff  --git a/clang/lib/Basic/Targets/AMDGPU.h 
b/clang/lib/Basic/Targets/AMDGPU.h
index 700b76452eea2..8b9d7ce79c16f 100644
--- a/clang/lib/Basic/Targets/AMDGPU.h
+++ b/clang/lib/Basic/Targets/AMDGPU.h
@@ -440,7 +440,7 @@ class LLVM_LIBRARY_VISIBILITY AMDGPUTargetInfo final : 
public TargetInfo {
 WavefrontSize = 64;
   bool IsOn = F.front() == '+';
   StringRef Name = StringRef(F).drop_front();
-  if (llvm::find(TargetIDFeatures, Name) == TargetIDFeatures.end())
+  if (!llvm::is_contained(TargetIDFeatures, Name))
 return;
   assert(OffloadArchFeatures.find(Name) == OffloadArchFeatures.end());
   OffloadArchFeatures[Name] = IsOn;

diff  --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index 8fa19064d41f7..38ce61386bb4c 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -311,8 +311,7 @@ static constexpr llvm::StringLiteral ValidFamilyNames[] = {
 "avrxmega6", "avrxmega7", "avrtiny"};
 
 bool AVRTargetInfo::isValidCPUName(StringRef Name) const {
-  bool IsFamily =
-  llvm::find(ValidFamilyNames, Name) != std::end(ValidFamilyNames);
+  bool IsFamily = llvm::is_contained(ValidFamilyNames, Name);
 
   bool IsMCU =
   llvm::find_if(AVRMcus, [&](const MCUInfo &Info) {

diff  --git a/clang/lib/Basic/Targets/BPF.cpp b/clang/lib/Basic/Targets/BPF.cpp
index 0b0298df30a57..2dfe21564cc1a 100644
--- a/clang/lib/Basic/Targets/BPF.cpp
+++ b/clang/lib/Basic/Targets/BPF.cpp
@@ -35,7 +35,7 @@ static constexpr llvm::StringLiteral ValidCPUNames[] = 
{"generic", "v1", "v2",
 "v3", "probe"};
 
 bool BPFTargetInfo::isValidCPUName(StringRef Name) const {
-  return llvm::find(ValidCPUNames, Name) != std::end(ValidCPUNames);
+  return llvm::is_contained(ValidCPUNames, Name);
 }
 
 void BPFTargetInfo::fillValidCPUList(SmallVectorImpl &Values) const 
{

diff  --git a/clang/lib/Basic/Targets/Mips.cpp 
b/clang/lib/Basic/Targets/Mips.cpp
index 3a32fd492c6bb..39246f650cce2 100644
--- a/clang/lib/Basic/Targets/Mips.cpp
+++ b/clang/lib/Basic/Targets/Mips.cpp
@@ -50,7 +50,7 @@ static constexpr llvm::StringLiteral ValidCPUNames[] = {
 {"octeon"}, {"octeon+"}, {"p5600"}};
 
 bool MipsTargetInfo::isValidCPUName(StringRef Name) const {
-  return llvm::find(ValidCPUNames, Name) != std::end(ValidCPUNames);
+  return llvm::is_contained(ValidCPUNames, Name);
 }
 
 void MipsTargetInfo::fillValidCPUList(

diff  --git a/clang/lib/Basic/Targets/PPC.cpp b/clang/lib/Basic/Targets/PPC.cpp
index 26be2bd9c5d7b..da21e97885dec 100644
--- a/clang/lib/Basic/Targets/PPC.cpp
+++ b/clang/lib/Basic/Targets/PPC.cpp
@@ -437,11 +437,11 @@ static bool ppcUserFeaturesCheck(DiagnosticsEngine &Diags,
  const std::vector &FeaturesVec) {
 
   // vsx was not explicitly turned off.
-  if (llvm::find(FeaturesVec, "-vsx") == FeaturesVec.end())
+  if (!llvm::is_contained(FeaturesVec, "-vsx"))
 return true;
 
   auto FindVSXSubfeature = [&](StringRef Feature, StringRef Option) {
-if (llvm::find(FeaturesVec, Feature) != FeaturesVec.end()) {
+if (llvm::is_contained(FeaturesVec, Feature)) {
   Diags.Report(diag::err_opt_not_valid_with_opt) << Option << "-mno-vsx";
   return true;
 }
@@ -562,28 +562,28 @@ bool PPCTargetInfo::initFeatureMap(
 return false;
 
   if (!(ArchDefs & ArchDefinePwr9) && (ArchDefs & ArchDefinePpcgr) &&
-  llvm::find(FeaturesVec, "+float128") !

[PATCH] D111041: [clang-tidy] Remove 'IgnoreDestructors = true' from cppcoreguidelines-explicit-virtual-functions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 378514.
carlosgalvezp edited the summary of this revision.
carlosgalvezp added a comment.

Added an extra RUN line to run the checks also for the alias, 
cppcoreguidelines-explicit-virtual-functions.


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

https://reviews.llvm.org/D111041

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp
@@ -1,4 +1,5 @@
 // RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-explicit-virtual-functions %t 
-- -- -fexceptions
 
 #define ABSTRACT = 0
 
@@ -52,7 +53,7 @@
 struct SimpleCases : public Base {
 public:
   virtual ~SimpleCases();
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or 
(rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or 
(rarely) 'final' instead of 'virtual'
   // CHECK-FIXES: {{^}}  ~SimpleCases() override;
 
   void a();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 Changes in existing checks
 ^^
 
+- Removed default setting 
`cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+  to match the current state of the C++ Core Guidelines.
+
+
 Removed checks
 ^^
 
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -106,9 +106,6 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "true";
 
-Opts["cppcoreguidelines-explicit-virtual-functions."
- "IgnoreDestructors"] = "true";
-
 return Options;
   }
 };


Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-override.cpp
@@ -1,4 +1,5 @@
 // RUN: %check_clang_tidy %s modernize-use-override %t -- -- -fexceptions
+// RUN: %check_clang_tidy %s cppcoreguidelines-explicit-virtual-functions %t -- -- -fexceptions
 
 #define ABSTRACT = 0
 
@@ -52,7 +53,7 @@
 struct SimpleCases : public Base {
 public:
   virtual ~SimpleCases();
-  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual'
   // CHECK-FIXES: {{^}}  ~SimpleCases() override;
 
   void a();
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -113,6 +113,10 @@
 Changes in existing checks
 ^^
 
+- Removed default setting `cppcoreguidelines-explicit-virtual-functions.IgnoreDestructors = "true"`,
+  to match the current state of the C++ Core Guidelines.
+
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -106,9 +106,6 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "true";
 
-Opts["cppcoreguidelines-explicit-virtual-functions."
- "IgnoreDestructors"] = "true";
-
 return Options;
   }
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111041: [clang-tidy] Remove 'IgnoreDestructors = true' from cppcoreguidelines-explicit-virtual-functions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

@aaron.ballman Let me know if this addresses your comments!


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

https://reviews.llvm.org/D111041

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

[PATCH] D77598: Integral template argument suffix and cast printing

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/AST/DeclPrinter.cpp:1101
   Out << ", ";
-Args[I].print(Policy, Out);
+if (TemplOverloaded || !Params)
+  Args[I].print(Policy, Out, /*IncludeType*/ true);

dblaikie wrote:
> Looks like this (& the `TemplOverloaded` in the other function, below) is 
> undertested.
> 
> Hardcoding:
> true in this function results in no failures
> false in this function results in 1 failure in 
> `SemaTemplate/temp_arg_enum_printing.cpp`
>  - this failure, at least to me, seems to not have much to do with 
> overloading - at least syntactically foo<2> (where the parameter is an enum 
> type) doesn't compile, not due to any overloading ambiguity, but due to a 
> lack of implicit conversion from int to the enum type, I think? and the 
> example doesn't look like it involves any overloading... 
> 
> true in the other overload results in no failures
> false in the other overload results in no failures
> 
> I came across this because I was confused by how this feature works - when 
> the suffixes are used and when they are not to better understand the 
> implications this might have for debug info. (though I'm still generally 
> leaning towards just always putting the suffixes on for debug info)
> 
> @rsmith - could you give an example of what you meant by passing in a 
> parameter when the template is overloaded & that should use the suffix? My 
> simple examples, like this:
> ```
> template
> void f1() { }
> template
> void f1() { }
> int main() {
>   f1<3U>();
> }
> ```
> That's just ambiguous, apparently, and doesn't compile despite the type 
> suffix on the literal. If I put a parameter on one of the functions it 
> doesn't seem to trigger the "TemplOverloaded" case - both instantiations 
> still get un-suffixed names "f1<3>"... 
Ping on this (posted https://reviews.llvm.org/D111477 for some further 
discussion on the debug info side of things)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77598

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

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:407-409
+  if (!NolintBegins.empty() &&
+  (NolintBegins.back().second == NolintBracket)) {
+NolintBegins.pop_back();

carlosgalvezp wrote:
> salman-javed-nz wrote:
> > It's not necessarily the last element of `NolintBegins` that you need to 
> > `pop_back()`.
> > 
> > In the case where you have overlapping `NOLINTBEGIN` regions...
> > ```
> > // NOLINTBEGIN(A)  <-- push A
> > // NOLINTBEGIN(B) <-- push B
> > // NOLINTEND(A) <-- pop A
> > // NOLINTEND(B) <-- pop B
> > ```
> > 
> > Therefore you need to search through all of the elements, not just the last 
> > one.
> Thanks, that's right. However that's a problem that already exists on master, 
> right? It's pushing from the back anyway? We should probably have a unit test 
> for this.
> 
> Would it make sense to leave that for a separate patch? Seems like the scope 
> is growing and I'd like to keep the change as small as possible.
> Thanks, that's right. However that's a problem that already exists on master, 
> right? It's pushing from the back anyway?

Actually yes, you're right. Disregard my previous comment. I was thinking of a 
scenario that isn't a problem.
It's all looking good to me now. 


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

https://reviews.llvm.org/D111208

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

[clang] 23d5fe6 - clang: Convert two loops to for-each

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-10-10T14:55:46-04:00
New Revision: 23d5fe6235e599388a1cb3c52596ff22cd557a9c

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

LOG: clang: Convert two loops to for-each

And rewrap a line at 80 columns while here. No behavior change.

Added: 


Modified: 
clang/lib/Analysis/ReachableCode.cpp

Removed: 




diff  --git a/clang/lib/Analysis/ReachableCode.cpp 
b/clang/lib/Analysis/ReachableCode.cpp
index 221d137dadb87..9abcf20eacba8 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -227,7 +227,8 @@ static bool isConfigurationValue(const Stmt *S,
   if (IncludeIntegers) {
 if (SilenceableCondVal && !SilenceableCondVal->getBegin().isValid())
   *SilenceableCondVal = E->getSourceRange();
-return WrappedInParens || isExpandedFromConfigurationMacro(E, PP, 
IgnoreYES_NO);
+return WrappedInParens ||
+   isExpandedFromConfigurationMacro(E, PP, IgnoreYES_NO);
   }
   return false;
 }
@@ -530,12 +531,11 @@ unsigned DeadCodeScan::scanBackwards(const 
clang::CFGBlock *Start,
   // earliest location.
   if (!DeferredLocs.empty()) {
 llvm::array_pod_sort(DeferredLocs.begin(), DeferredLocs.end(), SrcCmp);
-for (DeferredLocsTy::iterator I = DeferredLocs.begin(),
- E = DeferredLocs.end(); I != E; ++I) {
-  const CFGBlock *Block = I->first;
+for (const auto &I : DeferredLocs) {
+  const CFGBlock *Block = I.first;
   if (Reachable[Block->getBlockID()])
 continue;
-  reportDeadCode(Block, I->second, CB);
+  reportDeadCode(Block, I.second, CB);
   count += scanMaybeReachableFromBlock(Block, PP, Reachable);
 }
   }
@@ -704,8 +704,7 @@ void FindUnreachableCode(AnalysisDeclContext &AC, 
Preprocessor &PP,
 
   // There are some unreachable blocks.  We need to find the root blocks that
   // contain code that should be considered unreachable.
-  for (CFG::iterator I = cfg->begin(), E = cfg->end(); I != E; ++I) {
-const CFGBlock *block = *I;
+  for (const CFGBlock *block : *cfg) {
 // A block may have been marked reachable during this loop.
 if (reachable[block->getBlockID()])
   continue;



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

[clang] 62abc18 - clang: Add range-based CFG::try_blocks()

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-10-10T15:15:37-04:00
New Revision: 62abc1842bc8b4860f9f7c1165164740fccef64e

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

LOG: clang: Add range-based CFG::try_blocks()

..and use it. No behavior change.

Added: 


Modified: 
clang/include/clang/Analysis/CFG.h
clang/lib/Analysis/ReachableCode.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/CFG.h 
b/clang/include/clang/Analysis/CFG.h
index 9e32eb8e066a..f9223fe58a27 100644
--- a/clang/include/clang/Analysis/CFG.h
+++ b/clang/include/clang/Analysis/CFG.h
@@ -1337,6 +1337,7 @@ class CFG {
   const CFGBlock * getIndirectGotoBlock() const { return IndirectGotoBlock; }
 
   using try_block_iterator = std::vector::const_iterator;
+  using try_block_range = llvm::iterator_range;
 
   try_block_iterator try_blocks_begin() const {
 return TryDispatchBlocks.begin();
@@ -1346,6 +1347,10 @@ class CFG {
 return TryDispatchBlocks.end();
   }
 
+  try_block_range try_blocks() const {
+return try_block_range(try_blocks_begin(), try_blocks_end());
+  }
+
   void addTryDispatchBlock(const CFGBlock *block) {
 TryDispatchBlocks.push_back(block);
   }

diff  --git a/clang/lib/Analysis/ReachableCode.cpp 
b/clang/lib/Analysis/ReachableCode.cpp
index 9abcf20eacba..be4ab0be373e 100644
--- a/clang/lib/Analysis/ReachableCode.cpp
+++ b/clang/lib/Analysis/ReachableCode.cpp
@@ -694,10 +694,8 @@ void FindUnreachableCode(AnalysisDeclContext &AC, 
Preprocessor &PP,
   // If there aren't explicit EH edges, we should include the 'try' dispatch
   // blocks as roots.
   if (!AC.getCFGBuildOptions().AddEHEdges) {
-for (CFG::try_block_iterator I = cfg->try_blocks_begin(),
- E = cfg->try_blocks_end() ; I != E; ++I) {
-  numReachable += scanMaybeReachableFromBlock(*I, PP, reachable);
-}
+for (const CFGBlock *B : cfg->try_blocks())
+  numReachable += scanMaybeReachableFromBlock(B, PP, reachable);
 if (numReachable == cfg->getNumBlockIDs())
   return;
   }



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

[clang] 9c8f950 - clang release notes: document the -Wbool-operation improvement

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2021-10-10T21:28:40+02:00
New Revision: 9c8f950a04004736bddb5093eda15f9a1c4f2eea

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

LOG: clang release notes: document the -Wbool-operation improvement

Reviewed By: xbolva00

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc8f8a3068245..e98a3fe00fa92 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -51,7 +51,7 @@ Major New Features
 Improvements to Clang's diagnostics
 ^^^
 
-- ...
+- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of 
bitwise with boolean operands which have side effects.
 
 Non-comprehensive list of changes in this release
 -



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

[PATCH] D111215: clang release notes: document the -Wbool-operation improvement

[Sylvestre Ledru via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c8f950a0400: clang release notes: document the 
-Wbool-operation improvement (authored by sylvestre.ledru).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111215

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -51,7 +51,7 @@
 Improvements to Clang's diagnostics
 ^^^
 
-- ...
+- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of 
bitwise with boolean operands which have side effects.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -51,7 +51,7 @@
 Improvements to Clang's diagnostics
 ^^^
 
-- ...
+- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of bitwise with boolean operands which have side effects.
 
 Non-comprehensive list of changes in this release
 -
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111215: clang release notes: document the -Wbool-operation improvement

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:54
 
-- ...
+- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of 
bitwise with boolean operands
 

xbolva00 wrote:
> I think we should mention that with boolean operands which have side effects.
s/`use of bitwise with`/`use of bitwise operators with`/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111215

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

[PATCH] D80833: [CodeView] Add full repro to LF_BUILDINFO record

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a subscriber: dexonsmith.
aganea added a comment.

In D80833#3052551 , @dexonsmith wrote:

> In D80833#2069411 , @aganea wrote:
>
>> In D80833#2069246 , @amccarth wrote:
>>
>>> Does embedding full paths affect distributed builds or build 
>>> reproducibility?
>>
>> It does affect reproducibility for distributed builds, the contents of 
>> `LF_BUILDINFO` for a remotely-compiled .OBJ will be different in the final 
>> .PDB.
>
> I don't think the path to the compiler is the only concern regarding 
> reproducible builds. Including the full command-line means that artifacts 
> change when options that have no (other) effect on the output are added. 
> E.g., adding `-Wall` will create a different object file than `-Wmost`. I 
> worry this is the wrong direction for default clang behaviour.

In my sense, the fact that an option doesn't change the output is orthogonal to 
producing a deterministic output. I think most caching systems treat the 
compiler as a black box, by simply hashing the command-line and assuming the 
same .OBJ will be generated from that command-line. The build system could 
filter out options that are known to have effect on the output, to address your 
concern. But I feel this outside of the scope of this patch?

> Should this new instability be restricted to when users explicitly request it?

The goal of this patch (and D43002 ) is simply 
to be on-par with what MSVC generates by default. It seems a bit 
counter-intuitive to provide `-grecord-command-line` to clang-cl only to match 
the MSVC behavior, but if that's the general consensus, I'm fine as long as 
`LF_BUILDINFO` is supported in LLVM.

In D80833#3052741 , @dexonsmith wrote:

> If you do go with "off-by-default", the natural driver option to use would be 
> the existing `-grecord-command-line`. You could use the same plumbing through 
> `-cc1` that it does.

You mean using the `llvm.commandline` metadata? That's an interesting idea 
which should be explored.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80833

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

[clang] b07ea8a - clang release notes: improve the wording

[Sylvestre Ledru via cfe-commits]

Author: Sylvestre Ledru
Date: 2021-10-10T22:26:11+02:00
New Revision: b07ea8a967c5e77b42eeac9307fa006b7df87ca4

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

LOG: clang release notes: improve the wording

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e98a3fe00fa9..f32ea39a7718 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -51,7 +51,7 @@ Major New Features
 Improvements to Clang's diagnostics
 ^^^
 
-- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of 
bitwise with boolean operands which have side effects.
+- -Wbitwise-instead-of-logical (part of -Wbool-operation) warns about use of 
bitwise operators with boolean operands which have side effects.
 
 Non-comprehensive list of changes in this release
 -



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

[PATCH] D111215: clang release notes: document the -Wbool-operation improvement

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

@Quuxplusone fixed, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111215

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

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added a comment.

Thanks for all the changes. You've addressed all my questions ad I don't have 
anything more to add. Let's wait for the reviewers to take a look.


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

https://reviews.llvm.org/D111208

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

[clang] 2fc0d43 - [Clang] [PowerPC] Fix header include typo in smmintrin.h

[Qiu Chaofan via cfe-commits]

Author: Qiu Chaofan
Date: 2021-10-11T10:44:08+08:00
New Revision: 2fc0d439a4b6dd231f5b40e67e5c4c5863f5ae45

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

LOG: [Clang] [PowerPC] Fix header include typo in smmintrin.h

The SSE4 header (smmintrin.h) should include SSSE3 (tmmintrin.h) instead
of SSE2 (emmintrin.h).

Reviewed By: jsji

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

Added: 


Modified: 
clang/lib/Headers/ppc_wrappers/smmintrin.h
clang/test/CodeGen/ppc-smmintrin.c

Removed: 




diff  --git a/clang/lib/Headers/ppc_wrappers/smmintrin.h 
b/clang/lib/Headers/ppc_wrappers/smmintrin.h
index 64f0c761994d5..f41264b27584d 100644
--- a/clang/lib/Headers/ppc_wrappers/smmintrin.h
+++ b/clang/lib/Headers/ppc_wrappers/smmintrin.h
@@ -32,7 +32,7 @@
 #if defined(__linux__) && defined(__ppc64__)
 
 #include 
-#include 
+#include 
 
 extern __inline int
 __attribute__((__gnu_inline__, __always_inline__, __artificial__))

diff  --git a/clang/test/CodeGen/ppc-smmintrin.c 
b/clang/test/CodeGen/ppc-smmintrin.c
index 644037f03afb9..6b88b7ac09310 100644
--- a/clang/test/CodeGen/ppc-smmintrin.c
+++ b/clang/test/CodeGen/ppc-smmintrin.c
@@ -145,3 +145,14 @@ test_insert() {
 // CHECK: %[[R0:[0-9a-zA-Z_.]+]] = and i32 %{{[0-9a-zA-Z_.]+}}, 1
 // CHECK: %{{[0-9a-zA-Z_.]+}} = insertelement <2 x i64> %{{[0-9a-zA-Z_.]+}}, 
i64 %{{[0-9a-zA-Z_.]+}}, i32 %[[R0:[0-9a-zA-Z_.]+]]
 // CHECK: ret <2 x i64> %{{[0-9a-zA-Z_.]+}}
+
+// To test smmintrin.h includes tmmintrin.h
+
+void __attribute__((noinline))
+test_abs_ssse3() {
+  _mm_abs_epi16(m1);
+}
+
+// CHECK-LABEL: @test_abs_ssse3
+
+// CHECK: define available_externally <2 x i64> @_mm_abs_epi16(<2 x i64> 
{{[0-9a-zA-Z_%.]+}})



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

[PATCH] D111482: [Clang] [PowerPC] Fix header include typo in smmintrin.h

[Qiu Chaofan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2fc0d439a4b6: [Clang] [PowerPC] Fix header include typo in 
smmintrin.h (authored by qiucf).

Changed prior to commit:
  https://reviews.llvm.org/D111482?vs=378414&id=378550#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111482

Files:
  clang/lib/Headers/ppc_wrappers/smmintrin.h
  clang/test/CodeGen/ppc-smmintrin.c


Index: clang/test/CodeGen/ppc-smmintrin.c
===
--- clang/test/CodeGen/ppc-smmintrin.c
+++ clang/test/CodeGen/ppc-smmintrin.c
@@ -145,3 +145,14 @@
 // CHECK: %[[R0:[0-9a-zA-Z_.]+]] = and i32 %{{[0-9a-zA-Z_.]+}}, 1
 // CHECK: %{{[0-9a-zA-Z_.]+}} = insertelement <2 x i64> %{{[0-9a-zA-Z_.]+}}, 
i64 %{{[0-9a-zA-Z_.]+}}, i32 %[[R0:[0-9a-zA-Z_.]+]]
 // CHECK: ret <2 x i64> %{{[0-9a-zA-Z_.]+}}
+
+// To test smmintrin.h includes tmmintrin.h
+
+void __attribute__((noinline))
+test_abs_ssse3() {
+  _mm_abs_epi16(m1);
+}
+
+// CHECK-LABEL: @test_abs_ssse3
+
+// CHECK: define available_externally <2 x i64> @_mm_abs_epi16(<2 x i64> 
{{[0-9a-zA-Z_%.]+}})
Index: clang/lib/Headers/ppc_wrappers/smmintrin.h
===
--- clang/lib/Headers/ppc_wrappers/smmintrin.h
+++ clang/lib/Headers/ppc_wrappers/smmintrin.h
@@ -32,7 +32,7 @@
 #if defined(__linux__) && defined(__ppc64__)
 
 #include 
-#include 
+#include 
 
 extern __inline int
 __attribute__((__gnu_inline__, __always_inline__, __artificial__))


Index: clang/test/CodeGen/ppc-smmintrin.c
===
--- clang/test/CodeGen/ppc-smmintrin.c
+++ clang/test/CodeGen/ppc-smmintrin.c
@@ -145,3 +145,14 @@
 // CHECK: %[[R0:[0-9a-zA-Z_.]+]] = and i32 %{{[0-9a-zA-Z_.]+}}, 1
 // CHECK: %{{[0-9a-zA-Z_.]+}} = insertelement <2 x i64> %{{[0-9a-zA-Z_.]+}}, i64 %{{[0-9a-zA-Z_.]+}}, i32 %[[R0:[0-9a-zA-Z_.]+]]
 // CHECK: ret <2 x i64> %{{[0-9a-zA-Z_.]+}}
+
+// To test smmintrin.h includes tmmintrin.h
+
+void __attribute__((noinline))
+test_abs_ssse3() {
+  _mm_abs_epi16(m1);
+}
+
+// CHECK-LABEL: @test_abs_ssse3
+
+// CHECK: define available_externally <2 x i64> @_mm_abs_epi16(<2 x i64> {{[0-9a-zA-Z_%.]+}})
Index: clang/lib/Headers/ppc_wrappers/smmintrin.h
===
--- clang/lib/Headers/ppc_wrappers/smmintrin.h
+++ clang/lib/Headers/ppc_wrappers/smmintrin.h
@@ -32,7 +32,7 @@
 #if defined(__linux__) && defined(__ppc64__)
 
 #include 
-#include 
+#include 
 
 extern __inline int
 __attribute__((__gnu_inline__, __always_inline__, __artificial__))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111518: Add release note about `TypeLoc` matchers.

[James King via Phabricator via cfe-commits]

jcking1034 created this revision.
jcking1034 added reviewers: ymandel, aaron.ballman, tdl-g.
jcking1034 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111518

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -183,7 +183,11 @@
 AST Matchers
 
 
-- ...
+- ``TypeLoc`` AST Matchers are now available. These matchers provide helpful
+  utilities for matching ``TypeLoc`` nodes, such as the ``pointerTypeLoc``
+  matcher or the ``hasReturnTypeLoc`` matcher. The addition of these matchers
+  was made possible by changes to the handling of ``TypeLoc`` nodes that
+  allows them to enjoy the same static type checking as other AST node kinds.
 
 clang-format
 


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -183,7 +183,11 @@
 AST Matchers
 
 
-- ...
+- ``TypeLoc`` AST Matchers are now available. These matchers provide helpful
+  utilities for matching ``TypeLoc`` nodes, such as the ``pointerTypeLoc``
+  matcher or the ``hasReturnTypeLoc`` matcher. The addition of these matchers
+  was made possible by changes to the handling of ``TypeLoc`` nodes that
+  allows them to enjoy the same static type checking as other AST node kinds.
 
 clang-format
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111199: [POC][BTF] support btf_type_tag attribute

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@aaron.ballman Ping. This is to address your concern in D110127 
, could you take a look whether my proposal 
for a new attribute btf_type_tag will be okay for you or not? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99

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

[PATCH] D109950: [Clang] Enable IC/IF mode for __ibm128

[Qiu Chaofan via Phabricator via cfe-commits]

qiucf updated this revision to Diff 378561.
qiucf added a comment.

Add codegen test to show generated type in IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109950

Files:
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGenCXX/ibm128-declarations.cpp
  clang/test/Sema/attr-mode.c


Index: clang/test/Sema/attr-mode.c
===
--- clang/test/Sema/attr-mode.c
+++ clang/test/Sema/attr-mode.c
@@ -92,6 +92,12 @@
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
+typedef float w128ibm __attribute__((mode(IF)));
+typedef _Complex float cw128ibm __attribute__((mode(IC)));
+void f_ibm128_arg(__ibm128 *x);
+void f_ibm128_complex_arg(_Complex __ibm128 *x);
+void test_IFtype(w128ibm *a) { f_ibm128_arg(a); }
+void test_ICtype(cw128ibm *a) { f_ibm128_complex_arg(a); }
 #elif TEST_F128_PPC64
 typedef int invalid_7 __attribute((mode(KF))); // expected-error{{type of 
machine mode does not match type of base type}}
 typedef int invalid_8 __attribute((mode(KI))); // expected-error{{unknown 
machine mode}}
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- clang/test/CodeGenCXX/ibm128-declarations.cpp
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -59,6 +59,12 @@
   constexpr static __ibm128 mem = Q;
 };
 
+typedef float w128ibm __attribute__((mode(IF)));
+typedef _Complex float w128ibm_c __attribute__((mode(IC)));
+
+w128ibm icmode_self(w128ibm x) { return x; }
+w128ibm_c icmode_self_complex(w128ibm_c x) { return x; }
+
 int main(void) {
   __ibm128 lf;
   CTest ct(lf);
@@ -115,6 +121,9 @@
 // CHECK:   ret ppc_fp128 %2
 // CHECK: }
 
+// CHECK: define dso_local ppc_fp128 @_Z11icmode_selfg(ppc_fp128 %x)
+// CHECK: define dso_local { ppc_fp128, ppc_fp128 } 
@_Z19icmode_self_complexCg(ppc_fp128 %x.coerce0, ppc_fp128 %x.coerce1)
+
 // CHECK: define dso_local signext i32 @main()
 // CHECK: entry:
 // CHECK:   %0 = load ppc_fp128, ppc_fp128* %lf, align 16
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4233,6 +4233,10 @@
   ExplicitType = FloatModeKind::LongDouble;
   DestWidth = 128;
   break;
+case 'I':
+  ExplicitType = FloatModeKind::Ibm128;
+  DestWidth = Str[1] == 'I' ? 0 : 128;
+  break;
 }
 if (Str[1] == 'F') {
   IntegerMode = false;
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -297,11 +297,11 @@
 if (ExplicitType == FloatModeKind::Float128)
   return hasFloat128Type() ? FloatModeKind::Float128
: FloatModeKind::NoFloat;
-if (&getLongDoubleFormat() == &llvm::APFloat::PPCDoubleDouble() ||
-&getLongDoubleFormat() == &llvm::APFloat::IEEEquad())
-  return FloatModeKind::LongDouble;
-if (hasFloat128Type())
-  return FloatModeKind::Float128;
+if (ExplicitType == FloatModeKind::Ibm128)
+  return hasIbm128Type() ? FloatModeKind::Ibm128
+ : FloatModeKind::NoFloat;
+if (ExplicitType == FloatModeKind::LongDouble)
+  return ExplicitType;
 break;
   }
 


Index: clang/test/Sema/attr-mode.c
===
--- clang/test/Sema/attr-mode.c
+++ clang/test/Sema/attr-mode.c
@@ -92,6 +92,12 @@
 void f_ft128_complex_arg(_Complex long double *x);
 void test_TFtype(f128ibm *a) { f_ft128_arg (a); }
 void test_TCtype(c128ibm *a) { f_ft128_complex_arg (a); }
+typedef float w128ibm __attribute__((mode(IF)));
+typedef _Complex float cw128ibm __attribute__((mode(IC)));
+void f_ibm128_arg(__ibm128 *x);
+void f_ibm128_complex_arg(_Complex __ibm128 *x);
+void test_IFtype(w128ibm *a) { f_ibm128_arg(a); }
+void test_ICtype(cw128ibm *a) { f_ibm128_complex_arg(a); }
 #elif TEST_F128_PPC64
 typedef int invalid_7 __attribute((mode(KF))); // expected-error{{type of machine mode does not match type of base type}}
 typedef int invalid_8 __attribute((mode(KI))); // expected-error{{unknown machine mode}}
Index: clang/test/CodeGenCXX/ibm128-declarations.cpp
===
--- clang/test/CodeGenCXX/ibm128-declarations.cpp
+++ clang/test/CodeGenCXX/ibm128-declarations.cpp
@@ -59,6 +59,12 @@
   constexpr static __ibm128 mem = Q;
 };
 
+typedef float w128ibm __attribute__((mode(IF)));
+typedef _Complex float w128ibm_c __attribute__((mode(IC)));
+
+w128ibm icmode_self(w128ibm x) { return x; }
+w128ibm_c icmode_self_complex(w128ibm_c x) { return x; }
+
 int main(void) {
   __ibm128 lf

[PATCH] D111521: [DebugInfo] Mark OpenMP generated functions as artificial

[Alok Kumar Sharma via Phabricator via cfe-commits]

alok created this revision.
alok added reviewers: aprantl, djtodoro, jmorse, dblaikie, jini.susan, 
jini.susan.george.
alok added a project: debug-info.
Herald added subscribers: guansong, yaxunl.
alok requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

The OpenMP generates many functions which are not in user program.

Example: "__captured_stmt_debug_", "__captured_stmt", ".omp_outlined._debug__",
 ".omp_task_entry.", ".omp_outlined.", ".omp_outlined..1"

Current patch marks these all as artificial.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111521

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/OpenMP/outlined_artificial.c


Index: clang/test/OpenMP/outlined_artificial.c
===
--- /dev/null
+++ clang/test/OpenMP/outlined_artificial.c
@@ -0,0 +1,63 @@
+// This testcase checks emission of DIFlagArtificial flag for outlined
+// subroutines generated by compiler.
+
+// REQUIRES: x86_64-linux
+
+// RUN: %clang_cc1 -debug-info-kind=constructor -x c -verify -triple 
x86_64-pc-linux-gnu -fopenmp -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#if defined(_WIN32)
+#define __KAI_KMPC_CONVENTION __cdecl
+#else
+#define __KAI_KMPC_CONVENTION
+#endif
+
+extern int printf(const char *, ...);
+extern void __KAI_KMPC_CONVENTION omp_set_num_threads(int);
+extern int __KAI_KMPC_CONVENTION omp_get_thread_num(void);
+
+#define N 10
+
+float f[10];
+void foo_simd(int low, int up) {
+  for (int i = low; i < up; ++i) {
+f[i] = 0.0;
+#pragma omp ordered simd
+f[i] = 1.0;
+  }
+}
+
+int main() {
+  int arr[10];
+  int i;
+  omp_set_num_threads(2);
+#pragma omp parallel
+#pragma omp single
+#pragma omp taskloop num_tasks(10)
+  for (i = 0; i < N; i++) {
+arr[i] = i * i;
+  }
+
+  for (int j = 0; j < N; j++) {
+printf("%d\n", arr[j]);
+  }
+  return 0;
+}
+
+// CHECK: !DISubprogram(name: "__captured_stmt_debug__"
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: "__captured_stmt"
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: ".omp_outlined._debug__"
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(linkageName: ".omp_task_entry."
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: ".omp_outlined."
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: ".omp_outlined..1"
+// CHECK-SAME: DIFlagArtificial
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4020,7 +4020,8 @@
 Name = Name.substr(1);
 
   if (!HasDecl || D->isImplicit() || D->hasAttr() ||
-  (isa(D) && GD.getDynamicInitKind() != DynamicInitKind::NoStub)) 
{
+  (isa(D) && GD.getDynamicInitKind() != DynamicInitKind::NoStub) 
||
+  isa(D)) {
 Flags |= llvm::DINode::FlagArtificial;
 // Artificial functions should not silently reuse CurLoc.
 CurLoc = SourceLocation();


Index: clang/test/OpenMP/outlined_artificial.c
===
--- /dev/null
+++ clang/test/OpenMP/outlined_artificial.c
@@ -0,0 +1,63 @@
+// This testcase checks emission of DIFlagArtificial flag for outlined
+// subroutines generated by compiler.
+
+// REQUIRES: x86_64-linux
+
+// RUN: %clang_cc1 -debug-info-kind=constructor -x c -verify -triple x86_64-pc-linux-gnu -fopenmp -emit-llvm %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#if defined(_WIN32)
+#define __KAI_KMPC_CONVENTION __cdecl
+#else
+#define __KAI_KMPC_CONVENTION
+#endif
+
+extern int printf(const char *, ...);
+extern void __KAI_KMPC_CONVENTION omp_set_num_threads(int);
+extern int __KAI_KMPC_CONVENTION omp_get_thread_num(void);
+
+#define N 10
+
+float f[10];
+void foo_simd(int low, int up) {
+  for (int i = low; i < up; ++i) {
+f[i] = 0.0;
+#pragma omp ordered simd
+f[i] = 1.0;
+  }
+}
+
+int main() {
+  int arr[10];
+  int i;
+  omp_set_num_threads(2);
+#pragma omp parallel
+#pragma omp single
+#pragma omp taskloop num_tasks(10)
+  for (i = 0; i < N; i++) {
+arr[i] = i * i;
+  }
+
+  for (int j = 0; j < N; j++) {
+printf("%d\n", arr[j]);
+  }
+  return 0;
+}
+
+// CHECK: !DISubprogram(name: "__captured_stmt_debug__"
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: "__captured_stmt"
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: ".omp_outlined._debug__"
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(linkageName: ".omp_task_entry."
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: ".omp_outlined."
+// CHECK-SAME: DIFlagArtificial
+
+// CHECK: !DISubprogram(name: ".omp_outlined..1"
+// CHECK-SAME: DIFlagArtificial
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugI

[PATCH] D111208: [clang-tidy] Support globbing in NOLINT* expressions

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Awesome, thanks a lot for the review, I really improved my understanding of the 
tool :)

@aaron.ballman Could you review? Otherwise could you point me to who should 
review? I tagged @alexfh as owner of clang-tidy but haven't really seen him 
around... Shouldn't there be multiple owners, so that there's not a "single 
point of failure"?


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

https://reviews.llvm.org/D111208

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

[PATCH] D110618: [HIPSPV][2/4] Add HIPSPV tool chain

[Pekka Jääskeläinen via Phabricator via cfe-commits]

pekka.jaaskelainen added a comment.

> I don't feel it is different for OpenCL though... I am not in favour of 
> repeating the same functionality for every language since the requirement 
> will be likely identical. There is no timeline for when this functionality 
> will be dropped so we have to assume that even though temporary it might be a 
> solution for long enough.

If it is likely the SPIR-V backend won’t land soon and it is expected that also 
other languages might benefit from the calls to the llvm-spirv tool, the 
functionality to do so would be better placed in a more generally useful place. 
Do you have suggestions where this functionality could be moved?

I believe it concerns mostly the call to the llvm-spriv (this is sought in 
PATH, not given via a command line parameter) inside 
constructEmitSpirvCommand(). This could be extracted to another function in a 
utility library so it could be accessed by other languages/tools in the future. 
Does it sound OK?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110618

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