[C++20][Modules][HU 2/5] Support searching Header Units in user or system search paths.

[iains]

Summary

This is support for the user-facing options to create importable header units
from headers in the user or system search paths (or to be given an absolute path).

This means that an incomplete header path will be passed by the driver and the
lookup carried out using the search paths present when the front end is run.

To support this, we introduce file fypes for c++-{user,system,header-unit}-header.
These terms are the same as the ones used by GCC, to minimise the differences for
tooling (and users).

The preprocessor checks for headers before issuing a warning for
"#pragma once" in a header build. We ensure that the importable header units
are recognised as headers in order to avoid such warnings.

[iains]

amend a testcase.

I reordered and squashed two of the patches before posting, and failed to spot that a test case had
a (non-critical) dependency on functionality introduced by a later patch. Fixed by removing the
dependency.

[ChuanqiXu]

clang/include/clang/Frontend/FrontendOptions.h

157

I prefer IsHeader

252

How about getHeaderUnitKind ?

clang/lib/Frontend/CompilerInvocation.cpp

2788–2791

How do you think about the suggested changes? I feel it is less confusing.

2796–2797

Oh, now I find Header and HeaderUnit might be a little bit confusing. It looks like Header should include HeaderUnit at the first sight. It is not true. But I don't have only suggestions...

2838–2839

2855–2857

So the compiler would crash if there is multiple inputs for header unit? I feel it is not most friendly. How about emitting an error here?

clang/lib/Frontend/FrontendAction.cpp

814–816

Is it same as the suggested?

[ChuanqiXu]

BTW, all the tests uses `clang_cc1. How should the users do with clang? Could them use -Xclang only?

[iains]

In D121096#3368996, @ChuanqiXu wrote:

BTW, all the tests uses `clang_cc1. How should the users do with clang? Could them use -Xclang only?

The driver changes are in separate patches (they will make the same user-facing interface options as used by GCC). I wanted to have the FE implementation available so that the driver has something to drive.

[ChuanqiXu]

In D121096#3369277, @iains wrote:

In D121096#3368996, @ChuanqiXu wrote:

