> On Jul 16, 2014, at 3:42 PM, Richard Smith <[email protected]> wrote:
> 
> On Fri, Jul 11, 2014 at 8:42 AM, Ben Langmuir <[email protected]> wrote:
> Hey RIchard,
> 
> Sorry to take so long to reply to this, but I am still interested in getting 
> this stopgap into tree.
> 
> Sorry about the delay getting back to you!
>  
>> Please do not add a stopgap workaround to our stable and 
>> backwards-compatible driver interface; just add it to -cc1 instead.
> 
> 
> Sure.
> 
>> I don't see any relation between the flag's name and its functionality; 
>> there seems to be no reason for this to be linked to the translation unit 
>> being the implementation of any particular module (and if there were, that's 
>> what -fmodule-name is for). Instead, I think what you're trying to specify 
>> is that a particular module is included textually for this compilation. 
>> Please pick a name that suggests that functionality instead.
> 
> 
> In the abstract I agree with this, but the use case I have is only for TUs 
> that are implementation files for a module and I know that is the only time 
> that this flag will be used by our tools.  It is more useful for the 
> diagnostic to say “don’t do this in the implementation of module Foo”, since 
> that matches when the build system will be passing in this flag.  Given that 
> this doesn’t go into the driver, is this still an issue? If not, I can update 
> and commit this patch, or can post it again for review if you prefer :-)
> 
> I'm fine with this as a short-term cc1-only flag. Longer-term I think we need 
> to evaluate whether we can make the import-of-same-module cases "just work" 
> (I think we can), and I hope this becomes unnecessary at that point.

r213767

> 
>> >>> What’s unexpected to me is that changing a header whose contents are not 
>> >>> usually visible may still require rebuilding all of my .cpp files.
>> >>> module Foo { module One { header “One.h” } module Two { header “Two.h” } 
>> >>> }
>> >>>
>> >>> // One.cpp - I don’t want to rebuild when Two.h changes
>> >>> #import <Foo/One.h>
>> >>>
>> >>> Do we agree that this is unnecessary if submodules cannot accidentally 
>> >>> be affected by changes in other submodules they don’t import (and we 
>> >>> have some way to get the set of dependency files for just the submodule)?
>> >>
>> >>
>> >> No, I don't agree with that. One.cpp might inline some function 
>> >> definitions from Two.h, for instance. Or it might fail to build because 
>> >> it declares something that conflicts with something in Two.h.
>> >
>> >
>> > I feel like I”m missing something - how is that different from One.cpp 
>> > having conflicts with some completely different header or module that is 
>> > not imported into that particular TU?
>> 
>> If you import any part of a module, you have the whole module as part of 
>> your translation unit, even though only some of it might be visible. Thus we 
>> will diagnose your declarations that conflict with unimported portions of an 
>> imported module.
>> 
> 
> Maybe we need to have this discussion on cfe-dev at some point.  I think we 
> need a driver flag to control whether clang reports headers from unimported 
> submodules as dependencies, which will allow users/build systems to make the 
> tradeoff.  As for the default, I strongly feel we shouldn't penalize build 
> performance for correct code in order to guarantee that these particular ODR 
> violations get diagnosed in incremental builds.  A full rebuild will still 
> see any diagnostics and the subset of errors that this affects are not being 
> diagnosed today with headers, so we’re still improving.
> 
> Conversely, I think that we should provide a guarantee that incremental and 
> full builds produce bit-for-bit identical results. As you say, it's a 
> tradeoff, but note that this isn't just about ODR violation checking -- the 
> incremental approach you're suggesting can generate wrong code in some cases 
> (we can inline a function definition from the old version of Two.h) -- so if 
> we want to support this partial-rebuild mode, we'll need to be /very/ careful 
> that we don't pull in any information from an unimported submodule in that 
> mode.

Maybe you can help me understand how this would come about.  In our 
documentation we say:

> Modules are modeled as if each submodule were a separate translation unit, 
> and a module import makes names from the other translation unit visible


Here’s my understanding:
If I don’t import the submodule containing “Two.h”, then I shouldn’t get its 
definitions in my TU.  If I have an inline declaration for a function in Two, 
then I still need to have a definition in my own TU because of inline.  If I 
have a non-inline decl, then Two can’t have an inline decl and if it has a 
definition for the function not marked inline then having that definition show 
up in my TU would lead to multiple definitions if Two is imported somewhere 
else.

Ben
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to