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

document what a 'small integer' is; make "need positive small int" errors consistent; avoid 32/64 bit specific error messages #2946

Merged
merged 6 commits into from
Oct 29, 2018

Conversation

fingolfin
Copy link
Member

No description provided.

* use `IS_POS_INTOBJ` is possible
* consistently refer to "positive small integer", where "small integer"
  is understood to have a special meaning
@fingolfin fingolfin added topic: documentation Issues and PRs related to documentation topic: kernel labels Oct 27, 2018
@codecov
Copy link

codecov bot commented Oct 27, 2018

Codecov Report

Merging #2946 into master will decrease coverage by 8.12%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2946      +/-   ##
==========================================
- Coverage    83.8%   75.67%   -8.13%     
==========================================
  Files         681      625      -56     
  Lines      346416   312462   -33954     
==========================================
- Hits       290303   236453   -53850     
- Misses      56113    76009   +19896
Impacted Files Coverage Δ
src/integer.c 97.99% <ø> (+0.11%) ⬆️
src/vars.c 79.67% <ø> (-13.16%) ⬇️
src/code.c 97.43% <ø> (+1.16%) ⬆️
src/trans.c 99.79% <100%> (-0.02%) ⬇️
src/blister.c 96.96% <100%> (-0.23%) ⬇️
src/exprs.c 91% <100%> (-6.59%) ⬇️
src/plist.c 94.1% <100%> (+0.26%) ⬆️
src/cyclotom.c 92.21% <100%> (-2.38%) ⬇️
src/weakptr.c 100% <100%> (+1.48%) ⬆️
src/intrprtr.c 93.92% <100%> (-4.92%) ⬇️
... and 383 more

These were wrong since at least 1997...
Copy link
Contributor

@ChrisJefferson ChrisJefferson left a comment

Choose a reason for hiding this comment

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

This looks great.

This PR is so long I wanted to find a comment, just to prove I've read it all, but I didn't find one :)

I thought about this causing package failures, but really packages probably should be in general testing for errors in library code (although there might be some good exceptions of course).

@fingolfin fingolfin merged commit cb1f635 into gap-system:master Oct 29, 2018
@fingolfin fingolfin deleted the mh/smallint branch October 29, 2018 16:11
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Apr 15, 2019
@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
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: documentation Issues and PRs related to documentation topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants