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

Gábor Horváth via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 6 01:45:18 PST 2017


xazax.hun added a comment.

In https://reviews.llvm.org/D33722#915440, @aaron.ballman wrote:

> 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.


Oh, you are right! Sorry for my oversight, I did not notice your example. I got a bit lost in all of the comments. Fortunately, I was able to get rid of the relexing, so this should be ok now.

>> 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.

The resulting code is not incorrect. We do not introduce undefined behavior, nor change the meaning of the code. But if we do not want to introduce new warnings either, I could skip the fixits if there is an initializer already for any of the base classes. What do you think?


https://reviews.llvm.org/D33722





More information about the cfe-commits mailing list