RFC: allow Tuple{Union{}}, returning Union{}

[vtjnash]

@oscardssmith requested that I put this up as a PR, where instead of throwing an exception, the call instead returns Union{}. It appears this is actually already consistent with the way we had been choosing to represent this particular operation in inference (specific to Tuple, since other types do not use exactly the same code path there), so no changes appear to be necessary there.

Fixes #52385

[ararslan]

I'd argue that this a bugfix and have marked it for backport to the active release branches accordingly.

[mbauman]

My main question is: is this enough of a bug fix? It doesn't completely fix all of the regressed use-cases identified in #52385. At a minimum, I think we also need to have NamedTuple{_, Union{}} similarly become Union{} — both StructArrays and TypedTables benefit from that.

Then the fixes required for StructArrays are two ambiguous methods:

staticschema(::Type{Union{}}) = Union{}
strip_params(::Type{Union{}}) = identity

And the fixes required for TypedTables tests is just an ambiguity/unbound typevar around printing:

columnnames(::AbstractArray{Union{}}) = ()
columnnames(::Table{<:Any,<:Any,<:NamedTuple{names}}) where {names} = names

[vtjnash]

Those seem like unambiguously bugfixes regardless of what we do with Tuple{Union{}}?

[aplavin]

How are these "unambiguously bugfixes" if these methods could not even be triggered before this change in Julia? (that's for StructArrays, not too familiar with TypedTables)

[vtjnash]

Whoops, Oscar and I thought no changes would be necessary in inference with this change, but I found a counter-example today demonstrating that apply_type_tfunc would not be sound after this change:

julia> const VT = Val{Tuple{T}} where T; code_typed((Type,)) do T; VT{T}; end
1-element Vector{Any}:
 CodeInfo(
    @ REPL[1]:1 within `#3`
1 ─ %1 = Core.apply_type(Main.VT, T)::Type{Val{Tuple{T}}} where T
└──      return %1
) => Type{Val{Tuple{T}}} where T

julia> Val{Union{}} isa Type{Val{Tuple{T}}} where T
false

[vtjnash]

How are these "unambiguously bugfixes"

The list of fixes from @mbauman above are equivalent to fixing the output of the ambiguity test checker. I suppose calling them a bugfix is a bit strong though, since it is not actually a bug to have ambiguities, even though their presence causes this test to fail.

julia> Test.detect_ambiguities(StructArrays, ambiguous_bottom=true)

I believe many packages use Aqua.test_ambiguities() to run this test as part of their normal CI and would consider having it passing to be a quality improvement over having it failing.

[mbauman]

Suggested change

	@testset "#52385" begin
	eagerzip() = error()
	function eagerzip(args::AbstractArray...)
	allequal(axes.(args)) || throw(DimensionMismatch())
	Base.require_one_based_indexing(args...)
	res = similar(first(args), Tuple{eltype.(args)...})
	for i in eachindex(args...)
	res[i] = getindex.(args, i)
	end
	res
	end
	@test isempty(eagerzip(Int[], []))
	@test eltype(eagerzip(Int[], [])) === Tuple{Int64, Any}
	@test isempty(eagerzip(Union{}[], []))
	@test eltype(eagerzip(Union{}[], [])) === Union{}
	end

Seems worthwhile to grab @LilithHafner's test case from #52385 (comment)

[aplavin]

The list of fixes from @mbauman above are equivalent to fixing the output of the ambiguity test checker.

How so? There is an issue on Aqua-detected ambiguities in StructArrays, and I don't see any overlap with @mbauman's methods: JuliaArrays/StructArrays.jl#293.

I believe many packages use Aqua.test_ambiguities() to run this test as part of their normal CI and would consider having it passing to be a quality improvement over having it failing.

Sure, it's better if there are no ambiguities in methods at all. But achieving this in practice is both nontrivial, and not even clearly useful.

For example, take StructArrays and the aforementioned issue on ambiguities. Those ambiguities don't even seem realizable in practice, and potential new methods added to resolve them would be dead weight (or even unreachable and untestable). This appears to be a very common position among devs: there are many issues with the same title by the same author over Julia packages, and almost all of them remain open.

Even worse, these ambiguity detectors don't catch some real problems. See JuliaArrays/StructArrays.jl#279 with a short example exhibiting the ambiguity. This issue is clearly relevant, and remains open just because it's not clear how to best resolve it.

This is somewhat off-topic wrt this issue, but yet, ambiguities are also a pain point (related to multiple dispatch) – but that's for real ambiguities that can actually be realized. Relationship with "detected" ones is unclear imo.

[vtjnash]

This is quite tangential now, but there is actually a performance benefit now to solving those, even though they may seem completely pointless, their presence documents to the compiler that they are pointless, which allows it to skip a lot of work when inferring or loading the package. The added method does need to a slightly peculiar exact form though, with a Vararg{Any} slurp argument immediately after the Type{Union{}} argument. Due to changes in morespecific matching, this method is also guaranteed to be more specific than any other, so that makes it actually possible to define this without just getting more ambiguities now. The relevant change for that feature addition was: #49470

Secondly, the default ambiguity checker is quite conservative, even though that means it may miss many relevant cases. You can make it less conservative by passing ambiguous_bottom=true. A fully non-conservative mode is likely also possible, if there is demand for that. In general currently the algorithm chooses to ignore most cases where ambiguities result from the heavy use of Union or other complex constraints, but we could report those cases as ambiguities instead when it detects them instead of choosing to suppress those when detected. Please file a separate feature request if you would consider that useful and I will see what I can do about it.