NoQ added a comment. This looks great, i think we should make a single super simple mock test and commit this.
@MTC, i really appreciate your help! ================ Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:59 + QualType RegType = TypedR->getValueType(); + if (RegType.getAsString() != "std::string") + return; ---------------- MTC wrote: > A little tip, there are other string types besides `std::string`, like > `std::wstring`, which can be added in the future. See [[ > http://en.cppreference.com/w/cpp/string/basic_string | basic_string ]]. Yup, the current check should work in many cases, but it should be better to compare the class name to `basic_string`. I'm also worried about various sorts of `std::__1::string`s (an inline namespace in libc++). ================ Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:64 + + if (Call.isCalled(CStrFn)) { + SymbolRef RawPtr = Call.getReturnValue().getAsSymbol(); ---------------- It has long been planned to extend `isCalled` to C++ methods, i.e. something like this: ``` DanglingStringPointerChecker() : CStrFn("std::string::c_str") {} ... void DanglingStringPointerChecker::checkPostCall(const CallEvent &Call, CheckerContext &C) const { // Skip the class check. if (Call.isCalled(CStrFn)) { ... } ... } ``` Still looking for volunteers^^ ================ Comment at: lib/StaticAnalyzer/Checkers/DanglingStringPointerChecker.cpp:71 + + if (dyn_cast<CXXDestructorCall>(ICall)) { + if (State->contains<RawPtrMap>(TypedR)) { ---------------- MTC wrote: > `CXXDestructorCall::classof(const CallEvent *CA)` can also be used here. `isa`. ================ Comment at: lib/StaticAnalyzer/Checkers/RegionState.h:18 + +namespace region_state { + ---------------- MTC wrote: > I'm not sure whether `region_state` follows the naming conventions of LLVM > coding standards. Can someone answer this question? I think it does, cf. `namespace ast_matchers`. The name should be more specific though, a lot of checkers track "region states". Maybe "allocation_state"? Repository: rC Clang https://reviews.llvm.org/D47135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
