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

chore(remap, external docs): Document compile-time errors #6325

Merged
merged 15 commits into from
Feb 3, 2021

Conversation

binarylogic
Copy link
Contributor

Signed-off-by: binarylogic [email protected]

@binarylogic binarylogic requested review from a team and pablosichert and removed request for a team February 2, 2021 19:40
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Signed-off-by: binarylogic <[email protected]>
Copy link
Contributor

@pablosichert pablosichert left a comment

Choose a reason for hiding this comment

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

Looks good! Some smaller questions / suggestions.

Copy link
Contributor

@JeanMertz JeanMertz left a comment

Choose a reason for hiding this comment

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

Sweet!

Left a few comments, and we'll likely want to update the examples once all of this lands, but this is a great starting point.

Comment on lines +53 to +55
+if is_string(.message) {
downcase(.message)
+ }
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work yet. I'm not certain if we're tracking this in an issue yet, I believe we are? cc @FungusHumungus.

Comment on lines 46 to 47
+string!(.message)
downcase(.message)
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't work, because you still have to assign the result of the string call to have the compiler deduce the type:

.message = string!(.message)
downcase(.message)

or:

downcase(string!(.message))

While technically the current example could be made to work, I'm hesitant to do so, as it would mean one of two things:

  • we'd have to special-case string (etc) functions (basically converting them to a function-like built-in) to allow manipulating compiler type state, or
  • allow functions in general to manipulate type state, which I suspect would be a recipe for breaking core language safety guarantees by loading a badly implemented function.

@binarylogic
Copy link
Contributor Author

Thanks for the reviews. We will circle back and update the examples before launching.

@binarylogic binarylogic merged commit f6229e8 into master Feb 3, 2021
@binarylogic binarylogic deleted the remap-errors branch February 3, 2021 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants