Skip to content

[C] Fix parsing of [[clang::assume]] #141747

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

Merged
merged 1 commit into from
May 28, 2025

Conversation

AaronBallman
Copy link
Collaborator

The assumption attribute is exposed with a Clang spelling, which means we support attribute((assume)) as well as [[clang::assume]] as spellings for the attribute.

In C++, the [[clang::assume]] spelling worked as expected. In C, that spelling would emit an "unknown attribute ignored" diagnostic. This was happening because we were failing to pass in the scope name and source location when creating the attribute. In C++, this worked by chance because [[assume]] is a known attribute in C++. But in C, where there is thankfully no [[assume]] standard attribute, the lack of a scope meant we would set the attribute kind to "unknown".

The assumption attribute is exposed with a Clang spelling, which means
we support __attribute__((assume)) as well as [[clang::assume]] as
spellings for the attribute.

In C++, the [[clang::assume]] spelling worked as expected. In C, that
spelling would emit an "unknown attribute ignored" diagnostic. This was
happening because we were failing to pass in the scope name and source
location when creating the attribute. In C++, this worked by chance
because [[assume]] is a known attribute in C++. But in C, where there
is thankfully no [[assume]] standard attribute, the lack of a scope
meant we would set the attribute kind to "unknown".
@AaronBallman AaronBallman added clang Clang issues not falling into any other category c23 clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 28, 2025
@llvmbot
Copy link
Member

llvmbot commented May 28, 2025

@llvm/pr-subscribers-clang

Author: Aaron Ballman (AaronBallman)

Changes

The assumption attribute is exposed with a Clang spelling, which means we support attribute((assume)) as well as [[clang::assume]] as spellings for the attribute.

In C++, the [[clang::assume]] spelling worked as expected. In C, that spelling would emit an "unknown attribute ignored" diagnostic. This was happening because we were failing to pass in the scope name and source location when creating the attribute. In C++, this worked by chance because [[assume]] is a known attribute in C++. But in C, where there is thankfully no [[assume]] standard attribute, the lack of a scope meant we would set the attribute kind to "unknown".


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

5 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+3)
  • (modified) clang/include/clang/Parse/Parser.h (+5-5)
  • (modified) clang/lib/Parse/ParseDecl.cpp (+4-2)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+8-8)
  • (added) clang/test/Sema/assume.c (+25)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 35ab1461e7b89..59086bd68d9b9 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -226,6 +226,9 @@ C Language Changes
 - Added the existing ``-Wduplicate-decl-specifier`` diagnostic, which is on by
   default, to ``-Wc++-compat`` because duplicated declaration specifiers are
   not valid in C++.
+- The ``[[clang::assume()]]`` attribute is now correctly recognized in C. The
+  ``__attribute__((assume()))`` form has always been supported, so the fix is
+  specific to the attribute syntax used.
 
 C2y Feature Support
 ^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index e6492b81dfff8..c4bef4729fd36 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3037,11 +3037,11 @@ class Parser : public CodeCompletionHandler {
 
   /// Parse the argument to C++23's [[assume()]] attribute. Returns true on
   /// error.
-  bool ParseCXXAssumeAttributeArg(ParsedAttributes &Attrs,
-                                  IdentifierInfo *AttrName,
-                                  SourceLocation AttrNameLoc,
-                                  SourceLocation *EndLoc,
-                                  ParsedAttr::Form Form);
+  bool
+  ParseCXXAssumeAttributeArg(ParsedAttributes &Attrs, IdentifierInfo *AttrName,
+                             SourceLocation AttrNameLoc,
+                             IdentifierInfo *ScopeName, SourceLocation ScopeLoc,
+                             SourceLocation *EndLoc, ParsedAttr::Form Form);
 
   /// Try to parse an 'identifier' which appears within an attribute-token.
   ///
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index f7eb56426f6de..d6c36616bab47 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -676,7 +676,8 @@ void Parser::ParseGNUAttributeArgs(
                          Form);
     return;
   } else if (AttrKind == ParsedAttr::AT_CXXAssume) {
-    ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form);
+    ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, ScopeName,
+                               ScopeLoc, EndLoc, Form);
     return;
   }
 
