Author: rsmith Date: Fri Jul 10 17:27:17 2015 New Revision: 241953 URL: http://llvm.org/viewvc/llvm-project?rev=241953&view=rev Log: [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.
Added: cfe/trunk/test/Modules/Inputs/multiple-include/ cfe/trunk/test/Modules/Inputs/multiple-include/a.h cfe/trunk/test/Modules/Inputs/multiple-include/b.h cfe/trunk/test/Modules/Inputs/multiple-include/c.h cfe/trunk/test/Modules/Inputs/multiple-include/module.modulemap cfe/trunk/test/Modules/Inputs/multiple-include/x.h cfe/trunk/test/Modules/multiple-include.cpp Modified: cfe/trunk/include/clang/Lex/Preprocessor.h cfe/trunk/lib/Lex/HeaderSearch.cpp Modified: cfe/trunk/include/clang/Lex/Preprocessor.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=241953&r1=241952&r2=241953&view=diff ============================================================================== --- cfe/trunk/include/clang/Lex/Preprocessor.h (original) +++ cfe/trunk/include/clang/Lex/Preprocessor.h Fri Jul 10 17:27:17 2015 @@ -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(); Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=241953&r1=241952&r2=241953&view=diff ============================================================================== --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original) +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Jul 10 17:27:17 2015 @@ -995,7 +995,8 @@ HeaderFileInfo &HeaderSearch::getFileInf 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( HeaderFileInfo &HFI = FileInfo[FE->getUID()]; HFI.isModuleHeader = true; - HFI.isCompilingModuleHeader = isCompilingModuleHeader; + HFI.isCompilingModuleHeader |= isCompilingModuleHeader; HFI.setHeaderRole(Role); } @@ -1058,15 +1059,16 @@ bool HeaderSearch::ShouldEnterIncludeFil // 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; Added: cfe/trunk/test/Modules/Inputs/multiple-include/a.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/multiple-include/a.h?rev=241953&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/multiple-include/a.h (added) +++ cfe/trunk/test/Modules/Inputs/multiple-include/a.h Fri Jul 10 17:27:17 2015 @@ -0,0 +1 @@ +#include "x.h" Added: cfe/trunk/test/Modules/Inputs/multiple-include/b.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/multiple-include/b.h?rev=241953&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/multiple-include/b.h (added) +++ cfe/trunk/test/Modules/Inputs/multiple-include/b.h Fri Jul 10 17:27:17 2015 @@ -0,0 +1,3 @@ +#pragma clang __debug macro C_H +#include "c.h" +inline int get() { return c; } Added: cfe/trunk/test/Modules/Inputs/multiple-include/c.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/multiple-include/c.h?rev=241953&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/multiple-include/c.h (added) +++ cfe/trunk/test/Modules/Inputs/multiple-include/c.h Fri Jul 10 17:27:17 2015 @@ -0,0 +1,4 @@ +#ifndef C_H +#define C_H +extern int c; +#endif Added: cfe/trunk/test/Modules/Inputs/multiple-include/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/multiple-include/module.modulemap?rev=241953&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/multiple-include/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/multiple-include/module.modulemap Fri Jul 10 17:27:17 2015 @@ -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" } } Added: cfe/trunk/test/Modules/Inputs/multiple-include/x.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/multiple-include/x.h?rev=241953&view=auto ============================================================================== (empty) Added: cfe/trunk/test/Modules/multiple-include.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/multiple-include.cpp?rev=241953&view=auto ============================================================================== --- cfe/trunk/test/Modules/multiple-include.cpp (added) +++ cfe/trunk/test/Modules/multiple-include.cpp Fri Jul 10 17:27:17 2015 @@ -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(); _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits