[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added a subscriber: cfe-commits.

Currently, modules built using module map files embed the path to the directory
that houses their inputs (e.g. headers and module map file) into the output
module file.  This path is embedded as an absolute path and the various input
files are either relativized to this path (if they are underneath it) or also
embedded as an absolute path.

This adds a flag which disables use of this absolute module directory path and
instead writes out paths into the module unmodified (e.g. if they were specified
on the command line as relative paths to the CWD, then they remain relative to
the CWD). This allows building modules without any absolute paths, allowing them
to be built and shared between different working directories.


Repository:
  rC Clang

https://reviews.llvm.org/D51568

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Lex/HeaderSearchOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Serialization/ASTWriter.cpp
  test/Modules/relocatable-modules.modulemap

Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fdisable-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {// 
+}   // 
+
+#include "b.h"  // 
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1339,9 +1339,14 @@
 ///
 /// \return \c true if the path was changed.
 static bool cleanPathForOutput(FileManager &FileMgr,
-   SmallVectorImpl &Path) {
-  bool Changed = FileMgr.makeAbsolutePath(Path);
-  return Changed | llvm::sys::path::remove_dots(Path);
+   SmallVectorImpl &Path,
+   bool MakeAbsolute = true) {
+  bool Changed = false;
+  if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+  }
+  Changed |= llvm::sys::path::remove_dots(Path);
+  return Changed;
 }
 
 /// Adjusts the given filename to only write out the portion of the
@@ -1503,7 +1508,11 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
   }
 
-  if (WritingModule && WritingModule->Directory) {
+  if (WritingModule &&
+  WritingModule->Directory &&
+  !PP.getHeaderSearchInfo()
+  .getHeaderSearchOpts()
+  .DisableModuleDirectory) {
 SmallString<128> BaseDir(WritingModule->Directory->getName());
 cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
 
@@ -2952,12 +2961,20 @@
 // Emit the umbrella header, if there is one.
 if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
   RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
-  Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-UmbrellaHeader.NameAsWritten);
+  SmallString<128> Buffer;
+  llvm::sys::path::append(Buffer,
+  Mod->Directory->getName(),
+  UmbrellaHeader.NameAsWritten);
+  llvm::sys::path::remove_dots(Buffer);
+  Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record, Buffer);
 } else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
   RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
-  Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-UmbrellaDir.NameAsWritten);
+  SmallString<128> Buffer;
+  llvm::sys::path::append(Buffer,
+  Mod->Directory->getName(),
+  UmbrellaDir.NameAsWritten);
+  llvm::sys::path::remove_dots(Buffer);
+  Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record, Buffer);
 }
 
 // Emit the headers.
@@ -4515,7 +4532,11 @@
   assert(Context && "should have context when outputting path");
 
   bool Changed =
-  cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+  cleanPathForOutput(Context->getSourceManager().getFileManager(),
+   

[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

I'm not sure this is the best approach, but I wasn't sure of a better one (to 
support module files w/o absolute paths).  Another approach I tried, was 
relativizing the other input files (from outside the module directory) using 
chains of `../` (e.g. `../../../../../other/modules/module.modulemap`), but 
this causes issues when symlinks appear in the module directory path.  Another 
potential option could be to add a bit to the serialized module format for each 
input, to allow some inputs to mark themselves as relative to the CWD, rather 
than the module directory.


Repository:
  rC Clang

https://reviews.llvm.org/D51568



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51568: [modules] Add `-fdisable-module-directory` flag for relocatable modules

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 163621.
andrewjcg added a comment.

fix umbrella writing


Repository:
  rC Clang

https://reviews.llvm.org/D51568

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Lex/HeaderSearchOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Serialization/ASTWriter.cpp
  test/Modules/relocatable-modules.modulemap

Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fdisable-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fdisable-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {// 
+}   // 
+
+#include "b.h"  // 
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1339,9 +1339,14 @@
 ///
 /// \return \c true if the path was changed.
 static bool cleanPathForOutput(FileManager &FileMgr,
-   SmallVectorImpl &Path) {
-  bool Changed = FileMgr.makeAbsolutePath(Path);
-  return Changed | llvm::sys::path::remove_dots(Path);
+   SmallVectorImpl &Path,
+   bool MakeAbsolute = true) {
+  bool Changed = false;
+  if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+  }
+  Changed |= llvm::sys::path::remove_dots(Path);
+  return Changed;
 }
 
 /// Adjusts the given filename to only write out the portion of the
@@ -1503,7 +1508,11 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
   }
 
-  if (WritingModule && WritingModule->Directory) {
+  if (WritingModule &&
+  WritingModule->Directory &&
+  !PP.getHeaderSearchInfo()
+  .getHeaderSearchOpts()
+  .DisableModuleDirectory) {
 SmallString<128> BaseDir(WritingModule->Directory->getName());
 cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
 
@@ -2952,12 +2961,26 @@
 // Emit the umbrella header, if there is one.
 if (auto UmbrellaHeader = Mod->getUmbrellaHeader()) {
   RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_HEADER};
-  Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record,
-UmbrellaHeader.NameAsWritten);
+  SmallString<128> Buffer;
+  if (PP->getHeaderSearchInfo()
+  .getHeaderSearchOpts()
+  .DisableModuleDirectory) {
+llvm::sys::path::append(Buffer, Mod->Directory->getName());
+  }
+  llvm::sys::path::append(Buffer, UmbrellaHeader.NameAsWritten);
+  llvm::sys::path::remove_dots(Buffer);
+  Stream.EmitRecordWithBlob(UmbrellaAbbrev, Record, Buffer);
 } else if (auto UmbrellaDir = Mod->getUmbrellaDir()) {
   RecordData::value_type Record[] = {SUBMODULE_UMBRELLA_DIR};
-  Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record,
-UmbrellaDir.NameAsWritten);
+  SmallString<128> Buffer;
+  if (PP->getHeaderSearchInfo()
+  .getHeaderSearchOpts()
+  .DisableModuleDirectory) {
+llvm::sys::path::append(Buffer, Mod->Directory->getName());
+  }
+  llvm::sys::path::append(Buffer, UmbrellaDir.NameAsWritten);
+  llvm::sys::path::remove_dots(Buffer);
+  Stream.EmitRecordWithBlob(UmbrellaDirAbbrev, Record, Buffer);
 }
 
 // Emit the headers.
@@ -4515,7 +4538,11 @@
   assert(Context && "should have context when outputting path");
 
   bool Changed =
-  cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+  cleanPathForOutput(Context->getSourceManager().getFileManager(),
+ Path,
+ !PP->getHeaderSearchInfo()
+ .getHeaderSearchOpts()
+ .DisableModuleDirectory);
 
   // Remove a prefix to make the path relative, if relevant.
   const char *PathBegin = Path.data();
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1

[PATCH] D51568: [modules] Add `-fno-absolute-module-directory` flag for relocatable modules

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg marked 2 inline comments as done.
andrewjcg added a comment.

> I don't think we need to change the serialization format for this: a 
> serialized path beginning with / is already treated as absolute and any other 
> path is already treated as relative, so we don't need a flag to carry that 
> information.

But I think we need this since we now have two types of relative paths -- a 
CWD-relative path and a module-home-relative path -- and we use this flag to 
discern them for the AST reader.  Previously, because `cleanPathForOutput` 
would always absolutify input paths, we didn't need this flag -- any relative 
path was relative the module home and all other paths were absolute.

I attempted another take on this diff which just made all paths relative the 
CWD (and did away with module home relative paths), but this didn't work since 
the reader substitutes in a new module home.

> I'm also not convinced we need to put this behind a flag. It would seem 
> reasonable to me to simply always emit the MODULE_DIRECTORY as relative (if 
> we found the module via a relative path), at least for an explicitly-built 
> module.

Yeah, makes sense.  Will fix this.




Comment at: lib/Serialization/ASTWriter.cpp:4524-4546
+bool ASTWriter::PreparePathForOutput(SmallVectorImpl &Path,
+ bool &IsRelativeModuleDirectory) {
   assert(Context && "should have context when outputting path");
 
   bool Changed =
-  cleanPathForOutput(Context->getSourceManager().getFileManager(), Path);
+  cleanPathForOutput(Context->getSourceManager().getFileManager(), Path,
+ llvm::sys::path::is_absolute(BaseDirectory));

rsmith wrote:
> The intent of this function is to use an absolute form of both the file path 
> and `BaseDirectory` so that we can strip off `BaseDirectory` even if the file 
> path ends up absolute somehow.  Rather than changing `BaseDirectory` above 
> and then changing the code here to compensate, could you only change the code 
> that writes out the `MODULE_DIRECTORY` record to write a relative path?
So we also several non-module-home relative paths get processed here, so just I 
think we'd have to throw away the output of `cleanPathForOutput` if the 
`Changed == false`, to preserve the input relative path when 
non-module-home-relative?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D51568/new/

https://reviews.llvm.org/D51568



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

Sorry for the delay.  Just catching up on the code this covers, so apologies if 
the questions don't make sense.




Comment at: lib/Sema/SemaDeclCXX.cpp:9214-9215
 getStdNamespace()->setImplicit(true);
+if (getLangOpts().Modules)
+  getStdNamespace()->setHasExternalVisibleStorage(true);
   }

I think you mentioned in another forum that one side effect of this change is 
that it's causing multiple candidates for names in `std`.  Is this the implicit 
align constructros/destructors?  Is this because we're marking the implicit 
namespace as being externally visible?



Comment at: lib/Serialization/ASTReader.cpp:9291-9295
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
PendingDeclChains[I].second);
 PendingDeclChains.clear();

How does modules normally "connect up" definitions of the same namspace 
occurring in the imported module?  Is it done here (e.g. so that a name lookup 
will search all namespace definitions)?  Is an alternate solution to this diff 
to make sure this handles the `std` implicit special case?  Apologies for the 
naivety here -- still getting up to speed on this code.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58920/new/

https://reviews.llvm.org/D58920



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51568: [modules] Add `-fno-absolute-module-directory` flag for relocatable modules

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 166709.
andrewjcg added a comment.

Dropping the module directory entirely and fully resolving paths on 
serialization
broke some things during deserialization, specifically when the deserializer 
wanted
to update paths to use an alternate module directory.

This switches to a different strategy of only relativizing the paths that are
actually under the module home dir, and adding a bit to the serialized paths to
indiciate this.  This bit is read on deserializiation to determine whether the
path is resolved against the module directory or not.


Repository:
  rC Clang

https://reviews.llvm.org/D51568

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Lex/HeaderSearchOptions.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ASTWriter.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderInternals.h
  lib/Serialization/ASTWriter.cpp
  lib/Serialization/GlobalModuleIndex.cpp
  test/Modules/relocatable-modules.modulemap

