george.karpenkov requested changes to this revision.
george.karpenkov added a comment.
This revision now requires changes to proceed.
Definitely looks much cleaner, some nits inline. Can we protect against API
misuse?
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:769
+ /// that tracks objects under construction.
+ static ProgramStateRef finishObjectConstruction(ProgramStateRef State,
+ const Stmt *S,
----------------
Should we somehow assert that those functions are called in the correct order?
What will happen if e.g. `finishObjectConstruction` is called twice with the
same statement?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:101
-using InitializedTemporariesMap =
- llvm::ImmutableMap<std::pair<const CXXBindTemporaryExpr *,
- const StackFrameContext *>,
- const CXXTempObjectRegion *>;
-
-// Keeps track of whether CXXBindTemporaryExpr nodes have been evaluated.
-// The StackFrameContext assures that nested calls due to inlined recursive
-// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedTemporaries,
- InitializedTemporariesMap)
-
-using TemporaryMaterializationMap =
- llvm::ImmutableMap<std::pair<const MaterializeTemporaryExpr *,
- const StackFrameContext *>,
- const CXXTempObjectRegion *>;
-
-// Keeps track of temporaries that will need to be materialized later.
-// The StackFrameContext assures that nested calls due to inlined recursive
-// functions do not interfere.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(TemporaryMaterializations,
- TemporaryMaterializationMap)
-
-using CXXNewAllocatorValuesMap =
- llvm::ImmutableMap<std::pair<const CXXNewExpr *, const LocationContext *>,
- SVal>;
-
-// Keeps track of return values of various operator new() calls between
-// evaluation of the inlined operator new(), through the constructor call,
-// to the actual evaluation of the CXXNewExpr.
-// TODO: Refactor the key for this trait into a LocationContext sub-class,
-// which would be put on the stack of location contexts before operator new()
-// is evaluated, and removed from the stack when the whole CXXNewExpr
-// is fully evaluated.
-// Probably do something similar to the previous trait as well.
-REGISTER_TRAIT_WITH_PROGRAMSTATE(CXXNewAllocatorValues,
- CXXNewAllocatorValuesMap)
+// Keeps track of whether objects corresponding to the statement have been
+// fully constructed.
----------------
The comment seems incorrect, the map is not only answering the "whether"
question, it also maps those pairs into --- (their regions where these objects
are constructed into?)
================
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:377
+ if (!State->contains<ObjectsUnderConstruction>(Key)) {
+ return State->set<ObjectsUnderConstruction>(Key, V);
}
----------------
nitpick: most C++ containers eliminate the need for two lookups by allowing the
`get` method to return a reference. I'm not sure whether this can be done here
though.
Repository:
rC Clang
https://reviews.llvm.org/D47303
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
