aaron.ballman added a comment. In D117593#3274408 <https://reviews.llvm.org/D117593#3274408>, @njames93 wrote:
> In D117593#3273562 <https://reviews.llvm.org/D117593#3273562>, @aaron.ballman > wrote: > >> Hmmm, this is a rule for checking a specific style guide. >> https://google.github.io/styleguide/cppguide.html#Implicit_Conversions >> doesn't say that `explicit(false)` is a reasonable marking. In fact, it says >> "Implicit conversions can sometimes be necessary and appropriate for types >> that are designed to be interchangeable, for example when objects of two >> types are just different representations of the same underlying value. In >> that case, contact your project leads to request a waiver of this rule." >> >> So I think this behavior needs to be behind a flag so that the default >> behavior continues to match what the style guide requires (or the style >> guide should be updated to clarify the behavior of `explicit(false)`). > > My understanding of the rule(as a non Googler) was that `explicit(false)` is > an effective way to disable the rule by explicitly declaring the constructor > to be implicit. Which is much cleaner than throwing `NOLINT` markers about. FWIW, I agree that this would be a reasonable exception to the rule and makes for cleaner code than `NOLINT` comments. I'm pointing out that the rule text doesn't say that exceptions are something they want to silence diagnostics for. The way I read the guideline suggests that they don't consider the diagnostic a false positive. > In any case this c++20 syntax is not supported as the current fixit produces > invalid code, as evidenced in the initial bug report. Agreed that issue needs to be fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117593/new/ https://reviews.llvm.org/D117593 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
