NoQ added a comment. Looks great, thanks!
> CXXDynamicCastExpr: I don't think evalCast would handle this correctly, does > it? Seems so. It is partially supported by `StoreManager::attemptDownCast()`. I also see relatively little urgency in handling it here because the whole point of `dynamic_cast` is to be unknown in compile-time; why would anybody need to dynamic_cast a compile-time null pointer? Also for what-does-what sorts of answers you can consult the code in `ExprEngine::Visit*` which is essentially a huge switch by statement kinds. > ObjCBridgedCastExpr: I don't know ObjectiveC Same here, it's quite pointless to have a constant null pointer casted from one language to another. > CastKinds for floating point: Floats cannot yet be handled by the analyzer, > right? Yep, that's right. In https://reviews.llvm.org/D45774#1071666, @MTC wrote: > Test files for `initialization` missing? : ) Yep, seems so. This is stuff like struct S { int x = 0; }; I'm not sure it even needs to be supported explicitly. If the field is static, it most likely acts like a simple global variable (i.e. a `VarDecl`, not a `FieldDecl`). If the field is non-static, we should pick-up the in-class initializer in the constructor. Also if it's non-static and const, it is unlikely to have a constant initializer, because why would anybody copy the same constant into every instance of the class. ================ Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1638 + if (const Expr *Init = VD->getInit()) { + if (const InitListExpr *InitList = dyn_cast<InitListExpr>(Init)) { + // The array index has to be known. ---------------- Using `const auto *` is recommended with `dyn_cast` (also a plain `auto` with `getAs` and such), so that not to write down the type twice. We have a lot of old code around, but the new code should probably follow this recommendation. ================ Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1642-1645 + // Return unknown value if index is out of bounds. + if (i < 0 || i >= InitList->getNumInits()) { + return UnknownVal(); + } ---------------- Hmm, we might as well want to return an `UndefinedVal()` when the index is out of bounds of the actual array. The size of the array can be retrieved from `VD`. Though if you decide to do that, please put it into a separate patch :) ================ Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1723 + if (const InitListExpr *InitList = dyn_cast<InitListExpr>(Init)) { + if (const Expr *FieldInit = InitList->getInit(FD->getFieldIndex())) { + if (Optional<SVal> V = svalBuilder.getConstantVal(FieldInit)) ---------------- This method crashes when the index is out of range. Not sure - is it possible to skip a few fields in the list? Would the list's AST automatically contain any placeholders in this case? Repository: rC Clang https://reviews.llvm.org/D45774 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
