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

Missing coercion in ternary operator #1220

Closed
bbannier opened this issue Jun 23, 2022 · 2 comments · Fixed by #1259
Closed

Missing coercion in ternary operator #1220

bbannier opened this issue Jun 23, 2022 · 2 comments · Fixed by #1259
Assignees
Labels
Bug Something isn't working

Comments

@bbannier
Copy link
Member

bbannier commented Jun 23, 2022

When using a Null as a value in a ternary operator it does not seem to be coerced to the correct type.

module nil;

function g(): optional<uint64> {
	return True ? optional(10) : Null;
}

I would expect Null to be coerced to an optional<uint8> here but instead I see

$ spicyc -j nil.spicy
[error] nil.spicy:4:9: types of alternatives do not match in ternary expression (optional<uint<64>> vs. <null type>)
[error] spicyc: aborting after errors

At least in this example we should be able derive some target type from the first branch of the ternary. The same approach seems to break down for ternaries where Null appears as first branch; if we do not want to change how types for ternaries are resolved (e.g., from the use of the whole expression), we might need to introduce support for users to specify a type of a Null (currently we only support creating set optionals -- still with implicitly derived type).

The workaround is to use a typed Null value,

function g(): optional<uint64> {
	local nil: optional<uint64> = Null;
	return True ? optional(10) : nil;
}
@bbannier bbannier added the Bug Something isn't working label Jun 23, 2022
@rsmmr
Copy link
Member

rsmmr commented Aug 10, 2022

On further consideration, I think this is ok to leave as is, I don't think this actually a bug, C++ has the same issue when returning an optional vs std::nullopt through a ternary. I'll go ahead and close, reopen if you disagree.

@rsmmr rsmmr closed this as completed Aug 10, 2022
@rsmmr
Copy link
Member

rsmmr commented Aug 10, 2022

That said, #1143 needs coercion for ternary, too, so that fix will address this too.

@rsmmr rsmmr reopened this Aug 10, 2022
rsmmr added a commit that referenced this issue Aug 10, 2022
…er types.

There are some situations where that's fine to coerce, so adding that.
Also adding a coercion for ternary expressions that coerces the 2dn
expression into the type of the 1st (which we already assume provides
the result type).

Closes #1143.
Closes #1220.
@rsmmr rsmmr self-assigned this Aug 10, 2022
@rsmmr rsmmr closed this as completed in 14478b2 Aug 18, 2022
bbannier pushed a commit that referenced this issue Aug 22, 2022
…s of their inner types.

Also adding a coercion for ternary expressions that coerces the 2dn
expression into the type of the 1st (which we already assume provides
the result type).

Closes #1143.
Closes #1220.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants