[PATCH] D77522: [ELF] --warn-backrefs: don't warn for linking sandwich problems

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 6 05:55:09 PDT 2020


grimar added inline comments.


================
Comment at: lld/ELF/Symbols.cpp:381
+    warn(it.second);
+  backrefDiags.clear();
+}
----------------
> backrefDiags.clear();

Should it be called before populating the `backrefDiags` and not after? I guess it might be
possible when using LLD as a library to return !0 error code after `backrefDiags` was filled, but before reporting and cleaning.

We initialize a few things like this in 

```
bool link(ArrayRef<const char *> args, bool canExitEarly, raw_ostream &stdoutOS,
          raw_ostream &stderrOS)
```

We have the known mess with the global variables. Should we at least stop adding them at random places and stick to initializing
new ones in the single place, like in the `link()`?


================
Comment at: lld/ELF/Symbols.cpp:530
+           toString(other.file) + " refers to " + toString(file))
+              .str());
+    }
----------------
Looking on this, my first impression was that probably since you now have the `reportBackrefs` method,
you could move the creation and printing of the warning there probably somehow.
(i.e. leave `backrefDiags.try_emplace` here, but change the map value from `std::string` to something else probably).
This should having excessive string allocations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77522





More information about the llvm-commits mailing list