> 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