r304190 - Diagnose attempts to build a preprocessed module that defines an unavailable submodule.

[Richard Smith via cfe-commits]

On 5 June 2017 at 09:35, David Blaikie via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> On Mon, May 29, 2017 at 10:23 PM Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Tue May 30 00:22:59 2017
>> New Revision: 304190
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=304190&view=rev
>> Log:
>> Diagnose attempts to build a preprocessed module that defines an
>> unavailable submodule.
>>
>> The errors we would otherwise get are incomprehensible, as we would enter
>> the
>> module but not make its contents visible to itself.
>>
>> Added:
>>     cfe/trunk/test/Modules/preprocess-unavailable.cpp
>> Modified:
>>     cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>>     cfe/trunk/lib/Lex/Pragma.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/
>> Basic/DiagnosticLexKinds.td?rev=304190&r1=304189&r2=304190&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue May 30
>> 00:22:59 2017
>> @@ -525,6 +525,8 @@ def err_pp_module_begin_without_module_e
>>  def err_pp_module_end_without_module_begin : Error<
>>    "no matching '#pragma clang module begin' for this "
>>    "'#pragma clang module end'">;
>> +def note_pp_module_begin_here : Note<
>> +  "entering module '%0' due to this pragma">;
>>
>>  def err_defined_macro_name : Error<"'defined' cannot be used as a macro
>> name">;
>>  def err_paste_at_start : Error<
>>
>> Modified: cfe/trunk/lib/Lex/Pragma.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Pragma
>> .cpp?rev=304190&r1=304189&r2=304190&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/Lex/Pragma.cpp (original)
>> +++ cfe/trunk/lib/Lex/Pragma.cpp Tue May 30 00:22:59 2017
>> @@ -1407,6 +1407,24 @@ struct PragmaModuleBeginHandler : public
>>        M = NewM;
>>      }
>>
>> +    // If the module isn't available, it doesn't make sense to enter it.
>> +    if (!M->isAvailable()) {
>> +      Module::Requirement Requirement;
>> +      Module::UnresolvedHeaderDirective MissingHeader;
>> +      (void)M->isAvailable(PP.getLangOpts(), PP.getTargetInfo(),
>> +                           Requirement, MissingHeader);
>>
>
> This looks a tad weird ^ should this function have a different name (or a
> version of the function that only does the side-effecting work that I'm
> guessing is desired here)?
>

It's not exactly producing side-effects; rather, it's filling in output
parameters (and we happen to have already checked the primary return value
as an 'optimization' here). But it seems like this isn't worth the
confusion, plus we repeat this code in four (!) different places. Tidied up
and factored out the common code in r304728 (along with a minor improvement
to diagnostic quality in one of the four repetitions).


> +      if (MissingHeader.FileNameLoc.isValid()) {
>> +        PP.Diag(MissingHeader.FileNameLoc,
>> diag::err_module_header_missing)
>> +          << MissingHeader.IsUmbrella << MissingHeader.FileName;
>> +      } else {
>> +        PP.Diag(M->DefinitionLoc, diag::err_module_unavailable)
>> +          << M->getFullModuleName() << Requirement.second <<
>> Requirement.first;
>> +      }
>> +      PP.Diag(BeginLoc, diag::note_pp_module_begin_here)
>> +        << M->getTopLevelModuleName();
>> +      return;
>> +    }
>> +
>>      // Enter the scope of the submodule.
>>      PP.EnterSubmodule(M, BeginLoc, /*ForPragma*/true);
>>      PP.EnterAnnotationToken(SourceRange(BeginLoc,
>> ModuleName.back().second),
>>
>> Added: cfe/trunk/test/Modules/preprocess-unavailable.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/
>> preprocess-unavailable.cpp?rev=304190&view=auto
>> ============================================================
>> ==================
>> --- cfe/trunk/test/Modules/preprocess-unavailable.cpp (added)
>> +++ cfe/trunk/test/Modules/preprocess-unavailable.cpp Tue May 30
>> 00:22:59 2017
>> @@ -0,0 +1,12 @@
>> +// RUN: %clang_cc1 -x c++-module-map %s -fmodule-name=a -verify
>> +module a {
>> +  module b {
>> +    requires cplusplus11
>> +  }
>> +}
>> +#pragma clang module contents
>> +// expected-error at 3 {{module 'a.b' requires feature 'cplusplus11'}}
>> +#pragma clang module begin a.b // expected-note {{entering module 'a'
>> due to this pragma}}
>> +int f();
>> +int g() { f(); }
>> +#pragma clang module end // expected-error {{no matching '#pragma clang
>> module begin'}}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170605/35713e3a/attachment.html>