Context's .container deemed useless #53

New Issue

deavmi · 2022-12-16T14:20:22Z

deavmi commented

2022-12-16 14:20:22 +00:00

What is `Context`?

The Context is passed around to indicate InitScope of an item and the container of such a thing. However, many parser nodes have a parentOf field, so this part of Context can actually be removed and is not needed.

I believe the reason it probably was added was I realised that things like Expression in the parser had no parent information and then I added this but everything then started using Context. What would be a good fix would be to remove the container field from Context and switch up dependency to be able to work with just that as it, literally copies it across.

For example, when we have a VariableDeclaration, something the parser could easily (and in this case probably already does) set the parent of, we call generalPass(module, context):

Context context = new Context(modulle, InitScope.STATIC);
DNode moduleDNode = generalPass(modulle, context);

THis could be retrived, for the variable declaration itself, by the first argument of generalPass which is a container.

We need to make sure everything is parented first in parser before such a switch can be made.

Original discussion

Notes

_ What was the reason for a Context object? Othee than for me to store InitScope

Like all entities have a Container which is where they live, and entity.setContext(context)
really just contains that copy of the Container (an object reference).
I think it actually comes from having not used parentToContainer in situations like
parseExpression() where we needed to localise like where it could be used.

Obviousl InitScope info was needed but we didn't like have parentOF() set for things
like expressions, hence it was added as an after thought.

We really could have done that, and passed Context only really as for
stuff like InitScopre. It shouldn't be a huge work around but I will do it later.

Thankfully we only use Context wherever we go in the dependency generator.

## What is `Context`? The `Context` is passed around to indicate InitScope of an item and the container of such a thing. However, many parser nodes have a `parentOf` field, so this part of Context can actually be removed and is not needed. I believe the reason it probably was added was I realised that things like `Expression` in the parser had no parent information and then I added this **but** everything then started using `Context`. What would be a good fix would be to remove the `container` field from `Context` and switch up dependency to be able to work with just that as it, literally copies it across. For example, when we have a VariableDeclaration, something the parser could easily (and in this case probably already does) set the parent of, we call `generalPass(module, context)`: ```d Context context = new Context(modulle, InitScope.STATIC); DNode moduleDNode = generalPass(modulle, context); ``` THis could be retrived, for the variable declaration itself, by the first argument of `generalPass` which **is** a container. We need to make sure everything is parented first in parser before such a switch can be made. ## Original discussion Notes _ What was the reason for a Context object? Othee than for me to store InitScope - Like all entities have a `Container` which is where they live, and `entity.setContext(context)` really just contains that copy of the Container (an object reference). - I think it actually comes from having not used `parentToContainer` in situations like `parseExpression()` where we needed to localise like where it could be used. Obviousl InitScope info was needed but we didn't like have `parentOF()` set for things like expressions, hence it was added as an after thought. We really could have done that, and passed Context only really as for stuff like InitScopre. It shouldn't be a huge work around but I will do it later. Thankfully we only use Context wherever we go in the dependency generator.

deavmi added the

dependency

parser

qol

labels 2022-12-16 14:20:31 +00:00

deavmi self-assigned this 2022-12-16 14:20:37 +00:00

deavmi referenced this issue

2022-12-16 14:23:19 +00:00

Context for VariableExpression and Variable missing #9

deavmi referenced this issue

2022-12-16 16:43:44 +00:00

VariableExpression in function referring to global not found #54

deavmi added the

question

label 2022-12-19 09:43:28 +00:00

deavmi added this to the Dependency tree, type-checking and codegen project 2022-12-19 09:43:33 +00:00

deavmi referenced this issue

2023-05-29 15:15:01 +01:00

🧠 Feature: Meta-programming engine #116

deavmi commented

2023-05-29 15:15:32 +01:00

Bump

deavmi referenced this issue

2023-05-29 15:33:36 +01:00

🧠 Feature: Meta-programming engine #116

deavmi added the due date 2024-05-31

2024-05-11 10:24:40 +01:00

deavmi started working 2024-05-11 10:24:44 +01:00

deavmi commented

2024-05-11 10:26:21 +01:00

What is Context?

The Context is passed around to indicate InitScope of an item and the container of such a thing. However, many parser nodes have a parentOf field, so this part of Context can actually be removed and is not needed.

I believe the reason it probably was added was I realised that things like Expression in the parser had no parent information and then I added this but everything then started using Context. What would be a good fix would be to remove the container field from Context and switch up dependency to be able to work with just that as it, literally copies it across.

For example, when we have a VariableDeclaration, something the parser could easily (and in this case probably already does) set the parent of, we call generalPass(module, context):
Context context = new Context(modulle, InitScope.STATIC);
DNode moduleDNode = generalPass(modulle, context);
THis could be retrived, for the variable declaration itself, by the first argument of generalPass which is a container.

We need to make sure everything is parented first in parser before such a switch can be made.

Original discussion

Notes

_ What was the reason for a Context object? Othee than for me to store InitScope

Like all entities have a Container which is where they live, and entity.setContext(context)
really just contains that copy of the Container (an object reference).

I think it actually comes from having not used parentToContainer in situations like
parseExpression() where we needed to localise like where it could be used.

Obviousl InitScope info was needed but we didn't like have parentOF() set for things
like expressions, hence it was added as an after thought.

We really could have done that, and passed Context only really as for
stuff like InitScopre. It shouldn't be a huge work around but I will do it later.

Thankfully we only use Context wherever we go in the dependency generator.

Yeah so update:

I think it actually comes from having not used parentToContainer in situations like
parseExpression() where we needed to localise like where it could be used.

This won't be a concern anymore really with the advent of the recursive painting parentor. It is basically that the parentToContainer(...) was updated to fill in the parentOf() for all entities with special case handling as well.

Verdict: We can therefore deprecate the Container (and perhaps the Context, at least for now). But the container aspect can defs go. We have ways to resolve stuff now upwards based on the fact that everything should be parented via recursive painting, and if not, it must be implemented then.

> ## What is `Context`? > > The `Context` is passed around to indicate InitScope of an item and the container of such a thing. However, many parser nodes have a `parentOf` field, so this part of Context can actually be removed and is not needed. > > I believe the reason it probably was added was I realised that things like `Expression` in the parser had no parent information and then I added this **but** everything then started using `Context`. What would be a good fix would be to remove the `container` field from `Context` and switch up dependency to be able to work with just that as it, literally copies it across. > > For example, when we have a VariableDeclaration, something the parser could easily (and in this case probably already does) set the parent of, we call `generalPass(module, context)`: > > ```d > Context context = new Context(modulle, InitScope.STATIC); > DNode moduleDNode = generalPass(modulle, context); > ``` > > THis could be retrived, for the variable declaration itself, by the first argument of `generalPass` which **is** a container. > > We need to make sure everything is parented first in parser before such a switch can be made. > > ## Original discussion > > Notes > > _ What was the reason for a Context object? Othee than for me to store InitScope > - Like all entities have a `Container` which is where they live, and `entity.setContext(context)` > really just contains that copy of the Container (an object reference). > - I think it actually comes from having not used `parentToContainer` in situations like > `parseExpression()` where we needed to localise like where it could be used. > > Obviousl InitScope info was needed but we didn't like have `parentOF()` set for things > like expressions, hence it was added as an after thought. > > We really could have done that, and passed Context only really as for > stuff like InitScopre. It shouldn't be a huge work around but I will do it later. > > Thankfully we only use Context wherever we go in the dependency generator. Yeah so update: > I think it actually comes from having not used parentToContainer in situations like parseExpression() where we needed to localise like where it could be used. This won't be a concern anymore really with the advent of the recursive painting parentor. It is basically that the `parentToContainer(...)` was updated to fill in the `parentOf()` for all entities with special case handling as well. --- **Verdict:** We can therefore deprecate the `Container` (and perhaps the `Context`, at least for now). But the container aspect can defs go. We have ways to resolve stuff now upwards based on the fact that everything _should_ be parented via recursive painting, and if not, it must be implemented then.

deavmi stopped working 2024-05-11 10:27:32 +01:00

2 minutes 48 seconds

deavmi started working 2024-05-13 18:42:54 +01:00

deavmi added this to the Performance and Clarity milestone 2024-05-13 18:42:57 +01:00

deavmi modified the milestone from Performance and Clarity to Clean ups

2024-05-13 18:43:01 +01:00

deavmi commented

2024-05-13 18:43:17 +01:00

Okay, so it is deemed useless for the dependency generation - yes. But not for Instructions

Okay, so it is deemed useless for the dependency generation - **yes**. But not for `Instruction`s

deavmi stopped working 2024-05-13 18:51:54 +01:00

8 minutes 59 seconds