[cfe-dev] Small change to ObjC builtin type definition

[Daniel Dunbar]

Hi,

I agree this is a problem, but I'm not sure of the solution yet and
haven't had time to think about it much.

To clarify the issues:

(1) We need to have predefined types for all of id/Class/SEL/Protocol
because they occur in the language (self, _cmd, @protocol, etc).

(2) If these types are subsequently defined in headers, then they need
to bind to the internal types.

(3) At CodeGen, we need provide actual definitions for these types. In
theory we could do a lot of bitcasting to avoid this, but that makes very
ugly IR.

>From a design perspective it seems like the predefined types should start
as opaque types in clang. This I believe solves the original issue David
presented, but I think it also means a number of changes to Sema. This
should handle (1) and (2).

For CodeGen, I think we just look and see if the type has been refined. If
so we make sure it is compatible with the real type. If it hasn't been refined,
we provide the definition.

Is this workable and I am missing things?

 - Daniel

On Tue, Sep 9, 2008 at 7:44 AM, steve naroff <snaroff at apple.com> wrote:
> Hi David,
>
> Commit r55990 implements your patch below.
>
> The "good reason" clang predefines or hard codes id/SEL/Class/Protocol
> is gcc does. My original GCC (or clang) implementation(s) didn't do
> this. Nevertheless, I believe GCC changed this within the past 5 years
> or so (so I changed clang). Off hand, I don't know why they changed
> it. It complicates the dance between the compiler and the headers
> (which is weird). Hacking MergeTypeDefDecl() is unfortunate.
>
> Nevertheless, I don't see how this patch completely solves your
> problem. When the FIXME to verify the underlying types is implemented,
> I don't see how this is going to work. Are you going to hack
> Sema::ActOnTranslationUnitScope() to install different types? (that
> correspond to headers from your runtime?).
>
> Please advise,
>
> snaroff
>
>
> On Sep 9, 2008, at 7:36 AM, David Chisnall wrote:
>
>> On 9 Sep 2008, at 08:23, Jean-Daniel Dupas wrote:
>>
>>>
>>> Le 9 sept. 08 à 02:23, David Chisnall a écrit :
>>>
>>>> On 8 Sep 2008, at 18:30, Daniel Dunbar wrote:
>>>>
>>>>> This causes several failure in the test suite
>>>>> (Parser/objc-forcollection-*) if I apply. Can
>>>>> you investigate?
>>>>
>>>> I see.  The problem was that now the new id type is being used, but
>>>> ASTContext doesn't recognise it as the ObjC type.  I've tweaked it
>>>> to set it in ASTContext and removed the assert()s here that check it
>>>> isn't already set.  Note that this will now leak, but I'm not sure
>>>> what the best way of fixing this is (can we safely delete the old
>>>> version?  Should we lazily create them somewhere if they're
>>>> referenced before first use (and if so where)?).
>>>>
>>>> The attached version passes all of the tests that it was passing
>>>> before the patch.
>>>
>>> I not aware of any objc file that does not include at least an header
>>> with the definition of the objc runtime base types (id, SEL, BOOL,
>>> …).
>>> Why where these types hard-coded in clang first ?
>>
>> That was the question I asked last week.  Having received no answer, I
>> presumed that there was some good reason.  If there isn't, then we can
>> delete them from Sema.cpp and reinstate the asserts.
>>
>> David
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
>