SemaExceptionSpec: ensure old/new specs are both parsed and evaluated when comparing

[elsteveogrande]

Summary

Before checking that the overriding definition of a method is not more lax than the overridden one, ensure the exception spec is parsed, and also evaluated.

Test case by Andrew Gallagher!

https://bugs.llvm.org/show_bug.cgi?id=38627

[vitaut]

lib/Sema/SemaExceptionSpec.cpp

915

Did you mean Old here?

[elsteveogrande]

lib/Sema/SemaExceptionSpec.cpp

915

facepalm, haha. Thanks @vitaut ! :)

[elsteveogrande]

fix dopey copy-paste error. Tested again with ninja check-clang-modules

[elsteveogrande]

This broke some tests. It fixes the test case added here under test/Modules but causes errors in test/CodeGenCXX and other dirs.

[elsteveogrande]

Abandoning in favor of https://reviews.llvm.org/D51608 which works better.

lib/Sema/SemaExceptionSpec.cpp

bool Sema::CheckExceptionSpecCompatibility(Expr *From, QualType ToType) {
   return CheckExceptionSpecSubset(PDiag(DiagID), PDiag(NestedDiagID), PDiag(),
                                   ToFunc, From->getSourceRange().getBegin(),
                                   FromFunc, SourceLocation()) &&
          !getLangOpts().CPlusPlus17;
 }
 
 bool Sema::CheckOverridingFunctionExceptionSpec(const CXXMethodDecl *New,
                                                 const CXXMethodDecl *Old) {
-  // If the new exception specification hasn't been parsed yet, skip the check.
+  // If the new exception spec hasn't been determined yet, skip the check.
   // We'll get called again once it's been parsed.
-  if (New->getType()->castAs<FunctionProtoType>()->getExceptionSpecType() ==
-      EST_Unparsed)
+  switch (New->getType()->castAs<FunctionProtoType>()->getExceptionSpecType()) {
+  case EST_Unparsed:
+  case EST_Unevaluated:
     return false;
+  default:
+    ;
+  }
+
+  // If the old exception spec hasn't been determined yet, remember that
+  // we need to perform this check when we get to the end of the outermost
+  // lexically-surrounding class.
+  switch (Old->getType()->castAs<FunctionProtoType>()->getExceptionSpecType()) {
+  case EST_Unparsed:
+  case EST_Unevaluated:
+    DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
+    return false;
+  default:
+    ;
+  }
+
   if (getLangOpts().CPlusPlus11 && isa<CXXDestructorDecl>(New)) {
     // Don't check uninstantiated template destructors at all. We can only
     // synthesize correct specs after the template is instantiated.
     if (New->getParent()->isDependentType())
       return false;
     if (New->getParent()->isBeingDefined()) {
       // The destructor might be updated once the definition is finished. So
       // remember it and check later.
       DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
       return false;
     }
   }
-  // If the old exception specification hasn't been parsed yet, remember that
-  // we need to perform this check when we get to the end of the outermost
-  // lexically-surrounding class.
-  if (Old->getType()->castAs<FunctionProtoType>()->getExceptionSpecType() ==
-      EST_Unparsed) {
-    DelayedExceptionSpecChecks.push_back(std::make_pair(New, Old));
-    return false;
-  }
+
   unsigned DiagID = diag::err_override_exception_spec;
   if (getLangOpts().MicrosoftExt)
     DiagID = diag::ext_override_exception_spec;
+
   return CheckExceptionSpecSubset(PDiag(DiagID),
                                   PDiag(diag::err_deep_exception_specs_differ),
                                   PDiag(diag::note_overridden_virtual_function),
                                   Old->getType()->getAs<FunctionProtoType>(),
                                   Old->getLocation(),
                                   New->getType()->getAs<FunctionProtoType>(),
                                   New->getLocation());
 }

test/Modules/Inputs/lax-base-except/a.h

+class A {
+ public:
+  virtual ~A() = default;
+};
+
+class B {
+ public:
+  virtual ~B() {
+    c.func();
+  }
+
+  struct C{
+    void func() {}
+    friend B::~B();
+  };
+
+  C c;
+};
+

test/Modules/Inputs/lax-base-except/module.modulemap

+module a {
+  header "a.h"
+}

test/Modules/lax-base-except.cpp

+// RUN: rm -rf %t
+// RUN: %clang -c -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/lax-base-except %s -o %t2.o
+// expected-no-diagnostics
+
+#include "a.h"
+
+class D : public A, public B {
+ public:
+  virtual ~D() override = default;
+};