This enables automatically parsing and generating CC1 arguments for options where two flags control the same field, e.g. -fexperimental-new-pass-manager and -fno-experimental new pass manager.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
146 | I'm not sure if removing the llvm:: namespace is intentional here, but if so please do it in a separate NFC patch to avoid adding noise in this one. | |
181–183 | This should be: if (Arg *A = Args.getLastArg(PosOpt, NegOpt)) return A->getOption().matches(PosOpt); return None; Note that hasArg and hasFlag both resolve to getLastArg, so calling them one after another would be unfortunate. ... but I'm not even sure where NegOpt is coming from here, that looks like it hasn't been passed in. I think you need to change the signature to something like this: static llvm::Optional<bool> normalizeSimpleFlag(OptSpecifier Opt, Optional<OptSpecifier> NegOpt, unsigned TableIndex, const ArgList &Args, DiagnosticsEngine &Diags) { // Handle case without a `no-*` flag. if (!NegOpt) return Args.hasArg(Opt); // Handle case with a `no-*` flag. return Args.hasFlagAsOptional(Opt, *NegOpt); } It's possible you'll need to split up OPTION_WITH_MARSHALLING into two disjoint lists of options:
| |
4043–4056 | I realize this commit doesn't introduce it, but it seems unfortunate to call EXTRACTOR twice. Maybe in a follow-up or prep commit you can fix that... maybe something like this? if ((FLAGS)&options::CC1Option) { const auto &Extracted = EXTRACTOR(this->KEYPATH); if (ALWAYS_EMIT || Extracted != DEFAULT_VALUE) DENORMALIZER(Args, SPELLING, SA, TABLE_INDEX, EXTRACTOR(this->KEYPATH)); } | |
llvm/include/llvm/Option/OptParser.td | ||
181 | Nit: missing a space in ... Normalizer ="norma...; same typo repeats below. |
clang/lib/Frontend/CompilerInvocation.cpp | ||
---|---|---|
146 | Yes it was accidental sorry about that. | |
181–183 | NegOpt is passed in via the template parameter, the only weird bit about it is that the option name (for example OPT_fno_experimental_pass_manager) is constructed in tablegen by hardcoding the OPT_ prefix. What is currently there supports the case where the positive option takes a value and the negative one doesn't by using different normalizer/denormalizer pairs for the positive and the negative option. The bad thing about the current setup is that both options have identical normalizers, but I felt that was less bad than splitting the list of options and using different macros. | |
4043–4056 | Yes I can do that of course. Although EXTRACTOR is meant to be very cheap and in most cases it expands to just this->KEYPATH |
I'm not sure if removing the llvm:: namespace is intentional here, but if so please do it in a separate NFC patch to avoid adding noise in this one.