Skip to content

[clang][NFC] Add missing placement-new after Allocate() calls #68382

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

Merged
merged 2 commits into from
Oct 6, 2023

Conversation

Endilll
Copy link
Contributor

@Endilll Endilll commented Oct 6, 2023

While working on #68377 inspecting Allocate() calls, I found out that there are couple of places where we forget to use placement-new to create objects in the allocated memory.

@Endilll Endilll added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Oct 6, 2023
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Oct 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 6, 2023

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Changes

While working on #68377 inspecting Allocate() calls, I found out that there are couple of places where we forget to use placement-new to create objects in the allocated memory.


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

3 Files Affected:

  • (modified) clang/lib/AST/DeclCXX.cpp (+2-1)
  • (modified) clang/lib/AST/DeclObjC.cpp (+2-2)
  • (modified) clang/lib/Serialization/ASTReaderDecl.cpp (+5-2)
diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp
index 42bab4ed51b7290..a92b788366434ce 100644
--- a/clang/lib/AST/DeclCXX.cpp
+++ b/clang/lib/AST/DeclCXX.cpp
@@ -1484,7 +1484,8 @@ void CXXRecordDecl::setCaptures(ASTContext &Context,
     if (Captures[I].isExplicit())
       ++Data.NumExplicitCaptures;
 
-    *ToCapture++ = Captures[I];
+    new (ToCapture) LambdaCapture(Captures[I]);
+    ToCapture++;
   }
 
   if (!lambdaIsDefaultConstructibleAndAssignable())
diff --git a/clang/lib/AST/DeclObjC.cpp b/clang/lib/AST/DeclObjC.cpp
index e934a81d086e3c0..e1eef2dbd9c3d28 100644
--- a/clang/lib/AST/DeclObjC.cpp
+++ b/clang/lib/AST/DeclObjC.cpp
@@ -931,8 +931,8 @@ void ObjCMethodDecl::setParamsAndSelLocs(ASTContext &C,
   unsigned Size = sizeof(ParmVarDecl *) * NumParams +
                   sizeof(SourceLocation) * SelLocs.size();
   ParamsAndSelLocs = C.Allocate(Size);
-  std::copy(Params.begin(), Params.end(), getParams());
-  std::copy(SelLocs.begin(), SelLocs.end(), getStoredSelLocs());
+  std::uninitialized_copy(Params.begin(), Params.end(), getParams());
+  std::uninitialized_copy(SelLocs.begin(), SelLocs.end(), getStoredSelLocs());
 }
 
 void ObjCMethodDecl::getSelectorLocs(
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index d553b3c6d78dedc..6a2f607d916c472 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2002,13 +2002,16 @@ void ASTDeclReader::ReadCXXDefinitionData(
       case LCK_StarThis:
       case LCK_This:
       case LCK_VLAType:
-        *ToCapture++ = Capture(Loc, IsImplicit, Kind, nullptr,SourceLocation());
+        new (ToCapture)
+            Capture(Loc, IsImplicit, Kind, nullptr, SourceLocation());
+        ToCapture++;
         break;
       case LCK_ByCopy:
       case LCK_ByRef:
         auto *Var = readDeclAs<VarDecl>();
         SourceLocation EllipsisLoc = readSourceLocation();
-        *ToCapture++ = Capture(Loc, IsImplicit, Kind, Var, EllipsisLoc);
+        new (ToCapture) Capture(Loc, IsImplicit, Kind, Var, EllipsisLoc);
+        ToCapture++;
         break;
       }
     }

@Endilll Endilll removed clang Clang issues not falling into any other category clang:modules C++20 modules and Clang Header Modules labels Oct 6, 2023
@github-actions
Copy link

github-actions bot commented Oct 6, 2023

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

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! Thank you for the cleanup!

@AaronBallman AaronBallman merged commit 99e6ef3 into llvm:main Oct 6, 2023
Guzhu-AMD pushed a commit to GPUOpen-Drivers/llvm-project that referenced this pull request Oct 12, 2023
Local branch amd-gfx f039005 Merged main:ff843c00ce1d into amd-gfx:8bf801a67373
Remote branch main 99e6ef3 [clang][NFC] Add missing placement-new after Allocate() calls (llvm#68382)
@Endilll Endilll deleted the add-missing-placement-new branch April 6, 2024 04:21
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"
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants