[clang] [Clang][Sema] Earlier type checking for builtin unary operators (PR #90500)

2024-05-14 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-14 Thread Erich Keane via cfe-commits

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)

2024-05-14 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-14 Thread Fangrui Song via cfe-commits

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)

2024-05-14 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-14 Thread Erich Keane via cfe-commits

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)

2024-05-14 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-14 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-14 Thread Erich Keane via cfe-commits

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)

2024-05-14 Thread Shafik Yaghmour via cfe-commits

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)

2024-05-14 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-08 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-06 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Erich Keane via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-05-03 Thread Krystian Stasiowski via cfe-commits

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)

2024-04-29 Thread Shafik Yaghmour via cfe-commits

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)

2024-04-29 Thread Erich Keane via cfe-commits

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)

2024-04-29 Thread Krystian Stasiowski via cfe-commits

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)

2024-04-29 Thread via cfe-commits

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)

2024-04-29 Thread Krystian Stasiowski via cfe-commits

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:},
 //