BTW, all the tests uses `clang_cc1. How should the users do with clang? Could them use -Xclang only?

The driver changes are in separate patches (they will make the same user-facing interface options as used by GCC). I wanted to have the FE implementation available so that the driver has something to drive.

Makes sense to me. Thanks for the explanation.

[iains]

rebased, address review comments.

clang/include/clang/Frontend/FrontendOptions.h

157

OK.

252

OK. I prefer more descriptive names, but they do get rather long sometimes.

clang/lib/Frontend/CompilerInvocation.cpp

2788–2791

OK,

2796–2797

We have an amount of existing handling specific to "generic headers".
(header unit = none, header = true)

The intention is that HeaderUnitKind is used to disambiguate the cases we are dealing with C+=20 header units.

I am not sure it would be worth the code churn to replace all uses of header with a HUK.

2855–2857

well the driver is supposed to catch these problems (in the case of C++20 modules mode it will generate several compile jobs, one per header).

I added an error for this tho.

clang/lib/Frontend/FrontendAction.cpp

814–816

not quite because getDirectory() is "llvm::ErrorOr<const DirectoryEntry *>" ...

[ChuanqiXu]

LGTM except we should remove the dead error emitting.

clang/lib/Frontend/CompilerInvocation.cpp

2796–2797

OK, this is a historical problem. It makes no sense require us to fix it in this patch.

2855–2857

Oh, so if the compiler wouldn't crash in that case, I think the original assertion is acceptable. And the current error is dead code, which is bad. Let's take the original assertion.

[iains]

match windows path delimiters in testcase.

clang/lib/Frontend/CompilerInvocation.cpp

2855–2857

actually, it does protect against the case that the frontend is called directly with XClang or so, so it's not dead code, and it is better for the compiler to emit a diagnostic rather than crash (if built with assertions).

[ChuanqiXu]

clang/lib/Frontend/CompilerInvocation.cpp

2855–2857

If it is not dead codes, I feel better if we could add tests to cover it.

[iains]

rebased.

[iains]

rebased, added a testcase for multiple inputs.

clang/include/clang/Frontend/FrontendOptions.h

 } // namespace frontend
 
 /// The kind of a file that we've been handed as an input.
 class InputKind {
 private:
   Language Lang;
   unsigned Fmt : 3;
   unsigned Preprocessed : 1;
+  unsigned HeaderUnit : 3;
+  unsigned Header : 1;
 
 public:
   /// The input file format.
   enum Format {
     Source,
     ModuleMap,
     Precompiled
   };
 
+  // If we are building a header unit, what kind it is; this affects whether
+  // we look for the file in the user or system include search paths before
+  // flagging a missing input.
+  enum HeaderUnitKind {
+    HeaderUnit_None,
+    HeaderUnit_User,
+    HeaderUnit_System,
+    HeaderUnit_Abs
+  };
+
   constexpr InputKind(Language L = Language::Unknown, Format F = Source,
-                      bool PP = false)
-      : Lang(L), Fmt(F), Preprocessed(PP) {}
+                      bool PP = false, HeaderUnitKind HU = HeaderUnit_None,
+                      bool HD = false)
+      : Lang(L), Fmt(F), Preprocessed(PP), HeaderUnit(HU), Header(HD) {}
 
   Language getLanguage() const { return static_cast<Language>(Lang); }
   Format getFormat() const { return static_cast<Format>(Fmt); }
+  HeaderUnitKind getHeaderUnit() const {
+    return static_cast<HeaderUnitKind>(HeaderUnit);
+  }
   bool isPreprocessed() const { return Preprocessed; }
+  bool isHeader() const { return Header; }
+  bool isHeaderUnit() const { return HeaderUnit != HeaderUnit_None; }
 
   /// Is the input kind fully-unknown?
   bool isUnknown() const { return Lang == Language::Unknown && Fmt == Source; }
 
   /// Is the language of the input some dialect of Objective-C?
   bool isObjectiveC() const {
     return Lang == Language::ObjC || Lang == Language::ObjCXX;
   }
 
   InputKind getPreprocessed() const {
-    return InputKind(getLanguage(), getFormat(), true);
+    return InputKind(getLanguage(), getFormat(), true, getHeaderUnit(),
+                     isHeader());
+  }
+
+  InputKind getHeader() const {
+    return InputKind(getLanguage(), getFormat(), isPreprocessed(),
+                     getHeaderUnit(), true);
+  }
+
+  InputKind withHeaderUnit(HeaderUnitKind HU) const {
+    return InputKind(getLanguage(), getFormat(), isPreprocessed(), HU,
+                     isHeader());
   }
 
   InputKind withFormat(Format F) const {
-    return InputKind(getLanguage(), F, isPreprocessed());
+    return InputKind(getLanguage(), F, isPreprocessed(), getHeaderUnit(),
+                     isHeader());
   }
 };
 
 /// An input file for the front end.
 class FrontendInputFile {
   /// The file name, or "-" to read from standard input.
   std::string File;
 
public:
 
   InputKind getKind() const { return Kind; }
   bool isSystem() const { return IsSystem; }
 
   bool isEmpty() const { return File.empty() && Buffer == None; }
   bool isFile() const { return !isBuffer(); }
   bool isBuffer() const { return Buffer != None; }
   bool isPreprocessed() const { return Kind.isPreprocessed(); }
+  bool isHeader() const { return Kind.isHeader(); }
+  InputKind::HeaderUnitKind getHeaderUnit() const {
+    return Kind.getHeaderUnit();
+  }
 
   StringRef getFile() const {
     assert(isFile());
     return File;
   }
 
   llvm::MemoryBufferRef getBuffer() const {
     assert(isBuffer());

clang/lib/Frontend/CompilerInvocation.cpp

#undef FRONTEND_OPTION_WITH_MARSHALLING
   if (Opts.AuxTargetFeatures.hasValue())
     for (const auto &Feature : *Opts.AuxTargetFeatures)
       GenerateArg(Args, OPT_aux_target_feature, Feature, SA);
 
   {
     StringRef Preprocessed = Opts.DashX.isPreprocessed() ? "-cpp-output" : "";
     StringRef ModuleMap =
         Opts.DashX.getFormat() == InputKind::ModuleMap ? "-module-map" : "";
+    StringRef HeaderUnit = "";
+    switch (Opts.DashX.getHeaderUnit()) {
+    case InputKind::HeaderUnit_None:
+      break;
+    case InputKind::HeaderUnit_User:
+      HeaderUnit = "-user";
+      break;
+    case InputKind::HeaderUnit_System:
+      HeaderUnit = "-system";
+      break;
+    case InputKind::HeaderUnit_Abs:
+      HeaderUnit = "-header-unit";
+      break;
+    }
     StringRef Header = IsHeader ? "-header" : "";
 
     StringRef Lang;
     switch (Opts.DashX.getLanguage()) {
     case Language::C:
       Lang = "c";
       break;
     case Language::OpenCL:
case Language::Unknown:
              "Generating -x argument for unknown language (not precompiled).");
       Lang = "ast";
       break;
     case Language::LLVM_IR:
       Lang = "ir";
       break;
     }
 
-    GenerateArg(Args, OPT_x, Lang + Header + ModuleMap + Preprocessed, SA);
+    GenerateArg(Args, OPT_x,
+                Lang + HeaderUnit + Header + ModuleMap + Preprocessed, SA);
   }
 
   // OPT_INPUT has a unique class, generate it directly.
   for (const auto &Input : Opts.Inputs)
     Args.push_back(SA(Input.getFile()));
 }
 
 static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
if (Opts.ARCMTAction != FrontendOptions::ARCMT_None &&
     Diags.Report(diag::err_drv_argument_not_allowed_with)
       << "ARC migration" << "ObjC migration";
   }
 
   InputKind DashX(Language::Unknown);
   if (const Arg *A = Args.getLastArg(OPT_x)) {
     StringRef XValue = A->getValue();
 
-    // Parse suffixes: '<lang>(-header|[-module-map][-cpp-output])'.
+    // Parse suffixes:
+    // '<lang>(-[{header-unit,user,system}-]header|[-module-map][-cpp-output])'.
     // FIXME: Supporting '<lang>-header-cpp-output' would be useful.
     bool Preprocessed = XValue.consume_back("-cpp-output");
     bool ModuleMap = XValue.consume_back("-module-map");
-    IsHeaderFile = !Preprocessed && !ModuleMap &&
-                   XValue != "precompiled-header" &&
-                   XValue.consume_back("-header");
+    // Detect and consume the header indicator.
+    bool IsHeader =
+        XValue != "precompiled-header" && XValue.consume_back("-header");
+
+    // If we have c++-{user,system}-header, that indicates a header unit input
+    // likewise, if the user put -fmodule-header together with a header with an
+    // absolute path (header-unit-header).
+    InputKind::HeaderUnitKind HUK = InputKind::HeaderUnit_None;
+    if (IsHeader || Preprocessed) {
+      HUK =
+          XValue.consume_back("-header-unit") ? InputKind::HeaderUnit_Abs : HUK;
+      HUK = XValue.consume_back("-system") ? InputKind::HeaderUnit_System : HUK;
+      HUK = XValue.consume_back("-user") ? InputKind::HeaderUnit_User : HUK;
+    }
+
+    // The value set by this processing is an un-preprocessed source which is
+    // not intended to be a module map or header unit.
+    IsHeaderFile = IsHeader && !Preprocessed && !ModuleMap &&
+                   HUK == InputKind::HeaderUnit_None;
 
     // Principal languages.
     DashX = llvm::StringSwitch<InputKind>(XValue)
                 .Case("c", Language::C)
                 .Case("cl", Language::OpenCL)
                 .Case("clcpp", Language::OpenCLCXX)
                 .Case("cuda", Language::CUDA)
                 .Case("hip", Language::HIP)
                 .Case("c++", Language::CXX)
                 .Case("objective-c", Language::ObjC)
                 .Case("objective-c++", Language::ObjCXX)
                 .Case("renderscript", Language::RenderScript)
                 .Default(Language::Unknown);
 
     // "objc[++]-cpp-output" is an acceptable synonym for
     // "objective-c[++]-cpp-output".
-    if (DashX.isUnknown() && Preprocessed && !IsHeaderFile && !ModuleMap)
+    if (DashX.isUnknown() && Preprocessed && !IsHeaderFile && !ModuleMap &&
+        HUK == InputKind::HeaderUnit_None)
       DashX = llvm::StringSwitch<InputKind>(XValue)
                   .Case("objc", Language::ObjC)
                   .Case("objc++", Language::ObjCXX)
                   .Default(Language::Unknown);
 
     // Some special cases cannot be combined with suffixes.
-    if (DashX.isUnknown() && !Preprocessed && !ModuleMap && !IsHeaderFile)
+    if (DashX.isUnknown() && !Preprocessed && !IsHeaderFile && !ModuleMap &&
+        HUK == InputKind::HeaderUnit_None)
       DashX = llvm::StringSwitch<InputKind>(XValue)
                   .Case("cpp-output", InputKind(Language::C).getPreprocessed())
                   .Case("assembler-with-cpp", Language::Asm)
                   .Cases("ast", "pcm", "precompiled-header",
                          InputKind(Language::Unknown, InputKind::Precompiled))
                   .Case("ir", Language::LLVM_IR)
                   .Default(Language::Unknown);
 
     if (DashX.isUnknown())
       Diags.Report(diag::err_drv_invalid_value)
         << A->getAsString(Args) << A->getValue();
 
     if (Preprocessed)
       DashX = DashX.getPreprocessed();
+    // A regular header is considered mutually exclusive with a header unit
+    // one
+    if (HUK != InputKind::HeaderUnit_None) {
+      DashX = DashX.withHeaderUnit(HUK);
+      IsHeaderFile = true;
+    } else if (IsHeaderFile)
+      DashX = DashX.getHeader();
     if (ModuleMap)
       DashX = DashX.withFormat(InputKind::ModuleMap);
   }
 
   // '-' is the default input if none is given.
   std::vector<std::string> Inputs = Args.getAllArgValues(OPT_INPUT);
   Opts.Inputs.clear();
   if (Inputs.empty())
     Inputs.push_back("-");
+
+  assert((DashX.getHeaderUnit() == InputKind::HeaderUnit_None ||
+          Inputs.size() == 1) &&
+         "Expected only one input file for header unit");
+
   for (unsigned i = 0, e = Inputs.size(); i != e; ++i) {
     InputKind IK = DashX;
     if (IK.isUnknown()) {
       IK = FrontendOptions::getInputKindForExtension(
         StringRef(Inputs[i]).rsplit('.').second);
       // FIXME: Warn on this?
       if (IK.isUnknown())
         IK = Language::C;
else if (A->getOption().matches(options::OPT_mlong_double_80))
       Opts.LongDoubleSize = 80;
     else if (A->getOption().matches(options::OPT_mlong_double_128))
       Opts.LongDoubleSize = 128;
     else
       Opts.LongDoubleSize = 0;
   }
   if (Opts.FastRelaxedMath)
     Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
+
   llvm::sort(Opts.ModuleFeatures);
 
   // -mrtd option
   if (Arg *A = Args.getLastArg(OPT_mrtd)) {
     if (Opts.getDefaultCallingConv() != LangOptions::DCC_None)
       Diags.Report(diag::err_drv_argument_not_allowed_with)
           << A->getSpelling() << "-fdefault-calling-conv";
     else {

clang/lib/Frontend/FrontendAction.cpp

bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
   if (!isModelParsingAction())
     CI.createPreprocessor(getTranslationUnitKind());
 
   // Inform the diagnostic client we are processing a source file.
   CI.getDiagnosticClient().BeginSourceFile(CI.getLangOpts(),
                                            &CI.getPreprocessor());
   HasBegunSourceFile = true;
 
-  // Initialize the main file entry.
+  // Handle C++20 header units.
+  // Here, the user has the option to specify that the header name should be
+  // looked up in the pre-processor search paths (and the main filename as
+  // passed by the driver might therefore be incomplete until that look-up).
+  if (CI.getLangOpts().CPlusPlusModules && Input.getKind().isHeaderUnit() &&
+      !Input.getKind().isPreprocessed()) {
+    StringRef FileName = Input.getFile();
+    InputKind Kind = Input.getKind();
+    if (Kind.getHeaderUnit() != InputKind::HeaderUnit_Abs) {
+      assert(CI.hasPreprocessor() &&
+             "trying to build a header unit without a Pre-processor?");
+      HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
+      // Relative searches begin from CWD.
+      const DirectoryEntry *Dir = nullptr;
+      if (auto DirOrErr = CI.getFileManager().getDirectory("."))
+        Dir = *DirOrErr;
+      SmallVector<std::pair<const FileEntry *, const DirectoryEntry *>, 1> CWD;
+      CWD.push_back({nullptr, Dir});
+      Optional<FileEntryRef> FE =
+          HS.LookupFile(FileName, SourceLocation(),
+                        /*Angled*/ Input.getKind().getHeaderUnit() ==
+                            InputKind::HeaderUnit_System,
+                        nullptr, nullptr, CWD, nullptr, nullptr, nullptr,
+                        nullptr, nullptr, nullptr);
+      if (!FE) {
+        CI.getDiagnostics().Report(diag::err_module_header_file_not_found)
+            << FileName;
+        return false;
+      }
+      // We now have the filename...
+      FileName = FE->getFileEntry().getName();
+      // ... still a header unit, but now use the path as written.
+      Kind = Input.getKind().withHeaderUnit(InputKind::HeaderUnit_Abs);
+      Input = FrontendInputFile(FileName, Kind, Input.isSystem());
+    }
+    // Unless the user has overridden the name, the header unit module name is
+    // the pathname for the file.
+    if (CI.getLangOpts().ModuleName.empty())
+      CI.getLangOpts().ModuleName = std::string(FileName);
+    CI.getLangOpts().CurrentModule = CI.getLangOpts().ModuleName;
+  }
+
   if (!CI.InitializeSourceManager(Input))
     return false;
 
   // For module map files, we first parse the module map and synthesize a
   // "<module-includes>" buffer before more conventional processing.
   if (Input.getKind().getFormat() == InputKind::ModuleMap) {
     CI.getLangOpts().setCompilingModule(LangOptions::CMK_ModuleMap);
 

clang/test/Modules/cxx20-hu-02.cpp

+// Test generation and import of user and system C++20 Header Units.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// check user path
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -I user \
+// RUN: -xc++-user-header hu-01.h -o hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-01.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface imp-hu-01.cpp \
+// RUN:  -I user -fmodule-file=hu-01.pcm -o B.pcm -Rmodule-import \
+// RUN: 2>&1  | FileCheck --check-prefix=CHECK-IMP %s -DTDIR=%t
+
+// check system path
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -isystem system \
+// RUN: -xc++-system-header hu-02.h -o hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-02.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU2 %s -DTDIR=%t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface imp-hu-02.cpp \
+// RUN:  -isystem system -fmodule-file=hu-02.pcm -o C.pcm \
+// RUN: -Rmodule-import 2>&1 | \
+// RUN: FileCheck --check-prefix=CHECK-SYS-IMP %s -DTDIR=%t
+
+// check absolute path
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit  \
+// RUN: -xc++-header-unit-header %t/hu-03.h -o hu-03.pcm
+
+// RUN: %clang_cc1 -std=c++20 -module-file-info hu-03.pcm | \
+// RUN: FileCheck --check-prefix=CHECK-HU3 %s -DTDIR=%t
+
+//--- user/hu-01.h
+int foo(int);
+
+// CHECK-HU:  ====== C++20 Module structure ======
+// CHECK-HU-NEXT:  Header Unit 'user/hu-01.h' is the Primary Module at index #1
+
+//--- imp-hu-01.cpp
+export module B;
+import "hu-01.h";
+
+int bar(int x) {
+  return foo(x);
+}
+// CHECK-IMP: remark: importing module 'user/hu-01.h' from 'hu-01.pcm'
+// expected-no-diagnostics
+
+//--- system/hu-02.h
+int baz(int);
+
+// CHECK-HU2:  ====== C++20 Module structure ======
+// CHECK-HU2-NEXT:  Header Unit 'system/hu-02.h' is the Primary Module at index #1
+
+//--- imp-hu-02.cpp
+module;
+import <hu-02.h>;
+
+export module C;
+
+int bar(int x) {
+  return baz(x);
+}
+// CHECK-SYS-IMP: remark: importing module 'system/hu-02.h' from 'hu-02.pcm'
+// expected-no-diagnostics
+
+//--- hu-03.h
+int curly(int);
+
+// CHECK-HU3:  ====== C++20 Module structure ======
+// CHECK-HU3-NEXT:  Header Unit '[[TDIR]]/hu-03.h' is the Primary Module at index #1
+// expected-no-diagnostics

clang/test/Modules/cxx20-hu-03.cpp

+// Test check that processing headers as C++20 units allows #pragma once.
+
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-01.h \
+// RUN: -Werror -o hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-header-unit -xc++-user-header hu-02.h \
+// RUN: -fmodule-file=%t/hu-01.pcm -o hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only imports-01.cpp \
+// RUN: -fmodule-file=%t/hu-01.pcm
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only imports-02.cpp \
+// RUN: -fmodule-file=%t/hu-02.pcm
+
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only imports-03.cpp \
+// RUN: -fmodule-file=%t/hu-02.pcm
+
+//--- hu-01.h
+#pragma once
+#define FORTYTWO 42
+struct HU {
+  int a;
+};
+// expected-no-diagnostics
+
+//--- hu-02.h
+export import "hu-01.h";
+// expected-no-diagnostics
+
+//--- imports-01.cpp
+import "hu-01.h";
+
+HU foo(int x) {
+  return {FORTYTWO};
+}
+// expected-no-diagnostics
+
+//--- imports-02.cpp
+import "hu-02.h";
+
+HU foo(int x) {
+  return {FORTYTWO};
+}
+// expected-no-diagnostics
+
+//--- imports-03.cpp
+import "hu-01.h";
+import "hu-02.h";
+
+HU foo(int x) {
+  return {FORTYTWO};
+}
+// expected-no-diagnostics