rZhBoYao marked 6 inline comments as done. rZhBoYao added a comment. In D152632#4443284 <https://reviews.llvm.org/D152632#4443284>, @shafik wrote:
> I am wondering why we don't fold this into `-Wreserved-identifier` The "ud-suffix" of the user-defined-string-literal or the identifier in a literal-operator-id is called a literal suffix identifier. However "//ud-suffix//" and "identifiers appearing as a //token// or //preprocessing-token//" are treated differently in the standard. Thus, I don't think folding -Wuser-defined-literals into -Wreserved-identifier is a good idea. ================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:411 + "identifier '%0' preceded by space(s) in the literal operator declaration " + "is deprecated">, InGroup<DeprecatedLiteralOperator>, DefaultIgnore; def warn_reserved_module_name : Warning< ---------------- rZhBoYao wrote: > aaron.ballman wrote: > > Oye, I'm of two minds about `DefaultIgnore`. > > > > On the one hand, this is going to fire *a lot*: > > https://sourcegraph.com/search?q=context:global+operator%5B%5B:space:%5D%5D*%5C%22%5C%22%5B%5B:space:%5D%5D%5BA-Za-z0-9_%5D%2B&patternType=regexp&case=yes&sm=1&groupBy=repo > > > > On the other hand, if we don't warn about it being deprecated, users won't > > know about it, which makes it harder to do anything about it the longer we > > silently allow it. (We have plenty of experience with this particular > > flavor of pain.) > > > > I think we should warn about this by default. It's under its own warning > > group, so users who want to ignore the warning and live dangerously can do > > so. And it will be silenced in system headers automatically, so issuing the > > diagnostic should generally be actionable for users. > I'm thinking the same after seeing [[ > https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2863r0.html#D.9 | > P2863R0 ยง 6.9 ]] > > However, a lot of tests are written in the deprecated way. I think a patch > removing all the whitespaces only preceding this one is needed, to not > clutter up this one. Default on and diagnose for C++23 only. Enable for all the lang modes in https://reviews.llvm.org/D153156 ================ Comment at: clang/include/clang/Basic/IdentifierTable.h:56 + NotStartsWithUnderscore, + ContainsDoubleUnderscore, +}; ---------------- shafik wrote: > I would think starting with a double underscore would also not be allowed, at > least that is my reading. That is indeed the case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152632/new/ https://reviews.llvm.org/D152632 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
