[PATCH] D85183: [llvm-rc] Allow string table values split into multiple string literals

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth added a comment.

I'll defer to @thakis, but I have a couple suggestions inline.



================
Comment at: llvm/tools/llvm-rc/ResourceFileWriter.h:106
     struct Bundle {
-      std::array<Optional<StringRef>, 16> Data;
+      std::array<Optional<std::vector<StringRef>>, 16> Data;
       ObjectInfo DeclTimeInfo;
----------------
Would it be more straightforward to represent the string as a Twine rather than a vector for StringRefs?


================
Comment at: llvm/tools/llvm-rc/ResourceFileWriter.h:161
+                                uint16_t StringID,
+                                const std::vector<StringRef> &String);
 
----------------
I don't agree with the name change from singular to plural.  Conceptually, it is addjust a single StringID and String into the string table's Bundle.  The fact that the string's text may be a concatenation of several pieces doesn't change that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85183