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

[Luke Drummond via Phabricator via llvm-commits]

ldrumm reopened this revision.
ldrumm added inline comments.
This revision is now accepted and ready to land.


================
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)
+
----------------
rnk wrote:
> 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
Okay. I understand. We can't have the pragma *inside* the initializer (hard error), so we have to take the declaration name from the user code and emit that after the pragma a'la:

```
#ifdef __GNUC__
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Woverlength-strings"
#endif
extern const char AArch64RegStrings[] = {
  /* 0 */ "B10\0"
 [...]
  /* 2397 */ "NZCV\0"
};
#ifdef __GNUC__
#pragma GCC diagnostic pop
#endif
```
and the api for this now looks like this in use:
```c++
  // Emit the string table.
  RegStrings.layout();
  RegStrings.emitStringLiteralDef(
      OS, Twine("extern const char ") + TargetName + "RegStrings[]");
```


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

https://reviews.llvm.org/D73044