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

Fix several issues related to schema loading in declarative #5565

Merged
merged 6 commits into from
May 31, 2023

Conversation

msullivan
Copy link
Member

The main change is a new schema where if we can't figure out the type
of an expression in a path, create weak dependencies to all pointers
with the same name.

This doesn't always work but in practice it will do very well.

I have some ideas for how we could incorporate correct typechecking
into schema loading, but in the mean time this probably solves 90% of
the problem at least.

In this PR there is also a commit that adds a system for categorizing
objects so that things without expressions will go first, and things
with expressions will be prioritized based on whether they can be
referred to.
I wound up reverting it because it isn't needed to fix the issues that
weak pointers fix, and other than those, I think it mostly would work
around bugs that could be fixed in straightforward ways.

Fixes #4683. Fixes #5163.

This reverts commit 6861cf05761f9f1d73416f23ab6ab65396d44e21.
If we can't figure out the type of an expression in a path,
create weak dependencies to all pointers with the same name.

This doesn't *always* work but in practice it will do pretty well.

Fixes #4683. Fixes #5163.
Copy link
Contributor

@aljazerzen aljazerzen left a comment

Choose a reason for hiding this comment

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

I've a few conceptual questions, but let's talk about that on the call today.

@@ -461,6 +469,8 @@ def _fork_context(ctx: TracerContext) -> TracerContext:
schema=ctx.schema,
module=ctx.module,
objects=dict(ctx.objects),
# XXX?
Copy link
Member

Choose a reason for hiding this comment

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

Can you elaborate the XXX or remove it?

@msullivan msullivan merged commit f014ec0 into master May 31, 2023
@msullivan msullivan deleted the declarative branch May 31, 2023 19:34
msullivan added a commit that referenced this pull request Jun 1, 2023
The main change is a new schema where if we can't figure out the type
of an expression in a path, create weak dependencies to all pointers
with the same name.

This doesn't always work but in practice it will do very well.

I have some ideas for how we could incorporate correct typechecking
into schema loading, but in the mean time this probably solves 90% of
the problem at least.

Fixes #4683. Fixes #5163.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants