[clang] [Clang] Introduce `CXXTypeidExpr::hasNullCheck` (PR #95718)
Mital Ashok via cfe-commits
cfe-commits at lists.llvm.org
Sun Jun 16 14:38:33 PDT 2024
https://github.com/MitalAshok updated https://github.com/llvm/llvm-project/pull/95718
>From c3912611dd63f81ea7067a4b26ef5450f6f01f75 Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sun, 16 Jun 2024 22:35:38 +0100
Subject: [PATCH 1/2] [Clang] Introduce CXXTypeidExpr::hasNullCheck
---
clang/docs/ReleaseNotes.rst | 3 ++
clang/include/clang/AST/ExprCXX.h | 4 ++
clang/lib/AST/Expr.cpp | 16 +++++--
clang/lib/AST/ExprCXX.cpp | 49 ++++++++++++++++++++++
clang/lib/CodeGen/CGCXXABI.h | 3 +-
clang/lib/CodeGen/CGExprCXX.cpp | 53 ++++--------------------
clang/lib/CodeGen/ItaniumCXXABI.cpp | 7 ++--
clang/lib/CodeGen/MicrosoftCXXABI.cpp | 8 ++--
clang/lib/Sema/SemaExceptionSpec.cpp | 20 +++------
clang/test/CXX/drs/cwg21xx.cpp | 13 ++++++
clang/test/SemaCXX/warn-unused-value.cpp | 10 +++++
clang/www/cxx_dr_status.html | 2 +-
12 files changed, 113 insertions(+), 75 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 69aea6c21ad39..6c92177d71298 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -268,6 +268,9 @@ Resolutions to C++ Defect Reports
- Clang now requires a template argument list after a template keyword.
(`CWG96: Syntactic disambiguation using the template keyword <https://cplusplus.github.io/CWG/issues/96.html>`_).
+- Clang no longer always reports ``!noexcept(typeid(expr))`` when the ``typeid`` cannot throw a ``std::bad_typeid``.
+ (`CWG2191: Incorrect result for noexcept(typeid(v)) <https://cplusplus.github.io/CWG/issues/2191.html>`_).
+
C Language Changes
------------------
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index d2e8d93656359..c2feac525c1ea 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -919,6 +919,10 @@ class CXXTypeidExpr : public Expr {
reinterpret_cast<Stmt **>(&const_cast<CXXTypeidExpr *>(this)->Operand);
return const_child_range(begin, begin + 1);
}
+
+ /// Whether this is of a form like "typeid(*ptr)" that can throw a
+ /// std::bad_typeid if a pointer is a null pointer ([expr.typeid]p2)
+ bool hasNullCheck() const;
};
/// A member reference to an MSPropertyDecl.
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 7e555689b64c4..37ba5b69f446d 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3769,10 +3769,18 @@ bool Expr::HasSideEffects(const ASTContext &Ctx,
break;
}
- case CXXTypeidExprClass:
- // typeid might throw if its subexpression is potentially-evaluated, so has
- // side-effects in that case whether or not its subexpression does.
- return cast<CXXTypeidExpr>(this)->isPotentiallyEvaluated();
+ case CXXTypeidExprClass: {
+ const auto *TE = cast<CXXTypeidExpr>(this);
+ if (!TE->isPotentiallyEvaluated())
+ return false;
+
+ // If this type id expression can throw because of a null pointer, that is a
+ // side-effect independent of if the operand has a side-effect
+ if (IncludePossibleEffects && TE->hasNullCheck())
+ return true;
+
+ break;
+ }
case CXXConstructExprClass:
case CXXTemporaryObjectExprClass: {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 2abc0acbfde3b..7ecdb908e7d9f 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -166,6 +166,55 @@ QualType CXXTypeidExpr::getTypeOperand(ASTContext &Context) const {
Operand.get<TypeSourceInfo *>()->getType().getNonReferenceType(), Quals);
}
+namespace {
+static bool isGLValueFromPointerDeref(const Expr *E) {
+ E = E->IgnoreParens();
+
+ if (const auto *CE = dyn_cast<CastExpr>(E)) {
+ if (!CE->getSubExpr()->isGLValue())
+ return false;
+ return isGLValueFromPointerDeref(CE->getSubExpr());
+ }
+
+ if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E))
+ return isGLValueFromPointerDeref(OVE->getSourceExpr());
+
+ if (const auto *BO = dyn_cast<BinaryOperator>(E))
+ if (BO->getOpcode() == BO_Comma)
+ return isGLValueFromPointerDeref(BO->getRHS());
+
+ if (const auto *ACO = dyn_cast<AbstractConditionalOperator>(E))
+ return isGLValueFromPointerDeref(ACO->getTrueExpr()) ||
+ isGLValueFromPointerDeref(ACO->getFalseExpr());
+
+ // C++11 [expr.sub]p1:
+ // The expression E1[E2] is identical (by definition) to *((E1)+(E2))
+ if (isa<ArraySubscriptExpr>(E))
+ return true;
+
+ if (const auto *UO = dyn_cast<UnaryOperator>(E))
+ if (UO->getOpcode() == UO_Deref)
+ return true;
+
+ return false;
+}
+} // namespace
+
+bool CXXTypeidExpr::hasNullCheck() const {
+ if (!isPotentiallyEvaluated())
+ return false;
+
+ // C++ [expr.typeid]p2:
+ // If the glvalue expression is obtained by applying the unary * operator to
+ // a pointer and the pointer is a null pointer value, the typeid expression
+ // throws the std::bad_typeid exception.
+ //
+ // However, this paragraph's intent is not clear. We choose a very generous
+ // interpretation which implores us to consider comma operators, conditional
+ // operators, parentheses and other such constructs.
+ return isGLValueFromPointerDeref(getExprOperand());
+}
+
QualType CXXUuidofExpr::getTypeOperand(ASTContext &Context) const {
assert(isTypeOperand() && "Cannot call getTypeOperand for __uuidof(expr)");
Qualifiers Quals;
diff --git a/clang/lib/CodeGen/CGCXXABI.h b/clang/lib/CodeGen/CGCXXABI.h
index c7eccbd0095a9..104a20db8efaf 100644
--- a/clang/lib/CodeGen/CGCXXABI.h
+++ b/clang/lib/CodeGen/CGCXXABI.h
@@ -274,8 +274,7 @@ class CGCXXABI {
getAddrOfCXXCatchHandlerType(QualType Ty, QualType CatchHandlerType) = 0;
virtual CatchTypeInfo getCatchAllTypeInfo();
- virtual bool shouldTypeidBeNullChecked(bool IsDeref,
- QualType SrcRecordTy) = 0;
+ virtual bool shouldTypeidBeNullChecked(QualType SrcRecordTy) = 0;
virtual void EmitBadTypeidCall(CodeGenFunction &CGF) = 0;
virtual llvm::Value *EmitTypeid(CodeGenFunction &CGF, QualType SrcRecordTy,
Address ThisPtr,
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 3c4f59fc765fe..8eb6ab7381acb 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -2143,40 +2143,9 @@ void CodeGenFunction::EmitCXXDeleteExpr(const CXXDeleteExpr *E) {
}
}
-static bool isGLValueFromPointerDeref(const Expr *E) {
- E = E->IgnoreParens();
-
- if (const auto *CE = dyn_cast<CastExpr>(E)) {
- if (!CE->getSubExpr()->isGLValue())
- return false;
- return isGLValueFromPointerDeref(CE->getSubExpr());
- }
-
- if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E))
- return isGLValueFromPointerDeref(OVE->getSourceExpr());
-
- if (const auto *BO = dyn_cast<BinaryOperator>(E))
- if (BO->getOpcode() == BO_Comma)
- return isGLValueFromPointerDeref(BO->getRHS());
-
- if (const auto *ACO = dyn_cast<AbstractConditionalOperator>(E))
- return isGLValueFromPointerDeref(ACO->getTrueExpr()) ||
- isGLValueFromPointerDeref(ACO->getFalseExpr());
-
- // C++11 [expr.sub]p1:
- // The expression E1[E2] is identical (by definition) to *((E1)+(E2))
- if (isa<ArraySubscriptExpr>(E))
- return true;
-
- if (const auto *UO = dyn_cast<UnaryOperator>(E))
- if (UO->getOpcode() == UO_Deref)
- return true;
-
- return false;
-}
-
static llvm::Value *EmitTypeidFromVTable(CodeGenFunction &CGF, const Expr *E,
- llvm::Type *StdTypeInfoPtrTy) {
+ llvm::Type *StdTypeInfoPtrTy,
+ bool HasNullCheck) {
// Get the vtable pointer.
Address ThisPtr = CGF.EmitLValue(E).getAddress();
@@ -2189,16 +2158,11 @@ static llvm::Value *EmitTypeidFromVTable(CodeGenFunction &CGF, const Expr *E,
CGF.EmitTypeCheck(CodeGenFunction::TCK_DynamicOperation, E->getExprLoc(),
ThisPtr, SrcRecordTy);
- // C++ [expr.typeid]p2:
- // If the glvalue expression is obtained by applying the unary * operator to
- // a pointer and the pointer is a null pointer value, the typeid expression
- // throws the std::bad_typeid exception.
- //
- // However, this paragraph's intent is not clear. We choose a very generous
- // interpretation which implores us to consider comma operators, conditional
- // operators, parentheses and other such constructs.
- if (CGF.CGM.getCXXABI().shouldTypeidBeNullChecked(
- isGLValueFromPointerDeref(E), SrcRecordTy)) {
+ // Whether we need an explicit null pointer check. For example, with the
+ // Microsoft ABI, if this is a call to __RTtypeid, the null pointer check and
+ // exception throw is inside the __RTtypeid(nullptr) call
+ if (HasNullCheck &&
+ CGF.CGM.getCXXABI().shouldTypeidBeNullChecked(SrcRecordTy)) {
llvm::BasicBlock *BadTypeidBlock =
CGF.createBasicBlock("typeid.bad_typeid");
llvm::BasicBlock *EndBlock = CGF.createBasicBlock("typeid.end");
@@ -2244,7 +2208,8 @@ llvm::Value *CodeGenFunction::EmitCXXTypeidExpr(const CXXTypeidExpr *E) {
// type) to which the glvalue refers.
// If the operand is already most derived object, no need to look up vtable.
if (E->isPotentiallyEvaluated() && !E->isMostDerived(getContext()))
- return EmitTypeidFromVTable(*this, E->getExprOperand(), PtrTy);
+ return EmitTypeidFromVTable(*this, E->getExprOperand(), PtrTy,
+ E->hasNullCheck());
QualType OperandTy = E->getExprOperand()->getType();
return MaybeASCast(CGM.GetAddrOfRTTIDescriptor(OperandTy));
diff --git a/clang/lib/CodeGen/ItaniumCXXABI.cpp b/clang/lib/CodeGen/ItaniumCXXABI.cpp
index 8427286dee887..b6c76e1e8ea75 100644
--- a/clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ b/clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -178,7 +178,7 @@ class ItaniumCXXABI : public CodeGen::CGCXXABI {
return CatchTypeInfo{getAddrOfRTTIDescriptor(Ty), 0};
}
- bool shouldTypeidBeNullChecked(bool IsDeref, QualType SrcRecordTy) override;
+ bool shouldTypeidBeNullChecked(QualType SrcRecordTy) override;
void EmitBadTypeidCall(CodeGenFunction &CGF) override;
llvm::Value *EmitTypeid(CodeGenFunction &CGF, QualType SrcRecordTy,
Address ThisPtr,
@@ -1419,9 +1419,8 @@ static llvm::FunctionCallee getBadTypeidFn(CodeGenFunction &CGF) {
return CGF.CGM.CreateRuntimeFunction(FTy, "__cxa_bad_typeid");
}
-bool ItaniumCXXABI::shouldTypeidBeNullChecked(bool IsDeref,
- QualType SrcRecordTy) {
- return IsDeref;
+bool ItaniumCXXABI::shouldTypeidBeNullChecked(QualType SrcRecordTy) {
+ return true;
}
void ItaniumCXXABI::EmitBadTypeidCall(CodeGenFunction &CGF) {
diff --git a/clang/lib/CodeGen/MicrosoftCXXABI.cpp b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
index e4f798f6a97d9..9ab634fa6ce2e 100644
--- a/clang/lib/CodeGen/MicrosoftCXXABI.cpp
+++ b/clang/lib/CodeGen/MicrosoftCXXABI.cpp
@@ -144,7 +144,7 @@ class MicrosoftCXXABI : public CGCXXABI {
return CatchTypeInfo{nullptr, 0x40};
}
- bool shouldTypeidBeNullChecked(bool IsDeref, QualType SrcRecordTy) override;
+ bool shouldTypeidBeNullChecked(QualType SrcRecordTy) override;
void EmitBadTypeidCall(CodeGenFunction &CGF) override;
llvm::Value *EmitTypeid(CodeGenFunction &CGF, QualType SrcRecordTy,
Address ThisPtr,
@@ -977,11 +977,9 @@ MicrosoftCXXABI::performBaseAdjustment(CodeGenFunction &CGF, Address Value,
PolymorphicBase);
}
-bool MicrosoftCXXABI::shouldTypeidBeNullChecked(bool IsDeref,
- QualType SrcRecordTy) {
+bool MicrosoftCXXABI::shouldTypeidBeNullChecked(QualType SrcRecordTy) {
const CXXRecordDecl *SrcDecl = SrcRecordTy->getAsCXXRecordDecl();
- return IsDeref &&
- !getContext().getASTRecordLayout(SrcDecl).hasExtendableVFPtr();
+ return !getContext().getASTRecordLayout(SrcDecl).hasExtendableVFPtr();
}
static llvm::CallBase *emitRTtypeidCall(CodeGenFunction &CGF,
diff --git a/clang/lib/Sema/SemaExceptionSpec.cpp b/clang/lib/Sema/SemaExceptionSpec.cpp
index 17acfca6b0112..67e0c7c63909e 100644
--- a/clang/lib/Sema/SemaExceptionSpec.cpp
+++ b/clang/lib/Sema/SemaExceptionSpec.cpp
@@ -1114,21 +1114,10 @@ static CanThrowResult canTypeidThrow(Sema &S, const CXXTypeidExpr *DC) {
if (DC->isTypeOperand())
return CT_Cannot;
- Expr *Op = DC->getExprOperand();
- if (Op->isTypeDependent())
+ if (DC->isValueDependent())
return CT_Dependent;
- const RecordType *RT = Op->getType()->getAs<RecordType>();
- if (!RT)
- return CT_Cannot;
-
- if (!cast<CXXRecordDecl>(RT->getDecl())->isPolymorphic())
- return CT_Cannot;
-
- if (Op->Classify(S.Context).isPRValue())
- return CT_Cannot;
-
- return CT_Can;
+ return DC->hasNullCheck() ? CT_Can : CT_Cannot;
}
CanThrowResult Sema::canThrow(const Stmt *S) {
@@ -1157,8 +1146,9 @@ CanThrowResult Sema::canThrow(const Stmt *S) {
}
case Expr::CXXTypeidExprClass:
- // - a potentially evaluated typeid expression applied to a glvalue
- // expression whose type is a polymorphic class type
+ // - a potentially evaluated typeid expression applied to a (possibly
+ // parenthesized) built-in unary * operator applied to a pointer to a
+ // polymorphic class type
return canTypeidThrow(*this, cast<CXXTypeidExpr>(S));
// - a potentially evaluated call to a function, member function, function
diff --git a/clang/test/CXX/drs/cwg21xx.cpp b/clang/test/CXX/drs/cwg21xx.cpp
index 082deb42e4fa0..d7bc52dd9d446 100644
--- a/clang/test/CXX/drs/cwg21xx.cpp
+++ b/clang/test/CXX/drs/cwg21xx.cpp
@@ -11,6 +11,10 @@
// cxx98-error at -1 {{variadic macros are a C99 feature}}
#endif
+namespace std {
+struct type_info;
+}
+
namespace cwg2100 { // cwg2100: 12
template<const int *P, bool = true> struct X {};
template<typename T> struct A {
@@ -231,6 +235,15 @@ static_assert(!__is_trivially_assignable(NonConstCopy &&, NonConstCopy &&), "");
#endif
} // namespace cwg2171
+namespace cwg2191 { // cwg2191: 19
+#if __cplusplus >= 201103L
+struct B { virtual void f() { } };
+struct D : B { } d;
+static_assert(noexcept(typeid(d)), "");
+static_assert(!noexcept(typeid(*static_cast<D*>(nullptr))), "");
+#endif
+} // namespace cwg2191
+
namespace cwg2180 { // cwg2180: yes
class A {
A &operator=(const A &); // #cwg2180-A-copy
diff --git a/clang/test/SemaCXX/warn-unused-value.cpp b/clang/test/SemaCXX/warn-unused-value.cpp
index d964684069155..2a07a0324f3f0 100644
--- a/clang/test/SemaCXX/warn-unused-value.cpp
+++ b/clang/test/SemaCXX/warn-unused-value.cpp
@@ -102,6 +102,16 @@ void f() {
Bad b;
(void)typeid(b.f()); // expected-warning {{expression with side effects will be evaluated despite being used as an operand to 'typeid'}}
+ extern Bad * pb;
+ // This typeid can throw but that is not a side-effect that we care about
+ // warning for since this is idiomatic code
+ (void)typeid(*pb);
+ (void)sizeof(typeid(*pb));
+ (void)typeid(*++pb); // expected-warning {{expression with side effects will be evaluated despite being used as an operand to 'typeid'}}
+ (void)sizeof(typeid(*++pb)); // expected-warning {{expression with side effects has no effect in an unevaluated context}}
+ // FIXME: we should not warn about this in an unevaluated context
+ // expected-warning at -2 {{expression with side effects will be evaluated despite being used as an operand to 'typeid'}}
+
// A dereference of a volatile pointer is a side effecting operation, however
// since it is idiomatic code, and the alternatives induce higher maintenance
// costs, it is allowed.
diff --git a/clang/www/cxx_dr_status.html b/clang/www/cxx_dr_status.html
index c00d022b86446..dac38cedfcb75 100755
--- a/clang/www/cxx_dr_status.html
+++ b/clang/www/cxx_dr_status.html
@@ -12954,7 +12954,7 @@ <h2 id="cxxdr">C++ defect report implementation status</h2>
<td><a href="https://cplusplus.github.io/CWG/issues/2191.html">2191</a></td>
<td>C++17</td>
<td>Incorrect result for <TT>noexcept(typeid(v))</TT></td>
- <td class="unknown" align="center">Unknown</td>
+ <td class="unreleased" align="center">Clang 19</td>
</tr>
<tr class="open" id="2192">
<td><a href="https://cplusplus.github.io/CWG/issues/2192.html">2192</a></td>
>From ba2b4497a358b8ba86673c6421860872ae70d59e Mon Sep 17 00:00:00 2001
From: Mital Ashok <mital at mitalashok.co.uk>
Date: Sun, 16 Jun 2024 22:38:26 +0100
Subject: [PATCH 2/2] Update clang/docs/ReleaseNotes.rst
Co-authored-by: Vlad Serebrennikov <serebrennikov.vladislav at gmail.com>
---
clang/docs/ReleaseNotes.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6c92177d71298..ff7d019651fa6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -268,7 +268,7 @@ Resolutions to C++ Defect Reports
- Clang now requires a template argument list after a template keyword.
(`CWG96: Syntactic disambiguation using the template keyword <https://cplusplus.github.io/CWG/issues/96.html>`_).
-- Clang no longer always reports ``!noexcept(typeid(expr))`` when the ``typeid`` cannot throw a ``std::bad_typeid``.
+- Clang now considers ``noexcept(typeid(expr))`` more carefully, instead of always assuming that ``std::bad_typeid`` can be thrown.
(`CWG2191: Incorrect result for noexcept(typeid(v)) <https://cplusplus.github.io/CWG/issues/2191.html>`_).
C Language Changes
More information about the cfe-commits
mailing list