[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: The issue seems to be that `TreeTransform::RebuildCXXOperatorCallExpr` relies on `isOverloadableType` to always be true for dependent types https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
erichkeane wrote: > Well this took forever to reduce: > > ```c++ > template > struct A; > > template > bool operator==(const A&, const A&); > > template > void f(int *x) > { > [&](auto *y) { return x == y; }; > } > > void g() > { > f(nullptr); > } > ``` > > We initially build a `CXXOperatorCallExpr` during parsing for the comparison, > but `TreeTransform` prematurely rebuilds it as a builtin binary operator when > `f` is instantiated. Yeah, unfortunately that is a limitation of Clang, we still don't correctly implement the instantiation of Lambdas. We do so 'greedily', in a way that the standard doesn't allow. We eventually need to delay ALL instantiation until the actual first full instantiation of the lambda. That said, in this case I'm surprised that we don't skip that diagnostic based on the RHS (in your repro) being dependent. THAT would probably fix this case at least. https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: Well this took forever to reduce: ```cpp template struct A; template bool operator==(const A&, const A&); template void f(int *x) { [&](auto *y) { return x == y; }; } void g() { f(nullptr); } ``` We initially build a `CXXOperatorCallExpr` during parsing for the comparison, but `TreeTransform` prematurely rebuilds it as a builtin binary operator when `f` is instantiated. https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
MaskRay wrote: > This seems to [break > something](https://lab.llvm.org/buildbot/#/builders/121/builds/41631) in > LLVM... investigating I've also noticed a stage-2-build issue. ``` llvm::any_of(LHS->users(), [&](auto *U) { return U != I && !(U->hasOneUser() && *U->users().begin() == I); }) || ``` ``` llvm/lib/Transforms/Scalar/NaryReassociate.cpp:610:32: error: comparison of distinct pointer types ('auto *' and 'Instruction *') 610 | return U != I && | ~ ^ ~ ``` https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: Reverted in #92149 https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
erichkeane wrote: This is unfortunately one of the cases where if you can't fix it 'quickly', a revert is necessary, as it breaks the build. https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: This seems to [break something](https://lab.llvm.org/buildbot/#/builders/121/builds/41631) in LLVM... investigating https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian closed https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
erichkeane wrote: > @erichkeane Since I addressed @shafik's comments, it this good to be merged? Yep, go for it! https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/shafik approved this pull request. Thank you for the additional test coverage, LGTM https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: @erichkeane Since I addressed @shafik's comments, it this good to be merged? https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: Ping @shafik https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 68ae8a9321b96da8cde1a1813d5e2b0c352649b7 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH 1/4] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++--- .../test/SemaTemplate/class-template-spec.cpp | 12 +- 9 files changed, 36 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index e6643469e0b334..da3834f19ca044 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 b1322f30fa6b6a..ad670165242187 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,12 +671,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 @@ -7,7 +7,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14288,7 +14288,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14742,7 +14744,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: @shafik More tests added https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 1b3476db3208ccb0b425ff604755349437d28863 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH 1/4] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++--- .../test/SemaTemplate/class-template-spec.cpp | 12 +- 9 files changed, 36 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index e6643469e0b334..da3834f19ca044 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 0c37f43f75401b..cf61c191e5baef 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,12 +671,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 @@ -6,7 +6,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14287,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14727,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: Yup, working on it :) https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 1b3476db3208ccb0b425ff604755349437d28863 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH 1/3] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++--- .../test/SemaTemplate/class-template-spec.cpp | 12 +- 9 files changed, 36 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index e6643469e0b334..da3834f19ca044 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 0c37f43f75401b..cf61c191e5baef 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,12 +671,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 @@ -6,7 +6,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14287,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14727,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/erichkeane commented: I'm still OK with this, but @shafik : Please ensure this has the test coverage you asked for. https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
sdkrystian wrote: @erichkeane Release note added https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 1b3476db3208ccb0b425ff604755349437d28863 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH 1/3] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++--- .../test/SemaTemplate/class-template-spec.cpp | 12 +- 9 files changed, 36 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index e6643469e0b334..da3834f19ca044 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 0c37f43f75401b..cf61c191e5baef 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,12 +671,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 @@ -6,7 +6,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14287,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14727,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 1b3476db3208ccb0b425ff604755349437d28863 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH 1/2] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++--- .../test/SemaTemplate/class-template-spec.cpp | 12 +- 9 files changed, 36 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index e6643469e0b334..da3834f19ca044 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 0c37f43f75401b..cf61c191e5baef 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,12 +671,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 @@ -6,7 +6,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14287,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14727,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 1b3476db3208ccb0b425ff604755349437d28863 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 22 +++ clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++--- .../test/SemaTemplate/class-template-spec.cpp | 12 +- 9 files changed, 36 insertions(+), 29 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index e6643469e0b334..da3834f19ca044 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 0c37f43f75401b..cf61c191e5baef 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,12 +671,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 @@ -6,7 +6,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14287,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14727,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/shafik commented: So this seems to apply to member pointer types, pointer types, arrays and function types but I don't see coverage for the full set of paths in the tests. Can we please add more testing. Erich also expressed some concern about breaking existing code due to eager instantiation. Can we make sure try to cover any change in behavior in the tests so reviewers can get a better idea of impact. https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/erichkeane approved this pull request. This needs a release note, likely in the 'potentially breaking changes'. Projects are filled with code that is not ever instantiated, and Clang's aggressive instantiation can be a problem. Else, LGTM. https://github.com/llvm/llvm-project/pull/90500 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian updated https://github.com/llvm/llvm-project/pull/90500 >From 6a2d3719508fbd765a97e81688ae06996e7daf21 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 5 - clang/lib/Sema/SemaExpr.cpp | 21 --- clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++ .../test/SemaTemplate/class-template-spec.cpp | 12 +-- 9 files changed, 36 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index dff02d4861b3db..fae4730130ae50 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -8040,7 +8040,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 50f92c496a539a..9284ccb3eb7715 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,11 +671,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 || - T->isDependentType() || - 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 @@ -6,7 +7,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14288,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14728,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:}, -// CHECK-NEXT:"valueCategory": "prvalue", +// CHECK-NEXT:"valueCategory": "lvalue", // CHECK-NEXT:"isPostfix": false, // CHECK-NEXT:"opcode": "*", // CHECK-NEXT:"canOverflow": false, diff --git a/clang/test/AST/ast-dump-expr.cpp b/clang/test/AST/ast-dump-expr.cpp index 69e65e22d61d0d..4df5ba4276abd2 100644 ---
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Krystian Stasiowski (sdkrystian) Changes Currently, clang postpones all semantic analysis of unary operators with operands of pointer/pointer to member/array/function type until instantiation whenever that type is dependent (e.g. `T*` where `T` is a type template parameter). Consequently, the uninstantiated AST nodes all have the type `ASTContext::DependentTy` (which, for the purposes of #90152, is undesirable as that type may be the current instantiation! (e.g. `*this`)) This patch moves the point at which we perform semantic analysis for such expression to be prior to instantiation. --- Full diff: https://github.com/llvm/llvm-project/pull/90500.diff 9 Files Affected: - (modified) clang/include/clang/AST/Type.h (+5-1) - (modified) clang/lib/Sema/SemaExpr.cpp (+13-8) - (modified) clang/test/AST/ast-dump-expr-json.cpp (+2-2) - (modified) clang/test/AST/ast-dump-expr.cpp (+1-1) - (modified) clang/test/AST/ast-dump-lambda.cpp (+1-1) - (modified) clang/test/CXX/over/over.built/ast.cpp (+5-3) - (modified) clang/test/Frontend/noderef_templates.cpp (+2-2) - (modified) clang/test/SemaCXX/cxx2b-deducing-this.cpp (+2-4) - (modified) clang/test/SemaTemplate/class-template-spec.cpp (+6-6) ``diff diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index dff02d4861b3db..471f78bbdf283c 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -8040,7 +8040,11 @@ 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(); + QualType Canonical = getCanonicalTypeInternal(); + if (!Canonical->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 50f92c496a539a..9284ccb3eb7715 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,11 +671,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 || - T->isDependentType() || - 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 @@ -6,7 +7,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14288,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14728,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, // CHECK-NEXT:"type": { -// CHECK-NEXT: "qualType": "" +// CHECK-NEXT: "qualType": "V" // CHECK-NEXT:
[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)
https://github.com/sdkrystian created https://github.com/llvm/llvm-project/pull/90500 Currently, clang postpones all semantic analysis of unary operators with operands of pointer/pointer to member/array/function type until instantiation whenever that type is dependent (e.g. `T*` where `T` is a type template parameter). Consequently, the uninstantiated AST nodes all have the type `ASTContext::DependentTy` (which, for the purposes of #90152, is undesirable as that type may be the current instantiation! (e.g. `*this`)) This patch moves the point at which we perform semantic analysis for such expression to be prior to instantiation. >From f292ba98578a51ba79541e1b44bf4f4749326067 Mon Sep 17 00:00:00 2001 From: Krystian Stasiowski Date: Thu, 25 Apr 2024 08:17:21 -0400 Subject: [PATCH] [Clang][Sema] Earlier type checking for builtin unary operators --- clang/include/clang/AST/Type.h| 6 +- clang/lib/Sema/SemaExpr.cpp | 21 --- clang/test/AST/ast-dump-expr-json.cpp | 4 ++-- clang/test/AST/ast-dump-expr.cpp | 2 +- clang/test/AST/ast-dump-lambda.cpp| 2 +- clang/test/CXX/over/over.built/ast.cpp| 8 --- clang/test/Frontend/noderef_templates.cpp | 4 ++-- clang/test/SemaCXX/cxx2b-deducing-this.cpp| 6 ++ .../test/SemaTemplate/class-template-spec.cpp | 12 +-- 9 files changed, 37 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index dff02d4861b3db..471f78bbdf283c 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -8040,7 +8040,11 @@ 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(); + QualType Canonical = getCanonicalTypeInternal(); + if (!Canonical->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 50f92c496a539a..9284ccb3eb7715 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -671,11 +671,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 || - T->isDependentType() || - 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 @@ -6,7 +7,7 @@ static bool checkArithmeticIncompletePointerType(Sema , SourceLocation Loc, if (const AtomicType *ResAtomicType = ResType->getAs()) ResType = ResAtomicType->getValueType(); - assert(ResType->isAnyPointerType() && !ResType->isDependentType()); + assert(ResType->isAnyPointerType()); QualType PointeeTy = ResType->getPointeeType(); return S.RequireCompleteSizedType( Loc, PointeeTy, @@ -14287,7 +14288,9 @@ static QualType CheckIncrementDecrementOperand(Sema , Expr *Op, ExprObjectKind , SourceLocation OpLoc, bool IsInc, bool IsPrefix) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; QualType ResType = Op->getType(); @@ -14725,7 +14728,9 @@ static void RecordModifiableNonNullParam(Sema , const Expr *Exp) { static QualType CheckIndirectionOperand(Sema , Expr *Op, ExprValueKind , SourceLocation OpLoc, bool IsAfterAmp = false) { - if (Op->isTypeDependent()) + if (Op->isTypeDependent() && + (Op->hasPlaceholderType() || + Op->getType()->isSpecificBuiltinType(BuiltinType::Dependent))) return S.Context.DependentTy; ExprResult ConvResult = S.UsualUnaryConversions(Op); diff --git a/clang/test/AST/ast-dump-expr-json.cpp b/clang/test/AST/ast-dump-expr-json.cpp index 0fb07b0b434cc3..4b7365e554cb7c 100644 --- a/clang/test/AST/ast-dump-expr-json.cpp +++ b/clang/test/AST/ast-dump-expr-json.cpp @@ -4261,9 +4261,9 @@ void TestNonADLCall3() { // CHECK-NEXT: } // CHECK-NEXT:}, //