Skip to content
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

feat: Warn when discarded value #179

Merged
merged 1 commit into from
Sep 20, 2023
Merged
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,15 @@
- `std.serialize` takes any buzz value and return a serializable version of it (objects become maps, etc.) provided the data is has no circular reference and does not contain not serializable values (functions, fibers, etc.)
- UTF8 helpers: `str.utf8Len`, `str.utf8Codepoints`, `str.utf8Valid`
- New integer literal for single chars: `'A' == 65`
- Compiler will warn you when a local or global is never used or when an expression value is discarded. To silence those warnings you can use the `_ = <expression>` or named the local/global `_`.

## Changed

- `json` lib is renamed `serialize`
- `Json` now returns a `Boxed` object (which can be reused in other contexts than JSON)
- Identifiers can now have `_` since pattern delimiters have changed
- Changed pattern delimiters (https://github.com/buzz-language/buzz/issues/165)
- `list.append` does not returned the appended value anymore

## Fixed

Expand Down
4 changes: 1 addition & 3 deletions src/builtin/list.zig
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,7 @@ pub fn append(ctx: *NativeCtx) c_int {
return -1;
};

ctx.vm.push(list_value);

return 1;
return 0;
}

pub fn insert(ctx: *NativeCtx) c_int {
Expand Down
4 changes: 2 additions & 2 deletions src/chunk.zig
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,8 @@ pub const Chunk = struct {
}

pub fn write(self: *Self, code: u32, where: Token) !void {
_ = try self.code.append(code);
_ = try self.lines.append(where);
try self.code.append(code);
try self.lines.append(where);
}

pub fn addConstant(self: *Self, vm: ?*VM, value: Value) !u24 {
Expand Down
2 changes: 1 addition & 1 deletion src/lib/serialize.buzz
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ object JsonParser {
const str? char = this.peek();

if (char == " " or char == "\r" or char == "\t" or char == "\n") {
this.advance();
_ = this.advance();
} else {
return;
}
Expand Down
24 changes: 24 additions & 0 deletions src/node.zig
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,16 @@ pub const ExpressionNode = struct {
return GenError.NotConstant;
}

fn isLoneExpression(self: *Self) bool {
// zig fmt: off
return (self.expression.node_type != .NamedVariable or NamedVariableNode.cast(self.expression).?.value == null)
and (self.expression.node_type != .Subscript or SubscriptNode.cast(self.expression).?.value == null)
and (self.expression.node_type != .Dot or DotNode.cast(self.expression).?.value == null)
and self.expression.type_def != null
and self.expression.type_def.?.def_type != .Void;
// zig fmt: on
}

fn generate(nodePtr: *anyopaque, codegenPtr: *anyopaque, breaks: ?*std.ArrayList(usize)) anyerror!?*ObjFunction {
var codegen: *CodeGen = @ptrCast(@alignCast(codegenPtr));
const node: *ParseNode = @ptrCast(@alignCast(nodePtr));
Expand All @@ -273,6 +283,20 @@ pub const ExpressionNode = struct {

try codegen.emitOpCode(node.location, .OP_POP);

if (self.isLoneExpression()) {
const type_def_str = self.expression.type_def.?.toStringAlloc(codegen.gc.allocator) catch unreachable;
defer type_def_str.deinit();

codegen.reporter.warnFmt(
.discarded_value,
node.location,
"Discarded value of type `{s}`",
.{
type_def_str.items,
},
);
}

try node.patchOptJumps(codegen);
try node.endScope(codegen);

Expand Down
2 changes: 1 addition & 1 deletion src/obj.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ pub const ObjList = struct {
.name = try parser.gc.copyString("append"),
.parameters = parameters,
.defaults = std.AutoArrayHashMap(*ObjString, Value).init(parser.gc.allocator),
.return_type = obj_list,
.return_type = try parser.gc.type_registry.getTypeDef(.{ .def_type = .Void }),
.yield_type = try parser.gc.type_registry.getTypeDef(.{ .def_type = .Void }),
.generic_types = std.AutoArrayHashMap(*ObjString, *ObjTypeDef).init(parser.gc.allocator),
.function_type = .Extern,
Expand Down
134 changes: 128 additions & 6 deletions src/parser.zig
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,17 @@ pub const Local = struct {
depth: i32,
is_captured: bool,
constant: bool,
referenced: bool = false,

pub fn isReferenced(self: Local) bool {
// zig fmt: off
return self.referenced
or self.type_def.def_type == .Void
or self.type_def.def_type == .Placeholder
or self.name.string[0] == '$'
or (self.name.string[0] == '_' and self.name.string.len == 1);
// zig fmt: on
}
};

pub const Global = struct {
Expand All @@ -176,8 +187,26 @@ pub const Global = struct {
export_alias: ?[]const u8 = null,
hidden: bool = false,
constant: bool,
referenced: bool = false,
// When resolving a placeholder, the start of the resolution is the global
// If `constant` is true, we can search for any `.Assignment` link and fail then.

pub fn isReferenced(self: Global) bool {
const function_type = if (self.type_def.def_type == .Function)
self.type_def.resolved_type.?.Function.function_type
else
null;

// zig fmt: off
return self.referenced
or self.type_def.def_type == .Void
or self.type_def.def_type == .Placeholder
or (function_type != null and (function_type == .Extern or function_type == .Abstract or function_type == .EntryPoint or function_type == .ScriptEntryPoint))
or self.name.string[0] == '$'
or (self.name.string[0] == '_' and self.name.string.len == 1)
or self.exported;
// zig fmt: on
}
};

pub const UpValue = struct { index: u8, is_local: bool };
Expand Down Expand Up @@ -602,6 +631,46 @@ pub const Parser = struct {
}

fn endFrame(self: *Self) *FunctionNode {
var i: usize = 0;
while (i < self.current.?.local_count) : (i += 1) {
const local = self.current.?.locals[i];

// Check discarded locals
if (!local.isReferenced()) {
const type_def_str = local.type_def.toStringAlloc(self.gc.allocator) catch unreachable;
defer type_def_str.deinit();

self.reporter.warnFmt(
.unused_argument,
local.location,
"Unused local of type `{s}`",
.{
type_def_str.items,
},
);
}
}

// If global scope, check unused globals
const function_type = self.current.?.function_node.node.type_def.?.resolved_type.?.Function.function_type;
if (function_type == .Script or function_type == .ScriptEntryPoint) {
for (self.globals.items) |global| {
if (!global.isReferenced()) {
const type_def_str = global.type_def.toStringAlloc(self.gc.allocator) catch unreachable;
defer type_def_str.deinit();

self.reporter.warnFmt(
.unused_argument,
global.location,
"Unused global of type `{s}`",
.{
type_def_str.items,
},
);
}
}
}

var current_node = self.current.?.function_node;
self.current = self.current.?.enclosing;

Expand All @@ -618,12 +687,29 @@ pub const Parser = struct {
current.scope_depth -= 1;

while (current.local_count > 0 and current.locals[current.local_count - 1].depth > current.scope_depth) {
if (current.locals[current.local_count - 1].is_captured) {
const local = current.locals[current.local_count - 1];

if (local.is_captured) {
try closing.append(.OP_CLOSE_UPVALUE);
} else {
try closing.append(.OP_POP);
}

// Check discarded locals
if (!local.isReferenced()) {
const type_def_str = local.type_def.toStringAlloc(self.gc.allocator) catch unreachable;
defer type_def_str.deinit();

self.reporter.warnFmt(
.unused_argument,
local.location,
"Unused local of type `{s}`",
.{
type_def_str.items,
},
);
}

current.local_count -= 1;
}

Expand Down Expand Up @@ -1210,6 +1296,13 @@ pub const Parser = struct {
constant,
true,
)
else if (self.parser.current_token != null and self.parser.current_token.?.token_type == .Identifier and self.parser.current_token.?.lexeme.len == 1 and self.parser.current_token.?.lexeme[0] == '_')
try self.varDeclaration(
try self.gc.type_registry.getTypeDef(.{ .def_type = .Any }),
.Semicolon,
constant,
true,
)
else if (try self.match(.Fib))
try self.varDeclaration(
try self.parseFiberType(null),
Expand Down Expand Up @@ -1320,6 +1413,13 @@ pub const Parser = struct {
constant,
true,
);
} else if (self.parser.current_token != null and self.parser.current_token.?.token_type == .Identifier and self.parser.current_token.?.lexeme.len == 1 and self.parser.current_token.?.lexeme[0] == '_') {
return try self.varDeclaration(
try self.gc.type_registry.getTypeDef(.{ .def_type = .Any }),
.Semicolon,
constant,
true,
);
} else if (try self.match(.Type)) {
return try self.varDeclaration(
try self.gc.type_registry.getTypeDef(.{ .optional = try self.match(.Question), .def_type = .Type }),
Expand Down Expand Up @@ -1837,12 +1937,11 @@ pub const Parser = struct {
}

fn expressionStatement(self: *Self, hanging: bool) !*ParseNode {
const start_location = self.parser.previous_token.?;
var node = try self.gc.allocator.create(ExpressionNode);
node.* = ExpressionNode{
.expression = try self.expression(hanging),
};
node.node.location = start_location;
node.node.location = node.expression.location;
node.node.end_location = self.parser.previous_token.?;

try self.consume(.Semicolon, "Expected `;` after expression.");
Expand Down Expand Up @@ -2549,6 +2648,7 @@ pub const Parser = struct {
// Search for a global with that name
if (try self.resolveGlobal(null, identifier)) |slot| {
const global: *Global = &self.globals.items[slot];
global.referenced = true;
var alias: ?Token = null;

global.exported = true;
Expand All @@ -2575,6 +2675,8 @@ pub const Parser = struct {
} else {
self.exporting = true;
if (try self.declarationsOrExport(false)) |decl| {
self.globals.items[self.globals.items.len - 1].referenced = true;

self.exporting = false;
var node = try self.gc.allocator.create(ExportNode);
node.* = ExportNode{
Expand Down Expand Up @@ -4930,6 +5032,10 @@ pub const Parser = struct {

const function_node = try self.function(name_token, FunctionType.Test, null);

if (function_node.type_def) |type_def| {
self.globals.items[slot].type_def = type_def;
}

// Make it as a global definition
var node = try self.gc.allocator.create(VarDeclarationNode);
node.* = VarDeclarationNode{
Expand Down Expand Up @@ -5718,7 +5824,7 @@ pub const Parser = struct {
break;
}

if (mem.eql(u8, name.lexeme, local.name.string)) {
if (!mem.eql(u8, name.lexeme, "_") and mem.eql(u8, name.lexeme, local.name.string)) {
self.reporter.reportWithOrigin(
.variable_already_exists,
name,
Expand All @@ -5737,8 +5843,8 @@ pub const Parser = struct {
} else {
if (check_name) {
// Check a global with the same name doesn't exists
for (self.globals.items, 0..) |global, index| {
if (mem.eql(u8, name.lexeme, global.name.string) and !global.hidden) {
for (self.globals.items, 0..) |*global, index| {
if (!mem.eql(u8, name.lexeme, "_") and mem.eql(u8, name.lexeme, global.name.string) and !global.hidden) {
// If we found a placeholder with that name, try to resolve it with `variable_type`
if (global.type_def.def_type == .Placeholder and global.type_def.resolved_type.?.Placeholder.name != null and mem.eql(u8, name.lexeme, global.type_def.resolved_type.?.Placeholder.name.?.string)) {
// A function declares a global with an incomplete typedef so that it can handle recursion
Expand All @@ -5759,6 +5865,8 @@ pub const Parser = struct {
try self.resolvePlaceholder(global.type_def, variable_type, constant);
}

global.referenced = true;

return index;
} else if (global.prefix == null) {
self.reportError(.variable_already_exists, "A global with the same name already exists.");
Expand All @@ -5777,13 +5885,16 @@ pub const Parser = struct {
return 0;
}

const function_type = self.current.?.function_node.node.type_def.?.resolved_type.?.Function.function_type;
self.current.?.locals[self.current.?.local_count] = Local{
.name = try self.gc.copyString(name.lexeme),
.location = name,
.depth = -1,
.is_captured = false,
.type_def = local_type,
.constant = constant,
// Extern and abstract function arguments are considered referenced
.referenced = function_type == .Extern or function_type == .Abstract,
};

self.current.?.local_count += 1;
Expand Down Expand Up @@ -5844,6 +5955,10 @@ pub const Parser = struct {
return null;
}

if (std.mem.eql(u8, name.lexeme, "_")) {
return null;
}

var i: usize = frame.local_count - 1;
while (i >= 0) : (i -= 1) {
var local: *Local = &frame.locals[i];
Expand All @@ -5852,6 +5967,7 @@ pub const Parser = struct {
self.reportError(.local_initializer, "Can't read local variable in its own initializer.");
}

local.referenced = true;
return i;
}

Expand All @@ -5867,6 +5983,10 @@ pub const Parser = struct {
return null;
}

if (std.mem.eql(u8, name.lexeme, "_")) {
return null;
}

var i: usize = self.globals.items.len - 1;
while (i >= 0) : (i -= 1) {
var global: *Global = &self.globals.items[i];
Expand All @@ -5879,6 +5999,8 @@ pub const Parser = struct {
);
}

global.referenced = true;

return i;
// Is it an import prefix?
} else if (global.prefix != null and mem.eql(u8, name.lexeme, global.prefix.?)) {
Expand Down
Loading
Loading