This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0950332e91df: Fix false positive with unreachable C++ catch
handlers (authored by aaron.ballman).
Changed prior to commit:
https://reviews.llvm.org/D145408?vs=502739&id=505106#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D145408/new/
https://reviews.llvm.org/D145408
Files:
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaStmt.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/SemaCXX/unreachable-catch-clauses.cpp
Index: clang/test/SemaCXX/unreachable-catch-clauses.cpp
===================================================================
--- clang/test/SemaCXX/unreachable-catch-clauses.cpp
+++ clang/test/SemaCXX/unreachable-catch-clauses.cpp
@@ -9,5 +9,25 @@
void test()
try {}
catch (BaseEx &e) { f(); } // expected-note 2{{for type 'BaseEx &'}}
-catch (Ex1 &e) { f(); } // expected-warning {{exception of type 'Ex1 &' will be caught by earlier handler}}
-catch (Ex2 &e) { f(); } // expected-warning {{exception of type 'Ex2 &' (aka 'Ex1 &') will be caught by earlier handler}}
+catch (Ex1 &e) { f(); } // expected-warning {{exception of type 'Ex1 &' will be caught by earlier handler}} \
+ expected-note {{for type 'Ex1 &'}}
+// FIXME: It would be nicer to only issue one warning on the below line instead
+// of two. We get two diagnostics because the first one is noticing that there
+// is a class hierarchy inversion where the earlier base class handler will
+// catch throwing the derived class and the second one is because Ex2 and Ex1
+// are the same type after canonicalization.
+catch (Ex2 &e) { f(); } // expected-warning 2{{exception of type 'Ex2 &' (aka 'Ex1 &') will be caught by earlier handler}}
+
+namespace GH61177 {
+void func() {
+ const char arr[4] = "abc";
+
+ // We should not issue an "exception will be caught by earlier handler"
+ // diagnostic, as that is a lie.
+ try {
+ throw arr;
+ } catch (char *p) {
+ } catch (const char *p) {
+ }
+}
+} // GH61177
Index: clang/test/CXX/drs/dr3xx.cpp
===================================================================
--- clang/test/CXX/drs/dr3xx.cpp
+++ clang/test/CXX/drs/dr3xx.cpp
@@ -161,6 +161,15 @@
struct C : A {};
struct D : B, C {};
void f() {
+ // NB: the warning here is correct despite being the opposite of the
+ // comments in the catch handlers. The "unreachable" comment is correct
+ // because there is an ambiguous base path to A from the D that is thrown.
+ // The warnings generated are also correct because the handlers handle
+ // const B& and const A& and we don't check to see if other derived classes
+ // exist that would cause an ambiguous base path. We issue the diagnostic
+ // despite the potential for a false positive because users are not
+ // expected to have ambiguous base paths all that often, so the false
+ // positive rate should be acceptably low.
try {
throw D();
} catch (const A&) { // expected-note {{for type 'const A &'}}
Index: clang/lib/Sema/SemaStmt.cpp
===================================================================
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -4351,9 +4351,9 @@
if (QT->isPointerType())
IsPointer = true;
+ QT = QT.getUnqualifiedType();
if (IsPointer || QT->isReferenceType())
QT = QT->getPointeeType();
- QT = QT.getUnqualifiedType();
}
/// Used when creating a CatchHandlerType from a base class type; pretends the
@@ -4402,31 +4402,43 @@
namespace {
class CatchTypePublicBases {
ASTContext &Ctx;
- const llvm::DenseMap<CatchHandlerType, CXXCatchStmt *> &TypesToCheck;
- const bool CheckAgainstPointer;
+ const llvm::DenseMap<QualType, CXXCatchStmt *> &TypesToCheck;
CXXCatchStmt *FoundHandler;
- CanQualType FoundHandlerType;
+ QualType FoundHandlerType;
+ QualType TestAgainstType;
public:
- CatchTypePublicBases(
- ASTContext &Ctx,
- const llvm::DenseMap<CatchHandlerType, CXXCatchStmt *> &T, bool C)
- : Ctx(Ctx), TypesToCheck(T), CheckAgainstPointer(C),
- FoundHandler(nullptr) {}
+ CatchTypePublicBases(ASTContext &Ctx,
+ const llvm::DenseMap<QualType, CXXCatchStmt *> &T,
+ QualType QT)
+ : Ctx(Ctx), TypesToCheck(T), FoundHandler(nullptr), TestAgainstType(QT) {}
CXXCatchStmt *getFoundHandler() const { return FoundHandler; }
- CanQualType getFoundHandlerType() const { return FoundHandlerType; }
+ QualType getFoundHandlerType() const { return FoundHandlerType; }
bool operator()(const CXXBaseSpecifier *S, CXXBasePath &) {
if (S->getAccessSpecifier() == AccessSpecifier::AS_public) {
- CatchHandlerType Check(S->getType(), CheckAgainstPointer);
+ QualType Check = S->getType().getCanonicalType();
const auto &M = TypesToCheck;
auto I = M.find(Check);
if (I != M.end()) {
- FoundHandler = I->second;
- FoundHandlerType = Ctx.getCanonicalType(S->getType());
- return true;
+ // We're pretty sure we found what we need to find. However, we still
+ // need to make sure that we properly compare for pointers and
+ // references, to handle cases like:
+ //
+ // } catch (Base *b) {
+ // } catch (Derived &d) {
+ // }
+ //
+ // where there is a qualification mismatch that disqualifies this
+ // handler as a potential problem.
+ if (I->second->getCaughtType()->isPointerType() ==
+ TestAgainstType->isPointerType()) {
+ FoundHandler = I->second;
+ FoundHandlerType = Check;
+ return true;
+ }
}
}
return false;
@@ -4465,6 +4477,7 @@
assert(!Handlers.empty() &&
"The parser shouldn't call this if there are no handlers.");
+ llvm::DenseMap<QualType, CXXCatchStmt *> HandledBaseTypes;
llvm::DenseMap<CatchHandlerType, CXXCatchStmt *> HandledTypes;
for (unsigned i = 0; i < NumHandlers; ++i) {
CXXCatchStmt *H = cast<CXXCatchStmt>(Handlers[i]);
@@ -4482,8 +4495,7 @@
// Walk the type hierarchy to diagnose when this type has already been
// handled (duplication), or cannot be handled (derivation inversion). We
// ignore top-level cv-qualifiers, per [except.handle]p3
- CatchHandlerType HandlerCHT =
- (QualType)Context.getCanonicalType(H->getCaughtType());
+ CatchHandlerType HandlerCHT = H->getCaughtType().getCanonicalType();
// We can ignore whether the type is a reference or a pointer; we need the
// underlying declaration type in order to get at the underlying record
@@ -4499,10 +4511,12 @@
// as the original type.
CXXBasePaths Paths;
Paths.setOrigin(RD);
- CatchTypePublicBases CTPB(Context, HandledTypes, HandlerCHT.isPointer());
+ CatchTypePublicBases CTPB(Context, HandledBaseTypes,
+ H->getCaughtType().getCanonicalType());
if (RD->lookupInBases(CTPB, Paths)) {
const CXXCatchStmt *Problem = CTPB.getFoundHandler();
- if (!Paths.isAmbiguous(CTPB.getFoundHandlerType())) {
+ if (!Paths.isAmbiguous(
+ CanQualType::CreateUnsafe(CTPB.getFoundHandlerType()))) {
Diag(H->getExceptionDecl()->getTypeSpecStartLoc(),
diag::warn_exception_caught_by_earlier_handler)
<< H->getCaughtType();
@@ -4511,11 +4525,16 @@
<< Problem->getCaughtType();
}
}
+ // Strip the qualifiers here because we're going to be comparing this
+ // type to the base type specifiers of a class, which are ignored in a
+ // base specifier per [class.derived.general]p2.
+ HandledBaseTypes[Underlying.getUnqualifiedType()] = H;
}
// Add the type the list of ones we have handled; diagnose if we've already
// handled it.
- auto R = HandledTypes.insert(std::make_pair(H->getCaughtType(), H));
+ auto R = HandledTypes.insert(
+ std::make_pair(H->getCaughtType().getCanonicalType(), H));
if (!R.second) {
const CXXCatchStmt *Problem = R.first->second;
Diag(H->getExceptionDecl()->getTypeSpecStartLoc(),
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -193,6 +193,10 @@
(`#60405 <https://github.com/llvm/llvm-project/issues/60405>`_)
- Fix aggregate initialization inside lambda constexpr.
(`#60936 <https://github.com/llvm/llvm-project/issues/60936>`_)
+- No longer issue a false positive diagnostic about a catch handler that cannot
+ be reached despite being reachable. This fixes
+ `#61177 <https://github.com/llvm/llvm-project/issues/61177>`_ in anticipation
+ of `CWG2699 <https://wg21.link/CWG2699>_` being accepted by WG21.
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
