[PATCH] D40849: CodeGen: support an extension to pass linker options on ELF

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

If you would make it perfectly clear how to parse an embedded string, I'd be fine with the single string approach rather than the key-value approach. But currently it seems the new feature gives too much freedom on how to use it.

Besides the space issue that I commented below, I think you need to define the character code of the embedded string. That's important because pathnames may contain non-ASCII characters particularly on Windows. I'd recommend you always store strings in UTF-8.



================
Comment at: test/Feature/elf-linker-options.ll:6
+
+!0 = !{!"spaced", !"option"}
+!1 = !{!"nospace"}
----------------
Imagine that these two strings are pathnames.  Pathnames can contain any character other than \0. That mean you can't distinguish "spaced options" from "spaced" and "options, which is bad. You should allow only one string.


Repository:
  rL LLVM

https://reviews.llvm.org/D40849