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

[Richard Smith]

On Thu, Jan 29, 2015 at 3:57 PM, Richard Smith <richard at metafoo.co.uk>
wrote:

> 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.
>

(There are also a small number of checks we need to turn off for the
synthesized definition; in particular, for weird cases like:

int a[];
void f() { extern int a[5]; }

... we should not reject even though the synthesized definition has type
'int a[1];' which is not compatible with the declaration in 'f'. I suppose
we could even detect this case and not emit a definition of 'a' at all
here, because we know that another TU must be providing one with the right
size.)

> 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/71cb6be5/attachment.html>