[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 336533.
nick edited the summary of this revision.
nick added reviewers: njames93, steveire.
nick added a comment.

Rebased. Now using native `cxxBaseSpecifier` and `hasDirectBase`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69000

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-iterator-base.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s modernize-deprecated-iterator-base %t 
+
+namespace std {
+using ptrdiff_t = int;
+struct input_iterator_tag;
+template 
+struct iterator {
+  using iterator_category = C;
+  using value_type= T;
+  using difference_type   = D;
+  using pointer   = P;
+  using reference = R;
+};
+}
+
+
+using iterator_alias = std::iterator;
+typedef std::iterator iterator_typedef;
+
+
+// Sugar
+
+// CHECK-FIXES: struct from_alias {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct from_alias: iterator_alias {};
+
+// CHECK-FIXES: struct from_typedef {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct from_typedef: iterator_typedef {};
+
+
+// False-positive
+
+// CHECK-FIXES: struct indirect_base: from_alias {};
+// CHECK-MESSAGES-NOT: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct indirect_base: from_alias {};
+
+
+// Unsupported
+
+// CHECK-FIXES: class skipif_non_public_inheritance: iterator_alias {};
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+class skipif_non_public_inheritance: iterator_alias {};
+
+
+// Base removal
+
+struct A {};
+struct B {};
+
+struct collection {
+  template 
+  struct iterator;
+};
+
+// CHECK-FIXES: template <> struct collection::iterator<> {
+// CHECK-MESSAGES: :[[@LINE+1]]:45: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator<> : iterator_alias {};
+// CHECK-FIXES: template <> struct collection::iterator : A {
+// CHECK-MESSAGES: :[[@LINE+1]]:49: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : A, iterator_alias {};
+// CHECK-FIXES: template <> struct collection::iterator : B {
+// CHECK-MESSAGES: :[[@LINE+1]]:46: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : iterator_alias, B {};
+// CHECK-FIXES: template <> struct collection::iterator : A, B {
+// CHECK-MESSAGES: :[[@LINE+1]]:52: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : A, iterator_alias, B {};
+
+// CHECK-FIXES: struct do_not_strip_final final {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct do_not_strip_final final : iterator_alias {};
+
+// CHECK-FIXES: struct iteratorZ // iterator_alias
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+2]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorZ   // iteratorZ
+  : iterator_alias // iterator_alias
+{};
+// CHECK-FIXES: struct iteratorA   // iteratorA
+// CHECK-FIXES:   : A // iterator_alias
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+3]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorA   // iteratorA
+  : A  // A
+  , iterator_alias // iterator_alias
+{};
+// CHECK-FIXES: struct iteratorB   // iteratorB
+// CHECK-FIXES:   : B  // B
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+2]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorB   // iteratorB
+  : iterator_alias // iterator_alias
+  , B  // B
+{};
+// CHECK-FIXES: struct iteratorAB  // iteratorAB
+// CHECK-FIXES:   : A  // A
+// CHECK-FIXES:   , B  // B
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+3]]:5: warning: 

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

In D69218#2667523 , @steveire wrote:

> What name/email should I use for the commit?

Nikita Kniazev 


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

https://reviews.llvm.org/D69218

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick added inline comments.



Comment at: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp:301
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =

steveire wrote:
> @nick Is this implemented in another MR? I don't see anything in your list of 
> revisions. I think this is reasonable as is, but wondering if you intend to 
> implement the top-level support too.
The patch had some of the top-level matcher parts, but it cut them off when 
rebased. I have no need in it myself and I am not sure there is any kind of use 
for it.


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

https://reviews.llvm.org/D69218

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 333728.
nick added a comment.

In D69218#2654614 , @steveire wrote:

> @nick Sorry that getting these changes merged takes so long.
>
> @njames93 If you have an alternative way forward, please let us know what it 
> is.
>
> Otherwise, this LGTM too and we should merge it soon unless there are 
> objections which haven't been addressed.

I had a PR in Boost that took 4 years to merge, so it is nothing new to me :D

