On Sep 20, 2011, at 9:45 AM, Ted Kremenek wrote: > 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).
That is a better solution. Thanks, Anna.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
