iains added inline comments.
================
Comment at: clang/include/clang/Frontend/FrontendOptions.h:157
+ unsigned HeaderUnit : 3;
+ unsigned Header : 1;
----------------
ChuanqiXu wrote:
> I prefer `IsHeader`
OK.
================
Comment at: clang/include/clang/Frontend/FrontendOptions.h:252
+ bool isHeader() const { return Kind.isHeader(); }
+ InputKind::HeaderUnitKind getHeaderUnit() const {
+ return Kind.getHeaderUnit();
----------------
ChuanqiXu wrote:
> How about `getHeaderUnitKind` ?
OK. I prefer more descriptive names, but they do get rather long sometimes.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2788-2791
+ 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;
----------------
ChuanqiXu wrote:
> How do you think about the suggested changes? I feel it is less confusing.
OK,
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2796-2797
+ // not intended to be a module map or header unit.
+ IsHeaderFile = IsHeader && !Preprocessed && !ModuleMap &&
+ HUK == InputKind::HeaderUnit_None;
----------------
ChuanqiXu wrote:
> 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...
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.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2855-2857
+ assert((DashX.getHeaderUnit() == InputKind::HeaderUnit_None ||
+ Inputs.size() == 1) &&
+ "Expected only one input file for header unit");
----------------
ChuanqiXu wrote:
> 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?
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.
================
Comment at: clang/lib/Frontend/FrontendAction.cpp:814-816
+ const DirectoryEntry *Dir = nullptr;
+ if (auto DirOrErr = CI.getFileManager().getDirectory("."))
+ Dir = *DirOrErr;
----------------
ChuanqiXu wrote:
> Is it same as the suggested?
not quite because getDirectory() is "llvm::ErrorOr<const DirectoryEntry *>" ...
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121096/new/
https://reviews.llvm.org/D121096
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
