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

Issue a warning on gnu void pointer arithmetic #776

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

wrongnull
Copy link
Contributor

Closes #759
In this request I implemented the Type.isPtrTo function but only for the .pointer type specifier. I'm not sure if it is right approach to do that. Maybe other specifiers should be considered as well. I'm open for the conversation.

@wrongnull wrongnull changed the title Wgnu pointer arith Issue a warning on gnu void pointer arithmetic Sep 29, 2024
@ehaas
Copy link
Collaborator

ehaas commented Sep 29, 2024

It's not a bad idea but we already have a specialized function for it; you can just use ty.isVoidStar()

@wrongnull
Copy link
Contributor Author

It's not a bad idea but we already have a specialized function for it; you can just use ty.isVoidStar()

I agree. Implementation of such a function is out of this pull requests scope

@@ -5504,6 +5504,10 @@ pub const Result = struct {
// if both aren't arithmetic then either both should be pointers or just a
if (!a_ptr or !(b_ptr or b_int)) return a.invalidBinTy(tok, b, p);

if ((a.ty.isVoidStar() or b.ty.isVoidStar()) and
(p.comp.diagnostics.options.@"gnu-pointer-arith" == .warning or p.comp.diagnostics.options.pedantic == .warning))
Copy link
Collaborator

Choose a reason for hiding this comment

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

The last condition here (p.comp.diagnostics.options.@"gnu-pointer-arith" == .warning or p.comp.diagnostics.options.pedantic == .warning) is not needed - that is all checked in Diagnostics.add via Diagnostics.tagKind

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ehaas
Copy link
Collaborator

ehaas commented Sep 30, 2024

We'll also need a check in the .add branch of the switch where the current check is, to handle things like ptr + 1 or 1 + ptr. In the .sub branch, I just realized we only need to check for a.ty.isVoidStar() because you can only subtract a pointer from a same-type pointer. So there's no valid subtraction where b is void * and a is not. But for addition either one could be void *.

One final area to add the checks would be for .plus_plus and .minus_minus in the suffixExpr and unExpr functions to handle the case of ptr++/ptr--/++ptr/--ptr when ptr is void *.

@wrongnull
Copy link
Contributor Author

We'll also need a check in the .add branch of the switch where the current check is, to handle things like ptr + 1 or 1 + ptr. In the .sub branch, I just realized we only need to check for a.ty.isVoidStar() because you can only subtract a pointer from a same-type pointer. So there's no valid subtraction where b is void * and a is not. But for addition either one could be void *.

One final area to add the checks would be for .plus_plus and .minus_minus in the suffixExpr and unExpr functions to handle the case of ptr++/ptr--/++ptr/--ptr when ptr is void *.

done

@Vexu Vexu merged commit ecd5c72 into Vexu:master Oct 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add warning for pointer arithmetic on pointers to void
3 participants