<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Thu, 2 Aug 2018 at 20:53, John McCall via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> On Aug 2, 2018, at 7:49 PM, Leonard Chan <<a href="mailto:leonardchan@google.com" target="_blank">leonardchan@google.com</a>> wrote:<br>
> As I have it now, I store a map of source locations to macro<br>
> definition names as they are being parsed and that's where I attempt<br>
> to find the attribute. Since the printing of the address_space is<br>
> controlled by the qualifier, I added an extra field to the qualifier<br>
> instead for storing the macro name. I can get this name during the<br>
> address space attribute construction where the source location of<br>
> address_space is available to me.<br>
> <br>
> Would you mind if I added you as a reviewer to this patch after I clean it up?<br>
<br>
Most of this preprocessor stuff is really beyond me; I'm probably not the best<br>
reviewer. Richard might have a better suggestion.<br>
<br>
I *think* the macro name can be recovered from the existing recorded<br>
information in the AST; it's just not really obvious. (Adding a method for this<br>
would be great!) Again, I'm not an expert, but I think the idea is:<br>
- You need to find the right level of macro expansion, keeping in mind that the<br>
reference to the attribute macro might itself be written in a macro.<br>
- You need to get the ExpansionInfo for that expansion.<br>
- You need to verify that it's a macro body expansion and that it's not a function<br>
macro expansion.<br>
- At that point, getExpansionLocStart() should be the location of the token for the<br>
macro, and you can just ask the preprocessor for the spelling of that token.<br></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
John.<br>
<br>
<br>
> On Thu, Aug 2, 2018 at 3:38 PM John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
>> <br>
>>> On Aug 2, 2018, at 5:41 PM, Leonard Chan <<a href="mailto:leonardchan@google.com" target="_blank">leonardchan@google.com</a>> wrote:<br>
>>> <br>
>>> On Thu, Aug 2, 2018 at 2:33 PM John McCall <<a href="mailto:rjmccall@apple.com" target="_blank">rjmccall@apple.com</a>> wrote:<br>
>>>> <br>
>>>>> On Aug 2, 2018, at 5:28 PM, Leonard Chan <<a href="mailto:leonardchan@google.com" target="_blank">leonardchan@google.com</a>> wrote:<br>
>>>>> <br>
>>>>> Ok. So we will no longer use the pragma. We will instead opt for your<br>
>>>>> suggestion of printing the macro. So the new goals are now:<br>
>>>>> <br>
>>>>> - Having address_space() accept strings in addition to integers to<br>
>>>>> represent unique address spaces<br>
>>>> <br>
>>>> Is this still important if you get the diagnostic improvement? If you were declaring address<br>
>>>> spaces somehow, this seems fine, but I don't really want to make address_space("whatever")<br>
>>>> implicitly declare an address space with specific semantics when it's not hard to imagine<br>
>>>> wanting that for other purposes in the future.<br>
>>> <br>
>>> Not entirely. This was something that's still remnant of when we were<br>
>>> using pragma. It's something that would be nice for us now, but could<br>
>>> also be discussed later down the line.<br>
>>> <br>
>>>>> - Changing the type printer to print the macro instead of the whole<br>
>>>>> attribute if the attribute was passed a string instead of an int.<br>
>>>> <br>
>>>> I don't know that the latter needs to be restricted; it seems like a nice usability improvement<br>
>>>> for all address-space users.<br>
>>> <br>
>>> Ok, so in general if address_space is used in a macro, then print the macro.<br>
>> <br>
>> Yeah. You'll need to record this spelling information in the AttributedType, since<br>
>> the Type won't otherwise have a source location attached.<br>
>> <br>
>> I'm not sure we actually make an AttributedType for address_space, but doing<br>
>> so is reasonable.<br>
>> <br>
>> We should do a quick parse of the macro's token sequence to verify that it's just<br>
>> an attribute. Unfortunately, I'm not really sure how to do that.<br>
>> <br>
>> John.<br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div></div>