[cfe-commits] r71759 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/Sema.h lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp test/SemaCXX/virtual-override.cpp
Anders Carlsson
andersca at mac.com
Wed May 13 18:09:20 PDT 2009
Author: andersca
Date: Wed May 13 20:09:04 2009
New Revision: 71759
URL: http://llvm.org/viewvc/llvm-project?rev=71759&view=rev
Log:
Add return type checking for overriding virtual functions. We currently don't check covariance but that's next.
Added:
cfe/trunk/test/SemaCXX/virtual-override.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=71759&r1=71758&r2=71759&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed May 13 20:09:04 2009
@@ -301,6 +301,12 @@
"cannot initialize object parameter of type %0 with an expression "
"of type %1">;
+def err_different_return_type_for_overriding_virtual_function : Error<
+ "virtual function %0 has a different return type (%1) than the "
+ "function it overrides (which has return type %2)">;
+def note_overridden_virtual_function : Note<
+ "overridden virtual function is here">;
+
// C++ constructors
def err_constructor_cannot_be : Error<"constructor cannot be declared '%0'">;
def err_invalid_qualified_constructor : Error<
Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=71759&r1=71758&r2=71759&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Wed May 13 20:09:04 2009
@@ -1780,6 +1780,12 @@
DeclarationName Name);
std::string getAmbiguousPathsDisplayString(BasePaths &Paths);
+
+ /// CheckReturnTypeCovariance - Checks whether two types are covariant,
+ /// according to C++ [class.virtual]p5.
+ bool CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
+ const CXXMethodDecl *Old);
+
//===--------------------------------------------------------------------===//
// C++ Access Control
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=71759&r1=71758&r2=71759&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed May 13 20:09:04 2009
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "Sema.h"
+#include "SemaInherit.h"
#include "clang/AST/APValue.h"
#include "clang/AST/ASTConsumer.h"
#include "clang/AST/ASTContext.h"
@@ -2113,10 +2114,6 @@
// nonstatic class member functions that appear within a
// member-specification of a class declaration; see 10.3.
//
- // FIXME: Checking the 'virtual' specifier is not sufficient. A
- // function is also virtual if it overrides an already virtual
- // function. This is important to do here because it's part of the
- // declaration.
if (isVirtual && !NewFD->isInvalidDecl()) {
if (!isVirtualOkay) {
Diag(D.getDeclSpec().getVirtualSpecLoc(),
@@ -2137,6 +2134,36 @@
}
}
+ if (CXXMethodDecl *NewMD = dyn_cast<CXXMethodDecl>(NewFD)) {
+ // Look for virtual methods in base classes that this method might override.
+
+ BasePaths Paths;
+ // FIXME: This will not include hidden member functions.
+ if (LookupInBases(cast<CXXRecordDecl>(DC),
+ MemberLookupCriteria(Name, LookupMemberName,
+ // FIXME: Shouldn't IDNS_Member be
+ // enough here?
+ Decl::IDNS_Member |
+ Decl::IDNS_Ordinary), Paths)) {
+ for (BasePaths::decl_iterator I = Paths.found_decls_begin(),
+ E = Paths.found_decls_end(); I != E; ++I) {
+ if (CXXMethodDecl *OldMD = dyn_cast<CXXMethodDecl>(*I)) {
+ OverloadedFunctionDecl::function_iterator MatchedDecl;
+ // FIXME: Is this OK? Should it be done by LookupInBases?
+ if (IsOverload(NewMD, OldMD, MatchedDecl))
+ continue;
+
+ if (!CheckOverridingFunctionReturnType(NewMD, OldMD)) {
+ // FIXME: Add OldMD to the list of methods NewMD overrides.
+ }
+
+ }
+ }
+
+ }
+
+ }
+
if (SC == FunctionDecl::Static && isa<CXXMethodDecl>(NewFD) &&
!CurContext->isRecord()) {
// C++ [class.static]p1:
Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=71759&r1=71758&r2=71759&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed May 13 20:09:04 2009
@@ -2688,3 +2688,25 @@
SearchForReturnInStmt(*this, Handler);
}
}
+
+bool Sema::CheckOverridingFunctionReturnType(const CXXMethodDecl *New,
+ const CXXMethodDecl *Old) {
+ QualType NewTy = New->getType()->getAsFunctionType()->getResultType();
+ QualType OldTy = Old->getType()->getAsFunctionType()->getResultType();
+
+ QualType CNewTy = Context.getCanonicalType(NewTy);
+ QualType COldTy = Context.getCanonicalType(OldTy);
+
+ if (CNewTy == COldTy &&
+ CNewTy.getCVRQualifiers() == COldTy.getCVRQualifiers())
+ return false;
+
+ // FIXME: Check covariance.
+
+ Diag(New->getLocation(),
+ diag::err_different_return_type_for_overriding_virtual_function)
+ << New->getDeclName() << NewTy << OldTy;
+ Diag(Old->getLocation(), diag::note_overridden_virtual_function);
+
+ return true;
+}
Added: cfe/trunk/test/SemaCXX/virtual-override.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/virtual-override.cpp?rev=71759&view=auto
==============================================================================
--- cfe/trunk/test/SemaCXX/virtual-override.cpp (added)
+++ cfe/trunk/test/SemaCXX/virtual-override.cpp Wed May 13 20:09:04 2009
@@ -0,0 +1,13 @@
+// RUN: clang-cc -fsyntax-only -verify %s
+
+namespace T1 {
+
+class A {
+ virtual int f(); // expected-note{{overridden virtual function is here}}
+};
+
+class B : A {
+ virtual void f(); // expected-error{{virtual function 'f' has a different return type ('void') than the function it overrides (which has return type 'int')}}
+};
+
+}
More information about the cfe-commits
mailing list