On Oct 30, 2012, at 9:18 AM, Jordan Rose wrote: > > On Oct 29, 2012, at 21:17 , Anna Zaks <[email protected]> wrote: > >> Author: zaks >> Date: Mon Oct 29 23:17:40 2012 >> New Revision: 167000 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=167000&view=rev >> Log: >> [analyzer] Fix a bug in REGISTER_MAP_WITH_PROGRAMSTATE >> >> The ImmutableMap should not be the key into the GDM map as there could >> be several entries with the same map type. Thanks, Jordan. >> >> This complicates the usage of the macro a bit. When we want to retrieve >> the whole map, we need to use another name. Currently, I set it to be >> Name ## Ty as in "type of the map we are storing in the ProgramState". > > Not bad. I wonder if we can do better, though (untested): > > class Name : public llvm::ImmutableMap<Key, Value> { > explicit Name(const ImmutableMap::TreeTy *Root) : ImmutableMap(Root) {} > }; > > It makes the macro a bit uglier, because of the constructor, but it does > reunify the lookup type and the map type. Maybe it's not worth it, though. > What do you think?
I could not get your suggestion to work as is, so something is missing. Complicating the macro is not a problem if we can get rid of Name ## Ty. Anna. > >> Modified: >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >> cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp >> >> Modified: >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h?rev=167000&r1=166999&r2=167000&view=diff >> ============================================================================== >> --- >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >> (original) >> +++ >> cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h >> Mon Oct 29 23:17:40 2012 >> @@ -18,20 +18,21 @@ >> #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h" >> #include "llvm/ADT/ImmutableMap.h" >> >> -/// Declare an immutable map suitable for placement into the ProgramState. >> -#define REGISTER_MAP_WITH_PROGRAMSTATE(Map, Key, Value) \ >> - typedef llvm::ImmutableMap<Key, Value> Map; \ >> +/// Declares an immutable map of type NameTy, suitable for placement into >> +/// the ProgramState. The macro should not be used inside namespaces. >> +#define REGISTER_MAP_WITH_PROGRAMSTATE(Name, Key, Value) \ >> + class Name {}; \ >> + typedef llvm::ImmutableMap<Key, Value> Name ## Ty; \ >> namespace clang { \ >> namespace ento { \ >> template <> \ >> - struct ProgramStateTrait<Map> \ >> - : public ProgramStatePartialTrait<Map> { \ >> + struct ProgramStateTrait<Name> \ >> + : public ProgramStatePartialTrait<Name ## Ty> { \ >> static void *GDMIndex() { static int Index; return &Index; } \ >> }; \ >> } \ >> } >> >> - >> namespace clang { >> namespace ento { >> >> >> Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp?rev=167000&r1=166999&r2=167000&view=diff >> ============================================================================== >> --- cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp (original) >> +++ cfe/trunk/lib/StaticAnalyzer/Checkers/SimpleStreamChecker.cpp Mon Oct 29 >> 23:17:40 2012 >> @@ -106,9 +106,7 @@ >> CheckerContext &C) const { >> initIdentifierInfo(C.getASTContext()); >> >> - if (C.getCalleeIdentifier(Call) != IIfclose) >> - return; >> - if (Call->getNumArgs() != 1) >> + if (C.getCalleeIdentifier(Call) != IIfclose || Call->getNumArgs() != 1) >> return; >> >> // Get the symbolic value corresponding to the file handle. >> @@ -130,9 +128,9 @@ >> void SimpleStreamChecker::checkDeadSymbols(SymbolReaper &SymReaper, >> CheckerContext &C) const { >> ProgramStateRef State = C.getState(); >> - StreamMap TrackedStreams = State->get<StreamMap>(); >> + StreamMapTy TrackedStreams = State->get<StreamMap>(); >> SymbolVector LeakedStreams; >> - for (StreamMap::iterator I = TrackedStreams.begin(), >> + for (StreamMapTy::iterator I = TrackedStreams.begin(), >> E = TrackedStreams.end(); I != E; ++I) { >> SymbolRef Sym = I->first; >> if (SymReaper.isDead(Sym)) { >> @@ -154,9 +152,9 @@ >> ProgramStateRef SimpleStreamChecker::evalAssume(ProgramStateRef State, >> SVal Cond, >> bool Assumption) const { >> - StreamMap TrackedStreams = State->get<StreamMap>(); >> + StreamMapTy TrackedStreams = State->get<StreamMap>(); >> SymbolVector LeakedStreams; >> - for (StreamMap::iterator I = TrackedStreams.begin(), >> + for (StreamMapTy::iterator I = TrackedStreams.begin(), >> E = TrackedStreams.end(); I != E; ++I) { >> SymbolRef Sym = I->first; >> if (State->getConstraintManager().isNull(State, Sym).isTrue()) >> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
