Always register asm attribute first in a Decl.

[giulianobelinassi]

Previously, clang's SemaDecl processed asm attributes after every other attribute in the Decl, unlike gcc and clang's own parser which requires asm attributes to be the first one in the declaration (see #162556). Therefore, move the processing of asm attributes to be the first one in the attributes list.

Closes #162126

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Giuliano Belinassi (giulianobelinassi)

Changes

Previously, clang's SemaDecl processed asm attributes after every other attribute in the Decl, unlike gcc and clang's own parser which requires asm attributes to be the first one in the declaration (see #162556). Therefore, move the processing of asm attributes to be the first one in the attributes list.

Closes #162126

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+11)
• (modified) clang/lib/Sema/SemaDecl.cpp (+50-50)
• (modified) clang/test/Sema/attr-print.c (+3)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5e9a71e1e74d6..4d7b2b071b6f8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,17 @@ AST Dumping Potentially Breaking Changes
 
 - Default arguments of template template parameters are pretty-printed now.
 
+- Pretty-printing of ``asm`` attributes are now always the first attribute
+  on the right side of the declaration.  Before we had, e.g.:
+
+    ``__attribute__(("visibility")) asm("string")``
+
+  Now we have:
+
+    ``asm("string") __attribute__(("visibility"))``
+
+  Which is accepted by both clang and gcc parsers.
+
 Clang Frontend Potentially Breaking Changes
 -------------------------------------------
 - Members of anonymous unions/structs are now injected as ``IndirectFieldDecl``
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 6eaf7b9435491..8408843b11da1 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -8124,56 +8124,6 @@ NamedDecl *Sema::ActOnVariableDeclarator(
     }
   }
 
-  // Handle attributes prior to checking for duplicates in MergeVarDecl
-  ProcessDeclAttributes(S, NewVD, D);
-
-  if (getLangOpts().HLSL)
-    HLSL().ActOnVariableDeclarator(NewVD);
-
-  if (getLangOpts().OpenACC)
-    OpenACC().ActOnVariableDeclarator(NewVD);
-
-  // FIXME: This is probably the wrong location to be doing this and we should
-  // probably be doing this for more attributes (especially for function
-  // pointer attributes such as format, warn_unused_result, etc.). Ideally
-  // the code to copy attributes would be generated by TableGen.
-  if (R->isFunctionPointerType())
-    if (const auto *TT = R->getAs<TypedefType>())
-      copyAttrFromTypedefToDecl<AllocSizeAttr>(*this, NewVD, TT);
-
-  if (getLangOpts().CUDA || getLangOpts().isTargetDevice()) {
-    if (EmitTLSUnsupportedError &&
-        ((getLangOpts().CUDA && DeclAttrsMatchCUDAMode(getLangOpts(), NewVD)) ||
-         (getLangOpts().OpenMPIsTargetDevice &&
-          OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(NewVD))))
-      Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
-           diag::err_thread_unsupported);
-
-    if (EmitTLSUnsupportedError &&
-        (LangOpts.SYCLIsDevice ||
-         (LangOpts.OpenMP && LangOpts.OpenMPIsTargetDevice)))
-      targetDiag(D.getIdentifierLoc(), diag::err_thread_unsupported);
-    // CUDA B.2.5: "__shared__ and __constant__ variables have implied static
-    // storage [duration]."
-    if (SC == SC_None && S->getFnParent() != nullptr &&
-        (NewVD->hasAttr<CUDASharedAttr>() ||
-         NewVD->hasAttr<CUDAConstantAttr>())) {
-      NewVD->setStorageClass(SC_Static);
-    }
-  }
-
-  // Ensure that dllimport globals without explicit storage class are treated as
-  // extern. The storage class is set above using parsed attributes. Now we can
-  // check the VarDecl itself.
-  assert(!NewVD->hasAttr<DLLImportAttr>() ||
-         NewVD->getAttr<DLLImportAttr>()->isInherited() ||
-         NewVD->isStaticDataMember() || NewVD->getStorageClass() != SC_None);
-
-  // In auto-retain/release, infer strong retension for variables of
-  // retainable type.
-  if (getLangOpts().ObjCAutoRefCount && ObjC().inferObjCARCLifetime(NewVD))
-    NewVD->setInvalidDecl();
-
   // Handle GNU asm-label extension (encoded as an attribute).
   if (Expr *E = D.getAsmLabel()) {
     // The parser guarantees this is a string.
@@ -8234,6 +8184,56 @@ NamedDecl *Sema::ActOnVariableDeclarator(
     }
   }
 
