[PATCH] D133448: Make llvm-tli-checker support static libraries

[Paul Robinson via Phabricator via llvm-commits]

probinson marked 3 inline comments as done.
probinson added inline comments.


================
Comment at: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp:168
+// Insert defined global function symbols into the map.
+void SDKNameMap::maybeInsertSymbol(const SymbolRef &S, section_iterator End) {
+  SymbolRef::Type Type = unwrapIgnoreError(S.getType());
----------------
rengolin wrote:
> Feels weird to pass the end() iterator here, because in theory you could pass any valid iterator and it wouldn't be the same logic.
> 
> I think passing a reference to the object and then using `Section != o.section_end()` would be more clear as well as avoiding improper use by accident.
Yeah, passing the ObjectFile does help avoid mistakes.


================
Comment at: llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp:176
+      Section != End)
+    insert({ Name, true });
+}
----------------
rengolin wrote:
> Nit: I'd wrap this in curly brackets because of the two-line if statement.
OK. And that lets me sink the getName() inside the if as well.


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

https://reviews.llvm.org/D133448