[PATCH] D33722: [clang-tidy] Add checker for undelegated copy of base classes

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 3 12:15:08 PDT 2017


aaron.ballman added a comment.

In https://reviews.llvm.org/D33722#915134, @xazax.hun wrote:

> Two problems are not resolved. One is Aaron prefers to query some infor from the AST instead of relexing. The second is providing base initializers in the wrong order.
>  I think there are other checks that do relexing in some cases, this should not be a blocker.


In the other cases, it is not clear that the re-lexed information should be carried in the AST. In this case, I think it's pretty clear that the AST should carry this information. Further, I don't know that the re-lexing is correct (I pointed out an example that I think will be problematic) and carrying the information in the AST would solve that more cleanly than trying to reimplement the logic here.

> I am not sure that we should complicate the fixit logic with the order. If -Wreorder has fixit hints, user should be able to get rid of the warning by applying that.

I disagree. We should not be changing code to be incorrect, requiring the user to find that next incorrectness only through an additional compilation. That is a rather poor user experience.


https://reviews.llvm.org/D33722





More information about the cfe-commits mailing list