Skip to content

Commit

Permalink
disallow either types with differing is_type_val property (#1791)
Browse files Browse the repository at this point in the history 
fixes #1790
  • Loading branch information
@Techatrix
Techatrix authored Feb 27, 2024
1 parent b04a268 commit 2720509
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 30 deletions.
67 changes: 44 additions & 23 deletions src/analysis.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1767,21 +1767,22 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e
return (try analyser.resolveTypeOfNodeInternal(.{ .handle = handle, .node = if_node.ast.then_expr })) orelse break :blk;
}

var either = std.ArrayListUnmanaged(Type.EitherEntry){};
var either: std.BoundedArray(Type.TypeWithDescriptor, 2) = .{};

if (try analyser.resolveTypeOfNodeInternal(.{ .handle = handle, .node = if_node.ast.then_expr })) |t|
try either.append(analyser.arena.allocator(), .{ .type_with_handle = t, .descriptor = offsets.nodeToSlice(tree, if_node.ast.cond_expr) });
either.appendAssumeCapacity(.{ .type = t, .descriptor = offsets.nodeToSlice(tree, if_node.ast.cond_expr) });
if (try analyser.resolveTypeOfNodeInternal(.{ .handle = handle, .node = if_node.ast.else_expr })) |t|
try either.append(analyser.arena.allocator(), .{ .type_with_handle = t, .descriptor = try std.fmt.allocPrint(analyser.arena.allocator(), "!({s})", .{offsets.nodeToSlice(tree, if_node.ast.cond_expr)}) });
either.appendAssumeCapacity(.{ .type = t, .descriptor = try std.fmt.allocPrint(analyser.arena.allocator(), "!({s})", .{offsets.nodeToSlice(tree, if_node.ast.cond_expr)}) });

return Type.fromEither(analyser.arena.allocator(), either.items);
return Type.fromEither(analyser.arena.allocator(), either.constSlice());
},
.@"switch",
.switch_comma,
=> {
const extra = tree.extraData(datas[node].rhs, Ast.Node.SubRange);
const cases = tree.extra_data[extra.start..extra.end];

var either = std.ArrayListUnmanaged(Type.EitherEntry){};
var either = std.ArrayListUnmanaged(Type.TypeWithDescriptor){};

for (cases) |case| {
const switch_case = tree.fullSwitchCase(case).?;
Expand All @@ -1794,7 +1795,7 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e

if (try analyser.resolveTypeOfNodeInternal(.{ .handle = handle, .node = switch_case.ast.target_expr })) |t|
try either.append(analyser.arena.allocator(), .{
.type_with_handle = t,
.type = t,
.descriptor = try descriptor.toOwnedSlice(analyser.arena.allocator()),
});
}
Expand Down Expand Up @@ -2100,11 +2101,12 @@ fn resolveTypeOfNodeUncached(analyser: *Analyser, node_handle: NodeWithHandle) e
// TODO Make this better, nested levels of type vals
pub const Type = struct {
pub const EitherEntry = struct {
type_with_handle: Type,
/// the `is_type_val` property is inherited from the containing `Type`
type_data: Data,
descriptor: []const u8,
};

data: union(enum) {
pub const Data = union(enum) {
/// *T, [*]T, [T], [*c]T
pointer: struct {
size: std.builtin.Type.Pointer.Size,
Expand Down Expand Up @@ -2152,7 +2154,9 @@ pub const Type = struct {
/// this stores both the type and the value
index: InternPool.Index,
},
},
};

data: Data,
/// If true, the type `type`, the attached data is the value of the type value.
/// ```zig
/// const foo = u32; // is_type_val == true
Expand Down Expand Up @@ -2199,7 +2203,8 @@ pub const Type = struct {
.either => |entries| {
for (entries) |entry| {
hasher.update(entry.descriptor);
entry.type_with_handle.hashWithHasher(hasher);
const entry_ty = Type{ .data = entry.type_data, .is_type_val = self.is_type_val };
entry_ty.hashWithHasher(hasher);
}
},
.ip_index => |payload| {
Expand Down Expand Up @@ -2247,7 +2252,9 @@ pub const Type = struct {
if (a_entries.len != b_entries.len) return false;
for (a_entries, b_entries) |a_entry, b_entry| {
if (!std.mem.eql(u8, a_entry.descriptor, b_entry.descriptor)) return false;
if (!a_entry.type_with_handle.eql(b_entry.type_with_handle)) return false;
const a_entry_ty = Type{ .data = a_entry.type_data, .is_type_val = a.is_type_val };
const b_entry_ty = Type{ .data = b_entry.type_data, .is_type_val = b.is_type_val };
if (!a_entry_ty.eql(b_entry_ty)) return false;
}
},
.ip_index => |a_payload| {
Expand Down Expand Up @@ -2276,26 +2283,34 @@ pub const Type = struct {
};
}

pub fn fromEither(arena: std.mem.Allocator, entries: []const Type.EitherEntry) error{OutOfMemory}!?Type {
pub const TypeWithDescriptor = struct {
type: Type,
descriptor: []const u8,
};

pub fn fromEither(arena: std.mem.Allocator, entries: []const TypeWithDescriptor) error{OutOfMemory}!?Type {
if (entries.len == 0)
return null;

if (entries.len == 1)
return entries[0].type_with_handle;
return entries[0].type;

// Note that we don't hash/equate descriptors to remove
// duplicates

const DeduplicatorContext = struct {
pub fn hash(self: @This(), item: Type.EitherEntry) u32 {
_ = self;
return item.type_with_handle.hash32();
const ty = Type{ .data = item.type_data, .is_type_val = true };
return ty.hash32();
}

pub fn eql(self: @This(), a: Type.EitherEntry, b: Type.EitherEntry, b_index: usize) bool {
_ = b_index;
_ = self;
return a.type_with_handle.eql(b.type_with_handle);
const a_ty = Type{ .data = a.type_data, .is_type_val = true };
const b_ty = Type{ .data = b.type_data, .is_type_val = true };
return a_ty.eql(b_ty);
}
};
const Deduplicator = std.ArrayHashMapUnmanaged(Type.EitherEntry, void, DeduplicatorContext, true);
Expand All @@ -2306,14 +2321,18 @@ pub const Type = struct {
var has_type_val: bool = false;

for (entries) |entry| {
try deduplicator.put(arena, entry, {});
if (entry.type_with_handle.is_type_val) {
try deduplicator.put(
arena,
.{ .type_data = entry.type.data, .descriptor = entry.descriptor },
{},
);
if (entry.type.is_type_val) {
has_type_val = true;
}
}

if (deduplicator.count() == 1)
return entries[0].type_with_handle;
return entries[0].type;

return .{
.data = .{ .either = try arena.dupe(Type.EitherEntry, deduplicator.keys()) },
Expand All @@ -2333,7 +2352,8 @@ pub const Type = struct {
switch (ty.data) {
.either => |entries| {
for (entries) |entry| {
try entry.type_with_handle.getAllTypesWithHandlesArrayList(arena, all_types);
const entry_ty = Type{ .data = entry.type_data, .is_type_val = ty.is_type_val };
try entry_ty.getAllTypesWithHandlesArrayList(arena, all_types);
}
},
else => try all_types.append(arena, ty),
Expand Down Expand Up @@ -2512,7 +2532,8 @@ pub const Type = struct {
.either => |entries| {
// TODO: Return all options instead of first valid one
for (entries) |entry| {
if (try entry.type_with_handle.lookupSymbol(analyser, symbol)) |decl| {
const entry_ty = Type{ .data = entry.type_data, .is_type_val = self.is_type_val };
if (try entry_ty.lookupSymbol(analyser, symbol)) |decl| {
return decl;
}
}
Expand Down Expand Up @@ -3527,7 +3548,7 @@ pub const DeclWithHandle = struct {
// TODO: Set `workspace` to true; current problems
// - we gather dependencies, not dependents

var possible = std.ArrayListUnmanaged(Type.EitherEntry){};
var possible = std.ArrayListUnmanaged(Type.TypeWithDescriptor){};

for (refs.items) |ref| {
const handle = analyser.store.getOrLoadHandle(ref.uri).?;
Expand Down Expand Up @@ -3560,7 +3581,7 @@ pub const DeclWithHandle = struct {

const loc = offsets.tokenToPosition(tree, main_tokens[call.ast.params[real_param_idx]], .@"utf-8");
try possible.append(analyser.arena.allocator(), .{
.type_with_handle = ty,
.type = ty,
.descriptor = try std.fmt.allocPrint(analyser.arena.allocator(), "{s}:{d}:{d}", .{ handle.uri, loc.line + 1, loc.character + 1 }),
});
}
Expand Down Expand Up @@ -3772,7 +3793,7 @@ fn iterateUsingnamespaceContainerSymbols(
},
.either => |entries| {
for (entries) |entry| {
switch (entry.type_with_handle.data) {
switch (entry.type_data) {
.other => |expr| {
try analyser.iterateSymbolsContainer(
expr,
Expand Down
8 changes: 5 additions & 3 deletions src/features/completions.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,11 @@ fn typeToCompletion(
analyser.ip,
payload.index,
),
.either => |bruh| {
for (bruh) |a|
try typeToCompletion(server, analyser, arena, list, a.type_with_handle, orig_handle, a.descriptor);
.either => |either_entries| {
for (either_entries) |entry| {
const entry_ty = Analyser.Type{ .data = entry.type_data, .is_type_val = ty.is_type_val };
try typeToCompletion(server, analyser, arena, list, entry_ty, orig_handle, entry.descriptor);
}
},
.error_union,
.union_tag,
Expand Down
22 changes: 18 additions & 4 deletions tests/lsp_features/completion.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2170,28 +2170,42 @@ test "either" {
\\ fn alpha() void {}
\\};
\\const Beta = struct {
\\ fn beta() void {}
\\ fn beta(_: @This()) void {}
\\};
\\const foo: if (undefined) Alpha else Beta = undefined;
\\const bar = foo.<cursor>
, &.{
.{ .label = "alpha", .kind = .Function, .detail = "fn () void" },
.{ .label = "beta", .kind = .Function, .detail = "fn () void" },
.{ .label = "beta", .kind = .Method, .detail = "fn (_: @This()) void" },
});
try testCompletion(
\\const Alpha = struct {
\\ fn alpha() void {}
\\};
\\const Beta = struct {
\\ fn beta() void {}
\\ fn beta(_: @This()) void {}
\\};
\\const alpha: Alpha = undefined;
\\const beta: Beta = undefined;
\\const gamma = if (undefined) alpha else beta;
\\const foo = gamma.<cursor>
, &.{
.{ .label = "alpha", .kind = .Function, .detail = "fn () void" },
.{ .label = "beta", .kind = .Function, .detail = "fn () void" },
.{ .label = "beta", .kind = .Method, .detail = "fn (_: @This()) void" },
});

try testCompletion(
\\const Alpha = struct {
\\ fn alpha() void {}
\\};
\\const Beta = struct {
\\ fn beta(_: @This()) void {}
\\};
\\const T = if (undefined) Alpha else Beta;
\\const bar = T.<cursor>
, &.{
.{ .label = "alpha", .kind = .Function, .detail = "fn () void" },
.{ .label = "beta", .kind = .Function, .detail = "fn (_: @This()) void" },
});
}

Expand Down

0 comments on commit 2720509

Please sign in to comment.