Rebased, even though there was no conflicts.


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4400,6 +4400,13 @@
 return;
   }
 
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
   EXPECT_TRUE(matches(
   R"cc(
 class Base {};
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -175,6 +175,7 @@
   REGISTER_MATCHER(coreturnStmt);
   REGISTER_MATCHER(coyieldExpr);
   REGISTER_MATCHER(cudaKernelCallExpr);
+  REGISTER_MATCHER(cxxBaseSpecifier);
   REGISTER_MATCHER(cxxBindTemporaryExpr);
   REGISTER_MATCHER(cxxBoolLiteral);
   REGISTER_MATCHER(cxxCatchStmt);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ 

[PATCH] D99107: [clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

Is this hack actually needed? I could not reproduce a problem with 
https://bugs.chromium.org/p/chromium/issues/detail?id=860398#c13 repro, the 
breakpoint fires for me and I see the variable.

The difference with the hack and without, using `clang.exe src.cpp -S 
-masm=intel -O2 -g -o out.asm` is

  diff
  ---
  +++
  @@ -26,10 +26,10 @@
  .seh_stackalloc 32
  .seh_endprologue
   .Ltmp0:
  -   #DEBUG_VALUE: test0:x <- [DW_OP_deref] $rcx
  +   #DEBUG_VALUE: test0:x <- [$rcx+0]
  mov rsi, rcx
   .Ltmp1:
  -   #DEBUG_VALUE: test0:x <- [DW_OP_deref] $rsi
  +   #DEBUG_VALUE: test0:x <- [$rsi+0]
  .cv_loc 0 1 9 0 # nrvox.cpp:9:0
  call"??0X@@QEAA@XZ"
  .cv_loc 0 1 10 0# nrvox.cpp:10:0




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99107

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

[PATCH] D99107: [clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

In D99107#2642451 , @rnk wrote:

> We try to uphold the invariant that -g flags do not affect generated code, so 
> I don't think we should do this.

Even under `-O0`? I could change the check to always emit on `-O0`. With 
optimizations enabled there is no change in generated code, DSE+DCE removes it 
anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99107

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

[PATCH] D99107: [clang][CodeGen] Do not emit NVRO debug helper when not emitting debug info

[Nikita Kniazev via Phabricator via cfe-commits]

nick created this revision.
nick added reviewers: akhuang, rnk, aprantl.
nick requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When no debug information is emitted there is no point in emitting a hack 
introduced in D63361 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D99107

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/arm64-microsoft-arguments.cpp


Index: clang/test/CodeGen/arm64-microsoft-arguments.cpp
===
--- clang/test/CodeGen/arm64-microsoft-arguments.cpp
+++ clang/test/CodeGen/arm64-microsoft-arguments.cpp
@@ -43,7 +43,7 @@
 // Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
 // Passed directly but returned indirectly.
 // CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias 
sret(%struct.S4) align 4 %agg.result)
-// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret(%struct.S4) align 
4 %agg.result, [2 x i64] %5)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret(%struct.S4) align 
4 %agg.result, [2 x i64] %4)
 struct S4 {
   int a[3];
   ~S4();
@@ -57,7 +57,7 @@
 
 // Pass and return from instance method called from instance method.
 // CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* {{[^,]*}} %this, 
%class.P1* inreg noalias sret(%class.P1) align 1 %agg.result)
-// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* {{[^,]*}} %ref.tmp, 
%class.P1* inreg sret(%class.P1) align 1 %agg.result, i8 %1)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* {{[^,]*}} %ref.tmp, 
%class.P1* inreg sret(%class.P1) align 1 %agg.result, i8 %0)
 
 class P1 {
 public:
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1055,7 +1055,8 @@
 if (CurFnInfo->getReturnInfo().isSRetAfterThis())
   ++AI;
 ReturnValue = Address(&*AI, CurFnInfo->getReturnInfo().getIndirectAlign());
-if (!CurFnInfo->getReturnInfo().getIndirectByVal()) {
+if (getDebugInfo() && CGM.getCodeGenOpts().hasReducedDebugInfo() &&
+!CurFnInfo->getReturnInfo().getIndirectByVal()) {
   ReturnValuePointer =
   CreateDefaultAlignTempAlloca(Int8PtrTy, "result.ptr");
   Builder.CreateStore(Builder.CreatePointerBitCastOrAddrSpaceCast(


Index: clang/test/CodeGen/arm64-microsoft-arguments.cpp
===
--- clang/test/CodeGen/arm64-microsoft-arguments.cpp
+++ clang/test/CodeGen/arm64-microsoft-arguments.cpp
@@ -43,7 +43,7 @@
 // Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
 // Passed directly but returned indirectly.
 // CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret(%struct.S4) align 4 %agg.result)
-// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret(%struct.S4) align 4 %agg.result, [2 x i64] %5)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret(%struct.S4) align 4 %agg.result, [2 x i64] %4)
 struct S4 {
   int a[3];
   ~S4();
@@ -57,7 +57,7 @@
 
 // Pass and return from instance method called from instance method.
 // CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* {{[^,]*}} %this, %class.P1* inreg noalias sret(%class.P1) align 1 %agg.result)
-// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* {{[^,]*}} %ref.tmp, %class.P1* inreg sret(%class.P1) align 1 %agg.result, i8 %1)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* {{[^,]*}} %ref.tmp, %class.P1* inreg sret(%class.P1) align 1 %agg.result, i8 %0)
 
 class P1 {
 public:
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1055,7 +1055,8 @@
 if (CurFnInfo->getReturnInfo().isSRetAfterThis())
   ++AI;
 ReturnValue = Address(&*AI, CurFnInfo->getReturnInfo().getIndirectAlign());
-if (!CurFnInfo->getReturnInfo().getIndirectByVal()) {
+if (getDebugInfo() && CGM.getCodeGenOpts().hasReducedDebugInfo() &&
+!CurFnInfo->getReturnInfo().getIndirectByVal()) {
   ReturnValuePointer =
   CreateDefaultAlignTempAlloca(Int8PtrTy, "result.ptr");
   Builder.CreateStore(Builder.CreatePointerBitCastOrAddrSpaceCast(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3812-3821
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
 hasType,
 AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, TypedefNameDecl,
-ValueDecl),
+ValueDecl, CXXBaseSpecifier),
 internal::Matcher, InnerMatcher, 0) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())

njames93 wrote:
> I don't think the change to this matcher is warranted.
> The `hasType` matcher that accepts a DeclarationMatcher already has support 
> for cxxBaseSpecifier.
> However overloading the matcher that takes a QualType matcher doesn't make 
> sense as base specifiers have no qualifications.
What should I use in D69000 then? It is been a very long time since I developed 
and published these patches, but D69000 definitely requires this matcher, 
without it I get:

```
llvm-project\clang\include\clang\ASTMatchers\ASTMatchersInternal.h(1569): error 
C2338: right polymorphic conversion
llvm-project\clang-tools-extra\clang-tidy\modernize\DeprecatedIteratorBaseCheck.cpp(200):
 note: see reference to function template instantiation 
'clang::ast_matchers::internal::PolymorphicMatcher),clang::ast_matchers::internal::Matcher>::operator
 clang::ast_matchers::internal::Matcher(void) 
const' being compiled
llvm-project\clang-tools-extra\clang-tidy\modernize\DeprecatedIteratorBaseCheck.cpp(192):
 note: see reference to function template instantiation 
'clang::ast_matchers::internal::PolymorphicMatcher),clang::ast_matchers::internal::Matcher>::operator
 clang::ast_matchers::internal::Matcher(void) 
const' being compiled
```



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

https://reviews.llvm.org/D69218

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

Could you please commit the patch for me? I do not have commit rights.


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

https://reviews.llvm.org/D69218

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 332151.
nick added a comment.

Lint fixes


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4400,6 +4400,13 @@
 return;
   }
 
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
   EXPECT_TRUE(matches(
   R"cc(
 class Base {};
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -172,6 +172,7 @@
   REGISTER_MATCHER(containsDeclaration);
   REGISTER_MATCHER(continueStmt);
   REGISTER_MATCHER(cudaKernelCallExpr);
+  REGISTER_MATCHER(cxxBaseSpecifier);
   REGISTER_MATCHER(cxxBindTemporaryExpr);
   REGISTER_MATCHER(cxxBoolLiteral);
   REGISTER_MATCHER(cxxCatchStmt);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -756,6 +756,7 @@
 const internal::VariadicDynCastAllOfMatcher parmVarDecl;
 const internal::VariadicDynCastAllOfMatcher
 accessSpecDecl;
+const internal::VariadicAllOfMatcher cxxBaseSpecifier;
 const internal::VariadicAllOfMatcher cxxCtorInitializer;
 const internal::VariadicAllOfMatcher templateArgument;
 const internal::VariadicAllOfMatcher templateArgumentLoc;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:329
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}

aaron.ballman wrote:
> I'd like to see another test along these lines:
> ```
> struct Base {};
> struct Intermediate : Base {};
> struct Derived : Intermediate {};
> ```
> Where we test that `Derived` does not have a direct base relationship with 
> `Base`, but does with `hasAnyBase`.
I do not understand why it should be here. The point of this test is to ensure 
that `hasType` can be used inside `cxxBaseSpecifier`.  The support was added in 
D79063 without a test and I just fill the gaps. 



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

https://reviews.llvm.org/D69218

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

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 331955.
nick added a comment.

Forgot to remove a duplicated test


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4400,6 +4400,13 @@
 return;
   }
 
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
   EXPECT_TRUE(matches(
   R"cc(
 class Base {};
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -179,6 +179,7 @@
   REGISTER_MATCHER(cxxConstructExpr);
   REGISTER_MATCHER(cxxConstructorDecl);
   REGISTER_MATCHER(cxxConversionDecl);
+  REGISTER_MATCHER(cxxBaseSpecifier);
   REGISTER_MATCHER(cxxCtorInitializer);
   REGISTER_MATCHER(cxxDeductionGuideDecl);
   REGISTER_MATCHER(cxxDefaultArgExpr);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -756,6 +756,7 @@
 const internal::VariadicDynCastAllOfMatcher parmVarDecl;
 const internal::VariadicDynCastAllOfMatcher
 accessSpecDecl;
+const internal::VariadicAllOfMatcher cxxBaseSpecifier;
 const internal::VariadicAllOfMatcher cxxCtorInitializer;
 const internal::VariadicAllOfMatcher templateArgument;
 const internal::VariadicAllOfMatcher templateArgumentLoc;
Index: 

[PATCH] D69218: [ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 331945.
nick retitled this revision from "[clang][AST] Add `CXXBaseSpecifier` matcher 
support" to "[ASTMatchers] Add `cxxBaseSpecifier` matcher (non-top-level)".
nick edited the summary of this revision.
nick added a reviewer: aaron.ballman.
nick added a comment.

Rebased. Removed things reimplemented and landed in D81552 
 + D79063 .


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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/RegistryTest.cpp
@@ -297,6 +297,17 @@
   EXPECT_TRUE(matches("int b[7];", M));
 }
 
+TEST_F(RegistryTest, CXXBaseSpecifier) {
+  // TODO: rewrite with top-level cxxBaseSpecifier matcher when available
+  DeclarationMatcher ClassHasAnyDirectBase =
+  constructMatcher("cxxRecordDecl",
+   constructMatcher("hasDirectBase",
+constructMatcher("cxxBaseSpecifier")))
+  .getTypedMatcher();
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+}
+
 TEST_F(RegistryTest, CXXCtorInitializer) {
   Matcher CtorDecl = constructMatcher(
   "cxxConstructorDecl",
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,15 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +346,15 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1278,6 +1278,35 @@
   ZIsDirectlyDerivedFromX));
 }
 
+TEST_P(ASTMatchersTest, ClassHasBase) {
+  if (!GetParam().isCXX()) {
+return;
+  }
+  DeclarationMatcher ClassHasAnyDirectBase =
+  cxxRecordDecl(hasDirectBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyDirectBase));
+  EXPECT_TRUE(matches("class X {}; class Y : public virtual X {};",
+  ClassHasAnyDirectBase));
+
+  TypeMatcher ClassX = asString("class X");
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX =
+  cxxRecordDecl(hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : X, Y {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : Y, X {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class W {}; class Y {}; class Z : W, Y {};",
+ ClassHasBaseClassX));
+  DeclarationMatcher ClassZHasBaseClassX =
+  cxxRecordDecl(hasName("Z"), hasDirectBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {}; class Z : X {};",
+  

[PATCH] D82425: [SemaCXX] Fix false positive of -Wuninitialized-const-reference in empty function body.

[Nikita Kniazev via Phabricator via cfe-commits]

nick added inline comments.



Comment at: clang/lib/Analysis/UninitializedValues.cpp:435
   if ((*I)->getType().isConstQualified())
-classify((*I), ConstRefUse);
+if (!hasTrivialBody(CE))
+  classify((*I), ConstRefUse);

zequanwu wrote:
> aaron.ballman wrote:
> > This can be hoisted out of the loop so that we don't have to check the same 
> > thing on every argument.
> The `DeclRefExpr` needs to be set to `Ignore` like `VisitCastExpr` does. 
> Otherwise, it maybe classified to `Init` by `isTrackedVar` in 
> `ClassifyRefs::get`.
Could not the empty body check be done in `reportConstRefUse`, after 
`isUninitialized`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82425



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

[PATCH] D82425: [SemaCXX] Fix false positive of -Wuninitialized-const-reference in empty function body.

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

Thanks @zequanwu, much appreciated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82425



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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

> We didn't see it in the code bases I work with, so is boost a special case, 
> or an example of a common practice?

I do not have resources to make such statistics, but there are compilers where 
casting to void is not enough to suppress the warning. 
https://herbsutter.com/2009/10/18/mailbag-shutting-up-compiler-warnings/#comment-1509
 https://godbolt.org/z/pS_iQ3

> If it's just boost, fixing the code seems better

Have you tried to push a fix for a warning in Boost? If it was that simple. A 
part of my warning-fixing PRs are not merged in years. And considering that 
fixing this warning will reintroduce warnings for other compilers I probably 
will have a bad luck with this one too.

> (it will compile faster too).

Should I open a PR with replacing `std::forward` with `static_cast` because it 
compiles faster? :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79895



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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

> I feel like doing interprocedural analysis for this is overkill. What is the 
> benefit of boost::ignore_unused(foo); rather than the more common (void) 
> foo;? Any examples?



> I haven't seen boost::ignore_unused before. In my experience, the idiomatic 
> way of ignoring an unused variable in C/C++ is to cast it to void, as Arthur 
> said.

This is a weak argument to have false positives, don't you agree? You may have 
not seen it, but it exists and is used: 
https://github.com/search?q=%22boost%3A%3Aignore_unused%22+NOT+%22Boost+Software+License%22=Code

> The combination of having an uninitialized variable and explicitly marking it 
> unused like this seems to me like it would be unusual.

It could be a variable used exclusively inside an `assert` or some other 
conditionally no-op macro-function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79895



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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

> This warning can be turned off by the flag 
> `-Wno-uninitialized-const-reference`.

Suggesting to turn off the warning should be the last resort. I am pointing to 
the false positives for large existing code bases from `-Wall` diagnostic.

> I don't think we can just make the diagnostic not fire for empty body 
> consuming functions, if the function declaration and definition are in 
> different translation units.

I am talking about the particular situation that is involves only inline 
functions with empty bodies. `boost::ignore_unused`-like functions are 
obviously come with definition.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79895



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

[PATCH] D79895: Add a new warning to warn when passing uninitialized variables as const reference parameters to a function

[Nikita Kniazev via Phabricator via cfe-commits]

nick added a comment.

This diagnostic bring headaches because frequently `-Wunused-variable` 
suppression is done via no-op pseudo-consuming function like 
`boost::ignore_unused` 
.
 Particularly, it fires in Boost here 
https://github.com/boostorg/concept_check/blob/e69c81326d5a4359ac53f9c6fe53fc2baf24df50/include/boost/concept_check.hpp#L135-L141.
Is it possible to make the diagnostic not fire for empty body consuming 
functions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79895



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

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 225783.
nick added a comment.

Fixed typo.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,14 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +345,14 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -678,6 +678,29 @@
   "@interface Z : A @end", ZIsDirectlyDerivedFromX));
 }
 
+TEST(DeclarationMatcher, ClassHasBase) {
+  DeclarationMatcher ClassHasAnyBase =
+  cxxRecordDecl(hasBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyBase));
+
+  TypeMatcher ClassX = asString("class X");
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : X, Y {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : Y, X {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class W {}; class Y {}; class Z : W, Y {};",
+ ClassHasBaseClassX));
+  DeclarationMatcher ClassZHasBaseClassX =
+  cxxRecordDecl(hasName("Z"), hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {}; class Z : X {};",
+  ClassZHasBaseClassX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : X {}; class Z : Y {};",
+ ClassZHasBaseClassX));
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -112,6 +112,7 @@
   VERIFY_NAME(NestedNameSpecifierLoc);
   VERIFY_NAME(QualType);
   VERIFY_NAME(TypeLoc);
+  VERIFY_NAME(CXXBaseSpecifier);
   VERIFY_NAME(CXXCtorInitializer);
   VERIFY_NAME(NestedNameSpecifier);
   VERIFY_NAME(Decl);
@@ -141,6 +142,15 @@
   EXPECT_TRUE(Verifier.match("void f() {}", typeLoc(loc(functionType();
 }
 
+#if 0 // Cannot be used as a top level matcher at the time
+TEST(DynTypedNode, CXXBaseSpecifierSourceRange) {
+  RangeVerifier Verifier;
+  Verifier.expectRange(1, 23, 1, 39);
+  EXPECT_TRUE(Verifier.match("class B {}; class C : public virtual B {};",
+ cxxBaseSpecifier()));
+}
+#endif
+
 TEST(DynTypedNode, NNSLocSourceRange) {
   RangeVerifier Verifier;
   Verifier.expectRange(1, 33, 1, 34);
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ 

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 225770.
nick added a comment.

Regenerated the docs, added more tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69218

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -320,6 +320,14 @@
 varDecl(hasType(pointsTo(ClassX);
 }
 
+TEST(HasType, TakesQualTypeMatcherAndMatchesCXXBaseSpecifier) {
+  DeclarationMatcher ClassX = recordDecl(hasName("X"));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, TakesDeclMatcherAndMatchesExpr) {
   DeclarationMatcher ClassX = recordDecl(hasName("X"));
   EXPECT_TRUE(
@@ -337,6 +345,14 @@
 notMatches("class X {}; void y() { X *x; }", varDecl(hasType(ClassX;
 }
 
+TEST(HasType, TakesDeclMatcherAndMatchesCXXBaseSpecifier) {
+  TypeMatcher ClassX = hasDeclaration(recordDecl(hasName("X")));
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", ClassHasBaseClassX));
+}
+
 TEST(HasType, MatchesTypedefDecl) {
   EXPECT_TRUE(matches("typedef int X;", typedefDecl(hasType(asString("int");
   EXPECT_TRUE(matches("typedef const int T;",
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -678,6 +678,29 @@
   "@interface Z : A @end", ZIsDirectlyDerivedFromX));
 }
 
+TEST(DeclarationMatcher, ClassHasBase) {
+  DeclarationMatcher ClassHasAnyBase =
+  cxxRecordDecl(hasBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(notMatches("class X {};", ClassHasAnyBase));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", ClassHasAnyBase));
+
+  TypeMatcher ClassX = asString("class X");
+  CXXBaseSpecifierMatcher BaseClassX = cxxBaseSpecifier(hasType(ClassX));
+  DeclarationMatcher ClassHasBaseClassX = cxxRecordDecl(hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : X, Y {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y {}; class Z : Y, X {};",
+  ClassHasBaseClassX));
+  EXPECT_TRUE(notMatches("class W {}; class Y {}; class Z : W, Y {};",
+ ClassHasBaseClassX));
+  DeclarationMatcher ClassZHasBaseClassX =
+  cxxRecordDecl(hasName("Z"), hasBase(BaseClassX));
+  EXPECT_TRUE(matches("class X {}; class Y : X {}; class Z : X {};",
+  ClassZHasBaseClassX));
+  EXPECT_TRUE(notMatches("class X {}; class Y : X {}; class Z : Y {};",
+ ClassZHasBaseClassX));
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -112,6 +112,7 @@
   VERIFY_NAME(NestedNameSpecifierLoc);
   VERIFY_NAME(QualType);
   VERIFY_NAME(TypeLoc);
+  VERIFY_NAME(CXXBaseSpecifier);
   VERIFY_NAME(CXXCtorInitializer);
   VERIFY_NAME(NestedNameSpecifier);
   VERIFY_NAME(Decl);
@@ -141,6 +142,15 @@
   EXPECT_TRUE(Verifier.match("void f() {}", typeLoc(loc(functionType();
 }
 
+#if 0 // Cannot be used as a top level matcher at the time
+TEST(DynTypedNode, CXXBaseSpecifierSourceRange) {
+  RangeVerifier Verifier;
+  Verifier.expectRange(1, 23, 1, 39);
+  EXPECT_TRUE(Verifier.match("class B {}; class C : public virtual B {};",
+ cxxBaseSpecifier()));
+}
+#endif
+
 TEST(DynTypedNode, NNSLocSourceRange) {
   RangeVerifier Verifier;
   Verifier.expectRange(1, 33, 1, 34);
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- 

[PATCH] D69218: [clang][AST] Add `CXXBaseSpecifier` matcher support

[Nikita Kniazev via Phabricator via cfe-commits]

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

Required for capturing base specifier in matchers:

  `cxxRecordDecl(hasBase(cxxBaseSpecifier().bind("base")))`

Will make implementation of D69000  much 
clearer and simpler.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69218

Files:
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/unittests/AST/ASTTypeTraitsTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -678,6 +678,17 @@
   "@interface Z : A @end", ZIsDirectlyDerivedFromX));
 }
 
+TEST(DeclarationMatcher, ClassHasBase) {
+  DeclarationMatcher HasAnyBase = cxxRecordDecl(hasBase(cxxBaseSpecifier()));
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", HasAnyBase));
+  EXPECT_TRUE(notMatches("class X {};", HasAnyBase));
+
+  DeclarationMatcher HasBaseX =
+  cxxRecordDecl(hasBase(cxxBaseSpecifier(hasType(asString("class X");
+  EXPECT_TRUE(matches("class X {}; class Y : X {};", HasBaseX));
+  EXPECT_TRUE(notMatches("class Z {}; class Y : Z {};", HasBaseX));
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/unittests/AST/ASTTypeTraitsTest.cpp
===
--- clang/unittests/AST/ASTTypeTraitsTest.cpp
+++ clang/unittests/AST/ASTTypeTraitsTest.cpp
@@ -112,6 +112,7 @@
   VERIFY_NAME(NestedNameSpecifierLoc);
   VERIFY_NAME(QualType);
   VERIFY_NAME(TypeLoc);
+  VERIFY_NAME(CXXBaseSpecifier);
   VERIFY_NAME(CXXCtorInitializer);
   VERIFY_NAME(NestedNameSpecifier);
   VERIFY_NAME(Decl);
@@ -141,6 +142,15 @@
   EXPECT_TRUE(Verifier.match("void f() {}", typeLoc(loc(functionType();
 }
 
+#if 0 // Cannot be used as a top level matcher at the time
+TEST(DynTypedNode, CXXBaseSpecifierSourceRange) {
+  RangeVerifier Verifier;
+  Verifier.expectRange(1, 23, 1, 39);
+  EXPECT_TRUE(Verifier.match("class B {}; class C : public virtual B {};",
+ cxxBaseSpecifier()));
+}
+#endif
+
 TEST(DynTypedNode, NNSLocSourceRange) {
   RangeVerifier Verifier;
   Verifier.expectRange(1, 33, 1, 34);
Index: clang/lib/AST/ASTTypeTraits.cpp
===
--- clang/lib/AST/ASTTypeTraits.cpp
+++ clang/lib/AST/ASTTypeTraits.cpp
@@ -27,6 +27,7 @@
   { NKI_None, "NestedNameSpecifierLoc" },
   { NKI_None, "QualType" },
   { NKI_None, "TypeLoc" },
+  { NKI_None, "CXXBaseSpecifier" },
   { NKI_None, "CXXCtorInitializer" },
   { NKI_None, "NestedNameSpecifier" },
   { NKI_None, "Decl" },
@@ -163,6 +164,8 @@
 }
 
 SourceRange DynTypedNode::getSourceRange() const {
+  if (const CXXBaseSpecifier *CBS = get())
+return CBS->getSourceRange();
   if (const CXXCtorInitializer *CCI = get())
 return CCI->getSourceRange();
   if (const NestedNameSpecifierLoc *NNSL = get())
Index: clang/include/clang/ASTMatchers/ASTMatchersInternal.h
===
--- clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -130,6 +130,9 @@
 return TSI->getType();
   return QualType();
 }
+inline QualType getUnderlyingType(const CXXBaseSpecifier ) {
+  return Node.getType();
+}
 
 /// Unifies obtaining the FunctionProtoType pointer from both
 /// FunctionProtoType and FunctionDecl nodes..
@@ -926,6 +929,7 @@
   std::is_same::value ||
   std::is_same::value ||
   std::is_same::value ||
+  std::is_same::value ||
   std::is_same::value;
 };
 template 
@@ -1094,7 +1098,7 @@
 /// Useful for matchers like \c anything and \c unless.
 using AllNodeBaseTypes =
 TypeList;
+ Type, TypeLoc, CXXBaseSpecifier, CXXCtorInitializer>;
 
 /// Helper meta-function to extract the argument out of a function of
 ///   type void(Arg).
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -140,6 +140,7 @@
 using TypeLocMatcher = internal::Matcher;
 using NestedNameSpecifierMatcher = internal::Matcher;
 using NestedNameSpecifierLocMatcher = internal::Matcher;
+using CXXBaseSpecifierMatcher = internal::Matcher;
 using CXXCtorInitializerMatcher = internal::Matcher;
 /// @}
 
@@ -468,6 +469,16 @@
 extern const internal::VariadicDynCastAllOfMatcher
 

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

[Nikita Kniazev via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.cpp:218
+  // Requires C++.
+  if (!getLangOpts().CPlusPlus)
+return;

Eugene.Zelenko wrote:
> Should it check for C++17 or newer?
I do not think so. It is a backward compatible change. The standard never 
required to inherit iterators from the `std::iterator`, and the standard 
library itself had not inheriting from `std::iterator` iterators.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69000



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

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

[Nikita Kniazev via Phabricator via cfe-commits]

nick updated this revision to Diff 225133.
nick added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69000

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-iterator-base.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s modernize-deprecated-iterator-base %t 
+
+namespace std {
+using ptrdiff_t = int;
+struct input_iterator_tag;
+template 
+struct iterator {
+  using iterator_category = C;
+  using value_type= T;
+  using difference_type   = D;
+  using pointer   = P;
+  using reference = R;
+};
+}
+
+
+using iterator_alias = std::iterator;
+typedef std::iterator iterator_typedef;
+
+
+// Sugar
+
+// CHECK-FIXES: struct from_alias {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct from_alias: iterator_alias {};
+
+// CHECK-FIXES: struct from_typedef {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct from_typedef: iterator_typedef {};
+
+
+// False-positive
+
+// CHECK-FIXES: struct indirect_base: from_alias {};
+// CHECK-MESSAGES-NOT: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct indirect_base: from_alias {};
+
+
+// Unsupported
+
+// CHECK-FIXES: class skipif_non_public_inheritance: iterator_alias {};
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+class skipif_non_public_inheritance: iterator_alias {};
+
+
+// Base removal
+
+struct A {};
+struct B {};
+
+struct collection {
+  template 
+  struct iterator;
+};
+
+// CHECK-FIXES: template <> struct collection::iterator<> {
+// CHECK-MESSAGES: :[[@LINE+1]]:45: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator<> : iterator_alias {};
+// CHECK-FIXES: template <> struct collection::iterator : A {
+// CHECK-MESSAGES: :[[@LINE+1]]:49: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : A, iterator_alias {};
+// CHECK-FIXES: template <> struct collection::iterator : B {
+// CHECK-MESSAGES: :[[@LINE+1]]:46: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : iterator_alias, B {};
+// CHECK-FIXES: template <> struct collection::iterator : A, B {
+// CHECK-MESSAGES: :[[@LINE+1]]:52: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : A, iterator_alias, B {};
+
+// CHECK-FIXES: struct do_not_strip_final final {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct do_not_strip_final final : iterator_alias {};
+
+// CHECK-FIXES: struct iteratorZ // iterator_alias
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+2]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorZ   // iteratorZ
+  : iterator_alias // iterator_alias
+{};
+// CHECK-FIXES: struct iteratorA   // iteratorA
+// CHECK-FIXES:   : A // iterator_alias
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+3]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorA   // iteratorA
+  : A  // A
+  , iterator_alias // iterator_alias
+{};
+// CHECK-FIXES: struct iteratorB   // iteratorB
+// CHECK-FIXES:   : B  // B
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+2]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorB   // iteratorB
+  : iterator_alias // iterator_alias
+  , B  // B
+{};
+// CHECK-FIXES: struct iteratorAB  // iteratorAB
+// CHECK-FIXES:   : A  // A
+// CHECK-FIXES:   , B  // B
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+3]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorAB  // iteratorAB
+  : A  

[PATCH] D69000: [clang-tidy] new check: modernize-deprecated-iterator-base

[Nikita Kniazev via Phabricator via cfe-commits]

nick created this revision.
nick added a reviewer: alexfh.
nick added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, kristof.beyls, xazax.hun, mgorny.
Herald added a project: clang.

Finds deprecated in C++17 inheritance from `std::iterator` and replaces it with 
type aliases. The check itself is not marked as C++17 because it could be 
applied to any project with no harm.

---

Because of need to catch `CXXBaseSpecifier` I extended 
`clang::ast_type_traits`, but locally and in a hacky way. I would be glad if 
someone who knows `ASTMatcher` internals do add a proper `CXXBaseSpecifier` 
support.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69000

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.cpp
  clang-tools-extra/clang-tidy/modernize/DeprecatedIteratorBaseCheck.h
  clang-tools-extra/clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/modernize-deprecated-iterator-base.rst
  
clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-deprecated-iterator-base.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s modernize-deprecated-iterator-base %t 
+
+namespace std {
+using ptrdiff_t = int;
+struct input_iterator_tag;
+template 
+struct iterator {
+  using iterator_category = C;
+  using value_type= T;
+  using difference_type   = D;
+  using pointer   = P;
+  using reference = R;
+};
+}
+
+
+using iterator_alias = std::iterator;
+typedef std::iterator iterator_typedef;
+
+
+// Sugar
+
+// CHECK-FIXES: struct from_alias {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct from_alias: iterator_alias {};
+
+// CHECK-FIXES: struct from_typedef {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct from_typedef: iterator_typedef {};
+
+
+// False-positive
+
+// CHECK-FIXES: struct indirect_base: from_alias {};
+// CHECK-MESSAGES-NOT: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct indirect_base: from_alias {};
+
+
+// Unsupported
+
+// CHECK-FIXES: class skipif_non_public_inheritance: iterator_alias {};
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+class skipif_non_public_inheritance: iterator_alias {};
+
+
+// Base removal
+
+struct A {};
+struct B {};
+
+struct collection {
+  template 
+  struct iterator;
+};
+
+// CHECK-FIXES: template <> struct collection::iterator<> {
+// CHECK-MESSAGES: :[[@LINE+1]]:45: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator<> : iterator_alias {};
+// CHECK-FIXES: template <> struct collection::iterator : A {
+// CHECK-MESSAGES: :[[@LINE+1]]:49: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : A, iterator_alias {};
+// CHECK-FIXES: template <> struct collection::iterator : B {
+// CHECK-MESSAGES: :[[@LINE+1]]:46: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : iterator_alias, B {};
+// CHECK-FIXES: template <> struct collection::iterator : A, B {
+// CHECK-MESSAGES: :[[@LINE+1]]:52: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+template <> struct collection::iterator : A, iterator_alias, B {};
+
+// CHECK-FIXES: struct do_not_strip_final final {
+// CHECK-MESSAGES: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct do_not_strip_final final : iterator_alias {};
+
+// CHECK-FIXES: struct iteratorZ // iterator_alias
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+2]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorZ   // iteratorZ
+  : iterator_alias // iterator_alias
+{};
+// CHECK-FIXES: struct iteratorA   // iteratorA
+// CHECK-FIXES:   : A // iterator_alias
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+3]]:5: warning: inheriting from 'std::iterator' is deprecated [modernize-deprecated-iterator-base]
+struct iteratorA   // iteratorA
+  : A  // A
+  , iterator_alias // iterator_alias
+{};
+// CHECK-FIXES: struct iteratorB   // iteratorB
+// CHECK-FIXES:   : B  // B
+// CHECK-FIXES: {
+// CHECK-MESSAGES: :[[@LINE+2]]:5: warning: inheriting from 'std::iterator' is