Index: test/Modules/relocatable-modules.modulemap
===
--- /dev/null
+++ test/Modules/relocatable-modules.modulemap
@@ -0,0 +1,29 @@
+// Build two otherwise identical modules in two different directories and
+// verify that using `-fno-absolute-module-directory` makes them identical.
+//
+// RUN: rm -rf %t
+//
+// RUN: mkdir -p %t/p1
+// RUN: cd %t/p1
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fno-absolute-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: mkdir -p %t/p2
+// RUN: cd %t/p2
+// RUN: mkdir -p main other
+// RUN: grep "" %s > main/a.modulemap
+// RUN: grep "" %s > main/a.h
+// RUN: grep "" %s > other/b.h
+// RUN: %clang_cc1 -x c++ -fmodules -emit-module -fno-absolute-module-directory \
+// RUN:   -fmodule-name="a" -Imain -I. -o - main/a.modulemap > a.pcm
+//
+// RUN: diff %t/p1/a.pcm %t/p2/a.pcm
+
+module "a" {// 
+}   // 
+
+#include "b.h"  // 
Index: lib/Serialization/GlobalModuleIndex.cpp
===
--- lib/Serialization/GlobalModuleIndex.cpp
+++ lib/Serialization/GlobalModuleIndex.cpp
@@ -628,6 +628,7 @@
 SmallString<128> ImportedFile(Record.begin() + Idx,
   Record.begin() + Idx + Length);
 Idx += Length;
+Idx++;  // Relative
 
 // Find the imported module file.
 const FileEntry *DependsOnFile
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -1327,9 +1327,14 @@
 ///
 /// \return \c true if the path was changed.
 static bool cleanPathForOutput(FileManager &FileMgr,
-   SmallVectorImpl &Path) {
-  bool Changed = FileMgr.makeAbsolutePath(Path);
-  return Changed | llvm::sys::path::remove_dots(Path);
+   SmallVectorImpl &Path,
+   bool MakeAbsolute = true) {
+  bool Changed = false;
+  if (MakeAbsolute) {
+Changed |= FileMgr.makeAbsolutePath(Path);
+  }
+  Changed |= llvm::sys::path::remove_dots(Path);
+  return Changed;
 }
 
 /// Adjusts the given filename to only write out the portion of the
@@ -1493,7 +1498,10 @@
 
   if (WritingModule && WritingModule->Directory) {
 SmallString<128> BaseDir(WritingModule->Directory->getName());
-cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
+cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir,
+   !PP.getHeaderSearchInfo()
+.getHeaderSearchOpts()
+.NoAbsoluteModuleDirectory);
 
 // If the home of the module is the current working directory, then we
 // want to pick up the cwd of the build process loading the module, not
@@ -1708,6 +1716,7 @@
 auto FileAbbrev = std::make_shared();
 FileAbbrev->Add(BitCodeAbbrevOp(ORIGINAL_FILE));
 FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File ID
+FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relative
 FileAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
 unsigned FileAbbrevCode = Stream.EmitAbbrev(std::move(FileAbbrev));
 
@@ -1772,6 +1781,7 @@
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Overridden
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Transient
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Module map
+  IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Relative
   IFAbbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // File name
   unsigned IF

[PATCH] D52956: Support `-fno-visibility-inlines-hidden`

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added subscribers: cfe-commits, eraman.

Undoes `-fvisibility-inlines-hidden`.

Test Plan: added test


Repository:
  rC Clang

https://reviews.llvm.org/D52956

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/Driver/visibility-inlines-hidden.cpp


Index: test/Driver/visibility-inlines-hidden.cpp
===
--- /dev/null
+++ test/Driver/visibility-inlines-hidden.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -S -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-1 %s < %t.log
+// CHECK-1: "-fvisibility-inlines-hidden"
+// CHECK-1-NOT: "-fno-visibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-2 %s < %t.log
+// CHECK-2: "-fno-visibility-inlines-hidden"
+// CHECK-2-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fvisibility-inlines-hidden 
-fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-3 %s < %t.log
+// CHECK-3: "-fno-visibility-inlines-hidden"
+// CHECK-3-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden 
-fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-4 %s < %t.log
+// CHECK-4: "-fvisibility-inlines-hidden"
+// CHECK-4-NOT: "-fno-visibility-inlines-hidden"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2310,8 +2310,11 @@
 Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
-  if (Args.hasArg(OPT_fvisibility_inlines_hidden))
+  if (Args.hasFlag(OPT_fvisibility_inlines_hidden,
+   OPT_fno_visibility_inlines_hidden,
+   false)) {
 Opts.InlineVisibilityHidden = 1;
+  }
 
   if (Args.hasArg(OPT_ftrapv)) {
 Opts.setSignedOverflowBehavior(LangOptions::SOB_Trapping);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4167,7 +4167,8 @@
 }
   }
 
-  Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden);
+  Args.AddLastArg(CmdArgs, options::OPT_fvisibility_inlines_hidden,
+  options::OPT_fno_visibility_inlines_hidden);
 
   Args.AddLastArg(CmdArgs, options::OPT_ftlsmodel_EQ);
 
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1714,6 +1714,9 @@
 def fvisibility_inlines_hidden : Flag<["-"], "fvisibility-inlines-hidden">, 
Group,
   HelpText<"Give inline C++ member functions hidden visibility by default">,
   Flags<[CC1Option]>;
+def fno_visibility_inlines_hidden : Flag<["-"], 
"fno-visibility-inlines-hidden">, Group,
+  HelpText<"Do not give inline C++ member functions hidden visibility by 
default">,
+  Flags<[CC1Option]>;
 def fvisibility_ms_compat : Flag<["-"], "fvisibility-ms-compat">, 
Group,
   HelpText<"Give global types 'default' visibility and global functions and "
"variables 'hidden' visibility by default">;


Index: test/Driver/visibility-inlines-hidden.cpp
===
--- /dev/null
+++ test/Driver/visibility-inlines-hidden.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -### -S -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-1 %s < %t.log
+// CHECK-1: "-fvisibility-inlines-hidden"
+// CHECK-1-NOT: "-fno-visibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-2 %s < %t.log
+// CHECK-2: "-fno-visibility-inlines-hidden"
+// CHECK-2-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fvisibility-inlines-hidden -fno-visibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-3 %s < %t.log
+// CHECK-3: "-fno-visibility-inlines-hidden"
+// CHECK-3-NOT: "-fvisibility-inlines-hidden"
+
+// RUN: %clang -### -S -fno-visibility-inlines-hidden -fvisibility-inlines-hidden %s 2> %t.log
+// RUN: FileCheck -check-prefix=CHECK-4 %s < %t.log
+// CHECK-4: "-fvisibility-inlines-hidden"
+// CHECK-4-NOT: "-fno-visibility-inlines-hidden"
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2310,8 +2310,11 @@
 Opts.setTypeVisibilityMode(Opts.getValueVisibilityMode());
   }
 
-  if (Args.hasArg(OPT_fvisibility_inlines_hidden))
+  if (Args.hasFlag(OPT_fvisibility_inlines_hidden,
+   OPT_fno_visibility_inlines_hidden,
+   false)) {
 Opts.InlineVisibilityHidden = 1;
+  }
 
   if

[PATCH] D52956: Support `-fno-visibility-inlines-hidden`

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.
Herald added a project: clang.

Sorry for the delay here, but this should be ready to go.

As this is my first accepted diff to LLVM, should I follow 
https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access to get 
commit access, or is there some other process for first-time committers?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D52956/new/

https://reviews.llvm.org/D52956



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85084: [modules] Repro for pure virtual base class method crash

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
andrewjcg requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85084

Files:
  clang/test/Modules/Inputs/set-pure-crash/a.h
  clang/test/Modules/Inputs/set-pure-crash/b.h
  clang/test/Modules/Inputs/set-pure-crash/c.h
  clang/test/Modules/Inputs/set-pure-crash/module.modulemap
  clang/test/Modules/set-pure-crash.cpp


Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps 
-fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = func();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct func {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();


Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = func();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct func {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83154: clang: Add -fcoverage-prefix-map

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added inline comments.



Comment at: clang/lib/CodeGen/CoverageMappingGen.cpp:1334
+  llvm::SmallString<256> Path(Filename);
+  llvm::sys::fs::make_absolute(Path);
+  llvm::sys::path::remove_dots(Path, /*remove_dot_dot=*/true);

keith wrote:
> rnk wrote:
> > keith wrote:
> > > rnk wrote:
> > > > Please only make the path absolute if nothing in the prefix map 
> > > > matches. Otherwise, the user must embed the CWD into the prefix map, 
> > > > which is needlessly difficult for the build system. I believe it is 
> > > > also consistent with the way that the debug info prefix map works. It 
> > > > appears to operate on the possibly relative source paths received on 
> > > > the command line (-I...).
> > > Are you suggesting that I try to remap them relatively, and if that 
> > > fails, absolutize it and attempt the remapping again? Or don't absolutize 
> > > them at all anymore?
> > I'd prefer to do the remapping once without any absolutization, and if that 
> > fails, preserve the behavior of absolutizing.
> > 
> > It would be my preference to not absolutize paths at all, since that is 
> > what is done for debug info. However, @vsk implemented things this way, and 
> > I do understand that this is convenient default behavior for most users: 
> > the paths in the coverage are valid anywhere on their system. So, changing 
> > this behavior is out of scope for this patch.
> I'm not sure how this changed would work for our use case. With bazel the 
> usage of this works something like:
> 
> 1. Relative paths are passed to the compiler `clang ... foo.c`
> 2. We would normally do `-fprofile-prefix-map=$PWD=.` to remap them
> 
> I think if we made this change, we would either have to:
> 
> 1. Make the paths we pass absolute, which we couldn't do for reproducibility
> 2. Add some known prefix to the paths, like `.`, so we could 
> `-fprofile-prefix-map=.=.` just to avoid this absolutizing codepath
> 
> So I think without actually removing the absolutizing behavior at the same 
> time, this wouldn't work as we'd hope.
> 
> Let me know if I'm mis-understanding your suggestion!
> 
> If we did what I said above, I think it would work since relative path 
> remapping would fail, but then prefix mapping the absolute path would work.
FWIW, there's a similar issue with doing the absolutizing for the Buck build 
tool, which by default sets `-fdebug-compilation-dir=.` for all compilations, 
then expects to use `-fdebug-prefix-map` (or `-ffile-prefix-map`) to fixup 
relative paths of sandboxed header symlink trees to point to their *real* paths 
(e.g. something like `clang -c -fdebug-compilation-dir=. -Iheader-tree-sandbox 
-ffile-prefix-map=header-tree-sandbox=original_dir`) (e.g. 
https://github.com/facebook/buck/blob/master/src/com/facebook/buck/cxx/toolchain/PrefixMapDebugPathSanitizer.java#L134).

So, in this case, *not* absolutizing would help here, but it kind of feels that 
the real issue is that there's not an equivalent `-fprofile-compilation-dir` to 
opt-out of the absolutizing of profile paths (which is orthogonal to this 
change)...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83154/new/

https://reviews.llvm.org/D83154

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
andrewjcg requested review of this revision.

In some build environments/systems, flags for explicit module map files
may be propagated up to dependents which may not choose to enable use of
modules (e.g. if a particular compilation doesn't currently work in
modular builds).

In this case, the explicit module files are still passed, even though
they're not used (and can trigger additional I/O at header search path,
like realpath'ing include roots if an explicit  module map contains a
umbrella dir).

This diff avoids parsing/loading these module map files if modules
haven't been enabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86802

Files:
  clang/lib/Frontend/FrontendAction.cpp


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file


Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

> Can you add a simple testcase to prove the point of the change?

Yup, will do!




Comment at: clang/lib/Frontend/FrontendAction.cpp:814
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else

bruno wrote:
> Is this clang-formatted? 
Ahh, yeah.  Should I avoid this sort of thing (and e.g. instead do this in 
explicit codemod/refactoring diffs)?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86802/new/

https://reviews.llvm.org/D86802

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

> How about a malformed module map is not loaded and gives no errors?

Heh yeah, was thinking the same :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86802/new/

https://reviews.llvm.org/D86802

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 288695.
andrewjcg added a comment.

add test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86802/new/

https://reviews.llvm.org/D86802

Files:
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
  clang/test/Modules/explicit-module-maps.cpp


Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,5 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: %clang_cc1 
-fmodule-map-file=%S/Inputs/explicit-module-maps/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
@@ -0,0 +1 @@
+DOES NOT PARSE!
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file


Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,5 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: %clang_cc1 -fmodule-map-file=%S/Inputs/explicit-module-maps/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-module-maps/invalid.modulemap
@@ -0,0 +1 @@
+DOES NOT PARSE!
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86853: [modules] Repro for pure virtual base class method crash

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added subscribers: cfe-commits, danielkiss.
Herald added a project: clang.
andrewjcg requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86853

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/set-pure-crash/a.h
  clang/test/Modules/Inputs/set-pure-crash/b.h
  clang/test/Modules/Inputs/set-pure-crash/c.h
  clang/test/Modules/Inputs/set-pure-crash/module.modulemap
  clang/test/Modules/set-pure-crash.cpp

Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -O0 -emit-llvm -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct simple {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -864,7 +864,10 @@
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1012,6 +1015,10 @@
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector Params;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 289236.
andrewjcg added a comment.

simplify test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86802/new/

https://reviews.llvm.org/D86802

Files:
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Modules/explicit-module-maps.cpp


Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,8 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
+// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file


Index: clang/test/Modules/explicit-module-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-module-maps.cpp
@@ -0,0 +1,8 @@
+// Test that an explicit module map (which fails to parse/load in this case)
+// isn't used if modules aren't enabled.
+
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo 'DOES NOT PARSE!' > %t/invalid.modulemap
+// RUN: %clang_cc1 -fmodule-map-file=%t/invalid.modulemap -verify %s
+// expected-no-diagnostics
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -807,12 +807,14 @@
 goto failure;
 
   // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  *File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  if (CI.getLangOpts().Modules) {
+for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+  if (auto File = CI.getFileManager().getFile(Filename))
+CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+*File, /*IsSystem*/ false);
+  else
+CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+}
   }
 
   // Add a module declaration scope so that modules from -fmodule-map-file
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86802: [Modules] Don't parse/load explicit module maps if modules are disabled

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg abandoned this revision.
andrewjcg added a comment.

Ahh, I see, make sense.

The motivating issue was due to an apparent bug where realpaths in umbrella dir 
support for module map files get leaked into dep files for includes starting 
with `..` (e.g. `#include "../foo.h"`) in non-modular builds.

I'll try get a repo together for a bug report for that issue instead.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86802/new/

https://reviews.llvm.org/D86802

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D86853: [modules] Fix crash in call to `FunctionDecl::setPure()`

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 289506.
andrewjcg marked 2 inline comments as done.
andrewjcg added a comment.

feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86853/new/

https://reviews.llvm.org/D86853

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/test/Modules/Inputs/set-pure-crash/a.h
  clang/test/Modules/Inputs/set-pure-crash/b.h
  clang/test/Modules/Inputs/set-pure-crash/c.h
  clang/test/Modules/Inputs/set-pure-crash/module.modulemap
  clang/test/Modules/set-pure-crash.cpp

Index: clang/test/Modules/set-pure-crash.cpp
===
--- /dev/null
+++ clang/test/Modules/set-pure-crash.cpp
@@ -0,0 +1,9 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fsyntax-only -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -x c++ -I %S/Inputs/set-pure-crash -verify %s -o %t
+
+// expected-no-diagnostics
+
+#include "b.h"
+#include "c.h"
+
+auto t = simple();
Index: clang/test/Modules/Inputs/set-pure-crash/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/module.modulemap
@@ -0,0 +1,11 @@
+module a {
+  header "a.h"
+}
+
+module b {
+  header "b.h"
+}
+
+module c {
+  header "c.h"
+}
Index: clang/test/Modules/Inputs/set-pure-crash/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/c.h
@@ -0,0 +1,5 @@
+#pragma once
+
+template 
+struct simple {
+};
Index: clang/test/Modules/Inputs/set-pure-crash/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/b.h
@@ -0,0 +1,14 @@
+#pragma once
+
+#include "a.h"
+#include "c.h"
+
+template >
+void foo(Fun) {}
+
+class Child : public Base {
+public:
+  void func() {
+foo([]() {});
+  }
+};
Index: clang/test/Modules/Inputs/set-pure-crash/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/set-pure-crash/a.h
@@ -0,0 +1,11 @@
+#pragma once
+
+struct Tag {};
+
+template 
+class Base {
+public:
+  virtual void func() = 0;
+};
+
+Base bar();
Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -864,7 +864,10 @@
   FD->setInlineSpecified(Record.readInt());
   FD->setImplicitlyInline(Record.readInt());
   FD->setVirtualAsWritten(Record.readInt());
-  FD->setPure(Record.readInt());
+  // We defer calling `FunctionDecl::setPure()` here as for methods of
+  // `CXXTemplateSpecializationDecl`s, we may not have connected up the
+  // definition (which is required for `setPure`).
+  const bool Pure = Record.readInt();
   FD->setHasInheritedPrototype(Record.readInt());
   FD->setHasWrittenPrototype(Record.readInt());
   FD->setDeletedAsWritten(Record.readInt());
