-
-
Notifications
You must be signed in to change notification settings - Fork 667
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
Use runtime check instead compile time for Array#flat #1353
Conversation
MaxGraey
commented
Jun 24, 2020
- I've read the contributing guidelines
Solved most of problems. But found another edge case: export class ArrayArrayI32 extends Array<Array<i32>> {} Which print errors:
Which is make sense but I'm wondering could we also handle this case? Perhaps add some helpter like |
export class ArrayU8 extends Array<u8> {} | ||
export class ArrayStr extends Array<string> {} | ||
// TODO: | ||
// export class ArrayArrayI32 extends Array<Array<i32>> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's expected due to Array
hasn't comparison operators <
and >
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, hmm. Perhaps if we had a better standard than @operator
, like using unique symbols, one could check with isDefined
. Looks like this isn't easily solvable right now and requires a follow-up PR anyway, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it could be solved something like this:
function comparator<T>(a : T, b: T): i32 {
if (hasOperator<T>("<") && hasOperator<T>(">")) {
return i32(a > b) - i32(a < b);
} else {
throw new TypeError("type hasn't comparison operators");
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this isn't easily solvable right now and requires a follow-up PR anyway, though.
Yes, definitely it's for separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged this so we have a fix for the reported use case already. Let's think about improving the other cases in a follow-up :)
🎉 This PR is included in version 0.12.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Turns out that some compile time checks aren't all that clever :) |
@MaxGraey Do you think it can be reverted now because I have tried in the current compiler. It can pass the compile: class MyArray extends Array<Array<u8>> {}
export function _start(): void {
let a = new MyArray();
a.push([1])
a.push([2,3]);
let b = a.flat();
} |
Yeah, I guess we could revert this. PR are welcomed |