george.karpenkov added inline comments.
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:570
+ struct EvalCallFlags {
+ bool IsConstructorWithImproperlyModeledTargetRegion : 1;
----------------
Those are not really flags. Perhaps options?
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:572
+ bool IsConstructorWithImproperlyModeledTargetRegion : 1;
+ bool IsArrayConstructorOrDestructor : 1;
+ };
----------------
OK my C++ knowledge is weak here.
What happens if you don't initialize those at the callsite and then read?
Wouldn't be safer to set them both to false in the declaration?
================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h:661
+ /// When the lookahead fails, a temporary region is returned, and a flag is
+ /// set in \p Flags.
const MemRegion *getRegionForConstructedObject(const CXXConstructExpr *CE,
----------------
Which flag?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:92
///
/// If the type is not an array type at all, the original value is returned.
static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
----------------
Doxygen comment for out-parameter? E.g. it's non-obvious that it only sets the
flag to true and does not touch it otherwise.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:94
static SVal makeZeroElementRegion(ProgramStateRef State, SVal LValue,
- QualType &Ty) {
+ QualType &Ty, bool &IsIndeedAnArray) {
SValBuilder &SVB = State->getStateManager().getSValBuilder();
----------------
Perhaps `IsArray` will also do?
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:142
+ bool IsArray = false;
+ LValue = makeZeroElementRegion(State, LValue, Ty, IsArray);
+ if (IsArray)
----------------
Or I suppose you could do `makeZeroElementRegion(...,
&Flags.IsArrayConstructorOrDestructor)` to express the intent
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:171
+ bool IsArray = false;
+ FieldVal = makeZeroElementRegion(State, FieldVal, Ty, IsArray);
+ if (IsArray)
----------------
Same as previous comment, I suppose you could write into
`IsArrayConstructorOrDestructor` directly.
================
Comment at: lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:451
+ if (IsArray)
+ Flags.IsArrayConstructorOrDestructor = true;
+
----------------
Again perhaps write into `Flags.Is...` directly?
Repository:
rC Clang
https://reviews.llvm.org/D42457
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
