xbolva00 added a comment. Ping @arthur.j.odwyer
================ Comment at: clang/include/clang/Basic/DiagnosticGroups.td:158 +def DeprecatedCopy : DiagGroup<"deprecated-copy", [DeprecatedCopyUserProvided]>; +def DeprecatedCopyDtor : DiagGroup<"deprecated-copy-dtor", [DeprecatedCopyDtorUserProvided]>; def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">; ---------------- Quuxplusone wrote: > xbolva00 wrote: > > Quuxplusone wrote: > > > If we're going to provide these options at all, I think it would be more > > > grammatical to call them `-Wdeprecated-copy-with-deleted-copy` and > > > `-Wdeprecated-copy-with-deleted-dtor`. > > > > > > The existing code is already confusing by talking about a "copy dtor" as > > > if that's a thing; I think it makes it even worse to talk about > > > "deprecated copy user provided," since the actually deprecated thing is > > > the //implicitly generated// copy. > > > > > > I get that we're trying to be terse, and also somewhat hierarchical in > > > putting all the `-Wdeprecated-copy`-related warnings under > > > `-Wdeprecated-copy-foo-bar`; but the current names are too far into the > > > ungrammatical realm for me personally. > > Yeah, better names are welcome :) > Do the current names match existing practice in GCC or anything? > I continue to opine that these options are poorly named. My best suggestion is > `deprecated-copy`, `deprecated-copy-with-dtor`, > `deprecated-copy-with-deleted-copy`, `deprecated-copy-with-deleted-dtor` — > operating on the (wrong?) assumption that absolutely the only difference > between "user-declared" and "user-provided" corresponds to "user-declared as > deleted." > > Even if everything else remains the same, the internal identifier > `warn_deprecated_copy_dtor_operation_user_provided` should certainly be > `warn_deprecated_copy_operation_dtor_user_provided` — the phrase that goes > together is "copy operation", not "dtor operation". > > Your "deprecated-dtor-user-provided.cpp" passes > `-Wdeprecated-copy-dtor-user-provided`, but the message text talks about > "user-//declared//." Shouldn't the spelling of the option reflect the wording > of the message text? This isn't important from the POV of someone who's just > going to look at the message text and fix their code, but it's important from > the POV of someone who's going to use `-Wno-` and wants to know exactly which > bad situations they're ignoring. (Also, the name of the file should match the > spelling of the option.) Yeah, deprecated-copy and deprecated-copy-dtor is already defined by GCC. I think deprecated-copy-user-provided-copy and deprecated-copy-user-provided-dtor could be good solution here, it is also quite easy to follow implementation of the checking logic in Sema with these names. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79714/new/ https://reviews.llvm.org/D79714 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
