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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to