[PATCH] D29563: [Polly][External] Move jsoncpp to lib/External/JSON. NFC.

Tobias Grosser via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 5 07:25:54 PST 2017


grosser accepted this revision.
grosser added a comment.
This revision is now accepted and ready to land.

Hi Michael,

this LGTM!

Regarding the idea of making this an external library, I am a little hesitant to add more build system features. Traditionally, this always caused bugs. However, if you feel this is the right choice and keep your eyes a little open, feel free to change things here.

One thing that I have thought of already a couple of times was to replace the JSON printer with the LLVM internal YAML printer. This would allow us to drop this external dependency. Even though JSON is (theoretically) a subset of YAML, I am not sure if we would want to exploit this or if we possibly even want to  move the JSON printer to YAML. Obviously, this does not impact this very review, but might something to consider.


https://reviews.llvm.org/D29563





More information about the llvm-commits mailing list