Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Oct 11, 2024

Conversation

erichkeane
Copy link
Collaborator

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.

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.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 3, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

Changes

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.


Patch is 48.24 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/111027.diff

33 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+22)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+10-11)
  • (modified) clang/test/CXX/drs/cwg14xx.cpp (+1-1)
  • (modified) clang/test/CXX/drs/cwg17xx.cpp (+3)
  • (modified) clang/test/CXX/drs/cwg25xx.cpp (+3)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p1.cpp (+1-1)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp (+1-1)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p11.cpp (+3-3)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp (+5-5)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p4.cpp (+5-5)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p5.cpp (+4-4)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p6.cpp (+4-4)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p7.cpp (+3-3)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p8.cpp (+3-3)
  • (modified) clang/test/CXX/lex/lex.literal/lex.ext/p9.cpp (+1-1)
  • (modified) clang/test/CXX/over/over.oper/over.literal/p2.cpp (+14-14)
  • (modified) clang/test/CXX/over/over.oper/over.literal/p3.cpp (+33-33)
  • (modified) clang/test/CXX/over/over.oper/over.literal/p5.cpp (+10-10)
  • (modified) clang/test/CXX/over/over.oper/over.literal/p6.cpp (+6-6)
  • (modified) clang/test/CXX/over/over.oper/over.literal/p7.cpp (+5-5)
  • (modified) clang/test/CXX/over/over.oper/over.literal/p8.cpp (+8-8)
  • (modified) clang/test/FixIt/fixit-c++11.cpp (+2-2)
  • (modified) clang/test/Parser/cxx0x-literal-operators.cpp (+4-2)
  • (modified) clang/test/Parser/cxx11-user-defined-literals.cpp (+16-13)
  • (modified) clang/test/SemaCXX/cxx11-user-defined-literals-unused.cpp (+2-2)
  • (modified) clang/test/SemaCXX/cxx11-user-defined-literals.cpp (+30-30)
  • (modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+3-3)
  • (modified) clang/test/SemaCXX/cxx98-compat.cpp (+1-1)
  • (modified) clang/test/SemaCXX/literal-operators.cpp (+23-23)
  • (modified) clang/test/SemaCXX/no-warn-user-defined-literals-in-system-headers.cpp (+2-1)
  • (modified) clang/test/SemaCXX/reserved-identifier.cpp (+5-3)
  • (modified) clang/test/SemaCXX/warn-xor-as-pow.cpp (+3-3)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 44d5f348ed2d54..edd1f1d2302d58 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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.
+
+  .. 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
 ---------------------------
 
@@ -216,6 +230,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
 
 - Clang now emits a diagnostic note at the class declaration when the method definition does not match any declaration (#GH110638).
 
+- 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
 ----------------------------------
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index dc84110ef78211..3c9f6ecb506e36 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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<
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index d490452e91c3bb..44ebfbaa043ce2 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -503,17 +503,16 @@ 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());
+
+    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())
diff --git a/clang/test/CXX/drs/cwg14xx.cpp b/clang/test/CXX/drs/cwg14xx.cpp
index 5301185d046982..cb2f34bf5e427f 100644
--- a/clang/test/CXX/drs/cwg14xx.cpp
+++ b/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
 }
diff --git a/clang/test/CXX/drs/cwg17xx.cpp b/clang/test/CXX/drs/cwg17xx.cpp
index fb53a56923b104..907240b0fc8b28 100644
--- a/clang/test/CXX/drs/cwg17xx.cpp
+++ b/clang/test/CXX/drs/cwg17xx.cpp
@@ -203,6 +203,9 @@ namespace cwg1762 { // cwg1762: 14
   float operator ""E(const char *);
   // since-cxx11-error@-1 {{invalid suffix on literal; C++11 requires a space between literal and identifier}}
   // since-cxx11-warning@-2 {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}
+  // since-cxx11-warning@-3 {{identifier 'E' preceded by whitespace in a literal operator declaration is deprecated}}
+  // FIXME: We could probably do a better job reacting to the fixit here, the
+  // fact that we complain about the space we added is a little strange.
 #endif
 }
 
