Skip to content

Fix issue 11006 - Make sure the same type is used in pointer ops#6440

Closed
LemonBoy wants to merge 3 commits intodlang:masterfrom
LemonBoy:b11006
Closed

Fix issue 11006 - Make sure the same type is used in pointer ops#6440
LemonBoy wants to merge 3 commits intodlang:masterfrom
LemonBoy:b11006

Conversation

@LemonBoy
Copy link
Contributor

If both operands are pointers, and the operator is -, the pointers are subtracted and the result is divided by the size of the type pointed to by the operands. It is an error if the pointers point to different types. The type of the result is ptrdiff_t.

@dlang-bot
Copy link
Contributor

dlang-bot commented Jan 13, 2017

Fix Bugzilla Description
11006 Subtraction of pointers for void and non-void types compiles
16967 No switch case fallthrough warnings in in/out contracts

@denis-sh
Copy link
Contributor

denis-sh commented Jan 14, 2017

It's better to also check error message in test.
Also at least druntime source code must be adjusted first (get rid of "Operands must point to the same type" errors) to be able to merge this.

@LemonBoy
Copy link
Contributor Author

Good idea, I've amended the test case.
I've already sent two PRs for druntime and phobos, we just have to wait for them to get merged.

@WalterWaldron
Copy link
Contributor

Deprecate first

Type p1 = t1.nextOf();
Type p2 = t2.nextOf();

if (!p1.equivalent(p2))
Copy link
Member

Choose a reason for hiding this comment

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

I'd expect this to break a lot of code, maybe rather require types of same size than requiring identity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Phobos and druntime have been mostly chill as all the errors were due to the use of void* and byte*. Relying on the size of the types still allows the user to mix and match the types which is not what you want in 99% of the cases (and if you really need to do so you're just explicit cast away) as some types may vary between platforms and one may end up enlarging/shrinking one of the types involved and end up with a not so nice compilation error.
I root for following the specification here.

d_int64 stride;

// make sure pointer types are compatible
if (Expression ex = typeCombine(this, sc))
Copy link
Member

Choose a reason for hiding this comment

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

Smells like this could do more than just dealing with void*.

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 just brings the types down to a common one, I couldn't find a case where something more than that was being done.

@LemonBoy
Copy link
Contributor Author

Should this go trough the deprecation phase too? I think this is a bugfix since we stray away from the specification here.

@Geod24
Copy link
Member

Geod24 commented Jan 15, 2017

Should this go trough the deprecation phase too? I think this is a bugfix since we stray away from the specification here.

Doesn't matter if it's a bug fix or not, if it breaks code and there is a sane depreciation path we have to take it. Remember that a single occurrence in an unused code path in a library will cause downstream projects to fail to compile. That's not something acceptable.

@yebblies
Copy link
Contributor

Are pointers with different modifiers handled correctly? eg const(void)* and void*

@LemonBoy
Copy link
Contributor Author

Sure, the equivalent predicate covers those cases

@JinShil
Copy link
Contributor

JinShil commented Nov 18, 2017

Revived at #7332

@wilzbach wilzbach closed this Nov 18, 2017
@LemonBoy
Copy link
Contributor Author

You're just grabbing the commits and throwing away all the informations about the author, that's not nice.

@JinShil
Copy link
Contributor

JinShil commented Nov 18, 2017

You're just grabbing the commits and throwing away all the informations about the author, that's not nice.

Not at all. I've posted links in the old PRs to the new PRs, and from the new PRs to the old PRs. So there is an audit trail that anyone can follow. In terms of the source code itself, there was no copyright or other information about the author to keep, otherwise I would have kept it. If you're concerned about the merge being in your name, revive them yourself.

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.

9 participants