[cfe-dev] Modules, redeclaration chains, and locally-scoped extern declarations

[Richard Smith via cfe-dev]

On Thu, 26 Sep 2019 at 15:08, Jordan Rose via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> On Sep 26, 2019, at 14:50, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Thu, 26 Sep 2019 at 14:38, Jordan Rose via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> Hi, modules folks and cfe-dev. I *think* I've discovered a bug in how
>> module loading handles redeclaration merging, but it might also be by
>> design. (And honestly "by design" would be nice in my particular use case.)
>> Here it is:
>>
>> // ModuleA.h
>> void test() {
>>   extern void sharedFunc(void);
>> }
>>
>> // ModuleB.h
>> void sharedFunc(int);
>>
>> // client.c
>> #import <ModuleA.h>
>> #import <ModuleB.h>
>> // or the other order, for a slightly different experiment
>>
>>
>> In this situation, the locally-scoped declaration of 'sharedFunc' in
>> ModuleA isn't resolved with the declaration in ModuleB. There's even a
>> conflict in their types! Is this by design, or an oversight, or what?
>>
>
> Well.. we only merge declarations that have matching types. This decision
> predates my involvement, but I think it makes some sense: we probably don't
> want the deserialization code somewhat nondeterministically producing
> diagnostics when it notices problems like this, and deferring the
> diagnostic to an ambiguity error when the function is actually used seems
> like the most consistent user experience. And that ambiguity error will
> presumably not appear for a call to sharedFunc(), because
> (non-redeclaration) lookup for sharedFunc won't find the local extern
> declaration in 'test'. (But I would not be surprised to see an error if you
> attempt to redeclare sharedFunc after importing both modules, or if you try
> to build both headers as part of the same module.)
>
> Regardless of whether we accept it, the above program is ill-formed
> (undefined in C / ill-formed with no diagnostic required in C++), so I
> would encourage you to limit your reliance on it. I don't think we want to
> guarantee that such code keeps compiling.
>
> One of the consequences of this is that in client.c's context, asking for
>> the canonical decl for 'sharedFunc' gives me the declaration in ModuleB.
>> From client.c's perspective, that's *more* useful than giving me the one
>> in ModuleA, but it's not consistent with the behavior with modules turned
>> off.
>>
>> Any insights to offer? Should I file this at bugs.llvm.org?
>>
>
> We generally don't produce eager diagnostics for conflicts between
> imported modules, and that's a feature: we want module import to be
> constant-time, which means we can't diagnose such things eagerly. So for
> the above program (with no use of sharedFunc), I don't think we should
> produce a diagnostic.
>
> I think the current behavior is largely by design.
>
>
> Thanks for the explanation. There's still weirdness here even if they do
> have the same types, since the canonical decl is going to be different, and
> that can have user-visible effects if, say, the local declaration of
> 'sharedFunc' has an attribute like warn_unused_result. Is that by design?
> (I made the types different to be provocative and to make the problem
> obvious.)
>

I think at least part of this is a more general issue unrelated to local
extern declarations. Inheritable attributes are not inherited during
merging, so if the same declaration appears in two different modules with
different sets of accumulated attributes at the end of each module, you
will get surprising behavior.

I know module loading does make an effort to merge redeclaration chains
> normally; it's only locally-scoped extern declarations that are being left
> out.
>

Is this issue C-specific? (I could easily believe that we don't properly
query/update the global scope in this case, and so don't end up merging
local extern declarations, but I think we do update the translation unit
DeclContext -- and merge declarations such as the above -- in C++.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190926/9f86e72d/attachment.html>