Besides the comments above, see
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
on how to get a full diff. Giving Phabricator a full diff makes reviewing
easier.
================
Comment at: lib/Sema/SemaDeclCXX.cpp:2643
@@ +2642,3 @@
+class DiagnoseMemberCallVisitor
+ : public RecursiveASTVisitor<DiagnoseMemberCallVisitor> {
+ Sema &S;
----------------
There's multiple visitors in Clang. The RecursiveASTVisitor will visit every
AST node, including unevaluated contexts such as sizeof expressions. It would
be better to use an EvaluatedExprVisitor which ignores unevaluated expressions.
Since there already is an EvaluatedExprVisitor for member initializers, it
would be a better idea to try to add to that visitor instead of writing your
own to minimize how often these expressions are traversed.
The best place to look is HandleMemberExpr inside UninitializedFieldVisitor.
This is were all MemberExpr's that need to be checked are funneled into.
================
Comment at: test/SemaCXX/member-call-before-bases-init.cpp:1
@@ +1,2 @@
+// RUN: %clang_cc1 -fsyntax-only -Wuninitialized -verify %s
+
----------------
Merge this test case with uninitalized.cpp. The warnings here and in
uninitalized.cpp come from the same warning group and should be kept together
in test cases.
================
Comment at: test/SemaCXX/member-call-before-bases-init.cpp:31
@@ +30,3 @@
+public:
+ D() : C(f()), // expected-warning{{member function 'f' called before all
base classes were initialized}}
+ i(f()) {} // no-warning
----------------
f() is from class B, which is fully initialized when class C is being
constructed. There is no need for a warning here.
================
Comment at: test/SemaCXX/member-call-before-bases-init.cpp:33
@@ +32,2 @@
+ i(f()) {} // no-warning
+};
----------------
Add a test case using an unevaluated context to check that no warning is
emitted. For example, sizeof(f()) in an initializer.
================
Comment at: test/SemaCXX/uninitialized.cpp:1326
@@ -1325,2 +1325,3 @@
// expected-warning@-1 {{base_class_access::A' is uninitialized when used
here to access 'base_class_access::A::foo'}}
+ // expected-warning@-2 {{member function 'foo' called before all base
classes were initialized}}
};
----------------
Don't trigger the warning here. The existing warning is good enough and the
new warning doesn't add anything new here.
================
Comment at: test/SemaCXX/uninitialized.cpp:1342
@@ -1340,2 +1341,3 @@
// expected-warning@-1 {{base_class_access::A' is uninitialized when used
here to access 'base_class_access::A::foo'}}
+ // expected-warning@-2 {{member function 'foo' called before all base
classes were initialized}}
};
----------------
Same as above.
http://reviews.llvm.org/D6561
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
