<div dir="ltr">On Wed, Jun 19, 2013 at 9:36 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On Tue, Jun 18, 2013 at 11:08 AM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br>

> On Tue, Jun 18, 2013 at 6:57 AM, Rafael Espíndola<br>
> <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
>>> Suppose overloading completely ignores whether a declaration is extern "C",<br>
>>> and suppose you have a hashtable which returns the extern "C" decl for a<br>
>>> given identifier.  After you conclude foo(int) overloads foo(), you look in<br>
>>> the hashtable and say "oops, there was already a declaration with that<br>
>>> name".  (I haven't really thought through whether this is a good idea.)<br>
>><br>
>> The case I don't understand is<br>
>><br>
>><br>
>> extern "C" {<br>
>>   static void foo() { }<br>
>>   void foo(int x) {}<br>
>> }<br>
>><br>
>> Overload in this proposal will decide that the second foo is an<br>
>> overload, we see that it is extern C, add it to the hash and now,<br>
>> where do we find the first foo? Note that it is *not* extern C<br>
>> according to the standard and our current implementation.<br>
><br>
> There are three separate lookups relevant here:<br>
><br>
> 1) Normal redeclaration lookup<br>
> 2) When declaring an extern "C" function that is not yet a<br>
> redeclaration, look for extern "C" functions in other scopes<br>
> 3) When declaring an extern "C" function that is (still) not yet a<br>
> redeclaration, error if there is a declaration of that name in the<br>
> global namespace<br>
><br>
> We already do (2), but miss some cases because our map of extern "C"<br>
> declarations is incomplete (this causes various bugs regardless of<br>
> whether we allow internal linkage declarations to be extern "C").<br>
><br>
> I think you're asking how we do (3). The answer is that we currently<br>
> miss this check if the declarations are not in the same scope, and<br>
> with my suggestion, would miss it in all cases. Again, this is<br>
> something we should fix regardless.<br>
<br>
</div></div>Attached patch deals with all of the above, and fixes PR16247.<br>
<br>
This is slightly complicated by us using LocallyScopedExternCDecls for<br>
two completely different things. Prior to this change:<br>
 * In C, we use it to track function-scope extern declarations<br>
 * In C++, we fail to track function-scope extern declarations<br>
correctly (see PR7560) and instead used the map to track<br>
function-scope extern "C" declarations.<br>
<br>
After this change, the state in C is the same, but in C++, we use the<br>
map to track *all* extern "C" declarations. (This change doesn't fix<br>
PR7560, for which we should probably treat local extern variables the<br>
same way we handle injected friend declarations.)<br>
</blockquote></div><br></div><div class="gmail_extra">If I'm understanding your intent correctly, the direction you want to go in is to eventually stop using LocallyScopedExternCDecls in C altogether; am I following this correctly?</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">It would be nice to add a comment to make it obvious that checkGlobalOrExternCConflict is only used in C++.</div><div class="gmail_extra"><br></div><div class="gmail_extra">
Would it be worth trying to optimize for the "simple" case of an extern "C" function whose redecl context is the translation unit?  It looks like the code ends up doing an unnecessary lookup.</div><div class="gmail_extra">
<br></div><div class="gmail_extra">Otherwise, LGTM.</div><div class="gmail_extra"><br></div><div class="gmail_extra">-Eli</div></div>