VariableExpression in function referring to global not found #54

New Issue

deavmi · 2022-12-16T15:56:30Z

deavmi commented

2022-12-16 15:56:30 +00:00

Given the following T program:

int k = 21;

int banana(int arg1)
{
    int h = 64;

    k=1+h+apple(1, apple(2, 3))+k;   
}

Lookups are working for assignment but not the variable expression itself of k (right at the end).

Given the following T program: ```d int k = 21; int banana(int arg1) { int h = 64; k=1+h+apple(1, apple(2, 3))+k; } ``` Lookups are working for assignment but not the variable expression itself of `k` (right at the end).

deavmi added the

dependency

label 2022-12-16 15:56:30 +00:00

deavmi self-assigned this 2022-12-16 15:56:30 +00:00

deavmi added this to the Dependency tree, type-checking and codegen project 2022-12-16 15:56:30 +00:00

deavmi added reference vardec_varass_dependency

2022-12-16 16:10:47 +00:00

deavmi started working 2022-12-16 16:15:22 +00:00

deavmi commented

2022-12-16 16:16:27 +00:00

Technically realted to the same code handling issue #10

deavmi commented

2022-12-16 16:17:32 +00:00

Solution 1

I think I see why this is returning null is become we use:

/* Resolve the Entity */
Entity namedEntity = tc.getResolver().resolveWithin(context.getContainer(), nearestName);

Which uses resolveWithin however, we want resolve best in such a case as it looks up and outside of the function container and can then reach the global variable, k.

Secondly, this is fine for local lookups WITHIN the function container as resolveBest does a within check and if not found then looks up.

Results

Looks like it works (now stops later in emit) but we are reaching the variable it seems:

[INFO] transform(): const(compiler.codegen.instruction.Instruction)
[INFO] type: FetchValueVar
[INFO] AssertFail Check: Variable (Ident: k, Type: int)
core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(126): Assertion failure

## Solution 1 I think I see why this is returning `null` is become we use: ```d /* Resolve the Entity */ Entity namedEntity = tc.getResolver().resolveWithin(context.getContainer(), nearestName); ``` Which uses `resolveWithin` however, we want resolve best in such a case as it looks up and outside of the function container and can then reach the global variable, `k`. **Secondly**, this is fine for local lookups **WITHIN** the function container as resolveBest does a within check and if not found _then_ looks up. ## Results Looks like it works (now stops later in emit) but we are reaching the variable it seems: ``` [INFO] transform(): const(compiler.codegen.instruction.Instruction) [INFO] type: FetchValueVar [INFO] AssertFail Check: Variable (Ident: k, Type: int) core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(126): Assertion failure ```

👍 1

deavmi commented

2022-12-16 16:20:18 +00:00

Resolution 126 assertion error

[INFO] transform(): const(compiler.codegen.instruction.Instruction)
[INFO] type: FetchValueVar
[INFO] AssertFail Check: Variable (Ident: k, Type: int)
core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(126): Assertion failure

I believe this happens when entity has no parent.

## Resolution 126 assertion error ``` [INFO] transform(): const(compiler.codegen.instruction.Instruction) [INFO] type: FetchValueVar [INFO] AssertFail Check: Variable (Ident: k, Type: int) core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(126): Assertion failure ``` I believe this happens when entity has no parent.

deavmi commented

2022-12-16 16:38:07 +00:00

Function is not paranted?

[INFO] transform(): const(compiler.codegen.instruction.Instruction)
[INFO] type: FetchValueVar
[INFO] simple_function_decls.k
[INFO] Yo and me babe, what about it?: 96
[INFO] c: Function (Name: banana, ReturnType: int, Args: int)
[INFO] currentEntity: Variable (Ident: k, Type: int)
[INFO] AssertFail Check: Variable (Ident: k, Type: int)
[INFO] c: Function (Name: banana, ReturnType: int, Args: int)
[INFO] currentEntity: 97
core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(128): Assertion failure

Is function not parented?

