[PATCH] PR16182 - Visit parameter declaration of implicitly declared copy assignment operator.

Richard Smith richard at metafoo.co.uk
Tue Sep 10 14:48:16 PDT 2013



================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1779-1786
@@ -1778,3 +1778,10 @@
     TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
-  }
+  } else if (getDerived().shouldVisitImplicitCode())
+    // Visit parameter variable declarations of the implicit function
+    // if the traverser is visiting implicit code. Parameter variable
+    // declarations do not have valid TypeSourceInfo, so to visit them
+    // we need to traverse the declarations explicitly.
+    for (FunctionDecl::param_const_iterator I = D->param_begin(),
+                                            E = D->param_end(); I != E; ++I)
+      TRY_TO(TraverseDecl(*I));
 
----------------
Please add braces around this (7 lines is a bit much with no braces, and this is inconsistent with the other arm of this 'if').

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:1364
@@ -1363,1 +1363,3 @@
       matches("class X {}; void y(X y) { const X &x = y; }", ReferenceClassX));
+  // The match here is on the implicit copy assignment operator code for
+  // class X, not on code 'X x = y'.
----------------
Nit: it's on the copy constructor, not the copy assignment operator.

================
Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:1366-1367
@@ -1364,3 +1365,4 @@
+  // class X, not on code 'X x = y'.
   EXPECT_TRUE(
-      notMatches("class X {}; void y(X y) { X x = y; }", ReferenceClassX));
+      matches("class X {}; void y(X y) { X x = y; }", ReferenceClassX));
   EXPECT_TRUE(
----------------
It would be great to preserve the intent of this test better (that `ReferenceClassX` does not match a declaration of a variable of type `X`). Maybe add something like:

  EXPECT_TRUE(notMatches("class X {}; extern X x;", ReferenceClassX));


http://llvm-reviews.chandlerc.com/D958



More information about the cfe-commits mailing list