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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D102216: [Clang][... Mirzazyanov Gleb via Phabricator via cfe-commits
    • [PATCH] D102216: [Cl... Sam McCall via Phabricator via cfe-commits

Reply via email to