[PATCH] D102216: [Clang][docs]JSONLinkDatabase

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Hi, thanks for writing this up! This is an interesting idea and whole-program information is something our tooling libraries don't cover.

A few high-level thoughts:

- this has wider scope and probably have wider discussion than a code review, an RFC on the cfe-dev mailing list is the usual starting point
- it's unusual for LLVM to define file-formats without tools for working with them. compile_commands.json was developed in tandem with a library to use it for clang-based analysis. Do you have plans to develop such tools in the LLVM tree?
- whether in or out of the LLVM tree, what are the most important use cases for such a format? Why was this design picked over others to satisfy them?
- is command/flags the right abstraction for information about how source code is ultimately linked? It follows the compile_commands.json precedent, but the underlying reasons seem much weaker. (With parsing commands there are a huge diversity of options, it's critical we preserve them ~all exactly, and there's a clear impl strategy for doing so)
- if the database can contain arbitrarily mixed linker/object file manipulation/non-linker commands and the tool is responsible for guessing/interpreting them seems to mean this format will be hard to consume robustly.
- one motivating case is in the readme (resolving one of multiple definitions in the code base). IIUC the suggested implementation is to index the codebase using compile_commands.json, and then use link info to choose between multiple definitions. This seems quite narrow (only helps when there are multiple defintions, but not if they're each linked to the caller in different binaries), doesn't yield a particularly *efficient* implementation, and seems to admit less invasive solutions (e.g. a subset compile_commands.json that only covers sources for one binary).

(Sorry about the delay, I've been off work for a few weeks)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102216