[PATCH] D121588: [C++20][Modules][Driver][HU 1/N] Initial handling for -xc++-{system,user}-header.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Thanks for explaining the desired interplay between PCH and PCM. The problem I'm seeing is that it can be hard to find correct compiler flags to achieve the desired result (what is enabled and what is disabled). But that is out of scope for this change.



================
Comment at: clang/include/clang/Driver/Types.def:66
 TYPE("c++-header",               CXXHeader,    PP_CXXHeader,    "hh",     phases::Preprocess, phases::Precompile)
-TYPE("objective-c++-header-cpp-output", PP_ObjCXXHeader, INVALID, "mii",  phases::Precompile)
+TYPE("c++-header-unit-cpp-output", PP_CXXHeaderUnit,INVALID,    "iih",    phases::Precompile)
+TYPE("c++-header-unit-header",   CXXHUHeader,  PP_CXXHeaderUnit,"hh",     phases::Preprocess, phases::Precompile)
----------------
iains wrote:
> vsapsai wrote:
> > Sorry, it's not really related to your change but do you have a rule where "ii" should go? It's just we have both "mii" and "iim" and I want to make sure it should be "iih" and not "hii". I haven't tried to find a pattern here myself, asking you first.
> > Sorry, it's not really related to your change but do you have a rule where "ii" should go? It's just we have both "mii" and "iim" and I want to make sure it should be "iih" and not "hii". I haven't tried to find a pattern here myself, asking you first.
> 
> My understanding is this:
> 
> ObjectiveC/C++ append "I" and "ii" (mi and mii)
> 
> C++ Modules-related code pre-pends "ii".
> 
> so that a pre-processed header unit ==> "iih"
> and a pre-processed module ==> "iim".
> 
> 
Thanks for the explanation. Now I see it should be "iih" and not "hii". One could argue that modular-related stuff can be expressed by appending "m", like ".ii" -> ".iim". But I don't know what that would mean for PP_CXXHeaderUnit ("iihm"?) and I think "iih" is reasonable.


================
Comment at: clang/test/Driver/cxx20-header-units-01.cpp:7
+
+// RUN: %clang++ -### -std=c++20 -xc++-header-unit-header %S/Inputs/header-unit-01.hh 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-ABS %s -DTDIR=%S/Inputs
----------------
iains wrote:
> vsapsai wrote:
> > What should happen in case of inconsistencies like `%clang++ -### -std=c++20 -xc++-system-header %S/Inputs/header-unit-01.hh`? Or `-xc++-system-header foo.h`?
> If the user states that `%S/Inputs/header-unit-01.hh` is a system header, I do not think that the driver is in a position to argue?
> 
> `  -xc++-system-header foo.h `  again the user has made an explicit statement..
> 
> I suppose that we can always diagnose these things with warnings - but I do not think we can easily reject them (and we risk producing unhelpful output).
What is the intended use of `-xc++-user-header` and `-xc++-system-header`? Are these supposed to be commonly-used flags or as an escape hatch of some sort? Because if it is commonly-used, I have concerns about usability that forces users to track user and system headers correctly, and duplicates the information already encoded in header search paths.

Also I think it is out of scope but for usability it is important to know the consequences of using a wrong flag and how one can diagnose it. Have no idea what you've already planned in this area, just mentioning it because I'm scarred by inscrutable clang behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121588