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

[Guillaume Papin]

================
Comment at: clang-modernize/PassByValue/PassByValueActions.cpp:143-146
@@ -137,2 +142,6 @@
     //  ~~~~~~~~~~~           ~~~^
-    ValueStr += ' ';
+    SourceLocation NextChar =
+        Lexer::getLocForEndOfToken(ParamTL.getEndLoc(), 0, SM, LangOptions());
+    if (isIdentifierHead(*FullSourceLoc(NextChar, SM).getCharacterData()))
+      ValueStr += ' ';
+
----------------
Edwin Vane wrote:
> Is all this extra work really necessary? Can't we rely on formatting to fix whitespace problems?
I agree with you but the bug report complained about an extra space when it was not necessary, so I figured out this solution wasn't too complicated and may produce correct code without reformatting needed most of the time (for people using `A(const Foo & f);` (a space after `&`).

The bug report complains about the case a param was already passed by value. I suppose I can handle this another way such as not replacing a parameter if it's already a value.

Anyway, looking at the grammar I realized the check isn't quite correct, we can have a parameter with default value but no name for the parameter, in these cases a space before `=` is probably wanted so we better let the reformatting do the smart stuff.

================
Comment at: test/clang-modernize/PassByValue/removing_const_fail.cpp:10
@@ +9,3 @@
+struct A {
+  A(Movable const M) : M(M) {}
+  // CHECK: A(Movable M) : M(std::move(M)) {}
----------------
Edwin Vane wrote:
> Why should this be transformed? It's not being passed by const reference.
It's still a valid candidate for the move, it's a parameter used only once and copied in class local storage.

I can skip this test the truth is this code will be changed. I can try to make this code invalid to a change but I think this solution will only make the code very-slightly more complex. TBH there are some other problem with const being on the right side (see CM-141).

I can remove this for the current patch and fix the issue related to const as a part of my fixes for CM-141.


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