Skip to content

enable -d:nimPreviewFloatRoundtrip #19388

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

Closed
wants to merge 5 commits into from

Conversation

ringabout
Copy link
Member

@ringabout ringabout commented Jan 14, 2022

The test intends to find whether -d:nimPreviewFloatRoundtrip and -d:nimPreviewDotLikeOps work for important packages etc.

ref nim-lang/RFCs#437

The test intends to find whether -d:nimPreviewFloatRoundtrip and -d:nimPreviewDotLikeOps work for important packages etc.
@ringabout
Copy link
Member Author

datamancer failed to pass, so I made a PR there: SciNim/Datamancer#23

@arnetheduck
Copy link
Contributor

arnetheduck commented Jan 21, 2022

There's a problem with this approach: by making patches to important_packages to add incompatibility workarounds, you're at the same time distorting the view of the impact of the breaking changes: everything that is not in there will still break even though important_packages looks green on the surface - this in turn means that 2.0 becomes yet another "throw out all valid nim code out there and start over"-kind-of-release.

@ringabout
Copy link
Member Author

ringabout commented Jan 21, 2022

nimpreviewFloatRoundtrip makes float representation more precise, how can they be compatible with the algorithm before without introducing "incompatibility workarounds"?

@Araq
Copy link
Member

Araq commented Jan 22, 2022

everything that is not in there will still break even though important_packages looks green on the surface - this in turn means that 2.0 becomes yet another "throw out all valid nim code out there and start over"-kind-of-release.

Well 2.0 will break somebody's code in order to get a better language and library, that's the point of the exercise. The question is: "how much code?" and "how hard is it to fix?" And experiments with important_packages are providing valuable clues.

@arnetheduck
Copy link
Contributor

arnetheduck commented Jan 24, 2022

The question is: "how much code?" and "how hard is it to fix?" And experiments with important_packages are providing valuable clues.

yes, but by fixing them in upstream also distorts the picture of which packages continue to work - one way forward would be to document which important packages have been broken and required a fix: if you notice that after a year of fixing important packages to work with 2.0 almost every package needs a fix, you'll know that 2.0 breaks all nim code out there, and everyone else will have to spend years trying to adapt - we're spending several man-months of developer time trying to fix all things broken by 1.2 -> 1.6 alone - this is a significant cost when using nim at scale.

edit: a lot of information has already been lost this way btw: if you count all "make this work with orc/arc"-kind of PR:s in almost every single nim package out there, it's pretty grim - this kind of information is critical when making a go/no-go decision for using nim in a project.

@Araq
Copy link
Member

Araq commented Jan 24, 2022

f you notice that after a year of fixing important packages to work with 2.0 almost every package needs a fix, you'll know that 2.0 breaks all nim code out there, and everyone else will have to spend years trying to adapt - we're spending several man-months of developer time trying to fix all things broken by 1.2 -> 1.6 alone - this is a significant cost when using nim at scale.

A couple of remarks in mostly random order:

  1. If the survey results can be of any guide, complaints about code breakages have been few and far between. We try to keep up and improve further.
  2. Both 1.2 and 1.6 are LTS releases, take all the time you need to port the code.
  3. For 2.0 we can actually do better than for 1.6, in this case. If system.nim stopped providing $ for floats and instead offered a module of its own for that, the code stops compiling and the secret runtime change becomes a compile-time breaking change. What do you think?

@ringabout
Copy link
Member Author

ringabout commented Mar 9, 2022

update: SciNim/Datamancer#23 is merged

@ringabout ringabout changed the title examine whether experimental features work for important packages enable -d:nimPreviewFloatRoundtrip May 11, 2022
@ringabout
Copy link
Member Author

The PR is ready

@ringabout ringabout requested a review from Araq May 11, 2022 12:30
@ringabout ringabout mentioned this pull request May 11, 2022
33 tasks
@Araq
Copy link
Member

Araq commented May 11, 2022

@xflywind as I said, can we move $ for floats out of system.nim instead? We already did the same for IO for version 2.

@ringabout
Copy link
Member Author

Ok.

@ringabout ringabout self-assigned this May 20, 2022
@ringabout ringabout added the TODO: followup needed remove tag once fixed or tracked elsewhere label May 20, 2022
@ringabout
Copy link
Member Author

done. succeeded by #20185

@ringabout ringabout closed this Aug 9, 2022
@ringabout ringabout deleted the pr_experimental branch August 9, 2022 11:10
@ringabout ringabout removed the TODO: followup needed remove tag once fixed or tracked elsewhere label Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants