gribozavr2 added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525 + return "; cannot be fixed because '" + Fixup + + "' is not a valid identifier"; ---------------- njames93 wrote: > gribozavr2 wrote: > > njames93 wrote: > > > gribozavr2 wrote: > > > > I don't think we need to tell that to the user. When Clang can't > > > > provide a fix, it just silently omits it. It does not help the user to > > > > know. that Clang tried, but failed. > > > > > > > > (This message could be also read as "this code is so bad it can't be > > > > fixed"...) > > > So just return an empty string. > > > The user will see the normal diagnostic message but there will be no > > > fix-its available. > > Agreed. > Maybe a slightly better idea is to use the same message as the empty fix up > case > `; cannot be fixed automatically`. WDYT? > Oh I see, this checker has a number of messages like that... Well, feel free to keep it, but what seems weird to me as a user is that for an input name of "_0Bad" the checker automatically chose somehow "0_Bad" (why?), and then concluded that it is not a good choice. This looks like irrelevant information because the new name is weird. I can totally see how reminding the user that "Break"->"break" is not easily possible because the latter is a keyword. Feel free to ignore me since this checker already produces messages like this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91915/new/ https://reviews.llvm.org/D91915 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
