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: revise formats supported by Pr and Emit #5556

Merged
merged 6 commits into from
Jan 4, 2024

Conversation

fingolfin
Copy link
Member

For some motivation and background, see also my comments in #5546

They were not used (anymore). Also remove the only other remaining
occurrence in src/compiler.c of %s.
Instead use %C in the single caller. This is arguably more correct
(except that %C itself is insufficient right now)
It was a rarely used synonym for %d
Also clarify %g documentation.
We used to only have %I which expects a C string; this was later joined
by %H which does the same but takes a GAP string. In the meantime, all
uses of %I were replaced by %H as in all cases this was necessary for GC
safety.

Since %I is for printing "identifiers" it seems sensible to keep that
format name instead of %H which was only used because H is the next
letter after I but which otherwise has no special meaning.
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.

Good cleanup. We should watch this doesn't break any packages (I don't expect it to, but with this kind of low level change weird stuff can always happen)

@ChrisJefferson ChrisJefferson merged commit adf04ef into gap-system:master Jan 4, 2024
22 checks passed
@fingolfin fingolfin deleted the mh/Pr branch January 5, 2024 09:37
@fingolfin
Copy link
Member Author

I did verify that no packages have kernel extensions calling Pr directly, but of course you are still right. That said, the PackageDistro CI checks still passed this morning

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release notes: not needed PRs introducing changes that are wholly irrelevant to the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants