Turn -Wdeprecated-literal-operator on by default (#111027)

It would be nice to see what our users think about this change, as this
is something that WG21/EWG quite wants to fix a handful of questionable
issues with UB. Depending on the outcome of this after being committed,
we might instead suggest EWG undeprecate this, and require a bit of
'magic' from the lexer.

Additionally, this patch makes it so we emit this diagnostic ALSO in
cases where the literal name is reserved. It doesn't make sense to limit
that.

---------

Co-authored-by: Vlad Serebrennikov <[email protected]>

Author: erichkeane

clang/docs/ReleaseNotes.rst

@@ -99,6 +99,20 @@ C++ Specific Potentially Breaking Changes
     // Was error, now evaluates to false.
     constexpr bool b = f() == g();
 
+- The warning ``-Wdeprecated-literal-operator`` is now on by default, as this is
+  something that WG21 has shown interest in removing from the language. The
+  result is that anyone who is compiling with ``-Werror`` should see this
+  diagnostic.  To fix this diagnostic, simply removing the space character from
+  between the ``operator""`` and the user defined literal name will make the
+  source no longer deprecated. This is consistent with `CWG2521 <https://cplusplus.github.io/CWG/issues/2521.html>_`.
+
+  .. code-block:: c++
+
+    // Now diagnoses by default.
+    unsigned operator"" _udl_name(unsigned long long);
+    // Fixed version:
+    unsigned operator""_udl_name(unsigned long long);
+
 ABI Changes in This Version
 ---------------------------
 
@@ -215,6 +229,10 @@ Resolutions to C++ Defect Reports
 - Clang now allows trailing requires clause on explicit deduction guides.
   (`CWG2707: Deduction guides cannot have a trailing requires-clause <https://cplusplus.github.io/CWG/issues/2707.html>`_).
 
+- Clang now diagnoses a space in the first production of a ``literal-operator-id``
+  by default.
+  (`CWG2521: User-defined literals and reserved identifiers <https://cplusplus.github.io/CWG/issues/2521.html>`_).
+
 C Language Changes
 ------------------
 
@@ -378,6 +396,10 @@ Improvements to Clang's diagnostics
 - The warning for an unsupported type for a named register variable is now phrased ``unsupported type for named register variable``,
   instead of ``bad type for named register variable``. This makes it clear that the type is not supported at all, rather than being
   suboptimal in some way the error fails to mention (#GH111550).
+  
+- Clang now emits a ``-Wdepredcated-literal-operator`` diagnostic, even if the
+  name was a reserved name, which we improperly allowed to suppress the
+  diagnostic.
 
 Improvements to Clang's time-trace
 ----------------------------------

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -439,7 +439,7 @@ def warn_reserved_extern_symbol: Warning<
   InGroup<ReservedIdentifier>, DefaultIgnore;
 def warn_deprecated_literal_operator_id: Warning<
   "identifier %0 preceded by whitespace in a literal operator declaration "
-  "is deprecated">, InGroup<DeprecatedLiteralOperator>, DefaultIgnore;
+  "is deprecated">, InGroup<DeprecatedLiteralOperator>;
 def warn_reserved_module_name : Warning<
   "%0 is a reserved name for a module">, InGroup<ReservedModuleIdentifier>;
 def warn_import_implementation_partition_unit_in_interface_unit : Warning<

clang/lib/Basic/IdentifierTable.cpp

@@ -406,6 +406,9 @@ ReservedLiteralSuffixIdStatus
 IdentifierInfo::isReservedLiteralSuffixId() const {
   StringRef Name = getName();
 
+  // Note: the diag::warn_deprecated_literal_operator_id diagnostic depends on
+  // this being the first check we do, so if this order changes, we have to fix
+  // that as well.
   if (Name[0] != '_')
     return ReservedLiteralSuffixIdStatus::NotStartsWithUnderscore;
 

clang/lib/Sema/SemaExprCXX.cpp

@@ -503,17 +503,23 @@ bool Sema::checkLiteralOperatorId(const CXXScopeSpec &SS,
     const IdentifierInfo *II = Name.Identifier;
     ReservedIdentifierStatus Status = II->isReserved(PP.getLangOpts());
     SourceLocation Loc = Name.getEndLoc();
-    if (!PP.getSourceManager().isInSystemHeader(Loc)) {
-      if (auto Hint = FixItHint::CreateReplacement(
-              Name.getSourceRange(),
-              (StringRef("operator\"\"") + II->getName()).str());
-          isReservedInAllContexts(Status)) {
-        Diag(Loc, diag::warn_reserved_extern_symbol)
-            << II << static_cast<int>(Status) << Hint;
-      } else {
-        Diag(Loc, diag::warn_deprecated_literal_operator_id) << II << Hint;
-      }
-    }
+
+    auto Hint = FixItHint::CreateReplacement(
+        Name.getSourceRange(),
+        (StringRef("operator\"\"") + II->getName()).str());
+
+    // Only emit this diagnostic if we start with an underscore, else the
+    // diagnostic for C++11 requiring a space between the quotes and the
+    // identifier conflicts with this and gets confusing. The diagnostic stating
+    // this is a reserved name should force the underscore, which gets this
+    // back.
+    if (II->isReservedLiteralSuffixId() !=
+        ReservedLiteralSuffixIdStatus::NotStartsWithUnderscore)
+      Diag(Loc, diag::warn_deprecated_literal_operator_id) << II << Hint;
+
+    if (isReservedInAllContexts(Status))
+      Diag(Loc, diag::warn_reserved_extern_symbol)
+          << II << static_cast<int>(Status) << Hint;
   }
 
   if (!SS.isValid())

clang/test/CXX/drs/cwg14xx.cpp

@@ -627,7 +627,7 @@ int i = N::f();
 
 namespace cwg1479 { // cwg1479: 3.1
 #if __cplusplus >= 201103L
-  int operator"" _a(const char*, std::size_t = 0);
+  int operator""_a(const char*, std::size_t = 0);
   // since-cxx11-error@-1 {{literal operator cannot have a default argument}}
 #endif
 }

clang/test/CXX/drs/cwg25xx.cpp

@@ -88,6 +88,9 @@ operator""  _div();
 using ::cwg2521::operator"" _\u03C0___;
 using ::cwg2521::operator""_div;
 // since-cxx11-warning@-2 {{identifier '_π___' preceded by whitespace in a literal operator declaration is deprecated}}
+
+long double operator"" _RESERVED(long double);
+// since-cxx11-warning@-1 {{identifier '_RESERVED' preceded by whitespace in a literal operator declaration is deprecated}}
 #pragma clang diagnostic pop
 #endif
 } // namespace cwg2521

clang/test/CXX/lex/lex.literal/lex.ext/p1.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wno-deprecated-literal-operator -verify %s
 
 void operator "" p31(long double); // expected-warning{{user-defined literal suffixes not starting with '_' are reserved}}
 void operator "" _p31(long double);

clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++11 -verify %s
+// RUN: %clang_cc1 -std=c++11 -Wno-deprecated-literal-operator -verify %s
 
 using size_t = decltype(sizeof(int));
 void operator "" wibble(const char *); // expected-warning {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}

clang/test/CXX/lex/lex.literal/lex.ext/p11.cpp

@@ -6,16 +6,16 @@ template<typename T, typename U> struct same_type;
 template<typename T> struct same_type<T, T> {};
 template<typename T> using X = T;
 template<typename CharT, X<CharT>...>
-int operator "" _x(); // expected-warning {{string literal operator templates are a GNU extension}}
+int operator ""_x(); // expected-warning {{string literal operator templates are a GNU extension}}
 template<char...>
-double operator "" _x();
+double operator ""_x();
 
 auto a="string"_x;
 auto b=42_x;
 same_type<decltype(a), int> test_a;
 same_type<decltype(b), double> test_b;
 
-char operator "" _x(const char *begin, size_t size);
+char operator ""_x(const char *begin, size_t size);
 auto c="string"_x;
 auto d=L"string"_x;
 same_type<decltype(c), char> test_c;

clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp

@@ -1,18 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
 
-int &operator "" _x1 (unsigned long long);
+int &operator ""_x1 (unsigned long long);
 int &i1 = 0x123_x1;
 
-double &operator "" _x1 (const char *);
+double &operator ""_x1 (const char *);
 int &i2 = 45_x1;
 
-template<char...> char &operator "" _x1 ();
+template<char...> char &operator ""_x1 ();
 int &i3 = 0377_x1;
 
 int &i4 = 90000000000000000000000000000000000000000000000_x1; // expected-error {{integer literal is too large to be represented in any integer type}}
 
-double &operator "" _x2 (const char *);
+double &operator ""_x2 (const char *);
 double &i5 = 123123123123123123123123123123123123123123123_x2;
 
-template<char...Cs> constexpr int operator "" _x3() { return sizeof...(Cs); }
+template<char...Cs> constexpr int operator ""_x3() { return sizeof...(Cs); }
 static_assert(123456789012345678901234567890123456789012345678901234567890_x3 == 60, "");