Skip to content

Comments

Bugfix for "undefined identifier _min" in SharedFreeList.#5556

Merged
dlang-bot merged 1 commit intodlang:masterfrom
chadjoan:shared_freelist_min_typo
Jul 10, 2017
Merged

Bugfix for "undefined identifier _min" in SharedFreeList.#5556
dlang-bot merged 1 commit intodlang:masterfrom
chadjoan:shared_freelist_min_typo

Conversation

@chadjoan
Copy link
Contributor

@chadjoan chadjoan commented Jul 6, 2017

When instantiating SharedFreeList with a 0 minSize and a
chooseAtRuntime maxSize, it will give an error like so:

std/experimental/allocator/building_blocks/free_list.d(822): Error: undefined identifier _min, did you mean alias min?

This commit resolves the error message by using 'min' instead
of '_min', because '_min' is only available when
(minSize == chooseAtRuntime).

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @chadjoan! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

  • smaller, focused PRs are easier to review than big ones

  • try not to mix up refactoring or style changes with bug fixes or feature enhancements

  • provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

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.

Copy link
Collaborator

@RazvanN7 RazvanN7 left a comment

Choose a reason for hiding this comment

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

Cool. You should add a unittest which covers the lines of code you wrote so that we get rid of the codecoverage failing test

@chadjoan
Copy link
Contributor Author

chadjoan commented Jul 6, 2017

Roger. Hope that does it.

@RazvanN7
Copy link
Collaborator

RazvanN7 commented Jul 7, 2017

You should try to rebase [1] . Hopefully that will get rid of the CyberShadow failure (you probably have an older version of phobos and the tools that come with it.)

[1] https://wiki.dlang.org/Starting_as_a_Contributor#Rebasing

@wilzbach
Copy link
Contributor

wilzbach commented Jul 7, 2017

You should try to rebase

Thanks to the CircleCi magic at #5559, simply restarting CircleCi should be enough (I just did this)

Copy link
Contributor

@wilzbach wilzbach left a comment

Choose a reason for hiding this comment

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

Looks reasonable, though while you are at it, a test for min as well, wouldn't hurt ;-)

@CyberShadow
Copy link
Member

Hopefully that will get rid of the CyberShadow failure (you probably have an older version of phobos and the tools that come with it.)

I don't think that should cause the documentation tester to fail.

Please ping me if everything but the documentation tester is failing, and you think it shouldn't.

@chadjoan chadjoan force-pushed the shared_freelist_min_typo branch from b775b0f to efe31d8 Compare July 8, 2017 04:09
@chadjoan
Copy link
Contributor Author

chadjoan commented Jul 8, 2017

I squashed/rebased and added a test for min being chooseAtRuntime while maxSize is chosen at compile time.

@wilzbach wilzbach force-pushed the shared_freelist_min_typo branch from efe31d8 to 0c41d96 Compare July 10, 2017 04:43
When instantiating SharedFreeList with a 0 minSize and a
chooseAtRuntime maxSize, it will give an error like so:

  std/experimental/allocator/building_blocks/free_list.d(822): Error: undefined identifier _min, did you mean alias min?

This commit resolves the error message by using 'min' instead
of '_min', because '_min' is only available when
(minSize == chooseAtRuntime).
@wilzbach
Copy link
Contributor

I squashed/rebased and added a test for min being chooseAtRuntime while maxSize is chosen at compile time.

Thanks. I also added deallocateAll on exit as, even if the sizes are small, it's a good practice to cleanup the "global state" one messed with after each tests.

@dlang-bot dlang-bot merged commit 0958adf into dlang:master Jul 10, 2017
@chadjoan chadjoan deleted the shared_freelist_min_typo branch July 29, 2017 23:57
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.

5 participants