Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang] Remove CXXNewInitializationStyle::Implicit #78793

Merged
merged 3 commits into from
Jan 21, 2024

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Jan 19, 2024

This is a follow up to #71417 , which aims to resolve concerns brought up there. Namely, this patch replaces CXXNewInitializationStyle::Implicit with a dedicated HasInitializer flag. This makes CXXNewInitializationStyle to model syntax again. This patch also renames Call and List to less confusing Parens and Braces.

This is a follow up to llvm#71417 , which aims to resolve concerns brought up there. Namely, this patch replaces `CXXNewInitializationStyle::Implicit` with a dedicated `HasInitializer` flag. This makes `CXXNewInitializationStyle` to model syntax again. This patch also renames `Call` and `List` to less confusing `Parens` and `Braces`.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang-tidy clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Jan 19, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 19, 2024

@llvm/pr-subscribers-clang-tools-extra
@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-modules

Author: Vlad Serebrennikov (Endilll)

Changes

This is a follow up to #71417 , which aims to resolve concerns brought up there. Namely, this patch replaces CXXNewInitializationStyle::Implicit with a dedicated HasInitializer flag. This makes CXXNewInitializationStyle to model syntax again. This patch also renames Call and List to less confusing Parens and Braces.


Full diff: https://github.com/llvm/llvm-project/pull/78793.diff

10 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp (+3-4)
  • (modified) clang/include/clang/AST/ExprCXX.h (+3-16)
  • (modified) clang/include/clang/AST/Stmt.h (+5-3)
  • (modified) clang/lib/AST/ExprCXX.cpp (+5-5)
  • (modified) clang/lib/AST/ItaniumMangle.cpp (+2-2)
  • (modified) clang/lib/AST/JSONNodeDumper.cpp (+2-3)
  • (modified) clang/lib/AST/StmtPrinter.cpp (+2-3)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+8-18)
  • (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+1)
  • (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+1)
diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
index 616e57efa76ded..d1d7e9dcfa9c0d 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -323,8 +323,7 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
     return false;
   };
   switch (New->getInitializationStyle()) {
-  case CXXNewInitializationStyle::None:
-  case CXXNewInitializationStyle::Implicit: {
+  case CXXNewInitializationStyle::None: {
     if (ArraySizeExpr.empty()) {
       Diag << FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd));
     } else {
@@ -335,7 +334,7 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
     }
     break;
   }
-  case CXXNewInitializationStyle::Call: {
+  case CXXNewInitializationStyle::Parens: {
     // FIXME: Add fixes for constructors with parameters that can be created
     // with a C++11 braced-init-list (e.g. std::vector, std::map).
     // Unlike ordinal cases, braced list can not be deduced in
@@ -372,7 +371,7 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
     }
     break;
   }
