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

Add simple conversions #3

Merged
merged 2 commits into from
Mar 22, 2022
Merged

Add simple conversions #3

merged 2 commits into from
Mar 22, 2022

Conversation

rboyer
Copy link
Member

@rboyer rboyer commented Mar 21, 2022

This should handle type Label string automatic conversions to/from each other.

@rboyer rboyer requested review from erichaberkorn, dhiaayachi and a team March 21, 2022 21:34
@rboyer rboyer self-assigned this Mar 21, 2022
Comment on lines +285 to +288
right = &ast.CallExpr{
Fun: leftType,
Args: []ast.Expr{right},
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This basically fudges the right in this:

left = right

to be

left = leftType(right)

@@ -54,7 +55,8 @@ type StringSlice []string
type WorkloadSlice []*Workload

type Workload struct {
ID string
ID string
Value int
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I was here, I also edited the e2e to have an example of int-to-int32 conversion which does require a user provided function since it could be lossy.

@rboyer rboyer changed the base branch from main to fix-tests March 21, 2022 21:37
// Only allow this for basic and named.

switch typeDecode.(type) {
case *types.Basic, *types.Named:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basic: stuff like string or int
Named: stuff like Label in type Label string

return false
}

if types.ConvertibleTo(typ, typeDecode) && types.ConvertibleTo(typeDecode, typ) {
Copy link
Member Author

@rboyer rboyer Mar 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typeDecode is the most underlying type for typ.

  • For pointers this is the type of the ultimate memory location
  • For named types, it is the type of the real underlying type

This check only lets this special code execute if BOTH the left and right are convertible to their underlying type (which in effect selects type LEFT RIGHT types).

return false
}

if types.Identical(typ, typeDecode) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for example typ=string will have typeDecode=string too and string is convertible to string but we don't want that to trigger the special code.

Base automatically changed from fix-tests to main March 21, 2022 22:01
Copy link

@kyhavlov kyhavlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks good to me - I'm not very familiar with this code yet though.

Copy link
Collaborator

@dhiaayachi dhiaayachi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Thank you @rboyer!! this will remove a bunch of annotations in Consul.

@rboyer rboyer merged commit e8f2d83 into main Mar 22, 2022
@rboyer rboyer deleted the add-simple-conversions branch March 22, 2022 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants