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

[WIP/NOTREADY] WIP json metadata #824

Closed
wants to merge 2 commits into from
Closed

[WIP/NOTREADY] WIP json metadata #824

wants to merge 2 commits into from

Conversation

jacobdufault
Copy link
Contributor

Code is not ready yet, I'll leave some comments where I have questions. I'd appreciate any feedback.

switch (decl_table_entry->value->id) {
case TldIdVar:
{
auto *var = (TldVar *)decl_table_entry->value;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like var is not fully populated here, ie, var->var is always null. Any idea why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like this might be because the variable is not resolved (ie, the TldResolution value is unresolved). It looks like resolution happens in codegen_build, which the JSON driver in main.cpp is calling - any idea why the code is not being resolved? Here is how I'm testing:

const Aaaaaaaa = struct {
  x: u8
};

pub fn main() void {
  var a = Aaaaaaaa { .x = 3 };
  const std = @import("std");
  std.debug.warn("{}\n", a.x);
}

Copy link
Member

Choose a reason for hiding this comment

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

I pushed changes to your branch merging conflicts with master. I tried testing the code with this file:

pub fn main() void {
}

and the command: zig json test.zig

the result was an empty array: [ ].

what have you been using to test?

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I didn't take into account your comment above. I tried with

const Aaaaaaaa = struct {
  x: u8
};

pub fn main() void {
  var a = Aaaaaaaa { .x = 3 };
  const std = @import("std");
  std.debug.warn("{}\n", a.x);
}

but zig json still gives an empty output. is that what you've been getting so far?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a version that I rebased onto master and tested. Can you try again? Maybe there was an issue with the merge.

I'm running like this

zig json hello.zig

where hello.zig is the file above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found the issue (I hard-coded the path to restrict output) - it should work now, though everything is still unresolved.

Copy link
Member

Choose a reason for hiding this comment

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

Try adding this to your test file:

export fn foo() void {
    var a: Aaaaaaaa = undefined;
}

you can see that foo as well as Aaaaaaa are now resolved.

looks like the code in build-exe that detects we should add std/special/bootstrap.zig to the compilation isn't triggering when you do zig json.

    if (!g->is_test_build && g->zig_target.os != OsFreestanding &&
        !g->have_c_main && !g->have_winmain && !g->have_winmain_crt_startup &&
        ((g->have_pub_main && g->out_type == OutTypeObj) || g->out_type == OutTypeExe))
    {
        g->bootstrap_import = add_special_code(g, create_bootstrap_pkg(g, g->root_package), "bootstrap.zig");
    }

I bet it's the out_type

            } else if (strcmp(arg, "json") == 0) {
                cmd = CmdJson;
                out_type = OutTypeJson;

I think that instead of trying to make zig json act like zig build-exe, we should modify the lazy analysis code to check every top level declaration when out_type == OutTypeJson. We will do something similar to this for autogenerated documentation.

Copy link
Member

Choose a reason for hiding this comment

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

Where you would add code to check all top level decls:

analyze.cpp:3066 in
static void add_top_level_decl(CodeGen *g, ScopeDecls *decls_scope, Tld *tld)

    if (is_export) {
        g->resolve_queue.append(tld);

move that to outside the if.

@andrewrk
Copy link
Member

The overall structure looks good. I'll play around with this in a little bit and help you with some of those details.

zig_unreachable();
}

static const char *type_table_entry_id_str(TypeTableEntryId id) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you maybe want to use

const char *type_id_name(TypeTableEntryId id);

instead of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@andrewrk
Copy link
Member

I'm excited about this progress. Re-open when you want some more guidance or review.

@andrewrk andrewrk closed this Mar 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants