Skip to content

Comments

Make allocators' resolveInternalPointer take a const pointer as source#5321

Merged
dlang-bot merged 1 commit intodlang:masterfrom
edi33416:allocator_resolveIntPtr
Apr 3, 2017
Merged

Make allocators' resolveInternalPointer take a const pointer as source#5321
dlang-bot merged 1 commit intodlang:masterfrom
edi33416:allocator_resolveIntPtr

Conversation

@edi33416
Copy link
Contributor

@edi33416 edi33416 commented Apr 3, 2017

No description provided.


/// Ditto
pure nothrow Ternary resolveInternalPointer(void* p, ref void[] result) shared
pure nothrow
Copy link
Member

Choose a reason for hiding this comment

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

I think this entire function can be trusted, maybe in a later step

@dlang-bot dlang-bot merged commit 4e62786 into dlang:master Apr 3, 2017
Ternary resolveInternalPointer(const void* p, ref void[] result) shared
{
auto r = GC.addrOf(p);
auto r = GC.addrOf(cast(void*)p);
Copy link
Member

Choose a reason for hiding this comment

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

Style check failed:

Enforce space after cast(...)
grep -nrE '[^"]cast\([^)]*?\)[[:alnum:]]' $(find . -name '*.d') ; test $? -eq 1
./std/experimental/allocator/gc_allocator.d:76:        auto r = GC.addrOf(cast(void*)p);

Please fix/revert :)

@schveiguy
Copy link
Member

Please fix/revert :)

#5325

@schveiguy
Copy link
Member

ping @wilzbach can dlang-bot not auto-merge when circleci complains?

@CyberShadow
Copy link
Member

ping @wilzbach can dlang-bot not auto-merge when circleci complains?

dlang/dlang-bot#69 (or we can set CircleCI as required).

@andralex
Copy link
Member

andralex commented Apr 5, 2017

Yah that'd be useful, thanks.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 5, 2017

we can set CircleCI as required

Yep, that would be the easiest solution...

@CyberShadow
Copy link
Member

@wilzbach Should we do it? Seems like we're in agreement.

@wilzbach
Copy link
Contributor

wilzbach commented Apr 5, 2017

@wilzbach Should we do it? Seems like we're in agreement

Yes please! I have been trying to push for this since last summer :)

@CyberShadow
Copy link
Member

Yes please!

OK, I've set CircleCI as required for Phobos for now, we can expand to other repos if this works well.

I have been trying to push for this since last summer :)

Oh, I knew there was a sentiment in favor of doing it, but I don't think I've received an explicit request to enable it in my inbox, otherwise I probably would have :)

@wilzbach
Copy link
Contributor

wilzbach commented Apr 5, 2017

Oh, I knew there was a sentiment in favor of doing it, but I don't think I've received an explicit request to enable it in my inbox, otherwise I probably would have :)

Well, there were always a few problems with our CI, so with "pushing" I meant more wanting to make it happen.
Thanks a lot!

@wilzbach
Copy link
Contributor

I know that this is in std.experimental, but Allocator is treated as nearly "standard" by the community, so breaking changes do affect projects, e.g. vibe-d/vibe.d#1774
At least you should mention breaking changes in the changelog!

@edi33416 edi33416 deleted the allocator_resolveIntPtr branch October 9, 2017 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants