[PATCH] D56294: [ObjectYAML] [COFF] Support multiple symbols with the same name

Martin Storsjö via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 4 13:55:05 PST 2019


mstorsjo marked an inline comment as done.
mstorsjo added inline comments.


================
Comment at: tools/obj2yaml/coff2yaml.cpp:208
       }
-      Rel.SymbolName = *SymbolNameOrErr;
+      if (SymbolUnique[*SymbolNameOrErr])
+        Rel.SymbolName = *SymbolNameOrErr;
----------------
ruiu wrote:
> mstorsjo wrote:
> > ruiu wrote:
> > > Doesn't this insert a new key if `*SymbolNameOrErr` does not exist in the map? I don't think that affects correctness of the program, but that's probably a waste of time and memory. I'd use `count` instead.
> > Yes, but every symbol will exist in the map already. We're not interested in whether the key exists in the map or not (it will always exist), but whether the map entry actually is set to true or false.
> Ah OK, then this is fine. I'd still use `count` to not surprise careless readers like me though. :)
Why `count`? Wouldn't that just return 1 saying that the symbol indeed is present in the map? We're not interested in whether the key is present or not (it always is), we're interested in the value of the map element, and `count` won't help with that.


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

https://reviews.llvm.org/D56294





More information about the llvm-commits mailing list