[PATCH] Fix useAuto replacing variable declaration lists containing non-initialized variables.

Ariel Bernal ariel.j.bernal at intel.com
Thu May 16 21:05:08 PDT 2013



================
Comment at: cpp11-migrate/UseAuto/UseAuto.cpp:43
@@ -42,3 +42,3 @@
 
-  Finder.addMatcher(makeIteratorDeclMatcher(), &ReplaceIterators);
+  Finder.addMatcher(makeDeclStatmentMatcher(), &ReplaceIterators);
   Finder.addMatcher(makeDeclWithNewMatcher(), &ReplaceNew);
----------------
Edwin Vane wrote:
> Please keep this name the same. We're still matching iterator declarations.
done

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:160
@@ -159,2 +159,3 @@
 
+
 } // namespace ast_matchers
----------------
Edwin Vane wrote:
> Can remove this extra blank line.
done

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:238
@@ +237,3 @@
+  return declStmt(
+    has(varDecl()),
+    unless(
----------------
Edwin Vane wrote:
> What's the purpose of this part?
I can add a comment if you think it is necessary. We have to explicitly say that there's at least one varDecl in the declStmt meaning that the declStmt is a declaration list, otherwise we could match declaration statements that don't have any varDecl within e.g. using directives or typedefs.

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:239
@@ +238,3 @@
+    has(varDecl()),
+    unless(
+      has(
----------------
Edwin Vane wrote:
> I know there's not really any guidelines for indenting these matchers but since `unless` has only one child, I'd join `unless(has(varDecl(` all on one line.
agree it makes sense. done

================
Comment at: cpp11-migrate/UseAuto/UseAutoMatchers.cpp:245
@@ +244,3 @@
+            hasType(autoType()),
+            unless(
+              hasType(
----------------
Edwin Vane wrote:
> Here too: `unless(hasType(`.
done

================
Comment at: test/cpp11-migrate/UseAuto/iterator.cpp:164
@@ -159,2 +163,3 @@
+  }
   return 0;
 }
----------------
Edwin Vane wrote:
> Can we have a positive test too?
> 
>   std::vector<int>::iterator I = Vec.begin(), E = Vec.end();
> 
I thought there was one but there's none, I'll add one, 


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



More information about the cfe-commits mailing list