llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Giuliano Belinassi (giulianobelinassi)

<details>
<summary>Changes</summary>

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
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")));

``````````

</details>


https://github.com/llvm/llvm-project/pull/162687
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to