erichkeane wrote:

> > > @erichkeane So, looking why the `asm` attribute is written after the 
> > > visibility attribute, I see on
> > > https://github.com/llvm/llvm-project/blob/92266681bfc89b71d1846b68da296186cd8bfbec/clang/lib/Sema/SemaDecl.cpp#L8128
> > > 
> > > :
> > > ```
> > >   ProcessDeclAttributes(S, NewVD, D);
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > which creates the VisibilityAttr. Then later on
> > > https://github.com/llvm/llvm-project/blob/92266681bfc89b71d1846b68da296186cd8bfbec/clang/lib/Sema/SemaDecl.cpp#L8223
> > > 
> > > ```
> > > if (Expr *E = D.getAsmLabel()) {
> > >     <...>
> > >     NewVD->addAttr(AsmLabelAttr::Create(Context, Label, 
> > > SE->getStrTokenLoc(0)));
> > > }
> > > ```
> > > 
> > > 
> > >     
> > >       
> > >     
> > > 
> > >       
> > >     
> > > 
> > >     
> > >   
> > > Which then creates the `asm` attribute. Hence the ASM attribute is 
> > > created after visibility attributes at least. The question now is which 
> > > attributes can be placed before an `asm` attribute which may resulted in 
> > > this code layout.
> > 
> > 
> > Ah, whew, so it IS just a handful that are inserted incorrectly! That is 
> > good to know. The `AsmLabelAttr` being a 'special' thing there is a little 
> > frustrating... however, does its order ever matter? I wouldn't expect so? 
> > So it at least shouldn't cause any problems, right?
> 
> I just looked at the ISO/IEC 9899:2024 if the language standard does define 
> anything about `asm` and attribute specific layouts and they don't define 
> anything for `asm`, just that it "may be used to insert assembly language 
> directly into the translator output". It doesn't define order of any sort, so 
> this behavior is here probably due to implementation details or to mimic gcc 
> behavior.
> 
> So, switching the order may fix the problem (i.e., print the Asm _before_ the 
> other attributes) but I wonder what other problems this may expose.
> 
> Do you suggest any specific approach here to properly solve this?

I see two options:  Either 1: Always insert Asm 'first'.  Or 2: figure out how 
to insert Asm in the 'right' place.  

I would lean towards #1, and wonder if we're better off moving the 
ProcessDeclAttrs after it (or the Asm handling before it?).

@AaronBallman : curious of your thoughts.

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

Reply via email to