[PATCH] D108710: [clang][deps] Reset non-modular language and preprocessor options
Jan Svoboda via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 25 12:37:21 PDT 2021
jansvoboda11 added inline comments.
================
Comment at: clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template:4
+ "directory": "DIR",
+ "command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -include DIR/header.h -o DIR/tu.o",
+ "file": "DIR/tu.c"
----------------
jansvoboda11 wrote:
> dexonsmith wrote:
> > It looks to me like that this test would have passed before the code change, since there was already logic for dropping `-include`. Can you also add some other argument(s) that wouldn't have been dropped before, but will be now?
> Only the `-include-pch` argument was being dropped (`PreprocessorOptions::ImplicitPCHInclude`). The `-include` arguments (`PreprocessorOptions::Includes`) were being preserved. This test would not pass without the changes to `ModuleDepCollector.cpp`.
>
> I can add checks for other arguments that are being dropped now that weren't before, but assuming `resetNonModularOptions` are already being tested elsewhere, I don't think there's much value in that.
Note that the driver will expand `-include <header>` into `-include-pch <header>.gch` only if the PCH `<header>.gch` file exists. Since we're not precompiling the header in this particular test, that won't be the case. This argument therefore doesn't map onto `PreprocessorOptions::ImplicitPCHInclude`. It maps onto `PreprocessorOptions::Includes`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D108710/new/
https://reviews.llvm.org/D108710
More information about the cfe-commits
mailing list