Skip to content

Commit

Permalink
feat: Warn when discarded value
Browse files Browse the repository at this point in the history
closes #162
  • Loading branch information
giann committed Sep 20, 2023
1 parent 0248b60 commit 51d84a3
Show file tree
Hide file tree
Showing 32 changed files with 277 additions and 59 deletions.
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 return 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

0 comments on commit 51d84a3

Please sign in to comment.