diff --git a/clang/test/CXX/drs/cwg25xx.cpp b/clang/test/CXX/drs/cwg25xx.cpp
index 1924008f15ba58..87a728088ee6e4 100644
--- a/clang/test/CXX/drs/cwg25xx.cpp
+++ b/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
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p1.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p1.cpp
index 1c227a1b10d385..ec478fbba60a18 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p1.cpp
+++ b/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);
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp
index 1b5d3880cb6129..6a9d713ca72d2e 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p10.cpp
+++ b/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}}
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p11.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p11.cpp
index 8b5fcf4b609b65..d69a58a7dfad20 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p11.cpp
+++ b/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;
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
index d764989312c246..e5ab09c628bcfa 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p3.cpp
+++ b/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, "");
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p4.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p4.cpp
index 011e832c69d729..7dbe70ce084e76 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p4.cpp
+++ b/clang/test/CXX/lex/lex.literal/lex.ext/p4.cpp
@@ -1,18 +1,18 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
 
-int &operator "" _x1 (long double);
+int &operator ""_x1 (long double);
 int &i1 = 0.123_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 = 0377e-1_x1;
 
 int &i4 = 1e1000000_x1; // expected-warning {{too large for type 'long double'}}
 
-double &operator "" _x2 (const char *);
+double &operator ""_x2 (const char *);
 double &i5 = 1e1000000_x2;
 
-template<char...Cs> constexpr int operator "" _x3() { return sizeof...(Cs); }
+template<char...Cs> constexpr int operator ""_x3() { return sizeof...(Cs); }
 static_assert(1e1000000_x3 == 9, "");
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p5.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p5.cpp
index aee20545ececfc..afadba282e626c 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p5.cpp
+++ b/clang/test/CXX/lex/lex.literal/lex.ext/p5.cpp
@@ -3,19 +3,19 @@
 
 using size_t = decltype(sizeof(int));
 
-int &operator "" _x1 (const char *);
-double &operator "" _x1 (const char *, size_t);
+int &operator ""_x1 (const char *);
+double &operator ""_x1 (const char *, size_t);
 double &i1 = "foo"_x1;
 #if __cplusplus >= 202002L
 using char8 = float;
-float &operator "" _x1 (const char8_t *, size_t);
+float &operator ""_x1 (const char8_t *, size_t);
 #else
 using char8 = double;
 #endif
 char8 &i2 = u8"foo"_x1;
 double &i3 = L"foo"_x1; // expected-error {{no matching literal operator for call to 'operator""_x1' with arguments of types 'const wchar_t *' and 'unsigned long'}}
 
-char &operator "" _x1(const wchar_t *, size_t);
+char &operator ""_x1(const wchar_t *, size_t);
 char &i4 = L"foo"_x1; // ok
 double &i5 = R"(foo)"_x1; // ok
 char8 &i6 = u\
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p6.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p6.cpp
index 23cd7081d5e3ee..b1df641f2dc43c 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p6.cpp
+++ b/clang/test/CXX/lex/lex.literal/lex.ext/p6.cpp
@@ -2,13 +2,13 @@
 
 using size_t = decltype(sizeof(int));
 
-int &operator "" _x1 (const char *);
+int &operator ""_x1 (const char *);
 double &i1 = 'a'_x1; // expected-error {{no matching literal operator}}
-double &operator "" _x1 (wchar_t);
+double &operator ""_x1 (wchar_t);
 double &i2 = L'a'_x1;
 double &i3 = 'a'_x1; // expected-error {{no matching literal operator}}
-double &i4 = operator"" _x1('a'); // ok
+double &i4 = operator""_x1('a'); // ok
 
-char &operator "" _x1(char16_t);
+char &operator ""_x1(char16_t);
 char &i5 = u'a'_x1; // ok
 double &i6 = L'a'_x1; // ok
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p7.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p7.cpp
index 0b40ecdc143fcb..d571fcb8697eb0 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p7.cpp
+++ b/clang/test/CXX/lex/lex.literal/lex.ext/p7.cpp
@@ -10,9 +10,9 @@ template<typename T> struct same_type<T, T> {};
 
 namespace std_example {
 
-long double operator "" _w(long double);
-std::string operator "" _w(const char16_t*, size_t);
-unsigned operator "" _w(const char*);
+long double operator ""_w(long double);
+std::string operator ""_w(const char16_t*, size_t);
+unsigned operator ""_w(const char*);
 int main() {
   auto v1 = 1.2_w;    // calls operator""_w(1.2L)
   auto v2 = u"one"_w; // calls operator""_w(u"one", 3)
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p8.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p8.cpp
index d9078221ff5e3a..67d976263e0167 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p8.cpp
+++ b/clang/test/CXX/lex/lex.literal/lex.ext/p8.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++11 -verify %s
 
 using size_t = decltype(sizeof(int));
-constexpr const char *operator "" _id(const char *p, size_t) { return p; }
+constexpr const char *operator ""_id(const char *p, size_t) { return p; }
 constexpr const char *s = "foo"_id "bar" "baz"_id "quux";
 
 constexpr bool streq(const char *p, const char *q) {
@@ -9,8 +9,8 @@ constexpr bool streq(const char *p, const char *q) {
 }
 static_assert(streq(s, "foobarbazquux"), "");
 
-constexpr const char *operator "" _trim(const char *p, size_t n) {
-  return *p == ' ' ? operator "" _trim(p + 1, n - 1) : p;
+constexpr const char *operator ""_trim(const char *p, size_t n) {
+  return *p == ' ' ? operator ""_trim(p + 1, n - 1) : p;
 }
 constexpr const char *t = "   " " "_trim "  foo";
 static_assert(streq(t, "foo"), "");
diff --git a/clang/test/CXX/lex/lex.literal/lex.ext/p9.cpp b/clang/test/CXX/lex/lex.literal/lex.ext/p9.cpp
index 65e27b41b06680..fbdedd119d3d68 100644
--- a/clang/test/CXX/lex/lex.literal/lex.ext/p9.cpp
+++ b/clang/test/CXX/lex/lex.literal/lex.ext/p9.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
 
 using size_t = decltype(sizeof(int));
-void operator "" _x(const wchar_t *, size_t);
+void operator ""_x(const wchar_t *, size_t);
 
 namespace std_example {
 
diff --git a/clang/test/CXX/over/over.oper/over.literal/p2.cpp b/clang/test/CXX/over/over.oper/over.literal/p2.cpp
index f3ebadd2b8b9bc..cf26806e9e7d75 100644
--- a/clang/test/CXX/over/over.oper/over.literal/p2.cpp
+++ b/clang/test/CXX/over/over.oper/over.literal/p2.cpp
@@ -1,43 +1,43 @@
 // RUN: %clang_cc1 -std=c++11 %s -verify
 
-void operator "" _a(const char *);
+void operator ""_a(const char *);
 
 namespace N {
-  using ::operator "" _a;
+  using ::operator ""_a;
 
-  void operator "" _b(const char *);
+  void operator ""_b(const char *);
 }
 
-using N::operator "" _b;
+using N::operator ""_b;
 
 class C {
-  void operator "" _c(const char *); // expected-error {{must be in a namespace or global scope}}
+  void operator ""_c(const char *); // expected-error {{must be in a namespace or global scope}}
 
-  static void operator "" _c(unsigned long long); // expected-error {{must be in a namespace or global scope}}
+  static void operator ""_c(unsigned long long); // expected-error {{must be in a namespace or global scope}}
 
-  friend void operator "" _d(const char *);
+  friend void operator ""_d(const char *);
 };
 
-int operator "" _e; // expected-error {{cannot be the name of a variable}}
+int operator ""_e; // expected-error {{cannot be the name of a variable}}
 
 void f() {
-  int operator "" _f; // expected-error {{cannot be the name of a variable}}
+  int operator ""_f; // expected-error {{cannot be the name of a variable}}
 }
 
 extern "C++" {
-  void operator "" _g(const char *);
+  void operator ""_g(const char *);
 }
 
-template<char...> void operator "" _h() {}
+template<char...> void operator ""_h() {}
 
-template<> void operator "" _h<'a', 'b', 'c'>() {}
+template<> void operator ""_h<'a', 'b', 'c'>() {}
 
-template void operator "" _h<'a', 'b', 'c', 'd'>();
+template void operator ""_h<'a', 'b', 'c', 'd'>();
 
 namespace rdar13605348 {
 
 class C {
-  double operator"" _x(long double value) { return double(value); } // expected-error{{literal operator 'operator""_x' must be in a namespace or global scope}}
+  double operator""_x(long double value) { return double(value); } // expected-error{{literal operator 'operator""_x' must be in a namespace or global scope}}
   double value() { return 3.2_x; } // expected-error{{no matching literal operator for call to}}
 };
 
diff --git a/clang/test/CXX/over/over.oper/over.literal/p3.cpp b/clang/test/CXX/over/over.oper/over.literal/p3.cpp
index 674ace9aee1929..53ebe102630843 100644
--- a/clang/test/CXX/over/over.oper/over.literal/p3.cpp
+++ b/clang/test/CXX/over/over.oper/over.literal/p3.cpp
@@ -3,38 +3,38 @@
 using size_t = decltype(sizeof(int));
 
 // Acceptable parameter declarations
-char operator "" _a(const char *);
-char operator "" _a(const char []);
-char operator "" _a(unsigned long long);
-char operator "" _a(long double);
-char operator "" _a(char);
-char operator "" _a(const volatile char);
-char operator "" _a(wchar_t);
-char operator "" _a(char16_t);
-char operator "" _a(char32_t);
-char operator "" _a(const char *, size_t);
-char operator "" _a(const wchar_t *, size_t);
-char operator "" _a(const char16_t *, size_t);
-char operator "" _a(const char32_t *, size_t);
-char operator "" _a(const char [32], size_t);
+char operator ""_a(const char *);
+char operator ""_a(const char []);
+char operator ""_a(unsigned long long);
+char operator ""_a(long double);
+char operator ""_a(char);
+char operator ""_a(const volatile char);
+char operator ""_a(wchar_t);
+char operator ""_a(char16_t);
+char operator ""_a(char32_t);
+char operator ""_a(const char *, size_t);
+char operator ""_a(const wchar_t *, size_t);
+char operator ""_a(const char16_t *, size_t);
+char operator ""_a(const char32_t *, size_t);
+char operator ""_a(const char [32], size_t);
 
 // Unacceptable parameter declarations
-char operator "" _b(); // expected-error {{parameter}}
-char operator "" _b(const wchar_t *); // expected-error {{parameter}}
-char operator "" _b(long long); // expected-error {{parameter}}
-char operator "" _b(double); // expected-error {{parameter}}
-char operator "" _b(short); // expected-error {{parameter}}
-char operator "" _a(char, int = 0); // expected-error {{parameter}}
-char operator "" _b(unsigned short); // expected-error {{parameter}}
-char operator "" _b(signed char); // expected-error {{parameter}}
-char operator "" _b(unsigned char); // expected-error {{parameter}}
-char operator "" _b(const short *, size_t); // expected-error {{parameter}}
-char operator "" _b(const unsigned short *, size_t); // expected-error {{parameter}}
-char operator "" _b(const signed char *, size_t); // expected-error {{parameter}}
-char operator "" _b(const unsigned char *, size_t); // expected-error {{parameter}}
-char operator "" _a(const volatile char *, size_t); // expected-error {{parameter}}
-char operator "" _a(volatile wchar_t *, size_t); // expected-error {{parameter}}
-char operator "" _a(char16_t *, size_t); // expected-error {{parameter}}
-char operator "" _a(const char32_t *, size_t, bool = false); // expected-error {{parameter}}
-char operator "" _a(const char *, signed long); // expected-error {{parameter}}
-char operator "" _a(const char *, size_t = 0); // expected-error {{default argument}}
+char operator ""_b(); // expected-error {{parameter}}
+char operator ""_b(const wchar_t *); // expected-error {{parameter}}
+char operator ""_b(long long); // expected-error {{parameter}}
+char operator ""_b(double); // expected-error {{parameter}}
+char operator ""_b(short); // expected-error {{parameter}}
+char operator ""_a(char, int = 0); // expected-error {{parameter}}
+char operator ""_b(unsigned short); // expected-error {{parameter}}
+char operator "...
[truncated]

@@ -503,17 +503,16 @@ 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)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't clear to me that this check was necessary at all? So I've removed it. Both diags happen now without the check, and unconditionally on each-other. They also share the FixItHint, which was done in a frightening-ish way.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need it any longer because I believe we suppress diagnostics in system headers via the diagnostics engine itself; you can test it out by using GNU line markers to pretend some code is a system header: https://godbolt.org/z/PsaM19Pe8

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest of changes to DR tests look good.

@@ -203,6 +203,9 @@ namespace cwg1762 { // cwg1762: 14
float operator ""E(const char *);
// since-cxx11-error@-1 {{invalid suffix on literal; C++11 requires a space between literal and identifier}}
// since-cxx11-warning@-2 {{user-defined literal suffixes not starting with '_' are reserved; no literal will invoke this operator}}
// since-cxx11-warning@-3 {{identifier 'E' preceded by whitespace in a literal operator declaration is deprecated}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's one strange warning. Are you sure we can deploy this patch like this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We kinda already do ship this!

But yeah, I put a fixme there because I don't have a good way of figuring out how to avoid it. Is there a good way of detecting that we added the space with a fixit, so that we can skip the warning in that case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussion (and to save this thought): What about making the "user-defined literal suffixes not starting with '_' are reserved" be a default error, then making this 'new' diagnostic only happen if the thing starts with an underscore.

I think that fixes THIS case as well as makes the non-fixit-change case actually sane?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this might be a way forward.

@Endilll Endilll changed the title Turn Wdeprecated-literal-operator on by default Turn -Wdeprecated-literal-operator on by default Oct 3, 2024
Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy that we stop issuing diagnostics that contradict each other.

clang/docs/ReleaseNotes.rst Outdated Show resolved Hide resolved
Co-authored-by: Vlad Serebrennikov <[email protected]>
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, please keep an eye out for fallout in the wild though. It may be interesting to see how many instances of -Wno-deprecated-literal-operator exist now and do a search again before release to see how many new instances show up.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@erichkeane
Copy link
Collaborator Author

LGTM, please keep an eye out for fallout in the wild though. It may be interesting to see how many instances of -Wno-deprecated-literal-operator exist now and do a search again before release to see how many new instances show up.

Thats a good idea! I did our initial search:

https://github.com/search?q=Wno-deprecated-literal-operator&type=code

Looks like 63 total files, 285 issues, 127 PRs, 203 commits.

So so far, most projects are fixing them appears. We can re-do this search at one point.

@AaronBallman
Copy link
Collaborator

LGTM, please keep an eye out for fallout in the wild though. It may be interesting to see how many instances of -Wno-deprecated-literal-operator exist now and do a search again before release to see how many new instances show up.

Thats a good idea! I did our initial search:

https://github.com/search?q=Wno-deprecated-literal-operator&type=code

Looks like 63 total files, 285 issues, 127 PRs, 203 commits.

So so far, most projects are fixing them appears. We can re-do this search at one point.

Another search:
https://sourcegraph.com/search?q=context:global+Wno-deprecated-literal-operator+lang:Makefile&patternType=keyword&sm=0
https://sourcegraph.com/search?q=context:global+Wno-deprecated-literal-operator+lang:CMake&patternType=keyword&sm=0

So one in a Makefile and and nine across six CMake files.

@AaronBallman
Copy link
Collaborator

Pre-commit CI failures appear to be unrelated.

@erichkeane erichkeane merged commit c8554e1 into llvm:main Oct 11, 2024
7 of 9 checks passed
ichaer added a commit to splunk/ichaer-llvm-project that referenced this pull request Oct 11, 2024
…ent-indentonly

* llvm-trunk/main: (6379 commits)
  [gn build] Port 1c94388
  [RISCV] Introduce VLOptimizer pass (llvm#108640)
  [mlir][vector] Add more tests for ConvertVectorToLLVM (7/n) (llvm#111895)
  [libc++] Add output groups to run-buildbot (llvm#111739)
  [libc++abi] Remove unused LIBCXXABI_LIBCXX_INCLUDES CMake option (llvm#111824)
  [clang] Ignore inline namespace for `hasName` (llvm#109147)
  [AArch64] Disable consecutive store merging when Neon is unavailable (llvm#111519)
  [lldb] Fix finding make tool for tests (llvm#111980)
  Turn `-Wdeprecated-literal-operator` on by default (llvm#111027)
  [AMDGPU] Rewrite RegSeqNames using !foreach. NFC. (llvm#111994)
  Revert "Reland: [clang] Finish implementation of P0522 (llvm#111711)"
  Revert "[clang] CWG2398: improve overload resolution backwards compat (llvm#107350)"
  Revert "[clang] Implement TTP P0522 pack matching for deduced function template calls. (llvm#111457)"
  [Clang] Replace Intrinsic::getDeclaration with getOrInsertDeclaration (llvm#111990)
  Revert "[NVPTX] Prefer prmt.b32 over bfi.b32 (llvm#110766)"
  [RISCV] Add DAG combine to turn (sub (shl X, 8-Y), (shr X, Y)) into orc.b (llvm#111828)
  [libc] Fix compilation of new trig functions (llvm#111987)
  [NFC] Rename `Intrinsic::getDeclaration` to `getOrInsertDeclaration` (llvm#111752)
  [NFC][CodingStandard] Add additional example for if-else brace rule (llvm#111733)
  CodeGen: Remove redundant REQUIRES registered-target from tests (llvm#111982)
  ...
@jyknight
Copy link
Member

Some projects still seem to have a requirement for compatibility with GCC 4.8 -- which supports a lot of C++11 but didn't apply the change in CWG 1473, and thus require the space in operator"" _udl. That is somewhat unfortunate.

Two examples from the first page of github search for "Wdeprecated-literal-operator":

DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
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]>
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants