[cfe-commits] [patch] Trying to fix __va_list_tag serialization issue (bug 13189)

[Jean-Daniel Dupas]

Le 29 juin 2012 à 19:31, Meador Inge a écrit :

> 
> On Jun 29, 2012, at 12:20 PM, Douglas Gregor wrote:
> 
>> 
>> On Jun 29, 2012, at 8:07 AM, Jean-Daniel Dupas <devlists at shadowlab.org> wrote:
>> 
>>> 
>>> Le 29 juin 2012 à 16:58, Meador Inge a écrit :
>>> 
>>>> 
>>>> On Jun 29, 2012, at 1:49 AM, Jean-Daniel Dupas wrote:
>>>> 
>>>>> 
>>>>> Le 29 juin 2012 à 07:57, Meador Inge a écrit :
>>>>> 
>>>>>> Moving to cfe-commits...
>>>>>> 
>>>>>> On Jun 28, 2012, at 6:06 PM, Jean-Daniel Dupas wrote:
>>>>>> 
>>>>>>> Hello (and please ignore previous mail to cfe-dev),
>>>>>>> 
>>>>>>> I'm trying to fix a regression with va_list handling in precompiled headers.
>>>>>>> 
>>>>>>> http://llvm.org/bugs/show_bug.cgi?id=13189
>>>>>>> 
>>>>>>> Can someone tell me if the attached patch is the right approach to solve this type serialization issue.
>>>>>> 
>>>>>> 
>>>>>> I was working on something slightly more simplified where a reference to the __va_list_tag type is just saved when building the decls.  Something like the attached. 
>>>>>> 
>>>>> 
>>>>> +    case PREDEF_TYPE_VA_LIST_TAG:
>>>>> +      T = Context.VaListTagTy;
>>>>> +      break;
>>>>> 
>>>>> Is there any guarantee the type will be already built at this point ? 
>>>> 
>>>> 
>>>> When the AST reader is created a Sema object is initialized that insures that type will be available by nature of Sema::Initialize calling ASTContext::getBuiltinVaListDecl.  If this makes folks nervous we could add a wrapper ASTContext::getVaTagType (similar to what you proposed, but takes advantage of the fact that ASTContext::getBuiltinVaListDecl saves the tag type) to ensure the construction and then call the wrapper instead.
>>> 
>>> 
>>> The simpler, the better. I added the getVaTagType method because I didn't know that getBuiltinVaListDecl was guarantee to be call in initialization.
>>> 
>>> For nervous folks, an assert in the ASTReader may probably be enough.
>> 
>> 
>> I'm nervous. It'd much prefer that this be lazily initialized, in case we don't have a Sema object when loading a serialized AST.
> 
> I will make the change.  OK with the rest?


You may add a comment in the test case with a reference to the bug tracker so it will be easier to know what it test.

-- Jean-Daniel