[PATCH] D38760: [LLD] [COFF] Add support for automatically exporting all symbols

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: COFF/Driver.cpp:1214
+                      Args.hasArg(OPT_export_all_symbols))) {
+    std::vector<std::string> ExcludeSymbols = {
+        "_NULL_IMPORT_DESCRIPTOR",
----------------
mstorsjo wrote:
> zturner wrote:
> > ruiu wrote:
> > > Can you make this a separate function which returns a std::set?
> > Can you make this a `vector<StringRef>`, but add an additional `_` to the beginning of each item?
> > 
> > Then, below you can write:
> > 
> > ```
> > if (Config->Machine != I386) {
> >   for (auto &Str : ExcludeSymbols) {
> >     Str = Str.drop_front().substr(0, Str.find('@'));
> >   }
> > }
> > ```
> > 
> > saving unnecessary allocations on all code paths.
> Ok, that should be doable.
Actually, I prefer a simple std::set because the number of items in the set is really small. I wouldn't avoid string copy at all because it's cost is negligible. I'd put emphasis simplicity over efficiency unless it is a performance-critical part of the linker.


================
Comment at: COFF/Driver.cpp:580
+
+static void writeDefFile(StringRef Name) {
+  std::error_code EC;
----------------
Please mention that this is MinGW specific.


================
Comment at: COFF/Driver.cpp:1278
 
+  // Handle /output-def
+  if (auto *Arg = Args.getLastArg(OPT_output_def))
----------------
Please mention that this is MinGW specific.


https://reviews.llvm.org/D38760