r178105 - [modules] Re-enable the "ambiguous expansion of macro" warning.

[Richard Smith]

On Mon, Oct 28, 2013 at 12:15 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> On Sat, Oct 26, 2013 at 10:14 AM, Argyrios Kyrtzidis <akyrtzi at gmail.com>wrote:
>
>> As I said above, I think there's a misunderstanding here; to reiterate:
>> when you say "I want to use the interface provided by M1. If I've not
>> imported M2" this is case 1) I mentioned and I agree with what you said
>> should happen. If this does not happen currently then it should be fixed.
>>
>> Should we focus on what should happen when importing *both* M1 and M2 ?
>>
>
> Yes, I think refocusing will help. In particular, I think this is the key
> question on which we have different views:
>
>
>> How do I know that two modules are *related* ? One module may have
>> imported the other but if it doesn't re-export it it might as well be an
>> *unrelated* one from my perspective; why do I need to care what modules it
>> imported internally ?
>>
>
> There are (I think) two different ways that two modules can have a
> conflict on what a macro name means:
>
> 1) Both modules want the name to mean something (which be mean "it's a
> macro expanding to X" or might be "it's not a macro").
> 2) One module wants to *override* what another module meant the name to
> mean (M2 says "it expands to FOO BAR", and M1 says "actually, I override M2
> and say it expands to BAZ")
>
> There are also two different ways that a module can express its intent for
> a macro name to mean something:
>
> A) It can #define the name to something
> B) It can #undef the name
>
> I think we agree that:
>
> 1A: If two modules want the same macro name to be defined to different
> things, and that macro name is used, that is clearly ambiguous and should
> be diagnosed.
>
> I'm on the fence about this one:
>
> 1B: If one module wants the macro name to be #defined to something, and
> another module wants it to be undefined, currently Clang silently picks the
> definition. I am not entirely convinced that's appropriate -- if my module
> explicitly #undef's a macro, that's a statement of intent: in my interface,
> I'm using that name to mean something that's not a macro. I don't think
> it's obvious that:
>
>   #undef X
>   #define X X
>
> and
>
>   #undef X
>
> should mean fundamentally different things here. Either way, I'm saying "I
> own the macro name X and it produces the token sequence X."
>

Hmm, I think it's actually rather important that we diagnose this: on the
upgrade path from #includes to modules, we'll get a silent change of
behavior without this.

If we do diagnose this case (where two imported modules disagree on a
macro: one says it should be defined and the other says it should not),
then I care a lot less about case 2. (It's enough for me that we diagnose
this; we don't need to preserve the existing behavior.)

That said, I think we should take as a guiding rule that modules and
submodules should behave the same, and we currently violate that in the
case of macros.


> Case 2 is where we really seem to disagree. It's also a weird case for
> modules, since we rarely have cases in which the interface of one module
> includes "modify the interface of another module". Which brings me to your
> next point:
>
>
>> I'm suggesting that If I explicitly import 2 modules, and the modules'
>> interfaces disagree on what an identifier means, then there is ambiguity.
>> One module may or may not have imported the other module internally, but
>> that doesn't change the fact that the modules' interfaces disagree on the
>> meaning of an identifier.
>>
>
> The key thing for me is, if one module imports another module, undefines
> some part of it, then re-exports it, it seems pretty clear that it intends
> to *modify* the interface of that first module. This happens in other
> guises too: extending overload sets, specializing templates, shadowing a
> tag name with a non-tag name, and so on. In all other cases, importing both
> modules gives you the modified interface *with no ambiguity*. The ambiguity
> has already been resolved by the dominating module saying how to resolve
> it. And if I import a module that says that it modifies another module's
> interface, I expect to get that modified interface.
>
> So: 2A) if M2 defines our macro, and M1 imports M2 and undefines and
> redefines that macro, and either I import M1 or I import *both* M1 and M2,
> I've expressed my intent to use M1's resolution to the conflict, and I
> think I should get M1's definition.
> 2B) if M2 defines our macro, and M1 imports M2 and undefines the macro,
> and either I import M1 or I import both M1 and M2, I think the macro should
> not be defined.
>
> By exporting a macro undefinition that undefines M2's macro, M1 is saying
> "importing me undefines M2's macro definition". There's no ambiguity in
> that, and if I import both modules, that's what I expect to happen. This
> would behave in (largely) the same way we behave across submodules in the
> same module, which makes the intuitive model simpler. If M1 didn't want
> this behavior, it should not export the macro.
>
>
> Let me make my proposal a bit more concrete, since I think I've been vague
> up until now.
>
> Instead of storing a single flat list of macro definitions and undefines,
> we store a DAG. The predecessors of each directive are the set of
> directives that were visible at the point of the directive. (For a #define,
> these must all be either #undefs or #defines to the same value.) When we
> see a macro name being used, we take the set of visible leaves of this
> graph, check they are consistent[1], and use that consistent value as the
> value of the macro. Otherwise, we diagnose.
>
> [1] I'm not certain whether consistency should allow a mixture of #undefs
> and #defines, but I'm inclined to think it should not.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131028/f003de98/attachment.html>