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

Thu Mar 17 20:43:57 PDT 2022

ChuanqiXu added inline comments.

================
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");
----------------
iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > 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.
> > 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.
> 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).
> 
If it is not dead codes, I feel better if we could add tests to cover it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121096