Conversation
5d782d1 to
0995d45
Compare
Signed-off-by: David Gageot <david.gageot@docker.com>
0995d45 to
893e65b
Compare
There was a problem hiding this comment.
PR Review Summary
The Go 1.26 upgrade looks good overall, but there are invalid uses of the new() builtin. While Go 1.26 enhanced new() to accept non-constant type expressions, it still requires type arguments, not values or function calls.
| if cfg.Models[name].ParallelToolCalls == nil { | ||
| m := cfg.Models[name] | ||
| m.ParallelToolCalls = boolPtr(true) | ||
| m.ParallelToolCalls = new(true) |
There was a problem hiding this comment.
Invalid syntax: new() called with a value instead of a type
The expression new(true) is invalid Go syntax. The new() builtin function requires a type as its argument, but true is a boolean literal/value, not a type. This code will fail to compile with an error like "cannot use true (untyped bool constant) as type in argument to new".
The field ParallelToolCalls is of type *bool, so the correct approach would be:
t := true
m.ParallelToolCalls = &tOr use a helper function that returns *bool. Even though Go 1.26 enhanced the new() builtin to accept non-constant expressions, those expressions must still be type expressions, not value literals.
| ToolCall: acp.RequestPermissionToolCall{ | ||
| ToolCallId: "max_iterations", | ||
| Title: acp.Ptr(fmt.Sprintf("Maximum iterations (%d) reached", e.MaxIterations)), | ||
| Title: new(fmt.Sprintf("Maximum iterations (%d) reached", e.MaxIterations)), |
There was a problem hiding this comment.
Invalid syntax: new() called with fmt.Sprintf expression instead of a type
The expression new(fmt.Sprintf("Maximum iterations (%d) reached", e.MaxIterations)) is invalid Go syntax. The new() builtin accepts only a type expression (e.g., new(string)), not a value-producing function call. fmt.Sprintf() returns a string value, not a type. This will fail to compile.
The correct approach would be:
title := fmt.Sprintf("Maximum iterations (%d) reached", e.MaxIterations)
Title: &title,Or use the acp.Ptr() helper function if available (as seen elsewhere in the codebase):
Title: acp.Ptr(fmt.Sprintf("Maximum iterations (%d) reached", e.MaxIterations)),Looking at line 753 in the same file, you correctly use &title for string pointers.
No description provided.