[PATCH] [modules] PR20507: Avoid silent textual inclusion.

[Richard Smith]

On Fri, Jun 19, 2015 at 5:33 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
> On Fri, Jun 19, 2015 at 4:31 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Fri, Jun 19, 2015 at 4:18 PM, Sean Silva <chisophugis at gmail.com>
>> wrote:
>>
>>> ================
>>> Comment at: include/clang/Basic/DiagnosticLexKinds.td:626
>>> @@ -625,1 +625,3 @@
>>> +def note_implicit_top_level_module_import_here : Note<
>>> +  "submodule of top-level module '%0' implicitly imported here">;
>>>  def warn_uncovered_module_header : Warning<
>>> ----------------
>>> benlangmuir wrote:
>>> > I'd just drop "submodule of top-level ".
>>> I think it's an important qualification because the module that was in
>>> fact imported is not %0. %0 is the top-level module containing it, and the
>>> error is because some submodule of %0 (perhaps the one being imported,
>>> perhaps not) has the missing file.
>>>
>>> From a user's perspective: "The header I included didn't ask for %0 to
>>> be imported. It asked for this submodule of it". Ideally we would be able
>>> to somehow communicate the "unity build" aspect of top-level modules for
>>> this to make sense, but I couldn't find a way to fit all that in a
>>> diagnostic.
>>>
>>> Does that make sense?
>>
>>
>> We use this kind of introductory text for a diagnostic produced within a
>> module:
>>
>> In module '<top-level module>' imported from <use of submodule>:
>>
>> ... and a similar note for a diagnostic that occurs while implicitly
>> building a module, so removing the "submodule of top-level" from your
>> diagnostic would be consistent with that. In fact, I'm not sure your note
>> is necessary; don't we already produce a "While building <top-level module>
>> imported from <somewhere>" message prior to emitting the error message?
>>
>
> Are you saying that these things automatically happen? The test output of
> test/Modules/missing-header.cpp with the current patch is as follows:
>
> /Users/Sean/pg/llvm/tools/clang/test/Modules/Inputs/submodules/module.map:15:27:
> error: header 'missing.h' not found
>
>   module missing { header "missing.h" }
>
>                           ^
>
> /Users/Sean/pg/llvm/tools/clang/test/Modules/missing-header.cpp:10:10:
> note: submodule of top-level module 'missing_headers' implicitly imported
> here
>
> #include "not_missing.h"
>
>          ^
>
> 1 error generated.
>
> There doesn't appear to be any "In module ...." or "While building ...".
> Under what circumstances are those emitted? Or do I need to add something
> to have them be emitted?
>

Ah, your diagnostic location is pointing to the module map, not to some
file within the module (from the source manager's point of view). These
extra lines are produced by the same code that handles the "In file
included from"-style diagnostics, based on the inclusion information
carried by the SourceLocation.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150619/c5a991cd/attachment.html>