[C++20][Modules][Driver][HU 2/N] Add fmodule-header, fmodule-header=

[iains]

Summary

These command-line flags are alternates to providing the -x
c++-*-header indicators that we are building a header unit.

Act on fmodule-header= for headers on the c/l:

If we have x.hh -fmodule-header, then we should treat that header
as a header unit input (equivalent to -xc++-header-unit-header x.hh).

Likewise, for fmodule-header={user,system} the source should be now
recognised as a header unit input (since this can affect the job list
that we need).

It's not practical to recognise a header without any suffix so
-fmodule-header=system foo isn't going to happen. Although
-fmodule-header=system foo.hh will work OK. However we can make it
work if the user indicates that the item without a suffix is a valid
header. (so -fmodule-header=system -xc++-header vector)

[ChuanqiXu]

It's not practical to recognise a header without any suffix so

-fmodule-header=system foo isn't going to happen.

May I ask the reason? It looks not so good with -fmodule-header=system -xc++-header vector

[iains]

In D121589#3381343, @ChuanqiXu wrote:

It's not practical to recognise a header without any suffix so

-fmodule-header=system foo isn't going to happen.

May I ask the reason? It looks not so good with -fmodule-header=system -xc++-header vector

OK. One should never say "never" ;) , it would be nicer if we could avoid this.

... it would require a policy change in the driver - since we cannot recognise files like 'vector' as headers, they are currently unclaimed (which means that they default to being considered as linker inputs).

This is a long-standing (forever, I suspect) situation;
Although we could make it so that if we see certain options, all unknown inputs get claimed as source files (or headers) I wonder how much build system code that might break.

[ChuanqiXu]

In D121589#3381894, @iains wrote:

In D121589#3381343, @ChuanqiXu wrote:

It's not practical to recognise a header without any suffix so

-fmodule-header=system foo isn't going to happen.

May I ask the reason? It looks not so good with -fmodule-header=system -xc++-header vector

OK. One should never say "never" ;) , it would be nicer if we could avoid this.

... it would require a policy change in the driver - since we cannot recognise files like 'vector' as headers, they are currently unclaimed (which means that they default to being considered as linker inputs).

This is a long-standing (forever, I suspect) situation;
Although we could make it so that if we see certain options, all unknown inputs get claimed as source files (or headers) I wonder how much build system code that might break.

If this is the policy, I am OK. For the build systems, due to the dependences, all build systems want to support C++20 modules should do works. So if we are doing our best, I think it might not be bad.

[iains]

In D121589#3381989, @ChuanqiXu wrote:

In D121589#3381894, @iains wrote:

In D121589#3381343, @ChuanqiXu wrote:

It's not practical to recognise a header without any suffix so

-fmodule-header=system foo isn't going to happen.

May I ask the reason? It looks not so good with -fmodule-header=system -xc++-header vector

OK. One should never say "never" ;) , it would be nicer if we could avoid this.

... it would require a policy change in the driver - since we cannot recognise files like 'vector' as headers, they are currently unclaimed (which means that they default to being considered as linker inputs).

This is a long-standing (forever, I suspect) situation;
Although we could make it so that if we see certain options, all unknown inputs get claimed as source files (or headers) I wonder how much build system code that might break.

If this is the policy, I am OK. For the build systems, due to the dependences, all build systems want to support C++20 modules should do works. So if we are doing our best, I think it might not be bad.

So I had a chance to discuss this briefly with some of the other modules folks, and the consensus there was that changing the behaviour of the driver w.r.t linker inputs would likely cause build system problems, as I suggested above.
One possibility might be to find some shorter way (than -xc++-header) for the user to say that a file was a header (but IMO that is something for the future and there are interactions with shells to consider etc. - here we are trying to implement the functionality correctly and optimisation can follow later).

[iains]

rebased.

[iains]

rebased, fix testcase for windows, add command line options to docs.

[iains]

fix some formatting.

[iains]

amend filecheck match to avoid quoting windows pathnames.

[iains]

amend second testcase for windows

[iains]

another tweak to the second testcase for windows
*sigh* ... I will get the recipe right eventually.

[iains]

we need the quotes around the components.

[tahonermann]

clang/docs/ClangCommandLineReference.rst

1183–1185 ↗

