On Jun 7, 2012, at 9:55 AM, Jordan Rose wrote:
>
> On Jun 6, 2012, at 18:49 , Meador Inge <[email protected]> wrote:
>
>> On Wed, Jun 6, 2012 at 12:25 PM, Jordan Rose <[email protected]> wrote:
>>> Author: jrose
>>> Date: Wed Jun 6 12:25:21 2012
>>> New Revision: 158085
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=158085&view=rev
>>> Log:
>>> Add pedantic warning -Wempty-translation-unit (C11 6.9p1).
>>>
>>> In standard C since C89, a 'translation-unit' is syntactically defined to
>>> have
>>> at least one "external-declaration", which is either a decl or a function
>>> definition. In Clang the latter gives us a declaration as well.
>>>
>>> The tricky bit about this warning is that our predefines can contain
>>> external
>>> declarations (__builtin_va_list and the 128-bit integer types). Therefore
>>> our
>>> AST parser now makes sure we have at least one declaration that doesn't come
>>> from the predefines buffer.
>>>
>>> Also, remove bogus warning about empty source files. This doesn't catch
>>> source
>>> files that only contain comments, and never fired anyway because of our
>>> predefines.
>>>
>>> PR12665 and <rdar://problem/9165548>
>>
>> I agree with the decision to track the pre-defined decls as a
>> short-term solution. Longer term I still think injecting strings into
>> the preprocessor input like this is still gross because: (1)
>> 'InitializePredefinedMacros' is for initializing, uh, macros and
>> __builtin_va_list is not a macro, (2) there isn't a clean separation
>> between all of the builtin types and the builtin preprocessor input,
>> (3) it causes issue like PR 12665 that we have to work around by doing
>> extra unnecessary tracking, and (4) there is special casing in
>> 'Sema::ActOnTypedefNameDecl' to notify the AST context of the types
>> that can be removed.
>>
>> Attached is a work in progress patch where I move the
>> __builtin_va_list type construction to the TargetInfo classes.
>> However, there are some drawbacks to this approach at this point in
>> time: (1) it is a breaking change for the AST serialization format and
>> (2) libclangBasic now depends on libclangAST thus introducing a cyclic
>> dependency.
>>
>> Anyway, I am glad PR 12665 is fixed and I just wanted to share the
>> __builtin_va_list type construction work I have been playing with.
>
> I totally agree with you that this is a hack, and that defining
> __builtin_va_list textually is a worse hack. Again, since this is a warning
> that doesn't really fire on real code, it wasn't worth ripping up the
> existing hack just for this. But you're right that we probably should in the
> long run.
>
> The cyclic dependency kills the idea of just putting the manual AST-building
> in each TargetInfo instance, but maybe we could have an enum like this?
>
> enum VAListKind {
> VALK_VoidPtr,
> VALK_CharPtr,
> VALK_FourIntArray,
> VALK_ELFStruct,
> VALK_DarwinStruct
> };
>
> It's still ugly, and it means that ASTContext has to know about all the
> possibilities, but at least this way it /doesn't/ leak out into the rest of
> the world. Or at least not as much.
I think this solution is perfectly reasonable. If we're ever forced to
generalize further, we can do something like we do with built-in functions, and
have a mini-language that's expressive enough to describe these data structures.
> As far as changing serialization, there are a couple options for backwards
> compatibility. First, leave the enum in there as unused. Second, continue
> serializing __builtin_va_list even though we can reconstruct it later. This
> gives us the option to check for incompatible definitions, though I don't
> think that actually happens right now, in case the serialized AST came from a
> different target. (Although we should already be complaining about that…)
>
> Third, go ahead and break backwards compatibility. When we add new attributes
> to Expr and Decl we have to change the serialization code. But I don't want
> to be the one to make the call there.
Go ahead and break backward compatibility for serialization. We do it all the
time.
- Doug
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
