-
-
Notifications
You must be signed in to change notification settings - Fork 36
Design document for variable mutability and namespacing #469
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
Conversation
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.
A first pass.
_What properties does the solution have to manifest to enable the use-cases above?_ | ||
|
||
- Be able to re-annotate variables without having to rename them in the message body | ||
- Allow static analysis to detect mistakes when referencing an undefined local variable |
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.
- Allow static analysis to detect mistakes when referencing an undefined local variable |
I don't think this should be considered as a requirement. At best it's a nice-to-have feature improving static analysis when a single translated message needs to be considered in isolation, and not with respect to its form in the source locale.
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.
Would you prefer a use case for this? This specific list was taken from @stam's comment in #310 near the end (after I had written a longer and more detailed set of requirements). The "static analysis" requirement is kind of "boiling down" the question of whether people will make mistakes with overwriting.
exploration/variable-mutability.md
Outdated
|
||
```abnf | ||
variable = local_var / external_var | ||
local_var = "@_" name |
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.
Yeah, no. We categorically do not need any two-character prefixes.
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.
See later discussion. This is horrifying while we agree on something...
> ``` | ||
> let @_local = {$external :transform} | ||
> modify @_local = {@_local :modification with=options} | ||
> modify $external = {$external :transform adding=annotation} |
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.
This looks an awful lot like we're doing pass-by-reference for formatting function arguments and we're modifying the $external
value in the calling context. Is that intentional?
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.
It intentionally looks like it is modifying the value. In practice, it (probably) does not have write access to the external value and is only masking it for the duration of this message. But the idea is that the value is mutable only if you are consciously mutating it.
Note that we could disallow modify
to externals, that is, you have to either let
or modify
a local variable in order to annotate an external var.
exploration/variable-mutability.md
Outdated
- `@@foo` | ||
- `@foo@` | ||
- `@!foo` | ||
- `@ONLY_UPPER_ASCII_SNAKE` |
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.
These are all pretty ugly, but I presume that's intentional. Do we not want users to use local variables? Because that's what we're saying by making them ugly and clumsy.
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.
Is there a beautiful way to make locale variables and not have them be confused with external variables "of the same name"?
An alternative not presented is the "rope merchant" option: do nothing (ugly or otherwise) and let people adopt whatever convention prevents mistakes, e.g. $foo
and #foo
are separate values (external vs. local).
Note: I just recalled that we wanted to use @
for annotation, so will change that to #
globally.
{"arg1": "10000"} | ||
... | ||
let $arg1 = {42} | ||
{This always says {$arg1} == 42} |
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.
The proposed design has this same flaw:
let @_arg1 = {42}
{This always says {@_arg1} == 42}
Anyone not working with MF2 daily will easily forget when looking at that message which prefix is for external and which for internal ones, leading to the exact same failure mode.
Is this the only argument against this alternative?
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.
The proposed design has the matching example in the next block, in which I call out the similarity error. But it isn't "the same flaw". In this case, the declaration completely blocks the original value.
This may not be super terrible. Not having separate namespaces is conceptually simpler and the ability to just annotate via declaration is much easier than doing multiple assignments and teaching developers/translators/etc. about local vs. external vars. Maybe modify
(however we choose to spell it in the end) solves the "accidental overwrite" problem and is enough?
``` | ||
{ "arg1": "37"} | ||
... | ||
let $arg1 = {|42| :number maxFractionDigits=2} // error |
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.
Not throwing an error for this should be considered a requirement.
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.
In this option, this is an error, because one is attempting to overwrite $arg1
with a new declaration. Presumably if we chose this option, it would be to ensure that this was an error.
I happen to agree that there is a use case that needs to be accounted for here (I have made the argument elsewhere many times about declarations not knowing what all the values are externally)
- change `@` to `#` because we want to use `@` for annotations such as `@locale` - Provide text that considers not making ugly local variables - Provide use cases for static analysis - Call out the perfidy of the author in stealing ill-baked requirements
Here's an alternative proposal with immutable variables that I think could work. I think someone else may have previously proposed something like this, but I can't find that source right away. Let us introduce a new keyword
This effectively replaces the current "hack" of saying At the syntax/data model level, In the ABNF the change would look like this: message = [s] *(input-annotation [s]) *(declaration [s]) body [s]
input-annotation = input [s] "{" [s] operand [s annotation] [s] "}"
input = %x69.6E.70.75.74 ; "input" The expression rule can't be used here directly because the operand is required. With this in place, I would support prohibiting re-assignment/shadowing, as long as a local The use case of chaining operations on a variable with a single name is not supported here, and the One observation I do have is that if our variables are immutable, then we should be able to reorder local variable declarations exactly like we allow for |
I think this is an interesting proposal. I am not sure that I'm excited about it, although it does solve annotation of external values.
This would bring back shadowing concerns, though, wouldn't it? For inputs
|
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
We briefly discussed defining input variables in #403 (comment). The, in #310 (comment) @aphillips suggested the I quite like the idea of Earlier in the thread, @eemeli commented on proposed syntax for local variables:
Maybe we actually don't want local variables to be common, especially if there's a convenient and dedicated solution to annotate input variables (like the
|
Not really, because each
So if the value gets determined in step 2, the corresponding input is ignored.
With logic as defined above, both of those would end up formatting as |
@eemeli noted:
I think I'm still worried about the conceptual simplicity element of this. For example, would this be technically allowed?
I put the reordered examples because people might make mistakes based on thinking of the lines as being executed in order. @stasm noted:
I wasn't keen on Stepping back: does everyone agree with the matrix of options in the document? Are there other options we want to consider? Comparing the options and weighting our reasons behind those choices seems useful |
exploration/variable-mutability.md
Outdated
The order of declarations does not matter, | ||
to allow for e.g. `input` annotations to refer to `local` variables. | ||
|
||
References to later declarations are not allowed, | ||
so this is considered an error: |
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.
This confuses me. If the order doesn't matter then forward definitions should work, but then you say that they don't (so the order would matter). I think the first paragraph might be a holdover from when you had input followed by local (the new ABNF allows intermixing: intentional?)
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.
We ended up determining that forward-referencing should not be allowed, because if we do allow it, someone not aware of that fact looking at the example message could easily presume that the first $bar
reference is to an external variable that's shadowed by the local $bar
declaration.
Co-authored-by: Addison Phillips <addison@unicode.org>
Specifically the one about forward references
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.
We're clearly already presuming this to be our syntax in our further work, so it'd be good to note this as "Accepted".
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
Co-authored-by: Eemeli Aro <eemeli@mozilla.com>
* Create notes-2023-10-02.md (#486) * Design document for variable mutability and namespacing (#469) * Design document for variable mutability and namespacing * style: Apply Prettier * Partly address #299 * style: Apply Prettier * Address comments, fix sigil choice - change `@` to `#` because we want to use `@` for annotations such as `@locale` - Provide text that considers not making ugly local variables - Provide use cases for static analysis - Call out the perfidy of the author in stealing ill-baked requirements * style: Apply Prettier * Add @eemelie's `input` proposal as an option considered * Update exploration/variable-mutability.md Co-authored-by: Eemeli Aro <eemeli@mozilla.com> * Add new proposed design * Update exploration/variable-mutability.md Co-authored-by: Addison Phillips <addison@unicode.org> * Address @eemeli's comments Specifically the one about forward references * style: Apply Prettier * Update exploration/variable-mutability.md Co-authored-by: Eemeli Aro <eemeli@mozilla.com> * Update exploration/variable-mutability.md Co-authored-by: Eemeli Aro <eemeli@mozilla.com> * Update exploration/variable-mutability.md Co-authored-by: Eemeli Aro <eemeli@mozilla.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Eemeli Aro <eemeli@mozilla.com> Co-authored-by: Eemeli Aro <eemeli@gmail.com> * Create notes-2023-10-09.md * Update notes-2023-10-09.md * Remove the Prettier push action (#491) Remove the Prettier lint action * Remove numbers from the existing design proposals (#490) --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Eemeli Aro <eemeli@mozilla.com> Co-authored-by: Eemeli Aro <eemeli@gmail.com> Co-authored-by: Stanisław Małolepszy <sta@malolepszy.org>
Per today's call (2023-09-04) here is the start of the design doc to address the need to decide how to handle:
This is based in large part on the discussion in #310 coupled with some discussion elsewhere. I incorporated commentary liberally.
The design presented here is for limited mutability (only via a separate keyword) and for separate namespaces.
Let the games begin.