[Clang] disallow # operators in attribute argument lists

[a-tarasyuk]

Fixes #147217

---

This PR addresses the parsing of attribute arguments by diagnosing and disallowing the # operator

[llvmbot]

@llvm/pr-subscribers-clang

Author: Oleksandr T. (a-tarasyuk)

Changes

Fixes #147217

---

This PR addresses the parsing of attribute arguments by diagnosing and disallowing the # operator

---

Full diff: https://github.com/llvm/llvm-project/pull/147308.diff

4 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+2)
• (modified) clang/lib/Parse/ParseDecl.cpp (+7)
• (modified) clang/test/Parser/cxx0x-attributes.cpp (+5)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a6be59f1d6bd7..b8032ee9c03da 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -673,6 +673,7 @@ Improvements to Clang's diagnostics
   false positives in exception-heavy code, though only simple patterns
   are currently recognized.
 
+- Clang now rejects ``#`` operators in attribute argument lists. (#GH147217)
 
 Improvements to Clang's time-trace
 ----------------------------------
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 6c30da376dafb..b39e2a7359c22 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -830,6 +830,8 @@ def err_ms_property_expected_comma_or_rparen : Error<
   "expected ',' or ')' at end of property accessor list">;
 def err_ms_property_initializer : Error<
   "property declaration cannot have a default member initializer">;
+def err_invalid_attribute_argument
+    : Error<"'%0' is not allowed in attribute argument lists">;
 
 def err_assume_attr_expects_cond_expr : Error<
   "use of this expression in an %0 attribute requires parentheses">;
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 7e739e09b15e8..059636653723c 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -488,6 +488,13 @@ unsigned Parser::ParseAttributeArgsCommon(
   bool AttributeHasVariadicIdentifierArg =
       attributeHasVariadicIdentifierArg(*AttrName, Form.getSyntax(), ScopeName);
 
+  if (Tok.is(tok::hash) || Tok.is(tok::hashhash)) {
+    Diag(Tok.getLocation(), diag::err_invalid_attribute_argument)
+        << PP.getSpelling(Tok);
+    SkipUntil(tok::r_paren, StopAtSemi);
+    return 0;
+  }
+
   // Interpret "kw_this" as an identifier if the attributed requests it.
   if (ChangeKWThisToIdent && Tok.is(tok::kw_this))
     Tok.setKind(tok::identifier);
diff --git a/clang/test/Parser/cxx0x-attributes.cpp b/clang/test/Parser/cxx0x-attributes.cpp
index 372a373a49ec5..d343cd4f3a93e 100644
--- a/clang/test/Parser/cxx0x-attributes.cpp
+++ b/clang/test/Parser/cxx0x-attributes.cpp
@@ -477,3 +477,8 @@ namespace P2361 {
 }
 
 alignas(int) struct AlignAsAttribute {}; // expected-error {{misplaced attributes; expected attributes here}}
+
+namespace GH147217 {
+  [[clang::annotate(#)]] void a();      // expected-error {{'#' is not allowed in attribute argument lists}}
+  [[clang::annotate(##)]] void b();     // expected-error {{'##' is not allowed in attribute argument lists}}
+}

[cor3ntin]

There is divergence between C and C++, I sent a mail to core https://lists.isocpp.org/liaison/2025/07/1549.php

[AaronBallman]

LGTM!

[AaronBallman]

Actually, the changes still LGTM, but let's hold off on landing the changes until we give WG21 a chance to weigh in. It's possible we'll end up converging and not need to have different behaviors.

[erichkeane]

1 nit on the release note, else this LGTM.

We still need to let Aaron decide how long we are going to give WG21 a chance to change this :)

[AaronBallman]

1 nit on the release note, else this LGTM.

We still need to let Aaron decide how long we are going to give WG21 a chance to change this :)

CC @hubert-reinterpretcast for other opinions. My reading of the CWG thread is that Core seems to think this should be rejected, though I'm surprised that they'd like to limit the utility of attributes that way. We already have existing attributes that form invalid tokens which are used in the wild, like availability accepting a version number like 1.2.3. # doesn't mean "preprocessor" within an attribute argument list the same as how . doesn't mean floating-point number or member access operator. For example, maybe someone wants an attribute that maps to GitHub issue or PR numbers, like [[github(issues: #123, #456, pulls: #12)]] or HTML color codes like [[color(#ea64f9)]], etc.

[a-tarasyuk]

@erichkeane @AaronBallman Thanks — it seems the changes will need to be revisited based on further guidance from WG21. Would it make sense to close this PR for now?

[AaronBallman]

@erichkeane @AaronBallman Thanks — it seems the changes will need to be revisited based on further guidance from WG21. Would it make sense to close this PR for now?

I could see WG21 going either way, but there's a case to be made that this is UB by omission and therefore we're fine to accept in both C and C++ (with a pedantic diagnostic about the extension). I think I'd prefer we accept with a diagnostic over rejecting outright, and if WG21 decides to make it ill-formed we can determine whether we want to turn it into an error at that time or whether we want to keep it as a conforming extension. It's not critical either way, we have no attributes currently that can make use of this, so it's not a strongly held opinion. WDYT?

[cor3ntin]

My preference would be to be maximally useful (ie accept almost any token), but a few tokens are problematic. namely # and \.

Consider

[[foo(#\)]]

[[foo(
#
\
)]]

Given that, I think it's fairly safe to disable # and go with this PR. On the flip side, I don't think there is any urgency.