[llvm-commits] [PATCH] [lld] Add TargetInfo abstraction.

[Chandler Carruth]

================
Comment at: include/lld/ReaderWriter/ELFTargetInfo.h:25
@@ +24,3 @@
+
+  static std::unique_ptr<ELFTargetInfo> create(const LinkerOptions &lo);
+};
----------------
Michael Spencer wrote:
> Chandler Carruth wrote:
> > Michael Spencer wrote:
> > > kledzik at apple.com wrote:
> > > > Why is there this factory method, when the constructor is also public?  Do you intend for there to be hidden subclasses of ELFTargetInfo?
> > > It's public because it was easier to test like that. It should be protected.
> > How are you testing stuff?
> > 
> > This seems like the perfect kind of class to write unittests against. Can you rig up LLD to build unittests much like LLVM and Clang do?
> > 
> > 
> > FWIW, when using unittests, the more common pattern I've seen is to befriend the unittest fixture, and expose helper functions in the fixture for individual test cases to use.
> I don't really see how to unit test this interface. I have another patch that makes everything use this interface, and it's tested there. This is really just a refactor, it doesn't add any new functionality.
(FYI, I chatted w/ Michael about this on IRC, but it doesn't belong in this review thread. Sorry for derailing things. My only real goal was to suggest friendship for testing rather than public interfaces.... I'll go back to my corner now and let the real review conclude....)


http://llvm-reviews.chandlerc.com/D311