[PATCH] D25806: Module: correctly set the module file kind when emitting diagnostics for file_modified

[Richard Smith via cfe-commits]

On Wed, Oct 19, 2016 at 5:05 PM, Manman Ren <manman.ren at gmail.com> wrote:

> manmanren created this revision.
> manmanren added reviewers: bruno, rsmith, benlangmuir.
> manmanren added a subscriber: cfe-commits.
>
> I don't quite like the if statement in the patch, but given that
> ASTReader::Error and DelayedDiagnostic only take strings, it is hard to
> just change err_fe_pch_file_modified to take a %select that depends on an
> integer.
>
> On the other hard, it seems impossible to have another diagnostics in
> flight when emitting err_fe_pch_file_modified. Is it okay to just use Diag
> instead of Error?
>

No, this diagnostic can be produced lazily when first trying to emit a
diagnostic that points into the modified file.


> Unfortunately I was not able to come up with a test that outputs this
> diagnostics for a module file.
>

Try something like this:

// RUN: rm -rf %t
// RUN: mkdir %t
// RUN: echo 'int foo = 0;' > %t/a
// RUN: echo 'module A { header "a" }' > %t/m
// RUN: %clang_cc1 -fmodules -emit-module -fmodule-name=A -x c %t/m -o
%t/pcm
// RUN: echo 'int bar;' > %t/a
// RUN: %clang_cc1 -fmodules -fmodule-file=%t/pcm -fmodule-map-file=%t/m -x
c %s -I%t
#include "a"
int foo = 0;

https://reviews.llvm.org/D25806
>
> Files:
>   include/clang/Basic/DiagnosticSerializationKinds.td
>   lib/Serialization/ASTReader.cpp
>
>
> Index: lib/Serialization/ASTReader.cpp
> ===================================================================
> --- lib/Serialization/ASTReader.cpp
> +++ lib/Serialization/ASTReader.cpp
> @@ -1983,6 +1983,7 @@
>    return R;
>  }
>
> +static unsigned moduleKindForDiagnostic(ModuleKind Kind);
>  InputFile ASTReader::getInputFile(ModuleFile &F, unsigned ID, bool
> Complain) {
>    // If this ID is bogus, just return an empty input file.
>    if (ID == 0 || ID > F.InputFilesLoaded.size())
> @@ -2079,7 +2080,13 @@
>
>        // The top-level PCH is stale.
>        StringRef TopLevelPCHName(ImportStack.back()->FileName);
> -      Error(diag::err_fe_pch_file_modified, Filename, TopLevelPCHName);
> +      unsigned DiagnosticKind = moduleKindForDiagnostic(Import
> Stack.back()->Kind);
> +      if (DiagnosticKind == 0)
> +        Error(diag::err_fe_pch_file_modified, Filename, TopLevelPCHName);
> +      else if (DiagnosticKind == 1)
> +        Error(diag::err_fe_module_file_modified, Filename,
> TopLevelPCHName);
> +      else
> +        Error(diag::err_fe_ast_file_modified, Filename, TopLevelPCHName);
>
>        // Print the import stack.
>        if (ImportStack.size() > 1 && !Diags.isDiagnosticInFlight()) {
> Index: include/clang/Basic/DiagnosticSerializationKinds.td
> ===================================================================
> --- include/clang/Basic/DiagnosticSerializationKinds.td
> +++ include/clang/Basic/DiagnosticSerializationKinds.td
> @@ -21,6 +21,12 @@
>  def err_fe_pch_file_modified : Error<
>      "file '%0' has been modified since the precompiled header '%1' was
> built">,
>      DefaultFatal;
> +def err_fe_module_file_modified : Error<
> +    "file '%0' has been modified since the module file '%1' was built">,
> +    DefaultFatal;
> +def err_fe_ast_file_modified : Error<
> +    "file '%0' has been modified since the AST file '%1' was built">,
> +    DefaultFatal;
>  def err_fe_pch_file_overridden : Error<
>      "file '%0' from the precompiled header has been overridden">;
>  def note_pch_required_by : Note<"'%0' required by '%1'">;
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161019/02e8e597/attachment-0001.html>