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

Remove option to return values from break loop in many places #2966

Merged
merged 1 commit into from
Nov 7, 2018

Conversation

ChrisJefferson
Copy link
Contributor

This removes the option to return a value from the break loop in many places (in particular, places Max just cleaned up).

This isn't all places, but I thought I'd make this simple pass first, then work through the other cases in further PRs.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Looks good to me. But I wonder: Are any calls to RequireArgumentConditionMayReplace left in there? If yes, why? If no, how about removing RequireArgumentConditionMayReplace and its various wrappers from error.h?

@ChrisJefferson ChrisJefferson added topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Nov 7, 2018
@ChrisJefferson
Copy link
Contributor Author

I wasn't sure if those functions might end up being wanted at some point in packages, or the library. I'm happy to delete them.

@fingolfin
Copy link
Member

Well, I see it like this: if somebody really wants those *MayReplace variants, they better have a really strong convincing argument; and then we can add them back. In the meantime we want to remove (and hence also discourage) this style of coding, don't we? Hence I'd remove them. Alas, that certainly can be done in another PR. No need to delay this one for it.

@fingolfin fingolfin merged commit 4265b7c into gap-system:master Nov 7, 2018
@ChrisJefferson ChrisJefferson deleted the remove-mayremove branch January 20, 2019 13:38
@fingolfin fingolfin added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Apr 15, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Aug 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: added PRs introducing changes that have since been mentioned in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants