Skip to content

stage2: detect ambiguous decl references #8921

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jun 8, 2021
Merged

stage2: detect ambiguous decl references #8921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 7 additions & 7 deletions lib/std/zig/render.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2562,8 +2562,8 @@ fn rowSize(tree: ast.Tree, exprs: []const ast.Node.Index, rtoken: ast.TokenIndex
fn AutoIndentingStream(comptime UnderlyingWriter: type) type {
return struct {
const Self = @This();
pub const Error = UnderlyingWriter.Error;
pub const Writer = std.io.Writer(*Self, Error, write);
pub const WriteError = UnderlyingWriter.Error;
pub const Writer = std.io.Writer(*Self, WriteError, write);

underlying_writer: UnderlyingWriter,

Expand All @@ -2589,7 +2589,7 @@ fn AutoIndentingStream(comptime UnderlyingWriter: type) type {
return .{ .context = self };
}

pub fn write(self: *Self, bytes: []const u8) Error!usize {
pub fn write(self: *Self, bytes: []const u8) WriteError!usize {
if (bytes.len == 0)
return @as(usize, 0);

Expand All @@ -2612,7 +2612,7 @@ fn AutoIndentingStream(comptime UnderlyingWriter: type) type {
self.indent_delta = new_indent_delta;
}

fn writeNoIndent(self: *Self, bytes: []const u8) Error!usize {
fn writeNoIndent(self: *Self, bytes: []const u8) WriteError!usize {
if (bytes.len == 0)
return @as(usize, 0);

Expand All @@ -2622,7 +2622,7 @@ fn AutoIndentingStream(comptime UnderlyingWriter: type) type {
return bytes.len;
}

pub fn insertNewline(self: *Self) Error!void {
pub fn insertNewline(self: *Self) WriteError!void {
_ = try self.writeNoIndent("\n");
}

Expand All @@ -2632,7 +2632,7 @@ fn AutoIndentingStream(comptime UnderlyingWriter: type) type {
}

/// Insert a newline unless the current line is blank
pub fn maybeInsertNewline(self: *Self) Error!void {
pub fn maybeInsertNewline(self: *Self) WriteError!void {
if (!self.current_line_empty)
try self.insertNewline();
}
Expand Down Expand Up @@ -2673,7 +2673,7 @@ fn AutoIndentingStream(comptime UnderlyingWriter: type) type {
}

/// Writes ' ' bytes if the current line is empty
fn applyIndent(self: *Self) Error!void {
fn applyIndent(self: *Self) WriteError!void {
const current_indent = self.currentIndent();
if (self.current_line_empty and current_indent > 0) {
if (self.disabled_offset == null) {
Expand Down
39 changes: 37 additions & 2 deletions src/AstGen.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2355,6 +2355,7 @@ fn varDecl(
.name = ident_name,
.ptr = init_scope.rl_ptr,
.token_src = name_token,
.is_comptime = true,
};
return &sub_scope.base;
},
Expand Down Expand Up @@ -2410,6 +2411,7 @@ fn varDecl(
.name = ident_name,
.ptr = var_data.alloc,
.token_src = name_token,
.is_comptime = is_comptime,
};
return &sub_scope.base;
},
Expand Down Expand Up @@ -3354,7 +3356,7 @@ fn structDeclInner(
};
defer block_scope.instructions.deinit(gpa);

var namespace: Scope.Namespace = .{ .parent = &gz.base };
var namespace: Scope.Namespace = .{ .parent = scope };
defer namespace.decls.deinit(gpa);

var wip_decls: WipDecls = .{};
Expand Down Expand Up @@ -5347,6 +5349,7 @@ fn forExpr(
.name = index_name,
.ptr = index_ptr,
.token_src = index_token,
.is_comptime = parent_gz.force_comptime,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this correct?

Copy link
Member

Choose a reason for hiding this comment

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

This should use is_inline since the index is also comptime known for inline loops. Also sorry for not answering sooner.

Copy link
Member

Choose a reason for hiding this comment

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

Btw @andrewrk, @SpexGuy What should the correct behavior be for

pub fn main() void {
    comptime var i = 0;
    inline while (i < 5) : (i+=1) {
        const S = struct {
            fn print() void {
                std.debug.print("loop {d}\n", .{i});
            }
        };
        S.print();
    }
}

Stage1 prints

loop 5
loop 5
loop 5
loop 5
loop 5

but I think most people would assume it to print

loop 0
loop 1
loop 2
loop 3
loop 4

};
break :blk &index_scope.base;
};
Expand Down Expand Up @@ -6072,9 +6075,16 @@ fn identifier(
const name_str_index = try astgen.identAsString(ident_token);
{
var s = scope;
var found_already: ?ast.Node.Index = null; // we have found a decl with the same name already
var hit_namespace = false;
while (true) switch (s.tag) {
.local_val => {
const local_val = s.cast(Scope.LocalVal).?;
if (hit_namespace) {
// captures of non-locals need to be emitted as decl_val or decl_ref
// This *might* be capturable depending on if it is comptime known
break;
}
if (local_val.name == name_str_index) {
return rvalue(gz, scope, rl, local_val.inst, ident);
}
Expand All @@ -6083,6 +6093,15 @@ fn identifier(
.local_ptr => {
const local_ptr = s.cast(Scope.LocalPtr).?;
if (local_ptr.name == name_str_index) {
if (hit_namespace) {
if (local_ptr.is_comptime)
break
else
return astgen.failNodeNotes(ident, "'{s}' not accessible from inner function", .{ident_name}, &.{
try astgen.errNoteTok(local_ptr.token_src, "declared here", .{}),
// TODO add crossed function definition here note.
});
}
switch (rl) {
.ref, .none_or_ref => return local_ptr.ptr,
else => {
Expand All @@ -6095,7 +6114,22 @@ fn identifier(
},
.gen_zir => s = s.cast(GenZir).?.parent,
.defer_normal, .defer_error => s = s.cast(Scope.Defer).?.parent,
.namespace, .top => break, // TODO look for ambiguous references to decls
// look for ambiguous references to decls
.namespace => {
const ns = s.cast(Scope.Namespace).?;
if (ns.decls.get(name_str_index)) |i| {
if (found_already) |f|
return astgen.failNodeNotes(ident, "ambiguous reference", .{}, &.{
try astgen.errNoteNode(i, "declared here", .{}),
try astgen.errNoteNode(f, "also declared here", .{}),
})
else
found_already = i;
}
hit_namespace = true;
s = ns.parent;
},
.top => break,
};
}

Expand Down Expand Up @@ -8044,6 +8078,7 @@ const Scope = struct {
token_src: ast.TokenIndex,
/// String table index.
name: u32,
is_comptime: bool,
};

const Defer = struct {
Expand Down
28 changes: 28 additions & 0 deletions test/stage2/test.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,34 @@ pub fn addCases(ctx: *TestContext) !void {
":10:8: error: cannot return from defer expression",
});

ctx.compileError("ambiguous references", linux_x64,
\\const T = struct {
\\ const T = struct {
\\ fn f() void {
\\ _ = T;
\\ }
\\ };
\\};
, &.{
":4:17: error: ambiguous reference",
":1:1: note: declared here",
":2:5: note: also declared here",
});

ctx.compileError("inner func accessing outer var", linux_x64,
\\pub fn f() void {
\\ var bar: bool = true;
\\ const S = struct {
\\ fn baz() bool {
\\ return bar;
\\ }
\\ };
\\}
, &.{
":5:20: error: 'bar' not accessible from inner function",
":2:9: note: declared here",
});

ctx.compileError("global variable redeclaration", linux_x64,
\\// dummy comment
\\var foo = false;
Expand Down