[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun marked 6 inline comments as done.
xazax.hun added inline comments.


================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:372
+
+  cross_tu::CrossTranslationUnitContext &CTUCtx =
+      Engine->getCrossTranslationUnitContext();
----------------
dcoughlin wrote:
> Can this logic be moved to AnalysisDeclContext->getBody()?
> 
> CallEvent::getRuntimeDefinition() is really all about modeling function dispatch at run time. It seems odd to have the cross-translation-unit loading (which is about compiler book-keeping rather than semantics) here.
I just tried that and unfortunately, that introduces cyclic dependencies. 
CrossTU depends on Frontend. Frontend depends on Sema. Sema depends on Analysis. Making Analysis depending on CrossTU introduces the cycle.


================
Comment at: lib/StaticAnalyzer/Core/CallEvent.cpp:382
+                    [&](const cross_tu::IndexError &IE) {
+                      CTUCtx.emitCrossTUDiagnostics(IE);
+                    });
----------------
dcoughlin wrote:
> I don't think it makes sense to diagnose index errors here.
> 
> Doing it when during analysis means that, for example, the parse error could be emitted or not emitted depending on whether the analyzer thinks a particular call site is reached.
> 
> It would be better to validate/parse the index before starting analysis rather than during analysis itself.
> 
> 
While I agree, right now this validation is not the part of the analyzer but the responsibility of the "driver" script for example CodeChecker. It is useful to have this diagnostics to catch bugs in the driver. 


https://reviews.llvm.org/D30691