Skip to content

Compiler: allow functions prototypes to have their own scopes #18044

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

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/compiler_rt/clzsi2_test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const testing = @import("std").testing;

fn test__clzsi2(a: u32, expected: i32) !void {
const nakedClzsi2 = clz.__clzsi2;
const actualClzsi2 = @as(*const fn (a: i32) callconv(.C) i32, @ptrCast(&nakedClzsi2));
const actualClzsi2 = @as(*const fn (i32) callconv(.C) i32, @ptrCast(&nakedClzsi2));
const x: i32 = @bitCast(a);
const result = actualClzsi2(x);
try testing.expectEqual(expected, result);
Expand Down
4 changes: 2 additions & 2 deletions lib/compiler_rt/int.zig
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ fn test_one_aeabi_ldivmod(a: i64, b: i64, expected_q: i64, expected_r: i64) !voi
q: i64, // r1:r0
r: i64, // r3:r2
};
const actualIdivmod = @as(*const fn (a: i64, b: i64) callconv(.AAPCS) LdivmodRes, @ptrCast(&arm.__aeabi_ldivmod));
const actualIdivmod = @as(*const fn (i64, i64) callconv(.AAPCS) LdivmodRes, @ptrCast(&arm.__aeabi_ldivmod));
const arm_res = actualIdivmod(a, b);
try testing.expectEqual(expected_q, arm_res.q);
try testing.expectEqual(expected_r, arm_res.r);
Expand Down Expand Up @@ -266,7 +266,7 @@ fn test_one_aeabi_idivmod(a: i32, b: i32, expected_q: i32, expected_r: i32) !voi
q: i32, // r0
r: i32, // r1
};
const actualIdivmod = @as(*const fn (a: i32, b: i32) callconv(.AAPCS) IdivmodRes, @ptrCast(&arm.__aeabi_idivmod));
const actualIdivmod = @as(*const fn (i32, i32) callconv(.AAPCS) IdivmodRes, @ptrCast(&arm.__aeabi_idivmod));
const arm_res = actualIdivmod(a, b);
try testing.expectEqual(expected_q, arm_res.q);
try testing.expectEqual(expected_r, arm_res.r);
Expand Down
2 changes: 1 addition & 1 deletion lib/compiler_rt/udivmoddi4_test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const ARMRes = extern struct {
};

fn test__aeabi_uldivmod(a: u64, b: u64, expected_q: u64, expected_r: u64) !void {
const actualUldivmod = @as(*const fn (a: u64, b: u64) callconv(.AAPCS) ARMRes, @ptrCast(&__aeabi_uldivmod));
const actualUldivmod = @as(*const fn (u64, u64) callconv(.AAPCS) ARMRes, @ptrCast(&__aeabi_uldivmod));
const arm_res = actualUldivmod(a, b);
try testing.expectEqual(expected_q, arm_res.q);
try testing.expectEqual(expected_r, arm_res.r);
Expand Down
2 changes: 1 addition & 1 deletion lib/std/os.zig
Original file line number Diff line number Diff line change
Expand Up @@ -5475,7 +5475,7 @@ pub fn nanosleep(seconds: u64, nanoseconds: u64) void {
pub fn dl_iterate_phdr(
context: anytype,
comptime Error: type,
comptime callback: fn (info: *dl_phdr_info, size: usize, context: @TypeOf(context)) Error!void,
comptime callback: fn (info: *dl_phdr_info, size: usize, @TypeOf(context)) Error!void,
) Error!void {
const Context = @TypeOf(context);

Expand Down
6 changes: 3 additions & 3 deletions lib/std/os/uefi/tables/boot_services.zig
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ pub const BootServices = extern struct {
freePool: *const fn (buffer: [*]align(8) u8) callconv(cc) Status,

/// Creates an event.
createEvent: *const fn (type: u32, notify_tpl: usize, notify_func: ?*const fn (Event, ?*anyopaque) callconv(cc) void, notifyCtx: ?*const anyopaque, event: *Event) callconv(cc) Status,
createEvent: *const fn (u32, notify_tpl: usize, notify_func: ?*const fn (Event, ?*anyopaque) callconv(cc) void, notifyCtx: ?*const anyopaque, event: *Event) callconv(cc) Status,

/// Sets the type of timer and the trigger time for a timer event.
setTimer: *const fn (event: Event, type: TimerDelay, triggerTime: u64) callconv(cc) Status,
setTimer: *const fn (event: Event, timerDelay: TimerDelay, triggerTime: u64) callconv(cc) Status,

/// Stops execution until an event is signaled.
waitForEvent: *const fn (event_len: usize, events: [*]const Event, index: *usize) callconv(cc) Status,
Expand Down Expand Up @@ -156,7 +156,7 @@ pub const BootServices = extern struct {
setMem: *const fn (buffer: [*]u8, size: usize, value: u8) callconv(cc) void,

/// Creates an event in a group.
createEventEx: *const fn (type: u32, notify_tpl: usize, notify_func: EfiEventNotify, notify_ctx: *const anyopaque, event_group: *align(8) const Guid, event: *Event) callconv(cc) Status,
createEventEx: *const fn (u32, notify_tpl: usize, notify_func: EfiEventNotify, notify_ctx: *const anyopaque, event_group: *align(8) const Guid, event: *Event) callconv(cc) Status,

/// Opens a protocol with a structure as the loaded image for a UEFI application
pub fn openProtocolSt(self: *BootServices, comptime protocol: type, handle: Handle) !*protocol {
Expand Down
10 changes: 5 additions & 5 deletions lib/std/sort.zig
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ pub fn binarySearch(
key: anytype,
items: []const T,
context: anytype,
comptime compareFn: fn (context: @TypeOf(context), key: @TypeOf(key), mid_item: T) math.Order,
comptime compareFn: fn (@TypeOf(context), @TypeOf(key), mid_item: T) math.Order,
) ?usize {
var left: usize = 0;
var right: usize = items.len;
Expand Down Expand Up @@ -538,7 +538,7 @@ pub fn min(
comptime T: type,
items: []const T,
context: anytype,
comptime lessThan: fn (context: @TypeOf(context), lhs: T, rhs: T) bool,
comptime lessThan: fn (@TypeOf(context), lhs: T, rhs: T) bool,
) ?T {
const i = argMin(T, items, context, lessThan) orelse return null;
return items[i];
Expand All @@ -558,7 +558,7 @@ pub fn argMax(
comptime T: type,
items: []const T,
context: anytype,
comptime lessThan: fn (context: @TypeOf(context), lhs: T, rhs: T) bool,
comptime lessThan: fn (@TypeOf(context), lhs: T, rhs: T) bool,
) ?usize {
if (items.len == 0) {
return null;
Expand Down Expand Up @@ -590,7 +590,7 @@ pub fn max(
comptime T: type,
items: []const T,
context: anytype,
comptime lessThan: fn (context: @TypeOf(context), lhs: T, rhs: T) bool,
comptime lessThan: fn (@TypeOf(context), lhs: T, rhs: T) bool,
) ?T {
const i = argMax(T, items, context, lessThan) orelse return null;
return items[i];
Expand All @@ -610,7 +610,7 @@ pub fn isSorted(
comptime T: type,
items: []const T,
context: anytype,
comptime lessThan: fn (context: @TypeOf(context), lhs: T, rhs: T) bool,
comptime lessThan: fn (@TypeOf(context), lhs: T, rhs: T) bool,
) bool {
var i: usize = 1;
while (i < items.len) : (i += 1) {
Expand Down
2 changes: 1 addition & 1 deletion lib/std/sort/pdq.zig
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub fn pdq(
comptime T: type,
items: []T,
context: anytype,
comptime lessThanFn: fn (context: @TypeOf(context), lhs: T, rhs: T) bool,
comptime lessThanFn: fn (@TypeOf(context), lhs: T, rhs: T) bool,
) void {
const Context = struct {
items: []T,
Expand Down
2 changes: 1 addition & 1 deletion lib/std/zig/Parse.zig
Original file line number Diff line number Diff line change
Expand Up @@ -3996,7 +3996,7 @@ fn parseBuiltinCall(p: *Parse) !Node.Index {
}

/// IfPrefix <- KEYWORD_if LPAREN Expr RPAREN PtrPayload?
fn parseIf(p: *Parse, comptime bodyParseFn: fn (p: *Parse) Error!Node.Index) !Node.Index {
fn parseIf(p: *Parse, comptime bodyParseFn: fn (*Parse) Error!Node.Index) !Node.Index {
const if_token = p.eatToken(.keyword_if) orelse return null_node;
_ = try p.expectToken(.l_paren);
const condition = try p.expectExpr();
Expand Down
93 changes: 72 additions & 21 deletions src/AstGen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,7 @@ fn fnProtoExpr(
const block_inst = try gz.makeBlockInst(.block_inline, node);

var noalias_bits: u32 = 0;
var params_scope = &block_scope.base;
const is_var_args = is_var_args: {
var param_type_i: usize = 0;
var it = fn_proto.iterate(tree);
Expand All @@ -1303,80 +1304,129 @@ fn fnProtoExpr(
} else false;

const param_name: u32 = if (param.name_token) |name_token| blk: {
if (mem.eql(u8, "_", tree.tokenSlice(name_token)))
const name_bytes = tree.tokenSlice(name_token);
if (mem.eql(u8, "_", name_bytes))
break :blk 0;

break :blk try astgen.identAsString(name_token);
const param_name = try astgen.identAsString(name_token);
try astgen.detectLocalShadowing(params_scope, param_name, name_token, name_bytes, .@"function prototype parameter");
break :blk param_name;
} else 0;

if (is_anytype) {
const param_inst = if (is_anytype) param: {
const name_token = param.name_token orelse param.anytype_ellipsis3.?;

const tag: Zir.Inst.Tag = if (is_comptime)
.param_anytype_comptime
else
.param_anytype;
_ = try block_scope.addStrTok(tag, param_name, name_token);
} else {
break :param try block_scope.addStrTok(tag, param_name, name_token);
} else param: {
const param_type_node = param.type_expr;
assert(param_type_node != 0);
var param_gz = block_scope.makeSubBlock(scope);
defer param_gz.unstack();
const param_type = try expr(&param_gz, scope, coerced_type_ri, param_type_node);
const param_type = try expr(&param_gz, params_scope, coerced_type_ri, param_type_node);
const param_inst_expected: Zir.Inst.Index = @enumFromInt(astgen.instructions.len + 1);
_ = try param_gz.addBreakWithSrcNode(.break_inline, param_inst_expected, param_type, param_type_node);
const main_tokens = tree.nodes.items(.main_token);
const name_token = param.name_token orelse main_tokens[param_type_node];
const tag: Zir.Inst.Tag = if (is_comptime) .param_comptime else .param;
const param_inst = try block_scope.addParam(&param_gz, tag, name_token, param_name, param.first_doc_comment);
assert(param_inst_expected == param_inst);
}
break :param param_inst.toRef();
};

if (param_name == 0) continue;

const sub_scope = try astgen.arena.create(Scope.LocalVal);
sub_scope.* = .{
.parent = params_scope,
.gen_zir = &block_scope,
.name = param_name,
.inst = param_inst,
.token_src = param.name_token.?,
.id_cat = .@"function prototype parameter",
};
params_scope = &sub_scope.base;
}
break :is_var_args false;
};

var align_gz = block_scope.makeSubBlock(params_scope);
defer align_gz.unstack();
const align_ref: Zir.Inst.Ref = if (fn_proto.ast.align_expr == 0) .none else inst: {
break :inst try expr(&block_scope, scope, align_ri, fn_proto.ast.align_expr);
const inst = try expr(&block_scope, params_scope, coerced_align_ri, fn_proto.ast.align_expr);
if (align_gz.instructionsSlice().len == 0) {
// In this case we will send a len=0 body which can be encoded more efficiently.
break :inst inst;
}
const param_index: Zir.Inst.Index = @enumFromInt(astgen.instructions.len + 1);
_ = try align_gz.addBreakWithSrcNode(.break_inline, param_index, inst, fn_proto.ast.align_expr);
break :inst inst;
};

if (fn_proto.ast.addrspace_expr != 0) {
return astgen.failNode(fn_proto.ast.addrspace_expr, "addrspace not allowed on function prototypes", .{});
}
var addrspace_gz = block_scope.makeSubBlock(params_scope);
Copy link
Member

Choose a reason for hiding this comment

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

Unused.

Copy link
Contributor Author

@wrongnull wrongnull Nov 19, 2023

Choose a reason for hiding this comment

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

Unused.

This is true but without it the compiler crashes with 'use of null' at line 11328 in AstGen.zig in addFunc. There is following doc comment in above this fuction

/// Must be called with the following stack set up:
    ///  * gz (bottom)
    ///  * align_gz
    ///  * addrspace_gz
    ///  * section_gz
    ///  * cc_gz
    ///  * ret_gz
    ///  * body_gz (top)

Copy link
Member

Choose a reason for hiding this comment

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

Huh, it's strange how it properly checks each of them individually in the else branch but only ret_gz in the then branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I rewrite this check in addFn or let it be as it is?

defer addrspace_gz.unstack();

if (fn_proto.ast.section_expr != 0) {
return astgen.failNode(fn_proto.ast.section_expr, "linksection not allowed on function prototypes", .{});
}
var section_gz = block_scope.makeSubBlock(params_scope);
Copy link
Member

Choose a reason for hiding this comment

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

Also unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also unused.

Same here

defer section_gz.unstack();

const cc: Zir.Inst.Ref = if (fn_proto.ast.callconv_expr != 0)
try expr(
var cc_gz = block_scope.makeSubBlock(params_scope);
defer cc_gz.unstack();
const cc: Zir.Inst.Ref = if (fn_proto.ast.callconv_expr == 0) .none else inst: {
const inst = try expr(
&block_scope,
scope,
.{ .rl = .{ .ty = .calling_convention_type } },
params_scope,
.{ .rl = .{ .coerced_ty = .calling_convention_type } },
fn_proto.ast.callconv_expr,
)
else
Zir.Inst.Ref.none;
);
if (cc_gz.instructionsSlice().len == 0) {
// In this case we will send a len=0 body which can be encoded more efficiently.
break :inst inst;
}
const param_index: Zir.Inst.Index = @enumFromInt(astgen.instructions.len + 1);
_ = try cc_gz.addBreakWithSrcNode(.break_inline, param_index, inst, fn_proto.ast.callconv_expr);
break :inst inst;
};

const maybe_bang = tree.firstToken(fn_proto.ast.return_type) - 1;
const is_inferred_error = token_tags[maybe_bang] == .bang;
if (is_inferred_error) {
return astgen.failTok(maybe_bang, "function prototype may not have inferred error set", .{});
}
const ret_ty = try expr(&block_scope, scope, coerced_type_ri, fn_proto.ast.return_type);
var ret_gz = block_scope.makeSubBlock(params_scope);
defer ret_gz.unstack();
const ret_ty = inst: {
const inst = try expr(&block_scope, params_scope, coerced_type_ri, fn_proto.ast.return_type);
if (ret_gz.instructionsSlice().len == 0) {
// In this case we will send a len=0 body which can be encoded more efficiently.
break :inst inst;
}
const param_index: Zir.Inst.Index = @enumFromInt(astgen.instructions.len + 1);
_ = try ret_gz.addBreakWithSrcNode(.break_inline, param_index, inst, fn_proto.ast.return_type);
break :inst inst;
};

const result = try block_scope.addFunc(.{
.src_node = fn_proto.ast.proto_node,

.cc_ref = cc,
.cc_gz = null,
.cc_gz = &cc_gz,
.align_ref = align_ref,
.align_gz = null,
.align_gz = &align_gz,
.ret_ref = ret_ty,
.ret_gz = null,
.ret_gz = &ret_gz,
.section_ref = .none,
.section_gz = null,
.section_gz = &section_gz,
.addrspace_ref = .none,
.addrspace_gz = null,
.addrspace_gz = &addrspace_gz,

.param_block = block_inst,
.body_gz = null,
Expand Down Expand Up @@ -10926,6 +10976,7 @@ const Scope = struct {
/// The category of identifier. These tag names are user-visible in compile errors.
const IdCat = enum {
@"function parameter",
@"function prototype parameter",
Copy link
Member

Choose a reason for hiding this comment

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

I don't think there is any benefit to separating this from @"function parameter".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think there is any benefit to separating this from @"function parameter".

I think with such wording compiler diagnostics are more accurate. But I can delete it if necessary

@"local constant",
@"local variable",
@"switch tag capture",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ pub fn sort(
comptime T: type,
items: []T,
context: anytype,
lessThan: *const fn (context: @TypeOf(context), lhs: T, rhs: T) u32,
lessThan: *const fn (@TypeOf(context), lhs: T, rhs: T) u32,
) void {
_ = items;
_ = lessThan;
Expand Down
15 changes: 14 additions & 1 deletion test/cases/compile_errors/parameter_shadowing_global.zig
Original file line number Diff line number Diff line change
@@ -1,12 +1,25 @@
const Foo = struct {};
fn f(Foo: i32) void {}
export fn entry() void {
export fn entry0() void {
f(1234);
}

export fn entry1() void {
const foo = 0;
_ = fn(foo: u32) void;
}

export fn entry2() void {
_ = fn(arg: u8, arg: u32) void;
}

// error
// backend=stage2
// target=native
//
// :2:6: error: function parameter shadows declaration of 'Foo'
// :1:1: note: declared here
// :9:12: error: function prototype parameter 'foo' shadows local constant from outer scope
// :8:11: note: previous declaration here
// :13:21: error: redeclaration of function prototype parameter 'arg'
// :13:12: note: previous declaration here