spec: section on variable initialization is unclear #6703
Labels
Milestone
Comments
The reporting of the cycle in the first case is definitely correct: that's how we wrote the spec, to avoid needing to analyze what is called and what is not. The second case should report a cycle; that's the bug. It probably has to do with adding the method value syntax and forgetting to update the cycle detector. Labels changed: added priority-later, go1.3, removed priority-triage. |
I'm not convinced anymore that the 2nd example is showing a bug, having implemented this
in go/types. I believe
T.m is a "mention" of method m, and so is
T.m()
but:
T{}.m and
T{}.m() are not.
The latter (T{}.m) use the type of a value to determine which m is denoted, the former
(T.m) is explicitly denoting the method m of T.
Or in other words, if the latter constitutes a mention, than the following code would
constitute one as well:
var t T
t.m
and there are plenty arbitrary complex expressions that evaluate to a value of type T
for which this would hold. It gets more interesting if T embeds a type E which has an m.
For an x of such a type T, does x.m constitute a "mention" of E.m?
Conclusions:
- The compiler is clearly inconsistent, but possibly the last example (T{}.m()) should
not report an error instead.
- The spec needs to be clarified.
|
The approach that go/types uses seems to be consistent, but it is not compatible with
math/rand. In this package there is:
var globalRand = New(&lockedSource{src: NewSource(1)})
// NewSource returns a new pseudo-random Source seeded with the given value.
func NewSource(seed int64) Source {
var rng rngSource
rng.Seed(seed)
return &rng
}
// Seed uses the provided seed value to initialize the generator to a deterministic
state.
func (rng *rngSource) Seed(seed int64) {
[...]
u ^= rng_cooked[i]
[...]
}
var (
// cooked random numbers
// the state of the rng
// after 780e10 iterations
rng_cooked [_LEN]int64 = [...]int64{
5041579894721019882, 4646389086726545243, 1395769623340756751, 5333664234075297259,
[...]
}
)
Here, go/types fails to see the dependency from globalRand to rng_cooked. It may occur
(in my case it did) that globalRand is initialized before rng_cooked and then Seed will
only use zeros instead of the cooked random numbers. What if the same happens to a
package like crypto/rand? Then the security of the package would be nondeterministically
broken. What I want to say is that even if the decision is that math/rand is wrong
according to the specification, it wouldn't be good to allow this kind of mistake so
easily. Maybe certain operations should be not permitted in initializers?
|
Matrix of cases with current compilers (gc, gccgo): http://play.golang.org/p/fMQHtWtjwV . Both compilers are consistent, but: 1) There are several cases (function/method not called) that cause a cycle error where there's technically no cycle. 2) There are several cases (method called) that cause a cycle error and the spec is unclear. |
Updated test cases with embedded types: http://play.golang.org/p/dRVF5CSNhK |
With complete commentary: http://play.golang.org/p/zrG20l-DMk Observations: - Both compilers agree. - Independent of what the spec says, in the majority of cases the compilers seem to consider whether a method is actually called in order to decide if there's an initialization cycle. - The cases marked as "odd" only check if a function/method is "mentioned". While that seems to be more closely following the wording in the spec, those cases technically do not represent cycles. My conclusions: 1) The cases marked as "odd" should be considered compiler bugs and no cycles should be reported in these cases. This would not invalidate existing programs and thus be a backward-compatible change. 2) The spec should be clarified: If a function/method is not called, there's no dependency of that function/method on the "contents of its body". This would document the current behavior. |
> 2) The spec should be clarified: If a function/method is not called, there's no dependency of that function/method on the "contents of its body". This would document the current behavior. What about the case where a function/method is not called upon mention, but the value to which it was assigned is later called? As in http://play.golang.org/p/2IvPRGSedt Here, there is a dependency on the "contents of its body", so there should be an init cycle error reported; but I think you are suggesting that, because the function is not called upon mention, there should be no error. |
Re: #12: We don't want to require data-flow analysis to determine initialization dependencies - it should still be fairly easily decidable statically, by looking at the types involved. The rephrasing I am suggesting is simply taking into account whether a function or method which we statically know (after type-checking) is actually called or not. In #11, conclusion 2) was not meant to be taken as the literal wording in the spec. |
Re: #13: If you want to determine whether a function or method is actually called or not, you have to do data-flow analysis. I agree that it is too much to require data-flow analysis. It is much easier to assume that any mention of a function or method (whether it is in a call expression or not) could (indirectly) lead to a call of it. This will result in false positives (functions which are mentioned but not called leading to an init cycle error), which are preferable to false negatives (functions which are mentioned and indirectly called but no init cycle error is reported). My point is that we should not take into account whether a function is immediately called or not, because even if it is not called immediately (if it is stored in a value) it might be called later (via that value). |
Re: #14. Got it, good point. Since a function that is not called may be assigned later to some variable (at which point we have lost the actual "identity" of the function), and that variable may be called, we would loose a dependency. So the conclusions in #11 should actually be quite the opposite: 1) The cases marked as "odd" at the moment are actually correct. All cases marked as "ok" at the moment should imply a (potential) cycle and are thus not handled correctly in the compilers. 2) The spec's notion of "mention" is the right one, but perhaps it should be phrased more clearly. |
CL in progress: https://golang.org/cl/99020043 Once the spec clarification is accepted, this issue can be closed and separate compiler issues filed. Status changed to Started. |
CL https://golang.org/cl/99020043 mentions this issue. |
|
Labels changed: added documentation, release-go1.3maybe, removed compilerbug, release-go1.4. Owner changed to @griesemer. |
|
Issue #7282 has been merged into this issue. |
|
This issue was closed by revision a436698. Status changed to Fixed. |
This issue was closed.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
This code: package p var x = T.m var y = x type T struct{} func (T) m() int { _ = y return 0 } complains about an initialization loop: x.go:3: initialization loop: x.go:3 x refers to x.go:9 T.m refers to x.go:5 y refers to x.go:3 x even though technically, there isn't one (m is not called, so x is simply the function T.m, and so is y. However, the spec only requires that the function T.m be mentioned for the cycle to occur, so reporting a cycle here is correct. But: package p var x = T{}.m // <<<< added {} var y = x type T struct{} func (T) m() int { _ = y return 0 } does not report a cycle, even though the compiler knows which m is meant with T{}.m. But if we call that m: package p var x = T{}.m() // <<<< added () var y = x type T struct{} func (T) m() int { _ = y return 0 } a cycle is reported again. The compiler should report a cycle for the 2nd program (per the spec), or not report a cycle for the first program (because in both cases, technically there is no cycle, and the compiler can know that). The latter would require a refinement of the spec (and permit more programs as a result, so this would be a backward-compatible change).The text was updated successfully, but these errors were encountered: