aaron.ballman added a comment.
In http://reviews.llvm.org/D11784#224386, @alexfh wrote:
> A high level question: why is this check specific to base class
> initialization? Isn't the same issue possible when initializing members, for
> example? What would change if in your example `D` would have a member of type
> `B` instead of deriving from it?
Hmmm, I suppose this would apply equally to member initializers as well.
I was envisioning this as a strange dichotomy issue more than a performance
issue. The caller goes to the trouble of move constructing, and you only
half-move construct by calling the base class copy constructor which can
potentially break class hierarchy invariants.
However, I think that's overthinking the situation and I should warn on member
initialization that follows the same pattern.
> > One thing I am not certain of in this patch is how to test it. I have some
> > rudimentary tests, but am unable to test the "note:" diagnostics from
>
> > FileCheck (attempting to add any cause the "warning:" diagnostics to not
> > be found).
>
>
> Can you give an example of what you do and what results do you get?
I put "CHECK: :[[@LINE+1]]:3: note: copy constructor being called" into the
source file, and tests no longer pass because it cannot find the matching
"warning: " diagnostic. If I then remove the warning diagnostic, the tests pass
again. So it seems I can test one or the other, but not both. Specifically
(with note and warning):
E:\llvm\2015>clang-tidy
..\llvm\tools\clang\tools\extra\test\clang-tidy\misc-move-constructor-base.cpp
-checks=-*,misc-move-constructor-base -- -std=c++14 | FileCheck
..\llvm\tools\clang\tools\extra\test\clang-tidy\misc-move-constructor-base.cpp
2 warnings generated.
..\llvm\tools\clang\tools\extra\test\clang-tidy\misc-move-constructor-base.cpp:27:12:
error: expected string not found in input
// CHECK: :[[@LINE+1]]:16: warning: move constructor initializes base class by
calling a copy constructor [misc-move-constructor-base]
^
<stdin>:5:2: note: scanning from here
B(const B&) {}
^
<stdin>:5:2: note: with expression "@LINE+1" equal to "28"
B(const B&) {}
^
<stdin>:10:94: note: possible intended match here
E:\llvm\2015\..\llvm\tools\clang\tools\extra\test\clang-tidy\misc-move-constructor-base.cpp:70:16:
warning: move constructor initializes base class by calling a copy constructor
[misc-move-constructor-base]
^
with just note, no warning:
E:\llvm\2015>clang-tidy
..\llvm\tools\clang\tools\extra\test\clang-tidy\misc-move-constructor-base.cpp
-checks=-*,misc-move-constructor-base -- -std=c++14 | FileCheck
..\llvm\tools\clang\tools\extra\test\clang-tidy\misc-move-constructor-base.cpp
2 warnings generated.
~Aaron
>
>
> > I suspect this is why clang-tidy.sh exists, but unfortunately, that means
> > the tests will not be run on Windows (which is the platform I am
>
> > developing on). Suggestions on how to improve the tests are welcome, but
> > for now, I'm not testing the note diagnostics.
>
>
> Converting the script to python seems like the most universal approach.
> Should be trivial with the `sh` python package, but I'm not sure whether we
> can rely on it being installed.
http://reviews.llvm.org/D11784
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
