r241953 - [modules] When checking the include guard for a header, check whether it's

Richard Smith richard-llvm at metafoo.co.uk
Fri Jul 10 15:27:17 PDT 2015


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();





More information about the cfe-commits mailing list