FAQ
Reviewers: rsc,

Message:
Hello rsc@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
exp/types: type checking of type switches

Please review this at https://codereview.appspot.com/6846131/

Affected files:
M src/pkg/exp/types/expr.go
M src/pkg/exp/types/stmt.go
M src/pkg/exp/types/testdata/decls1.src
M src/pkg/exp/types/testdata/expr3.src
M src/pkg/exp/types/testdata/stmt0.src

Search Discussions

  • Rogpeppe at Nov 30, 2012 at 12:14 pm
    https://codereview.appspot.com/6846131/diff/7002/src/pkg/exp/types/stmt.go
    File src/pkg/exp/types/stmt.go (right):

    https://codereview.appspot.com/6846131/diff/7002/src/pkg/exp/types/stmt.go#newcode531
    src/pkg/exp/types/stmt.go:531: lhs.Type = typ
    doesn't this end up with the AST holding invalid types in all clauses
    but the last one?

    i suspect that if i typechecked this code and looked at the Object
    pointed to by the t argument to foo, i'd find it was string.

    switch t := interface{}(5).(type) {
    case int:
    foo(t)
    case string:
    bar(t)
    }

    https://codereview.appspot.com/6846131/
  • Gri at Nov 30, 2012 at 6:19 pm
    PTAL.


    https://codereview.appspot.com/6846131/diff/7002/src/pkg/exp/types/stmt.go
    File src/pkg/exp/types/stmt.go (right):

    https://codereview.appspot.com/6846131/diff/7002/src/pkg/exp/types/stmt.go#newcode531
    src/pkg/exp/types/stmt.go:531: lhs.Type = typ
    On 2012/11/30 12:14:35, rog wrote:
    doesn't this end up with the AST holding invalid types in all clauses but the
    last one?
    i suspect that if i typechecked this code and looked at the Object
    pointed to by
    the t argument to foo, i'd find it was string.
    switch t := interface{}(5).(type) {
    case int:
    foo(t)
    case string:
    bar(t)
    }
    That's correct - I was thinking about that last night. At the moment
    there's only one t object associated with all identifiers t. One could
    change that at the cost of an extra walk. However, the types associated
    with expressions as reported with the function f passed into types.Check
    is still correct (test needs to be added).

    I'm going to set the type to nil for now so that it cannot be used by
    mistake, and added a comment.

    In hindsight it would be better to not augment the AST at all (it leads
    to a cumbersome split between work done by the parser and work done
    here), and instead do it all in the type checker. It will be cleaner and
    lead to a leaner AST as well. It would also not raise questions as to
    what happens with scopes, objects, etc. if one manipulates the AST
    manually. It's best to rely on the results reported by the type checker
    if one is interested in the types. This will permit cleaning up the AST
    in a next phase (i.e., moving augmentation out).

    https://codereview.appspot.com/6846131/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 30, '12 at 4:06a
activeNov 30, '12 at 6:19p
posts3
users2
websitegolang.org

2 users in discussion

Gri: 2 posts Rogpeppe: 1 post

People

Translate

site design / logo © 2022 Grokbase