[cfe-dev] address_space pragma

[Richard Smith via cfe-dev]

On Thu, 2 Aug 2018 at 20:53, John McCall via cfe-dev <cfe-dev at lists.llvm.org>
wrote:

> > On Aug 2, 2018, at 7:49 PM, Leonard Chan <leonardchan at google.com> wrote:
> > As I have it now, I store a map of source locations to macro
> > definition names as they are being parsed and that's where I attempt
> > to find the attribute. Since the printing of the address_space is
> > controlled by the qualifier, I added an extra field to the qualifier
> > instead for storing the macro name. I can get this name during the
> > address space attribute construction where the source location of
> > address_space is available to me.
> >
> > Would you mind if I added you as a reviewer to this patch after I clean
> it up?
>
> Most of this preprocessor stuff is really beyond me; I'm probably not the
> best
> reviewer.  Richard might have a better suggestion.
>
> I *think* the macro name can be recovered from the existing recorded
> information in the AST; it's just not really obvious.  (Adding a method
> for this
> would be great!)  Again, I'm not an expert, but I think the idea is:
>   - You need to find the right level of macro expansion, keeping in mind
> that the
>     reference to the attribute macro might itself be written in a macro.
>   - You need to get the ExpansionInfo for that expansion.
>   - You need to verify that it's a macro body expansion and that it's not
> a function
>     macro expansion.
>   - At that point, getExpansionLocStart() should be the location of the
> token for the
>     macro, and you can just ask the preprocessor for the spelling of that
> token.
>

Yes, that's basically right. Lexer::getImmediateMacroName does most of this
dance for you, but you'd still need to work out the right level of macro
expansion for yourself. (You probably want to step out through expansion
ranges until you reach one whose start is __attribute__ token and whose end
is the closing `)` token, or something like that.) It would be nice to give
this treatment to all cases where a macro exactly covers all the tokens in
a type attribute.

Depending on how you want this to work, there's another option:
Preprocessor::getLastMacroWithSpelling can be used to find the macro
defined most recently before a given source location that expands to a
given sequence of tokens. Unlike getImmediateMacroName,
getLastMacroWithSpelling should only be used when producing a diagnostic,
because it needs to look through all defined macros (including all macros
imported from modules) which is not really efficient enough to be
appropriate for a compilation that doesn't produce diagnostics.


> John.
>
>
> > On Thu, Aug 2, 2018 at 3:38 PM John McCall <rjmccall at apple.com> wrote:
> >>
> >>> On Aug 2, 2018, at 5:41 PM, Leonard Chan <leonardchan at google.com>
> wrote:
> >>>
> >>> On Thu, Aug 2, 2018 at 2:33 PM John McCall <rjmccall at apple.com> wrote:
> >>>>
> >>>>> On Aug 2, 2018, at 5:28 PM, Leonard Chan <leonardchan at google.com>
> wrote:
> >>>>>
> >>>>> Ok. So we will no longer use the pragma. We will instead opt for your
> >>>>> suggestion of printing the macro. So the new goals are now:
> >>>>>
> >>>>> - Having address_space() accept strings in addition to integers to
> >>>>> represent unique address spaces
> >>>>
> >>>> Is this still important if you get the diagnostic improvement?  If
> you were declaring address
> >>>> spaces somehow, this seems fine, but I don't really want to make
> address_space("whatever")
> >>>> implicitly declare an address space with specific semantics when it's
> not hard to imagine
> >>>> wanting that for other purposes in the future.
> >>>
> >>> Not entirely. This was something that's still remnant of when we were
> >>> using pragma. It's something that would be nice for us now, but could
> >>> also be discussed later down the line.
> >>>
> >>>>> - Changing the type printer to print the macro instead of the whole
> >>>>> attribute if the attribute was passed a string instead of an int.
> >>>>
> >>>> I don't know that the latter needs to be restricted; it seems like a
> nice usability improvement
> >>>> for all address-space users.
> >>>
> >>> Ok, so in general if address_space is used in a macro, then print the
> macro.
> >>
> >> Yeah.  You'll need to record this spelling information in the
> AttributedType, since
> >> the Type won't otherwise have a source location attached.
> >>
> >> I'm not sure we actually make an AttributedType for address_space, but
> doing
> >> so is reasonable.
> >>
> >> We should do a quick parse of the macro's token sequence to verify that
> it's just
> >> an attribute.  Unfortunately, I'm not really sure how to do that.
> >>
> >> John.
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180806/7f323af7/attachment.html>