Skip to content

[Clang] Improve diagnostics for 'placement new' with const storage argument #144270

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,8 @@ Improvements to Clang's diagnostics
#GH69470, #GH59391, #GH58172, #GH46215, #GH45915, #GH45891, #GH44490,
#GH36703, #GH32903, #GH23312, #GH69874.

- Improve the diagnostics for placement new expression when const-qualified
object was passed as the storage argument.

Improvements to Clang's time-trace
----------------------------------
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -8285,6 +8285,9 @@ def err_need_header_before_typeid : Error<
def err_need_header_before_placement_new : Error<
"no matching %0 function for non-allocating placement new expression; "
"include <new>">;
def err_placement_new_into_const_qualified_storage : Error<
"placement new expression with a const-qualified argument of type %0 "
"is not allowed">;
def err_ms___leave_not_in___try : Error<
"'__leave' statement not in __try block">;
def err_uuidof_without_guid : Error<
Expand Down
10 changes: 9 additions & 1 deletion clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2753,10 +2753,18 @@ static bool resolveAllocationOverloadInterior(
if (Diagnose) {
// If this is an allocation of the form 'new (p) X' for some object
// pointer p (or an expression that will decay to such a pointer),
// diagnose the missing inclusion of <new>.
// diagnose the reason for the error.
if (!R.isClassLookup() && Args.size() == 2 &&
(Args[1]->getType()->isObjectPointerType() ||
Args[1]->getType()->isArrayType())) {
if (Args[1]->getType()->isPointerType()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be improved by the addition of something to the effect of

QualType Arg1Type = Args[1]->getType();

And then just reusing that.

But I think this check is in the wrong place, and should probably be earlier - this is inside an array of object check so I suspect the existing bad error message would likely occur with something like

void f(const void* ptr) {
  new (ptr) int;
}

you probably want something to the effect of

if (Context.getBaseElementType(Arg1Type).isConstQualified()) {
   emit the error
   return true;
}

if (Args[1]->getType()->getPointeeType().isConstQualified()) {
S.Diag(Args[1]->getExprLoc(),
diag::err_placement_new_into_const_qualified_storage)
<< Args[1]->getType() << Args[1]->getSourceRange();
return true;
}
}
S.Diag(R.getNameLoc(), diag::err_need_header_before_placement_new)
<< R.getLookupName() << Range;
// Listing the candidates is unlikely to be useful; skip it.
Expand Down
7 changes: 7 additions & 0 deletions clang/test/SemaCXX/new-delete.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,13 @@ void bad_news(int *ip)
#if __cplusplus < 201103L
(void)new int[]{}; // expected-error {{array size must be specified in new expression with no initializer}}
#endif
struct X { int n; };
const X cx = {5};
(void)new(&cx) X{10}; // expected-error {{placement new expression with a const-qualified argument of type 'const X *' is not allowed}}
const X* const cx2 = 0;
(void)new(cx2) X{10}; // expected-error {{placement new expression with a const-qualified argument of type 'const X *const' is not allowed}}
const int arr[1] = {1};
(void)new(&arr[0]) int(10); // expected-error {{placement new expression with a const-qualified argument of type 'const int *' is not allowed}}
}

void no_matching_placement_new() {
Expand Down
Loading