diff --git a/compiler/checker/checker.odin b/compiler/checker/checker.odin index 68646a3..2e4b80b 100644 --- a/compiler/checker/checker.odin +++ b/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, diff --git a/compiler/cimport/cimport.odin b/compiler/cimport/cimport.odin index 8a4a1dc..0478f3e 100644 --- a/compiler/cimport/cimport.odin +++ b/compiler/cimport/cimport.odin @@ -68,12 +68,13 @@ Alias :: struct { } Function :: struct { - name: string, - params: []Type_Id, - result: Type_Id, - variadic: bool, - link_name: string, - reason: string, + name: string, + params: []Type_Id, + param_names: []string, + result: Type_Id, + variadic: bool, + 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) } diff --git a/compiler/cimport/libclang.odin b/compiler/cimport/libclang.odin index cf7dcdc..46044d2 100644 --- a/compiler/cimport/libclang.odin +++ b/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(¶m_names, clone_cx_string(ctx.api, ctx.api.get_cursor_spelling(ctx.api.cursor_get_argument(cursor, u32(index))), ctx.allocator)) + } else { + append(¶m_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), diff --git a/compiler/parser/parser.odin b/compiler/parser/parser.odin index 1a24037..b398d67 100644 --- a/compiler/parser/parser.odin +++ b/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 } diff --git a/compiler/translatec/translatec.odin b/compiler/translatec/translatec.odin index 6d25a4f..f1a6e6f 100644 --- a/compiler/translatec/translatec.odin +++ b/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 -// the parser requires parameter names; names do not affect type identity. +// render_c_func writes `c_func(name T0, ...) R`, using the real C parameter name +// 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 diff --git a/compiler_tests.odin b/compiler_tests.odin index afdc130..41599a4 100644 --- a/compiler_tests.odin +++ b/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")) - testing.expect(t, strings.contains(output, "imported_add :: c_func(arg0 c_int, arg1 c_int) c_int")) + // Function-pointer types carry no parameter names, so the callback renders `_`. + 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) +} diff --git a/testbed/cstdio.bro b/testbed/cstdio.bro index 2e6353c..109ef84 100644 --- a/testbed/cstdio.bro +++ b/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 diff --git a/testbed/cstdio.h b/testbed/cstdio.h deleted file mode 100644 index 563ef4e..0000000 --- a/testbed/cstdio.h +++ /dev/null @@ -1 +0,0 @@ -int printf(const char *format, ...); diff --git a/testbed/main.bro b/testbed/main.bro index 1864162..ca39afe 100644 --- a/testbed/main.bro +++ b/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 }