+  // Handle attributes prior to checking for duplicates in MergeVarDecl
+  ProcessDeclAttributes(S, NewVD, D);
+
+  if (getLangOpts().HLSL)
+    HLSL().ActOnVariableDeclarator(NewVD);
+
+  if (getLangOpts().OpenACC)
+    OpenACC().ActOnVariableDeclarator(NewVD);
+
+  // FIXME: This is probably the wrong location to be doing this and we should
+  // probably be doing this for more attributes (especially for function
+  // pointer attributes such as format, warn_unused_result, etc.). Ideally
+  // the code to copy attributes would be generated by TableGen.
+  if (R->isFunctionPointerType())
+    if (const auto *TT = R->getAs<TypedefType>())
+      copyAttrFromTypedefToDecl<AllocSizeAttr>(*this, NewVD, TT);
+
+  if (getLangOpts().CUDA || getLangOpts().isTargetDevice()) {
+    if (EmitTLSUnsupportedError &&
+        ((getLangOpts().CUDA && DeclAttrsMatchCUDAMode(getLangOpts(), NewVD)) ||
+         (getLangOpts().OpenMPIsTargetDevice &&
+          OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(NewVD))))
+      Diag(D.getDeclSpec().getThreadStorageClassSpecLoc(),
+           diag::err_thread_unsupported);
+
+    if (EmitTLSUnsupportedError &&
+        (LangOpts.SYCLIsDevice ||
+         (LangOpts.OpenMP && LangOpts.OpenMPIsTargetDevice)))
+      targetDiag(D.getIdentifierLoc(), diag::err_thread_unsupported);
+    // CUDA B.2.5: "__shared__ and __constant__ variables have implied static
+    // storage [duration]."
+    if (SC == SC_None && S->getFnParent() != nullptr &&
+        (NewVD->hasAttr<CUDASharedAttr>() ||
+         NewVD->hasAttr<CUDAConstantAttr>())) {
+      NewVD->setStorageClass(SC_Static);
+    }
+  }
+
+  // Ensure that dllimport globals without explicit storage class are treated as
+  // extern. The storage class is set above using parsed attributes. Now we can
+  // check the VarDecl itself.
+  assert(!NewVD->hasAttr<DLLImportAttr>() ||
+         NewVD->getAttr<DLLImportAttr>()->isInherited() ||
+         NewVD->isStaticDataMember() || NewVD->getStorageClass() != SC_None);
+
+  // In auto-retain/release, infer strong retension for variables of
+  // retainable type.
+  if (getLangOpts().ObjCAutoRefCount && ObjC().inferObjCARCLifetime(NewVD))
+    NewVD->setInvalidDecl();
+
   // Find the shadowed declaration before filtering for scope.
   NamedDecl *ShadowedDecl = D.getCXXScopeSpec().isEmpty()
                                 ? getShadowedDeclaration(NewVD, Previous)
diff --git a/clang/test/Sema/attr-print.c b/clang/test/Sema/attr-print.c
index 8492356e5d2e5..211e61a937f63 100644
--- a/clang/test/Sema/attr-print.c
+++ b/clang/test/Sema/attr-print.c
@@ -35,3 +35,6 @@ int * __sptr * __ptr32 ppsp32;
 
 // CHECK: __attribute__((availability(macos, strict, introduced=10.6)));
 void f6(int) __attribute__((availability(macosx,strict,introduced=10.6)));
+
+// CHECK: _libc_intl_domainname asm("__gi__libc_intl_domainname") __attribute__((visibility("hidden")));
+extern const char _libc_intl_domainname[]; extern typeof (_libc_intl_domainname) _libc_intl_domainname asm("__gi__libc_intl_domainname") __attribute__((visibility("hidden")));

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.