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 IsNilpotentGroup: infinite does not imply "not p-group" #2866

Merged
merged 1 commit into from
Sep 27, 2018

Conversation

fingolfin
Copy link
Member

This should be backported to 4.10.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them regression A bug that only occurs in the branch, not in a release backport-to-4.10 labels Sep 25, 2018
@fingolfin
Copy link
Member Author

@stevelinton does this seem sensible to you?

Copy link
Contributor

@hulpke hulpke left a comment

Choose a reason for hiding this comment

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

This makes sense. Is it clear that nothing else assumes that IsPGroup implies finite?

@fingolfin fingolfin merged commit 6c85b97 into gap-system:master Sep 27, 2018
@fingolfin fingolfin deleted the mh/fix-infinite-pgroup branch September 27, 2018 21:57
@fingolfin
Copy link
Member Author

Backported to 4.10 in 7388170

@hulpke It is "clear" in so far as some time ago, we (well, mostly we) audited the library and all deposited packages for uses of IsPGroup, and added explicit IsFinite checks when relevant. But I neglected to check for code which explicitly sets IsPGroup to false. When creating this PR, I check for other places doing it, but the on modified in this PR was the only I found. So, to the best of knowledge and ability, no public code is left that assumes IsPGroup implies finite; but of course I still may have missed some, and then there is non-public code... But on the upside, I am not yet aware of any code in GAP providing infinite p-groups, so until that happens, no problems should arise, giving us some more time for the transition.

@hulpke
Copy link
Contributor

hulpke commented Sep 28, 2018

@fingolfin Excellent. Thanks.

@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-to-4.10-DONE kind: bug Issues describing general bugs, and PRs fixing them regression A bug that only occurs in the branch, not in a release release notes: added PRs introducing changes that have since been mentioned in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants