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

The ?? operator cannot return an optional #5944

Open
ShaiBer opened this issue Mar 14, 2024 · 9 comments · Fixed by #5984
Open

The ?? operator cannot return an optional #5944

ShaiBer opened this issue Mar 14, 2024 · 9 comments · Fixed by #5984
Labels
🐛 bug Something isn't working 🛠️ compiler Compiler 📐 language-design Language architecture

Comments

@ShaiBer
Copy link
Contributor

ShaiBer commented Mar 14, 2024

I tried this:

Wrote this code:

bring cloud;
let s1: str? = nil;
let s2: str? = nil;

let s3 = s1 ?? s2;

This happened:

Got this error:

Expected type to be "str", but got "str?" instead

It was on the part after the ?? operator.

Even explicitely defining that s3 is optional didn't help:
This code still produced the same error:

bring cloud;
let s1: str? = nil;
let s2: str? = nil;

let s3: str? = s1 ?? s2;

I expected this:

The code to compile an s3 to equal nil.

Is there a workaround?

let var s3 = s1;
if (s3 == nil){
  s3 = s2;
}

But it is uglier, and requires s3 to be set to var

Anything else?

I know it seems like a very small issue with a workaround, but for someone new to Wing, this can be the point that they lose interest and get back to a familiar language

Wing Version

0.61.1

Node.js Version

No response

Platform(s)

MacOS

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@ShaiBer ShaiBer added 🐛 bug Something isn't working 📐 language-design Language architecture 🛠️ compiler Compiler labels Mar 14, 2024
@Chriscbr
Copy link
Contributor

Totally agree with the confusion! We already have an issue so I'll close this as a duplicate -- I've gone ahead and added the other issue to our stability milestone (#1875)

@Chriscbr Chriscbr closed this as not planned Won't fix, can't repro, duplicate, stale Mar 14, 2024
@eladb
Copy link
Contributor

eladb commented Mar 19, 2024

Reopening this because the use case is a bit different from just chaining ??.

I expected that when y and z are both str?, then y ?? z will also resolve to str?:

let x: str? = y ?? z;

The use case is:

if let x = y ?? z {
  log("either y or z is defined");
} else {
  log("neither y nor z are defined");
}

Additionally, I'd like this to work:

let a: str = (y ?? z)!

@eladb eladb reopened this Mar 19, 2024
@yoav-steinberg
Copy link
Collaborator

yoav-steinberg commented Mar 19, 2024

Right, this is slightly different than #1875. In rust we differentiate the two use cases as:
unwrap_or that'll always evaluate to the inner value's type and or that'll always evaluate to the optional type.

I'm somewhat hesitant for using the same operator for both cases, since it feels weird and lacks explicity of what you're trying to do. Alternate syntax:

let x: str? = nil;
let y: str? = nil;
let default = "some default";

let or = x ?? y; // `or` is an optional.
let unwrap_or = x ?? y !? default; // `unwrap_or` is a str

@eladb
Copy link
Contributor

eladb commented Mar 19, 2024

@eladb
Copy link
Contributor

eladb commented Mar 19, 2024

Not sure I see the problem with using the same operator. The strong types will not allow developers to make mistakes here.

@Chriscbr
Copy link
Contributor

I'm somewhat hesitant for using the same operator for both cases, since it feels weird and lacks explicity of what you're trying to do.

I've been think about this a bit since the discussion in #5987. I can see how returning a non-optional could feel weird coming from Rust, if you associate ?? with unwrap_or. On the other hand, users coming from other languages (JavaScript, C#, Swift) where the ?? syntax already exists will probably be more surprised if this capability isn't supported (or if it requires a new operator, like !?) than if we conform to the existing semantics. Based on that, I think it makes sense to go forward and allow ?? to return optionals.

The way I'd reason about ?? is that it's a logical operator that returns the right hand operand when its left-hand operand is nil, otherwise it returns the left-hand operand. (We can update the docs to reflect this).

If someone writes a ?? b in their code and it produces str?, then they will still get a type checking error if they try assigning it to a field that expects str. So I don't think there's a loss in type safety here.

For comparison, there is a decent reason for restricting a to be an optional value in a ?? b. If a was non-optional, then a ?? b would always evaluate to a, implying that the ?? b part is just dead code. I think there's an argument to be made for changing this limitation to a warning or a lint rule instead, but so far I'm not sure it's been an issue.

@eladb
Copy link
Contributor

eladb commented Mar 19, 2024

+1 on what @Chriscbr said :-)

@yoav-steinberg
Copy link
Collaborator

yoav-steinberg commented Mar 19, 2024

Ok, I'm somewhat convinced, lets make sure there's a test that covers:

let a = nil;
let s = "string";
let b: str? = "string";
let x: str? = a ?? s; // or is this even an error? because of auto-wrapping 
                 //^ Error expected a `str?`
let y: str = a ?? b; // Error expected a `str`
                //^ Error expected a `str?`

But to nitpick on the subject, how about supporting (not really):

let x = "1" + 1;
assert(x == 2);
let y = 1 + "1";
assert(x == "11");
let z = 1 + 1 + "1";
assert(z == "21"); // or maybe "111", depending on right/left associativity 
let w = "1" + "1" + 1;
assert(w == 12); // or maybe "112"?, depending on right/left associativity

@mergify mergify bot closed this as completed in #5984 Mar 19, 2024
mergify bot pushed a commit that referenced this issue Mar 19, 2024
Fixes #5944

This improves the type checking for expressions of the form `a ?? b` in two ways:
1. `b` is allowed to be an optional. So for example, if `x` and `y` are both `num?`, then `x ?? y` is allowed, and its type of the resulting expression is `num?` (reflecting that you may still end up with a nil). 
2. The subtyping is more permissive. For example, suppose you have two classes where `Derived` is a derived class from `Base`. (In other words, `Derived` is a subtype of `Base`). All of these type combinations are allowed:
	a. Optional(Base) ?? Optional(Derived) = Optional(Base)
	b. Optional(Base) ?? Derived = Base
	c. Optional(Derived) ?? Optional(Base) = Optional(Base)
	d. Optional(Derived) ?? Base = Base

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [x] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
@monadabot
Copy link
Contributor

Congrats! 🚀 This was released in Wing 0.61.22.

@Chriscbr Chriscbr reopened this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🛠️ compiler Compiler 📐 language-design Language architecture
Projects
Status: 🤝 Backlog - handoff to owners
Development

Successfully merging a pull request may close this issue.

5 participants