-
Notifications
You must be signed in to change notification settings - Fork 68
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
Deprecate on 1.11 #523
Deprecate on 1.11 #523
Conversation
I suggest bumping the minor version number (since this is prerelease) as well |
That would be a breaking release, and require the whole ecosystem to go through a round of CompatHelper before PkgEval recovers (defeating most of the point of doing this). Seeing how this actively segfaults on 1.11 in several packages, I don't see why we should keep users from getting this fix automatically, especially if it doesn't affect semantics. EDIT: it asserts, doesn't segfault, but I don't think we can guarantee that the generate code does the right thing given the failed assertion. |
@chriselrod Can you approve this PR for CI to run, and if it passes, merge + tag? |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #523 +/- ##
==========================================
+ Coverage 80.31% 80.37% +0.06%
==========================================
Files 39 39
Lines 9604 9602 -2
==========================================
+ Hits 7713 7718 +5
+ Misses 1891 1884 -7 ☔ View full report in Codecov by Sentry. |
Well, this won't work either... See the report here: https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/2cbecf4_vs_18b4f3f/report.html AFAICT, it turns out
So packages are invoking At least the report also shows that LoopVectorization.jl is the source of 6 packages segfaulting, so getting it fixed (or successfully deprecated) for 1.11 is important. |
Regarding the change in semantics,
I doubt anyone other than me used |
I wonder if it might be a stopgap alternative to just deprecate on |
Not much code would be hitting this case. The only way that comes to mind is when passing an array to a function as an argument, without indexing the array as far as the macro can "see". |
7e86622
to
eeaa0b2
Compare
Fixed in a3e8081 |
Alternative for a4a160f, only disabling the macros but keeping all other functionality.
Fixes #520, re-fixes #518