[cfe-dev] address_space pragma

Tue Aug 7 15:06:59 PDT 2018

I currently have an implementation that uses
Preprocessor::getLastMacroWithSpelling to skim through the macro
definition tokens for finding __attribute__ and address_space, but
based off the unittests for Lexer::getImmediateMacroName, that looks
like it could also work.

The main issue I'm running into is that both of these require a
SourceLocation, which doesn't seem to be available when printing the
Qualifier in the typeprinter. So the only way I can actually use the
macro in the typeprinter is by saving either the SourceLocation or
macro name in the qualifier, which in turn is created through calls to
ASTContext::getAddrSpaceQualType.

My initial guess for tackling this is changing getAddrSpaceQualType to
accept an optional SourceLocation, but that function is used in many
places where a SourceLocation may not be available.
On Mon, Aug 6, 2018 at 2:06 PM Richard Smith <richard at metafoo.co.uk> wrote:
>
> 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