aaron.ballman added a comment.

In https://reviews.llvm.org/D47201#1119947, @Hahnfeld wrote:

> In https://reviews.llvm.org/D47201#1119254, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D47201#1119249, @tra wrote:
> >
> > > IIUIC, nv_weak is a synonym for weak (<rant>why, oh why did they need 
> > > it?</rant>)
> > >  You may need to hunt down and change few other places that deal with the 
> > > weak attribute.
> > >  E.g.: 
> > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/AST/Decl.cpp#L4267
> > >  
> > > https://github.com/llvm-project/llvm-project-20170507/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L3045
> >
> >
> > If it is truly a synonym for weak, then a better implementation would be to 
> > make no semantic distinction between the two attributes -- just add new 
> > spellings to weak. If you need to make minor distinctions between the 
> > spellings, you can do it using accessors on the attribute.
>
>
> I first went with this approach but then thought it would be better to 
> restrict the new attribute as much as possible. That's why I added a 
> completely new one which is only applicable to functions, but not to 
> variables and `CXXRecord`s. Let me know if you'd prefer `nv_weak` to be a 
> full alias of `weak` and I'll revert to what @aaron.ballman suggested.


I don't know enough about nv_weak's semantics to definitively say one way or 
the other -- I can find no documentation on this attribute (official or 
otherwise). However, based purely on the changes made here, I'd likely add the 
accessors and only go with a single semantic attribute. You can check for the 
proper subjects by looking at the parsed attribute kind in SemaDeclAttr.cpp and 
restricting the subjects there (it's a bit less declarative this way, but we 
don't have a way to map subject lists to spellings like we do for accessors 
because it's not a common need).


Repository:
  rC Clang

https://reviews.llvm.org/D47201



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to