[PATCH] D60127: Declare library dependency introduced by https://reviews.llvm.org/D59482

Pavel Labath via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Apr 2 06:42:55 PDT 2019


labath added a comment.

In D60127#1451385 <https://reviews.llvm.org/D60127#1451385>, @jhenderson wrote:

> I can't see how D59482 <https://reviews.llvm.org/D59482> introduces a dependency on LLVMObject in the main LLVMObjectYAML library. It does add unit tests that rely on the Object library, and includes the necessary reference. Where is the new dependency that this is needed for?


I think serge has linked the wrong patch. The culprit is https://reviews.llvm.org/D59634, which does add the dependency because it uses the Object library to construct the ObjectYAML model. This is a new dependency because for all the other binary formats the code to construct the yaml model lives in tool code (obj2yaml). Here, I chose to do that in the ObjectYAML library because I wanted to make that code accessible to unit tests.

@jhenderson do you see any problems with this new dependency (i.e., should I push a patch adding it), or does it need more discussion (i.e., revert the original patch) ? (FWIW, to me it seems natural for ObjectYAML to depend on Object, and I don't think this creates any danger of dependency cycles, as Object should never depend on ObjectYAML.)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60127





More information about the llvm-commits mailing list