Add attributes to most functions in allocator.mallocator and allocator.common.

[PetarKirov]

Also made the attribute style more consistent, because it was all over the place throughout the package.

I looked at the Phobos styleguide, but I couldn't find anything about this, so I decided to follow the soft limit of 80 chars per line. That's why I put the attributes on their own line.
Overall the scheme is something like this:

[@safe] [@nogc] [nothrow] [pure]
[protection-level] [extern] <function-name / unittest>[params] [shared]

Let me know if there is an existing style that I need to follow. I only care about adding correct attributes on the functions, not about bikeshedding about the code style.

[PetarKirov]

There were several functions in std.experimental.allocator.common that accept void[] or void* arguments. I decided to leave them as implicitly @system.

[JackStouffer]

I disagree with these changes from auto. auto should really be the default everywhere, see http://herbsutter.com/2013/08/12/gotw-94-solution-aaa-style-almost-always-auto/

[dnadlinger]

The type of the variable would be inferred to be a slice instead of a static array with auto, so it's not just a stylistic change. That being said, this is in an unit test, so one could argue that it doesn't really matter either way.

[JakobOvrum]

auto makes it a dynamic array, which precludes @nogc.

[PetarKirov]

I also prefer auto, but array literals are not stack allocated and the whole unittest will become non-@nogc if I leave the declarations as auto.
There were proposals for the syntax auto arr = [1, 2, 3]s or auto[$] arr = [1, 2, 3], however they were rejected :(

[JakobOvrum]

This isn't memory safe when asserts are disabled. posix_memalign fails with EINVAL when given a poor alignment argument, which isn't handled.

[JakobOvrum]

Unfortunately there's no satisfactory way to make this both memory safe and @nogc.

For ENOMEM, return null per the allocator spec, problem handled.

EINVAL only happens for poor values of a. Per the assert, we can see that this function expects a good value of a as a precondition. Preconditions cannot be relied on for the memory safety guarantee. Throwing an exception on EINVAL provides memory safety but precludes @nogc, even if onAssertError is used, as it's not @nogc either. Returning null is not an option as it signals OOM.

I suggest leaving it as @system or using code == EINVAL || assert(0); as a stopgap measure until we've figured out @nogc exceptions.

[JakobOvrum]

Yeah, I see now that this is an issue with the existing code, not with this PR.

[PetarKirov]

@JakobOvrum Added assert(0) for the other possible cases.

[JakobOvrum]

Some of these changes are introduced in #3891 but it's nice to have them in a separate PR like this.

[JakobOvrum]

I don't think we should be strict about placement of attributes at this time. The rationale of keeping to the column limit sounds sensible.

LGTM. I was initially confused as it looked like it added @trusted here and there, but those were already there.

[JakobOvrum]

Hm, does assert(0, …) still have its special behaviour when given a message string? Just how would that work?

[PetarKirov]

It's a NOP on the higher optimization levels, otherwise it calls _d_assert_msg, however I prefer to have an informative message for those willing to debug the assert if it happens.

[PetarKirov]

Maybe in the future it will be replaced with some form of abort on the compiler side.

[JakobOvrum]

assert(0) emits a HLT instruction in release mode. I am wondering if assert(0, …) behaves similarly.

[schveiguy]

Yes, it behaves exactly the same, just a HLT. On Posix systems (which obviously this will be on), this manifests as a segmentation fault. This is why I created abort as mentioned above -- no point in having a message when it is ignored.

[JakobOvrum]

I see, thanks!

[PetarKirov]

Thanks all for the review. Is there something else that I need to address?

[dnadlinger]

Auto-merge toggled on