dexonsmith requested changes to this revision.
dexonsmith added inline comments.
This revision now requires changes to proceed.
================
Comment at: clang/include/clang/Driver/Options.td:1292-1303
def fdwarf_exceptions : Flag<["-"], "fdwarf-exceptions">, Group<f_Group>,
- Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">;
+ Flags<[CC1Option]>, HelpText<"Use DWARF style exceptions">,
+ MarshallingInfoFlag<"LangOpts->DWARFExceptions">;
def fsjlj_exceptions : Flag<["-"], "fsjlj-exceptions">, Group<f_Group>,
- Flags<[CC1Option]>, HelpText<"Use SjLj style exceptions">;
+ Flags<[CC1Option]>, HelpText<"Use SjLj style exceptions">,
+ MarshallingInfoFlag<"LangOpts->SjLjExceptions">;
def fseh_exceptions : Flag<["-"], "fseh-exceptions">, Group<f_Group>,
----------------
These options should be mutually exclusive -- as in, the last flag wins -- but
I don't see how that's implemented now (the previous logic was via
`getLastArg`). If that is working properly, can you explain how?
If it's not working right now, my suggestion would be to separate these options
out to do in a separate patch series. I would suggest, rather than modelling
the current behaviour, we leverage our flexibility to change `-cc1` options
(e.g., could do three patches, where the first adds accessors to LangOpts and
updates all users, the second changes the keypath to a single `ExceptionStyle`
enum, and then the third patch changes the `-cc1` option to `-fexception-style`
and starts using the marshalling infrastructure).
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:293
static T mergeForwardValue(T KeyPath, U Value) {
- return Value;
+ return static_cast<T>(Value);
}
----------------
These nits might be better to do in a follow-up, which also updated
`extractForwardValue`, but since I'm seeing it now:
- Should this use `std::move`?
- Can we drop the `KeyPath` name?
```
template <typename T, typename U>
static T mergeForwardValue(T /*KeyPath*/, U Value) {
return static_cast<T>(std::move(Value));
}
```
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3655-3666
llvm::Triple T(Res.getTargetOpts().Triple);
if (DashX.getFormat() == InputKind::Precompiled ||
DashX.getLanguage() == Language::LLVM_IR) {
- // ObjCAAutoRefCount and Sanitize LangOpts are used to setup the
- // PassManager in BackendUtil.cpp. They need to be initializd no matter
- // what the input type is.
- if (Args.hasArg(OPT_fobjc_arc))
----------------
Previously, these arguments were only claimed depending on `-x`; are we
changing to claim these all the time? If so, that should be considered
separately; I think in general we may want the ability to do claim some options
only conditionally; @Bigcheese , any thoughts here?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D83979/new/
https://reviews.llvm.org/D83979
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
