[PATCH] D152632: [Clang] Add warnings for CWG2521

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Mostly just minor nits from me, otherwise this is looking close to good! If you're able to handle the libc++ failures as part of this patch as well, that'd be great (but isn't required for landing the changes).



================
Comment at: clang/docs/ReleaseNotes.rst:145-148
+- Implemented `CWG2521 <https://wg21.link/CWG2521>`_ which reserves using ``__`` in user defined literal suffixes
+  for C++ implementations, and deprecates literal operator function declarations using an identifier.
+  Taught ``-Wuser-defined-literals`` for the former, on by default for C++23 language mode,
+  and added ``-Wdeprecated-literal-operator`` for the latter, off by default.
----------------



================
Comment at: clang/docs/ReleaseNotes.rst:157-158
+
+    // Let's assume this is not in the global namespace.
+    // -Wdeprecated-literal-operator whines about extra spaces.
+    string operator "" _i18n(const char*, std::size_t);
----------------



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:410-411
+def warn_deprecated_literal_operator_id: Warning<
+  "identifier '%0' preceded by space(s) in the literal operator declaration "
+  "is deprecated">, InGroup<DeprecatedLiteralOperator>;
 def warn_reserved_module_name : Warning<
----------------



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16450-16451
 
-  StringRef LiteralName
-    = FnDecl->getDeclName().getCXXLiteralIdentifier()->getName();
-  if (LiteralName[0] != '_' &&
+  auto *II = FnDecl->getDeclName().getCXXLiteralIdentifier();
+  auto Status = II->isReservedLiteralSuffixId();
+  if (Status != ReservedLiteralSuffixIdStatus::NotReserved &&
----------------
Please spell out the types instead of using `auto`.


================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:514
+        Diag(Loc, diag::warn_deprecated_literal_operator_id)
+            << II->getName() << Hint;
+      }
----------------
Can you pass in `II` here instead of `II->getName()` or does that give bad behavior?


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

https://reviews.llvm.org/D152632