> On Jan 29, 2015, at 4:16 PM, Richard Smith <[email protected]> wrote:
> On Thu, Jan 29, 2015 at 4:04 PM, John McCall <[email protected] 
> <mailto:[email protected]>> wrote:
>> On Jan 29, 2015, at 3:59 PM, Richard Smith <[email protected] 
>> <mailto:[email protected]>> wrote:
>> On Thu, Jan 29, 2015 at 3:57 PM, Richard Smith <[email protected] 
>> <mailto:[email protected]>> wrote:
>> On Thu, Jan 29, 2015 at 3:47 PM, John McCall <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>>> On Jan 29, 2015, at 2:47 PM, Richard Smith <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> 
>>> On Fri, Jan 23, 2015 at 12:25 AM, John McCall <[email protected] 
>>> <mailto:[email protected]>> wrote:
>>> > On Jan 22, 2015, at 4:52 PM, Rafael Espíndola <[email protected] 
>>> > <mailto:[email protected]>> 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.
> 
> Ah.  I see what you’re saying.  This does become a channel of out-of-bound 
> information that other clients have to be aware of, but yes, it’s possible.
> 
> Yeah, we were intending on tracking this on VarDecl so that consumers who 
> care can find out that a particular implicit definition was synthesized to 
> represent the definition of a tentatively-defined variable. I contend that 
> very few consumers of the AST will need to be aware of this; fewer than need 
> to be aware of the isThisDeclarationADefinition special case today.

Yeah.  I think this is a reasonable design, and Doug concurs.

>> (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.)
> 
> Odds that that wouldn’t cause a failure somewhere? :)
> 
> If it does, the code is doing scary things today -- we'll emit 'a' as an 
> array of 1 element. But just because we could, doesn't mean we should... =) 
> *jurassic park flashback*

Well, it’s pretty easy to imagine f() not really caring about how long a[] is 
but putting a useless bound on it anyway.  I assume that’s technically a 
violation, but it’s a harmless one.

That said, the rule about defaulting incomplete array types to length 1 is just 
bad language design, especially under the official model that lacks common 
definitions.  We should consider warning about it, at least in translation 
units that actually use the array.

John.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to