mcgrathr added inline comments.
================
Comment at: lib/Parse/ParseDecl.cpp:244-252
+ // If this was declared in a macro, attatch the macro IdentifierInfo to the
+ // parsed attribute.
+ for (const auto &MacroPair : PP.getAttributeMacros()) {
+ if (SourceLocInSourceRange(AttrTok.getLocation(), MacroPair.first,
+ PP.getSourceManager())) {
+ ApplyMacroIIToParsedAttrs(attrs, NumParsedAttrs, MacroPair.second);
+ break;
----------------
rsmith wrote:
> leonardchan wrote:
> > rsmith wrote:
> > > You shouldn't do this if `NumParsedAttrs != 1`. We're only looking for a
> > > macro that exactly covers one attribute.
> > >
> > > (Also, your computation of the number of attributes in the attribute list
> > > is not correct in the presence of late-parsed attributes.)
> > One of the things we would like is for this to cover more than one
> > attribute in the attribute list since in sparse, `address_space` is
> > sometimes used with `noderef`.
> >
> > So given `# define __user __attribute__((noderef, address_space(1)))`,
> > `__user` would be saved into the `ParsedAttr` made for `noderef` and
> > `address_space`.
> >
> > What would be the appropriate way to track newly added attributes into the
> > `ParsedAttributes`, including late-parsed attributes?
> > One of the things we would like is for this to cover more than one
> > attribute in the attribute list since in sparse, `address_space` is
> > sometimes used with `noderef`.
>
> Hold on, this is a new requirement compared to what we'd previously discussed
> (giving a name to an address space). How important is this use case to you?
>
> I don't think it's a reasonable AST model to assign a macro identifier to an
> `AttributedType` if the macro defines multiple attributes. If you really want
> to handle that use case, I think that an identifier on the `AttributedType`
> is the wrong way to model it, and we should instead be creating a new type
> sugar node representing a type decoration written as a macro.
>
> Assuming you want to go ahead with the current patch direction (at least in
> the short term), please add the "only one attribute in the macro" check.
A single macro that uses multiple attributes is the central use case for us.
It would be fine if it's constrained to a single __attribute__ clause (or
[[...]] clause) with the attributes comma-separated, as in __attribute__((foo,
bar)) as opposed to two separate __attribute__((foo)) __attribute__((bar)) in
the macro, if that matters. It's even fine if it's constrained to the macro
being nothing but the __attribute__((foo, bar)) clause (aside from whitespace
and comments).
Leo can decide how he wants to proceed as far as incremental implementation.
But we won't have a real-world use for the feature only covering a single
attribute.
We'll start using when it can cover the cases like the Linux __user and __iomem
examples (address_space + noderef).
Repository:
rC Clang
https://reviews.llvm.org/D51329
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
