Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2998173002:
Specify what "is equivalent to" means. (Closed)
|
Created:
3 years, 4 months ago by Lasse Reichstein Nielsen Modified:
3 years, 4 months ago Reviewers:
eernst CC:
reviews_dartlang.org, Brian Wilkerson Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionSpecify what "is equivalent to" means.
R=eernst@google.com
Committed: https://github.com/dart-lang/sdk/commit/3744197e3ce238c3efbc0dbdce7bb3ea8673ba26
Patch Set 1 #
Total comments: 23
Patch Set 2 : Address comments. #Messages
Total messages: 8 (2 generated)
lrn@google.com changed reviewers: + eernst@google.com
WDYT
LGTM! There's just one thing to ponder (5516): Did we just forget the static analysis of identifier expressions (alternatively, did we discover that it was never specified)? E.g., does the analyzer get to check `id` as `C.id` when `id` is a static variable/getter/setter which is used in the body of `C`? https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.tex File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:208: When the specification says that one piece of syntax "is equivalent to" another syntax, it means that it is equivalent in all ways, and the former syntax should generate the same static warnings and have the same runtime behavior as the latter. `"is equivalent to"` --> `{\em is equivalent to}`, for consistency with other defining occurrences of a phrase. Might as well say `piece of syntax` both times. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:210: If the syntax contains errors, implementations should obviously ensure that error messages only refer to the original syntax. Maybe `Error messages, if any, should always refer to the original syntax.` https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:212: If execution or evaluation of a construct is said to be equivalent with execution or evaluation of another construct, then only the runtime behavior is equivalent, and only the static warnings or errors mentioned for the original syntax applies. Might as well use `equivalent to` everywhere (`with` --> `to`). Typo at the end: `applies` --> `apply`. Then empty line before \section{..}. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:838: A {\bf defaultNamedParameter} of the form: I actually looked at this one at some point. Googling again for 'on the form' and 'of the form' yield exactly 'About 498.000.000 results', so the search must be abstracted into something like '{of,on} the form', but 'of the form' is described as 'In mathematics, the phrase "of the form" indicates that a mathematical object, or a collection of objects, follows a certain pattern of expression.', https://en.wikipedia.org/wiki/Of_the_form. So I concluded that 'of the form' is actually the standard phrase, even though I also thought at first that it was funny and 'on the form' would be more natural. Given that we have 194 'of the form' and 5 'on the form', I'd suggest that we just stick to 'of the form' everywhere. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:842: is equivalent to one of the form: Ditto. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:4991: A bitwise expression of the form $e_1$ $op$ $e_2$ is equivalent to the method invocation $e_1.op(e_2)$. Can't really see any changes, but we might as well fix the double space. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:4992: A bitwise expression of the form \code{\SUPER{} $op$ $e_2$} is equivalent to the method invocation \code{\SUPER{}.op($e_2$)}. Font typo: should be `$op$`. Double space. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5325: where $e_1$ is not a type literal, proceeds as follows: Delete `where $e_1$ is not a type literal, `: The text ahead makes sense if and only if it's deleted. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5343: where $e_1$ is not a type literal, proceeds as follows: Ditto: delete `where $e_1$ is not a type literal, `. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5516: \item If $d$ is the declaration of a static variable, static getter or static setter declared in class $C$, then evaluation of $e$ is equivalent to evaluation of the property extraction (\ref{propertyExtraction}) $C.id$. Doesn't this eliminate the ability of the static analysis to recognize that `id` is declared as a static variable/getter/setter, i.e., we can run it but it will be a compile-time error (or it will be spuriously resolved to a global declaration)? I guess we had that problem all along because this piece of text says `Evaluation of an identifier expression $e$ .. proceeds as follows:` (so it was always just about the dynamic semantics), but it would be nice to know that the spec actually ensures that static analysis treats this `id` as a static variable/getter/setter as well. Do we have that somewhere else? https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5519: \item Otherwise, evaluation of $e$ is equivalent to evaluation of the property extraction (\ref{propertyExtraction}) \THIS{}.$id$. Again, static analysis should also be able to see that some such inherited-member access expressions will succeed (if we just leave them unchanged during static analysis they will resolve to something wrong, or they will be undefined). https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5857: Execution of an if statement of the form \code {\IF{} (}$b$\code{)} $s_1$ \code{\ELSE{} } $s_2$ where $s_1$ and $s_2$ are block statements, proceeds as follows: Might as well keep the simpler "all \code" style that we've used in many other locations: `of the form \code{\IF{} ($b$) $s_1$ \ELSE{}}`.
Issue resolved IRL: Saying `evaluation of` about the identifier expressions is fine!
On 2017/08/21 10:34:37, eernst wrote: > Issue resolved IRL: Saying `evaluation of` about the identifier expressions is > fine! (and still LGTM)
https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.tex File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:208: When the specification says that one piece of syntax "is equivalent to" another syntax, it means that it is equivalent in all ways, and the former syntax should generate the same static warnings and have the same runtime behavior as the latter. On 2017/08/21 10:15:20, eernst wrote: > `"is equivalent to"` --> `{\em is equivalent to}`, for consistency with other > defining occurrences of a phrase. Might as well say `piece of syntax` both > times. Done. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:210: If the syntax contains errors, implementations should obviously ensure that error messages only refer to the original syntax. On 2017/08/21 10:15:20, eernst wrote: > Maybe `Error messages, if any, should always refer to the original syntax.` Done. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:212: If execution or evaluation of a construct is said to be equivalent with execution or evaluation of another construct, then only the runtime behavior is equivalent, and only the static warnings or errors mentioned for the original syntax applies. On 2017/08/21 10:15:21, eernst wrote: > Might as well use `equivalent to` everywhere (`with` --> `to`). > Typo at the end: `applies` --> `apply`. > Then empty line before \section{..}. Done. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:838: A {\bf defaultNamedParameter} of the form: Agree, and I have probably added all five of them. I'll just fix it all now. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:4991: A bitwise expression of the form $e_1$ $op$ $e_2$ is equivalent to the method invocation $e_1.op(e_2)$. I removed a leading space, but yes, fixing double-space too, and wrapping the code in \code. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:4992: A bitwise expression of the form \code{\SUPER{} $op$ $e_2$} is equivalent to the method invocation \code{\SUPER{}.op($e_2$)}. On 2017/08/21 10:15:20, eernst wrote: > Font typo: should be `$op$`. Double space. Done. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5325: where $e_1$ is not a type literal, proceeds as follows: On 2017/08/21 10:15:20, eernst wrote: > Delete `where $e_1$ is not a type literal, `: The text ahead makes sense if and > only if it's deleted. Done. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5343: where $e_1$ is not a type literal, proceeds as follows: On 2017/08/21 10:15:20, eernst wrote: > Ditto: delete `where $e_1$ is not a type literal, `. Done. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5516: \item If $d$ is the declaration of a static variable, static getter or static setter declared in class $C$, then evaluation of $e$ is equivalent to evaluation of the property extraction (\ref{propertyExtraction}) $C.id$. I'm not actually sure. I worried about the same thing, and we should look into it, but I decided, as you also say, that *this* change only affects the runtime semantics anyway, no matter what it purported to say before. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5519: \item Otherwise, evaluation of $e$ is equivalent to evaluation of the property extraction (\ref{propertyExtraction}) \THIS{}.$id$. On 2017/08/21 10:15:20, eernst wrote: > Again, static analysis should also be able to see that some such > inherited-member access expressions will succeed (if we just leave them > unchanged during static analysis they will resolve to something wrong, or they > will be undefined). Acknowledged. https://codereview.chromium.org/2998173002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:5857: Execution of an if statement of the form \code {\IF{} (}$b$\code{)} $s_1$ \code{\ELSE{} } $s_2$ where $s_1$ and $s_2$ are block statements, proceeds as follows: Agree. This was moved verbatim from earlier, but let's improve it while we're here.
Description was changed from ========== Specify what "is equivalent to" means. ========== to ========== Specify what "is equivalent to" means. R=eernst@google.com Committed: https://github.com/dart-lang/sdk/commit/3744197e3ce238c3efbc0dbdce7bb3ea8673ba26 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 3744197e3ce238c3efbc0dbdce7bb3ea8673ba26 (presubmit successful). |