@@ -1012,6 +1015,10 @@
   }
   }
 
+  // Defer calling `setPure` until merging above has guaranteed we've set
+  // `DefinitionData` (as this will need to access it).
+  FD->setPure(Pure);
+
   // Read in the parameters.
   unsigned NumParams = Record.readInt();
   SmallVector Params;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added a subscriber: wenlei.
andrewjcg updated this revision to Diff 350757.
andrewjcg added a comment.
andrewjcg edited the summary of this revision.
andrewjcg added reviewers: bruno, rsmith.
andrewjcg published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

add tests


Previously, if a header was found via in a header map, and not just remapped.
we wouldn't also find the module it maps to when using implicit modules (for
module maps that were explicitly loaded).

This diff just updates these code paths to also locate the owning module via
`findUsableModuleForHeader`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json 
%T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap 
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s 
-I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" 
%S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap 
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s 
-I %T/hmap
+
+#define FOO
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -438,10 +438,22 @@
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
   FixupSearchPath();
+  if (!HS.findUsableModuleForHeader(&Result->getFileEntry(),
+Result->getFileEntry().getDir(),
+RequestingModule, SuggestedModule,
+isSystemHeaderDirectory())) {
+return None;
+  }
   return *Result;
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
+if (!HS.findUsableModuleForHeader(&Res->getFileEntry(),
+  Res->getFileEntry().getDir(),
+  RequestingModule, SuggestedModule,
+  isSystemHeaderDirectory())) {
+  return None;
+}
 return *Res;
   }
 


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

We were hitting this in our build environment when mixing header maps with 
clang module maps, where the use of the former would prevent properly 
associated an included header with it's module via the module map.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added inline comments.



Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+#include "Before/Mapping.h"

This include will fail if modules weren't used.

The include name itself doesn't exist and relies on the header map to remap it 
to the real header.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 350767.
andrewjcg added a comment.

lint


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json 
%T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap 
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s 
-I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" 
%S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap 
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s 
-I %T/hmap
+
+#define FOO
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -438,10 +438,20 @@
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
   FixupSearchPath();
+  if (!HS.findUsableModuleForHeader(
+  &Result->getFileEntry(), Result->getFileEntry().getDir(),
+  RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+return None;
+  }
   return *Result;
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
+if (!HS.findUsableModuleForHeader(
+&Res->getFileEntry(), Res->getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
 return *Res;
   }
 


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s -I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Input

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 351195.
andrewjcg added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json 
%T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap 
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s 
-I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" 
%S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap 
%T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules 
-fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s 
-I %T/hmap
+
+#define FOO
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -438,10 +438,20 @@
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
   FixupSearchPath();
+  if (!HS.findUsableModuleForHeader(
+  &Result->getFileEntry(), Result->getFileEntry().getDir(),
+  RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+return None;
+  }
   return *Result;
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
 FixupSearchPath();
+if (!HS.findUsableModuleForHeader(
+&Res->getFileEntry(), Res->getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
 return *Res;
   }
 


Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,27 @@
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %T/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %T/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%T/module.modulemap -fsyntax-only %s -I %T/hmap
+//
+// RUN: rm -rf %T
+// RUN: mkdir %T
+// RUN: cd %T
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%T:g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %T/hmap.json
+// RUN: %hmaptool write %T/hmap.json %T/hmap
+//
+// RUN: mkdir -p %T/After
+// RUN: cp %S/Inp

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

Hmm, I can't repro the module test failures locally.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352299.
andrewjcg added a comment.

feedback


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s:OUTPUTS_DIR:%t:g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&file.getFileEntry(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return file;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352740.
andrewjcg added a comment.

fix sed for windows test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&file.getFileEntry(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return file;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added inline comments.



Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

dexonsmith wrote:
> Do you really mean if modules aren't used, or do you mean if header maps 
> aren't used?
> 
> (I think we want to find the same headers on disk whether or not modules are 
> on... if this patch changes that, then I guess I'm not totally understanding 
> why...)
Ahh, meant if header maps aren't used.  Will fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added inline comments.



Comment at: clang/test/Modules/implicit-module-header-maps.cpp:27
+#define FOO
+// This include will fail if modules weren't used.  The include name itself
+// doesn't exist and relies on the header map to remap it to the real header.

andrewjcg wrote:
> dexonsmith wrote:
> > Do you really mean if modules aren't used, or do you mean if header maps 
> > aren't used?
> > 
> > (I think we want to find the same headers on disk whether or not modules 
> > are on... if this patch changes that, then I guess I'm not totally 
> > understanding why...)
> Ahh, meant if header maps aren't used.  Will fix.
Ah no wait.  The include should fail if either header maps or modules isn't 
used (header maps required for the remapping and modules required to prevent 
the `FOO` define from propagating into the included header and triggering the 
`#error`).  

Will update the comment.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352824.
andrewjcg added a comment.

fix comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto file) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&file.getFileEntry(), file.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return file;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 352898.
andrewjcg added a comment.

capitalize param


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

Files:
  clang/lib/Lex/HeaderSearch.cpp
  clang/test/Modules/Inputs/implicit-module-header-maps/a.h
  clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
  clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
  clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
  clang/test/Modules/implicit-module-header-maps.cpp

Index: clang/test/Modules/implicit-module-header-maps.cpp
===
--- /dev/null
+++ clang/test/Modules/implicit-module-header-maps.cpp
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: %hmaptool write %S/Inputs/implicit-module-header-maps/a.hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+//
+// RUN: sed -e "s|OUTPUTS_DIR|%t|g" %S/Inputs/implicit-module-header-maps/b.hmap.json > %t/hmap.json
+// RUN: %hmaptool write %t/hmap.json %t/hmap
+//
+// RUN: mkdir -p %t/After
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.h %t/After/Mapping.h
+// RUN: cp %S/Inputs/implicit-module-header-maps/a.module.modulemap %t/module.modulemap
+//
+// RUN: %clang -Rmodule-build -fmodules -fimplicit-modules -fimplicit-module-maps -fmodule-map-file=%t/module.modulemap -fsyntax-only %s -I %t/hmap
+
+#define FOO
+// This include will fail if:
+// 1) modules are't used, as the `FOO` define will propagate into the included
+//header and trip a `#error`, or
+// 2) header maps aren't usesd, as the header name doesn't exist and relies on
+//the header map to remap it to the real header.
+#include "Before/Mapping.h"
Index: clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/b.hmap.json
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "OUTPUTS_DIR/After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.module.modulemap
@@ -0,0 +1,3 @@
+module a {
+  header "After/Mapping.h"
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.hmap.json
@@ -0,0 +1,7 @@
+{
+  "mappings" :
+{
+ "Before/Mapping.h" : "After/Mapping.h",
+ "After/Mapping.h" : "After/Mapping.h"
+}
+}
Index: clang/test/Modules/Inputs/implicit-module-header-maps/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/implicit-module-header-maps/a.h
@@ -0,0 +1,3 @@
+#ifdef FOO
+#error foo
+#endif
Index: clang/lib/Lex/HeaderSearch.cpp
===
--- clang/lib/Lex/HeaderSearch.cpp
+++ clang/lib/Lex/HeaderSearch.cpp
@@ -417,7 +417,8 @@
 
   IsInHeaderMap = true;
 
-  auto FixupSearchPath = [&]() {
+  auto FixupSearchPathAndFindUsableModule =
+  [&](auto File) -> Optional {
 if (SearchPath) {
   StringRef SearchPathRef(getName());
   SearchPath->clear();
@@ -427,6 +428,12 @@
   RelativePath->clear();
   RelativePath->append(Filename.begin(), Filename.end());
 }
+if (!HS.findUsableModuleForHeader(
+&File.getFileEntry(), File.getFileEntry().getDir(),
+RequestingModule, SuggestedModule, isSystemHeaderDirectory())) {
+  return None;
+}
+return File;
   };
 
   // Check if the headermap maps the filename to a framework include
@@ -437,12 +444,10 @@
 Filename = StringRef(MappedName.begin(), MappedName.size());
 Optional Result = HM->LookupFile(Filename, HS.getFileMgr());
 if (Result) {
-  FixupSearchPath();
-  return *Result;
+  return FixupSearchPathAndFindUsableModule(*Result);
 }
   } else if (auto Res = HS.getFileMgr().getOptionalFileRef(Dest)) {
-FixupSearchPath();
-return *Res;
+return FixupSearchPathAndFindUsableModule(*Res);
   }
 
   return None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70219: Make `-fmodule-file==` apply to .pcm file compilations

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When precompiling a header module, `-fmodule-file=name>=` flags
can be used to provide an updated path to a module, which allows modules
to be moved from the location they were compiled against.

This diff makes these flags also apply to when compiling a `.pcm` file
into a `.o`.


Repository:
  rC Clang

https://reviews.llvm.org/D70219

Files:
  include/clang/Frontend/ASTUnit.h
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/FrontendAction.cpp
  test/Modules/Inputs/explicit-build-pcm/a.h
  test/Modules/Inputs/explicit-build-pcm/b.h
  test/Modules/Inputs/explicit-build-pcm/c.h
  test/Modules/Inputs/explicit-build-pcm/d.h
  test/Modules/Inputs/explicit-build-pcm/module.modulemap
  test/Modules/explicit-build-pcm.cpp

Index: test/Modules/explicit-build-pcm.cpp
===
--- /dev/null
+++ test/Modules/explicit-build-pcm.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-name=a -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/a.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-name=b -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/b.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-file=b=%t/b.pcm \
+// RUN:-fmodule-name=c -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/c.pcm
+
+// RUN: mv %t/a.pcm %t/a.moved.pcm
+// RUN: mv %t/b.pcm %t/b.moved.pcm
+// RUN: mv %t/c.pcm %t/c.moved.pcm
+//
+// RUN: %clang_cc1 -x pcm -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.moved.pcm \
+// RUN:-fmodule-file=b=%t/b.moved.pcm \
+// RUN:%t/c.moved.pcm -o %t/c.o
+// expected-no-diagnostics
+
+#include 
Index: test/Modules/Inputs/explicit-build-pcm/module.modulemap
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/module.modulemap
@@ -0,0 +1,4 @@
+module a { header "a.h" }
+module b { header "b.h" export * }
+module c { header "c.h" export * }
+module d { header "d.h" }
Index: test/Modules/Inputs/explicit-build-pcm/c.h
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/c.h
@@ -0,0 +1,7 @@
+#include "b.h"
+
+#if !__building_module(c)
+#error "should only get here when building module c"
+#endif
+
+const int c = 3;
Index: test/Modules/Inputs/explicit-build-pcm/b.h
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/b.h
@@ -0,0 +1,7 @@
+#include "a.h"
+
+#if !__building_module(b)
+#error "should only get here when building module b"
+#endif
+
+const int b = 2;
Index: test/Modules/Inputs/explicit-build-pcm/a.h
===
--- /dev/null
+++ test/Modules/Inputs/explicit-build-pcm/a.h
@@ -0,0 +1,5 @@
+#if !__building_module(a) && !BUILDING_A_PCH
+#error "should only get here when building module a"
+#endif
+
+const int a = 1;
Index: lib/Frontend/FrontendAction.cpp
===
--- lib/Frontend/FrontendAction.cpp
+++ lib/Frontend/FrontendAction.cpp
@@ -564,7 +564,8 @@
 
 std::unique_ptr AST = ASTUnit::LoadFromASTFile(
 InputFile, CI.getPCHContainerReader(), ASTUnit::LoadPreprocessorOnly,
-ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs,
+false, None, CaptureDiagsKind::None, false, false, CI.getHeaderSearchOpts().PrebuiltModuleFiles);
 if (!AST)
   goto failure;
 
@@ -630,7 +631,8 @@
 
 std::unique_ptr AST = ASTUnit::LoadFromASTFile(
 InputFile, CI.getPCHContainerReader(), ASTUnit::LoadEverything, Diags,
-CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs,
+false, None, CaptureDiagsKind::None, false, false, CI.getHeaderSearchOpts().PrebuiltModuleFiles);
 
 if (!AST)
   goto failure;
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@
 const FileSystemOptions &FileSystemOpts, bo

[PATCH] D70219: Make `-fmodule-file==` apply to .pcm file compilations

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg updated this revision to Diff 232190.
andrewjcg added a comment.

rebase onto monorepo and clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70219/new/

https://reviews.llvm.org/D70219

Files:
  clang/include/clang/Frontend/ASTUnit.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/test/Modules/Inputs/explicit-build-pcm/a.h
  clang/test/Modules/Inputs/explicit-build-pcm/b.h
  clang/test/Modules/Inputs/explicit-build-pcm/c.h
  clang/test/Modules/Inputs/explicit-build-pcm/d.h
  clang/test/Modules/Inputs/explicit-build-pcm/module.modulemap
  clang/test/Modules/explicit-build-pcm.cpp

Index: clang/test/Modules/explicit-build-pcm.cpp
===
--- /dev/null
+++ clang/test/Modules/explicit-build-pcm.cpp
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t
+// expected-no-diagnostics
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-name=a -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/a.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-name=b -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/b.pcm
+
+// RUN: %clang_cc1 -x c++ -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.pcm \
+// RUN:-fmodule-file=b=%t/b.pcm \
+// RUN:-fmodule-name=c -emit-module %S/Inputs/explicit-build/module.modulemap -o %t/c.pcm
+
+// RUN: mv %t/a.pcm %t/a.moved.pcm
+// RUN: mv %t/b.pcm %t/b.moved.pcm
+// RUN: mv %t/c.pcm %t/c.moved.pcm
+//
+// RUN: %clang_cc1 -x pcm -std=c++11 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -Rmodule-build -fno-modules-error-recovery \
+// RUN:-fmodule-file=a=%t/a.moved.pcm \
+// RUN:-fmodule-file=b=%t/b.moved.pcm \
+// RUN:%t/c.moved.pcm -o %t/c.o
+// expected-no-diagnostics
+
+#include 
Index: clang/test/Modules/Inputs/explicit-build-pcm/module.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/module.modulemap
@@ -0,0 +1,4 @@
+module a { header "a.h" }
+module b { header "b.h" export * }
+module c { header "c.h" export * }
+module d { header "d.h" }
Index: clang/test/Modules/Inputs/explicit-build-pcm/c.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/c.h
@@ -0,0 +1,7 @@
+#include "b.h"
+
+#if !__building_module(c)
+#error "should only get here when building module c"
+#endif
+
+const int c = 3;
Index: clang/test/Modules/Inputs/explicit-build-pcm/b.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/b.h
@@ -0,0 +1,7 @@
+#include "a.h"
+
+#if !__building_module(b)
+#error "should only get here when building module b"
+#endif
+
+const int b = 2;
Index: clang/test/Modules/Inputs/explicit-build-pcm/a.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/explicit-build-pcm/a.h
@@ -0,0 +1,5 @@
+#if !__building_module(a) && !BUILDING_A_PCH
+#error "should only get here when building module a"
+#endif
+
+const int a = 1;
Index: clang/lib/Frontend/FrontendAction.cpp
===
--- clang/lib/Frontend/FrontendAction.cpp
+++ clang/lib/Frontend/FrontendAction.cpp
@@ -565,7 +565,9 @@
 
 std::unique_ptr AST = ASTUnit::LoadFromASTFile(
 InputFile, CI.getPCHContainerReader(), ASTUnit::LoadPreprocessorOnly,
-ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+ASTDiags, CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs,
+false, None, CaptureDiagsKind::None, false, false,
+CI.getHeaderSearchOpts().PrebuiltModuleFiles);
 if (!AST)
   goto failure;
 
@@ -631,7 +633,9 @@
 
 std::unique_ptr AST = ASTUnit::LoadFromASTFile(
 InputFile, CI.getPCHContainerReader(), ASTUnit::LoadEverything, Diags,
-CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs);
+CI.getFileSystemOpts(), CI.getCodeGenOpts().DebugTypeExtRefs, false,
+None, CaptureDiagsKind::None, false, false,
+CI.getHeaderSearchOpts().PrebuiltModuleFiles);
 
 if (!AST)
   goto failure;
Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@
 const FileSystemOptions &FileSystemOpts, bool UseDebugInfo,
 

[PATCH] D103930: [clang][HeaderSearch] Fix implicit module when using header maps

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

@ivanmurashko: Sorry for the delay getting back to you here.  Feel free to 
commandeer, as I don't have plans to get to this soon.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103930/new/

https://reviews.llvm.org/D103930

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits