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

Backports for 1.5.2 #37210

Merged
merged 28 commits into from
Sep 15, 2020
Merged

Backports for 1.5.2 #37210

merged 28 commits into from
Sep 15, 2020

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Aug 26, 2020

Backported PRs:

Contains multiple commits, manual intervention needed:

Non-merged PRs with backport label:

JeffBezanson and others added 2 commits August 26, 2020 12:00
Removes ambiguity:
```
julia> Base.OrderStyle(Union{})
ERROR: MethodError: Base.OrderStyle(::Type{Union{}}) is ambiguous. Candidates:
```
This error is relevant as with current `unique!` definition that relies on `OrderStyle` one can have a problem in corner cases. E.g.:
```
julia> [i for i in ["1"] if i isa Int]
0-element Array{Union{},1}
```

Co-authored-by: Milan Bouchet-Valat <nalimilan@club.fr>
(cherry picked from commit 03e1a89)
@yuyichao
Copy link
Contributor

Seems that the back point of #37149 included more than just the bug fix. Only e change to gcutils.jl and secretbuffer.jl should be backported since those fixed the wrong return type from strlen call...

@KristofferC
Copy link
Member Author

Thanks, I missed that comment, will change it.

JeffBezanson and others added 2 commits September 6, 2020 16:23
We had been assuming that IdentityUnitRange matched the indices of the parent (like Slices) but they define their own offset axes. Further, other `AbstractRanges` may be similarly offset. We do not need to consider other offset `AbstractArrays` as this code only applies to `IndexLinear` `SubArray`s.

(cherry picked from commit 701885b)
@KristofferC KristofferC force-pushed the backports-release-1.5 branch 3 times, most recently from 2bfbb88 to 4ea0e8c Compare September 8, 2020 12:22
Copy link
Member

@martinholters martinholters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests from #37443 need a minor adjustment.

test/compiler/inference.jl Outdated Show resolved Hide resolved
test/compiler/inference.jl Outdated Show resolved Hide resolved
@JeffBezanson
Copy link
Member

There is a test failure here from 38c3e74. It looks like some part of dcc0696 is also needed to make that test work. That whole commit seems likely safe to backport though.

@JeffBezanson
Copy link
Member

This is the needed line:

--- a/base/compiler/inferencestate.jl
+++ b/base/compiler/inferencestate.jl
@@ -205,7 +205,7 @@ update_valid_age!(edge::InferenceState, sv::InferenceState) = update_valid_age!(
 function record_ssa_assign(ssa_id::Int, @nospecialize(new), frame::InferenceState)
     old = frame.src.ssavaluetypes[ssa_id]
     if old === NOT_FOUND || !(new ⊑ old)
-        frame.src.ssavaluetypes[ssa_id] = tmerge(old, new)
+        frame.src.ssavaluetypes[ssa_id] = old === NOT_FOUND ? new : tmerge(old, new)
         W = frame.ip
         s = frame.stmt_types

@martinholters
Copy link
Member

Ref. #37365 (comment) 😉

@KristofferC
Copy link
Member Author

Strange, I was sure I did that...

@KristofferC
Copy link
Member Author

@nanosoldier runbenchmarks(ALL, vs = ":release-1.5")

@KristofferC
Copy link
Member Author

@nanosoldier runtests(ALL, vs = ":release-1.5")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@KristofferC
Copy link
Member Author

https://github.com/JuliaCI/NanosoldierReports/blob/801671c4f9782fadd4f9b6718570c91a1414fd0e/pkgeval/by_hash/21d877d_vs_c4acbf9/logs/StatProfilerHTML/1.5.2-pre-be8475f41a.log

ERROR: LoadError: LoadError: syntax: invalid assignment location "#40#content..." around /home/pkgeval/.julia/packages/HAML/hMyIa/src/Templates.jl:49
Stacktrace:
 [1] top-level scope at none:1
 [2] eval at ./boot.jl:331 [inlined]
 [3] eval at ./Base.jl:39 [inlined]
 [4] _includehaml(::Module, ::Symbol, ::String, ::String) at /home/pkgeval/.julia/packages/HAML/hMyIa/src/Templates.jl:98
 [5] includehaml at /home/pkgeval/.julia/packages/HAML/hMyIa/src/Templates.jl:68 [inlined] (repeats 2 times)
 [6] #3 at /home/pkgeval/.julia/packages/HAML/hMyIa/src/Templates.jl:71 [inlined]
 [7] foreach at ./abstractarray.jl:2009 [inlined]
 [8] includehaml(::Module, ::Pair{Symbol,String}, ::Pair{Symbol,String}, ::Pair{Symbol,String}, ::Pair{Symbol,String}, ::Vararg{Pair{Symbol,String},N} where N) at /home/pkgeval/.julia/packages/HAML/hMyIa/src/Templates.jl:70
 [9] top-level scope at /home/pkgeval/.julia/packages/StatProfilerHTML/BuyY0/src/HTML.jl:12
 [10] include(::Function, ::Module, ::String) at ./Base.jl:380
 [11] include at ./Base.jl:368 [inlined]
 [12] include(::String) at /home/pkgeval/.julia/packages/StatProfilerHTML/BuyY0/src/StatProfilerHTML.jl:1
 [13] top-level scope at /home/pkgeval/.julia/packages/StatProfilerHTML/BuyY0/src/StatProfilerHTML.jl:6
 [14] include(::Function, ::Module, ::String) at ./Base.jl:380

looks a bit strange. @JeffBezanson?

@fredrikekre
Copy link
Member

Backport of #37315 should be HOME_PROJECT on 1.5 (#37315 (review))

KristofferC and others added 17 commits September 10, 2020 10:29
While in 1d08d70 we wanted to avoid
combining two types, we did not intend to prevent a type from being just
a pass-through value.

(cherry picked from commit a9743d8)
Fixes a crash when one of the incoming value for a union value phi node is of type `Union{}`.
Incoming value of this type can be generated by try-catch.

Fix #37265

(cherry picked from commit c3a63fc)
Unlike other nodes, Upsilon nodes can be potentially undefined and need
special handling to ensure they aren't literally undef, as we might try
to union-copy or gc-root the data later.

Fix #37262

(cherry picked from commit 5cf5717)
This is a compile error on 10+ and is probably triggering implicity conversion on LLVM 8 and 9.

(cherry picked from commit 5acd7bd)
The field should be either `Symbol` or `Int`. Ref #37423

Unlike #37423, in additional to worse type info in inference,
the missing type check here can actually cause type inference error
due to errors in user code.

(cherry picked from commit 1378bb6)
Fixes #37180.

(cherry picked from commit f047d7f)
In particular, make `obviously_unequal(Type{Union{}}, Type{T} where
{Union{}<:T<:Union{}})` false. Fixes #37255

(cherry picked from commit 79a9ea0)
JeffBezanson and others added 3 commits September 15, 2020 09:27
The undef ref error may be thrown at the wrong time or not at all and may cause crashes.

(cherry picked from commit 72854dc)
@KristofferC KristofferC added the release Release management and versioning. label Sep 15, 2020
@KristofferC
Copy link
Member Author

This should be good to go once CI finishes.

@KristofferC KristofferC merged commit 005352a into release-1.5 Sep 15, 2020
@KristofferC KristofferC deleted the backports-release-1.5 branch September 15, 2020 15:03
@KristofferC KristofferC changed the title WIP: Backports for 1.5.2 Backports for 1.5.2 Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release management and versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.