So parseFuntion() is called by parseTypedDeclaration() which is called by parseName(). Now this seems like an old parser bug or lack thereof. It seems that we never set the parenting here (this is tagentially related to our use of Context actually and issue #53, so this makes sense why we would only see it now).

We are looking at the call of parseName() in parseBody(). An easy fix is in parseFuncDef().

## Function is not paranted? ``` [INFO] transform(): const(compiler.codegen.instruction.Instruction) [INFO] type: FetchValueVar [INFO] simple_function_decls.k [INFO] Yo and me babe, what about it?: 96 [INFO] c: Function (Name: banana, ReturnType: int, Args: int) [INFO] currentEntity: Variable (Ident: k, Type: int) [INFO] AssertFail Check: Variable (Ident: k, Type: int) [INFO] c: Function (Name: banana, ReturnType: int, Args: int) [INFO] currentEntity: 97 core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(128): Assertion failure ``` Is function not parented? --- So `parseFuntion()` is called by `parseTypedDeclaration()` which is called by `parseName()`. Now this seems like an old parser bug or lack thereof. It seems that we never set the parenting here (this is tagentially related to our use of Context actually and issue #53, so this makes sense why we would only see it now). We are looking at the call of `parseName()` in `parseBody()`. An easy fix is in `parseFuncDef()`.

deavmi commented

2022-12-16 16:51:27 +00:00

What we need

Use resolveBest() and not resolveWithin() in dependency/core.d

In parseBody() we will call parseName() which will have our function definition. So we can traverse downwards, however we haven't loop through statements ~= parseName() (the statements) array) and PARENTED them to what ever called parseBody(). parseFUncDef has:

/* Parse the body (and it leaves ONLY when it gets the correct symbol, no expect needed) */
statements = parseBody();

And returns these statements as:

bruh.bodyStatements = statements;
bruh.args = argumentList;

return bruh;

Now, inside parseTypedDeclaration(), we call parseFuncDef(), which then we pass said statements into new Function() call. However, what calls this, is several mutaully recursive calls.

So I think we should check (and add parentTo calls):

parseName() in public Module parse()
- This one immediately should fix our problem

Wait.... it looks like we do do this?!?!?!:

/* Parent each Statement to the container (the module) */
parentToContainer(modulle, modulle.getStatements());

## What we need - [x] Use `resolveBest()` and not `resolveWithin()` in `dependency/core.d` In `parseBody()` we will call `parseName()` which will have our function definition. So we can traverse downwards, however we haven't loop through `statements ~= parseName()` (the `statements`) array) and **PARENTED** them to what ever called `parseBody()`. `parseFUncDef` has: ```d /* Parse the body (and it leaves ONLY when it gets the correct symbol, no expect needed) */ statements = parseBody(); ``` And returns these statements as: ```d bruh.bodyStatements = statements; bruh.args = argumentList; return bruh; ``` --- **Now**, inside `parseTypedDeclaration()`, we call `parseFuncDef()`, which then we pass said statements into `new Function()` call. However, what calls this, is several mutaully recursive calls. So I think we should check (and add `parentTo` calls): - [ ] `parseName()` in `public Module parse()` * This one immediately should fix our problem Wait.... it looks like we **do** do this?!?!?!: ```d /* Parent each Statement to the container (the module) */ parentToContainer(modulle, modulle.getStatements()); ```

deavmi commented

2022-12-16 17:11:35 +00:00

It's `generateName()`

//TODO: THis is giving me kak (see issue #54), it's generating name but trying to do it for the given container, relative to it
//TODO: We might need a version of generateName that is like generatenamebest (currently it acts like generatename, within)
string typedEntityVariableName = context.tc.getResolver().generateName(context.getContainer(), typedEntityVariable);

We need a generateNameWithin() and generateNameBest() which can climb for us out of the given container - this is the problem.

Slightly tangential

The crashing isDescdant is correct. Technically we should prpobabnly remove the assert in isDescendantr() and place it in this (which is where it fails descedancy):

/* If not */
else
{
	//TODOL technically an assert should be here and the one in isDescdant removed
    return null;
}

## It's `generateName()` ```d //TODO: THis is giving me kak (see issue #54), it's generating name but trying to do it for the given container, relative to it //TODO: We might need a version of generateName that is like generatenamebest (currently it acts like generatename, within) string typedEntityVariableName = context.tc.getResolver().generateName(context.getContainer(), typedEntityVariable); ``` We need a `generateNameWithin()` and `generateNameBest()` which can climb for us out of the given container - this is the problem. --- ### Slightly tangential The crashing `isDescdant` is correct. Technically we should prpobabnly remove the assert in `isDescendantr()` and place it in this (which is where it fails descedancy): ```d /* If not */ else { //TODOL technically an assert should be here and the one in isDescdant removed return null; } ```

deavmi commented

2022-12-16 17:21:50 +00:00

It's generateName()
//TODO: THis is giving me kak (see issue #54), it's generating name but trying to do it for the given container, relative to it
//TODO: We might need a version of generateName that is like generatenamebest (currently it acts like generatename, within)
string typedEntityVariableName = context.tc.getResolver().generateName(context.getContainer(), typedEntityVariable);
We need a generateNameWithin() and generateNameBest() which can climb for us out of the given container - this is the problem.

Slightly tangential

The crashing isDescdant is correct. Technically we should prpobabnly remove the assert in isDescendantr() and place it in this (which is where it fails descedancy):
/* If not */
else
{
	//TODOL technically an assert should be here and the one in isDescdant removed
    return null;
}

This actually is better than that ack we did for:

else if(cast(VariableAssignmentStdAlone)entity)
{
	VariableAssignmentStdAlone vAsStdAl = cast(VariableAssignmentStdAlone)entity;
    
    ...
    
    Variable variable = cast(Variable)tc.getResolver().resolveBest(c, vAsStdAl.getVariableName());
    assert(variable);

	/* Set the context of the assignment to the Context of the Variable being assigned to */
    vAsStdAl.setContext(variable.getContext());

I really don't like this hack and I think it is wrong, the Context should actually be set as the Context coming into generalPass() (as it was before). We should revert to that ASAP and then implement the generate name methods above (see issue #56)

> ## It's `generateName()` > > ```d > //TODO: THis is giving me kak (see issue #54), it's generating name but trying to do it for the given container, relative to it > //TODO: We might need a version of generateName that is like generatenamebest (currently it acts like generatename, within) > string typedEntityVariableName = context.tc.getResolver().generateName(context.getContainer(), typedEntityVariable); > ``` > > We need a `generateNameWithin()` and `generateNameBest()` which can climb for us out of the given container - this is the problem. > > > --- > > ### Slightly tangential > > The crashing `isDescdant` is correct. Technically we should prpobabnly remove the assert in `isDescendantr()` and place it in this (which is where it fails descedancy): > > ```d > /* If not */ > else > { > //TODOL technically an assert should be here and the one in isDescdant removed > return null; > } > ``` This actually is better than that ack we did for: ```d else if(cast(VariableAssignmentStdAlone)entity) { VariableAssignmentStdAlone vAsStdAl = cast(VariableAssignmentStdAlone)entity; ... Variable variable = cast(Variable)tc.getResolver().resolveBest(c, vAsStdAl.getVariableName()); assert(variable); /* Set the context of the assignment to the Context of the Variable being assigned to */ vAsStdAl.setContext(variable.getContext()); ``` I **really** don't like this hack and I think it is wrong, the `Context` should actually be set as the Context coming into `generalPass()` (as it was before). We should revert to that ASAP and then implement the generate name methods above (see issue #56)

deavmi added a new dependency 2022-12-16 17:25:09 +00:00

#58 Switch to `generateBest()`

deavmi removed a dependency 2022-12-16 17:25:19 +00:00

#58 Switch to `generateBest()`

deavmi added a new dependency 2022-12-16 17:25:51 +00:00

#58 Switch to `generateBest()`

deavmi commented

2022-12-16 17:26:21 +00:00

We can fix this when we have generateNameBest(), blocked by #58 and #56

We can fix this when we have `generateNameBest()`, blocked by #58 and #56

deavmi stopped working 2022-12-16 20:55:59 +00:00

4 hours 40 minutes