infer
infer copied to clipboard
Incremental build support for json-based frontend
Hi!
In this PR we add incremental build support for json-based frontend. All we need is provide a list of changed files in a txt file and feed in through flag --changed-files-index
for infer analyzejson
command.
Please let us know your thoughts. Thanks!
Hi @xi-liu-ds, sorry for the delay. It looks like this duplicates more code still from InferAnalyze.ml. Would that duplication still be needed if parsing the json was done in a capture phase instead of together with the analysis? It feels to me like doing so would give you incremental analysis for free as well as being more maintainable, as the analysis phase would be just the normal Infer analysis. Also there wouldn't need to be an analyzejson
command.
I think capturing json could be done similarly as how we capture clang compilation databases so you could take inspiration from those code paths. We use it like this:
# does capture + analysis
$ infer run --compilation-database file.json
# equivalent to running these two commands:
$ infer capture --compilation-database file.json
$ infer analyze
Similarly we'd have two options --json-cfg
and --json-tenv
that have to be specified together or not at all:
# does capture + analysis
$ infer run --json-cfg cfg.json --json-tenv tenv.json
# equivalent to running these two commands:
$ infer capture --json-cfg cfg.json --json-tenv tenv.json
$ infer analyze
So we would have a new driver mode JsonSIL
set similarly to ClangCompilationDB. Then it's just a matter of separating out the capture phase out of InferAnalyzeJson.ml and into its own module and call it from Driver.ml, then the analysis phase can be deleted from InferAnalyzeJson.ml (and so the rest of the file with it) as it's not a separate thing from the normal infer analysis anymore.
Would that approach work for you and are there any blockers? I'm not sure how much we need to ensure backwards compatibility or we can just get rid of the analyzejson
command altogether if we go down this route, for example.
Hi @xi-liu-ds, sorry for the delay. It looks like this duplicates more code still from InferAnalyze.ml. Would that duplication still be needed if parsing the json was done in a capture phase instead of together with the analysis? It feels to me like doing so would give you incremental analysis for free as well as being more maintainable, as the analysis phase would be just the normal Infer analysis. Also there wouldn't need to be an
analyzejson
command.I think capturing json could be done similarly as how we capture clang compilation databases so you could take inspiration from those code paths. We use it like this:
# does capture + analysis $ infer run --compilation-database file.json # equivalent to running these two commands: $ infer capture --compilation-database file.json $ infer analyze
Similarly we'd have two options
--json-cfg
and--json-tenv
that have to be specified together or not at all:# does capture + analysis $ infer run --json-cfg cfg.json --json-tenv tenv.json # equivalent to running these two commands: $ infer capture --json-cfg cfg.json --json-tenv tenv.json $ infer analyze
So we would have a new driver mode
JsonSIL
set similarly to ClangCompilationDB. Then it's just a matter of separating out the capture phase out of InferAnalyzeJson.ml and into its own module and call it from Driver.ml, then the analysis phase can be deleted from InferAnalyzeJson.ml (and so the rest of the file with it) as it's not a separate thing from the normal infer analysis anymore.Would that approach work for you and are there any blockers? I'm not sure how much we need to ensure backwards compatibility or we can just get rid of the
analyzejson
command altogether if we go down this route, for example.
Thanks Jules for the suggestion! We are aware that analyze json has some code duplication from InferAnalyze.ml
. We are investigating the solution you provided and see if it can fully replace the current experience from InferAnalyzeJson.ml
.
Hi @jvillard So sorry about the delay on this PR. I finally had chance to look deep into it this week 😂.
It turns out that your suggestions are great and helped to remove lots of duplicate code! Thank you so much!
Like you suggested, now we can do infer capture --cfg-json cfg.json --tenv-json tenv.json
to run json frontend analysis. And for incremental build, all we need is infer capture --cfg-json cfg.json --tenv-json tenv.json --incremental-analysis --changed-files-index change_file_index.txt
Please take a look when you get a chance. Thanks again!
Thanks for your comments! Now the incremental build support has been improved even more. It can be triggered by using the following commands:
infer run --cfg-json cfg.json --tenv-json tenv.json --incremental-analysis --changed-files-index change_file_index.txt -j 1
@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@xi-liu-ds has updated the pull request. You must reimport the pull request before landing.
@xi-liu-ds has updated the pull request. You must reimport the pull request before landing.
@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
@xi-liu-ds has updated the pull request. You must reimport the pull request before landing.
Fixed unit tests since we merged infer analyzejson
into infer run
now.
@xi-liu-ds has updated the pull request. You must reimport the pull request before landing.
@xi-liu-ds has updated the pull request. You must reimport the pull request before landing.
@xi-liu-ds has updated the pull request. You must reimport the pull request before landing.
@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.
thanks @xi-liu-ds, very happy to have finally merged this! :tada:
I also merged InferAnalyzeJson into CaptureSILJson in e6809d9b5196311d6cc68eb409313645da63ef81