[Clang] Introduce CXXTypeidExpr::hasNullCheck
#95718
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Mital Ashok (MitalAshok) ChangesUsed to implement CWG2191 where Also change Remove Full diff: https://github.com/llvm/llvm-project/pull/95718.diff 11 Files Affected:
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..a2bdd7d0aa7b6 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 Windows ABI, if this is a call to __RTtypeid,
+ // __RTtypeid(nullptr) is allowed will throw the std::bad_typeid
+ 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@-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..a2bf5148a4e9b 100644
--- a/clang/test/SemaCXX/warn-unused-value.cpp
+++ b/clang/test/SemaCXX/warn-unused-value.cpp
@@ -102,6 +102,14 @@ 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}}
+
// 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.
|
|
You need to run |
|
If you update the branch, you should be able to get rid of unrelated changes to |
Endilll
approved these changes
Jun 16, 2024
MitalAshok
force-pushed
the
noexcept-typeid
branch
from
0b7d2e2 to
c391261
Compare
Co-authored-by: Vlad Serebrennikov <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Used to implement CWG2191 where
typeidfor a polymorphic glvalue only becomes potentially-throwing if thetypeidoperand was already potentially throwing or anullptrcheck was inserted: https://cplusplus.github.io/CWG/issues/2191.htmlAlso change
Expr::hasSideEffectsforCXXTypeidExprto check the operand for side-effects instead of always reporting that there are side-effectsRemove
IsDerefparameter ofCGCXXABI::shouldTypeidBeNullCheckedbecause it should never returntrueif!IsDeref(we shouldn't add a null check that wasn't there in the first place)