From 9728a1156bc9c64cb7bd66fba80f0df0c7ea83a7 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Fri, 10 Jul 2015 22:27:17 +0000 Subject: [PATCH] [modules] When checking the include guard for a header, check whether it's visible in the module we're considering entering. Previously we assumed that if we knew the include guard for a modular header, we'd already parsed it, but that need not be the case if a header is present in the current module and one of its dependencies; the result of getting this wrong was that the current module's submodule for the header would end up empty. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@241953 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Lex/Preprocessor.h | 16 ++++++++++++++++ lib/Lex/HeaderSearch.cpp | 18 ++++++++++-------- test/Modules/Inputs/multiple-include/a.h | 1 + test/Modules/Inputs/multiple-include/b.h | 3 +++ test/Modules/Inputs/multiple-include/c.h | 4 ++++ .../Inputs/multiple-include/module.modulemap | 2 ++ test/Modules/Inputs/multiple-include/x.h | 0 test/Modules/multiple-include.cpp | 5 +++++ 8 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 test/Modules/Inputs/multiple-include/a.h create mode 100644 test/Modules/Inputs/multiple-include/b.h create mode 100644 test/Modules/Inputs/multiple-include/c.h create mode 100644 test/Modules/Inputs/multiple-include/module.modulemap create mode 100644 test/Modules/Inputs/multiple-include/x.h create mode 100644 test/Modules/multiple-include.cpp diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index bba0c38cec..b2f58ead0e 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -786,6 +786,22 @@ public: (!getLangOpts().Modules || (bool)getMacroDefinition(II)); } + /// \brief Determine whether II is defined as a macro within the module M, + /// if that is a module that we've already preprocessed. Does not check for + /// macros imported into M. + bool isMacroDefinedInLocalModule(const IdentifierInfo *II, Module *M) { + if (!II->hasMacroDefinition()) + return false; + auto I = Submodules.find(M); + if (I == Submodules.end()) + return false; + auto J = I->second.Macros.find(II); + if (J == I->second.Macros.end()) + return false; + auto *MD = J->second.getLatest(); + return MD && MD->isDefined(); + } + MacroDefinition getMacroDefinition(const IdentifierInfo *II) { if (!II->hasMacroDefinition()) return MacroDefinition(); diff --git a/lib/Lex/HeaderSearch.cpp b/lib/Lex/HeaderSearch.cpp index 6c5c64bd26..b805990eec 100644 --- a/lib/Lex/HeaderSearch.cpp +++ b/lib/Lex/HeaderSearch.cpp @@ -995,7 +995,8 @@ HeaderFileInfo &HeaderSearch::getFileInfo(const FileEntry *FE) { return HFI; } -bool HeaderSearch::tryGetFileInfo(const FileEntry *FE, HeaderFileInfo &Result) const { +bool HeaderSearch::tryGetFileInfo(const FileEntry *FE, + HeaderFileInfo &Result) const { if (FE->getUID() >= FileInfo.size()) return false; const HeaderFileInfo &HFI = FileInfo[FE->getUID()]; @@ -1028,7 +1029,7 @@ void HeaderSearch::MarkFileModuleHeader(const FileEntry *FE, HeaderFileInfo &HFI = FileInfo[FE->getUID()]; HFI.isModuleHeader = true; - HFI.isCompilingModuleHeader = isCompilingModuleHeader; + HFI.isCompilingModuleHeader |= isCompilingModuleHeader; HFI.setHeaderRole(Role); } @@ -1058,15 +1059,16 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP, // Next, check to see if the file is wrapped with #ifndef guards. If so, and // if the macro that guards it is defined, we know the #include has no effect. if (const IdentifierInfo *ControllingMacro - = FileInfo.getControllingMacro(ExternalLookup)) - // If the include file is part of a module, and we already know what its - // controlling macro is, then we've already parsed it and can safely just - // make it visible. This saves us needing to switch into the visibility - // state of the module just to check whether the macro is defined within it. - if (M || PP.isMacroDefined(ControllingMacro)) { + = FileInfo.getControllingMacro(ExternalLookup)) { + // If the header corresponds to a module, check whether the macro is already + // defined in that module rather than checking in the current set of visible + // modules. + if (M ? PP.isMacroDefinedInLocalModule(ControllingMacro, M) + : PP.isMacroDefined(ControllingMacro)) { ++NumMultiIncludeFileOptzn; return false; } + } // Increment the number of times this file has been included. ++FileInfo.NumIncludes; diff --git a/test/Modules/Inputs/multiple-include/a.h b/test/Modules/Inputs/multiple-include/a.h new file mode 100644 index 0000000000..8266190858 --- /dev/null +++ b/test/Modules/Inputs/multiple-include/a.h @@ -0,0 +1 @@ +#include "x.h" diff --git a/test/Modules/Inputs/multiple-include/b.h b/test/Modules/Inputs/multiple-include/b.h new file mode 100644 index 0000000000..f56ab64da6 --- /dev/null +++ b/test/Modules/Inputs/multiple-include/b.h @@ -0,0 +1,3 @@ +#pragma clang __debug macro C_H +#include "c.h" +inline int get() { return c; } diff --git a/test/Modules/Inputs/multiple-include/c.h b/test/Modules/Inputs/multiple-include/c.h new file mode 100644 index 0000000000..4e7d4b742d --- /dev/null +++ b/test/Modules/Inputs/multiple-include/c.h @@ -0,0 +1,4 @@ +#ifndef C_H +#define C_H +extern int c; +#endif diff --git a/test/Modules/Inputs/multiple-include/module.modulemap b/test/Modules/Inputs/multiple-include/module.modulemap new file mode 100644 index 0000000000..1228ae6cbb --- /dev/null +++ b/test/Modules/Inputs/multiple-include/module.modulemap @@ -0,0 +1,2 @@ +module A { module a { header "a.h" } module b { header "b.h" } module c { header "c.h" } } +module X { module x { header "x.h" } module c { header "c.h" } } diff --git a/test/Modules/Inputs/multiple-include/x.h b/test/Modules/Inputs/multiple-include/x.h new file mode 100644 index 0000000000..e69de29bb2 diff --git a/test/Modules/multiple-include.cpp b/test/Modules/multiple-include.cpp new file mode 100644 index 0000000000..7cbeefc817 --- /dev/null +++ b/test/Modules/multiple-include.cpp @@ -0,0 +1,5 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules -I%S/Inputs/multiple-include -fmodules-cache-path=%t -fimplicit-module-maps -verify %s -fmodules-local-submodule-visibility +// expected-no-diagnostics +#include "b.h" +int c = get(); -- 2.50.1