[cfe-commits] r140131 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h lib/StaticAnalyzer/Core/PathDiagnostic.cpp

[Ted Kremenek]

On Sep 20, 2011, at 9:27 AM, Anna Zaks wrote:

> Thanks for catching this! LocationContext's constructor is protected so we cannot construct one from AnalysisContext directly (passing ParentMap would solve the issue, I forgot that it's constructed on demand). We could require either LocationContextManager or AnalysisManager to be passed in by the users that do not have LocationContext. AnalysisManager is probably better since that's what is passed to the checkers.
> 


I don't think the managers should get involved here.    How would PathDiagnosticLocation know which AnalysisContext to create?  It also seems unnecessary since we already have an AnalysisContext object almost everywhere.  From a design point, LocationContexts represent points of context-sensitivity, so it doesn't make sense to create them on-the-fly when they aren't being used for that purpose.  Similarly, we create AnalysisContexts at key points when we start running an analysis to cache centralized data (e.g., CFGs, live variables information, etc.) used by one or more analyses.  PathDiagnosticLocation shouldn't be just creating them on-the-fly.

>From my perspective, I see two variants:

(1) PathDiagnosticLocations that utilize context-sensitivity.

(2) PathDiagnosticLocations that don't utilize context-sensitivity.

Only genRange() and genLocation() use the LocationContext*, and they could just use an AnalysisContext* instead.  We don't utilize the context-sensitivty in any way yet in PathDiagnosticLocation, but that is something we may add later.

It seems like want to support (1) and (2) easily for both clients.

Here's a possibility.  Take:

> PathDiagnosticLocation
>   PathDiagnosticLocation::createDeclBegin(const LocationContext *LC,
>                                           const SourceManager &SM) 


and make it:

> PathDiagnosticLocation
>   PathDiagnosticLocation::createDeclBegin(LocationOrAnalysisContext Ctx,
>                                           const SourceManager &SM) 


LocationOrAnalysisContext could be a typedef for a PointerUnion of LocationContext* and AnalysisContext*.  Then genRange() and genLocation() can just take an AnalysisContext* (because we can get an AnalysisContext* from a LocationContext*) and all of the clients don't need to provide a LocationContext* if they don't care about context-sensitivity.  When we decide to model context-sensitivty in PathDiagnosticLocation, we just query LocationOrAnalysisContext to see if it is a LocationContext*, and then do the right thing (whatever that ends up being).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110920/09b0c485/attachment.html>