Addressing a few more review comments

ojhunt · ojhunt · commit 6934655ca732 · 2025-03-29T23:28:14.000-07:00
Removed the new Type::* methods as the various refactorings have made
them unnecessary.

Renamed isOperatorNew, Delete, NewOrDelete, etc to say "Any" as while
going through adopting them everywhere i noticed a few places where
code is checking explicitly for non-array versions of the operators and
realised the existing name was a recipe for mistakes. Also adopted the
helper function in more places near other changes in this PR.
diff --git a/clang/include/clang/AST/DeclarationName.h b/clang/include/clang/AST/DeclarationName.h
@@ -477,7 +477,7 @@ class DeclarationName {
     return OO_None;
   }
 
-  bool isOperatorNew() const {
+  bool isAnyOperatorNew() const {
     if (getNameKind() != DeclarationName::CXXOperatorName)
       return false;
     switch (getCXXOverloadedOperator()) {
@@ -489,7 +489,7 @@ class DeclarationName {
     }
   }
 
-  bool isOperatorDelete() const {
+  bool isAnyOperatorDelete() const {
     if (getNameKind() != DeclarationName::CXXOperatorName)
       return false;
     switch (getCXXOverloadedOperator()) {
@@ -501,8 +501,8 @@ class DeclarationName {
     }
   }
 
-  bool isOperatorNewOrDelete() const {
-    return isOperatorNew() || isOperatorDelete();
+  bool isAnyOperatorNewOrDelete() const {
+    return isAnyOperatorNew() || isAnyOperatorDelete();
   }
 
   /// If this name is the name of a literal operator,
diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
@@ -2636,13 +2636,6 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
                                                 // C++14 decltype(auto)
   bool isTypedefNameType() const;               // typedef or alias template
 
-  bool isDestroyingDeleteT() const; // C++ std::destroying_delete_t
-
-  /// If this type is a template specialization return the TemplateDecl
-  /// that was specialized. It this is not a template specialization,
-  /// returns NULL.
-  TemplateDecl *getSpecializedTemplateDecl() const;
-
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   bool is##Id##Type() const;
 #include "clang/Basic/OpenCLImageTypes.def"
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -4883,7 +4883,6 @@ class Sema final : public SemaBase {
   EnumDecl *getStdAlignValT() const;
 
   TypeAwareAllocationMode ShouldUseTypeAwareOperatorNewOrDelete() const;
-  bool isTypeAwareOperatorNewOrDelete(const NamedDecl *FnDecl) const;
   FunctionDecl *BuildTypeAwareUsualDelete(FunctionTemplateDecl *FnDecl,
                                           QualType AllocType, SourceLocation);
 
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -4787,7 +4787,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
   // Calls to replaceable operator new/operator delete.
   if (FuncDecl &&
       FuncDecl->isUsableAsGlobalAllocationFunctionInConstantEvaluation()) {
-    if (FuncDecl->getDeclName().isOperatorNew()) {
+    if (FuncDecl->getDeclName().isAnyOperatorNew()) {
       return VisitBuiltinCallExpr(E, Builtin::BI__builtin_operator_new);
     } else {
       assert(FuncDecl->getDeclName().getCXXOverloadedOperator() == OO_Delete);
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
@@ -3363,7 +3363,7 @@ bool FunctionDecl::isMSVCRTEntryPoint() const {
 }
 
 bool FunctionDecl::isReservedGlobalPlacementOperator() const {
-  if (!getDeclName().isOperatorNewOrDelete())
+  if (!getDeclName().isAnyOperatorNewOrDelete())
     return false;
 
   if (!getDeclContext()->getRedeclContext()->isTranslationUnit())
@@ -3387,7 +3387,7 @@ bool FunctionDecl::isReservedGlobalPlacementOperator() const {
 
 bool FunctionDecl::isUsableAsGlobalAllocationFunctionInConstantEvaluation(
     std::optional<unsigned> *AlignmentParam, bool *IsNothrow) const {
-  if (!getDeclName().isOperatorNewOrDelete())
+  if (!getDeclName().isAnyOperatorNewOrDelete())
     return false;
 
   if (isa<CXXRecordDecl>(getDeclContext()))
@@ -3401,7 +3401,7 @@ bool FunctionDecl::isUsableAsGlobalAllocationFunctionInConstantEvaluation(
     return false;
 
   if (isTypeAwareOperatorNewOrDelete()) {
-    bool IsDelete = getDeclName().isOperatorDelete();
+    bool IsDelete = getDeclName().isAnyOperatorDelete();
     unsigned RequiredParameterCount =
         IsDelete ? FunctionDecl::RequiredTypeAwareDeleteParameterCount
                  : FunctionDecl::RequiredTypeAwareNewParameterCount;
@@ -3441,8 +3441,7 @@ bool FunctionDecl::isUsableAsGlobalAllocationFunctionInConstantEvaluation(
   // In C++14, the next parameter can be a 'std::size_t' for sized delete.
   bool IsSizedDelete = false;
   if (Ctx.getLangOpts().SizedDeallocation &&
-      (getDeclName().getCXXOverloadedOperator() == OO_Delete ||
-       getDeclName().getCXXOverloadedOperator() == OO_Array_Delete) &&
+      getDeclName().isAnyOperatorDelete() &&
       Ctx.hasSameType(Ty, Ctx.getSizeType())) {
     IsSizedDelete = true;
     Consume();
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
@@ -2529,8 +2529,7 @@ CXXMethodDecl *CXXMethodDecl::getDevirtualizedMethod(const Expr *Base,
 bool CXXMethodDecl::isUsualDeallocationFunction(
     SmallVectorImpl<const FunctionDecl *> &PreventedBy) const {
   assert(PreventedBy.empty() && "PreventedBy is expected to be empty");
-  if (getOverloadedOperator() != OO_Delete &&
-      getOverloadedOperator() != OO_Array_Delete)
+  if (!getDeclName().isAnyOperatorDelete())
     return false;
 
   if (isTypeAwareOperatorNewOrDelete()) {
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
@@ -8386,7 +8386,7 @@ class ExprEvaluatorBase
         } else
           FD = LambdaCallOp;
       } else if (FD->isUsableAsGlobalAllocationFunctionInConstantEvaluation()) {
-        if (FD->getDeclName().isOperatorNew()) {
+        if (FD->getDeclName().isAnyOperatorNew()) {
           LValue Ptr;
           if (!HandleOperatorNewCall(Info, E, Ptr))
             return false;
diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp
@@ -3133,23 +3133,6 @@ bool Type::isStdByteType() const {
   return false;
 }
 
-bool Type::isDestroyingDeleteT() const {
-  auto *RD = getAsCXXRecordDecl();
-  return RD && RD->isInStdNamespace() && RD->getIdentifier() &&
-         RD->getIdentifier()->isStr("destroying_delete_t");
-}
-
-TemplateDecl *Type::getSpecializedTemplateDecl() const {
-  auto UnderlyingType = getCanonicalTypeInternal();
-  if (auto *TST = UnderlyingType->getAs<TemplateSpecializationType>())
-    return TST->getTemplateName().getAsTemplateDecl();
-  if (auto *RT = UnderlyingType->getAsCXXRecordDecl()) {
-    if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RT))
-      return CTSD->getSpecializedTemplate();
-  }
-  return nullptr;
-}
-
 bool Type::isSpecifierType() const {
   // Note that this intentionally does not use the canonical type.
   switch (getTypeClass()) {
diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp
@@ -1110,7 +1110,7 @@ static bool DiagnoseTypeAwareAllocators(Sema &S, SourceLocation Loc,
   S.LookupQualifiedName(R, PromiseType->getAsCXXRecordDecl());
   bool HaveIssuedWarning = false;
   for (auto Decl : R) {
-    if (!S.isTypeAwareOperatorNewOrDelete(Decl))
+    if (!Decl->getAsFunction()->isTypeAwareOperatorNewOrDelete())
       continue;
     if (!HaveIssuedWarning) {
       S.Diag(Loc, DiagnosticID) << Name;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
@@ -10175,10 +10175,7 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
       // deallocation function shall not be declared with the consteval
       // specifier.
       if (ConstexprKind == ConstexprSpecKind::Consteval &&
-          (NewFD->getOverloadedOperator() == OO_New ||
-           NewFD->getOverloadedOperator() == OO_Array_New ||
-           NewFD->getOverloadedOperator() == OO_Delete ||
-           NewFD->getOverloadedOperator() == OO_Array_Delete)) {
+          NewFD->getDeclName().isAnyOperatorNewOrDelete()) {
         Diag(D.getDeclSpec().getConstexprSpecLoc(),
              diag::err_invalid_consteval_decl_kind)
             << NewFD;
@@ -10282,9 +10279,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
     //   A deallocation function with no exception-specification is treated
     //   as if it were specified with noexcept(true).
     const FunctionProtoType *FPT = R->getAs<FunctionProtoType>();
-    if ((Name.getCXXOverloadedOperator() == OO_Delete ||
-         Name.getCXXOverloadedOperator() == OO_Array_Delete) &&
-        getLangOpts().CPlusPlus11 && FPT && !FPT->hasExceptionSpec())
+    if (Name.isAnyOperatorDelete() && getLangOpts().CPlusPlus11 && FPT &&
+        !FPT->hasExceptionSpec())
       NewFD->setType(Context.getFunctionType(
           FPT->getReturnType(), FPT->getParamTypes(),
           FPT->getExtProtoInfo().withExceptionSpec(EST_BasicNoexcept)));
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -16448,16 +16448,6 @@ TypeAwareAllocationMode Sema::ShouldUseTypeAwareOperatorNewOrDelete() const {
   return typeAwareAllocationModeFromBool(SeenTypedOperators);
 }
 
-bool Sema::isTypeAwareOperatorNewOrDelete(const NamedDecl *ND) const {
-  const FunctionDecl *FnDecl = nullptr;
-  if (auto *FTD = dyn_cast<FunctionTemplateDecl>(ND))
-    FnDecl = FTD->getTemplatedDecl();
-  else if (auto *FD = dyn_cast<FunctionDecl>(ND))
-    FnDecl = FD;
-
-  return FnDecl->isTypeAwareOperatorNewOrDelete();
-}
-
 FunctionDecl *
 Sema::BuildTypeAwareUsualDelete(FunctionTemplateDecl *FnTemplateDecl,
                                 QualType DeallocType, SourceLocation Loc) {
@@ -16478,13 +16468,13 @@ Sema::BuildTypeAwareUsualDelete(FunctionTemplateDecl *FnTemplateDecl,
   if (NumParams != RequiredParameterCount)
     return nullptr;
 
-  for (size_t Idx = 1; Idx < NumParams; ++Idx) {
-    // A type aware allocation is only usual if the only dependent parameter is
-    // the first parameter.
-    const ParmVarDecl *ParamDecl = FnDecl->getParamDecl(Idx);
-    if (ParamDecl->getType()->isDependentType())
-      return nullptr;
-  }
+  // A type aware allocation is only usual if the only dependent parameter is
+  // the first parameter.
+  if (llvm::any_of(FnDecl->parameters().drop_front(),
+                   [](const ParmVarDecl *ParamDecl) {
+                     return ParamDecl->getType()->isDependentType();
+                   }))
+    return nullptr;
 
   QualType SpecializedTypeIdentity = tryBuildStdTypeIdentity(DeallocType, Loc);
   if (SpecializedTypeIdentity.isNull())
@@ -16563,6 +16553,12 @@ static bool IsPotentiallyTypeAwareOperatorNewOrDelete(Sema &SemaRef,
   return true;
 }
 
+static bool isDestroyingDeleteT(QualType Type) {
+  auto *RD = Type->getAsCXXRecordDecl();
+  return RD && RD->isInStdNamespace() && RD->getIdentifier() &&
+         RD->getIdentifier()->isStr("destroying_delete_t");
+}
+
 static bool IsPotentiallyDestroyingOperatorDelete(Sema &SemaRef,
                                                   const FunctionDecl *FD) {
   // C++ P0722:
@@ -16574,9 +16570,7 @@ static bool IsPotentiallyDestroyingOperatorDelete(Sema &SemaRef,
   unsigned DestroyingDeleteIdx = IsPotentiallyTypeAware + /* address */ 1;
   return isa<CXXMethodDecl>(FD) && FD->getOverloadedOperator() == OO_Delete &&
          FD->getNumParams() > DestroyingDeleteIdx &&
-         FD->getParamDecl(DestroyingDeleteIdx)
-             ->getType()
-             ->isDestroyingDeleteT();
+         isDestroyingDeleteT(FD->getParamDecl(DestroyingDeleteIdx)->getType());
 }
 
 static inline bool CheckOperatorNewDeleteTypes(
@@ -16767,7 +16761,7 @@ CheckOperatorDeleteDeclaration(Sema &SemaRef, FunctionDecl *FnDecl) {
       // results in a pile mismatched argument type errors that don't explain
       // the core problem.
       for (auto Param : MD->parameters()) {
-        if (Param->getType()->isDestroyingDeleteT()) {
+        if (isDestroyingDeleteT(Param->getType())) {
           SemaRef.Diag(MD->getLocation(),
                        diag::err_type_aware_destroying_operator_delete)
               << Param->getLocation();
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1749,8 +1749,7 @@ static bool isNonPlacementDeallocationFunction(Sema &S, FunctionDecl *FD) {
   if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(FD))
     return S.isUsualDeallocationFunction(Method);
 
-  if (FD->getOverloadedOperator() != OO_Delete &&
-      FD->getOverloadedOperator() != OO_Array_Delete)
+  if (!FD->getDeclName().isAnyOperatorDelete())
     return false;
 
   if (FD->isTypeAwareOperatorNewOrDelete())
@@ -2143,8 +2142,7 @@ void Sema::diagnoseUnavailableAlignedAllocation(const FunctionDecl &FD,
         getASTContext().getTargetInfo().getPlatformName());
     VersionTuple OSVersion = alignedAllocMinVersion(T.getOS());
 
-    OverloadedOperatorKind Kind = FD.getDeclName().getCXXOverloadedOperator();
-    bool IsDelete = Kind == OO_Delete || Kind == OO_Array_Delete;
+    bool IsDelete = FD.getDeclName().isAnyOperatorDelete();
     Diag(Loc, diag::err_aligned_allocation_unavailable)
         << IsDelete << FD.getType().getAsString() << OSName
         << OSVersion.getAsString() << OSVersion.empty();
@@ -2704,7 +2702,8 @@ static bool resolveAllocationOverloadInterior(
     // Even member operator new/delete are implicitly treated as
     // static, so don't use AddMemberCandidate.
     NamedDecl *D = (*Alloc)->getUnderlyingDecl();
-    if (S.isTypeAwareOperatorNewOrDelete(D) == (Mode != ResolveMode::Typed))
+    bool IsTypeAware = D->getAsFunction()->isTypeAwareOperatorNewOrDelete();
+    if (IsTypeAware == (Mode != ResolveMode::Typed))
       continue;
 
     if (FunctionTemplateDecl *FnTemplate = dyn_cast<FunctionTemplateDecl>(D)) {
@@ -2860,9 +2859,8 @@ static void LookupGlobalDeallocationFunctions(Sema &S, SourceLocation Loc,
     bool RemoveTypedDecl = Mode == DeallocLookupMode::Untyped;
     LookupResult::Filter Filter = FoundDelete.makeFilter();
     while (Filter.hasNext()) {
-      NamedDecl *Decl = Filter.next()->getUnderlyingDecl();
-      bool DeclIsTypeAware = S.isTypeAwareOperatorNewOrDelete(Decl);
-      if (DeclIsTypeAware == RemoveTypedDecl)
+      FunctionDecl *FD = Filter.next()->getUnderlyingDecl()->getAsFunction();
+      if (FD->isTypeAwareOperatorNewOrDelete() == RemoveTypedDecl)
         Filter.erase();
     }
     Filter.done();
@@ -3460,7 +3458,7 @@ void Sema::DeclareGlobalAllocationFunction(DeclarationName Name,
       /*IsVariadic=*/false, /*IsCXXMethod=*/false, /*IsBuiltin=*/true));
 
   QualType BadAllocType;
-  bool HasBadAllocExceptionSpec = Name.isOperatorNew();
+  bool HasBadAllocExceptionSpec = Name.isAnyOperatorNew();
   if (HasBadAllocExceptionSpec) {
     if (!getLangOpts().CPlusPlus11) {
       BadAllocType = Context.getTypeDeclType(getStdBadAlloc());
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
@@ -9898,13 +9898,24 @@ QualType Sema::DeduceTemplateSpecializationFromInitializer(
       Diag(Kind.getLocation(),
            diag::warn_cxx17_compat_ctad_for_alias_templates);
       LookupTemplateDecl = AliasTemplate;
-      auto UnderlyingType =
-          AliasTemplate->getTemplatedDecl()->getUnderlyingType();
+      auto UnderlyingType = AliasTemplate->getTemplatedDecl()
+                                ->getUnderlyingType()
+                                .getCanonicalType();
       // C++ [over.match.class.deduct#3]: ..., the defining-type-id of A must be
       // of the form
       //   [typename] [nested-name-specifier] [template] simple-template-id
-      auto SpecializedTD = UnderlyingType->getSpecializedTemplateDecl();
-      Template = dyn_cast_or_null<ClassTemplateDecl>(SpecializedTD);
+      if (const auto *TST =
+              UnderlyingType->getAs<TemplateSpecializationType>()) {
+        Template = dyn_cast_or_null<ClassTemplateDecl>(
+            TST->getTemplateName().getAsTemplateDecl());
+      } else if (const auto *RT = UnderlyingType->getAs<RecordType>()) {
+        // Cases where template arguments in the RHS of the alias are not
+        // dependent. e.g.
+        //   using AliasFoo = Foo<bool>;
+        if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(
+                RT->getAsCXXRecordDecl()))
+          Template = CTSD->getSpecializedTemplate();
+      }
     }
   }
   if (!Template) {
diff --git a/clang/test/SemaCXX/type-aware-coroutines.cpp b/clang/test/SemaCXX/type-aware-coroutines.cpp
@@ -142,4 +142,4 @@ resumable5 f7(float) {
   // expected-warning@-1 {{type aware 'operator delete' will not be used for coroutine allocation}}
   // expected-note@#resumable5_tad {{type aware 'operator delete' declared here}}
   co_return;
-}
+}

Original file line number	Diff line number	Diff line change
`@@ -477,7 +477,7 @@ class DeclarationName {`
`477`	`477`	`return OO_None;`
`478`	`478`	`}`
`479`	`479`
`480`		`- bool isOperatorNew() const {`
	`480`	`+ bool isAnyOperatorNew() const {`
`481`	`481`	`if (getNameKind() != DeclarationName::CXXOperatorName)`
`482`	`482`	`return false;`
`483`	`483`	`switch (getCXXOverloadedOperator()) {`
`@@ -489,7 +489,7 @@ class DeclarationName {`
`489`	`489`	`}`
`490`	`490`	`}`
`491`	`491`
`492`		`- bool isOperatorDelete() const {`
	`492`	`+ bool isAnyOperatorDelete() const {`
`493`	`493`	`if (getNameKind() != DeclarationName::CXXOperatorName)`
`494`	`494`	`return false;`
`495`	`495`	`switch (getCXXOverloadedOperator()) {`
`@@ -501,8 +501,8 @@ class DeclarationName {`
`501`	`501`	`}`
`502`	`502`	`}`
`503`	`503`
`504`		`- bool isOperatorNewOrDelete() const {`
`505`		`- return isOperatorNew() \|\| isOperatorDelete();`
	`504`	`+ bool isAnyOperatorNewOrDelete() const {`
	`505`	`+ return isAnyOperatorNew() \|\| isAnyOperatorDelete();`
`506`	`506`	`}`
`507`	`507`
`508`	`508`	`/// If this name is the name of a literal operator,`