(On Diff #424459)

Are "user" and "system" the right terms to use here? Existing options aren't all that consistent. Some examples from Options.td:

def MD : Flag<["-"], "MD">, Group<M_Group>,
    HelpText<"Write a depfile containing user and system headers">;
def MMD : Flag<["-"], "MMD">, Group<M_Group>,
    HelpText<"Write a depfile containing user headers">;
...
def iquote : JoinedOrSeparate<["-"], "iquote">, Group<clang_i_Group>, Flags<[CC1Option]>,
  HelpText<"Add directory to QUOTE include search path">, MetaVarName<"<directory>">;
def isystem : JoinedOrSeparate<["-"], "isystem">, Group<clang_i_Group>,
  Flags<[CC1Option]>,
  HelpText<"Add directory to SYSTEM include search path">, MetaVarName<"<directory>">;

For comparison, the related MSVC options (also present in Options.td) are:

def _SLASH_headerUnitAngle : CLJoinedOrSeparate<"headerUnit:angle">;
def _SLASH_headerUnitQuote : CLJoinedOrSeparate<"headerUnit:quote">;

Should there be a "both" or "any" option so that the search considers all include paths as opposed to just user OR system?

[iains]

clang/docs/ClangCommandLineReference.rst

1183–1185 ↗

(On Diff #424459)

Are "user" and "system" the right terms to use here? Existing options aren't all that consistent. Some examples from Options.td:

def MD : Flag<["-"], "MD">, Group<M_Group>,
    HelpText<"Write a depfile containing user and system headers">;
def MMD : Flag<["-"], "MMD">, Group<M_Group>,
    HelpText<"Write a depfile containing user headers">;
...
def iquote : JoinedOrSeparate<["-"], "iquote">, Group<clang_i_Group>, Flags<[CC1Option]>,
  HelpText<"Add directory to QUOTE include search path">, MetaVarName<"<directory>">;
def isystem : JoinedOrSeparate<["-"], "isystem">, Group<clang_i_Group>,
  Flags<[CC1Option]>,
  HelpText<"Add directory to SYSTEM include search path">, MetaVarName<"<directory>">;

For comparison, the related MSVC options (also present in Options.td) are:

def _SLASH_headerUnitAngle : CLJoinedOrSeparate<"headerUnit:angle">;
def _SLASH_headerUnitQuote : CLJoinedOrSeparate<"headerUnit:quote">;

Should there be a "both" or "any" option so that the search considers all include paths as opposed to just user OR system?

1183–1185 ↗

(On Diff #424459)

At this time, the objective was to be command-line-option compatible with the existing GCC implementation. There is a goal to minimise difference in interfaces presented to the user (and build systems).

Given that the latter [GCC] is already "in the wild", if we wanted terminology-compatibility with MSVC, we could add aliases (to both), I suppose. I have no axe to grind here - if folks form a consensus that we could have better terminology, I'm happy to add the aliases etc.

There is also -fmodule-header ... which means "find the header at the path specified" which can either be 'absolute' (i.e. /xxxx on unix-y systems) or relative to CWD,

I'm not sure about 'both' or 'any' in the sense of how that would relate to the existing behaviour of header searches (the mechanisms for substituting HUs for textual headers assume that the searches are done with the same process as for the textual).

Hopefully, that expresses the intent of the current landed work - that is not to say that the story ends here, but that probably there are other features we want to complete before fine-tuning this.

[tahonermann]

clang/docs/ClangCommandLineReference.rst

1183–1185 ↗

(On Diff #424459)

Ah, I didn't realize gcc already implemented these options. I agree compatibility with it is the more important goal. Looks good to me.

clang/include/clang/Driver/Driver.h

 /// Describes the kind of LTO mode selected via -f(no-)?lto(=.*)? options.
 enum LTOKind {
   LTOK_None,
   LTOK_Full,
   LTOK_Thin,
   LTOK_Unknown
 };
 
+/// Whether headers used to construct C++20 module units should be looked
+/// up by the path supplied on the command line, or in the user or system
+/// search paths.
+enum ModuleHeaderMode {
+  HeaderMode_None,
+  HeaderMode_Default,
+  HeaderMode_User,
+  HeaderMode_System
+};
+
 /// Driver - Encapsulate logic for constructing compilation processes
 /// from a set of gcc-driver-like command line arguments.
 class Driver {
   DiagnosticsEngine &Diags;
 
   IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS;
 
   enum DriverMode {
class Driver {
   } SaveTemps;
 
   enum BitcodeEmbedMode {
     EmbedNone,
     EmbedMarker,
     EmbedBitcode
   } BitcodeEmbed;
 
+  /// Header unit mode set by -fmodule-header={user,system}.
+  ModuleHeaderMode CXX20HeaderType;
+
+  /// Set if we should process inputs and jobs with C++20 module
+  /// interpretation.
+  bool ModulesModeCXX20;
+
   /// LTO mode selected via -f(no-)?lto(=.*)? options.
   LTOKind LTOMode;
 
   /// LTO mode selected via -f(no-offload-)?lto(=.*)? options.
   LTOKind OffloadLTOMode;
 
 public:
   enum OpenMPRuntimeKind {
public:
 
   /// ShouldUseFlangCompiler - Should the flang compiler be used to
   /// handle this action.
   bool ShouldUseFlangCompiler(const JobAction &JA) const;
 
   /// ShouldEmitStaticLibrary - Should the linker emit a static library.
   bool ShouldEmitStaticLibrary(const llvm::opt::ArgList &Args) const;
 
+  /// Returns true if the user has indicated a C++20 header unit mode.
+  bool hasHeaderMode() const { return CXX20HeaderType != HeaderMode_None; }
+
+  /// Get the mode for handling headers as set by fmodule-header{=}.
+  ModuleHeaderMode getModuleHeaderMode() const { return CXX20HeaderType; }
+
   /// Returns true if we are performing any kind of LTO.
   bool isUsingLTO(bool IsOffload = false) const {
     return getLTOMode(IsOffload) != LTOK_None;
   }
 
   /// Get the specific kind of LTO being performed.
   LTOKind getLTOMode(bool IsOffload = false) const {
     return IsOffload ? OffloadLTOMode : LTOMode;

clang/include/clang/Driver/Options.td

defm modules_search_all : BoolFOption<"modules-search-all",
   NegFlag<SetFalse>, BothFlags<[NoXarchOption, CC1Option,CoreOption]>>,
   ShouldParseIf<fmodules.KeyPath>;
 defm implicit_modules : BoolFOption<"implicit-modules",
   LangOpts<"ImplicitModules">, DefaultTrue,
   NegFlag<SetFalse, [CC1Option]>, PosFlag<SetTrue>, BothFlags<[NoXarchOption,CoreOption]>>;
 def fretain_comments_from_system_headers : Flag<["-"], "fretain-comments-from-system-headers">, Group<f_Group>, Flags<[CC1Option]>,
   MarshallingInfoFlag<LangOpts<"RetainCommentsFromSystemHeaders">>;
 
+def fmodule_header : Flag <["-"], "fmodule-header">, Group<f_Group>,
+  Flags<[NoXarchOption]>, HelpText<"Build a C++20 Header Unit from a header.">;
+def fmodule_header_EQ : Joined<["-"], "fmodule-header=">, Group<f_Group>,
+  Flags<[NoXarchOption]>, MetaVarName<"<kind>">,
+  HelpText<"Build a C++20 Header Unit from a header that should be found in the user (fmodule-header=user) or system (fmodule-header=system) search path.">;
+
 def fmudflapth : Flag<["-"], "fmudflapth">, Group<f_Group>;
 def fmudflap : Flag<["-"], "fmudflap">, Group<f_Group>;
 def fnested_functions : Flag<["-"], "fnested-functions">, Group<f_Group>;
 def fnext_runtime : Flag<["-"], "fnext-runtime">, Group<f_Group>;
 def fno_asm : Flag<["-"], "fno-asm">, Group<f_Group>;
 def fno_asynchronous_unwind_tables : Flag<["-"], "fno-asynchronous-unwind-tables">, Group<f_Group>;
 def fno_assume_sane_operator_new : Flag<["-"], "fno-assume-sane-operator-new">, Group<f_Group>,
   HelpText<"Don't assume that C++'s global operator new can't alias any pointer">,

clang/lib/Driver/Driver.cpp

std::string Driver::GetResourcesPath(StringRef BinaryPath,
 
   return std::string(P.str());
 }
 
 Driver::Driver(StringRef ClangExecutable, StringRef TargetTriple,
                DiagnosticsEngine &Diags, std::string Title,
                IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS)
     : Diags(Diags), VFS(std::move(VFS)), Mode(GCCMode),
-      SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone), LTOMode(LTOK_None),
+      SaveTemps(SaveTempsNone), BitcodeEmbed(EmbedNone),
+      CXX20HeaderType(HeaderMode_None), LTOMode(LTOK_None),
       ClangExecutable(ClangExecutable), SysRoot(DEFAULT_SYSROOT),
       DriverTitle(Title), CCCPrintBindings(false), CCPrintOptions(false),
       CCPrintHeaders(false), CCLogDiagnostics(false), CCGenDiagnostics(false),
       CCPrintProcessStats(false), TargetTriple(TargetTriple), Saver(Alloc),
       CheckInputsExist(true), GenReproducer(false),
       SuppressMissingInputWarning(false) {
   // Provide a sane fallback if no VFS is specified.
   if (!this->VFS)
unsigned Model = llvm::StringSwitch<unsigned>(Name)
         .Default(~0U);
     if (Model == ~0U) {
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
                                                 << Name;
     } else
       BitcodeEmbed = static_cast<BitcodeEmbedMode>(Model);
   }
 
+  // Setting up the jobs for some precompile cases depends on whether we are
+  // treating them as PCH, implicit modules or C++20 ones.
+  // TODO: inferring the mode like this seems fragile (it meets the objective
+  // of not requiring anything new for operation, however).
+  const Arg *Std = Args.getLastArg(options::OPT_std_EQ);
+  ModulesModeCXX20 =
+      !Args.hasArg(options::OPT_fmodules) && Std &&
+      (Std->containsValue("c++20") || Std->containsValue("c++2b") ||
+       Std->containsValue("c++2a") || Std->containsValue("c++latest"));
+
+  // Process -fmodule-header{=} flags.
+  if (Arg *A = Args.getLastArg(options::OPT_fmodule_header_EQ,
+                               options::OPT_fmodule_header)) {
+    // These flags force C++20 handling of headers.
+    ModulesModeCXX20 = true;
+    if (A->getOption().matches(options::OPT_fmodule_header))
+      CXX20HeaderType = HeaderMode_Default;
+    else {
+      StringRef ArgName = A->getValue();
+      unsigned Kind = llvm::StringSwitch<unsigned>(ArgName)
+                          .Case("user", HeaderMode_User)
+                          .Case("system", HeaderMode_System)
+                          .Default(~0U);
+      if (Kind == ~0U) {
+        Diags.Report(diag::err_drv_invalid_value)
+            << A->getAsString(Args) << ArgName;
+      } else
+        CXX20HeaderType = static_cast<ModuleHeaderMode>(Kind);
+    }
+  }
+
   std::unique_ptr<llvm::opt::InputArgList> UArgs =
       std::make_unique<InputArgList>(std::move(Args));
 
   // Perform the default argument translations.
   DerivedArgList *TranslatedArgs = TranslateInputArgs(*UArgs);
 
   // Owned by the host.
   const ToolChain &TC = getToolChain(
bool Driver::DiagnoseInputExistence(const DerivedArgList &Args, StringRef Value,
   if (!getCheckInputsExist())
     return true;
 
   // stdin always exists.
   if (Value == "-")
     return true;
 
   // If it's a header to be found in the system or user search path, then defer
-  // complaints about its absence until those searches can be done.
-  if (Ty == types::TY_CXXSHeader || Ty == types::TY_CXXUHeader)
+  // complaints about its absence until those searches can be done.  When we
+  // are definitely processing headers for C++20 header units, extend this to
+  // allow the user to put "-fmodule-header -xc++-header vector" for example.
+  if (Ty == types::TY_CXXSHeader || Ty == types::TY_CXXUHeader ||
+      (ModulesModeCXX20 && Ty == types::TY_CXXHeader))
     return true;
 
   if (getVFS().exists(Value))
     return true;
 
   if (TypoCorrect) {
     // Check if the filename is a typo for an option flag. OptTable thinks
     // that all args that are not known options and that start with / are
bool Driver::DiagnoseInputExistence(const DerivedArgList &Args, StringRef Value,
   // unknown flags.)
   if (IsCLMode() && Ty == types::TY_Object && !Value.startswith("/"))
     return true;
 
   Diag(clang::diag::err_drv_no_such_file) << Value;
   return false;
 }
 
+// Get the C++20 Header Unit type corresponding to the input type.
+static types::ID CXXHeaderUnitType(ModuleHeaderMode HM) {
+  switch (HM) {
+  case HeaderMode_User:
+    return types::TY_CXXUHeader;
+  case HeaderMode_System:
+    return types::TY_CXXSHeader;
+  case HeaderMode_Default:
+    break;
+  case HeaderMode_None:
+    llvm_unreachable("should not be called in this case");
+  }
+  return types::TY_CXXHUHeader;
+}
+
 // Construct a the list of inputs and their types.
 void Driver::BuildInputs(const ToolChain &TC, DerivedArgList &Args,
                          InputList &Inputs) const {
   const llvm::opt::OptTable &Opts = getOpts();
   // Track the current user specified (-x) input. We also explicitly track the
   // argument used to set the type; we only want to claim the type when we
   // actually use it, so we warn about unused -x arguments.
   types::ID InputType = types::TY_Nothing;
if (A->getOption().getKind() == Option::InputClass) {
         //
         // FIXME: Clean this up if we move the phase sequence into the type.
         if (Ty != types::TY_Object) {
           if (Args.hasArg(options::OPT_ObjC))
             Ty = types::TY_ObjC;
           else if (Args.hasArg(options::OPT_ObjCXX))
             Ty = types::TY_ObjCXX;
         }
+
+        // Disambiguate headers that are meant to be header units from those
+        // intended to be PCH.
+        if (Ty == types::TY_CXXHeader && hasHeaderMode())
+          Ty = CXXHeaderUnitType(CXX20HeaderType);
       } else {
         assert(InputTypeArg && "InputType set w/o InputTypeArg");
         if (!InputTypeArg->getOption().matches(options::OPT_x)) {
           // If emulating cl.exe, make sure that /TC and /TP don't affect input
           // object files.
           const char *Ext = strrchr(Value, '.');
           if (Ext && TC.LookupTypeForExtension(Ext + 1) == types::TY_Object)
             Ty = types::TY_Object;
if (A->getOption().getKind() == Option::InputClass) {
 
       // Follow gcc behavior and treat as linker input for invalid -x
       // options. Its not clear why we shouldn't just revert to unknown; but
       // this isn't very important, we might as well be bug compatible.
       if (!InputType) {
         Diag(clang::diag::err_drv_unknown_language) << A->getValue();
         InputType = types::TY_Object;
       }
+
+      // If the user has put -fmodule-header{,=} then we treat C++ headers as
+      // header unit inputs.  So we 'promote' -xc++-header appropriately.
+      if (InputType == types::TY_CXXHeader && hasHeaderMode())
+        InputType = CXXHeaderUnitType(CXX20HeaderType);
     } else if (A->getOption().getID() == options::OPT_U) {
       assert(A->getNumValues() == 1 && "The /U option has one value.");
       StringRef Val = A->getValue(0);
       if (Val.find_first_of("/\\") != StringRef::npos) {
         // Warn about e.g. "/Users/me/myfile.c".
         Diag(diag::warn_slash_u_filename) << Val;
         Diag(diag::note_use_dashdash);
       }

clang/test/Driver/cxx20-header-units-02.cpp

+// Test user-facing command line options to generate C++20 header units.
+
+// RUN: %clang++ -### -std=c++20 -fmodule-header=user foo.hh  2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-USER %s
+
+// RUN: %clang++ -### -std=c++20 -fmodule-header=system foo.hh 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-SYS1 %s
+
+// RUN: %clang++ -### -std=c++20 -fmodule-header=system \
+// RUN: -xc++-system-header vector 2>&1 | FileCheck -check-prefix=CHECK-SYS2 %s
+
+// RUN: %clang++ -### -std=c++20 -fmodule-header=system \
+// RUN: -xc++-header vector 2>&1 | FileCheck -check-prefix=CHECK-SYS2 %s
+
+// RUN: %clang++ -### -std=c++20 -fmodule-header %S/Inputs/header-unit-01.hh \
+// RUN: 2>&1 | FileCheck -check-prefix=CHECK-ABS %s -DTDIR=%S/Inputs
+
+// CHECK-USER: "-emit-header-unit"
+// CHECK-USER-SAME: "-o" "foo.pcm"
+// CHECK-USER-SAME: "-x" "c++-user-header" "foo.hh"
+
+// CHECK-SYS1: "-emit-header-unit"
+// CHECK-SYS1-SAME: "-o" "foo.pcm"
+// CHECK-SYS1-SAME: "-x" "c++-system-header" "foo.hh"
+
+// CHECK-SYS2: "-emit-header-unit"
+// CHECK-SYS2-SAME: "-o" "vector.pcm"
+// CHECK-SYS2-SAME: "-x" "c++-system-header" "vector"
+
+// CHECK-ABS: "-emit-header-unit"
+// CHECK-ABS-SAME: "-o" "header-unit-01.pcm"
+// CHECK-ABS-SAME: "-x" "c++-header-unit-header" "[[TDIR]]/header-unit-01.hh"