<div dir="auto">O right. My bad, I'll work on this first before moving onto parsing the macro tokens.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 9, 2018, 11:42 John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> On Aug 9, 2018, at 1:27 PM, Leonard Chan <<a href="mailto:leonardchan@google.com" target="_blank" rel="noreferrer">leonardchan@google.com</a>> wrote:<br>
> <br>
> Right now, the goal is to just change the diagnostic that would<br>
> normally be printed for mixing address spaces to instead print the<br>
> macro in place of the whole attribute that contains this address<br>
> space. That is:<br>
> <br>
> ```<br>
> # define __user __attribute__((noderef, address_space(1)))<br>
> <br>
> char __user *x = ...<br>
> char *y = x;  // err: initializing 'char *' with an expression of type<br>
> '__attribute__((address_space(1))) char *' changes address space of<br>
> pointer<br>
> ```<br>
> <br>
> The error would instead print `initializing 'char *' with an<br>
> expression of type '__user char *' changes address space of pointer`.<br>
> <br>
> It seems that the address space is attached to the type via a<br>
> qualifier instead of the type, that is, I don't think something like<br>
> `__attribute__((address_space(10))) int *` generates an<br>
> AttributedType. This makes me think that I need to attach something to<br>
> the Qualifier, but I don't think I want to increase how much it holds<br>
> since it gets copied everywhere and only holds a mask of 32 bits.<br>
> <br>
> The only alternative options I can think of is changing the printing<br>
> policy to accept information relating to the macro, but am unsure if<br>
> this is the right solution.<br>
<br>
Well, like I said before:<br>
<br>
> I'm not sure we actually make an AttributedType for address_space, but doing<br>
> so is reasonable.<br>
<br>
AttributedType wasn't part of the original AST for sugared types; IIRC we added it<br>
in 2011 or so, and we didn't update most of the existing places where attributes<br>
modify types.  It should be pretty straightforward to do that for address_space.<br>
<br>
John.<br>
<br>
> On Mon, Aug 6, 2018 at 2:26 AM Bevin Hansson <<a href="mailto:bevin.hansson@ericsson.com" target="_blank" rel="noreferrer">bevin.hansson@ericsson.com</a>> wrote:<br>
>> <br>
>> Hi,<br>
>> <br>
>> I'm a bit late to the party, but all this address space stuff sounds<br>
>> interesting to me as we are heavy users of address space functionality<br>
>> downstream.<br>
>> <br>
>> If I understand correctly, you want to include the spelling of the AS in<br>
>> the type. Would this be restricted purely to spellings as macros, or<br>
>> would it also work for keywords? There are a bunch of hard coded strings<br>
>> in the type printer to print out the specific names of ASes for OpenCL,<br>
>> and we have also added our ASes to that hack. If it were possible to<br>
>> encode this straight in the type itself based on keyword/identifier<br>
>> instead of special-casing in the printer, that would probably be cleaner.<br>
>> <br>
>> Feel free to add me as reviewer if you make a patch.<br>
>> <br>
>> / Bevin<br>
>> <br>
>> <br>
>> On 2018-08-03 01:49, Leonard Chan via cfe-dev 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>
>>> On Thu, Aug 2, 2018 at 3:38 PM John McCall <<a href="mailto:rjmccall@apple.com" target="_blank" rel="noreferrer">rjmccall@apple.com</a>> wrote:<br>
>>>>> On Aug 2, 2018, at 5:41 PM, Leonard Chan <<a href="mailto:leonardchan@google.com" target="_blank" rel="noreferrer">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" rel="noreferrer">rjmccall@apple.com</a>> wrote:<br>
>>>>>>> On Aug 2, 2018, at 5:28 PM, Leonard Chan <<a href="mailto:leonardchan@google.com" target="_blank" rel="noreferrer">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>
>>>>>> 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>
>>>>> 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>
>>>>>> 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>
>>>>> Ok, so in general if address_space is used in a macro, then print the macro.<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>
>>> cfe-dev mailing list<br>
>>> <a href="mailto:cfe-dev@lists.llvm.org" target="_blank" rel="noreferrer">cfe-dev@lists.llvm.org</a><br>
>>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
>> <br>
<br>
</blockquote></div>