[PATCH] D73044: {tablegen] Emit string literals instead of char arrays

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:652
+  # known to support them (everything but MSVC)
+  add_flag_if_supported("-Wno-overlength-strings" IGNORE_OVERLENGTH_STRINGS)
+
----------------
ldrumm wrote:
> rnk wrote:
> > Instead of this, I would suggest doing this in the generated source:
> > ```
> > #ifdef __GNUC
> > #pragma GCC diagnostic push
> > #pragma GCC diagnostic ignored "-Woverlength-strings"
> > #endif
> > ...
> > #ifdef __GNUC
> > #pragma GCC diagnostic pop
> > #endif
> > ```
> > That way, we still benefit from this warning in places where we aren't explicitly taking steps to avoid the problem.
> While limiting the surface of the warning might be nice, I'm not convinced by the value proposition of adding code to emit this in every tablegen backend.
> I could add  `emit_boilerplate_macros_prologue/epilogue()` functions to `SequenceToTableOffset` that are called at the beginning and end of every tablegen backend, but again I don't think it's worth the extra complexity vs silencing a basically pointless warning on a compiler that has never had this limitatation
I guess I was hoping it would be enough to splat that pragma out from `emit()`. If we can't place the pragma inside the array with long strings, one could update the API of `emit` to take the type and name of the array so we can get the pragma in the right place without too much duplication. All the callers use the same code pattern anyway, if you ignore the (IMO unnecessary) std::array usage:
https://reviews.llvm.org/P8187


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

https://reviews.llvm.org/D73044