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

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 4 10:17:04 PDT 2020


mstorsjo added inline comments.


================
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;
----------------
amccarth wrote:
> Would it be more straightforward to represent the string as a Twine rather than a vector for StringRefs?
No - we need the individual strings stored separarely as we need to call `processString` on each of them on their own.


================
Comment at: llvm/tools/llvm-rc/ResourceFileWriter.h:161
+                                uint16_t StringID,
+                                const std::vector<StringRef> &String);
 
----------------
amccarth wrote:
> 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.
Ok, good point, will revert to the original name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85183



More information about the llvm-commits mailing list