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 
* 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.

cfe-commits mailing list

Reply via email to