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

Proposal for std.experimental.allocator #3405

Merged
merged 186 commits into from
Oct 2, 2015
Merged

Conversation

andralex
Copy link
Member

Throws:
The first two overloads throw only if `alloc`'s primitives do. The
overloads that involve copy initialization deallocate memory and propagate the
exception if the copy operation throws.

Choose a reason for hiding this comment

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

There is a copy-paste error. There are no three overloads for shrinkArray.

P. S. Is there a way on github to search a proper place for comment other than scrolling down the whole pull request?

@andralex
Copy link
Member Author

andralex commented Aug 7, 2015

Seems to be blocked by dlang/druntime#1340

version (Posix) private extern(C) int posix_memalign(void**, size_t, size_t);
version (Windows)
{
private extern(C) void* _aligned_malloc(size_t, size_t);
Copy link

Choose a reason for hiding this comment

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

these functions (_aligned_malloc and its 2 counterparts) don't exist in the DigitalMars standard C library (snn.lib). So currently DMD 32bit & OptLink are totally unable to link any code that would use these functions and will produce:

 Error 42: Symbol Undefined __aligned_free
 Error 42: Symbol Undefined __aligned_malloc
 Error 42: Symbol Undefined __aligned_realloc

The problem has already hit a few users of DCD when they tried to compile the software with -debug -gc. (because DCD uses a temp. fork of std.experimental.allocators).
Actually there is an underlying bug in DMD or optlink with this because when the external symbols are not used they are eliminated by the compiler (unless the switch -debug -gc are toogled on).

Maybe the error is that the author thought that either windows.lib or snn.lib contain these functions but they don't.
So under windows

  • version( DigitalMars ):
    • solution 1: either the struct AlignedMallocator has to be disabled.
    • solution 2: the functions have to be manually implemented, based on malloc (MS states that their implementation is based on malloc, see remark section, so it should be do-able with snn malloc).
    • solution 3: @WalterBright has to implement the functions in snn.
  • version( CRuntime_Microsoft ): should be ok (DMD 64 bit).

{
auto result = n.allocate(s);
if (result.length != s) continue;
assert(owns(result) == Ternary.yes);
Copy link
Member

Choose a reason for hiding this comment

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

If you comment this line out the unit tests will fail.

Copy link
Member

Choose a reason for hiding this comment

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

Disregard that, it was a unit test for an FreeList!(AllocatorList!(n => Region!Mallocator(blockSize)), ns) that I had in my code somewhere that failed.

Copy link
Member

Choose a reason for hiding this comment

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

I figured it out. The problem is that the implementation of dispose() is incorrect.

@mihails-strasuns-sociomantic
Copy link
Contributor

Is anything (apart from tester failures) preventing the merge? Concept was approved through review process quite some time ago.

@andralex
Copy link
Member Author

andralex commented Oct 2, 2015

I rebased and fixed conflicts in posix.mak off of a fresh machine.

@jmdavis
Copy link
Member

jmdavis commented Oct 2, 2015

Auto-merge toggled on

jmdavis added a commit that referenced this pull request Oct 2, 2015
Proposal for std.experimental.allocator
@jmdavis jmdavis merged commit 6bb219b into dlang:master Oct 2, 2015
@John-Colvin
Copy link
Contributor

Hooray!

@DmitryOlshansky
Copy link
Member

Awes!

@MartinNowak
Copy link
Member

Added to changelog.
bcbe747

@@ -2953,6 +2953,7 @@ float ldexp(float n, int exp) @safe pure nothrow @nogc { return ldexp(cast(real)
}

/* workaround Issue 14718, float parsing depends on platform strtold
typed_allocator.d
Copy link
Member

Choose a reason for hiding this comment

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

How did this end up here?

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.