Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2190373002:
Adjusted the spec to include access to initializing formals. (Closed)
|
Created:
4 years, 4 months ago by eernst Modified:
4 years, 4 months ago Reviewers:
Lasse Reichstein Nielsen CC:
reviews_dartlang.org, floitsch Base URL:
https://github.com/dart-lang/sdk.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionAdjusts the spec to include access to initializing formals.
Adresses https://github.com/dart-lang/sdk/issues/26659.
R=lrn@google.com
Committed: https://github.com/dart-lang/sdk/commit/9a1e050a85d8c37267e103c6f8fe6e5a853e4d65
Patch Set 1 #
Total comments: 6
Patch Set 2 : Review response #
Total comments: 6
Patch Set 3 : Review response #Messages
Total messages: 11 (3 generated)
eernst@google.com changed reviewers: + lrn@google.com
This CL specifies access to initializing formals. It includes normative text which implies the "sibling" scope structure that we have used to describe the setup in https://gist.github.com/eernstg/cff159be9e34d5ea295d8c24b1a3e594, but does not explicitly use the word "sibling". So it is precise, but it does not explicitly mention that this is an exception with respect to the scoping structure. However, a commentary gives the relevant derived facts: Name clashes include initializing formals, and initializing formals are accessible resp. inaccessible in the initializer list resp. the body. This gives the reader enough information both at the normative level and at the intuitive level, without loudly flagging the fact that it is an unusual scope structure.
https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.tex File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:1230: When the formal parameter list of a non-redirecting generative constructor contains any initializing formals a new scope is introduced, the {\em formal parameter initializer scope}, which is the current scope of the initializer list of the constructor, and which is enclosed in the scope where the constructor is declared. Each initializing formal \code{\THIS{}.id} in the formal parameter list introduces a formal parameter name into the formal parameter initializer scope, but not into the formal parameter scope. This does not specify what that name will be bound to in the new scope. This is a synthetic parameter, so it doesn't correspond to an actual argument (well, it could, but I guess it won't). That means that it's not clear what the scope binds the name to - is it a variable or a value or something else? The spec says that a scope induces a namespace that maps a name to a variable, type or function declaration, so in this case I guess it should map to a synthetic final variable. https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:1233: This means that formal parameters including initializing formals must have distinct names, and that initializing formals are in scope for initializers, but they are not in scope for the body of the constructor. I don't see how this can be concluded from the previous section. The spec allows nested scopes to contain the same name, so you have to explicitly write that code is not allowed to use the same name for an initializing formal and a non-initializing formal parameter. https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:1237: The type of the constructor function is derived from the formal parameter initializer scope such that it takes all parameters into account, including initializing formals. Is this necessary? (That is: Was the type of a constructor undefined before?) I think the type of the function should be derived from it's *declaration*, not the scopes it contains internally. For one thing, scopes are not ordered, so you can't derive the correct type for positional parameters from a scope.
Review response. https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.tex File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:1230: When the formal parameter list of a non-redirecting generative constructor contains any initializing formals a new scope is introduced, the {\em formal parameter initializer scope}, which is the current scope of the initializer list of the constructor, and which is enclosed in the scope where the constructor is declared. Each initializing formal \code{\THIS{}.id} in the formal parameter list introduces a formal parameter name into the formal parameter initializer scope, but not into the formal parameter scope. I think the binding is already specified in the specification: Section 9.2 has 'Every formal parameter introduces a local variable into the formal parameter scope'. So is it true that an initializing formal is a formal parameter? We have (9.2.1) that 'A required formal parameter may be specified in one of three ways', one of which is an initializing formal. The use of `fieldFormalParameter` in required + optional positional + named parameters makes it clear that all three work the same: _Every_ initializing formal is a formal parameter. So they 'introduce a local variable into the formal parameter scope.' This location is then adding an exception to this rule, both in the old and the new wording. However, they both have a problem: We need to specify _explicitly_ that this paragraph is an exception to the rule in section 9.2. We also need to clarify that the formal parameter initializer scope gets all formals, not just the initializing ones. Adjusted the paragraph to fix those issues. This also brought the terminology more in line with 9.2 (explicitly mentioning the local variable). https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:1233: This means that formal parameters including initializing formals must have distinct names, and that initializing formals are in scope for initializers, but they are not in scope for the body of the constructor. On 2016/08/08 08:22:06, Lasse Reichstein Nielsen wrote: > I don't see how this can be concluded from the previous section. > The spec allows nested scopes to contain the same name, so you have to > explicitly write that code is not allowed to use the same name for an > initializing formal and a non-initializing formal parameter. I think this problem has been resolved with the change I made to the previous paragraph: It explicitly says that all formals are added to the formal parameter initializer scope (and the general rule about name clashes hence applies for them all). There is no need to consider conflicts among nested scopes, because the formal parameter initializer scope and the formal parameter scope are siblings (same parent), not nested, and that hasn't been changed. https://codereview.chromium.org/2190373002/diff/1/docs/language/dartLangSpec.... docs/language/dartLangSpec.tex:1237: The type of the constructor function is derived from the formal parameter initializer scope such that it takes all parameters into account, including initializing formals. I think you are right: We can omit this. Sect. 9.3 specifies the type of a function based on its formal parameters, and we have just checked that initializing formals are formal parameters. That makes this sentence redundant already before the CL (so we don't need to discuss whether the pre-CL and the post-CL wordings are equivalent). Changed it into a part of the commentary above.
https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... docs/language/dartLangSpec.tex:1230: Initializing formals constitute an exception to the rule that every formal parameter introduces a local variable into the formal parameter scope (\ref{formalParameters}). When the formal parameter list of a non-redirecting generative constructor contains any initializing formals a new scope is introduced, the {\em formal parameter initializer scope}, which is the current scope of the initializer list of the constructor, and which is enclosed in the scope where the constructor is declared. Each initializing formal in the formal parameter list introduces a local variable into the formal parameter initializer scope, but not into the formal parameter scope; every other formal parameter introduces a local variable into both the formal parameter scope and the formal parameter initializer scope. Consider splitting this into more lines. I believe TeX isn't picky about that. Comma after "formals" in "any initializing formals, a new scope" Are we absolutely sure this is unambiguous, and can't be read in such a way that the variables introduced into both scopes by normal formal parameters are different variables (have separate storage)? (I admit that the existing description isn't exactly precise about bindings) https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... docs/language/dartLangSpec.tex:1233: This means that formal parameters including initializing formals must have distinct names, and that initializing formals are in scope for initializers, but they are not in scope for the body of the constructor. The type of the constructor is defined in terms of its formal parameters, including the initializing formals. commas around "including initializing formals". "in scope for initializers" -> "in scope for the initializer list" (I don't think we use "initializers" alone as a noun).
LGTM.
https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... docs/language/dartLangSpec.tex:1230: Initializing formals constitute an exception to the rule that every formal parameter introduces a local variable into the formal parameter scope (\ref{formalParameters}). When the formal parameter list of a non-redirecting generative constructor contains any initializing formals a new scope is introduced, the {\em formal parameter initializer scope}, which is the current scope of the initializer list of the constructor, and which is enclosed in the scope where the constructor is declared. Each initializing formal in the formal parameter list introduces a local variable into the formal parameter initializer scope, but not into the formal parameter scope; every other formal parameter introduces a local variable into both the formal parameter scope and the formal parameter initializer scope. I can't see anything saying that the initializing-formal formal variable is final.
Review response. https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... File docs/language/dartLangSpec.tex (right): https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... docs/language/dartLangSpec.tex:1230: Initializing formals constitute an exception to the rule that every formal parameter introduces a local variable into the formal parameter scope (\ref{formalParameters}). When the formal parameter list of a non-redirecting generative constructor contains any initializing formals a new scope is introduced, the {\em formal parameter initializer scope}, which is the current scope of the initializer list of the constructor, and which is enclosed in the scope where the constructor is declared. Each initializing formal in the formal parameter list introduces a local variable into the formal parameter initializer scope, but not into the formal parameter scope; every other formal parameter introduces a local variable into both the formal parameter scope and the formal parameter initializer scope. On 2016/08/23 06:55:20, Lasse Reichstein Nielsen wrote: > Consider splitting this into more lines. > I believe TeX isn't picky about that. TeX/LaTeX is perfectly happy (it takes two consecutive line breaks to start a new paragraph), but the current style in the spec uses a single line for an overwhelming majority of the paragraphs. The typical exception is where there is a paragraph and the paragraph ends in `\begin{itemize}..\end{itemize}`. In that case the whole itemize block is given separate lines. So I was just trying to avoid messing up the style too much. We could start using lots of line breaks. I'm quite happy with a format where each line is up to 80 chars and M-q in emacs is used to "fill" the paragraph now and then, but this tends to make diffs harder to read. One intermediate form would be where we tend to put each sentence on a new line (so period forces a line break). This would probably give us more readable diffs while still avoiding the hugely long lines. Just for taking a look at it, I used that style now, in text that is affected by this CL. > Comma after "formals" in "any initializing formals, a new scope" Done. > Are we absolutely sure this is unambiguous, and can't be read in such a way that > the variables introduced into both scopes by normal formal parameters are > different variables (have separate storage)? The spec already mentions that variables "are" storage locations. The current wording implies that it is one variable which is added to two scopes, so you could say that it is already disambiguated. Because of this, I just added a sentence to the commentary in order to make it explicit that it is one variable and one storage location. > (I admit that the existing description isn't exactly precise about bindings) Right, but I believe we should be OK in that respect now. https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... docs/language/dartLangSpec.tex:1230: Initializing formals constitute an exception to the rule that every formal parameter introduces a local variable into the formal parameter scope (\ref{formalParameters}). When the formal parameter list of a non-redirecting generative constructor contains any initializing formals a new scope is introduced, the {\em formal parameter initializer scope}, which is the current scope of the initializer list of the constructor, and which is enclosed in the scope where the constructor is declared. Each initializing formal in the formal parameter list introduces a local variable into the formal parameter initializer scope, but not into the formal parameter scope; every other formal parameter introduces a local variable into both the formal parameter scope and the formal parameter initializer scope. On 2016/08/23 07:15:03, Lasse Reichstein Nielsen wrote: > I can't see anything saying that the initializing-formal formal variable is > final. I believe that this has now been covered by adding the word final. https://codereview.chromium.org/2190373002/diff/20001/docs/language/dartLangS... docs/language/dartLangSpec.tex:1233: This means that formal parameters including initializing formals must have distinct names, and that initializing formals are in scope for initializers, but they are not in scope for the body of the constructor. The type of the constructor is defined in terms of its formal parameters, including the initializing formals. On 2016/08/23 06:55:20, Lasse Reichstein Nielsen wrote: > commas around "including initializing formals". Done. > "in scope for initializers" -> "in scope for the initializer list" > (I don't think we use "initializers" alone as a noun). Done.
Description was changed from ========== Adjusted the spec to include access to initializing formals. Adresses https://github.com/dart-lang/sdk/issues/26659. ========== to ========== Adjusts the spec to include access to initializing formals. Adresses https://github.com/dart-lang/sdk/issues/26659. ==========
Description was changed from ========== Adjusts the spec to include access to initializing formals. Adresses https://github.com/dart-lang/sdk/issues/26659. ========== to ========== Adjusts the spec to include access to initializing formals. Adresses https://github.com/dart-lang/sdk/issues/26659. R=lrn@google.com Committed: https://github.com/dart-lang/sdk/commit/9a1e050a85d8c37267e103c6f8fe6e5a853e4d65 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as 9a1e050a85d8c37267e103c6f8fe6e5a853e4d65 (presubmit successful). |
