Skip to content

🐛 Fix MaxUint32 assignment to platform int#47

Merged
osteele merged 1 commit intoosteele:masterfrom
luminsmart-archive:master
Jul 5, 2021
Merged

🐛 Fix MaxUint32 assignment to platform int#47
osteele merged 1 commit intoosteele:masterfrom
luminsmart-archive:master

Conversation

@bendoerr
Copy link
Contributor

This fixes an issue where math.MaxUint32 is assigned to a platform
dependent int type. This works on 64-bit platforms without issue due to
there being plenty of space. On 32-bit platforms this is wrong and will
not compile as math.MaxUint32 > math.MaxInt32.

Checklist

  • I have read the contribution guidelines.
  • make test passes.
  • make lint passes.
  • New and changed code is covered by tests.
  • Performance improvements include benchmarks.
  • Changes match the documented (not just the implemented) behavior of Shopify.

This fixes an issue where math.MaxUint32 is assigned to a platform
dependent int type. This works on 64-bit platforms without issue due to
there being plenty of space. On 32-bit platforms this is wrong and will
not compile as math.MaxUint32 > math.MaxInt32.
@bendoerr
Copy link
Contributor Author

Would you like me to also open an associated issue?

@bendoerr
Copy link
Contributor Author

Looks like the build failed due to a missing golangci-lint.

golangci-lint run
make: golangci-lint: Command not found
Makefile:27: recipe for target 'lint' failed
make: *** [lint] Error 127
The command "make lint" exited with 2.

@osteele
Copy link
Owner

osteele commented Jun 27, 2021

Thanks. No need for a separate issue. I have ported the CI from Travis to GitHub Actions; if you rebase your work on master then the CI should pass.

Please also make the same change to expressions.y line 105. y.go is generated from this file.

@osteele osteele merged commit 0bc5e4c into osteele:master Jul 5, 2021
Copilot AI mentioned this pull request Nov 8, 2025
6 tasks
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.

2 participants