================
Comment at: cpp11-migrate/PassByValue/PassByValueActions.cpp:77
@@ +76,3 @@
+
+/// \brief Find all references to \p ParmVarDecls accross all of the
+/// constructors redeclarations.
----------------
Guillaume Papin wrote:
> Edwin Vane wrote:
> > Typo `ParmVarDecls` -> `ParamDecl`.
> Fixed.
> Now I'm wondering if there is a simple way to enable `-Wdocumentation` when
> building the migrator with CMake, I think it can catch these kind of issues.
Might be able to use cmake's -DCMAKE_CPP_FLAGS_* options. I just have Doxygen
installed locally and I run it looking for errors/warnings. In fact, you should
probably do this. Internally we have a continuous integration project that
builds docs nightly and it'll inform me if anything has been broken by commits
that day.
================
Comment at: test/cpp11-migrate/PassByValue/basic.cpp:100-106
@@ +99,8 @@
+// Test that templates aren't modified
+template <typename T> struct J {
+ J(const T &M) : M(M) {}
+ // CHECK: J(const T &M) : M(M) {}
+ T M;
+};
+J<Movable> j1(Movable());
+J<NotMovable> j2(NotMovable());
----------------
Guillaume Papin wrote:
> Edwin Vane wrote:
> > I know this makes sense but it's not mentioned anywhere in code comments or
> > in the documentation. You should probably update both.
> Documentation added to on the matcher to explain that this kind of templates
> aren't matched.
>
> I don't know what to write in the user documentation about this. Explaining
> what is changed makes sense but explaining every reason why we don't change
> something doesn't seem right to me. I think the transforms applied by the
> migrator should be something the people will be able to write themselves (an
> by extension understand).
>
> And now that I'm thinking about this exact example I believe it will benefit
> from using an universal reference here.
>
> template <typename T> struct J {
> J(T &&M) : M(M) {}
> T M;
> };
>
I agree generally that we don't have to talk about all the cases we don't
change things but templates are special. It's clear why the test case shouldn't
transform: the constructor arg type is a template parameter. But what about
this:
template <typename T>
struct K {
K(const Movable &M) : M(M) {}
Movable M;
T A;
};
This is clearly changeable but I'm pretty sure it won't be unless you
instantiate it. Then again, based on what you said in the matcher docs, perhaps
not. Can you try it? Add a test case for it?
btw, why would the test case benefit from a universal ref?
================
Comment at: docs/PassByValueTransform.rst:11
@@ +10,3 @@
+move constructors added for many types it is now interesting to take an
argument
+directly by value, instead of by const-reference and then copy. This
+transformation allows the compiler to take care of choosing the best way to
----------------
Need a comma after `const-reference`.
================
Comment at: docs/PassByValueTransform.rst:78-80
@@ +77,5 @@
+
+If the parameter is used more than once no transformation is performed since
+moved objects have an undefined state. It means the following code will be left
+untouched:
+
----------------
Right after this example would be a good place for the class template behaviour
blurb.
http://llvm-reviews.chandlerc.com/D1342
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