@@ -734,7 +735,8 @@ unsigned Parser::ParseClangAttributeArgs(
     break;
 
   case ParsedAttr::AT_CXXAssume:
-    ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form);
+    ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, ScopeName,
+                               ScopeLoc, EndLoc, Form);
     break;
   }
   return !Attrs.empty() ? Attrs.begin()->getNumArgs() : 0;
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index a665487a2f87d..2cf33a856c4f4 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -4452,11 +4452,10 @@ static bool IsBuiltInOrStandardCXX11Attribute(IdentifierInfo *AttrName,
   }
 }
 
-bool Parser::ParseCXXAssumeAttributeArg(ParsedAttributes &Attrs,
-                                        IdentifierInfo *AttrName,
-                                        SourceLocation AttrNameLoc,
-                                        SourceLocation *EndLoc,
-                                        ParsedAttr::Form Form) {
+bool Parser::ParseCXXAssumeAttributeArg(
+    ParsedAttributes &Attrs, IdentifierInfo *AttrName,
+    SourceLocation AttrNameLoc, IdentifierInfo *ScopeName,
+    SourceLocation ScopeLoc, SourceLocation *EndLoc, ParsedAttr::Form Form) {
   assert(Tok.is(tok::l_paren) && "Not a C++11 attribute argument list");
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
@@ -4498,8 +4497,8 @@ bool Parser::ParseCXXAssumeAttributeArg(ParsedAttributes &Attrs,
   ArgsUnion Assumption = Res.get();
   auto RParen = Tok.getLocation();
   T.consumeClose();
-  Attrs.addNew(AttrName, SourceRange(AttrNameLoc, RParen), nullptr,
-               SourceLocation(), &Assumption, 1, Form);
+  Attrs.addNew(AttrName, SourceRange(AttrNameLoc, RParen), ScopeName, ScopeLoc,
+               &Assumption, 1, Form);
 
   if (EndLoc)
     *EndLoc = RParen;
@@ -4565,7 +4564,8 @@ bool Parser::ParseCXX11AttributeArgs(
                                       ScopeName, ScopeLoc, Form);
   // So does C++23's assume() attribute.
   else if (!ScopeName && AttrName->isStr("assume")) {
-    if (ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, EndLoc, Form))
+    if (ParseCXXAssumeAttributeArg(Attrs, AttrName, AttrNameLoc, nullptr,
+                                   SourceLocation{}, EndLoc, Form))
       return true;
     NumArgs = 1;
   } else
diff --git a/clang/test/Sema/assume.c b/clang/test/Sema/assume.c
new file mode 100644
index 0000000000000..07a5490addfaf
--- /dev/null
+++ b/clang/test/Sema/assume.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c23 %s -verify
+
+// Validate that the attribute works in C.
+static_assert(!__has_c_attribute(assume));
+static_assert(__has_c_attribute(clang::assume));
+static_assert(__has_attribute(assume));
+
+void test(int n) {
+  // Smoke test.
+  __attribute__((assume(true)));
+  [[clang::assume(true)]];
+
+  // Test diagnostics
+  __attribute__((assume));    // expected-error {{'assume' attribute takes one argument}}
+  __attribute__((assume()));  // expected-error {{expected expression}}
+  [[clang::assume]];          // expected-error {{'assume' attribute takes one argument}}
+  [[clang::assume()]];        // expected-error {{expected expression}}
+
+  __attribute__((assume(n++))); // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
+  [[clang::assume(n++)]];       // expected-warning {{assumption is ignored because it contains (potential) side-effects}}
+
+  [[clang::assume(true)]] int x;       // expected-error {{'assume' attribute cannot be applied to a declaration}}
+  __attribute__((assume(true))) int y; // expected-error {{'assume' attribute cannot be applied to a declaration}}
+}
+

@AaronBallman AaronBallman merged commit 5e28af0 into llvm:main May 28, 2025
16 checks passed
@AaronBallman AaronBallman deleted the aballman-assume-syntax-fix branch May 28, 2025 14:07
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
The assumption attribute is exposed with a Clang spelling, which means
we support __attribute__((assume)) as well as [[clang::assume]] as
spellings for the attribute.

In C++, the [[clang::assume]] spelling worked as expected. In C, that
spelling would emit an "unknown attribute ignored" diagnostic. This was
happening because we were failing to pass in the scope name and source
location when creating the attribute. In C++, this worked by chance
because [[assume]] is a known attribute in C++. But in C, where there is
thankfully no [[assume]] standard attribute, the lack of a scope meant
we would set the attribute kind to "unknown".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c23 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.

3 participants