-  case CXXNewInitializationStyle::List: {
+  case CXXNewInitializationStyle::Braces: {
     // Range of the substring that we do not want to remove.
     SourceRange InitRange;
     if (const auto *NewConstruct = New->getConstructExpr()) {
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 24278016431837..9a7c632c36c5e8 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2210,14 +2210,11 @@ enum class CXXNewInitializationStyle {
   /// New-expression has no initializer as written.
   None,
 
-  /// New-expression has no written initializer, but has an implicit one.
-  Implicit,
-
   /// New-expression has a C++98 paren-delimited initializer.
-  Call,
+  Parens,
 
   /// New-expression has a C++11 list-initializer.
-  List
+  Braces
 };
 
 /// Represents a new-expression for memory allocation and constructor
@@ -2388,17 +2385,7 @@ class CXXNewExpr final
   bool isGlobalNew() const { return CXXNewExprBits.IsGlobalNew; }
 
   /// Whether this new-expression has any initializer at all.
-  bool hasInitializer() const {
-    switch (getInitializationStyle()) {
-    case CXXNewInitializationStyle::None:
-      return false;
-    case CXXNewInitializationStyle::Implicit:
-    case CXXNewInitializationStyle::Call:
-    case CXXNewInitializationStyle::List:
-      return true;
-    }
-    llvm_unreachable("Unknown initializer");
-  }
+  bool hasInitializer() const { return CXXNewExprBits.HasInitializer; }
 
   /// The kind of initializer this new-expression has.
   CXXNewInitializationStyle getInitializationStyle() const {
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index e1fde24e647789..55eca4007d17ea 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -868,9 +868,11 @@ class alignas(void *) Stmt {
     LLVM_PREFERRED_TYPE(bool)
     unsigned UsualArrayDeleteWantsSize : 1;
 
-    /// What kind of initializer do we have? Could be none, parens, or braces.
-    /// In storage, we distinguish between "none, and no initializer expr", and
-    /// "none, but an implicit initializer expr".
+    // Is initializer expr present?
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned HasInitializer : 1;
+
+    /// What kind of initializer syntax used? Could be none, parens, or braces.
     LLVM_PREFERRED_TYPE(CXXNewInitializationStyle)
     unsigned StoredInitializationStyle : 2;
 
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 83af7998f68338..e61c11dffd8841 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -194,14 +194,14 @@ CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
       DirectInitRange(DirectInitRange) {
 
   assert((Initializer != nullptr ||
-          InitializationStyle == CXXNewInitializationStyle::None ||
-          InitializationStyle == CXXNewInitializationStyle::Implicit) &&
-         "Only NoInit can have no initializer!");
+          InitializationStyle == CXXNewInitializationStyle::None) &&
+         "Only CXXNewInitializationStyle::None can have no initializer!");
 
   CXXNewExprBits.IsGlobalNew = IsGlobalNew;
   CXXNewExprBits.IsArray = ArraySize.has_value();
   CXXNewExprBits.ShouldPassAlignment = ShouldPassAlignment;
   CXXNewExprBits.UsualArrayDeleteWantsSize = UsualArrayDeleteWantsSize;
+  CXXNewExprBits.HasInitializer = Initializer != nullptr;
   CXXNewExprBits.StoredInitializationStyle =
       llvm::to_underlying(InitializationStyle);
   bool IsParenTypeId = TypeIdParens.isValid();
@@ -219,10 +219,10 @@ CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
     getTrailingObjects<SourceRange>()[0] = TypeIdParens;
 
   switch (getInitializationStyle()) {
-  case CXXNewInitializationStyle::Call:
+  case CXXNewInitializationStyle::Parens:
     this->Range.setEnd(DirectInitRange.getEnd());
     break;
-  case CXXNewInitializationStyle::List:
+  case CXXNewInitializationStyle::Braces:
     this->Range.setEnd(getInitializer()->getSourceRange().getEnd());
     break;
   default:
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b1678479888eb7..f78265df0713f5 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -4883,7 +4883,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
     Out << '_';
     mangleType(New->getAllocatedType());
     if (New->hasInitializer()) {
-      if (New->getInitializationStyle() == CXXNewInitializationStyle::List)
+      if (New->getInitializationStyle() == CXXNewInitializationStyle::Braces)
         Out << "il";
       else
         Out << "pi";
@@ -4898,7 +4898,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
         for (unsigned i = 0, e = PLE->getNumExprs(); i != e; ++i)
           mangleExpression(PLE->getExpr(i));
       } else if (New->getInitializationStyle() ==
-                     CXXNewInitializationStyle::List &&
+                     CXXNewInitializationStyle::Braces &&
                  isa<InitListExpr>(Init)) {
         // Only take InitListExprs apart for list-initialization.
         mangleInitListElements(cast<InitListExpr>(Init));
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index cb3d3b0df1ee34..3daba13d0fce7b 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -1356,12 +1356,11 @@ void JSONNodeDumper::VisitCXXNewExpr(const CXXNewExpr *NE) {
   attributeOnlyIfTrue("isPlacement", NE->getNumPlacementArgs() != 0);
   switch (NE->getInitializationStyle()) {
   case CXXNewInitializationStyle::None:
-  case CXXNewInitializationStyle::Implicit:
     break;
-  case CXXNewInitializationStyle::Call:
+  case CXXNewInitializationStyle::Parens:
     JOS.attribute("initStyle", "call");
     break;
-  case CXXNewInitializationStyle::List:
+  case CXXNewInitializationStyle::Braces:
     JOS.attribute("initStyle", "list");
     break;
   }
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index c04cb313c3387a..9d4aa07ec4da74 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2300,9 +2300,8 @@ void StmtPrinter::VisitCXXNewExpr(CXXNewExpr *E) {
     OS << ")";
 
   CXXNewInitializationStyle InitStyle = E->getInitializationStyle();
-  if (InitStyle != CXXNewInitializationStyle::None &&
-      InitStyle != CXXNewInitializationStyle::Implicit) {
-    bool Bare = InitStyle == CXXNewInitializationStyle::Call &&
+  if (InitStyle != CXXNewInitializationStyle::None) {
+    bool Bare = InitStyle == CXXNewInitializationStyle::Parens &&
                 !isa<ParenListExpr>(E->getInitializer());
     if (Bare)
       OS << "(";
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 8b6a80b45b27b1..482afb5a677ec5 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1957,7 +1957,7 @@ static bool isLegalArrayNewInitializer(CXXNewInitializationStyle Style,
   else if (CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(Init))
     return !CCE->isListInitialization() &&
            CCE->getConstructor()->isDefaultConstructor();
-  else if (Style == CXXNewInitializationStyle::List) {
+  else if (Style == CXXNewInitializationStyle::Braces) {
     assert(isa<InitListExpr>(Init) &&
            "Shouldn't create list CXXConstructExprs for arrays.");
     return true;
@@ -2011,9 +2011,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
   CXXNewInitializationStyle InitStyle;
   if (DirectInitRange.isValid()) {
     assert(Initializer && "Have parens but no initializer.");
-    InitStyle = CXXNewInitializationStyle::Call;
+    InitStyle = CXXNewInitializationStyle::Parens;
   } else if (Initializer && isa<InitListExpr>(Initializer))
-    InitStyle = CXXNewInitializationStyle::List;
+    InitStyle = CXXNewInitializationStyle::Braces;
   else {
     assert((!Initializer || isa<ImplicitValueInitExpr>(Initializer) ||
             isa<CXXConstructExpr>(Initializer)) &&
@@ -2023,7 +2023,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
 
   MultiExprArg Exprs(&Initializer, Initializer ? 1 : 0);
   if (ParenListExpr *List = dyn_cast_or_null<ParenListExpr>(Initializer)) {
-    assert(InitStyle == CXXNewInitializationStyle::Call &&
+    assert(InitStyle == CXXNewInitializationStyle::Parens &&
            "paren init for non-call init");
     Exprs = MultiExprArg(List->getExprs(), List->getNumExprs());
   }
@@ -2037,15 +2037,14 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
     //       initialized (8.5); if no initialization is performed,
     //       the object has indeterminate value
     case CXXNewInitializationStyle::None:
-    case CXXNewInitializationStyle::Implicit:
       return InitializationKind::CreateDefault(TypeRange.getBegin());
     //     - Otherwise, the new-initializer is interpreted according to the
     //       initialization rules of 8.5 for direct-initialization.
-    case CXXNewInitializationStyle::Call:
+    case CXXNewInitializationStyle::Parens:
       return InitializationKind::CreateDirect(TypeRange.getBegin(),
                                               DirectInitRange.getBegin(),
                                               DirectInitRange.getEnd());
-    case CXXNewInitializationStyle::List:
+    case CXXNewInitializationStyle::Braces:
       return InitializationKind::CreateDirectList(TypeRange.getBegin(),
                                                   Initializer->getBeginLoc(),
                                                   Initializer->getEndLoc());
@@ -2072,14 +2071,13 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
       return ExprError();
   } else if (Deduced && !Deduced->isDeduced()) {
     MultiExprArg Inits = Exprs;
-    bool Braced = (InitStyle == CXXNewInitializationStyle::List);
+    bool Braced = (InitStyle == CXXNewInitializationStyle::Braces);
     if (Braced) {
       auto *ILE = cast<InitListExpr>(Exprs[0]);
       Inits = MultiExprArg(ILE->getInits(), ILE->getNumInits());
     }
 
-    if (InitStyle == CXXNewInitializationStyle::None ||
-        InitStyle == CXXNewInitializationStyle::Implicit || Inits.empty())
+    if (InitStyle == CXXNewInitializationStyle::None || Inits.empty())
       return ExprError(Diag(StartLoc, diag::err_auto_new_requires_ctor_arg)
                        << AllocType << TypeRange);
     if (Inits.size() > 1) {
@@ -2447,14 +2445,6 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
       FullInit = Binder->getSubExpr();
 
     Initializer = FullInit.get();
-    // We don't know that we're generating an implicit initializer until now, so
-    // we have to update the initialization style as well.
-    //
-    // FIXME: it would be nice to determine the correct initialization style
-    // earlier so InitStyle doesn't need adjusting.
-    if (InitStyle == CXXNewInitializationStyle::None && Initializer) {
-      InitStyle = CXXNewInitializationStyle::Implicit;
-    }
 
     // FIXME: If we have a KnownArraySize, check that the array bound of the
     // initializer is no greater than that constant value.
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 21aed570ba26ce..85ecfa1a1a0bf2 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1901,6 +1901,7 @@ void ASTStmtReader::VisitCXXNewExpr(CXXNewExpr *E) {
   E->CXXNewExprBits.IsGlobalNew = Record.readInt();
   E->CXXNewExprBits.ShouldPassAlignment = Record.readInt();
   E->CXXNewExprBits.UsualArrayDeleteWantsSize = Record.readInt();
+  E->CXXNewExprBits.HasInitializer = Record.readInt();
   E->CXXNewExprBits.StoredInitializationStyle = Record.readInt();
 
   assert((IsArray == E->isArray()) && "Wrong IsArray!");
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 7f888e44dde1e0..e5836f5dcbe955 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1901,6 +1901,7 @@ void ASTStmtWriter::VisitCXXNewExpr(CXXNewExpr *E) {
   Record.push_back(E->isGlobalNew());
   Record.push_back(E->passAlignment());
   Record.push_back(E->doesUsualArrayDeleteWantSize());
+  Record.push_back(E->CXXNewExprBits.HasInitializer);
   Record.push_back(E->CXXNewExprBits.StoredInitializationStyle);
 
   Record.AddDeclRef(E->getOperatorNew());

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 19, 2024

@llvm/pr-subscribers-clang

Author: Vlad Serebrennikov (Endilll)

Changes

This is a follow up to #71417 , which aims to resolve concerns brought up there. Namely, this patch replaces CXXNewInitializationStyle::Implicit with a dedicated HasInitializer flag. This makes CXXNewInitializationStyle to model syntax again. This patch also renames Call and List to less confusing Parens and Braces.


Full diff: https://github.com/llvm/llvm-project/pull/78793.diff

10 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp (+3-4)
  • (modified) clang/include/clang/AST/ExprCXX.h (+3-16)
  • (modified) clang/include/clang/AST/Stmt.h (+5-3)
  • (modified) clang/lib/AST/ExprCXX.cpp (+5-5)
  • (modified) clang/lib/AST/ItaniumMangle.cpp (+2-2)
  • (modified) clang/lib/AST/JSONNodeDumper.cpp (+2-3)
  • (modified) clang/lib/AST/StmtPrinter.cpp (+2-3)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+8-18)
  • (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+1)
  • (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+1)
diff --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
index 616e57efa76ded..d1d7e9dcfa9c0d 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -323,8 +323,7 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
     return false;
   };
   switch (New->getInitializationStyle()) {
-  case CXXNewInitializationStyle::None:
-  case CXXNewInitializationStyle::Implicit: {
+  case CXXNewInitializationStyle::None: {
     if (ArraySizeExpr.empty()) {
       Diag << FixItHint::CreateRemoval(SourceRange(NewStart, NewEnd));
     } else {
@@ -335,7 +334,7 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
     }
     break;
   }
-  case CXXNewInitializationStyle::Call: {
+  case CXXNewInitializationStyle::Parens: {
     // FIXME: Add fixes for constructors with parameters that can be created
     // with a C++11 braced-init-list (e.g. std::vector, std::map).
     // Unlike ordinal cases, braced list can not be deduced in
@@ -372,7 +371,7 @@ bool MakeSmartPtrCheck::replaceNew(DiagnosticBuilder &Diag,
     }
     break;
   }
-  case CXXNewInitializationStyle::List: {
+  case CXXNewInitializationStyle::Braces: {
     // Range of the substring that we do not want to remove.
     SourceRange InitRange;
     if (const auto *NewConstruct = New->getConstructExpr()) {
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 24278016431837..9a7c632c36c5e8 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2210,14 +2210,11 @@ enum class CXXNewInitializationStyle {
   /// New-expression has no initializer as written.
   None,
 
-  /// New-expression has no written initializer, but has an implicit one.
-  Implicit,
-
   /// New-expression has a C++98 paren-delimited initializer.
-  Call,
+  Parens,
 
   /// New-expression has a C++11 list-initializer.
-  List
+  Braces
 };
 
 /// Represents a new-expression for memory allocation and constructor
@@ -2388,17 +2385,7 @@ class CXXNewExpr final
   bool isGlobalNew() const { return CXXNewExprBits.IsGlobalNew; }
 
   /// Whether this new-expression has any initializer at all.
-  bool hasInitializer() const {
-    switch (getInitializationStyle()) {
-    case CXXNewInitializationStyle::None:
-      return false;
-    case CXXNewInitializationStyle::Implicit:
-    case CXXNewInitializationStyle::Call:
-    case CXXNewInitializationStyle::List:
-      return true;
-    }
-    llvm_unreachable("Unknown initializer");
-  }
+  bool hasInitializer() const { return CXXNewExprBits.HasInitializer; }
 
   /// The kind of initializer this new-expression has.
   CXXNewInitializationStyle getInitializationStyle() const {
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index e1fde24e647789..55eca4007d17ea 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -868,9 +868,11 @@ class alignas(void *) Stmt {
     LLVM_PREFERRED_TYPE(bool)
     unsigned UsualArrayDeleteWantsSize : 1;
 
-    /// What kind of initializer do we have? Could be none, parens, or braces.
-    /// In storage, we distinguish between "none, and no initializer expr", and
-    /// "none, but an implicit initializer expr".
+    // Is initializer expr present?
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned HasInitializer : 1;
+
+    /// What kind of initializer syntax used? Could be none, parens, or braces.
     LLVM_PREFERRED_TYPE(CXXNewInitializationStyle)
     unsigned StoredInitializationStyle : 2;
 
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 83af7998f68338..e61c11dffd8841 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -194,14 +194,14 @@ CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
       DirectInitRange(DirectInitRange) {
 
   assert((Initializer != nullptr ||
-          InitializationStyle == CXXNewInitializationStyle::None ||
-          InitializationStyle == CXXNewInitializationStyle::Implicit) &&
-         "Only NoInit can have no initializer!");
+          InitializationStyle == CXXNewInitializationStyle::None) &&
+         "Only CXXNewInitializationStyle::None can have no initializer!");
 
   CXXNewExprBits.IsGlobalNew = IsGlobalNew;
   CXXNewExprBits.IsArray = ArraySize.has_value();
   CXXNewExprBits.ShouldPassAlignment = ShouldPassAlignment;
   CXXNewExprBits.UsualArrayDeleteWantsSize = UsualArrayDeleteWantsSize;
+  CXXNewExprBits.HasInitializer = Initializer != nullptr;
   CXXNewExprBits.StoredInitializationStyle =
       llvm::to_underlying(InitializationStyle);
   bool IsParenTypeId = TypeIdParens.isValid();
@@ -219,10 +219,10 @@ CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
     getTrailingObjects<SourceRange>()[0] = TypeIdParens;
 
   switch (getInitializationStyle()) {
-  case CXXNewInitializationStyle::Call:
+  case CXXNewInitializationStyle::Parens:
     this->Range.setEnd(DirectInitRange.getEnd());
     break;
-  case CXXNewInitializationStyle::List:
+  case CXXNewInitializationStyle::Braces:
     this->Range.setEnd(getInitializer()->getSourceRange().getEnd());
     break;
   default:
diff --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index b1678479888eb7..f78265df0713f5 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -4883,7 +4883,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
     Out << '_';
     mangleType(New->getAllocatedType());
     if (New->hasInitializer()) {
-      if (New->getInitializationStyle() == CXXNewInitializationStyle::List)
+      if (New->getInitializationStyle() == CXXNewInitializationStyle::Braces)
         Out << "il";
       else
         Out << "pi";
@@ -4898,7 +4898,7 @@ void CXXNameMangler::mangleExpression(const Expr *E, unsigned Arity,
         for (unsigned i = 0, e = PLE->getNumExprs(); i != e; ++i)
           mangleExpression(PLE->getExpr(i));
       } else if (New->getInitializationStyle() ==
-                     CXXNewInitializationStyle::List &&
+                     CXXNewInitializationStyle::Braces &&
                  isa<InitListExpr>(Init)) {
         // Only take InitListExprs apart for list-initialization.
         mangleInitListElements(cast<InitListExpr>(Init));
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index cb3d3b0df1ee34..3daba13d0fce7b 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -1356,12 +1356,11 @@ void JSONNodeDumper::VisitCXXNewExpr(const CXXNewExpr *NE) {
   attributeOnlyIfTrue("isPlacement", NE->getNumPlacementArgs() != 0);
   switch (NE->getInitializationStyle()) {
   case CXXNewInitializationStyle::None:
-  case CXXNewInitializationStyle::Implicit:
     break;
-  case CXXNewInitializationStyle::Call:
+  case CXXNewInitializationStyle::Parens:
     JOS.attribute("initStyle", "call");
     break;
-  case CXXNewInitializationStyle::List:
+  case CXXNewInitializationStyle::Braces:
     JOS.attribute("initStyle", "list");
     break;
   }
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index c04cb313c3387a..9d4aa07ec4da74 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2300,9 +2300,8 @@ void StmtPrinter::VisitCXXNewExpr(CXXNewExpr *E) {
     OS << ")";
 
   CXXNewInitializationStyle InitStyle = E->getInitializationStyle();
-  if (InitStyle != CXXNewInitializationStyle::None &&
-      InitStyle != CXXNewInitializationStyle::Implicit) {
-    bool Bare = InitStyle == CXXNewInitializationStyle::Call &&
+  if (InitStyle != CXXNewInitializationStyle::None) {
+    bool Bare = InitStyle == CXXNewInitializationStyle::Parens &&
                 !isa<ParenListExpr>(E->getInitializer());
     if (Bare)
       OS << "(";
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 8b6a80b45b27b1..482afb5a677ec5 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1957,7 +1957,7 @@ static bool isLegalArrayNewInitializer(CXXNewInitializationStyle Style,
   else if (CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(Init))
     return !CCE->isListInitialization() &&
            CCE->getConstructor()->isDefaultConstructor();
-  else if (Style == CXXNewInitializationStyle::List) {
+  else if (Style == CXXNewInitializationStyle::Braces) {
     assert(isa<InitListExpr>(Init) &&
            "Shouldn't create list CXXConstructExprs for arrays.");
     return true;
@@ -2011,9 +2011,9 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
   CXXNewInitializationStyle InitStyle;
   if (DirectInitRange.isValid()) {
     assert(Initializer && "Have parens but no initializer.");
-    InitStyle = CXXNewInitializationStyle::Call;
+    InitStyle = CXXNewInitializationStyle::Parens;
   } else if (Initializer && isa<InitListExpr>(Initializer))
-    InitStyle = CXXNewInitializationStyle::List;
+    InitStyle = CXXNewInitializationStyle::Braces;
   else {
     assert((!Initializer || isa<ImplicitValueInitExpr>(Initializer) ||
             isa<CXXConstructExpr>(Initializer)) &&
@@ -2023,7 +2023,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
 
   MultiExprArg Exprs(&Initializer, Initializer ? 1 : 0);
   if (ParenListExpr *List = dyn_cast_or_null<ParenListExpr>(Initializer)) {
-    assert(InitStyle == CXXNewInitializationStyle::Call &&
+    assert(InitStyle == CXXNewInitializationStyle::Parens &&
            "paren init for non-call init");
     Exprs = MultiExprArg(List->getExprs(), List->getNumExprs());
   }
@@ -2037,15 +2037,14 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
     //       initialized (8.5); if no initialization is performed,
     //       the object has indeterminate value
     case CXXNewInitializationStyle::None:
-    case CXXNewInitializationStyle::Implicit:
       return InitializationKind::CreateDefault(TypeRange.getBegin());
     //     - Otherwise, the new-initializer is interpreted according to the
     //       initialization rules of 8.5 for direct-initialization.
-    case CXXNewInitializationStyle::Call:
+    case CXXNewInitializationStyle::Parens:
       return InitializationKind::CreateDirect(TypeRange.getBegin(),
                                               DirectInitRange.getBegin(),
                                               DirectInitRange.getEnd());
-    case CXXNewInitializationStyle::List:
+    case CXXNewInitializationStyle::Braces:
       return InitializationKind::CreateDirectList(TypeRange.getBegin(),
                                                   Initializer->getBeginLoc(),
                                                   Initializer->getEndLoc());
@@ -2072,14 +2071,13 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
       return ExprError();
   } else if (Deduced && !Deduced->isDeduced()) {
     MultiExprArg Inits = Exprs;
-    bool Braced = (InitStyle == CXXNewInitializationStyle::List);
+    bool Braced = (InitStyle == CXXNewInitializationStyle::Braces);
     if (Braced) {
       auto *ILE = cast<InitListExpr>(Exprs[0]);
       Inits = MultiExprArg(ILE->getInits(), ILE->getNumInits());
     }
 
-    if (InitStyle == CXXNewInitializationStyle::None ||
-        InitStyle == CXXNewInitializationStyle::Implicit || Inits.empty())
+    if (InitStyle == CXXNewInitializationStyle::None || Inits.empty())
       return ExprError(Diag(StartLoc, diag::err_auto_new_requires_ctor_arg)
                        << AllocType << TypeRange);
     if (Inits.size() > 1) {
@@ -2447,14 +2445,6 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
       FullInit = Binder->getSubExpr();
 
     Initializer = FullInit.get();
-    // We don't know that we're generating an implicit initializer until now, so
-    // we have to update the initialization style as well.
-    //
-    // FIXME: it would be nice to determine the correct initialization style
-    // earlier so InitStyle doesn't need adjusting.
-    if (InitStyle == CXXNewInitializationStyle::None && Initializer) {
-      InitStyle = CXXNewInitializationStyle::Implicit;
-    }
 
     // FIXME: If we have a KnownArraySize, check that the array bound of the
     // initializer is no greater than that constant value.
diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp
index 21aed570ba26ce..85ecfa1a1a0bf2 100644
--- a/clang/lib/Serialization/ASTReaderStmt.cpp
+++ b/clang/lib/Serialization/ASTReaderStmt.cpp
@@ -1901,6 +1901,7 @@ void ASTStmtReader::VisitCXXNewExpr(CXXNewExpr *E) {
   E->CXXNewExprBits.IsGlobalNew = Record.readInt();
   E->CXXNewExprBits.ShouldPassAlignment = Record.readInt();
   E->CXXNewExprBits.UsualArrayDeleteWantsSize = Record.readInt();
+  E->CXXNewExprBits.HasInitializer = Record.readInt();
   E->CXXNewExprBits.StoredInitializationStyle = Record.readInt();
 
   assert((IsArray == E->isArray()) && "Wrong IsArray!");
diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp
index 7f888e44dde1e0..e5836f5dcbe955 100644
--- a/clang/lib/Serialization/ASTWriterStmt.cpp
+++ b/clang/lib/Serialization/ASTWriterStmt.cpp
@@ -1901,6 +1901,7 @@ void ASTStmtWriter::VisitCXXNewExpr(CXXNewExpr *E) {
   Record.push_back(E->isGlobalNew());
   Record.push_back(E->passAlignment());
   Record.push_back(E->doesUsualArrayDeleteWantSize());
+  Record.push_back(E->CXXNewExprBits.HasInitializer);
   Record.push_back(E->CXXNewExprBits.StoredInitializationStyle);
 
   Record.AddDeclRef(E->getOperatorNew());

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Endilll Endilll merged commit cc3fd19 into llvm:main Jan 21, 2024
4 checks passed
@Endilll Endilll deleted the new-init-style branch January 21, 2024 20:42
Copy link
Contributor

@tomasz-kaminski-sonarsource tomasz-kaminski-sonarsource left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks.
(No ability to approve).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category clang-tidy clang-tools-extra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants