[Clang] Back out the source location workaround for CXXConstructExpr #145260
+124
−86
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 Author: Younan Zhang (zyn0217) ChangesThis removes the workaround introduced in 3e1a9cf and 1ba7dc3. The workaround overwrote the right parenthesis location of the sub expression, which could be wrong when a CXXTemporaryObjectExpr occurs within a nested expression, e.g. To completely take it down, we now propagate the left parenthesis source location throughout SemaCast, such that the ParenOrBraceRange can be properly set at the point of its creation. Fixes #143711 Patch is 21.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/145260.diff 4 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 96477ef6ddc9a..7822a9a6c3cc6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -877,6 +877,7 @@ Bug Fixes to AST Handling
- Fixed a malformed printout of ``CXXParenListInitExpr`` in certain contexts.
- Fixed a malformed printout of certain calling convention function attributes. (#GH143160)
- Fixed dependency calculation for TypedefTypes (#GH89774)
+- Fixed the right parenthesis source location of ``CXXTemporaryObjectExpr``. (#GH143711)
Miscellaneous Bug Fixes
^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Initialization.h b/clang/include/clang/Sema/Initialization.h
index 0455e1fa5016b..a1c156eed5394 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -676,11 +676,12 @@ class InitializationKind {
}
/// Create a direct initialization for a functional cast.
- static InitializationKind CreateFunctionalCast(SourceRange TypeRange,
+ static InitializationKind CreateFunctionalCast(SourceLocation StartLoc,
+ SourceRange ParenRange,
bool InitList) {
return InitializationKind(InitList ? IK_DirectList : IK_Direct,
- IC_FunctionalCast, TypeRange.getBegin(),
- TypeRange.getBegin(), TypeRange.getEnd());
+ IC_FunctionalCast, StartLoc,
+ ParenRange.getBegin(), ParenRange.getEnd());
}
/// Create a copy initialization.
diff --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index e15a43c116516..c092902c688ad 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -98,7 +98,36 @@ namespace {
CXXCastPath BasePath;
bool IsARCUnbridgedCast;
- SourceRange OpRange;
+ struct OpRangeType {
+ SourceLocation Ranges[3];
+
+ OpRangeType(SourceLocation Begin, SourceLocation LParen,
+ SourceLocation RParen)
+ : Ranges{Begin, LParen, RParen} {}
+
+ OpRangeType() = default;
+
+ SourceLocation getBegin() const { return Ranges[0]; }
+
+ SourceLocation getLParenLoc() const { return Ranges[1]; }
+
+ SourceLocation getRParenLoc() const { return Ranges[2]; }
+
+ friend const StreamingDiagnostic &
+ operator<<(const StreamingDiagnostic &DB, OpRangeType Op) {
+ return DB << SourceRange(Op);
+ }
+
+ SourceRange getParenRange() const {
+ return SourceRange(getLParenLoc(), getRParenLoc());
+ }
+
+ operator SourceRange() const {
+ return SourceRange(getBegin(), getRParenLoc());
+ }
+ };
+
+ OpRangeType OpRange;
SourceRange DestRange;
// Top-level semantics-checking routines.
@@ -237,49 +266,46 @@ static TryCastResult TryLValueToRValueCast(Sema &Self, Expr *SrcExpr,
CastKind &Kind,
CXXCastPath &BasePath,
unsigned &msg);
-static TryCastResult TryStaticReferenceDowncast(Sema &Self, Expr *SrcExpr,
- QualType DestType, bool CStyle,
- SourceRange OpRange,
- unsigned &msg,
- CastKind &Kind,
- CXXCastPath &BasePath);
-static TryCastResult TryStaticPointerDowncast(Sema &Self, QualType SrcType,
- QualType DestType, bool CStyle,
- SourceRange OpRange,
- unsigned &msg,
- CastKind &Kind,
- CXXCastPath &BasePath);
+static TryCastResult
+TryStaticReferenceDowncast(Sema &Self, Expr *SrcExpr, QualType DestType,
+ bool CStyle, CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath);
+static TryCastResult
+TryStaticPointerDowncast(Sema &Self, QualType SrcType, QualType DestType,
+ bool CStyle, CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind, CXXCastPath &BasePath);
static TryCastResult TryStaticDowncast(Sema &Self, CanQualType SrcType,
CanQualType DestType, bool CStyle,
- SourceRange OpRange,
+ CastOperation::OpRangeType OpRange,
QualType OrigSrcType,
QualType OrigDestType, unsigned &msg,
- CastKind &Kind,
- CXXCastPath &BasePath);
-static TryCastResult TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr,
- QualType SrcType,
- QualType DestType,bool CStyle,
- SourceRange OpRange,
- unsigned &msg,
- CastKind &Kind,
- CXXCastPath &BasePath);
-
+ CastKind &Kind, CXXCastPath &BasePath);
static TryCastResult
-TryStaticImplicitCast(Sema &Self, ExprResult &SrcExpr, QualType DestType,
- CheckedConversionKind CCK, SourceRange OpRange,
- unsigned &msg, CastKind &Kind, bool ListInitialization);
+TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
+ QualType DestType, bool CStyle,
+ CastOperation::OpRangeType OpRange, unsigned &msg,
+ CastKind &Kind, CXXCastPath &BasePath);
+
+static TryCastResult TryStaticImplicitCast(Sema &Self, ExprResult &SrcExpr,
+ QualType DestType,
+ CheckedConversionKind CCK,
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ bool ListInitialization);
static TryCastResult TryStaticCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, CheckedConversionKind CCK,
- SourceRange OpRange, unsigned &msg,
- CastKind &Kind, CXXCastPath &BasePath,
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath,
bool ListInitialization);
static TryCastResult TryConstCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, bool CStyle,
unsigned &msg);
static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, bool CStyle,
- SourceRange OpRange, unsigned &msg,
- CastKind &Kind);
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind);
static TryCastResult TryAddressSpaceCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, bool CStyle,
unsigned &msg, CastKind &Kind);
@@ -319,7 +345,8 @@ Sema::BuildCXXNamedCast(SourceLocation OpLoc, tok::TokenKind Kind,
DestType->isDependentType() || Ex.get()->isTypeDependent();
CastOperation Op(*this, DestType, E);
- Op.OpRange = SourceRange(OpLoc, Parens.getEnd());
+ Op.OpRange =
+ CastOperation::OpRangeType(OpLoc, Parens.getBegin(), Parens.getEnd());
Op.DestRange = AngleBrackets;
Op.checkQualifiedDestType();
@@ -412,7 +439,7 @@ ExprResult Sema::BuildBuiltinBitCastExpr(SourceLocation KWLoc,
TypeSourceInfo *TSI, Expr *Operand,
SourceLocation RParenLoc) {
CastOperation Op(*this, TSI->getType(), Operand);
- Op.OpRange = SourceRange(KWLoc, RParenLoc);
+ Op.OpRange = CastOperation::OpRangeType(KWLoc, KWLoc, RParenLoc);
TypeLoc TL = TSI->getTypeLoc();
Op.DestRange = SourceRange(TL.getBeginLoc(), TL.getEndLoc());
@@ -431,8 +458,8 @@ ExprResult Sema::BuildBuiltinBitCastExpr(SourceLocation KWLoc,
/// Try to diagnose a failed overloaded cast. Returns true if
/// diagnostics were emitted.
static bool tryDiagnoseOverloadedCast(Sema &S, CastType CT,
- SourceRange range, Expr *src,
- QualType destType,
+ CastOperation::OpRangeType range,
+ Expr *src, QualType destType,
bool listInitialization) {
switch (CT) {
// These cast kinds don't consider user-defined conversions.
@@ -454,12 +481,13 @@ static bool tryDiagnoseOverloadedCast(Sema &S, CastType CT,
return false;
InitializedEntity entity = InitializedEntity::InitializeTemporary(destType);
- InitializationKind initKind
- = (CT == CT_CStyle)? InitializationKind::CreateCStyleCast(range.getBegin(),
- range, listInitialization)
- : (CT == CT_Functional)? InitializationKind::CreateFunctionalCast(range,
- listInitialization)
- : InitializationKind::CreateCast(/*type range?*/ range);
+ InitializationKind initKind =
+ (CT == CT_CStyle) ? InitializationKind::CreateCStyleCast(
+ range.getBegin(), range, listInitialization)
+ : (CT == CT_Functional)
+ ? InitializationKind::CreateFunctionalCast(
+ range.getBegin(), range.getParenRange(), listInitialization)
+ : InitializationKind::CreateCast(/*type range?*/ range);
InitializationSequence sequence(S, entity, initKind, src);
// It could happen that a constructor failed to be used because
@@ -544,8 +572,8 @@ static bool tryDiagnoseOverloadedCast(Sema &S, CastType CT,
/// Diagnose a failed cast.
static void diagnoseBadCast(Sema &S, unsigned msg, CastType castType,
- SourceRange opRange, Expr *src, QualType destType,
- bool listInitialization) {
+ CastOperation::OpRangeType opRange, Expr *src,
+ QualType destType, bool listInitialization) {
if (msg == diag::err_bad_cxx_cast_generic &&
tryDiagnoseOverloadedCast(S, castType, opRange, src, destType,
listInitialization))
@@ -1018,7 +1046,7 @@ void CastOperation::CheckAddrspaceCast() {
/// or downcast between respective pointers or references.
static void DiagnoseReinterpretUpDownCast(Sema &Self, const Expr *SrcExpr,
QualType DestType,
- SourceRange OpRange) {
+ CastOperation::OpRangeType OpRange) {
QualType SrcType = SrcExpr->getType();
// When casting from pointer or reference, get pointee type; use original
// type otherwise.
@@ -1361,8 +1389,9 @@ static bool IsAddressSpaceConversion(QualType SrcType, QualType DestType) {
/// and casting away constness.
static TryCastResult TryStaticCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, CheckedConversionKind CCK,
- SourceRange OpRange, unsigned &msg,
- CastKind &Kind, CXXCastPath &BasePath,
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath,
bool ListInitialization) {
// Determine whether we have the semantics of a C-style cast.
bool CStyle = (CCK == CheckedConversionKind::CStyleCast ||
@@ -1625,11 +1654,11 @@ TryCastResult TryLValueToRValueCast(Sema &Self, Expr *SrcExpr,
}
/// Tests whether a conversion according to C++ 5.2.9p5 is valid.
-TryCastResult
-TryStaticReferenceDowncast(Sema &Self, Expr *SrcExpr, QualType DestType,
- bool CStyle, SourceRange OpRange,
- unsigned &msg, CastKind &Kind,
- CXXCastPath &BasePath) {
+TryCastResult TryStaticReferenceDowncast(Sema &Self, Expr *SrcExpr,
+ QualType DestType, bool CStyle,
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath) {
// C++ 5.2.9p5: An lvalue of type "cv1 B", where B is a class type, can be
// cast to type "reference to cv2 D", where D is a class derived from B,
// if a valid standard conversion from "pointer to D" to "pointer to B"
@@ -1663,11 +1692,11 @@ TryStaticReferenceDowncast(Sema &Self, Expr *SrcExpr, QualType DestType,
}
/// Tests whether a conversion according to C++ 5.2.9p8 is valid.
-TryCastResult
-TryStaticPointerDowncast(Sema &Self, QualType SrcType, QualType DestType,
- bool CStyle, SourceRange OpRange,
- unsigned &msg, CastKind &Kind,
- CXXCastPath &BasePath) {
+TryCastResult TryStaticPointerDowncast(Sema &Self, QualType SrcType,
+ QualType DestType, bool CStyle,
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath) {
// C++ 5.2.9p8: An rvalue of type "pointer to cv1 B", where B is a class
// type, can be converted to an rvalue of type "pointer to cv2 D", where D
// is a class derived from B, if a valid standard conversion from "pointer
@@ -1697,11 +1726,12 @@ TryStaticPointerDowncast(Sema &Self, QualType SrcType, QualType DestType,
/// TryStaticDowncast - Common functionality of TryStaticReferenceDowncast and
/// TryStaticPointerDowncast. Tests whether a static downcast from SrcType to
/// DestType is possible and allowed.
-TryCastResult
-TryStaticDowncast(Sema &Self, CanQualType SrcType, CanQualType DestType,
- bool CStyle, SourceRange OpRange, QualType OrigSrcType,
- QualType OrigDestType, unsigned &msg,
- CastKind &Kind, CXXCastPath &BasePath) {
+TryCastResult TryStaticDowncast(Sema &Self, CanQualType SrcType,
+ CanQualType DestType, bool CStyle,
+ CastOperation::OpRangeType OpRange,
+ QualType OrigSrcType, QualType OrigDestType,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath) {
// We can only work with complete types. But don't complain if it doesn't work
if (!Self.isCompleteType(OpRange.getBegin(), SrcType) ||
!Self.isCompleteType(OpRange.getBegin(), DestType))
@@ -1810,12 +1840,12 @@ TryStaticDowncast(Sema &Self, CanQualType SrcType, CanQualType DestType,
/// converted to an rvalue of type "pointer to member of B of type cv2 T",
/// where B is a base class of D [...].
///
-TryCastResult
-TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
- QualType DestType, bool CStyle,
- SourceRange OpRange,
- unsigned &msg, CastKind &Kind,
- CXXCastPath &BasePath) {
+TryCastResult TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr,
+ QualType SrcType, QualType DestType,
+ bool CStyle,
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ CXXCastPath &BasePath) {
const MemberPointerType *DestMemPtr = DestType->getAs<MemberPointerType>();
if (!DestMemPtr)
return TC_NotApplicable;
@@ -1881,8 +1911,9 @@ TryStaticMemberPointerUpcast(Sema &Self, ExprResult &SrcExpr, QualType SrcType,
TryCastResult TryStaticImplicitCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType,
CheckedConversionKind CCK,
- SourceRange OpRange, unsigned &msg,
- CastKind &Kind, bool ListInitialization) {
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind,
+ bool ListInitialization) {
if (DestType->isRecordType()) {
if (Self.RequireCompleteType(OpRange.getBegin(), DestType,
diag::err_bad_cast_incomplete) ||
@@ -1899,8 +1930,8 @@ TryCastResult TryStaticImplicitCast(Sema &Self, ExprResult &SrcExpr,
? InitializationKind::CreateCStyleCast(OpRange.getBegin(), OpRange,
ListInitialization)
: (CCK == CheckedConversionKind::FunctionalCast)
- ? InitializationKind::CreateFunctionalCast(OpRange,
- ListInitialization)
+ ? InitializationKind::CreateFunctionalCast(
+ OpRange.getBegin(), OpRange.getParenRange(), ListInitialization)
: InitializationKind::CreateCast(OpRange);
Expr *SrcExprRaw = SrcExpr.get();
// FIXME: Per DR242, we should check for an implicit conversion sequence
@@ -2112,7 +2143,8 @@ static void DiagnoseCastOfObjCSEL(Sema &Self, const ExprResult &SrcExpr,
/// Diagnose casts that change the calling convention of a pointer to a function
/// defined in the current TU.
static void DiagnoseCallingConvCast(Sema &Self, const ExprResult &SrcExpr,
- QualType DstType, SourceRange OpRange) {
+ QualType DstType,
+ CastOperation::OpRangeType OpRange) {
// Check if this cast would change the calling convention of a function
// pointer type.
QualType SrcType = SrcExpr.get()->getType();
@@ -2255,9 +2287,8 @@ static bool fixOverloadedReinterpretCastExpr(Sema &Self, QualType DestType,
static TryCastResult TryReinterpretCast(Sema &Self, ExprResult &SrcExpr,
QualType DestType, bool CStyle,
- SourceRange OpRange,
- unsigned &msg,
- CastKind &Kind) {
+ CastOperation::OpRangeType OpRange,
+ unsigned &msg, CastKind &Kind) {
bool IsLValueCast = false;
DestType = Self.Context.getCanonicalType(DestType);
@@ -3413,7 +3444,8 @@ ExprResult Sema::BuildCStyleCastExpr(SourceLocation LPLoc,
Expr *CastExpr) {
CastOperation Op(*this, CastTypeInfo->getType(), CastExpr);
Op.DestRange = CastTypeInfo->getTypeLoc().getSourceRange();
- Op.OpRange = SourceRange(LPLoc, CastExpr->getEndLoc());
+ Op.OpRange = CastOperation::OpRangeType(Op.DestRange.getBegin(), LPLoc,
+ CastExpr->getEndLoc());
if (getLangOpts().CPlusPlus) {
Op.CheckCXXCStyleCast(/*FunctionalCast=*/ fa...
[truncated]
|
This removes the workaround introduced in 3e1a9cf and 1ba7dc3. The issues should have been already resolved elsewhere, at least removing these lines doesn't break any existing tests. The workaround overwrote the right parenthesis location of the sub expression, which could be wrong when a CXXTemporaryObjectExpr occurs within a nested expression, e.g. `A(A(1, 2))`.
erichkeane
approved these changes
Jun 24, 2025
| @@ -98,7 +98,36 @@ namespace { | |||
| CXXCastPath BasePath; | |||
| bool IsARCUnbridgedCast; | |||
|
|
|||
| SourceRange OpRange; | |||
| struct OpRangeType { | |||
| SourceLocation Ranges[3]; | |||
There was a problem hiding this comment.
This is a strange variable name here... Ranges for a series of locations?
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.
This removes the workaround introduced in 3e1a9cf and 1ba7dc3.
The workaround overwrote the right parenthesis location of the sub expression, which could be wrong when a CXXTemporaryObjectExpr occurs within a nested expression, e.g.
A(A(1, 2)).To completely take it down, we now propagate the left parenthesis source location throughout SemaCast, such that the ParenOrBraceRange can be properly set at the point of its creation.
Fixes #143711