xazax.hun added 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 {
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. 

Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:187
+  if (isCalledByConstructor(Context))
+    return;
Szelethus wrote:
> whisperity wrote:
> > I think somewhere there should be a bit of reasoning why exactly these 
> > cases are ignored by the checker.
> At this function's declaration I have some comments describing what this does 
> and why it does it. Did you find it insufficient?
I am a bit surprised why these errors are not deduplicated by the analyzer. 
Maybe it would worth some investigation?

Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:272
+// If Chain's value is None, that means that this function was called to
+// determine whether a union has any initialized fields. In this case we're not
+// collecting fields, we'd only like to know  whether the value contained in
I find the documentation and the name of the function below misleading. 
`isNonUnionUninit` tells me this function is not supposed to be called for 
unions but the documentation suggests otherwise.

Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:283
+  bool ContainsUninitField = false;
+  Optional<FieldChainInfo> LocalChain = Chain;
Why do you copy here explicitly instead of taking the `Chain` argument by value?

Comment at: lib/StaticAnalyzer/Checkers/CtorUninitializedMemberChecker.cpp:292
+      if (LocalChain)
+        LocalChain->push_back(FR);
+      if (isNonUnionUninit(FR, LocalChain))
Don't you need to pop somewhere?


cfe-commits mailing list

Reply via email to