[Sema] Don't emit "pure virtual" warning for fully qualified calls.

[thakis]

Qualified calls to virtual methods still go through the vtable in
-fapple-kext mode, so you probably don't want to change that there. (Look
for AppleKext in CodeGen and Sema.)

[davide]

Interesting, thanks. I didn't know about that. I'll update the patch accordingly tomorrow.

[davide]

Addressed Nico's comments. While at it added a test.
Nico, I hope this is the right way of checking.

[thakis]

The kext bits look good to me. I'm not sure if not warning on this at all is a good idea (it looks pretty weird), but the existing warning isn't correct. Maybe the "; overrides of 'f' in subclasses are not available in the constructor of 'A'" should just be omitted in this case? (This is probably up to Richard.)

[rsmith]

In D10935#199903, @thakis wrote:

Maybe the "; overrides of 'f' in subclasses are not available in the constructor of 'A'" should just be omitted in this case? (This is probably up to Richard.)

Maybe add a note "qualified call to 'A::f' is treated as a virtual call to 'f' due to -fapple-kext"?

(Also, how about adding "has undefined behavior" before the semicolon in the warning?)

I also wonder if we should add an performsVirtualDispatch(const LangOptions &) to MemberExpr, instead of the somewhat more opaque check for a qualifier here. (There are other places where we perform a similar check, such as MarkExprReferenced and Sema::MarkMemberReferenced, which get the kext case wrong.)

lib/Sema/SemaOverload.cpp

11796

Use hasQualifier instead of getQualifier.

[davide]

Addressed Richards' comments.

[rsmith]

LGTM

include/clang/AST/Expr.h

2584 ↗

(On Diff #29631)

Would it make sense for this to also check whether the named member is a virtual function?

include/clang/Basic/DiagnosticSemaKinds.td

1316 ↗

(On Diff #29631)

Remove the 'u' from behaviour; our diagnostics are in US English.

lib/Sema/SemaOverload.cpp

   if (CheckFunctionCall(Method, TheCall, Proto))
     return ExprError();
 
   if ((isa<CXXConstructorDecl>(CurContext) || 
        isa<CXXDestructorDecl>(CurContext)) && 
       TheCall->getMethodDecl()->isPure()) {
     const CXXMethodDecl *MD = TheCall->getMethodDecl();
 
-    if (isa<CXXThisExpr>(MemExpr->getBase()->IgnoreParenCasts())) {
-      Diag(MemExpr->getLocStart(), 
+    // If the member access is fully qualified, (i.e. X::f()), treat
+    // it as a non-virtual call and don't warn. In -fapple-kext mode
+    // qualified calls to virtual method will still go through the
+    // vtable so we don't want to skip the warning.
+    if (isa<CXXThisExpr>(MemExpr->getBase()->IgnoreParenCasts()) &&
+       (getLangOpts().AppleKext || !MemExpr->getQualifier())) {
+      Diag(MemExpr->getLocStart(),
            diag::warn_call_to_pure_virtual_member_function_from_ctor_dtor)
         << MD->getDeclName() << isa<CXXDestructorDecl>(CurContext)
         << MD->getParent()->getDeclName();
 
       Diag(MD->getLocStart(), diag::note_previous_decl) << MD->getDeclName();
     }
   }
   return MaybeBindToTemporary(TheCall);

test/SemaCXX/warn-pure-virtual-call-from-ctor-dtor.cpp

 // RUN: %clang_cc1 %s -fsyntax-only -verify
 struct A {
   A() { f(); } // expected-warning {{call to pure virtual member function 'f'; overrides of 'f' in subclasses are not available in the constructor of 'A'}}
   ~A() { f(); } // expected-warning {{call to pure virtual member function 'f'; overrides of 'f' in subclasses are not available in the destructor of 'A'}}
 
   virtual void f() = 0; // expected-note 2 {{'f' declared here}}
 };
 
 // Don't warn (or note) when calling the function on a pointer. (PR10195)
 struct B {
   A *a;
   B() { a->f(); };
   ~B() { a->f(); };
 };
+
+// Don't warn if the call is fully qualified. (PR23215)
+struct C {
+    virtual void f() = 0;
+    C() {
+        C::f();
+    }
+};

test/SemaCXX/warn-pure-virtual-kext.cpp

+// RUN: %clang_cc1 %s -fapple-kext -fsyntax-only -verify
+
+struct A {
+    virtual void f() = 0; // expected-note {{'f' declared here}}
+    A() {
+        A::f(); // expected-warning {{call to pure virtual member function 'f'; overrides of 'f' in subclasses are not available in the constructor of 'A'}}
+    }
+};