[PATCH] D127753: [lld-macho] Group undefined symbol diagnostics by symbol

[Jez Ng via Phabricator via llvm-commits]

int3 accepted this revision.
int3 added inline comments.


================
Comment at: lld/MachO/SymbolTable.cpp:349-355
+  struct SectionAndOffset {
+    const InputSection *isec;
+    uint64_t offset;
+  };
+
+  std::vector<std::string> namedLocs;
+  std::vector<SectionAndOffset> relocLocs;
----------------
thakis wrote:
> BertalanD wrote:
> > int3 wrote:
> > > Something like `-u` isn't really a location. It makes more sense to me to use `Loc` to mean section+offset, like LLD-ELF is doing.
> > > 
> > > How about calling this something like `namedSources`, given that `treatUndefinedSymbol` already calls the same value `source`?
> > That's a much better name, thanks for the suggestion!
> > 
> > Is `relocLocs` fine as is, or do you have a better idea for it? I know it doesn't have the best name.
> Maybe `codeReferences` for the relocs and `otherReferences` for the named ones? But the current names work for me personally too. Let's see if Jez has a preference.
I like @thakis' idea :)


================
Comment at: lld/MachO/SymbolTable.cpp:360
+
+void lld::macho::reportPendingUndefinedSymbols() {
+  for (const auto &undef : undefs) {
----------------
BertalanD wrote:
> int3 wrote:
> > 
> Changed the other two functions that had the incorrect namespace. I'm touching them anyways, so I hope it's fine to include that change in this patch.
looks good, thanks!


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

https://reviews.llvm.org/D127753