Assertion failure in `identifierTokenToNameSlice`

[paperdave]

Zig Version

0.14.0-dev.1511+54b668f8a

ZLS Version

0.14.0-dev.147+dd78968

Client / Code Editor / Extensions

No response

Steps to Reproduce and Observed Behavior

pub const hello = enum {
    @"wtf",
    @hello, // place your cursor in this builtin identifier
};

Expected Behavior

no crash. i think the mistake is simply that the token can be a builtin identfier in addition to a regular identifer. though maybe other tokens can exist here

Relevant log output

thread 2000114 panic: reached unreachable code
/Users/dave/zig/0.14.0-dev.1511+54b668f8a/files/lib/std/debug.zig:401:14: 0x10465cb47 in assert (zls)
    if (!ok) unreachable; // assertion failure
             ^
/Users/dave/code/zls/src/offsets.zig:278:21: 0x10497db0f in identifierTokenToNameLoc (zls)
    std.debug.assert(tree.tokens.items(.tag)[identifier_token] == .identifier);
                    ^
/Users/dave/code/zls/src/offsets.zig:283:60: 0x1048e0547 in identifierTokenToNameSlice (zls)
    return locToSlice(tree.source, identifierTokenToNameLoc(tree, identifier_token));
                                                           ^
/Users/dave/code/zls/src/DocumentScope.zig:394:60: 0x104a5d13f in pushDeclaration (zls)
            const name = offsets.identifierTokenToNameSlice(pushed.context.tree, identifier_token);
                                                           ^
/Users/dave/code/zls/src/DocumentScope.zig:866:42: 0x1049a02c3 in walkContainerDecl (zls)
                try scope.pushDeclaration(main_token, .{ .ast_node = decl }, .field);
                                         ^
/Users/dave/code/zls/src/DocumentScope.zig:631:29: 0x104a5ceaf in walkNode (zls)
        => walkContainerDecl(context, tree, node_idx),
                            ^
/Users/dave/code/zls/src/ast.zig:1426:28: 0x104c48a33 in inner (zls)
            return callback(@as(*const @TypeOf(context), @alignCast(@ptrCast(ctx))).*, t, n);
                           ^
/Users/dave/code/zls/src/ast.zig:1592:25: 0x1048e98d3 in iterateChildrenTypeErased (zls)
            try callback(context, tree, var_decl.init_node);
                        ^
