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

Krystian Stasiowski via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 29 10:40:09 PDT 2024


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 <sdkrystian at gmail.com>
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
@@ -11116,7 +11117,7 @@ static bool checkArithmeticIncompletePointerType(Sema &S, SourceLocation Loc,
   if (const AtomicType *ResAtomicType = ResType->getAs<AtomicType>())
     ResType = ResAtomicType->getValueType();
 
-  assert(ResType->isAnyPointerType() && !ResType->isDependentType());
+  assert(ResType->isAnyPointerType());
   QualType PointeeTy = ResType->getPointeeType();
   return S.RequireCompleteSizedType(
       Loc, PointeeTy,
@@ -14287,7 +14288,9 @@ static QualType CheckIncrementDecrementOperand(Sema &S, Expr *Op,
                                                ExprObjectKind &OK,
                                                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 &S, const Expr *Exp) {
 static QualType CheckIndirectionOperand(Sema &S, Expr *Op, ExprValueKind &VK,
                                         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": "<dependent type>"
+// 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
--- a/clang/test/AST/ast-dump-expr.cpp
+++ b/clang/test/AST/ast-dump-expr.cpp
@@ -282,7 +282,7 @@ void PrimaryExpressions(Ts... a) {
       // CHECK-NEXT: CompoundStmt
       // CHECK-NEXT: FieldDecl 0x{{[^ ]*}} <col:8> col:8 implicit 'V'
       // CHECK-NEXT: ParenListExpr 0x{{[^ ]*}} <col:8> 'NULL TYPE'
-      // CHECK-NEXT: UnaryOperator 0x{{[^ ]*}} <col:8> '<dependent type>' prefix '*' cannot overflow
+      // CHECK-NEXT: UnaryOperator 0x{{[^ ]*}} <col:8> 'V' lvalue prefix '*' cannot overflow
       // CHECK-NEXT: CXXThisExpr 0x{{[^ ]*}} <col:8> 'V *' this
     }
   };
diff --git a/clang/test/AST/ast-dump-lambda.cpp b/clang/test/AST/ast-dump-lambda.cpp
index ef8789cd97d3e7..a4d3fe4fbda57c 100644
--- a/clang/test/AST/ast-dump-lambda.cpp
+++ b/clang/test/AST/ast-dump-lambda.cpp
@@ -81,7 +81,7 @@ template <typename... Ts> void test(Ts... a) {
 // CHECK-NEXT:    |         | | `-CompoundStmt {{.*}} <col:15, col:16>
 // CHECK-NEXT:    |         | `-FieldDecl {{.*}} <col:8> col:8{{( imported)?}} implicit 'V'
 // CHECK-NEXT:    |         |-ParenListExpr {{.*}} <col:8> 'NULL TYPE'
-// CHECK-NEXT:    |         | `-UnaryOperator {{.*}} <col:8> '<dependent type>' prefix '*' cannot overflow
+// CHECK-NEXT:    |         | `-UnaryOperator {{.*}} <col:8> 'V' lvalue prefix '*' cannot overflow
 // CHECK-NEXT:    |         |   `-CXXThisExpr {{.*}} <col:8> 'V *' this
 // CHECK-NEXT:    |         `-CompoundStmt {{.*}} <col:15, col:16>
 // CHECK-NEXT:    |-DeclStmt {{.*}} <line:22:3, col:11>
diff --git a/clang/test/CXX/over/over.built/ast.cpp b/clang/test/CXX/over/over.built/ast.cpp
index 56a63431269f30..b95a453de90222 100644
--- a/clang/test/CXX/over/over.built/ast.cpp
+++ b/clang/test/CXX/over/over.built/ast.cpp
@@ -4,15 +4,17 @@ struct A{};
 
 template <typename T, typename U>
 auto Test(T* pt, U* pu) {
-  // CHECK: UnaryOperator {{.*}} '<dependent type>' lvalue prefix '*'
+  // CHECK: UnaryOperator {{.*}} 'T' lvalue prefix '*'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'T *' <LValueToRValue>
   // CHECK-NEXT: DeclRefExpr {{.*}} 'T *' lvalue ParmVar {{.*}} 'pt' 'T *'
   (void)*pt;
 
-  // CHECK: UnaryOperator {{.*}} '<dependent type>' lvalue prefix '++'
+  // CHECK: UnaryOperator {{.*}} 'T *' lvalue prefix '++'
   // CHECK-NEXT: DeclRefExpr {{.*}} 'T *' lvalue ParmVar {{.*}} 'pt' 'T *'
   (void)(++pt);
 
-  // CHECK: UnaryOperator {{.*}} '<dependent type>' prefix '+'
+  // CHECK: UnaryOperator {{.*}} 'T *' prefix '+'
+  // CHECK-NEXT: ImplicitCastExpr {{.*}} 'T *' <LValueToRValue>
   // CHECK-NEXT: DeclRefExpr {{.*}} 'T *' lvalue ParmVar {{.*}} 'pt' 'T *'
   (void)(+pt);
 
diff --git a/clang/test/Frontend/noderef_templates.cpp b/clang/test/Frontend/noderef_templates.cpp
index 5fde6efd87c7fb..9e54cd5d788992 100644
--- a/clang/test/Frontend/noderef_templates.cpp
+++ b/clang/test/Frontend/noderef_templates.cpp
@@ -3,8 +3,8 @@
 #define NODEREF __attribute__((noderef))
 
 template <typename T>
-int func(T NODEREF *a) { // expected-note 2 {{a declared here}}
-  return *a + 1;         // expected-warning 2 {{dereferencing a; was declared with a 'noderef' type}}
+int func(T NODEREF *a) { // expected-note 3 {{a declared here}}
+  return *a + 1;         // expected-warning 3 {{dereferencing a; was declared with a 'noderef' type}}
 }
 
 void func() {
diff --git a/clang/test/SemaCXX/cxx2b-deducing-this.cpp b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
index 5f29a955e053c3..aa64530bd5be3d 100644
--- a/clang/test/SemaCXX/cxx2b-deducing-this.cpp
+++ b/clang/test/SemaCXX/cxx2b-deducing-this.cpp
@@ -19,7 +19,7 @@ struct S {
     // new and delete are implicitly static
     void *operator new(this unsigned long); // expected-error{{an explicit object parameter cannot appear in a static function}}
     void operator delete(this void*); // expected-error{{an explicit object parameter cannot appear in a static function}}
-    
+
     void g(this auto) const; // expected-error{{explicit object member function cannot have 'const' qualifier}}
     void h(this auto) &; // expected-error{{explicit object member function cannot have '&' qualifier}}
     void i(this auto) &&; // expected-error{{explicit object member function cannot have '&&' qualifier}}
@@ -198,9 +198,7 @@ void func(int i) {
 void TestMutationInLambda() {
     [i = 0](this auto &&){ i++; }();
     [i = 0](this auto){ i++; }();
-    [i = 0](this const auto&){ i++; }();
-    // expected-error at -1 {{cannot assign to a variable captured by copy in a non-mutable lambda}}
-    // expected-note at -2 {{in instantiation of}}
+    [i = 0](this const auto&){ i++; }(); // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}}
 
     int x;
     const auto l1 = [x](this auto&) { x = 42; }; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}}
diff --git a/clang/test/SemaTemplate/class-template-spec.cpp b/clang/test/SemaTemplate/class-template-spec.cpp
index 56b8207bd9a435..faa54c36753831 100644
--- a/clang/test/SemaTemplate/class-template-spec.cpp
+++ b/clang/test/SemaTemplate/class-template-spec.cpp
@@ -18,7 +18,7 @@ int test_specs(A<float, float> *a1, A<float, int> *a2) {
   return a1->x + a2->y;
 }
 
-int test_incomplete_specs(A<double, double> *a1, 
+int test_incomplete_specs(A<double, double> *a1,
                           A<double> *a2)
 {
   (void)a1->x; // expected-error{{member access into incomplete type}}
@@ -39,7 +39,7 @@ template <> struct X<int, int> { int foo(); }; // #1
 template <> struct X<float> { int bar(); };  // #2
 
 typedef int int_type;
-void testme(X<int_type> *x1, X<float, int> *x2) { 
+void testme(X<int_type> *x1, X<float, int> *x2) {
   (void)x1->foo(); // okay: refers to #1
   (void)x2->bar(); // okay: refers to #2
 }
@@ -53,7 +53,7 @@ struct A<char> {
 A<char>::A() { }
 
 // Make sure we can see specializations defined before the primary template.
-namespace N{ 
+namespace N{
   template<typename T> struct A0;
 }
 
@@ -97,7 +97,7 @@ namespace M {
   template<> struct ::A<long double>; // expected-error{{must occur at global scope}}
 }
 
-template<> struct N::B<char> { 
+template<> struct N::B<char> {
   int testf(int x) { return f(x); }
 };
 
@@ -138,9 +138,9 @@ namespace PR18009 {
 
   template <typename T> struct C {
     template <int N, int M> struct S;
-    template <int N> struct S<N, N ? **(T(*)[N])0 : 0> {}; // expected-error {{depends on a template parameter of the partial specialization}}
+    template <int N> struct S<N, N ? **(T(*)[N])0 : 0> {}; // ok
   };
-  C<int> c; // expected-note {{in instantiation of}}
+  C<int> c;
 
   template<int A> struct outer {
     template<int B, int C> struct inner {};



More information about the cfe-commits mailing list