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

Fix warning about delete not matching new #4989

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

ChrisDodd
Copy link
Contributor

New gcc versions (11+) give a slew of warnings about a mismatched operator delete being called from a new expression. This trivial change fixes those warnings.

@ChrisDodd ChrisDodd requested a review from fruffy October 31, 2024 08:05
@@ -75,6 +75,7 @@ class Type_Any : Type, ITypeVar {
protected:
#emit
void *operator new(size_t size) { return ::operator new(size); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need new override for these nodes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was done to make them protected rather than public, so that other code could not directly call new and had to call the appropriate get method instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok, I missed that get methods.

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Oct 31, 2024
Signed-off-by: Chris Dodd <cdodd@nvidia.com>
@ChrisDodd ChrisDodd added this pull request to the merge queue Oct 31, 2024
Merged via the queue into p4lang:main with commit 09b4e63 Oct 31, 2024
19 checks passed
@ChrisDodd ChrisDodd deleted the cdodd-delwarn branch October 31, 2024 23:04
@asl
Copy link
Contributor

asl commented Nov 1, 2024

Looks like this broke sanitizers build

@fruffy
Copy link
Collaborator

fruffy commented Nov 1, 2024

Looks like this broke sanitizers build

Yup, the nightly failed: https://github.com/p4lang/p4c/actions/runs/11621767033/job/32366099914

@asl
Copy link
Contributor

asl commented Nov 1, 2024

GCC 10 does not enable sized deallocation by default. We need to make these delete optional.

@asl
Copy link
Contributor

asl commented Nov 1, 2024

We'd likely need to guard these with #ifdef __cpp_sized_deallocation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants