<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jul 11, 2014 at 8:42 AM, Ben Langmuir <span dir="ltr"><<a href="mailto:blangmuir@apple.com" target="_blank">blangmuir@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Hey RIchard,<div><br></div><div>Sorry to take so long to reply to this, but I am still interested in getting this stopgap into tree.</div>
</div></blockquote><div><br></div><div>Sorry about the delay getting back to you!</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">
<div class=""><div><blockquote type="cite">Please do not add a stopgap workaround to our stable and backwards-compatible driver interface; just add it to -cc1 instead.</blockquote></div><div><br></div></div><div>Sure.</div>
<div class=""><div><br></div><div><blockquote type="cite">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.</blockquote>
</div><div><br></div></div><div>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 :-)</div>
</div></blockquote><div><br></div><div>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.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><div class="h5"><div><blockquote type="cite"><div><p dir="ltr">

>>> 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.<br>
>>> module Foo { module One { header “One.h” } module Two { header “Two.h” } }<br>
>>><br>
>>> // One.cpp - I don’t want to rebuild when Two.h changes<br>
>>> #import <Foo/One.h><br>
>>><br>
>>> 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)?<br>


>><br>
>><br>
>> 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.<br>
><br>
><br>
> 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?</p><p dir="ltr">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.</p>


</div></blockquote></div></div></div><div>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.</div>
</div></div></blockquote><div><br></div><div>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.</div>
</div></div></div>