Alternative generateName behaviour for symbolLookup in DGen #49

New Issue

deavmi · 2022-12-15T10:47:52Z

deavmi commented

2022-12-15 10:47:52 +00:00

Currently

If you pass a Container c and Entity e to generateName(Container, Entity) but the entity is not contained by the given container (even if the container provided is contained byu the container that the Entity is contained by), then we get an assertion failure.

Currently we have an example code of:

int k = 22+j;

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

    k=1;   
}

Now rightfully so, k - the Variable has a Container which is the Module. So therefore the assertion will fail as we pass in a Function as a container.

I don't really want to change the behaviour of our resolver, so maybe it might be a good idea to come up with something else. Like, yeah?

## Currently If you pass a Container `c` and Entity `e` to `generateName(Container, Entity)` but the entity is not contained by the given container (even if the container provided is contained byu the container that the Entity is contained by), then we get an assertion failure. Currently we have an example code of: ```d int k = 22+j; int banana(int j) { int h = 64; k=1; } ``` Now rightfully so, `k` - the `Variable` has a `Container` which is the `Module`. So therefore the assertion will fail as we pass in a `Function` as a container. --- I don't really want to change the behaviour of our resolver, so maybe it might be a good idea to come up with something else. Like, yeah?

deavmi commented

2022-12-15 10:51:57 +00:00

Idea 1

What we could do perhaps, instead of chanigng things around could be to cache symbol lookups, as that is the entire point of this really, for name generation.

We already have (and working):

Entity lookups (needed for correct emit generation)

All we want:

Is generateName basically but to hash it, so perhaps we should cache it in the SymbolMapper, then we avoid things like this. So if it doesn't exist, then do the lookup with the generateName and all, but if it does exist then we should simply get a cached copy returned to us by SymbolMapper.

## Idea 1 What we could do perhaps, instead of chanigng things around could be to cache symbol lookups, as that is the entire point of this really, for name generation. We already have (and working): 1. Entity lookups (needed for correct emit generation) All we want: Is `generateName` basically but to hash it, so perhaps we should cache it in the SymbolMapper, then we avoid things like this. So if it doesn't exist, then do the lookup with the `generateName` and all, but if it **does** exist then we should simply get a cached copy returned to us by `SymbolMapper`.

👎 1

deavmi changed title from ~~Alternative generateName behaviour~~ to Alternative generateName behaviour for symbolLookup in DGen

2022-12-15 10:52:09 +00:00

deavmi added reference vardec_varass_dependency

2022-12-15 10:53:01 +00:00

deavmi added the

needsfix

resolution

emit

labels 2022-12-15 10:53:18 +00:00

deavmi added this to the Dependency tree, type-checking and codegen project 2022-12-15 10:53:21 +00:00

deavmi self-assigned this 2022-12-15 10:53:24 +00:00

deavmi commented

2022-12-15 11:32:25 +00:00

Idea 2

Better approach, we keep stuff as is in the DGen class to keep it simple and clean but in the VarStdAssignment we set the context there like this:

assert(tc.getResolver().resolveBest(c, vAsStdAl.getVariableName()));
gprintln("YEAST ENJOYER");

Variable variable = cast(Variable)tc.getResolver().resolveBest(c, vAsStdAl.getVariableName());
assert(variable);

vAsStdAl.setContext(variable.getContext());

Instead of this:

vAsStdAl.setContext(context);

Effectively using the context of the variable the standalone assignment refers to which should always be correct.

## Idea 2 Better approach, we keep stuff as is in the `DGen` class to keep it simple and clean **but** in the `VarStdAssignment` we set the context there like this: ```d assert(tc.getResolver().resolveBest(c, vAsStdAl.getVariableName())); gprintln("YEAST ENJOYER"); Variable variable = cast(Variable)tc.getResolver().resolveBest(c, vAsStdAl.getVariableName()); assert(variable); vAsStdAl.setContext(variable.getContext()); ``` Instead of this: ```d vAsStdAl.setContext(context); ``` --- Effectively using the context of the variable the standalone assignment refers to which should always be correct.

👍 1

deavmi commented

2022-12-16 12:50:42 +00:00

Going with idea 2