Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/types, types2: enable real, imag and complex with type parameter arguments #50937

Open
griesemer opened this issue Jan 31, 2022 · 13 comments
Open
Assignees
Labels
NeedsInvestigation
Milestone

Comments

@griesemer
Copy link
Contributor

@griesemer griesemer commented Jan 31, 2022

For 1.18 we disabled (report an error) when applying one of these built-in functions to arguments of type parameter type because it involves the creation of a synthetic type parameter which may break go/types API invariants if those types leak.

Need to investigate what the correct approach is (e.g., should we track the type parameters in the types.Info.Implicits map?) and re-enable the code. The compiler may also need minor adjustments.

cc: @findleyr
cc: @ianlancetaylor

@griesemer griesemer added the NeedsInvestigation label Jan 31, 2022
@griesemer griesemer added this to the Go1.19 milestone Jan 31, 2022
@griesemer griesemer self-assigned this Jan 31, 2022
@gopherbot
Copy link

@gopherbot gopherbot commented Jan 31, 2022

Change https://golang.org/cl/382116 mentions this issue: go/types, types2: disallow real, imag, complex on type parameters

gopherbot pushed a commit that referenced this issue Feb 1, 2022
We can type-check these fine but the API implications are unclear.

Fixes #50912.
For #50937.

Change-Id: If29bbb4a257ff6a85e3bfcd4755fd8f90c80fb87
Reviewed-on: https://go-review.googlesource.com/c/go/+/382116
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@fgm
Copy link

@fgm fgm commented Feb 1, 2022

Shouldn't that limitation be mentioned in the docs ? 4fea593 only contains code changes.

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Feb 1, 2022

@fgm, yes, doc changes are forthcoming.

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 1, 2022

Change https://golang.org/cl/381967 mentions this issue: doc/go1.18: document restrictions for real, imag, complex

gopherbot pushed a commit that referenced this issue Feb 1, 2022
For #47694.
For #50912.
For #50937.

Change-Id: I3fae6c8dbbd61a45e669b8fb0c18ac76f2183963
Reviewed-on: https://go-review.googlesource.com/c/go/+/381967
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Feb 1, 2022

Creating pseudo TypeParams to represent these derived types seems okay, but feels a bit inconsistent to me with how structural typing works otherwise in Go and the go/types API. If there's no declared name in the source file (i.e., no TypeName), then I think representing it as a TypeParam seems not ideal.

Could we instead create new Types to directly represent "type operations"? Like suppose we had types.FloatOf and types.ComplexOf, so that given value z of complex type C, FloatOf(C) would represent the type of real(z).

I expect we'll need a types.ElemOf operation too, if we allow things like dereferencing pointers of whose type is constrained to *P|*Q.

@findleyr
Copy link
Contributor

@findleyr findleyr commented Feb 1, 2022

inconsistent to me with how structural typing works otherwise in Go and the go/types API

Just to make sure I'm not missing something: currently where a structural type is used we expose the structural type itself in the API, right? Are you aware of any other conventions?

Edit: @mdempsky

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Feb 1, 2022

Instead of providing type operations, perhaps we can redefine the built-ins in a backward-compatible way, for instance:

func real[Argument ~complex64|~complex128, Result ~float32|~float64](z Argument) Result

and record the built-in signature as such. Then all type parameters are declared.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Feb 1, 2022

@findleyr Sorry, when I wrote "structural typing" I meant as opposed to nominal typing (i.e., https://en.wikipedia.org/wiki/Structural_type_system). I didn't mean the Go-specific use case.

@griesemer But I think we need a way to establish that Argument and Result are related types? E.g., for

func F[C1, C2 constraints.Complex](z1a, z1b C1, z2 C2) {
  _ = real(z1a) + real(z1b) // ok
  _ = real(z1a) + real(z2) // ERROR
}

we need to know that for any C1 the real(z1a) and real(z1b) calls will always have the same corresponding derived type and thus are addable, but that real(z1a) and real(z2) may not be.

@griesemer
Copy link
Contributor Author

@griesemer griesemer commented Feb 1, 2022

We could make the Result constraint a magic type constraint function that depends on the Argument. Maybe if real applied to a type, it produces a type result:

func real[Argument ~complex64|~complex128, Result real(Argument)](z Argument) Result

Such a type function would only be permitted in constraints.
But it opens the door to all kinds of similar constructs which we may not want to entertain.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Feb 1, 2022

We could make the Result constraint a magic type constraint function that depends on the Argument.

This sounds similar to the RealOf type operator I described above, but with the extra complexity of representing real as a generic function instead of a builtin function. I'm not opposed to that, but do the extra indirections buy us anything? Or is there a distinction I'm missing?

I also don't think that solution generalizes to allowing dereferencing *P|*Q-constrained pointers, because there's no function call that we can add the magic type parameter constraints to.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 1, 2022

I think that any discussion of how to handle these cases should also consider how to handle cases like

func First[T ~string | ~[]byte](s T) int {
    for _, c := range s {
        return int(c)
    }
}

Here c is either byte or rune. The code is fine if we can handle that case.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Feb 1, 2022

@ianlancetaylor My first proposal would be to have types.RangeKeyOf and types.RangeValueOf type operators, which represent the corresponding types assigned to the RangeStmt.{Key,Value} expressions, respectively.

But I think there's also value in trying to consider creating a common Type instance to represent all of these operations, rather than creating one-off Types for each operation.

@danscales
Copy link
Contributor

@danscales danscales commented Feb 1, 2022

I like @mdempsky's idea of type operations/functions types.FloatOf and types.ComplexOf I was thinking of it more in terms of rules, since there are only a limited number of them, but functions on types makes sense. Basically, the type operation is what the stenciler/instantiater can use to generate the final concrete or shape type for these particular cases when substituting in the type args of a generic function/method.

Since the shape type implementation may vary between compilers, it may be that thinking of these as an enumerated set of rules is a little better, since the compiler will have to implement the rule. I.e. if it sees a type type.RealOf(T), and the value of the T arg during instantiation is a shape like complex128, then it knows that the result type should be a shape like float64 (even though it hasn't even generated a shape like float64 before).

I definitely don't see any particular use in representing these dependent types as type parameters - but at the very least, they need to have a rule/function associated with them. (Or each compiler will just have to implement these rules independently based on the builtins/constructs being used at that point in the program.)

@griesemer griesemer removed this from the Go1.19 milestone Apr 5, 2022
@griesemer griesemer added this to the Go1.20 milestone Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation
Projects
None yet
Development

No branches or pull requests

7 participants