Reapply "[Clang][Sema] Earlier type checking for builtin unary operators (#90500)" #92283
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: Krystian Stasiowski (sdkrystian) ChangesThis patch reapplies #90500, addressing a bug which caused binary operators with dependent operands to be incorrectly rebuilt by Patch is 46.60 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/92283.diff 17 Files Affected:
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 49ab222bec405..a2e44efe41347 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -55,6 +55,9 @@ C++ Specific Potentially Breaking Changes
- Clang now rejects pointer to member from parenthesized expression in unevaluated context such as ``decltype(&(foo::bar))``. (#GH40906).
+- Clang now performs semantic analysis for unary operators with dependent operands
+ that are known to be of non-class non-enumeration type prior to instantiation.
+
ABI Changes in This Version
---------------------------
- Fixed Microsoft name mangling of implicitly defined variables used for thread
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index e6643469e0b33..da3834f19ca04 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -8044,7 +8044,10 @@ inline bool Type::isUndeducedType() const {
/// Determines whether this is a type for which one can define
/// an overloaded operator.
inline bool Type::isOverloadableType() const {
- return isDependentType() || isRecordType() || isEnumeralType();
+ if (!CanonicalType->isDependentType())
+ return isRecordType() || isEnumeralType();
+ return !isArrayType() && !isFunctionType() && !isAnyPointerType() &&
+ !isMemberPointerType();
}
/// Determines whether this type is written as a typedef-name.
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ec84798e4ce60..50569c1cd5367 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -672,12 +672,12 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) {
// We don't want to throw lvalue-to-rvalue casts on top of
// expressions of certain types in C++.
- if (getLangOpts().CPlusPlus &&
- (E->getType() == Context.OverloadTy ||
- // FIXME: This is a hack! We want the lvalue-to-rvalue conversion applied
- // to pointer types even if the pointee type is dependent.
- (T->isDependentType() && !T->isPointerType()) || T->isRecordType()))
- return E;
+ if (getLangOpts().CPlusPlus) {
+ if (T == Context.OverloadTy || T->isRecordType() ||
+ (T->isDependentType() && !T->isAnyPointerType() &&
+ !T->isMemberPointerType()))
+ return E;
+ }
// The C standard is actually really unclear on this point, and
// DR106 tells us what the result should be but not why. It's
@@ -10827,7 +10827,7 @@ static bool checkArithmeticIncompletePointerType(Sema &S, SourceLocation Loc,
if (const AtomicType *ResAtomicType = ResType->getAs<AtomicType>())
ResType = ResAtomicType->getValueType();
- assert(ResType->isAnyPointerType() && !ResType->isDependentType());
+ assert(ResType->isAnyPointerType());
QualType PointeeTy = ResType->getPointeeType();
return S.RequireCompleteSizedType(
Loc, PointeeTy,
@@ -13957,9 +13957,6 @@ static QualType CheckIncrementDecrementOperand(Sema &S, Expr *Op,
ExprObjectKind &OK,
SourceLocation OpLoc, bool IsInc,
bool IsPrefix) {
- if (Op->isTypeDependent())
- return S.Context.DependentTy;
-
QualType ResType = Op->getType();
// Atomic types can be used for increment / decrement where the non-atomic
// versions can, so ignore the _Atomic() specifier for the purpose of
@@ -14410,9 +14407,6 @@ static void RecordModifiableNonNullParam(Sema &S, const Expr *Exp) {
static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
SourceLocation OpLoc,
bool IsAfterAmp = false) {
- if (Op->isTypeDependent())
- return S.Context.DependentTy;
-
ExprResult ConvResult = S.UsualUnaryConversions(Op);
if (ConvResult.isInvalid())
return QualType();
@@ -15368,14 +15362,10 @@ ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
}
if (getLangOpts().CPlusPlus) {
- // If either expression is type-dependent, always build an
- // overloaded op.
- if (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent())
- return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
-
- // Otherwise, build an overloaded op if either expression has an
- // overloadable type.
- if (LHSExpr->getType()->isOverloadableType() ||
+ // Otherwise, build an overloaded op if either expression is type-dependent
+ // or has an overloadable type.
+ if (LHSExpr->isTypeDependent() || RHSExpr->isTypeDependent() ||
+ LHSExpr->getType()->isOverloadableType() ||
RHSExpr->getType()->isOverloadableType())
return BuildOverloadedBinOp(*this, S, OpLoc, Opc, LHSExpr, RHSExpr);
}
@@ -15466,190 +15456,191 @@ ExprResult Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
return ExprError(Diag(OpLoc, diag::err_hlsl_operator_unsupported) << 1);
}
- switch (Opc) {
- case UO_PreInc:
- case UO_PreDec:
- case UO_PostInc:
- case UO_PostDec:
- resultType =
- CheckIncrementDecrementOperand(*this, Input.get(), VK, OK, OpLoc,
- Opc == UO_PreInc || Opc == UO_PostInc,
- Opc == UO_PreInc || Opc == UO_PreDec);
- CanOverflow = isOverflowingIntegerType(Context, resultType);
- break;
- case UO_AddrOf:
- resultType = CheckAddressOfOperand(Input, OpLoc);
- CheckAddressOfNoDeref(InputExpr);
- RecordModifiableNonNullParam(*this, InputExpr);
- break;
- case UO_Deref: {
- Input = DefaultFunctionArrayLvalueConversion(Input.get());
- if (Input.isInvalid())
- return ExprError();
- resultType =
- CheckIndirectionOperand(*this, Input.get(), VK, OpLoc, IsAfterAmp);
- break;
- }
- case UO_Plus:
- case UO_Minus:
- CanOverflow = Opc == UO_Minus &&
- isOverflowingIntegerType(Context, Input.get()->getType());
- Input = UsualUnaryConversions(Input.get());
- if (Input.isInvalid())
- return ExprError();
- // Unary plus and minus require promoting an operand of half vector to a
- // float vector and truncating the result back to a half vector. For now, we
- // do this only when HalfArgsAndReturns is set (that is, when the target is
- // arm or arm64).
- ConvertHalfVec = needsConversionOfHalfVec(true, Context, Input.get());
-
- // If the operand is a half vector, promote it to a float vector.
- if (ConvertHalfVec)
- Input = convertVector(Input.get(), Context.FloatTy, *this);
- resultType = Input.get()->getType();
- if (resultType->isDependentType())
- break;
- if (resultType->isArithmeticType()) // C99 6.5.3.3p1
- break;
- else if (resultType->isVectorType() &&
- // The z vector extensions don't allow + or - with bool vectors.
- (!Context.getLangOpts().ZVector ||
- resultType->castAs<VectorType>()->getVectorKind() !=
- VectorKind::AltiVecBool))
- break;
- else if (resultType->isSveVLSBuiltinType()) // SVE vectors allow + and -
- break;
- else if (getLangOpts().CPlusPlus && // C++ [expr.unary.op]p6
- Opc == UO_Plus && resultType->isPointerType())
+ if (InputExpr->isTypeDependent() &&
+ InputExpr->getType()->isSpecificBuiltinType(BuiltinType::Dependent)) {
+ resultType = Context.DependentTy;
+ } else {
+ switch (Opc) {
+ case UO_PreInc:
+ case UO_PreDec:
+ case UO_PostInc:
+ case UO_PostDec:
+ resultType =
+ CheckIncrementDecrementOperand(*this, Input.get(), VK, OK, OpLoc,
+ Opc == UO_PreInc || Opc == UO_PostInc,
+ Opc == UO_PreInc || Opc == UO_PreDec);
+ CanOverflow = isOverflowingIntegerType(Context, resultType);
break;
-
- return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
- << resultType << Input.get()->getSourceRange());
-
- case UO_Not: // bitwise complement
- Input = UsualUnaryConversions(Input.get());
- if (Input.isInvalid())
- return ExprError();
- resultType = Input.get()->getType();
- if (resultType->isDependentType())
+ case UO_AddrOf:
+ resultType = CheckAddressOfOperand(Input, OpLoc);
+ CheckAddressOfNoDeref(InputExpr);
+ RecordModifiableNonNullParam(*this, InputExpr);
break;
- // C99 6.5.3.3p1. We allow complex int and float as a GCC extension.
- if (resultType->isComplexType() || resultType->isComplexIntegerType())
- // C99 does not support '~' for complex conjugation.
- Diag(OpLoc, diag::ext_integer_complement_complex)
- << resultType << Input.get()->getSourceRange();
- else if (resultType->hasIntegerRepresentation())
+ case UO_Deref: {
+ Input = DefaultFunctionArrayLvalueConversion(Input.get());
+ if (Input.isInvalid())
+ return ExprError();
+ resultType =
+ CheckIndirectionOperand(*this, Input.get(), VK, OpLoc, IsAfterAmp);
break;
- else if (resultType->isExtVectorType() && Context.getLangOpts().OpenCL) {
- // OpenCL v1.1 s6.3.f: The bitwise operator not (~) does not operate
- // on vector float types.
- QualType T = resultType->castAs<ExtVectorType>()->getElementType();
- if (!T->isIntegerType())
- return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
- << resultType << Input.get()->getSourceRange());
- } else {
- return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
- << resultType << Input.get()->getSourceRange());
- }
- break;
-
- case UO_LNot: // logical negation
- // Unlike +/-/~, integer promotions aren't done here (C99 6.5.3.3p5).
- Input = DefaultFunctionArrayLvalueConversion(Input.get());
- if (Input.isInvalid())
- return ExprError();
- resultType = Input.get()->getType();
-
- // Though we still have to promote half FP to float...
- if (resultType->isHalfType() && !Context.getLangOpts().NativeHalfType) {
- Input = ImpCastExprToType(Input.get(), Context.FloatTy, CK_FloatingCast)
- .get();
- resultType = Context.FloatTy;
}
+ case UO_Plus:
+ case UO_Minus:
+ CanOverflow = Opc == UO_Minus &&
+ isOverflowingIntegerType(Context, Input.get()->getType());
+ Input = UsualUnaryConversions(Input.get());
+ if (Input.isInvalid())
+ return ExprError();
+ // Unary plus and minus require promoting an operand of half vector to a
+ // float vector and truncating the result back to a half vector. For now,
+ // we do this only when HalfArgsAndReturns is set (that is, when the
+ // target is arm or arm64).
+ ConvertHalfVec = needsConversionOfHalfVec(true, Context, Input.get());
+
+ // If the operand is a half vector, promote it to a float vector.
+ if (ConvertHalfVec)
+ Input = convertVector(Input.get(), Context.FloatTy, *this);
+ resultType = Input.get()->getType();
+ if (resultType->isArithmeticType()) // C99 6.5.3.3p1
+ break;
+ else if (resultType->isVectorType() &&
+ // The z vector extensions don't allow + or - with bool vectors.
+ (!Context.getLangOpts().ZVector ||
+ resultType->castAs<VectorType>()->getVectorKind() !=
+ VectorKind::AltiVecBool))
+ break;
+ else if (resultType->isSveVLSBuiltinType()) // SVE vectors allow + and -
+ break;
+ else if (getLangOpts().CPlusPlus && // C++ [expr.unary.op]p6
+ Opc == UO_Plus && resultType->isPointerType())
+ break;
- // WebAsembly tables can't be used in unary expressions.
- if (resultType->isPointerType() &&
- resultType->getPointeeType().isWebAssemblyReferenceType()) {
return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
<< resultType << Input.get()->getSourceRange());
- }
- if (resultType->isDependentType())
- break;
- if (resultType->isScalarType() && !isScopedEnumerationType(resultType)) {
- // C99 6.5.3.3p1: ok, fallthrough;
- if (Context.getLangOpts().CPlusPlus) {
- // C++03 [expr.unary.op]p8, C++0x [expr.unary.op]p9:
- // operand contextually converted to bool.
- Input = ImpCastExprToType(Input.get(), Context.BoolTy,
- ScalarTypeToBooleanCastKind(resultType));
- } else if (Context.getLangOpts().OpenCL &&
- Context.getLangOpts().OpenCLVersion < 120) {
- // OpenCL v1.1 6.3.h: The logical operator not (!) does not
- // operate on scalar float types.
- if (!resultType->isIntegerType() && !resultType->isPointerType())
- return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
- << resultType << Input.get()->getSourceRange());
- }
- } else if (resultType->isExtVectorType()) {
- if (Context.getLangOpts().OpenCL &&
- Context.getLangOpts().getOpenCLCompatibleVersion() < 120) {
- // OpenCL v1.1 6.3.h: The logical operator not (!) does not
- // operate on vector float types.
+ case UO_Not: // bitwise complement
+ Input = UsualUnaryConversions(Input.get());
+ if (Input.isInvalid())
+ return ExprError();
+ resultType = Input.get()->getType();
+ // C99 6.5.3.3p1. We allow complex int and float as a GCC extension.
+ if (resultType->isComplexType() || resultType->isComplexIntegerType())
+ // C99 does not support '~' for complex conjugation.
+ Diag(OpLoc, diag::ext_integer_complement_complex)
+ << resultType << Input.get()->getSourceRange();
+ else if (resultType->hasIntegerRepresentation())
+ break;
+ else if (resultType->isExtVectorType() && Context.getLangOpts().OpenCL) {
+ // OpenCL v1.1 s6.3.f: The bitwise operator not (~) does not operate
+ // on vector float types.
QualType T = resultType->castAs<ExtVectorType>()->getElementType();
if (!T->isIntegerType())
return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
<< resultType << Input.get()->getSourceRange());
+ } else {
+ return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
+ << resultType << Input.get()->getSourceRange());
}
- // Vector logical not returns the signed variant of the operand type.
- resultType = GetSignedVectorType(resultType);
break;
- } else if (Context.getLangOpts().CPlusPlus && resultType->isVectorType()) {
- const VectorType *VTy = resultType->castAs<VectorType>();
- if (VTy->getVectorKind() != VectorKind::Generic)
+
+ case UO_LNot: // logical negation
+ // Unlike +/-/~, integer promotions aren't done here (C99 6.5.3.3p5).
+ Input = DefaultFunctionArrayLvalueConversion(Input.get());
+ if (Input.isInvalid())
+ return ExprError();
+ resultType = Input.get()->getType();
+
+ // Though we still have to promote half FP to float...
+ if (resultType->isHalfType() && !Context.getLangOpts().NativeHalfType) {
+ Input = ImpCastExprToType(Input.get(), Context.FloatTy, CK_FloatingCast)
+ .get();
+ resultType = Context.FloatTy;
+ }
+
+ // WebAsembly tables can't be used in unary expressions.
+ if (resultType->isPointerType() &&
+ resultType->getPointeeType().isWebAssemblyReferenceType()) {
return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
<< resultType << Input.get()->getSourceRange());
+ }
- // Vector logical not returns the signed variant of the operand type.
- resultType = GetSignedVectorType(resultType);
- break;
- } else {
- return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
- << resultType << Input.get()->getSourceRange());
- }
+ if (resultType->isScalarType() && !isScopedEnumerationType(resultType)) {
+ // C99 6.5.3.3p1: ok, fallthrough;
+ if (Context.getLangOpts().CPlusPlus) {
+ // C++03 [expr.unary.op]p8, C++0x [expr.unary.op]p9:
+ // operand contextually converted to bool.
+ Input = ImpCastExprToType(Input.get(), Context.BoolTy,
+ ScalarTypeToBooleanCastKind(resultType));
+ } else if (Context.getLangOpts().OpenCL &&
+ Context.getLangOpts().OpenCLVersion < 120) {
+ // OpenCL v1.1 6.3.h: The logical operator not (!) does not
+ // operate on scalar float types.
+ if (!resultType->isIntegerType() && !resultType->isPointerType())
+ return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
+ << resultType << Input.get()->getSourceRange());
+ }
+ } else if (resultType->isExtVectorType()) {
+ if (Context.getLangOpts().OpenCL &&
+ Context.getLangOpts().getOpenCLCompatibleVersion() < 120) {
+ // OpenCL v1.1 6.3.h: The logical operator not (!) does not
+ // operate on vector float types.
+ QualType T = resultType->castAs<ExtVectorType>()->getElementType();
+ if (!T->isIntegerType())
+ return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
+ << resultType << Input.get()->getSourceRange());
+ }
+ // Vector logical not returns the signed variant of the operand type.
+ resultType = GetSignedVectorType(resultType);
+ break;
+ } else if (Context.getLangOpts().CPlusPlus &&
+ resultType->isVectorType()) {
+ const VectorType *VTy = resultType->castAs<VectorType>();
+ if (VTy->getVectorKind() != VectorKind::Generic)
+ return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
+ << resultType << Input.get()->getSourceRange());
- // LNot always has type int. C99 6.5.3.3p5.
- // In C++, it's bool. C++ 5.3.1p8
- resultType = Context.getLogicalOperationType();
- break;
- case UO_Real:
- case UO_Imag:
- resultType = CheckRealImagOperand(*this, Input, OpLoc, Opc == UO_Real);
- // _Real maps ordinary l-values into ordinary l-values. _Imag maps ordinary
- // complex l-values to ordinary l-values and all other values to r-values.
- if (Input.isInvalid())
- return ExprError();
- if (Opc == UO_Real || Input.get()->getType()->isAnyComplexType()) {
- if (Input.get()->isGLValue() &&
- Input.get()->getObjectKind() == OK_Ordinary)
- VK = Input.get()->getValueKind();
- } else if (!getLangOpts().CPlusPlus) {
- // In C, a volatile scalar is read by __imag. In C++, it is not.
- Input = DefaultLvalueConversion(Input.get());
+ // Vector logical not returns the signed variant of the operand type.
+ resultType = GetSignedVectorType(resultType);
+ break;
+ } else {
+ return ExprError(Diag(OpLoc, diag::err_typecheck_unary_expr)
+ << resultType << Input.get()->getSourceRange());
+ }
+
+ // LNot always has type int. C99 6.5.3.3p5.
+ // In C++, it's bool. C++ 5.3.1p8
+ resultType = Context.getLogicalOperationType();
+ break;
+ case UO_Real:
+ case UO_Imag:
+ resultType = CheckRealImagOperand(*this, Input, OpLoc, Opc == UO_Real);
+ // _Real maps ordinary l-values into ordinary l-values. _Imag maps
+ // ordinary complex l-values to ordinary l-values and all other values to
+ // r-values.
+ if (Input.isInvalid())
+ return ExprError();
+ if (Opc == UO_Real || Input.get()->getType()->isAnyComplexType()) {
+ if (Input.get()->isGLValue() &&
+ Input.get()->getObjectKind() == OK_Ordinary)
+ VK = Input.get()->getValueKind();
+ } else if (!getLangOpts().CPlusPlus) {
+ // In C, a volatile scalar is read by __imag. In C++, it is not.
+ Input = DefaultLvalueConversion(Input.get());
+ }
+ break;
+ case UO_Extension:
+ resultType = Input.get()->getType();
+ VK = Input.get()->getValueKind();
+ OK = Input.get()->getObjectKind();
+ break;
+ case UO_Coawait:
+ // It's unnecessary to represent the pass-through operator co_await in the
+ // AST; j...
[truncated]
|
|
Can you point out the 'diff' inline what the change from the previous commit is? |
|
All the new changes are in 5ce0e96 |
erichkeane
approved these changes
May 15, 2024
|
Hi @sdkrystian - this appears to break in a circumstance where a custom iterator class defines the postfix operator: I went ahead and attached a reproducer. Could you please revert this change and investigate? |
|
@Caslyn [expr.prim.this] p3 states:
And [over.match.oper] p1 states:
Since the type of |
If the iterator is trying to call its own increment operator, that probably should be At the moment it is trying to do something that I'd be shocked if it isn't UB, which is incrementing 'this' itself, which is a pointer. |
|
Hi @sdkrystian & @erichkeane - my apologies! Yes, indeed this code calls its own increment operator and it should be |
|
See also: #92439 |
ahatanaka
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jul 18, 2024
…y operators (llvm#90500)" (llvm#92283)" This reverts commit 1595988. rdar://131028403 Conflicts: clang/docs/ReleaseNotes.rst
ahatanaka
added a commit
to swiftlang/llvm-project
that referenced
this pull request
Jul 18, 2024
…y operators (llvm#90500)" (llvm#92283)" This reverts commit 1595988. rdar://131028403 Conflicts: clang/docs/ReleaseNotes.rst
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 patch reapplies #90500, addressing a bug which caused binary operators with dependent operands to be incorrectly rebuilt by
TreeTransform.