[PATCH] Clang-Modernize.Pass-By-Value: Fix removing of namespace in parameter type

[Guillaume Papin]

Hi revane, arielbernal, tareqsiraj,

Fixes for CM-139 (https://cpp11-migrate.atlassian.net/browse/CM-139):
- Handle extraction of the value-type for a const-ref correctly (the old version was too greedy, for example it used to remove the nested name specifier when the type was a value-type)
- Add a whitespace only when necessary
- make the test more strict regarding to whitespaces
- add a test failure + a FIXME in the code about const-value parameters

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

Files:
  clang-modernize/PassByValue/PassByValueActions.cpp
  test/clang-modernize/PassByValue/basic.cpp
  test/clang-modernize/PassByValue/removing_const_fail.cpp

Index: clang-modernize/PassByValue/PassByValueActions.cpp
===================================================================
--- clang-modernize/PassByValue/PassByValueActions.cpp
+++ clang-modernize/PassByValue/PassByValueActions.cpp
@@ -120,9 +120,13 @@
     CharSourceRange CharRange = Lexer::makeFileCharRange(
         CharSourceRange::getTokenRange(Range), SM, LangOptions());
     TypeLoc ValueTypeLoc = ParamTL;
+
     // transform non-value parameters (e.g: const-ref) to values
-    if (!ParamTL.getNextTypeLoc().isNull())
-      ValueTypeLoc = ParamTL.getNextTypeLoc();
+    ReferenceTypeLoc RefTL = ParamTL.getAs<ReferenceTypeLoc>();
+    if (!RefTL.isNull())
+      ValueTypeLoc = RefTL.getPointeeLoc();
+    // FIXME: drop the 'const' qualifier on const-value parameters
+
     llvm::SmallString<32> ValueStr = Lexer::getSourceText(
         CharSourceRange::getTokenRange(ValueTypeLoc.getSourceRange()), SM,
         LangOptions());
@@ -133,9 +137,14 @@
         !Owner.isFileModifiable(SM, CharRange.getBegin()))
       return;
 
+    // Append a space when necessary (e.g: before a parameter)
     // 'const Foo &param' -> 'Foo param'
     //  ~~~~~~~~~~~           ~~~^
-    ValueStr += ' ';
+    SourceLocation NextChar =
+        Lexer::getLocForEndOfToken(ParamTL.getEndLoc(), 0, SM, LangOptions());
+    if (isIdentifierHead(*FullSourceLoc(NextChar, SM).getCharacterData()))
+      ValueStr += ' ';
+
     ParamReplaces[I] = Replacement(SM, CharRange, ValueStr);
   }
 
Index: test/clang-modernize/PassByValue/basic.cpp
===================================================================
--- test/clang-modernize/PassByValue/basic.cpp
+++ test/clang-modernize/PassByValue/basic.cpp
@@ -72,11 +72,17 @@
 // Test unnamed parameter in declaration
 struct G {
   G(const Movable &);
-  // CHECK: G(Movable );
+  // CHECK: G(Movable);
+
+  // Test that a space is not added unnecessarily before a comma
+  G(const Movable &, int);
+  // CHECK: G(Movable, int);
   Movable M;
 };
 G::G(const Movable &M) : M(M) {}
 // CHECK: G::G(Movable M) : M(std::move(M)) {}
+G::G(const Movable &M, int) : M(M) {}
+// CHECK: G::G(Movable M, int) : M(std::move(M)) {}
 
 // Test parameter with and without qualifier
 namespace ns_H {
@@ -166,3 +172,13 @@
   Movable C;
   double D;
 };
+
+// Test that value-parameters with a nested name specifier are left as-is
+namespace ns_R {
+typedef ::Movable RMovable;
+}
+struct R {
+  R(ns_R::RMovable M) : M(M) {}
+  // CHECK: R(ns_R::RMovable M) : M(std::move(M)) {}
+  ns_R::RMovable M;
+};
Index: test/clang-modernize/PassByValue/removing_const_fail.cpp
===================================================================
--- /dev/null
+++ test/clang-modernize/PassByValue/removing_const_fail.cpp
@@ -0,0 +1,13 @@
+// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.cpp
+// RUN: cpp11-migrate -pass-by-value %t.cpp -- -std=c++11 -fno-delayed-template-parsing -I %S
+// RUN: FileCheck --strict-whitespace -input-file=%t.cpp %s
+// XFAIL: *
+
+#include "basic.h"
+
+// Test with const on the right side of the type
+struct A {
+  A(Movable const M) : M(M) {}
+  // CHECK: A(Movable M) : M(std::move(M)) {}
+  Movable M;
+};
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1600.1.patch
Type: text/x-patch
Size: 3161 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130904/ae57e986/attachment.bin>