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

Fix AA iteration order dependency in testaa2.d #11361

Merged
merged 1 commit into from
Jul 2, 2020

Conversation

n8sh
Copy link
Member

@n8sh n8sh commented Jul 2, 2020

Blocking dlang/druntime#3148 (comment)

A DMD test is failing because it depends on a specific AA traversal order. I am inclined to consider that erroneous based on https://dlang.org/spec/hash-map.html: "The built-in associative arrays do not preserve the order of the keys inserted into the array. In particular, in a foreach loop the order in which the elements are iterated is typically unspecified."

Test runnable/testaa2.d failed: 
expected:
----
foo()
foo() 2
foo() 3
foo() 4
c["foo"] = 3
c["bar"] = 4
Success
----
actual:
----
foo()
foo() 2
foo() 3
foo() 4
c["bar"] = 4
c["foo"] = 3
Success

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @n8sh!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

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

dub run digger -- build "master + dmd#11361"

@n8sh n8sh force-pushed the testaa2-fix-order-dependency branch from 3d4eaed to b032621 Compare July 2, 2020 07:10
{
if ((key[i][0] == 'b') == isItTimeForB)
printf("c[\"%.*s\"] = %d\n", cast(int)key[i].length, key[i].ptr, value[i]);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this output and add appropriate asserts (the test is compiled with enabled assertions).

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks

@n8sh n8sh force-pushed the testaa2-fix-order-dependency branch from b032621 to e61310a Compare July 2, 2020 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants