r259624 - Make CF constant string decl visible to name lookup to fix module errors

[Ben Langmuir via cfe-commits]

Hey Doug,

I fixed this in r259734; could you take a glance at that patch too?

As I mentioned in person it required more changes than just moving this definition outside the if/fixing the name.

Thanks,

Ben

> On Feb 3, 2016, at 11:50 AM, Ben Langmuir via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> 
>> On Feb 3, 2016, at 11:45 AM, Douglas Gregor <dgregor at apple.com <mailto:dgregor at apple.com>> wrote:
>> 
>>> 
>>> On Feb 2, 2016, at 7:26 PM, Ben Langmuir via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>>> 
>>> Author: benlangmuir
>>> Date: Tue Feb  2 21:26:19 2016
>>> New Revision: 259624
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=259624&view=rev <http://llvm.org/viewvc/llvm-project?rev=259624&view=rev>
>>> Log:
>>> Make CF constant string decl visible to name lookup to fix module errors
>>> 
>>> The return type of the __builtin___*StringMakeConstantString functions
>>> is a pointer to a struct, so we need that struct to be visible to name
>>> lookup so that we will correctly merge multiple declarations of that
>>> type if they come from different modules.
>>> 
>>> Incidentally, to make this visible to name lookup we need to rename the
>>> type to __NSConstantString, since the real NSConstantString is an
>>> Objective-C interface type.  This shouldn't affect anyone outside the
>>> compiler since users of the constant string builtins cast the result
>>> immediately to CFStringRef.
>>> 
>>> Since this struct type is otherwise implicitly created by the AST
>>> context and cannot access namelookup, we make this a predefined type
>>> and initialize it in Sema.
>>> 
>>> Note: this issue of builtins that refer to types not visible to name
>>> lookup technically also affects other builtins (e.g. objc_msgSendSuper),
>>> but in all other cases the builtin is a library builtin and the issue
>>> goes away if you include the library that defines the types it uses,
>>> unlike for these constant string builtins.
>>> 
>>> rdar://problem/24425801 <rdar://problem/24425801>
>> 
>> Two comments below…
>> 
>>> [snip]
>>> 
>>> Modified: cfe/trunk/lib/Sema/Sema.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=259624&r1=259623&r2=259624&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=259624&r1=259623&r2=259624&view=diff>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/Sema.cpp (original)
>>> +++ cfe/trunk/lib/Sema/Sema.cpp Tue Feb  2 21:26:19 2016
>>> @@ -189,6 +189,10 @@ void Sema::Initialize() {
>>>    DeclarationName Protocol = &Context.Idents.get("Protocol");
>>>    if (IdResolver.begin(Protocol) == IdResolver.end())
>>>      PushOnScopeChains(Context.getObjCProtocolDecl(), TUScope);
>>> +
>>> +    DeclarationName ConstantString = &Context.Idents.get("NSConstantString");
>>> +    if (IdResolver.begin(ConstantString) == IdResolver.end())
>>> +      PushOnScopeChains(Context.getCFConstantStringDecl(), TUScope);
>> 
>> Shouldn’t this be looking for __NSConstantString?
> 
> D’oh.
> 
>> 
>> Also, isn’t CFSTR available even in C? This code path is Objective-C-only...
> 
> Yes on both counts, will fix, thanks!
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160203/e841894c/attachment.html>