[PATCH] D104299: Handle interactions between reserved identifier and user-defined suffixes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:2639
     // or a ud-suffix from the string literal.
+    const bool isUDSuffix = !Literal.getUDSuffix().empty();
     IdentifierInfo *II = nullptr;
----------------
Minor style nits


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:487
 bool Sema::checkLiteralOperatorId(const CXXScopeSpec &SS,
-                                  const UnqualifiedId &Name) {
+                                  const UnqualifiedId &Name, bool isUDSuffix) {
   assert(Name.getKind() == UnqualifiedIdKind::IK_LiteralOperatorId);
----------------



================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:491-493
+    // double operator""_Bq(long double);                  // OK: does not use
+    // the reserved identifier _­Bq ([lex.name]) double operator"" _Bq(long
+    // double);                 // ill-formed, no diagnostic required:
----------------
The flow of the previous comments was basically unreadable. I think this is more readable, but at the expense of being slightly different from the standard's example comments.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:495-496
+    IdentifierInfo *II = Name.Identifier;
+    auto Status = II->isReserved(PP.getLangOpts());
+    auto Loc = Name.getEndLoc();
+    if (Status != ReservedIdentifierStatus::NotReserved &&
----------------
Please don't use `auto` without the type being spelled out in the initialization.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:499
+        !PP.getSourceManager().isInSystemHeader(Loc)) {
+      Diag(Loc, diag::warn_reserved_extern_symbol)
+          << II << static_cast<int>(Status)
----------------
Because this situation is IFNDR, do we want to upgrade this from a warning to an error (that's controlled by the warning flag)?


================
Comment at: clang/test/Sema/reserved-identifier.cpp:79-84
+long double operator"" _BarbeBleue(long double) // expected-warning {{identifier '_BarbeBleue' is reserved because it starts with '_' followed by a capital letter}}
+{
+  return 0.;
+}
+
+long double operator""_SacreBleue(long double) // no-warning
----------------
Can you add test cases that use the suffix as well? Richard had an especially interesting example:
```
int operator""_Foo();
int k2 = operator"" _Foo(); // should warn here, regardless of whether the previous line is in a system header
```


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104299/new/

https://reviews.llvm.org/D104299