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

kernel: remove partial floats in scanner; fix support for identifiers of length 1023 and more #2467

Merged
merged 5 commits into from
May 16, 2018

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented May 15, 2018

This finally gets rid of the remaining S_PARTIALFLOAT* scanner states, simplifying parsing/scanning of float literals further.

It also fixes our treatment of overlong identifiers: the manual has always claimed that these are truncated at 1023 chars, but instead truncated to 1022 chars. I changed this to match the manual, as there is really no technical reason to truncate at 1022, and no serious existing code can be affected negatively by adding support for slightly longer identifiers.

While at it, I also made some other minor scanner/reader cleanups.

Fixes #1104

@fingolfin fingolfin added topic: kernel release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels May 15, 2018
@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them and removed release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes labels May 15, 2018
@codecov
Copy link

codecov bot commented May 15, 2018

Codecov Report

Merging #2467 into master will increase coverage by 0.01%.
The diff coverage is 85.84%.

@@            Coverage Diff             @@
##           master    #2467      +/-   ##
==========================================
+ Coverage   74.01%   74.03%   +0.01%     
==========================================
  Files         484      484              
  Lines      245371   245274      -97     
==========================================
- Hits       181615   181581      -34     
+ Misses      63756    63693      -63
Impacted Files Coverage Δ
src/gapstate.h 100% <ø> (ø) ⬆️
src/scanner.h 100% <ø> (ø) ⬆️
src/read.c 94.61% <100%> (+4.12%) ⬆️
src/records.c 68.26% <100%> (ø) ⬆️
src/scanner.c 92.84% <82.92%> (+0.96%) ⬆️
src/intrprtr.c 94.35% <90%> (+0.23%) ⬆️
hpcgap/lib/hpc/stdtasks.g 63.42% <0%> (-0.52%) ⬇️
src/objset.c 84.82% <0%> (+0.22%) ⬆️

The manual says that identifiers are truncated to 1023 chars, i.e., an
identifier has *at most* 1023 chars. But some of our code instead
restricted them to *less* than 1023 chars, i.e., the maximum was 1022.

We now do what the manual has been saying since forever.
Copy link
Contributor

@stevelinton stevelinton 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 like a fantastic simplification

@fingolfin fingolfin merged commit 52528c0 into gap-system:master May 16, 2018
@fingolfin fingolfin deleted the mh/no-partial-float branch May 16, 2018 11:37
@fingolfin fingolfin added the release notes: added PRs introducing changes that have since been mentioned in the release notes label Sep 21, 2018
@olexandr-konovalov olexandr-konovalov changed the title kernel: remove partial floats in scanner; fix support for identifiers of length 1023; and more kernel: remove partial floats in scanner; fix support for identifiers of length 1023 and more Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Issues describing general bugs, and PRs fixing them 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.

Parsing very long literal sequences is broken in several ways
2 participants