c argument names in bindings

compiler/checker/checker.odin

@@ -758,7 +758,7 @@ validate_declarations :: proc(checker: ^Checker) {
 					"void is only valid as a function result type",
 				)
 			}
-			if contains_name(locals[:], param.name) {
+			if param.name != checker.sink_symbol && contains_name(locals[:], param.name) {
 				source.addf(
 					checker.diagnostics,
 					param.span,

compiler/cimport/cimport.odin

@@ -68,12 +68,13 @@ Alias :: struct {
 }
 
 Function :: struct {
-	name:      string,
+	name:        string,
-	params:    []Type_Id,
+	params:      []Type_Id,
-	result:    Type_Id,
+	param_names: []string,
-	variadic:  bool,
+	result:      Type_Id,
-	link_name: string,
+	variadic:    bool,
-	reason:    string,
+	link_name:   string,
+	reason:      string,
 }
 
 // Trampoline is a generated C wrapper that gives an external symbol to a
@@ -172,6 +173,10 @@ destroy_result :: proc(result: ^Result) {
 	for function in result.functions {
 		delete(function.name, result.allocator)
 		delete(function.params, result.allocator)
+		for param_name in function.param_names {
+			delete(param_name, result.allocator)
+		}
+		delete(function.param_names, result.allocator)
 		delete(function.link_name, result.allocator)
 		delete(function.reason, result.allocator)
 	}

compiler/cimport/libclang.odin

@@ -91,6 +91,8 @@ Api :: struct {
 	get_result_type:               proc "c"(CXType) -> CXType,
 	get_num_arg_types:             proc "c"(CXType) -> i32,
 	get_arg_type:                  proc "c"(CXType, u32) -> CXType,
+	cursor_get_num_arguments:      proc "c"(CXCursor) -> i32,
+	cursor_get_argument:           proc "c"(CXCursor, u32) -> CXCursor,
 	is_function_type_variadic:     proc "c"(CXType) -> u32,
 	is_const_qualified_type:       proc "c"(CXType) -> u32,
 	is_volatile_qualified_type:    proc "c"(CXType) -> u32,
@@ -215,6 +217,8 @@ load_api_from :: proc(path: string) -> (Api, bool) {
 		load_proc(&api, "clang_getResultType", &api.get_result_type) &&
 		load_proc(&api, "clang_getNumArgTypes", &api.get_num_arg_types) &&
 		load_proc(&api, "clang_getArgType", &api.get_arg_type) &&
+		load_proc(&api, "clang_Cursor_getNumArguments", &api.cursor_get_num_arguments) &&
+		load_proc(&api, "clang_Cursor_getArgument", &api.cursor_get_argument) &&
 		load_proc(&api, "clang_isFunctionTypeVariadic", &api.is_function_type_variadic) &&
 		load_proc(&api, "clang_isConstQualifiedType", &api.is_const_qualified_type) &&
 		load_proc(&api, "clang_isVolatileQualifiedType", &api.is_volatile_qualified_type) &&
@@ -1371,6 +1375,9 @@ visit_cursor :: proc "c"(cursor, parent: CXCursor, client_data: rawptr) -> i32 {
 		}
 		params: [dynamic]Type_Id
 		params.allocator = ctx.allocator
+		param_names: [dynamic]string
+		param_names.allocator = ctx.allocator
+		named_count := ctx.api.cursor_get_num_arguments(cursor)
 		count := ctx.api.get_num_arg_types(function_type)
 		if count < 0 {
 			reason = "function declaration has no prototype"
@@ -1381,6 +1388,13 @@ visit_cursor :: proc "c"(cursor, parent: CXCursor, client_data: rawptr) -> i32 {
 				if param == INVALID_TYPE && len(reason) == 0 {
 					reason = "function parameter type is not supported"
 				}
+				// Parameter names come from the function cursor's argument cursors,
+				// not the function type. Absent (e.g. `int f(int, char*)`) -> "".
+				if i32(index) < named_count {
+					append(&param_names, clone_cx_string(ctx.api, ctx.api.get_cursor_spelling(ctx.api.cursor_get_argument(cursor, u32(index))), ctx.allocator))
+				} else {
+					append(&param_names, fmt.aprintf("", allocator=ctx.allocator))
+				}
 			}
 		}
 		linkage := ctx.api.get_cursor_linkage(cursor)
@@ -1406,6 +1420,7 @@ visit_cursor :: proc "c"(cursor, parent: CXCursor, client_data: rawptr) -> i32 {
 		append(&ctx.result.functions, Function{
 			name=fmt.aprintf("%s", name, allocator=ctx.allocator),
 			params=params[:],
+			param_names=param_names[:],
 			result=result_type,
 			variadic=variadic,
 			link_name=fmt.aprintf("%s", link_name, allocator=ctx.allocator),

compiler/parser/parser.odin

@@ -1110,7 +1110,7 @@ parse_params :: proc(parser: ^Parser) -> ([]ast.Param, bool) {
 		names: [dynamic]token.Token
 		names.allocator = parser.module.allocator
 		for {
-			if current(parser).kind != .Identifier {
+			if current(parser).kind != .Identifier && current(parser).kind != .Underscore {
 				source.add(parser.diagnostics, current(parser).span, "expected parameter name")
 				break
 			}

compiler/translatec/translatec.odin

@@ -12,6 +12,7 @@ package translatec
 // the output is an honest record of the whole header.
 
 import "../cimport"
+import "../lexer"
 import "core:fmt"
 import "core:mem"
 import "core:strings"
@@ -102,7 +103,8 @@ render_type :: proc(b: ^strings.Builder, result: ^cimport.Result, id: cimport.Ty
 		fmt.sbprintf(b, "[%d]", item.count)
 		render_type(b, result, item.child, record_names)
 	case .Function:
-		render_c_func(b, result, item.params, item.child, item.variadic, record_names)
+		// Function-pointer types carry no parameter names, so every slot renders `_`.
+		render_c_func(b, result, item.params, nil, item.child, item.variadic, record_names)
 	case .Record:
 		if int(item.record) >= 0 && int(item.record) < len(record_names) {
 			strings.write_string(b, record_names[item.record])
@@ -112,12 +114,14 @@ render_type :: proc(b: ^strings.Builder, result: ^cimport.Result, id: cimport.Ty
 	}
 }
 
-// render_c_func writes `c_func(arg0 T0, ...) R`. Params are named arg0.. because
+// render_c_func writes `c_func(name T0, ...) R`, using the real C parameter name
-// the parser requires parameter names; names do not affect type identity.
+// when `names` provides one and `_` (the sink) otherwise. Names do not affect type
+// identity; the parser requires a name slot, so unnamed params use `_`.
 render_c_func :: proc(
 	b: ^strings.Builder,
 	result: ^cimport.Result,
 	params: []cimport.Type_Id,
+	names: []string,
 	ret: cimport.Type_Id,
 	variadic: bool,
 	record_names: []string,
@@ -127,7 +131,8 @@ render_c_func :: proc(
 		if index > 0 {
 			strings.write_string(b, ", ")
 		}
-		fmt.sbprintf(b, "arg%d ", index)
+		name := names[index] if index < len(names) else ""
+		fmt.sbprintf(b, "%s ", safe_param_name(name))
 		render_type(b, result, param, record_names)
 	}
 	if variadic {
@@ -267,7 +272,7 @@ emit_functions :: proc(b: ^strings.Builder, result: ^cimport.Result, record_name
 			continue
 		}
 		fmt.sbprintf(b, "%s :: ", function.name)
-		render_c_func(b, result, function.params, function.result, function.variadic, record_names)
+		render_c_func(b, result, function.params, function.param_names, function.result, function.variadic, record_names)
 		strings.write_byte(b, '\n')
 		wrote = true
 	}
@@ -311,6 +316,16 @@ render_macro_value :: proc(b: ^strings.Builder, value: cimport.Macro_Value) {
 	}
 }
 
+// safe_param_name returns the C parameter name when it is a usable brolang
+// identifier, else `_` (the sink): empty names (unnamed C params) and names that
+// collide with a brolang keyword (e.g. legal C `int f(int and)`) both become `_`.
+safe_param_name :: proc(name: string) -> string {
+	if len(name) > 0 && lexer.keyword_kind(name) == .Identifier {
+		return name
+	}
+	return "_"
+}
+
 macro_scalar_kind :: proc(result: ^cimport.Result, id: cimport.Type_Id) -> bool {
 	if int(id) < 0 || int(id) >= len(result.types) {
 		return false

compiler_tests.odin

@@ -5814,7 +5814,8 @@ translate_c_emits_native_bindings_and_round_trips :: proc(t: ^testing.T) {
 	append(&result.aliases, cimport.Alias{name = "Mapper", type = cimport.Type_Id(4)})
 
 	add_params := []cimport.Type_Id{cimport.Type_Id(0), cimport.Type_Id(0)}
-	append(&result.functions, cimport.Function{name = "imported_add", params = add_params, result = cimport.Type_Id(0)})
+	add_param_names := []string{"a", "b"}
+	append(&result.functions, cimport.Function{name = "imported_add", params = add_params, param_names = add_param_names, result = cimport.Type_Id(0)})
 
 	append(&result.macros, cimport.Macro_Constant{
 		name = "MAX_LEN",
@@ -5844,8 +5845,10 @@ translate_c_emits_native_bindings_and_round_trips :: proc(t: ^testing.T) {
 	testing.expect(t, strings.contains(output, "\tleft c_int"))
 	testing.expect(t, strings.contains(output, "\tright c_int"))
 	testing.expect(t, strings.contains(output, "Size :: alias c_ulong"))
-	testing.expect(t, strings.contains(output, "Mapper :: alias ?*c_func(arg0 c_int) c_int"))
+	// Function-pointer types carry no parameter names, so the callback renders `_`.
-	testing.expect(t, strings.contains(output, "imported_add :: c_func(arg0 c_int, arg1 c_int) c_int"))
+	testing.expect(t, strings.contains(output, "Mapper :: alias ?*c_func(_ c_int) c_int"))
+	// Real C parameter names are used when present.
+	testing.expect(t, strings.contains(output, "imported_add :: c_func(a c_int, b c_int) c_int"))
 	testing.expect(t, strings.contains(output, "MAX_LEN c_int :: 256"))
 	testing.expect(t, strings.contains(output, "# unsupported in bindings: C union 'Choice'"))
 	testing.expect(t, strings.contains(output, "# unsupported in bindings: external variable 'some_global'"))
@@ -5864,3 +5867,31 @@ translate_c_emits_native_bindings_and_round_trips :: proc(t: ^testing.T) {
 	defer ast.destroy_module(&module)
 	testing.expect_value(t, len(diagnostics.items), 0)
 }
+
+@(test)
+sink_named_parameters_are_allowed_and_not_duplicates :: proc(t: ^testing.T) {
+	// Generated bindings use `_` for unnamed C params; the parser must accept it and
+	// the checker must not flag repeated `_` as duplicate parameters.
+	text := `foo :: c_func(_ c_int, _ c_int) c_int
+main :: func() void {
+	_ = foo(1, 2)
+}
+`
+	source_file := source.Source{path = "test.bro", text = text}
+	diagnostics := source.init_diagnostics(&source_file)
+	defer source.destroy_diagnostics(&diagnostics)
+	symbols := symbol.init_table()
+	defer symbol.destroy_table(&symbols)
+	stream := lexer.lex(&source_file, &diagnostics, &symbols)
+	defer delete(stream.items)
+	ast_module := parser.parse(&stream, &source_file, &diagnostics)
+	defer ast.destroy_module(&ast_module)
+	hir_module := checker.check(&ast_module, &diagnostics, &symbols)
+	defer hir.destroy_module(&hir_module)
+
+	for diagnostic in diagnostics.items {
+		testing.expect(t, !strings.contains(diagnostic.message, "duplicate parameter"))
+		testing.expect(t, !strings.contains(diagnostic.message, "expected parameter name"))
+	}
+	testing.expect_value(t, len(diagnostics.items), 0)
+}

testbed/cstdio.bro

@@ -1,4 +1,4 @@
 # generated by brolang translate-c from testbed/cstdio.h
 
-printf :: c_func(arg0 ?*c_char, ...) c_int
+printf :: c_func(format ?*c_char, ...) c_int
 

testbed/cstdio.h

@@ -1 +0,0 @@
-int printf(const char *format, ...);

testbed/main.bro

@@ -76,7 +76,7 @@ main :: func() i32 {
 		_ = printf("%d: %s projected=%d\n", index, player.name, score)
 	}
 
-	if best_player(players[..]) |winner : projected_score(winner^) >= 80| {
+	if best_player(&players) |winner : projected_score(winner^) >= 80| {
 		_ = printf("winner: %s\n", winner.name)
 		return 0
 	}