[PATCH] D121065: [X86] Fix MCSymbolizer interface for X86Disassembler

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added inline comments.


================
Comment at: llvm/unittests/MC/X86/X86MCDisassemblerTest.cpp:90-91
+};
+
+TEST(X86Disassembler, X86MCSymbolizerTest) {
+  if (!getContext())
----------------
skan wrote:
> maksfb wrote:
> > skan wrote:
> > > I think this unittest in the wrong place. You test X86MCSymbolizerTest here, which is defined in this unittest. It's meaningless. This unnit test should be added in D120928 and tests the real `X86MCSymbolizer`.
> > This diff clearly fixes an existing interface and needs a test and the only test possible is the unit test.
> > 
> > The only question is if this interface fix should be committed separately from its usage in D120928. I'm fine either way, but I can see a theoretical benefit in the split of commits.
> Question: which line of X86MCDisassemblerTest.cpp would fail w/o this patch?
Most of expects. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121065