Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Fix Issue 10535 - Add initialize function to allocate an empty AA #2504

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ntrel
Copy link
Contributor

@ntrel ntrel commented Mar 2, 2019

See changelog/docs for details. I'll make a dlang.org pull for this soon.

This is adapted from a patch by Ketmar Dark: https://issues.dlang.org/show_bug.cgi?id=16269#c4.
CC'ing @jmdavis @schveiguy.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Auto-close Bugzilla Severity Description
10535 normal [AA] Add a function to druntime which returns an empty AA

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + druntime#2504"

@dlang-bot dlang-bot added the Bug Fix Include reference to corresponding bugzilla issue label Mar 2, 2019
@wilzbach
Copy link
Member

wilzbach commented Mar 2, 2019

You might want to have a look at a very similar attempt: reserve -> #1929

* aa = The associative array.
* See_Also: $(LREF clear)
*/
void initialize(T : Value[Key], Value, Key)(ref T aa) @trusted
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like filling up the object module with a lot of symbols. This is a pretty common name as well.

@rainers
Copy link
Member

rainers commented Mar 3, 2019

I guess it has been suggested before, but maybe it would be better to make

    int[int] aa = [];

allocate the internal data struct. It currently yields a nonsense Error: cannot have associative array key of void.

@ntrel
Copy link
Contributor Author

ntrel commented Mar 4, 2019

@wilzbach reserve(0) could work for this, but it looks like we don't have an implementation for non-zero sizes ATM - would that be a blocker? (I think reserve is less intuitive).

@jacob-carlborg Andrei suggested adding a function to initialize an empty AA, instead of adding built-in syntax, in 2013:
https://forum.dlang.org/post/[email protected]

@rainers [] could work, but it then means a silent allocation in an AA context, and no allocation in a slice context, and which of these contexts it is is sometimes not visible at the use-site. I think users would also do things like aa == [], causing unnecessary allocations and possible confusion. I suggested aa = new AA as being clearer syntax in the issue tracker.

@rainers
Copy link
Member

rainers commented Mar 4, 2019

new AA sounds good to me too. With alias AA = int[string] it currently errors out with new can only create structs, dynamic arrays or class objects, not int[string]'s and new int[string] yields Error: cannot pass type string as a function argument. So code breakage is unlikely.

@jacob-carlborg
Copy link
Contributor

jacob-carlborg Andrei suggested adding a function to initialize an empty AA, instead of adding built-in syntax, in 2013:
https://forum.dlang.org/post/[email protected]

I still don't like adding a new function for it. Using new AA works for me too. It's not really like we're adding new built-in syntax. It's already there, we're just adding a meaning to it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Fix Include reference to corresponding bugzilla issue Needs Rebase needs a `git rebase` performed Needs Work stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants