fix: address review regressions in interface upcasts and vfmt attrs

Author: vincenzopalazzo

vlib/v/ast/attr.v

@@ -28,14 +28,15 @@ pub:
 	// original call-style metadata for `@[foo(...)]`, used by vfmt
 	call_name     string
 	call_arg_name string
+	call_arg_idx  int = -1
 pub mut:
 	ct_expr   Expr // .kind == comptime_define, for [if !name]
 	ct_evaled bool // whether ct_skip has been evaluated already
 	ct_skip   bool // is the comptime expr *false*, filled by checker
 }
 
 pub fn (a &Attr) debug() string {
-	return 'Attr{ name: "${a.name}", has_arg: ${a.has_arg}, arg: "${a.arg}", kind: ${a.kind}, ct_expr: ${a.ct_expr}, ct_opt: ${a.ct_opt}, ct_skip: ${a.ct_skip}, call_name: "${a.call_name}", call_arg_name: "${a.call_arg_name}" }'
+	return 'Attr{ name: "${a.name}", has_arg: ${a.has_arg}, arg: "${a.arg}", kind: ${a.kind}, ct_expr: ${a.ct_expr}, ct_opt: ${a.ct_opt}, ct_skip: ${a.ct_skip}, call_name: "${a.call_name}", call_arg_name: "${a.call_arg_name}", call_arg_idx: ${a.call_arg_idx} }'
 }
 
 // str returns the string representation without square brackets

vlib/v/checker/checker.v

@@ -6201,7 +6201,17 @@ fn (mut c Checker) fail_if_unreadable(expr ast.Expr, typ ast.Type, what string)
 }
 
 fn (mut c Checker) interface_embeds_interface(interface_type ast.Type, embedded_interface_type ast.Type) bool {
+	mut visited := map[int]bool{}
+	return c.interface_embeds_interface_recursive(interface_type, embedded_interface_type, mut
+		visited)
+}
+
+fn (mut c Checker) interface_embeds_interface_recursive(interface_type ast.Type, embedded_interface_type ast.Type, mut visited map[int]bool) bool {
 	u_interface_type := c.unwrap_generic(interface_type)
+	if visited[u_interface_type.idx()] {
+		return false
+	}
+	visited[u_interface_type.idx()] = true
 	embedded_idx := c.unwrap_generic(embedded_interface_type).idx()
 	final_embedded_idx := c.table.final_sym(embedded_interface_type).idx
 	if iface_decl := c.table.interfaces[u_interface_type] {
@@ -6211,6 +6221,11 @@ fn (mut c Checker) interface_embeds_interface(interface_type ast.Type, embedded_
 				|| c.table.final_sym(embed_typ).idx == final_embedded_idx {
 				return true
 			}
+			if c.interface_embeds_interface_recursive(embed_typ, embedded_interface_type, mut
+				visited)
+			{
+				return true
+			}
 		}
 	}
 	return false

vlib/v/fmt/attrs.v

@@ -141,8 +141,10 @@ fn attr_call_group_str(attrs []ast.Attr) string {
 	if attrs.len == 0 {
 		return ''
 	}
+	mut ordered_attrs := attrs.clone()
+	ordered_attrs.sort(a.call_arg_idx < b.call_arg_idx)
 	mut args := []string{}
-	for attr in attrs {
+	for attr in ordered_attrs {
 		if !attr.has_arg {
 			continue
 		}

vlib/v/fmt/tests/attribute_call_syntax_keep.vv

@@ -7,6 +7,12 @@ fn old_positional() {}
 @[deprecated(msg: 'use new_fn instead', after: '2999-10-10')]
 fn old_named() {}
 
+@[foo(bar: 'x', 123)]
+fn mixed_named_positional() {}
+
+@[foo(123, bar: 'x')]
+fn mixed_positional_named() {}
+
 struct Config {
 	flag  bool   @[custom(flag: true, count: 2)]
 	value string @[xml(name: 'cfg'); raw]

vlib/v/parser/attribute.v

@@ -43,10 +43,12 @@ fn (mut p Parser) parse_attr_call(name string, is_at bool, apos token.Pos) []ast
 	mut base_arg := ''
 	mut base_quote := u8(`'`)
 	mut base_arg_name := ''
+	mut base_call_arg_idx := -1
 	mut base_has_arg := false
 	mut attrs := []ast.Attr{}
 	mut has_base_arg := false
 	mut positional_arg_idx := 1
+	mut call_arg_idx := 0
 	for p.tok.kind !in [.rpar, .eof] {
 		mut is_named := false
 		mut arg_name := ''
@@ -68,6 +70,7 @@ fn (mut p Parser) parse_attr_call(name string, is_at bool, apos token.Pos) []ast
 				base_kind = kind
 				base_quote = quote
 				base_arg_name = arg_name
+				base_call_arg_idx = call_arg_idx
 				has_base_arg = true
 			} else {
 				attrs << ast.Attr{
@@ -80,27 +83,32 @@ fn (mut p Parser) parse_attr_call(name string, is_at bool, apos token.Pos) []ast
 					has_at:        is_at
 					call_name:     name
 					call_arg_name: arg_name
+					call_arg_idx:  call_arg_idx
 				}
 			}
 		} else if !has_base_arg {
 			base_has_arg = true
 			base_arg = arg
 			base_kind = kind
 			base_quote = quote
+			base_arg_name = arg_name
+			base_call_arg_idx = call_arg_idx
 			has_base_arg = true
 		} else {
 			attrs << ast.Attr{
-				name:      '${name}_${positional_arg_idx}'
-				has_arg:   true
-				arg:       arg
-				kind:      kind
-				quote:     quote
-				pos:       apos.extend(p.prev_tok.pos())
-				has_at:    is_at
-				call_name: name
+				name:         '${name}_${positional_arg_idx}'
+				has_arg:      true
+				arg:          arg
+				kind:         kind
+				quote:        quote
+				pos:          apos.extend(p.prev_tok.pos())
+				has_at:       is_at
+				call_name:    name
+				call_arg_idx: call_arg_idx
 			}
 			positional_arg_idx++
 		}
+		call_arg_idx++
 		if p.tok.kind == .comma {
 			p.next()
 			continue
@@ -118,6 +126,7 @@ fn (mut p Parser) parse_attr_call(name string, is_at bool, apos token.Pos) []ast
 		has_at:        is_at
 		call_name:     name
 		call_arg_name: base_arg_name
+		call_arg_idx:  base_call_arg_idx
 	}
 	attrs.insert(0, base_attr)
 	return attrs

vlib/v/tests/interfaces/interface_embedding_implicit_upcast_test.v

@@ -5,6 +5,10 @@ interface Solid {
 	Base
 }
 
+interface Dense {
+	Solid
+}
+
 struct Empty {
 }
 
@@ -20,4 +24,10 @@ fn test_interface_embedding_implicit_upcast() {
 	base = solid
 	assert base is Empty
 	assert (solid as Base) is Empty
+
+	dense := Dense(Empty{})
+	assert greet(dense)
+	base = dense
+	assert base is Empty
+	assert (dense as Base) is Empty
 }