[PATCH] D144529: [TextAPI] Add support for TBDv5 Files to nm & tapi-diff

[Cyndy Ishida via Phabricator via llvm-commits]

cishida added inline comments.


================
Comment at: llvm/test/Object/nm-tapi.test:71
+
+V5: /System/Library/Frameworks/Simple.framework/Versions/A/Simple (for architecture x86_64):
+V5-NEXT: 0000000000000000 D _OBJC_CLASS_$_Base
----------------
jhenderson wrote:
> Aside: It was confusing that the llvm-nm related testing is not in the test/tools/llvm-nm folder, unlike most llvm-nm testing. Perhaps worth considering moving this test in a separate change?
Yes, that's fair. Thinking back, the rationale was testing the libObject changes indirectly but I can certainly move this to test/tools/llvm-nm in a followup patch to avoid confusion. 


================
Comment at: llvm/test/tools/llvm-tapi-diff/v5.test:7
+
+; CHECK-NOT: error:
+; CHECK-NOT: warning:
----------------
jhenderson wrote:
> Isn't this unnecessary? Won't llvm-tapi-diff fail with a non-zero exit code if it encounters and error?
llvm-tapi-diff currently doesn't emit any warnings so all errors are fatal. In these two invocations the tool shouldn't report any differences.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144529