Szelethus marked 6 inline comments as done. Szelethus added a comment. I'm about to update the diff, I changed a quite a lot of stuff, so I'm not sure that I'd be able to respond to these inline comments.
================ Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:31 +/// every other element is a field, and the element that precedes it is the +/// object that contains it. +class FieldChainInfo { ---------------- xazax.hun wrote: > As far as I understand, for every operation, the only relevant contribution > of the non-last elements in the chain is the diagnostic message. > I wonder if you would build a string instead of a FieldRegion chain and only > store the last FieldRegion would make this more explicit in the code. I experimented with a number of implementations along this idea, but I just couldn't get it to work. Here are my findings: * `Twine`-s aren't meant to be copied, so using them didn't work out * `StringRef`s for some reason get invalidated by the time it'd make a call to `FieldChainInfo::getAsString()` * I didn't like the idea of storing the string because it'd not only impact performance greatly, but it'd also make the code a lot harder to understand, as opposed to making it more explicit I did however try to make the implementation more easy to understand. ================ Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:35 + + FieldChain Chain; + // If this is a fieldchain whose last element is an uninitialized region of a ---------------- xazax.hun wrote: > I was wondering, do you need the chain at all? I think a field region might > be sufficient. The enclosing object of the field should be accessible by > querying the super region of the field region. I tried it, but that approach made it impossible to include pointers and references in the fieldchain. ================ Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:218 + + std::string WarningMsg = std::to_string(UninitFields.size()) + + " uninitialized field" + ---------------- whisperity wrote: > Maybe one could use a Twine or a string builder to avoid all these > unneccessary string instantiations? Or `std::string::append()`? Doesn't move semantics take care of that? I'm not too sure on this one. https://reviews.llvm.org/D45532 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits