Grokbase › Groups › Go › golang-dev › December 2012
FAQ
Reviewers: adonovan,

Message:
Hello adonovan@google.com (cc: golang-dev@googlegroups.com),

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


Description:
go/types: moved from exp/types

This is a just a file move with no other changes
besides the manual import path adjustments in these
two files:

src/pkg/exp/gotype/gotype.go
src/pkg/exp/gotype/gotype_test.go

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

Affected files:
M src/pkg/exp/gotype/gotype.go
M src/pkg/exp/gotype/gotype_test.go
M src/pkg/go/types/api.go
M src/pkg/go/types/builtins.go
M src/pkg/go/types/check.go
M src/pkg/go/types/check_test.go
M src/pkg/go/types/const.go
M src/pkg/go/types/conversions.go
M src/pkg/go/types/errors.go
M src/pkg/go/types/exportdata.go
M src/pkg/go/types/expr.go
M src/pkg/go/types/gcimporter.go
M src/pkg/go/types/gcimporter_test.go
M src/pkg/go/types/operand.go
M src/pkg/go/types/predicates.go
M src/pkg/go/types/resolver_test.go
M src/pkg/go/types/stmt.go
M src/pkg/go/types/testdata/builtins.src
M src/pkg/go/types/testdata/const0.src
M src/pkg/go/types/testdata/conversions.src
M src/pkg/go/types/testdata/decls0.src
M src/pkg/go/types/testdata/decls1.src
M src/pkg/go/types/testdata/decls2a.src
M src/pkg/go/types/testdata/decls2b.src
M src/pkg/go/types/testdata/decls3.src
M src/pkg/go/types/testdata/exports.go
M src/pkg/go/types/testdata/expr0.src
M src/pkg/go/types/testdata/expr1.src
M src/pkg/go/types/testdata/expr2.src
M src/pkg/go/types/testdata/expr3.src
M src/pkg/go/types/testdata/stmt0.src
M src/pkg/go/types/types.go
M src/pkg/go/types/types_test.go
M src/pkg/go/types/universe.go


Index: src/pkg/exp/gotype/gotype.go
===================================================================
--- a/src/pkg/exp/gotype/gotype.go
+++ b/src/pkg/exp/gotype/gotype.go
@@ -6,13 +6,13 @@

import (
"errors"
- "exp/types"
"flag"
"fmt"
"go/ast"
"go/parser"
"go/scanner"
"go/token"
+ "go/types"
"io/ioutil"
"os"
"path/filepath"
Index: src/pkg/exp/gotype/gotype_test.go
===================================================================
--- a/src/pkg/exp/gotype/gotype_test.go
+++ b/src/pkg/exp/gotype/gotype_test.go
@@ -117,7 +117,6 @@
"flag",
"fmt",

- "exp/types",
"exp/gotype",

"go/ast",
@@ -128,6 +127,7 @@
"go/printer",
"go/scanner",
// "go/token",
+ "go/types",

"hash/adler32",
"hash/crc32",
Index: src/pkg/go/types/api.go
===================================================================
rename from src/pkg/exp/types/api.go
rename to src/pkg/go/types/api.go
Index: src/pkg/go/types/builtins.go
===================================================================
rename from src/pkg/exp/types/builtins.go
rename to src/pkg/go/types/builtins.go
Index: src/pkg/go/types/check.go
===================================================================
rename from src/pkg/exp/types/check.go
rename to src/pkg/go/types/check.go
Index: src/pkg/go/types/check_test.go
===================================================================
rename from src/pkg/exp/types/check_test.go
rename to src/pkg/go/types/check_test.go
Index: src/pkg/go/types/const.go
===================================================================
rename from src/pkg/exp/types/const.go
rename to src/pkg/go/types/const.go
Index: src/pkg/go/types/conversions.go
===================================================================
rename from src/pkg/exp/types/conversions.go
rename to src/pkg/go/types/conversions.go
Index: src/pkg/go/types/errors.go
===================================================================
rename from src/pkg/exp/types/errors.go
rename to src/pkg/go/types/errors.go
Index: src/pkg/go/types/exportdata.go
===================================================================
rename from src/pkg/exp/types/exportdata.go
rename to src/pkg/go/types/exportdata.go
Index: src/pkg/go/types/expr.go
===================================================================
rename from src/pkg/exp/types/expr.go
rename to src/pkg/go/types/expr.go
Index: src/pkg/go/types/gcimporter.go
===================================================================
rename from src/pkg/exp/types/gcimporter.go
rename to src/pkg/go/types/gcimporter.go
Index: src/pkg/go/types/gcimporter_test.go
===================================================================
rename from src/pkg/exp/types/gcimporter_test.go
rename to src/pkg/go/types/gcimporter_test.go
Index: src/pkg/go/types/operand.go
===================================================================
rename from src/pkg/exp/types/operand.go
rename to src/pkg/go/types/operand.go
Index: src/pkg/go/types/predicates.go
===================================================================
rename from src/pkg/exp/types/predicates.go
rename to src/pkg/go/types/predicates.go
Index: src/pkg/go/types/resolver_test.go
===================================================================
rename from src/pkg/exp/types/resolver_test.go
rename to src/pkg/go/types/resolver_test.go
Index: src/pkg/go/types/stmt.go
===================================================================
rename from src/pkg/exp/types/stmt.go
rename to src/pkg/go/types/stmt.go
Index: src/pkg/go/types/testdata/builtins.src
===================================================================
rename from src/pkg/exp/types/testdata/builtins.src
rename to src/pkg/go/types/testdata/builtins.src
Index: src/pkg/go/types/testdata/const0.src
===================================================================
rename from src/pkg/exp/types/testdata/const0.src
rename to src/pkg/go/types/testdata/const0.src
Index: src/pkg/go/types/testdata/conversions.src
===================================================================
rename from src/pkg/exp/types/testdata/conversions.src
rename to src/pkg/go/types/testdata/conversions.src
Index: src/pkg/go/types/testdata/decls0.src
===================================================================
rename from src/pkg/exp/types/testdata/decls0.src
rename to src/pkg/go/types/testdata/decls0.src
Index: src/pkg/go/types/testdata/decls1.src
===================================================================
rename from src/pkg/exp/types/testdata/decls1.src
rename to src/pkg/go/types/testdata/decls1.src
Index: src/pkg/go/types/testdata/decls2a.src
===================================================================
rename from src/pkg/exp/types/testdata/decls2a.src
rename to src/pkg/go/types/testdata/decls2a.src
Index: src/pkg/go/types/testdata/decls2b.src
===================================================================
rename from src/pkg/exp/types/testdata/decls2b.src
rename to src/pkg/go/types/testdata/decls2b.src
Index: src/pkg/go/types/testdata/decls3.src
===================================================================
rename from src/pkg/exp/types/testdata/decls3.src
rename to src/pkg/go/types/testdata/decls3.src
Index: src/pkg/go/types/testdata/exports.go
===================================================================
rename from src/pkg/exp/types/testdata/exports.go
rename to src/pkg/go/types/testdata/exports.go
Index: src/pkg/go/types/testdata/expr0.src
===================================================================
rename from src/pkg/exp/types/testdata/expr0.src
rename to src/pkg/go/types/testdata/expr0.src
Index: src/pkg/go/types/testdata/expr1.src
===================================================================
rename from src/pkg/exp/types/testdata/expr1.src
rename to src/pkg/go/types/testdata/expr1.src
Index: src/pkg/go/types/testdata/expr2.src
===================================================================
rename from src/pkg/exp/types/testdata/expr2.src
rename to src/pkg/go/types/testdata/expr2.src
Index: src/pkg/go/types/testdata/expr3.src
===================================================================
rename from src/pkg/exp/types/testdata/expr3.src
rename to src/pkg/go/types/testdata/expr3.src
Index: src/pkg/go/types/testdata/stmt0.src
===================================================================
rename from src/pkg/exp/types/testdata/stmt0.src
rename to src/pkg/go/types/testdata/stmt0.src
Index: src/pkg/go/types/types.go
===================================================================
rename from src/pkg/exp/types/types.go
rename to src/pkg/go/types/types.go
Index: src/pkg/go/types/types_test.go
===================================================================
rename from src/pkg/exp/types/types_test.go
rename to src/pkg/go/types/types_test.go
Index: src/pkg/go/types/universe.go
===================================================================
rename from src/pkg/exp/types/universe.go
rename to src/pkg/go/types/universe.go

Search Discussions

  • Adonovan at Dec 28, 2012 at 7:20 pm ⇧
    The code change looks good. :) I'm not sure I have the power to
    approve it though.

    Also: is it clear to all observers that this rename doesn't imply the
    API is nailed down into its final form?





    https://codereview.appspot.com/7013049/
  • Adonovan at Dec 28, 2012 at 7:39 pm ⇧
    LGTM

    https://codereview.appspot.com/7013049/
  • Gri at Dec 28, 2012 at 7:41 pm ⇧
    *** Submitted as
    https://code.google.com/p/go/source/detail?r=8ea98ec93704 ***

    go/types: moved from exp/types

    This is a just a file move with no other changes
    besides the manual import path adjustments in these
    two files:

    src/pkg/exp/gotype/gotype.go
    src/pkg/exp/gotype/gotype_test.go

    Note: The go/types API continues to be subject to
    possibly significant changes until Go 1.1. Do not
    rely on it being stable at this point.

    R=adonovan
    CC=golang-dev
    https://codereview.appspot.com/7013049


    https://codereview.appspot.com/7013049/
  • Andrew Wilkins at Dec 30, 2012 at 8:46 am ⇧

    On Saturday, 29 December 2012 03:41:53 UTC+8, gri wrote:

    Note: The go/types API continues to be subject to
    possibly significant changes until Go 1.1. Do not
    rely on it being stable at this point.
    Really glad to see this is coming along - thank you! I realise things are
    still changing, but I've started working on using go/types in llgo, and
    ditching the old version of exp/types I've been using and extending. I have
    a couple of questions:

    1) When go/types is complete, will untyped "types" escape from go/types?

    I'm using Context.Expr to store the type of each expression, which I'll
    later use to create an LLVM constant. Right now, constants' types are
    "untyped" when they're passed to Context.Expr, as it is invoked before the
    convertUntyped is called. Unless there's a change, I will have to reproduce
    the logic for convertUntyped; not a big deal, but it seems like something
    that go/types should help with. Maybe I'm just going about things the wrong
    way?

    2) What's the best way of locating the original *ast.Object for the "..."
    parameter of a variadic function?

    This is the same issue I wrote about back when I submitted some changes to
    exp/types. When llgo processes a FuncDecl, it creates a new, corresponding
    LLVM function, and allocates stack for each parameter and named result. The
    problem is how to associate the stack space with the identifiers inside the
    function body.

    In the case of a non-variadic parameter the variable in the function body
    have the same *ast.Object, so I can modify the Data field of the parameter
    and I'm done. In the case of a variadic parameter, I need to locate the
    function-body variable's *ast.Object given the parameter's *ast.Object to
    do the same thing. There are of course ways to do this in llgo: (1) look
    for the first non-shadowing identifier in the body with the same name as
    the parameter; or (2) before type-checking, walk the AST and record the
    original *ast.Object for variadic parameters, and then after type-checking
    record an association between that and the new copy. They both seem a bit
    convoluted.

    Is there any chance of recording the association somewhere during
    type-checking? Maybe the Data field of the parameter's *ast.Object copy
    could store the original one?

    Cheers,
    Andrew
  • Alan Donovan at Dec 30, 2012 at 6:25 pm ⇧

    On 30 December 2012 00:53, Andrew Wilkins wrote:
    On Saturday, 29 December 2012 03:41:53 UTC+8, gri wrote:

    Note: The go/types API continues to be subject to
    possibly significant changes until Go 1.1. Do not
    rely on it being stable at this point.
    Really glad to see this is coming along - thank you! I realise things are
    still changing, but I've started working on using go/types in llgo, and
    ditching the old version of exp/types I've been using and extending. I have
    a couple of questions:

    1) When go/types is complete, will untyped "types" escape from go/types?
    Yes, they must. While most constants are used exactly once in a context
    implying a specific representation, some constants are shared among varied
    uses. For example, the type of a const declaration such as:

    const x = 1/3

    is an "untyped" floating point number whose value is exactly one third. It
    may be used in both float32 and float64 contexts. "Untyped" is really a
    misnomer for a limited form of subtyping applicable only to types of
    constant expressions.


    2) What's the best way of locating the original *ast.Object for the "..."
    parameter of a variadic function?

    This is the same issue I wrote about back when I submitted some changes to
    exp/types. When llgo processes a FuncDecl, it creates a new, corresponding
    LLVM function, and allocates stack for each parameter and named result. The
    problem is how to associate the stack space with the identifiers inside the
    function body.

    In the case of a non-variadic parameter the variable in the function body
    have the same *ast.Object, so I can modify the Data field of the parameter
    and I'm done. In the case of a variadic parameter, I need to locate the
    function-body variable's *ast.Object given the parameter's *ast.Object to
    do the same thing. There are of course ways to do this in llgo: (1) look
    for the first non-shadowing identifier in the body with the same name as
    the parameter; or (2) before type-checking, walk the AST and record the
    original *ast.Object for variadic parameters, and then after type-checking
    record an association between that and the new copy. They both seem a bit
    convoluted.
    Avoid approach (1). If you're using name- (string) based lookups after the
    typechecking phase, you're doing something wrong.

    In my client of go/types---an SSA builder, no doubt relevant to your llgo
    work; will be ready for review in a week or two---I retain the syntax of
    ast.Func{Decl,Lit}.Type.Params.List when I visit each function since the
    ast.Objects associated with types.Signature.Params are not identical to
    them in the variadic case, and are thus not suitable for creating the local
    variables that will be referenced in the function body. I agree it's a bit
    of nuisance. I think there's a more consistent approach based on (in
    go/types) changing the type of the variadic argument to a slice of the
    apparent type (and setting the isVariadic bit). It would move the burden
    from the client into the typechecker (not a lot either way) but would
    simplify the API.

    cheers
    alan
  • Andrew Wilkins at Dec 31, 2012 at 1:21 am ⇧

    On Mon, Dec 31, 2012 at 2:25 AM, Alan Donovan wrote:

    1) When go/types is complete, will untyped "types" escape from go/types?
    Yes, they must. While most constants are used exactly once in a context
    implying a specific representation, some constants are shared among varied
    uses.
    Alan, thanks for your reply. Sorry, I understand that in the case of a
    shared constant it will have to be "untyped". I will try to clarify what I
    want below.

    For example, the type of a const declaration such as:

    const x = 1/3

    is an "untyped" floating point number whose value is exactly one third.
    It may be used in both float32 and float64 contexts. "Untyped" is really a
    misnomer for a limited form of subtyping applicable only to types of
    constant expressions.
    Each use, however, introduces a new *ast.Ident, with which one might
    associate a different type. e.g.:

    const x = 1/3
    var y = x // x converted to default type, int
    var z uint64 = x // x converted to uint64
    etc.

    In the two var assignments, we get two different *ast.Idents. I'd like for
    the type of the rhs to have a concrete type. Also, if I use a constant in a
    non-constant expression, it cannot be shared so its type could also be
    safely fixed. e.g.:

    var y uint
    var x uint64 = 123 << y

    The "123"'s *ast.BasicLit could have its type changed to uint64.
    Is this not feasible? I apologise if I'm missing something obvious; I'm a
    novice in this field.

    2) What's the best way of locating the original *ast.Object for the "..."
    parameter of a variadic function?

    This is the same issue I wrote about back when I submitted some changes
    to exp/types. When llgo processes a FuncDecl, it creates a new,
    corresponding LLVM function, and allocates stack for each parameter and
    named result. The problem is how to associate the stack space with the
    identifiers inside the function body.

    In the case of a non-variadic parameter the variable in the function body
    have the same *ast.Object, so I can modify the Data field of the parameter
    and I'm done. In the case of a variadic parameter, I need to locate the
    function-body variable's *ast.Object given the parameter's *ast.Object to
    do the same thing. There are of course ways to do this in llgo: (1) look
    for the first non-shadowing identifier in the body with the same name as
    the parameter; or (2) before type-checking, walk the AST and record the
    original *ast.Object for variadic parameters, and then after type-checking
    record an association between that and the new copy. They both seem a bit
    convoluted.
    Avoid approach (1). If you're using name- (string) based lookups after
    the typechecking phase, you're doing something wrong.

    In my client of go/types---an SSA builder, no doubt relevant to your llgo
    work; will be ready for review in a week or two---I retain the syntax of
    ast.Func{Decl,Lit}.Type.Params.List when I visit each function since the
    ast.Objects associated with types.Signature.Params are not identical to
    them in the variadic case, and are thus not suitable for creating the local
    variables that will be referenced in the function body. I agree it's a bit
    of nuisance. I think there's a more consistent approach based on (in
    go/types) changing the type of the variadic argument to a slice of the
    apparent type (and setting the isVariadic bit). It would move the burden
    from the client into the typechecker (not a lot either way) but would
    simplify the API.
    I had suggested this change earlier, but gri wasn't keen at the time. I
    probably didn't enunciate my reasons very well. Looking forward to seeing
    your work, it sounds interesting.

    Cheers,
    Andrew
  • Alan Donovan at Dec 31, 2012 at 2:10 am ⇧

    On 30 December 2012 20:21, Andrew Wilkins wrote:
    On Mon, Dec 31, 2012 at 2:25 AM, Alan Donovan wrote:

    1) When go/types is complete, will untyped "types" escape from go/types?
    Yes, they must. While most constants are used exactly once in a context
    implying a specific representation, some constants are shared among varied
    uses.
    Alan, thanks for your reply. Sorry, I understand that in the case of a
    shared constant it will have to be "untyped". I will try to clarify what I
    want below.

    For example, the type of a const declaration such as:

    const x = 1/3

    is an "untyped" floating point number whose value is exactly one third.
    It may be used in both float32 and float64 contexts. "Untyped" is really a
    misnomer for a limited form of subtyping applicable only to types of
    constant expressions.
    Each use, however, introduces a new *ast.Ident, with which one might
    associate a different type. e.g.:

    const x = 1/3
    var y = x // x converted to default type, int
    var z uint64 = x // x converted to uint64
    etc.

    In the two var assignments, we get two different *ast.Idents. I'd like for
    the type of the rhs to have a concrete type. Also, if I use a constant in a
    non-constant expression, it cannot be shared so its type could also be
    safely fixed. e.g.:

    var y uint
    var x uint64 = 123 << y
    The "123"'s *ast.BasicLit could have its type changed to uint64.
    >

    Idents are connected to the ast.Object of the underlying declaration, and
    that's where the type is located, so all references to a shared const
    declaration x necessarily have the same type.

    In the case of var z uint64 = x, I think the intention is that the implicit
    conversion from untyped integer to uint64 is handled by the client as part
    of the assignment, since all assignments (including implicit ones arising
    in such constructs as parameter passing, channel send, etc) must handle
    various kinds of conversions anyway. But gri is the expert here.

    (As an aside, this declaration:
    const z float64 = x
    should cause a value conversion from exact fraction to IEEE 754
    double-precision floating point, but not a type conversion. But this is
    not yet implemented and is waiting upon a pending change of mine that
    implements big.Rat <-> float conversion)

    cheers
    alan
  • Andrew Wilkins at Dec 31, 2012 at 9:08 am ⇧
    Oops, I meant to reply-all.

    Cheers,
    Andrew

    ---------- Forwarded message ----------
    From: Andrew Wilkins <axwalk@gmail.com>
    Date: Mon, Dec 31, 2012 at 5:07 PM
    Subject: Re: [golang-dev] Re: code review 7013049: go/types: moved from
    exp/types (issue 7013049)
    To: Alan Donovan <adonovan@google.com>

    On Mon, Dec 31, 2012 at 10:10 AM, Alan Donovan wrote:

    Idents are connected to the ast.Object of the underlying declaration, and
    that's where the type is located, so all references to a shared const
    declaration x necessarily have the same type.
    Sorry, I was being unclear again. Here's my final attempt at explaining
    myself for 2012.
    What I mean is, I would like Context.Expr to report the converted type for
    each *ast.Ident (likewise for *ast.BasicLit, and complex constant
    expressions) in the context in which it was used. The ast.Ident's Obj.Type
    need not be modified. e.g.:

    const x = 123
    var y = x
    var z uint64 = x

    go/parser will generate two AssignStmts (or VarDecls) for the vars y and z,
    each of which has a separate *ast.Ident for the rhs expr, with a common
    *ast.Object.
    When go/types checks the AssignStmt for "var y", it will determine that x
    should be converted to an int. For "var z", it should be converted to
    uint64. I would like for Context.Expr to be called like so:
    Context.Expr(ident1, types.Typ[types.Int], int64(123))
    Context.Expr(ident2, types.Typ[types.Uint64], int64(123))
    where ident1 and ident2 are the two *ast.Idents referring to "x". Currently
    it reports "UntypedInteger" as the second argument.

    For llgo, I'm doing something like this (pseudo-code, I don't have the real
    code handy right now):

    exprtypes := make(map[ast.Expr]Type)
    constvals := make(map[ast.Expr]interface{})
    ctx := &types.Context{
    Expr: func(x ast.Expr, typ Type, val interface{}) {
    exprtypes[x] = typ
    if val != nil {constvals[x] = val}
    },
    }

    Then when I walk the AST and come across an ast.Expr, I do this:
    Check if it's a constant (lookup constvals), and if so:
    1) Get type from exprtypes.
    2) Create the LLVM constant, and convert it to the type from step 1.

    The problem being that the type in exprtypes is the "untyped" type. So
    currently I'm having to update exprtypes as I process the nodes. e.g. For a
    BinaryExpr, check if lhs is untyped or rhs is untyped, and convert to the
    other's type. Except if it's a shift, in which case I convert an untyped
    lhs to the type of the shift expression. etc. I have to duplicate some
    logic that is already in go/types. If Expr were called *after* calls to
    "convertedUntyped", then I could cut out a heap of redundant code.

    In the case of var z uint64 = x, I think the intention is that the
    implicit conversion from untyped integer to uint64 is handled by the client
    as part of the assignment, since all assignments (including implicit ones
    arising in such constructs as parameter passing, channel send, etc) must
    handle various kinds of conversions anyway. But gri is the expert here.
    Assignments are trivial, but they're only place where conversions take
    place. There's also function calls, return statements, composite literals,
    binary expressions (and shift expressions need extra attention). It's not
    that it can't be done by the client, or that it's terribly complicated,
    it's just redundant. If go/types were to report the converted type through
    Context.Expr, then clients need not deal with untyped constants at all
    (only typed constants).

    Happy new year!

    Cheers,
    Andrew



    --
    Andrew Wilkins
    http://awilkins.id.au
  • Alan Donovan at Dec 31, 2012 at 4:37 pm ⇧


    ---------- Forwarded message ----------
    From: Andrew Wilkins <axwalk@gmail.com>
    Date: Mon, Dec 31, 2012 at 5:07 PM
    Subject: Re: [golang-dev] Re: code review 7013049: go/types: moved from
    exp/types (issue 7013049)
    To: Alan Donovan <adonovan@google.com>

    On Mon, Dec 31, 2012 at 10:10 AM, Alan Donovan wrote:

    Idents are connected to the ast.Object of the underlying declaration, and
    that's where the type is located, so all references to a shared const
    declaration x necessarily have the same type.
    Sorry, I was being unclear again. Here's my final attempt at explaining
    myself for 2012.
    What I mean is, I would like Context.Expr to report the converted type for
    each *ast.Ident (likewise for *ast.BasicLit, and complex constant
    expressions) in the context in which it was used. The ast.Ident's Obj.Type
    need not be modified. e.g.:

    const x = 123
    var y = x
    var z uint64 = x

    go/parser will generate two AssignStmts (or VarDecls) for the vars y and
    z, each of which has a separate *ast.Ident for the rhs expr, with a common
    *ast.Object.
    When go/types checks the AssignStmt for "var y", it will determine that x
    should be converted to an int. For "var z", it should be converted to
    uint64. I would like for Context.Expr to be called like so:
    Context.Expr(ident1, types.Typ[types.Int], int64(123))
    Context.Expr(ident2, types.Typ[types.Uint64], int64(123))
    where ident1 and ident2 are the two *ast.Idents referring to "x".
    Currently it reports "UntypedInteger" as the second argument.
    Ok, understood. (I proposed to gri that we use this mechanism---reporting
    a more specific type for ast.Ident than Ident.Obj.Type---for the case of
    polymorphic and variadic built-in functions since a types.Signature of the
    specific callsite is more useful than the types.builtin for, say, "append".
    But I haven't found a need for it in the "untyped" case; see below.)


    The problem being that the type in exprtypes is the "untyped" type. So
    currently I'm having to update exprtypes as I process the nodes. e.g. For a
    BinaryExpr, check if lhs is untyped or rhs is untyped, and convert to the
    other's type. Except if it's a shift, in which case I convert an untyped
    lhs to the type of the shift expression. etc. I have to duplicate some
    logic that is already in go/types. If Expr were called *after* calls to
    "convertedUntyped", then I could cut out a heap of redundant code.
    I too found comparisons and shifts to be quite complex to deal with. It
    would be nice if the typechecker could pass on many of its deductions to
    its clients, but I havent got a concrete wishlist yet.


    In the case of var z uint64 = x, I think the intention is that the
    implicit conversion from untyped integer to uint64 is handled by the client
    as part of the assignment, since all assignments (including implicit ones
    arising in such constructs as parameter passing, channel send, etc) must
    handle various kinds of conversions anyway. But gri is the expert here.
    Assignments are trivial, but they're [not the?] only place where
    conversions take place. There's also function calls, return statements,
    composite literals, binary expressions (and shift expressions need extra
    attention). It's not that it can't be done by the client, or that it's
    terribly complicated, it's just redundant. If go/types were to report the
    converted type through Context.Expr, then clients need not deal with
    untyped constants at all (only typed constants).
    My point was that you already need a routine to perform assignability
    conversion, not just for assignments but for call/return, send, and many
    other places, and that routine already must handle (e.g.) conversion to
    interfaces and suchlike, so adding an extra case in there for "untyped"
    conversions really doesn't add much complexity or redundancy.


    Happy New Year,
    alan
  • Andrew Wilkins at Jan 2, 2013 at 1:44 am ⇧

    On Tue, Jan 1, 2013 at 12:37 AM, Alan Donovan wrote:

    My point was that you already need a routine to perform assignability
    conversion, not just for assignments but for call/return, send, and many
    other places, and that routine already must handle (e.g.) conversion to
    interfaces and suchlike, so adding an extra case in there for "untyped"
    conversions really doesn't add much complexity or redundancy.

    Right, understood. Sorry, rereading your original it was clear.

    It might not be too bad for assignments, I could probably live with it, but
    it would be nicer to have the assigned type just to reduce complexity
    (though I acknowledge it pushes a little bit of complexity back to
    go/types). I've made a few changes to go/types in my tree, and what I want
    to do appears to be fairly trivial. What I've done is:
    - Modified convertUntyped to call "callExpr" again. Thus, the original
    "untyped" type can be overwritten in my map.
    - Modified convertUntyped to check if the target type is interface{}, and
    convert the type to the constant's default type (bool, int, float64, etc.)
    - Added calls to convertUntyped to assignments
    I'll wait and see what gri thinks, and propose a CL if acceptable.

    On an unrelated note, I did some more porting to go/types last night and
    found unsafe.{Size,Align,Offset}of lacking. There is a TODO regarding
    allowing users to specify platform-specific values for alignment. Sizeof is
    wrong/incompatible with gc until this is done (structs/arrays aren't
    packed), so I've modified my tree to implement them based on
    IntSize/PtrSize. I can provide a CL if desired, but I'd like to request
    that users be able to specify alignment/size of *all* types; otherwise the
    size/alignment can only be correct for one implementation of the language.

    Adding a field to Context with type "interface{Alignof(Type) int64;
    Sizeof(Type) int64}" would be great; a gc compatible implementation could
    be built into go/types. This will allow tools (language implementations) to
    adapt to changes in (change) runtime type implementations, e.g. gc and llgo
    are both possibly going to change function pointers to be a pair of
    pointers to accommodate closures without dynamic code generation; llgo's
    interfaces currently have a different representation than gc.

    Cheers,
    --
    Andrew Wilkins
    http://awilkins.id.au
  • Robert Griesemer at Jan 8, 2013 at 4:53 am ⇧
    Andrew;

    Regarding your questions:

    1) I think both you and Alan have a point regarding wanting the
    actually used (converted) type of a constant, and an expression in
    general. Perhaps the context callback should provide both. I'm not too
    keen at the moment on the 2nd callback for the same expression, but
    perhaps that is the right approach (it's like an implicit conversion
    applied to the same expression). Maybe a special conversion node could
    be introduced. Without this, I agree that you'd have to rebuild some
    functionality externally, and it's very non-trivial one at that (such
    as shift typing).

    2) I need to think more about the internal/external type of a variadic
    parameter.

    3) I am aware that the package unsafe needs to be customizable w/
    respect to type sizes and alignments.

    I am reluctant to give you more concrete answers because there are
    higher-priority items (from my point of view) that need to be done
    first. For one, one of the bigger missing pieces is tracking of
    package origin of fields an methods (hence the most recent changes):
    For correct type checking, each field and method must know which
    package declared it. This will require changes to the importer, and
    possibly its signature.

    I am also somewhat concerned about exporting too much because if
    go/types becomes part of Go 1.1, the API will be frozen and everthing
    that's there cannot be changed. At the same time I think we don't have
    enough experience with it yet.

    But here is some foward-looking information for your planning:

    - Hopefully soon I'll have the field/method origin tracking implemented.

    - Then there are a few pieces missing: 1) correct handling of
    receivers (ptr vs non-pointers), 2) actual type-checking on
    conversions, and 3) fixing the bugs in shift expression typing.

    - Only after that (i.e., basic functionality completeness) I will
    start refining the API (before it makes no sense because things may
    have to change again for missing functionality).

    Hopefully that makes sense. I understand it must be a bit frustrating
    from your point of view. I know from personal experience it's way
    easier to have a compiler's data structures completely under control.
    Also, development of this package has taken much much longer than I
    would have liked to see myself (mostly due to other things that also
    needed attention).

    In the long run, I am hoping to get rid of all ast.Object dependencies
    in the API: i.e., not even promise that the type checker sets the
    ast.Object's Types fields. The reason is that the current coupling of
    AST with resolver and type-checker is really complicated and has
    invariants that are difficult to maintain. Essentially, only an AST
    generated by the parser, and resolved via ast.NewPackage can be used
    by the type-checker. All bets are off when somebody is doing
    manual/programmatic AST changes. In retrospect it would be cleaner and
    simpler for the AST to simply represent source code with no semantics
    attached. It would be the task of the type checker to resolve
    identifiers. It would be easier to understand the AST API and
    invariants, and it would be easier to manipulate it. In turn, the
    typechecker would have to provide maps linking identifiers to
    declarations or something along those lines.

    Hope that helps for now.
    - gri
    On Tue, Jan 1, 2013 at 5:44 PM, Andrew Wilkins wrote:
    On Tue, Jan 1, 2013 at 12:37 AM, Alan Donovan wrote:

    My point was that you already need a routine to perform assignability
    conversion, not just for assignments but for call/return, send, and many
    other places, and that routine already must handle (e.g.) conversion to
    interfaces and suchlike, so adding an extra case in there for "untyped"
    conversions really doesn't add much complexity or redundancy.

    Right, understood. Sorry, rereading your original it was clear.

    It might not be too bad for assignments, I could probably live with it, but
    it would be nicer to have the assigned type just to reduce complexity
    (though I acknowledge it pushes a little bit of complexity back to
    go/types). I've made a few changes to go/types in my tree, and what I want
    to do appears to be fairly trivial. What I've done is:
    - Modified convertUntyped to call "callExpr" again. Thus, the original
    "untyped" type can be overwritten in my map.
    - Modified convertUntyped to check if the target type is interface{}, and
    convert the type to the constant's default type (bool, int, float64, etc.)
    - Added calls to convertUntyped to assignments
    I'll wait and see what gri thinks, and propose a CL if acceptable.

    On an unrelated note, I did some more porting to go/types last night and
    found unsafe.{Size,Align,Offset}of lacking. There is a TODO regarding
    allowing users to specify platform-specific values for alignment. Sizeof is
    wrong/incompatible with gc until this is done (structs/arrays aren't
    packed), so I've modified my tree to implement them based on
    IntSize/PtrSize. I can provide a CL if desired, but I'd like to request that
    users be able to specify alignment/size of *all* types; otherwise the
    size/alignment can only be correct for one implementation of the language.

    Adding a field to Context with type "interface{Alignof(Type) int64;
    Sizeof(Type) int64}" would be great; a gc compatible implementation could be
    built into go/types. This will allow tools (language implementations) to
    adapt to changes in (change) runtime type implementations, e.g. gc and llgo
    are both possibly going to change function pointers to be a pair of pointers
    to accommodate closures without dynamic code generation; llgo's interfaces
    currently have a different representation than gc.

    Cheers,
    --
    Andrew Wilkins
    http://awilkins.id.au
  • Andrew Wilkins at Jan 8, 2013 at 5:15 am ⇧

    On Tue, Jan 8, 2013 at 12:53 PM, Robert Griesemer wrote:

    1) I think both you and Alan have a point regarding wanting the
    actually used (converted) type of a constant, and an expression in
    general. Perhaps the context callback should provide both. I'm not too
    keen at the moment on the 2nd callback for the same expression, but
    perhaps that is the right approach (it's like an implicit conversion
    applied to the same expression). Maybe a special conversion node could
    be introduced. Without this, I agree that you'd have to rebuild some
    functionality externally, and it's very non-trivial one at that (such
    as shift typing).

    2) I need to think more about the internal/external type of a variadic
    parameter.
    The latest change you made makes it possible to do what I want.
    The difference in external/internal type is a little weird, though.

    3) I am aware that the package unsafe needs to be customizable w/
    respect to type sizes and alignments.

    I am reluctant to give you more concrete answers because there are
    higher-priority items (from my point of view) that need to be done
    first. For one, one of the bigger missing pieces is tracking of
    package origin of fields an methods (hence the most recent changes):
    For correct type checking, each field and method must know which
    package declared it. This will require changes to the importer, and
    possibly its signature.
    No worries, I will refrain from sending in any major CLs for now.

    I am also somewhat concerned about exporting too much because if
    go/types becomes part of Go 1.1, the API will be frozen and everthing
    that's there cannot be changed. At the same time I think we don't have
    enough experience with it yet.

    But here is some foward-looking information for your planning:

    - Hopefully soon I'll have the field/method origin tracking implemented.
    Great. If it's exposed that'd be helpful to me, though I can live without
    it.
    I require the origin information for named types and functions to generate
    runtime type info and symbol names and I currently need to trawl
    ast.Scopes to map objects back to their package.

    - Then there are a few pieces missing: 1) correct handling of
    receivers (ptr vs non-pointers), 2) actual type-checking on
    conversions, and 3) fixing the bugs in shift expression typing.
    Are you okay with me sending in some bug fix CLs, or is it too distracting
    at the moment? I've found a couple of places where type hints aren't set
    correctly, and other little things.

    - Only after that (i.e., basic functionality completeness) I will
    start refining the API (before it makes no sense because things may
    have to change again for missing functionality).

    Hopefully that makes sense. I understand it must be a bit frustrating
    from your point of view. I know from personal experience it's way
    easier to have a compiler's data structures completely under control.
    Also, development of this package has taken much much longer than I
    would have liked to see myself (mostly due to other things that also
    needed attention).
    Not a problem. FYI, with just a few changes to go/types in my tree,
    I've got my compiler passing all of its tests (though admittedly, they're
    not
    comprehensive).

    In the long run, I am hoping to get rid of all ast.Object dependencies
    in the API: i.e., not even promise that the type checker sets the
    ast.Object's Types fields. The reason is that the current coupling of
    AST with resolver and type-checker is really complicated and has
    invariants that are difficult to maintain. Essentially, only an AST
    generated by the parser, and resolved via ast.NewPackage can be used
    by the type-checker. All bets are off when somebody is doing
    manual/programmatic AST changes. In retrospect it would be cleaner and
    simpler for the AST to simply represent source code with no semantics
    attached. It would be the task of the type checker to resolve
    identifiers. It would be easier to understand the AST API and
    invariants, and it would be easier to manipulate it. In turn, the
    typechecker would have to provide maps linking identifiers to
    declarations or something along those lines.
    I can't quite picture how it'll work, so I'll just wait and see :)
    I guess I could update the objects myself using Context.Expr?

    Thanks very much for the detailed reply.

    Andrew
  • Robert Griesemer at Jan 8, 2013 at 5:21 am ⇧
    I'm always happy for concrete and to-the-point bug patches, preferably
    one at a time. Thanks.
    - gri
    On Mon, Jan 7, 2013 at 9:15 PM, Andrew Wilkins wrote:
    On Tue, Jan 8, 2013 at 12:53 PM, Robert Griesemer wrote:

    1) I think both you and Alan have a point regarding wanting the
    actually used (converted) type of a constant, and an expression in
    general. Perhaps the context callback should provide both. I'm not too
    keen at the moment on the 2nd callback for the same expression, but
    perhaps that is the right approach (it's like an implicit conversion
    applied to the same expression). Maybe a special conversion node could
    be introduced. Without this, I agree that you'd have to rebuild some
    functionality externally, and it's very non-trivial one at that (such
    as shift typing).

    2) I need to think more about the internal/external type of a variadic
    parameter.

    The latest change you made makes it possible to do what I want.
    The difference in external/internal type is a little weird, though.
    3) I am aware that the package unsafe needs to be customizable w/
    respect to type sizes and alignments.

    I am reluctant to give you more concrete answers because there are
    higher-priority items (from my point of view) that need to be done
    first. For one, one of the bigger missing pieces is tracking of
    package origin of fields an methods (hence the most recent changes):
    For correct type checking, each field and method must know which
    package declared it. This will require changes to the importer, and
    possibly its signature.

    No worries, I will refrain from sending in any major CLs for now.
    I am also somewhat concerned about exporting too much because if
    go/types becomes part of Go 1.1, the API will be frozen and everthing
    that's there cannot be changed. At the same time I think we don't have
    enough experience with it yet.

    But here is some foward-looking information for your planning:

    - Hopefully soon I'll have the field/method origin tracking implemented.

    Great. If it's exposed that'd be helpful to me, though I can live without
    it.
    I require the origin information for named types and functions to generate
    runtime type info and symbol names and I currently need to trawl
    ast.Scopes to map objects back to their package.
    - Then there are a few pieces missing: 1) correct handling of
    receivers (ptr vs non-pointers), 2) actual type-checking on
    conversions, and 3) fixing the bugs in shift expression typing.

    Are you okay with me sending in some bug fix CLs, or is it too distracting
    at the moment? I've found a couple of places where type hints aren't set
    correctly, and other little things.
    - Only after that (i.e., basic functionality completeness) I will
    start refining the API (before it makes no sense because things may
    have to change again for missing functionality).

    Hopefully that makes sense. I understand it must be a bit frustrating
    from your point of view. I know from personal experience it's way
    easier to have a compiler's data structures completely under control.
    Also, development of this package has taken much much longer than I
    would have liked to see myself (mostly due to other things that also
    needed attention).

    Not a problem. FYI, with just a few changes to go/types in my tree,
    I've got my compiler passing all of its tests (though admittedly, they're
    not
    comprehensive).
    In the long run, I am hoping to get rid of all ast.Object dependencies
    in the API: i.e., not even promise that the type checker sets the
    ast.Object's Types fields. The reason is that the current coupling of
    AST with resolver and type-checker is really complicated and has
    invariants that are difficult to maintain. Essentially, only an AST
    generated by the parser, and resolved via ast.NewPackage can be used
    by the type-checker. All bets are off when somebody is doing
    manual/programmatic AST changes. In retrospect it would be cleaner and
    simpler for the AST to simply represent source code with no semantics
    attached. It would be the task of the type checker to resolve
    identifiers. It would be easier to understand the AST API and
    invariants, and it would be easier to manipulate it. In turn, the
    typechecker would have to provide maps linking identifiers to
    declarations or something along those lines.

    I can't quite picture how it'll work, so I'll just wait and see :)
    I guess I could update the objects myself using Context.Expr?

    Thanks very much for the detailed reply.

    Andrew

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedDec 28, '12 at 7:05p
activeJan 8, '13 at 5:21a
posts14
users3
websitegolang.org

3 users in discussion

Andrew Wilkins: 5 posts Alan Donovan: 5 posts Robert Griesemer: 4 posts

Content

People

Support

Translate

site design / logo © 2022 Grokbase