dexonsmith added a comment.
I have a couple of drive-by suggestions, up to @dang and @Bigcheese whether to
incorporate them.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:137-142
+ .Case("static", llvm::Reloc::Static)
+ .Case("pic", llvm::Reloc::PIC_)
+ .Case("ropi", llvm::Reloc::ROPI)
+ .Case("rwpi", llvm::Reloc::RWPI)
+ .Case("ropi-rwpi", llvm::Reloc::ROPI_RWPI)
+ .Case("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
----------------
I wonder if it's worth creating a `.def` file for the driver enums, something
like:
```
#ifdef HANDLE_RELOCATION_MODEL
HANDLE_RELOCATION_MODEL("static", llvm::Reloc::Static)
HANDLE_RELOCATION_MODEL("pic", llvm::Reloc::PIC_)
HANDLE_RELOCATION_MODEL("ropi", llvm::Reloc::ROPI)
HANDLE_RELOCATION_MODEL("rwpi", llvm::Reloc::RWPI)
HANDLE_RELOCATION_MODEL("ropi-rwpio", llvm::Reloc::ROPI_RWPI)
HANDLE_RELOCATION_MODEL("dynamic-no-pic", llvm::Reloc::DynamicNoPIC)
#undef HANDLE_RELOCATION_MODEL
#endif // HANDLE_RELOCATION_MODEL
#ifdef HANDLE_DEBUG_INFO_KIND
HANDLE_DEBUG_INFO_KIND("line-tables-only", codegenoptions::DebugLineTablesOnly)
HANDLE_DEBUG_INFO_KIND("line-directives-only",
codegenoptions::DebugDirectivesOnly)
HANDLE_DEBUG_INFO_KIND("limited", codegenoptions::LimitedDebugInfo)
HANDLE_DEBUG_INFO_KIND("standalone", codegenoptions::FullDebugInfo)
#undef HANDLE_DEBUG_INFO_KIND
#endif // HANDLE_DEBUG_INFO_KIND
// ...
```
Then you can use `HANDLE_RELOCATION_MODEL` in both `normalize` and
`denormalize`, rather than duplicating the table.
Maybe we can go even further. Can you expand the `Values` array from the
tablegen to include this info? Or rejigger the help text to leverage
`HANDLE_RELOCATION_MODEL` (maybe pass in
`ValuesDefine<HANDLE_RELOCATION_MODEL>`)? The current patch adds a value table;
my first suggestion leaves us even; but maybe we can get rid of one.
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3647
+ DiagnosticsEngine &Diags) {
+#define OPTION_WITH_MARSHALLING
+#define OPTION_WITH_MARSHALLING_FLAG(PREFIX_TYPE, NAME, ID, KIND, GROUP,
\
----------------
Seems like `Options.inc` could provide this as a default definition, not sure
if that seems error-prone?
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3665-3667
+#undef OPTION_WITH_MARSHALLING_STRING
+#undef OPTION_WITH_MARSHALLING_FLAG
+#undef OPTION_WITH_MARSHALLING
----------------
I prefer the style where the `.inc` file is responsible for the `#undef` calls.
It avoids having to duplicate it everywhere (and the risk of forgetting it).
WDYT?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79796/new/
https://reviews.llvm.org/D79796
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