/Users/dave/code/zls/src/ast.zig:1429:34: 0x104bd5b5f in iterateChildren__anon_113737 (zls)
    if (iterateChildrenTypeErased(tree, node, @ptrCast(&context), &ctx.inner)) |_| {
                                 ^
/Users/dave/code/zls/src/DocumentScope.zig:1342:28: 0x104b304f7 in walkOtherNode (zls)
    try ast.iterateChildren(tree, node_idx, context, error{OutOfMemory}, walkNode);
                           ^
/Users/dave/code/zls/src/DocumentScope.zig:790:25: 0x104a5cfc7 in walkNode (zls)
        => walkOtherNode(context, tree, node_idx),
                        ^
/Users/dave/code/zls/src/DocumentScope.zig:846:21: 0x10499ff83 in walkContainerDecl (zls)
        try walkNode(context, tree, decl);
                    ^
/Users/dave/code/zls/src/DocumentScope.zig:556:26: 0x1048f8d2b in init (zls)
    try walkContainerDecl(&context, tree, 0);
                         ^
/Users/dave/code/zls/src/DocumentStore.zig:398:60: 0x10487074f in getDocumentScopeCold (zls)
                var document_scope = try DocumentScope.init(self.impl.allocator, self.tree);
                                                           ^
/Users/dave/code/zls/src/DocumentStore.zig:265:45: 0x1047f2c17 in getDocumentScope (zls)
        return try self.getDocumentScopeCold();
                                            ^
/Users/dave/code/zls/src/analysis.zig:1696:63: 0x104901637 in resolveTypeOfNodeUncached (zls)
            const document_scope = try handle.getDocumentScope();
                                                              ^
/Users/dave/code/zls/src/analysis.zig:1361:54: 0x104878ed3 in resolveTypeOfNodeInternal (zls)
    const ty = try analyser.resolveTypeOfNodeUncached(node_handle);
                                                     ^
/Users/dave/code/zls/src/analysis.zig:1401:62: 0x104904fdb in resolveTypeOfNodeUncached (zls)
                return try analyser.resolveTypeOfNodeInternal(value) orelse break :blk;
                                                             ^
/Users/dave/code/zls/src/analysis.zig:1361:54: 0x104878ed3 in resolveTypeOfNodeInternal (zls)
    const ty = try analyser.resolveTypeOfNodeUncached(node_handle);
                                                     ^
/Users/dave/code/zls/src/analysis.zig:1347:46: 0x10487732f in resolveTypeOfNode (zls)
    return analyser.resolveTypeOfNodeInternal(node_handle);
                                             ^
/Users/dave/code/zls/src/features/inlay_hints.zig:330:65: 0x1048e70a3 in typeStrOfNode (zls)
    const resolved_type = try builder.analyser.resolveTypeOfNode(.{ .handle = builder.handle, .node = node }) orelse return null;
                                                                ^
/Users/dave/code/zls/src/features/inlay_hints.zig:460:34: 0x1048646bf in writeNodeInlayHint (zls)
                try typeStrOfNode(builder, node) orelse return,
                                 ^
/Users/dave/code/zls/src/ast.zig:1820:25: 0x1048eb033 in recursive_callback (zls)
            try callback(@as(*const @TypeOf(context), @alignCast(@ptrCast(ctx))).*, ast, child_node);
                        ^
/Users/dave/code/zls/src/ast.zig:1578:29: 0x1048e968f in iterateChildrenTypeErased (zls)
                try callback(context, tree, child);
                            ^
/Users/dave/code/zls/src/ast.zig:1825:34: 0x1048654c7 in iterateChildrenRecursive__anon_56352 (zls)
    if (iterateChildrenTypeErased(tree, node, @ptrCast(&context), RecursiveContext.recursive_callback)) |_| {
                                 ^
/Users/dave/code/zls/src/features/inlay_hints.zig:598:41: 0x1047e7f6f in writeRangeInlayHint (zls)
        try ast.iterateChildrenRecursive(handle.tree, child, &builder, error{OutOfMemory}, writeNodeInlayHint);
                                        ^
/Users/dave/code/zls/src/Server.zig:1602:47: 0x104771b3b in inlayHintHandler (zls)
    return try inlay_hints.writeRangeInlayHint(
                                              ^
/Users/dave/code/zls/src/Server.zig:1878:66: 0x104709083 in sendRequestSync__anon_21616 (zls)
        .@"textDocument/inlayHint" => try server.inlayHintHandler(arena, params),
                                                                 ^
/Users/dave/code/zls/src/Server.zig:1953:58: 0x1046a3687 in processMessage (zls)
                const result = try server.sendRequestSync(arena_allocator.allocator(), @tagName(method), params);
                                                         ^
/Users/dave/code/zls/src/Server.zig:1967:33: 0x10467b813 in processMessageReportError (zls)
    return server.processMessage(message) catch |err| {
                                ^
/Users/dave/code/zls/src/Server.zig:2005:62: 0x1046613e3 in processJob (zls)
            const response = server.processMessageReportError(parsed_message.value) orelse return;
                                                             ^
/Users/dave/zig/0.14.0-dev.1511+54b668f8a/files/lib/std/Thread/Pool.zig:229:39: 0x1046600d3 in runFn (zls)
            @call(.auto, func, closure.arguments);
                                      ^
/Users/dave/zig/0.14.0-dev.1511+54b668f8a/files/lib/std/Thread/Pool.zig:291:32: 0x1046f3963 in worker (zls)
            run_node.data.runFn(&run_node.data, id);
                               ^
/Users/dave/zig/0.14.0-dev.1511+54b668f8a/files/lib/std/Thread.zig:409:13: 0x104696e6f in callFn__anon_13028 (zls)
            @call(.auto, f, args);

[WillLillis]

Adding .builtin to the assertion gets rid of that panic, but causes another in the next call to identifierIndexToLoc, as there isn't an "@" in the expected location:

https://github.com/zigtools/zls/blob/dd78968d4c8deefd33addc2b1cc14f60d89ec1a9/src/offsets.zig#L213-L234

Removing the assertion from identifierIndexToLoc then causes other problems later on. I tried making identifierTokenToNameLoc return ?Loc and updating all of the call sites with an appropriate orelse return, orelse return null, or else continue, etc. throughout the rest of the project, but this ended up breaking some existing logic and causing a different assert to trigger later on here. Not sure what the right solution is, this is an interesting edge case.

[llogick]

@WillLillis A non disruptive fix could be as simple as https://github.com/llogick/zls/commit/8b86b32f42e080b7873edd4cb4d4b035f017d996

[WillLillis]

@WillLillis A non disruptive fix could be as simple as llogick@8b86b32

@llogick I guess that would do it, nice! Kinda mad I missed that now :laughing: