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

[Ben Langmuir via cfe-commits]

> On Feb 3, 2016, at 11:45 AM, Douglas Gregor <dgregor at apple.com> wrote:
> 
>> 
>> On Feb 2, 2016, at 7:26 PM, Ben Langmuir via cfe-commits <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
>> 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
> 
> 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!

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