aaron.ballman added inline comments.
================
Comment at: clang-tidy/misc/CopyConstructorInitCheck.cpp:104
+
+ diag(Tok.getLocation(),
+ "calling an inherited constructor other than the copy constructor")
----------------
szdominik wrote:
> aaron.ballman wrote:
> > Insteaad of having to re-lex the physical source, can the AST should be
> > modified to carry the information you need if it doesn't already have it?
> > For instance, you can tell there is not initializer list by looking at
> > `CXXConstructorDecl::getNumCtorInitializers()`.
> The getNumCtorInitializers method counts the generated initializers as well,
> not just the manually written ones.
> My basic problem was that the AST's methods can't really distinguish the
> generated and the manually written initializers, so it's quite complicated to
> find the locations what we need. I found easier & more understandable if I
> start reading the tokens.
This sounds like a deficiency with the AST that should be rectified rather than
worked around. Going back to lexing the source can be very expensive (think
about source files that live on a network drive, for instance) and is often
tricky to get correct. For instance, it seems the lexing starts at the
constructor declaration itself, so does a default argument to that copy
constructor using `?:` cause issues? e.g., `S(const S&, int = 0 ? 1 : 2)`
================
Comment at: test/clang-tidy/misc-copy-constructor-init.cpp:27
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: calling an inherited
constructor other than the copy constructor [misc-copy-constructor-init]
+ // CHECK-FIXES: X3(const X3& other): Copyable2(other), Copyable(other)
{};
+};
----------------
szdominik wrote:
> aaron.ballman wrote:
> > Don't we want the ctor-inits to be in the same order as the bases are
> > specified?
> I think it's more readable if we put the FixIts to the beginning of the
> expression - it's easier to check that everyting is correct & it's more
> obvious what the changes are.
However, that then produces additional warnings because the ctor-inits are not
in the canonical order (`-Wreorder`). See
http://coliru.stacked-crooked.com/a/a9d77afe87618c13
================
Comment at: test/clang-tidy/misc-copy-constructor-init.cpp:9
+class X : public Copyable {
+ X(const X& other) : Copyable(other) {}
+ //Good code: the copy ctor call the ctor of the base class.
----------------
Please clang-format this file so it meets our usual formatting requirements.
================
Comment at: test/clang-tidy/misc-copy-constructor-init.cpp:19
+class X2 : public Copyable2 {
+ X2(const X2& other) {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: calling an inherited
constructor other than the copy constructor [misc-copy-constructor-init]
----------------
Spurious semicolon.
================
Comment at: test/clang-tidy/misc-copy-constructor-init.cpp:25
+class X3 : public Copyable, public Copyable2 {
+ X3(const X3& other): Copyable(other) {};
+ // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: calling an inherited
constructor other than the copy constructor [misc-copy-constructor-init]
----------------
Spurious semicolon (check the remainder of the file, this seems to be a common
issue).
https://reviews.llvm.org/D33722
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
