On Fri, Jun 28, 2013 at 11:35 AM, Eli Friedman <[email protected]> wrote: > On Wed, Jun 19, 2013 at 9:36 PM, Richard Smith <[email protected]> > wrote: >> >> On Tue, Jun 18, 2013 at 11:08 AM, Richard Smith <[email protected]> >> wrote: >> > On Tue, Jun 18, 2013 at 6:57 AM, Rafael Espíndola >> > <[email protected]> wrote: >> >>> Suppose overloading completely ignores whether a declaration is extern >> >>> "C", >> >>> and suppose you have a hashtable which returns the extern "C" decl for >> >>> a >> >>> given identifier. After you conclude foo(int) overloads foo(), you >> >>> look in >> >>> the hashtable and say "oops, there was already a declaration with that >> >>> name". (I haven't really thought through whether this is a good >> >>> idea.) >> >> >> >> The case I don't understand is >> >> >> >> >> >> extern "C" { >> >> static void foo() { } >> >> void foo(int x) {} >> >> } >> >> >> >> Overload in this proposal will decide that the second foo is an >> >> overload, we see that it is extern C, add it to the hash and now, >> >> where do we find the first foo? Note that it is *not* extern C >> >> according to the standard and our current implementation. >> > >> > There are three separate lookups relevant here: >> > >> > 1) Normal redeclaration lookup >> > 2) When declaring an extern "C" function that is not yet a >> > redeclaration, look for extern "C" functions in other scopes >> > 3) When declaring an extern "C" function that is (still) not yet a >> > redeclaration, error if there is a declaration of that name in the >> > global namespace >> > >> > We already do (2), but miss some cases because our map of extern "C" >> > declarations is incomplete (this causes various bugs regardless of >> > whether we allow internal linkage declarations to be extern "C"). >> > >> > I think you're asking how we do (3). The answer is that we currently >> > miss this check if the declarations are not in the same scope, and >> > with my suggestion, would miss it in all cases. Again, this is >> > something we should fix regardless. >> >> Attached patch deals with all of the above, and fixes PR16247. >> >> This is slightly complicated by us using LocallyScopedExternCDecls for >> two completely different things. Prior to this change: >> * In C, we use it to track function-scope extern declarations >> * In C++, we fail to track function-scope extern declarations >> correctly (see PR7560) and instead used the map to track >> function-scope extern "C" declarations. >> >> After this change, the state in C is the same, but in C++, we use the >> map to track *all* extern "C" declarations. (This change doesn't fix >> PR7560, for which we should probably treat local extern variables the >> same way we handle injected friend declarations.) > > > 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?
Yes. My thought was to handle local extern declarations (in both C and C++) in the same way we handle C++ friend injection: by treating them as semantically being members of the enclosing scope, but in an identifier namespace that is only searched by redeclaration lookup. (In C++, redeclaration lookup for such names is currently broken.) > It would be nice to add a comment to make it obvious that > checkGlobalOrExternCConflict is only used in C++. > > 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. Good idea, done. > Otherwise, LGTM. Thanks, r185229. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
