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] Add ::_placement_new expression for built-in global placement new #72209

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

MaxEW707
Copy link
Contributor

https://godbolt.org/z/7Khh33fxo for reference

Background

I work on a large software project where debug performance and compile-time performance are considered high priority.
Clang has already done a lot in this area which we very much appreciate especially since a lot of C++ constructs that should be language level constructs are implemented in code instead. We always appreciate builtins over function calls or template meta-programming for example.

We use clang on a variety of platforms some which do not use libcxx and some which have their own platform vendor stl implementations.
The project I work on also has our own stl generic library with our own differences, extensions, additions, removals from std, etc.
However sometimes we have to interface with vendor code or third-party libs that expose a std interface or include std headers.

Allocations in our project are tightly controlled and objects may be allocated from a variety of heaps or scratch buffers.
We do not use global new or class specific new.
Therefore we rely heavily on the reserved global placement new to construct objects in memory.

Problem

In general <new> is expensive to include and also brings in a lot of transitive includes.
libcxx has a config option to remove some of these: https://github.com/llvm/llvm-project/blob/main/libcxx/include/new#L369
However our software builds on a variety of stl implementations which we cannot control.
We avoid including stl headers however when we need to interface with vendor code in platform cpp files sometimes they get included from vendor headers.

Clang already treats the reserved global placement new as a builtin if it is declared.
We forward declare this operator new on platforms that use clang but do not ship with libcxx.

libcxx puts the abi_tag attribute on its declaration. This means if <new> is included after our declaration we get a compiler error since abi_tag cannot be re-declared.
I tried to first get a PR up to libcxx here but it was deemed that we shouldn't work around the std.

Constructing objects in memory is such a vital part of C++ and I believe that this support should be a zero-cost abstraction in the language.

Solution

I decided to add a clang specific ::_placement_new expression that is treated as a builtin.
This expression does the equivalent of the reserved global placement new expression without requiring library support.

This route had a couple extra benefits as well which are absolutely no header includes and absolutely no overload resolution to find the best operator new function for the new expression.
This is in addition to the already intended benefits of no debug overhead, no linker symbols and no debug symbols.

If this solution is not viable I would like to try to come up with a solution that gives us all the benefits of clangs builtin support for the reserved global placement new operator without having to work around different std implementations.
While I ran into an issue with libcxx, as noted above we use clang and vendor forks of clang with vendor specific stl implementations, we may very well run into some issue with another vendor stl implementation on clang in the future.

… new without header includes, overload resolution or dealing with std libraries that declare the global placement new with attributes that cannot be redeclared.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang:codegen clang:static analyzer labels Nov 14, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 14, 2023

@llvm/pr-subscribers-clang-static-analyzer-1
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: None (MaxEW707)

Changes

https://godbolt.org/z/7Khh33fxo for reference

Background

I work on a large software project where debug performance and compile-time performance are considered high priority.
Clang has already done a lot in this area which we very much appreciate especially since a lot of C++ constructs that should be language level constructs are implemented in code instead. We always appreciate builtins over function calls or template meta-programming for example.

We use clang on a variety of platforms some which do not use libcxx and some which have their own platform vendor stl implementations.
The project I work on also has our own stl generic library with our own differences, extensions, additions, removals from std, etc.
However sometimes we have to interface with vendor code or third-party libs that expose a std interface or include std headers.

Allocations in our project are tightly controlled and objects may be allocated from a variety of heaps or scratch buffers.
We do not use global new or class specific new.
Therefore we rely heavily on the reserved global placement new to construct objects in memory.

Problem

In general &lt;new&gt; is expensive to include and also brings in a lot of transitive includes.
libcxx has a config option to remove some of these: https://github.com/llvm/llvm-project/blob/main/libcxx/include/new#L369
However our software builds on a variety of stl implementations which we cannot control.
We avoid including stl headers however when we need to interface with vendor code in platform cpp files sometimes they get included from vendor headers.

Clang already treats the reserved global placement new as a builtin if it is declared.
We forward declare this operator new on platforms that use clang but do not ship with libcxx.

libcxx puts the abi_tag attribute on its declaration. This means if &lt;new&gt; is included after our declaration we get a compiler error since abi_tag cannot be re-declared.
I tried to first get a PR up to libcxx here but it was deemed that we shouldn't work around the std.

