r260543 - [Modules] Don't infinite recurse on implicit import of circular modules in preamble
Ben Langmuir via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 11 09:13:20 PST 2016
Argh, I forgot to update my commit message after I changed this patch. Instead of updating the PP’s module set in typo-correction, which broke a submodule visibility test, I fixed this by not updating the PP’s module set in the astreader if the source location is invalid.
> On Feb 11, 2016, at 9:04 AM, Ben Langmuir via cfe-commits <cfe-commits at lists.llvm.org> wrote:
>
> Author: benlangmuir
> Date: Thu Feb 11 11:04:42 2016
> New Revision: 260543
>
> URL: http://llvm.org/viewvc/llvm-project?rev=260543&view=rev
> Log:
> [Modules] Don't infinite recurse on implicit import of circular modules in preamble
>
> Update the Preprocessor's VisibleModuleSet when typo-correction creates
> an implicit module import so that we won't accidentally write an invalid
> SourceLocation into the preamble AST. This would later lead to infinite
> recursion when loading the preamble AST because we use the value in
> ImportLocs to prevent visiting a module twice.
>
> rdar://problem/24440990
>
> Added:
> cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h
> cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h
> cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h
> cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h
> cfe/trunk/test/Index/preamble-with-implicit-import.m
> Modified:
> cfe/trunk/lib/Basic/Module.cpp
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/test/Index/Inputs/module.map
>
> Modified: cfe/trunk/lib/Basic/Module.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=260543&r1=260542&r2=260543&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/Module.cpp (original)
> +++ cfe/trunk/lib/Basic/Module.cpp Thu Feb 11 11:04:42 2016
> @@ -492,6 +492,7 @@ LLVM_DUMP_METHOD void Module::dump() con
>
> void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc,
> VisibleCallback Vis, ConflictCallback Cb) {
> + assert(Loc.isValid() && "setVisible expects a valid import location");
> if (isVisible(M))
> return;
>
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=260543&r1=260542&r2=260543&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Feb 11 11:04:42 2016
> @@ -4062,7 +4062,9 @@ void ASTReader::InitializeContext() {
> if (Module *Imported = getSubmodule(Import.ID)) {
> makeModuleVisible(Imported, Module::AllVisible,
> /*ImportLoc=*/Import.ImportLoc);
> - PP.makeModuleVisible(Imported, Import.ImportLoc);
> + if (Import.ImportLoc.isValid())
> + PP.makeModuleVisible(Imported, Import.ImportLoc);
> + // FIXME: should we tell Sema to make the module visible too?
> }
> }
> ImportedModules.clear();
>
> Modified: cfe/trunk/test/Index/Inputs/module.map
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/module.map?rev=260543&r1=260542&r2=260543&view=diff
> ==============================================================================
> --- cfe/trunk/test/Index/Inputs/module.map (original)
> +++ cfe/trunk/test/Index/Inputs/module.map Thu Feb 11 11:04:42 2016
> @@ -6,3 +6,17 @@ module ModuleNeedsVFS {
> framework module * { }
>
> module ModuleUndef { header "module-undef.h" }
> +
> +module PreambleWithImplicitImport {
> + module A {
> + header "preamble-with-implicit-import-A.h"
> + }
> + module B {
> + header "preamble-with-implicit-import-B.h"
> + export *
> + }
> + module C {
> + header "preamble-with-implicit-import-C.h"
> + export *
> + }
> +}
>
> Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h?rev=260543&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h (added)
> +++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-A.h Thu Feb 11 11:04:42 2016
> @@ -0,0 +1 @@
> +// preamble-with-implicit-import-A
>
> Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h?rev=260543&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h (added)
> +++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-B.h Thu Feb 11 11:04:42 2016
> @@ -0,0 +1,3 @@
> +#pragma once
> +#include "preamble-with-implicit-import-C.h" // Circular
> +typedef struct { char x; } Typo;
>
> Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h?rev=260543&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h (added)
> +++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import-C.h Thu Feb 11 11:04:42 2016
> @@ -0,0 +1,2 @@
> +#pragma once
> +#include "preamble-with-implicit-import-B.h" // Circular
>
> Added: cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h?rev=260543&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h (added)
> +++ cfe/trunk/test/Index/Inputs/preamble-with-implicit-import.h Thu Feb 11 11:04:42 2016
> @@ -0,0 +1,4 @@
> +#include "preamble-with-implicit-import-A.h"
> +
> +// Typo is defined in B, which is not imported.
> +void useTypeFromB(Typo *);
>
> Added: cfe/trunk/test/Index/preamble-with-implicit-import.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/preamble-with-implicit-import.m?rev=260543&view=auto
> ==============================================================================
> --- cfe/trunk/test/Index/preamble-with-implicit-import.m (added)
> +++ cfe/trunk/test/Index/preamble-with-implicit-import.m Thu Feb 11 11:04:42 2016
> @@ -0,0 +1,6 @@
> +// RUN: rm -rf %t
> +// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-load-source-reparse 2 none %s -I %S/Inputs -fmodules -fmodules-cache-path=%t -fspell-checking 2>&1 | FileCheck %s
> +// CHECK: error: declaration of 'Typo' must be imported
> +// CHECK: error: declaration of 'Typo' must be imported
> +
> +#include "preamble-with-implicit-import.h"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list