<div dir="ltr"><div dir="ltr">On Thu, 26 Sep 2019 at 14:38, Jordan Rose via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;">Hi, modules folks and cfe-dev. I <i>think</i><span style="font-style:normal"> 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:</span><div><span style="font-style:normal"><br></span></div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"><div><span style="font-style:normal">// ModuleA.h</span></div><div><span style="font-style:normal">void test() {</span></div><div><span style="font-style:normal">  extern void sharedFunc(void);</span></div><div><span style="font-style:normal">}</span></div><div><span style="font-style:normal"><br></span></div><div><span style="font-style:normal">// ModuleB.h</span></div><div>void sharedFunc(int);</div><div><br></div><div>// client.c</div><div>#import <ModuleA.h></div><div>#import <ModuleB.h></div><div>// or the other order, for a slightly different experiment</div></blockquote><br><div>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?</div></div></blockquote><div><br></div><div>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.)</div><div><br></div><div>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.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div style="overflow-wrap: break-word;"><div>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 <i>more</i><span style="font-style:normal"> useful than giving me the one in ModuleA, but it's not consistent with the behavior with modules turned off.</span></div><div><span style="font-style:normal"><br></span></div><div><span style="font-style:normal">Any insights to offer? Should I file this at <a href="http://bugs.llvm.org" target="_blank">bugs.llvm.org</a>?</span></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>I think the current behavior is largely by design.</div></div></div>