[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