[patch][pr22217] Use the most recent decl for mangling

[Richard Smith]

On Thu, Jan 29, 2015 at 3:47 PM, John McCall <rjmccall at apple.com> wrote:

>
> On Jan 29, 2015, at 2:47 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Fri, Jan 23, 2015 at 12:25 AM, John McCall <rjmccall at apple.com> wrote:
>
>> > On Jan 22, 2015, at 4:52 PM, Rafael Espíndola <
>> rafael.espindola at gmail.com> wrote:
>> >
>> > Sent the email a bit early.
>> >
>> >
>> >>> That is not what I am seeing with gcc. Given
>> >>>
>> >>> int pr22217_foo;
>> >>> int *b = &pr22217_foo;
>> >>> extern int pr22217_foo __attribute__((section("zed")));
>>
>> This should be an error in both C and C++.  I see absolutely no reason to
>> allow a declaration following a definition (even a tentative definition) to
>> add a section attribute.  We should not be afraid to reject
>> stupidly-written code when it abuses language extensions, even when they’re
>> not “our” extensions.
>>
>
> I completely agree with the principle here. It is not reasonable to write
> attributes that affect a definition after the definition. It is not
> reasonable to write attributes that affect how a symbol is referenced (such
> as an asm label) after the first use (and perhaps we should simply require
> them on the first declaration).
>
> (Segue away from attributes and towards tentative definitons follows...)
>
> I don't agree with what you said about tentative definitions. The C
> standard is very clear on the model for tentative definitions: they act
> exactly like non-defining declarations until you get to the end of the
> translation unit; if you've not seen a non-tentative definition by that
> point "then the behavior is exactly as if the translation unit contains a
> file scope declaration of that identifier, with the composite type as of
> the end of the translation unit, with an initializer equal to 0.”
>
>
> So, this is interesting.  Unix C compilers have traditionally defaulted to
> -fcommon, i.e. to treating uninitialized variables as common definitions
> that are overridable not just within a translation unit, but within the
> entire program.  (I’m not sure whether ELF platforms implement this as
> “program” or “linkage unit”.  Darwin uses “linkage unit”.)  Whether that’s
> actually compliant is arguable, but regardless, it’s the semantics we use,
> and so we really do have to maintain the tri-state, because tentative
> definitions are semantically quite different from non-tentative definitions.
>
> But in the sense that non-tentative definitions fully replace tentative
> definitions, I agree that the correct behavior is probably to allow a
> non-tentative definition with a section attribute to “override” a tentative
> definition which lacks the attribute.
>
> That's reasonable as long as section attributes never affect the
> code-generation of accesses to an object.  I think we can agree that
> section attributes that do affect code-generation of references (in an
> incompatible way) would clearly need to be on all declarations.  But that’s
> more like an address-space attribute than a section attribute.
>
> Based on that simple semantic model, it is not reasonable for us to reject
> this:
>
> int pr22217_foo;
> int *b = &pr22217_foo;
> extern int pr22217_foo __attribute__((section("zed")));
> int pr22217_foo = 123;
>
> See also PR20688, which is a rejects-valid for standard C11 code due to
> our being confused about how tentative definitions work.
>
> And here's another case we get wrong:
>
>   int a[];
>   extern int a[5];
>
> We're required to emit a definition of 'a' with type 'int[5]', but we emit
> it with type 'int[1]'. We get the corresponding case with an incomplete
> struct correct:
>
>   struct foo x; // ok, tentative definition
>   struct foo { int n, m; };
>   // definition emitted now and has complete type; initializer is {0}.
>
> There are lots of ways we can fix this; perhaps the easiest one would be
> to literally follow what the C standard says: synthesize a definition for
> each tentatively-defined variable at the end of the translation unit. Then
> we can change isThisDeclarationADefinition to simply return 'bool' instead
> of an enum, and have it return 'false' for tentative definitions. Sema
> would track the tentative definitions it's seen, and consider converting
> each one to a definition at end-of-TU.
>
>
> Like I mentioned above, this isn’t actually allowed under -fcommon.
>

I don't see why not. We just need to make sure that the definition we
create at the end of TU is emitted as a common definition.

> Or we can try to keep our current model with a tristate for whether a
> declaration is a definition, but that requires both Sema and IRGen to get a
> lot smarter with regard to handling of tentative definitions.
>
>
> I think this is reasonable.  IRGen should be able to just completely
> replace an existing tentative definition.  As I mentioned up-thread, IRGen
> needs to hold persistent references to global variables with handles anyway
> just because types can change.
>
> John.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150129/f023776c/attachment.html>