Add On-the-fly analysis support #694
Conversation
There was a problem hiding this comment.
Nice and hard work 👍
It would be really benefitial to have a lit test similar to ctu-main.cpp (with a few functions).
Other than that, could you please write a novel about the absolute and relative path difficulties? The goal is to understand the problem and the solution even after a few months...
gamesh411
force-pushed
the
otf
branch
2 times, most recently
from
9b4cb57
to
bc42096
Compare
|
Could you plase check the new test files 'ctu-on-the-fly.c' and 'ctu-on-the-fly.cpp'. I have managed to get almost working, however there are 2 issues still reamining. First issue: Second issue: What are your thoughts? Thanks |
There was a problem hiding this comment.
First issue:
During the analysis of sometimes the file part is entirely left off from the diagnostic. The interesting part is that Line number is OK, just the file part gets lost. This could be fixed by making the check more lenient in case of on-demand analysis (eg. not requiring that the diagnostic contains the file information).
Could you please elaborate this with examples?
Second issue:
Inline assembly importing fails if -std=c89 is given inside compile_commands.json for the c test (see the test RUN: lines for the contents of the compile_commands.json). If however no -std=c89 is given, then unsupported AST node error is given during the analysis (I assume that it must be the clang tool which parses the file).
That is strange, I cannot say what could be wrong here, we should be able to import inline assembly without the unsupported node error. We must further debug it.
|
I have a working solution updated. The parsing logic of external TUs should be refactored to increase maintainability. However, this change is complex enough as is, and I would recommend refactoring in a separate change. There is another ongoing issue with passing configuration options to CTUContext during the CallEvent handling of the analyzer engine. This promotes parameter-style passing of information, however, the Context object would be better off with constructor parameter injection or getters and setters (which is still a bit inferior to constructor injection because the values are set every time an external TU definition is requested from the context). |
|
Fixed a remaining typo. |
|
I have refactored and rebased the solution. There is an extra diagnostic as requested, and I have asserted some invariants in the code. |
|
CI still fails... |
|
run tests (this message is intended for the build bot, if you answer to this message then you owe me a bottle of whiskey) |
|
run tests |
Add an option to enable on-demand parsing of needed ASTs during CTU analysis, and another option to specify the compilation database used. The option CTUOnDemandParsing is a boolean flag, which enables the new AST-loading mode. Option CTUOnDemandParsingDatabase is a string, which should be the path of the compilation database used for on-demand parsing. The compilation database is needed for on-demand mode, because it has all the necessary information to generate the ASTs. Please enter the commit message for your changes. Lines starting
|
run tests |
Add an option to enable on-the-fly parsing of needed ASTs during CTU
analysis. The option CTUCompilationDatabase should be a path to a
compilation database, which has all the necessary information to
generate the ASTs. In case an empty string is given, on-the-fly parsing
is disabled.