Constructing objects in memory is such a vital part of C++ and I believe that this support should be a zero-cost abstraction in the language.

Solution

I decided to add a clang specific ::_placement_new expression that is treated as a builtin.
This expression does the equivalent of the reserved global placement new expression without requiring library support.

This route had a couple extra benefits as well which are absolutely no header includes and absolutely no overload resolution to find the best operator new function for the new expression.
This is in addition to the already intended benefits of no debug overhead, no linker symbols and no debug symbols.

If this solution is not viable I would like to try to come up with a solution that gives us all the benefits of clangs builtin support for the reserved global placement new operator without having to work around different std implementations.
While I ran into an issue with libcxx, as noted above we use clang and vendor forks of clang with vendor specific stl implementations, we may very well run into some issue with another vendor stl implementation on clang in the future.


Patch is 39.32 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72209.diff

29 Files Affected:

  • (modified) clang/include/clang/AST/ExprCXX.h (+23)
  • (modified) clang/include/clang/AST/Stmt.h (+4)
  • (modified) clang/include/clang/Basic/DiagnosticParseKinds.td (+2)
  • (modified) clang/include/clang/Basic/TokenKinds.def (+1)
  • (modified) clang/include/clang/Sema/Sema.h (+2-2)
  • (modified) clang/lib/AST/ASTImporter.cpp (+12-6)
  • (modified) clang/lib/AST/ExprCXX.cpp (+34)
  • (modified) clang/lib/AST/ExprConstant.cpp (+3-4)
  • (modified) clang/lib/AST/JSONNodeDumper.cpp (+1)
  • (modified) clang/lib/AST/StmtPrinter.cpp (+4-1)
  • (modified) clang/lib/AST/StmtProfile.cpp (+1)
  • (modified) clang/lib/AST/TextNodeDumper.cpp (+2)
  • (modified) clang/lib/CodeGen/CGExprCXX.cpp (+4-4)
  • (modified) clang/lib/CodeGen/CodeGenModule.cpp (+4-1)
  • (modified) clang/lib/Interpreter/Interpreter.cpp (+2-2)
  • (modified) clang/lib/Parse/ParseDecl.cpp (+4-2)
  • (modified) clang/lib/Parse/ParseExpr.cpp (+1-1)
  • (modified) clang/lib/Parse/ParseExprCXX.cpp (+16-3)
  • (modified) clang/lib/Parse/ParseObjc.cpp (+1)
  • (modified) clang/lib/Parse/ParseTentative.cpp (+2-1)
  • (modified) clang/lib/Sema/SemaExprCXX.cpp (+35-13)
  • (modified) clang/lib/Sema/TreeTransform.h (+3-3)
  • (modified) clang/lib/Serialization/ASTReaderStmt.cpp (+2)
  • (modified) clang/lib/Serialization/ASTWriterStmt.cpp (+2)
  • (modified) clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp (+1-1)
  • (modified) clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (+5-4)
  • (modified) clang/test/AST/ast-dump-expr.cpp (+14)
  • (modified) clang/test/CodeGenCXX/new.cpp (+42-2)
  • (modified) clang/test/SemaCXX/new-delete.cpp (+7)
diff --git a/clang/include/clang/AST/ExprCXX.h b/clang/include/clang/AST/ExprCXX.h
index 24278016431837b..d760af796aea28f 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -2282,6 +2282,13 @@ class CXXNewExpr final
              QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
              SourceRange DirectInitRange);
 
+  /// Build a c++ builtin placement new expression
+  CXXNewExpr(Expr *PlacementArg,
+             SourceRange TypeIdParens, std::optional<Expr *> ArraySize,
+             CXXNewInitializationStyle InitializationStyle, Expr *Initializer,
+             QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+             SourceRange DirectInitRange);
+
   /// Build an empty c++ new expression.
   CXXNewExpr(EmptyShell Empty, bool IsArray, unsigned NumPlacementArgs,
              bool IsParenTypeId);
@@ -2297,6 +2304,14 @@ class CXXNewExpr final
          QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
          SourceRange DirectInitRange);
 
