Add return type checking for overriding virtual functions. We currently don't check covariance but that's next.

llvm-svn: 71759

3 files changed, 59 insertions, 4 deletions

diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h
index bd01ec9e514..5b263b7d92c 100644
--- a/clang/lib/Sema/Sema.h
+++ b/clang/lib/Sema/Sema.h
@@ -1780,6 +1780,12 @@ public:
                                     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
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index e27517e6896..a2f7af1d8be 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -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 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
   //   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 @@ Sema::ActOnFunctionDeclarator(Scope* S, Declarator& D, DeclContext* DC,
     }
   }
 
+  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:
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 726080b7ff6..bdd3cc2be14 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -2688,3 +2688,25 @@ void Sema::DiagnoseReturnInConstructorExceptionHandler(CXXTryStmt *TryBlock) {
     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;
+}