[PATCH] Fix UseAuto replacing declaration lists with new expressions

Ariel Bernal ariel.j.bernal at intel.com
Wed May 22 07:01:23 PDT 2013



================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:97
@@ -118,1 +96,3 @@
+                                                  ->IgnoreParenImpCasts());
+    assert(NewExpr && "Bad Callback. No CXXNewExpr provided");
 
----------------
Edwin Vane wrote:
> Change the assert message. It's not a callback problem but rather a CXXNewExpr should be expected thanks to the matcher.
done, also changed the previous one since the VarDecl should be expected too.

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:104
@@ +103,3 @@
+
+    if (V->getType()->isPointerType()) {
+      // Find and remove the '*' from pointer types.
----------------
Edwin Vane wrote:
> This will always be true. What you really care about is VarDecls #>=2. Also, you only care if a star is actually involved. If you're dealing with a typedef it may not be:
> 
>   typedef int * int_p;
>   int_p a = new int, b = new int;
true, I'll change the logic, this will also fix the two spaces below

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:125
@@ +124,3 @@
+  // changing 'int *p' into autop'.
+  if(!V->getType()->isPointerType())
+    Replace.insert(tooling::Replacement(SM, Range, "auto"));
----------------
Edwin Vane wrote:
> Again, the type will always be a pointer. Check about typedefs?
yes the same for typedefs

================
Comment at: cpp11-migrate/UseAuto/UseAutoActions.cpp:128
@@ +127,3 @@
+  else
+    Replace.insert(tooling::Replacement(SM, Range, "auto  "));
+  ++AcceptedChanges;
----------------
Edwin Vane wrote:
> why two and not one?
fixed


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



More information about the cfe-commits mailing list