Fix Issue 11006 - Subtraction of pointers for void and non-void type compiles

[JinShil]

Revival of #6440

[dlang-bot]

Thanks for your pull request, @JinShil! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Auto-close	Bugzilla	Description
✓	11006	Subtraction of pointers for void and non-void types compiles

⚠️⚠️⚠️ Warnings ⚠️⚠️⚠️

• Regression or critical bug fixes should always target the stable branch. Learn more about rebasing to stable or the D release process.

[JinShil]

This PR may have revealed the following problem:

Deprecation messages occur in std.experimental.allocator. For example: std/experimental/allocator/building_blocks/bitmapped_block.d(232): Deprecation: Operands point to types of different size; void (1 bytes), ulong (8 bytes).

Perhaps std.experimental.allocator's code should be audited and where there is no problem, intent expressed with explicit casts.

See output of auto-tester for more details.

[JinShil]

@andralex @wilzbach @ZombineDev As code owners of std.experimental.allocator, your thoughts on the error described above?

[andralex]

@JinShil thanks for pointing that out. The culprit seems to be the snippet:

        auto start = _control.rep.ptr, end = _payload.ptr + _payload.length;
        parent.deallocate(start[0 .. end - start]);

The end - start operation subtracts pointers to different types. The solution is:

        void* start = _control.rep.ptr, end = _payload.ptr + _payload.length;
        parent.deallocate(start[0 .. end - start]);

To my own surprise I managed to create dlang/phobos#5864 just by using the web interface - nice job github.

[andralex]

As an aside, the code was actually incorrect. It was equivalent to:

        ulong* start = _control.rep.ptr;
        void* end = _payload.ptr + _payload.length;
        parent.deallocate(start[0 .. end - start]);

The computation end - start yields the number of bytes, but the slice passed to deallocate will be 8 times larger. Probably this only worked because the parent allocator tolerated the bug (e.g. Mallocator and GCAllocator don't use the length of the chunk).

[JinShil]

From my perspective, this is ready to go.

[PetarKirov]

LGTM, modulo one question about how strict we want to be.

[PetarKirov]

According to spec, subtracting pointers pointing to objects of different types (regardless of size) is an error. In the case where we have pointers to different types, but with equal sizes, the result obviously would be the same if you cast one of the pointers to the same type as the other one, but such code may be actually be a mistake on the user's part. What are your opinions on being more strict, as the spec dictates?

(I just saw that @LemonBoy is also in favor of the more strict approach #6440 (comment).)

[JinShil]

Yeah, I'm for following the spec. I'll update the PR.

[JinShil]

My understanding is the Type.equivalent() function is all that's needed. I think the size comparison in the previous commit was unnecessary.

Again, I think this is ready to go unless I'm missing something.

[JinShil]

Is jenkins dub just finicky or what?

[WalterBright]

@andralex it is nice that this PR found a bug.

[WalterBright]

Not much point here to linking to the bug report. Instead link to the relevant spec
https://dlang.org/spec/expression.html#add_expressions and add the quote: "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."

Linking to and quoting from the spec should be a good way to clarify what's going on in the implementation.

[JinShil]

Done. Thanks!