+  /// Create a c++ builtin placement new expression.
+  static CXXNewExpr *
+  CreatePlacementNew(const ASTContext &Ctx, Expr *PlacementArg,
+                     SourceRange TypeIdParens, std::optional<Expr *> ArraySize,
+                     CXXNewInitializationStyle InitializationStyle, Expr *Initializer,
+                     QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+                     SourceRange DirectInitRange);
+
   /// Create an empty c++ new expression.
   static CXXNewExpr *CreateEmpty(const ASTContext &Ctx, bool IsArray,
                                  bool HasInit, unsigned NumPlacementArgs,
@@ -2332,6 +2347,12 @@ class CXXNewExpr final
   FunctionDecl *getOperatorDelete() const { return OperatorDelete; }
   void setOperatorDelete(FunctionDecl *D) { OperatorDelete = D; }
 
+  bool isReservedPlacementNew() const {
+    if (CXXNewExprBits.IsPlacementNewExpr)
+      return true;
+    return OperatorNew->isReservedGlobalPlacementOperator();
+  }
+
   bool isArray() const { return CXXNewExprBits.IsArray; }
 
   /// This might return std::nullopt even if isArray() returns true,
@@ -2387,6 +2408,8 @@ class CXXNewExpr final
 
   bool isGlobalNew() const { return CXXNewExprBits.IsGlobalNew; }
 
+  bool isPlacementNewExpr() const { return CXXNewExprBits.IsPlacementNewExpr; }
+
   /// Whether this new-expression has any initializer at all.
   bool hasInitializer() const {
     switch (getInitializationStyle()) {
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index da7b37ce0e1211f..2dc3746aee4fd71 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -878,6 +878,10 @@ class alignas(void *) Stmt {
     LLVM_PREFERRED_TYPE(bool)
     unsigned IsParenTypeId : 1;
 
+    /// True is this if the builtin placement-new expression.
+    LLVM_PREFERRED_TYPE(bool)
+    unsigned IsPlacementNewExpr : 1;
+
     /// The number of placement new arguments.
     unsigned NumPlacementArgs;
   };
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index de180344fcc5c74..c0ca7529fda892d 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -255,6 +255,8 @@ def err_expected_lbrace_in_compound_literal : Error<
   "expected '{' in compound literal">;
 def err_expected_while : Error<"expected 'while' in do/while loop">;
 
+def err_placement_new_expected_one_argument : Error<"expected only one argument in placement params">;
+
 def err_expected_semi_after_stmt : Error<"expected ';' after %0 statement">;
 def err_expected_semi_after_expr : Error<"expected ';' after expression">;
 def err_extraneous_token_before_semi : Error<"extraneous '%0' before ';'">;
diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 3ce317d318f9bb6..a2dfc1a9e08691a 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -365,6 +365,7 @@ KEYWORD(typeid                      , KEYCXX)
 KEYWORD(using                       , KEYCXX)
 KEYWORD(virtual                     , KEYCXX)
 KEYWORD(wchar_t                     , WCHARSUPPORT)
+KEYWORD(_placement_new              , KEYCXX)
 
 // C++ 2.5p2: Alternative Representations.
 CXX_KEYWORD_OPERATOR(and     , ampamp)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f69f366c1750918..ab9a0d0713435b6 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6810,14 +6810,14 @@ class Sema final {
                                        bool ListInitialization);
 
   /// ActOnCXXNew - Parsed a C++ 'new' expression.
-  ExprResult ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
+  ExprResult ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal, bool IsPlacementNewExpr,
                          SourceLocation PlacementLParen,
                          MultiExprArg PlacementArgs,
                          SourceLocation PlacementRParen,
                          SourceRange TypeIdParens, Declarator &D,
                          Expr *Initializer);
   ExprResult
-  BuildCXXNew(SourceRange Range, bool UseGlobal, SourceLocation PlacementLParen,
+  BuildCXXNew(SourceRange Range, bool UseGlobal, bool IsPlacementNewExpr, SourceLocation PlacementLParen,
               MultiExprArg PlacementArgs, SourceLocation PlacementRParen,
               SourceRange TypeIdParens, QualType AllocType,
               TypeSourceInfo *AllocTypeInfo, std::optional<Expr *> ArraySize,
diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index c4e931e220f69b5..3204fb69b3440f7 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -8102,12 +8102,18 @@ ExpectedStmt ASTNodeImporter::VisitCXXNewExpr(CXXNewExpr *E) {
       ImportContainerChecked(E->placement_arguments(), ToPlacementArgs))
     return std::move(Err);
 
-  return CXXNewExpr::Create(
-      Importer.getToContext(), E->isGlobalNew(), ToOperatorNew,
-      ToOperatorDelete, E->passAlignment(), E->doesUsualArrayDeleteWantSize(),
-      ToPlacementArgs, ToTypeIdParens, ToArraySize, E->getInitializationStyle(),
-      ToInitializer, ToType, ToAllocatedTypeSourceInfo, ToSourceRange,
-      ToDirectInitRange);
+  if (E->isPlacementNewExpr())
+    return CXXNewExpr::CreatePlacementNew(
+        Importer.getToContext(), ToPlacementArgs[0], ToTypeIdParens, ToArraySize,
+        E->getInitializationStyle(), ToInitializer, ToType, ToAllocatedTypeSourceInfo,
+        ToSourceRange, ToDirectInitRange);
+  else
+    return CXXNewExpr::Create(
+        Importer.getToContext(), E->isGlobalNew(), ToOperatorNew,
+        ToOperatorDelete, E->passAlignment(), E->doesUsualArrayDeleteWantSize(),
+        ToPlacementArgs, ToTypeIdParens, ToArraySize, E->getInitializationStyle(),
+        ToInitializer, ToType, ToAllocatedTypeSourceInfo, ToSourceRange,
+        ToDirectInitRange);
 }
 
 ExpectedStmt ASTNodeImporter::VisitCXXDeleteExpr(CXXDeleteExpr *E) {
diff --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index 83af7998f683382..7c7f4c2f31542b9 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -207,6 +207,7 @@ CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
   bool IsParenTypeId = TypeIdParens.isValid();
   CXXNewExprBits.IsParenTypeId = IsParenTypeId;
   CXXNewExprBits.NumPlacementArgs = PlacementArgs.size();
+  CXXNewExprBits.IsPlacementNewExpr = false;
 
   if (ArraySize)
     getTrailingObjects<Stmt *>()[arraySizeOffset()] = *ArraySize;
@@ -234,6 +235,18 @@ CXXNewExpr::CXXNewExpr(bool IsGlobalNew, FunctionDecl *OperatorNew,
   setDependence(computeDependence(this));
 }
 
+CXXNewExpr::CXXNewExpr(Expr *PlacementArg,
+                       SourceRange TypeIdParens, std::optional<Expr *> ArraySize,
+                       CXXNewInitializationStyle InitializationStyle, Expr *Initializer,
+                       QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+                       SourceRange DirectInitRange)
+    : CXXNewExpr(true, nullptr, nullptr, false, false,
+                 ArrayRef<Expr *>(&PlacementArg, 1), TypeIdParens, ArraySize,
+                 InitializationStyle, Initializer, Ty,
+                 AllocatedTypeInfo, Range, DirectInitRange) {
+    CXXNewExprBits.IsPlacementNewExpr = true;
+}
+
 CXXNewExpr::CXXNewExpr(EmptyShell Empty, bool IsArray,
                        unsigned NumPlacementArgs, bool IsParenTypeId)
     : Expr(CXXNewExprClass, Empty) {
@@ -265,6 +278,25 @@ CXXNewExpr *CXXNewExpr::Create(
                  AllocatedTypeInfo, Range, DirectInitRange);
 }
 
+CXXNewExpr *CXXNewExpr::CreatePlacementNew(
+    const ASTContext &Ctx, Expr *PlacementArg,
+    SourceRange TypeIdParens, std::optional<Expr *> ArraySize,
+    CXXNewInitializationStyle InitializationStyle, Expr *Initializer,
+    QualType Ty, TypeSourceInfo *AllocatedTypeInfo, SourceRange Range,
+    SourceRange DirectInitRange) {
+  bool IsArray = ArraySize.has_value();
+  bool HasInit = Initializer != nullptr;
+  bool IsParenTypeId = TypeIdParens.isValid();
+  void *Mem =
+      Ctx.Allocate(totalSizeToAlloc<Stmt *, SourceRange>(
+                       IsArray + HasInit + 1, IsParenTypeId),
+                   alignof(CXXNewExpr));
+  return new (Mem)
+      CXXNewExpr(PlacementArg, TypeIdParens,
+                 ArraySize, InitializationStyle, Initializer, Ty,
+                 AllocatedTypeInfo, Range, DirectInitRange);
+}
+
 CXXNewExpr *CXXNewExpr::CreateEmpty(const ASTContext &Ctx, bool IsArray,
                                     bool HasInit, unsigned NumPlacementArgs,
                                     bool IsParenTypeId) {
@@ -277,6 +309,8 @@ CXXNewExpr *CXXNewExpr::CreateEmpty(const ASTContext &Ctx, bool IsArray,
 }
 
 bool CXXNewExpr::shouldNullCheckAllocation() const {
+  if (isPlacementNewExpr())
+    return false;
   if (getOperatorNew()->getLangOpts().CheckNew)
     return true;
   return !getOperatorNew()->hasAttr<ReturnsNonNullAttr>() &&
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index e16fec6109e744e..7652e22a6a51b4f 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -9740,7 +9740,7 @@ static bool EvaluateArrayNewConstructExpr(EvalInfo &Info, LValue &This,
                                           QualType AllocType);
 
 bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
-  if (!Info.getLangOpts().CPlusPlus20)
+  if (!Info.getLangOpts().CPlusPlus20 && !E->isPlacementNewExpr())
     Info.CCEDiag(E, diag::note_constexpr_new);
 
   // We cannot speculatively evaluate a delete expression.
@@ -9751,8 +9751,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
 
   bool IsNothrow = false;
   bool IsPlacement = false;
-  if (OperatorNew->isReservedGlobalPlacementOperator() &&
-      Info.CurrentCall->isStdFunction() && !E->isArray()) {
+  if (E->isReservedPlacementNew() && Info.CurrentCall->isStdFunction() && !E->isArray()) {
     // FIXME Support array placement new.
     assert(E->getNumPlacementArgs() == 1);
     if (!EvaluatePointer(E->getPlacementArg(0), Result, Info))
@@ -9760,7 +9759,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
     if (Result.Designator.Invalid)
       return false;
     IsPlacement = true;
-  } else if (!OperatorNew->isReplaceableGlobalAllocationFunction()) {
+  } else if (E->isPlacementNewExpr() || !OperatorNew->isReplaceableGlobalAllocationFunction()) {
     Info.FFDiag(E, diag::note_constexpr_new_non_replaceable)
         << isa<CXXMethodDecl>(OperatorNew) << OperatorNew;
     return false;
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index ace5178bf625828..82c4f5e64416e21 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -1350,6 +1350,7 @@ void JSONNodeDumper::VisitCXXNewExpr(const CXXNewExpr *NE) {
   attributeOnlyIfTrue("isGlobal", NE->isGlobalNew());
   attributeOnlyIfTrue("isArray", NE->isArray());
   attributeOnlyIfTrue("isPlacement", NE->getNumPlacementArgs() != 0);
+  attributeOnlyIfTrue("isPlacementNewExpr", NE->isPlacementNewExpr());
   switch (NE->getInitializationStyle()) {
   case CXXNewInitializationStyle::None:
   case CXXNewInitializationStyle::Implicit:
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index ab4a013de5f552c..ce3d9b64de76ab0 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -2271,7 +2271,10 @@ void StmtPrinter::VisitCXXScalarValueInitExpr(CXXScalarValueInitExpr *Node) {
 void StmtPrinter::VisitCXXNewExpr(CXXNewExpr *E) {
   if (E->isGlobalNew())
     OS << "::";
-  OS << "new ";
+  if (E->isPlacementNewExpr())
+    OS << "_placement_new ";
+  else
+    OS << "new ";
   unsigned NumPlace = E->getNumPlacementArgs();
   if (NumPlace > 0 && !isa<CXXDefaultArgExpr>(E->getPlacementArg(0))) {
     OS << "(";
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index 8128219dd6f63c9..c3590f50eebfd2e 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2096,6 +2096,7 @@ void StmtProfiler::VisitCXXNewExpr(const CXXNewExpr *S) {
   ID.AddInteger(S->getNumPlacementArgs());
   ID.AddBoolean(S->isGlobalNew());
   ID.AddBoolean(S->isParenTypeId());
+  ID.AddBoolean(S->isPlacementNewExpr());
   ID.AddInteger(llvm::to_underlying(S->getInitializationStyle()));
 }
 
diff --git a/clang/lib/AST/TextNodeDumper.cpp b/clang/lib/AST/TextNodeDumper.cpp
index e8274fcd5cfe9cb..57b6b9087826355 100644
--- a/clang/lib/AST/TextNodeDumper.cpp
+++ b/clang/lib/AST/TextNodeDumper.cpp
@@ -1356,6 +1356,8 @@ void TextNodeDumper::VisitCXXNewExpr(const CXXNewExpr *Node) {
     OS << " global";
   if (Node->isArray())
     OS << " array";
+  if (Node->isPlacementNewExpr())
+    OS << " builtin placement-new expression";
   if (Node->getOperatorNew()) {
     OS << ' ';
     dumpBareDeclRef(Node->getOperatorNew());
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 98ae56e2df88184..cfc16f2ffd3ad04 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -693,7 +693,7 @@ static CharUnits CalculateCookiePadding(CodeGenFunction &CGF,
 
   // No cookie is required if the operator new[] being used is the
   // reserved placement operator new[].
-  if (E->getOperatorNew()->isReservedGlobalPlacementOperator())
+  if (E->isReservedPlacementNew())
     return CharUnits::Zero();
 
   return CGF.CGM.getCXXABI().GetArrayCookieSize(E);
@@ -1584,7 +1584,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
   // operator, just "inline" it directly.
   Address allocation = Address::invalid();
   CallArgList allocatorArgs;
-  if (allocator->isReservedGlobalPlacementOperator()) {
+  if (E->isReservedPlacementNew()) {
     assert(E->getNumPlacementArgs() == 1);
     const Expr *arg = *E->placement_arguments().begin();
 
@@ -1599,6 +1599,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
 
     // Set up allocatorArgs for the call to operator delete if it's not
     // the reserved global operator.
+    assert(!E->isPlacementNewExpr() || !E->getOperatorDelete());
     if (E->getOperatorDelete() &&
         !E->getOperatorDelete()->isReservedGlobalPlacementOperator()) {
       allocatorArgs.add(RValue::get(allocSize), getContext().getSizeType());
@@ -1724,8 +1725,7 @@ llvm::Value *CodeGenFunction::EmitCXXNewExpr(const CXXNewExpr *E) {
   // vptrs information which may be included in previous type.
   // To not break LTO with different optimizations levels, we do it regardless
   // of optimization level.
-  if (CGM.getCodeGenOpts().StrictVTablePointers &&
-      allocator->isReservedGlobalPlacementOperator())
+  if (CGM.getCodeGenOpts().StrictVTablePointers && E->isReservedPlacementNew())
     result = Builder.CreateLaunderInvariantGroup(result);
 
   // Emit sanitizer checks for pointer value now, so that in the case of an
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 4c7f516e308ca00..d0ed0289bcce52a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3837,7 +3837,10 @@ namespace {
     }
 
     bool VisitCXXNewExpr(CXXNewExpr *E) {
-      SafeToInline = E->getOperatorNew()->hasAttr<DLLImportAttr>();
+      if (E->isPlacementNewExpr())
+        SafeToInline = true;
+      else
+        SafeToInline = E->getOperatorNew()->hasAttr<DLLImportAttr>();
       return SafeToInline;
     }
   };
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 7968c62cbd3e7b3..93ba4a060342a22 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -606,8 +606,8 @@ class RuntimeInterfaceBuilder
       Expr *Args[] = {AllocCall.get()};
       ExprResult CXXNewCall = S.BuildCXXNew(
           E->getSourceRange(),
-          /*UseGlobal=*/true, /*PlacementLParen=*/SourceLocation(), Args,
-          /*PlacementRParen=*/SourceLocation(),
+          /*UseGlobal=*/true, /*IsPlacementNewExpr=*/false,
+          /*PlacementLParen=*/SourceLocation(), Args, /*PlacementRParen=*/SourceLocation(),
           /*TypeIdParens=*/SourceRange(), TSI->getType(), TSI, std::nullopt,
           E->getSourceRange(), E);
 
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 78c3ab72979a007..18b2fd86e566e5c 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -5457,7 +5457,8 @@ bool Parser::isTypeSpecifierQualifier() {
 
   case tok::coloncolon:   // ::foo::bar
     if (NextToken().is(tok::kw_new) ||    // ::new
-        NextToken().is(tok::kw_delete))   // ::delete
+        NextToken().is(tok::kw_delete) || // ::delete
+        NextToken().is(tok::kw__placement_new)) // ::_placement_new
       return false;
 
     if (TryAnnotateTypeOrScopeToken())
@@ -5650,7 +5651,8 @@ bool Parser::isDeclarationSpecifier(
     if (!getLangOpts().CPlusPlus)
       return false;
     if (NextToken().is(tok::kw_new) ||    // ::new
-        NextToken().is(tok::kw_delete))   // ::delete
+        NextToken().is(tok::kw_delete) || // ::delete
+        NextToken().is(tok::kw__placement_new)) // ::_placement_new
       return false;
 
     // Annotate typenames and C++ scope specifiers.  If we get one, just
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 53fba3b2f59242b..9b2e03221d5d407 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1682,7 +1682,7 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind,
     // ::new -> [C++] new-expression
     // ::delete -> [C++] delete-expression
     SourceLocation CCLoc = ConsumeToken();
-    if (Tok.is(tok::kw_new)) {
+    if (Tok.is(tok::kw_new) || Tok.is(tok::kw__placement_new)) {
       if (NotPrimaryExpression)
         *NotPrimaryExpression = true;
       Res = ParseCXXNewExpression(true, CCLoc);
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 99b4931004546c1..f21680834cfc531 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -186,7 +186,7 @@ bool Parser::ParseOptionalCXXScopeSpecifier(
   if (Tok.is(tok::coloncolon)) {
     // ::new and ::delete aren't nested-name-specifiers.
     tok::TokenKind NextKind = NextToken().getKind();
-    if (NextKind == tok::kw_new || NextKind == tok::kw_delete)
+    if (NextKind == tok::kw_new || NextKind == tok::kw_delete || NextKind == tok::kw__placement_new)
       return false;
 
     if (NextKind == tok::l_brace) {
@@ -3171,7 +3171,8 @@ bool Parser::ParseUnqualifiedId(CXXScopeSpec &SS, ParsedType ObjectType,
 ///
 ExprResult
 Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
-  assert(Tok.is(tok::kw_new) && "expected 'new' token");
+  assert((Tok.is(tok::kw_new) || Tok.is(tok::kw__placement_new)) && "expected 'new' token");
+  const bool IsPlacementNewExpr = Tok.is(tok::kw__placement_new);
   ConsumeToken();   // Consume 'new'
 
   // A '(' now can be a new-placement or the '(' wrapping the type-id in the
@@ -3201,6 +3202,14 @@ Parser::ParseCXXNewExpression(bool UseGlobal, SourceLocation Start) {
       return ExprError();
     }
...
[truncated]

Copy link

github-actions bot commented Nov 14, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@dwblaikie
Copy link
Collaborator

Might be worth cconsidering the points in "Contributing Extensions to Clang" here: https://clang.llvm.org/get_involved.html

It'd be great if there were some numbers/metrics associated with the benefit, ideally on some open source/commonly accessible codebase.

@MaxEW707
Copy link
Contributor Author

I was able to do a test on one of our smaller internal projects that uses a custom vendor clang and a custom vendor stl for this vendors platform.
I am planning to get a build of my fork for Linux and macOS so I can test against libcxx directly and test on some of our larger projects.
I am planning to test on some open-source projects in the near future as well.

The placement_new.h header that was used for the test looked as follows

#ifdef _TEST_INCLUDE_NEW
#include <new>
#else
void*  operator  new(decltype(sizeof(int)),  void*)  noexcept;
#endif

#define MY_PLACEMENT_NEW(p) ::new(p)

This test just show cases the build time reduction from including this vendor's <new> header and simulates being able to use ::_placement_new without a header include from the stl since this vendor's stl definitions for operator new do not conflict with our declaration of operator new.

This vendor's <new> header takes 29 ms to parse on my machine with their clang.
A clean debug build without <new> included takes 7 minutes and 54 seconds.
A clean debug build with <new> included takes 8 minutes and 16 seconds.
That is a difference of 22 seconds with just one include plus all its transitives removed.

As a reference on the same machine with a Ubuntu 20.04 WSL image with clang-10 installed from apt libc++ <new> takes 72 ms to parse and libstdc++ <new> takes 24 ms.
As a reference on the same machine with clang-cl 16.0.5 shipped with VS 17.7, msvc <new> takes 51 ms to parse.

I also want to note that it is a lot easier for us to guarantee behaviour from the compiler than to rely on vendor stl's that change as the cpp standard changes or whose headers parse times change between different toolchain versions that we need to support.

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

Successfully merging this pull request may close these issues.

None yet

3 participants