Function argument support #52

New Issue

deavmi · 2022-12-16T11:54:38Z

deavmi commented

2022-12-16 11:54:38 +00:00

Currently there is no support for the function arguments, they are not found in lookups. We need to make them known of and somehow declared but not in a way that emits them as stack variables WITHIN the body of the function (as they are not that).

deavmi self-assigned this 2022-12-16 11:56:09 +00:00

deavmi commented

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

Idea 1

We may need to somehow add it to the getStatements() function as the lookups on any class that inherits from Container would have such a method, as do the sub-class Function.

Now we would have to be careful because remember, the way instructions (at the end of the day) get emitted for the body of a function is that the dependency/core.d's generalPass() will call getStatements() on the Container-type Function. We, don't want to by mistake when we get to:

/**
* Variable declarations
*/
if(cast(Variable)entity)
{
	...
}

And then start adding to the fucntion's DNode a bunch of StaticVariableDeclaration objects. We should probably make some sort of either boolean flag of sub-type of Variable, that is then checked before a dnode addition is done. This will then avoid any declarations that would be generated by the typechecker/core.d when it runs through all the DNode's and produces Instructions.

We effectively just want them in getStatements() so that lookups work!

I think the sub-type option is nicer, we must then be sure to add that as a cast(VariableParameter)entity BEFORE the cast(Variable) in the check and just have something like this:

/**
*
*/
if(cast(VariableParameter)entity)
{
	continue;
}
/**
* Variable declarations
*/
else if(cast(Variable)entity)
{
	...
}

We could also do the check as the first in the cast(Variable)entity branch as well - might be a good idea for more concise code.

## Idea 1 We may need to somehow add it to the `getStatements()` function as the lookups on any class that inherits from `Container` would have such a method, as do the sub-class `Function`. Now we would have to be careful because remember, the way instructions (at the end of the day) get emitted for the body of a function is that the `dependency/core.d`'s `generalPass()` will call `getStatements()` on the `Container`-type `Function`. We, don't want to by mistake when we get to: ```d /** * Variable declarations */ if(cast(Variable)entity) { ... } ``` And then start adding to the fucntion's DNode a bunch of `StaticVariableDeclaration` objects. We should probably make some sort of either boolean flag of sub-type of `Variable`, that is then checked before a dnode addition is done. This will then avoid any declarations that would be generated by the `typechecker/core.d` when it runs through all the DNode's and produces Instructions. **We effectively just want them in `getStatements()` so that lookups work!** --- I think the sub-type option is nicer, we must then be sure to add that as a `cast(VariableParameter)entity` **BEFORE** the `cast(Variable)` in the check and just have something like this: ```d /** * */ if(cast(VariableParameter)entity) { continue; } /** * Variable declarations */ else if(cast(Variable)entity) { ... } ``` We could also do the check as the first in the `cast(Variable)entity` branch as well - might be a good idea for more concise code.

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

deavmi modified the project from Dependency tree, type-checking and codegen to Code emit

2022-12-16 12:09:33 +00:00

deavmi modified the project from Code emit to Dependency tree, type-checking and codegen

2022-12-16 12:09:36 +00:00

deavmi added the

typing

dependency

labels 2022-12-16 12:09:56 +00:00

deavmi added a new dependency 2022-12-16 12:59:32 +00:00

#42 Function definitions

deavmi referenced this issue

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

Function definitions #42

deavmi referenced this issue

2022-12-16 13:06:48 +00:00

Function definitions #42

deavmi added reference vardec_varass_dependency

2022-12-16 13:10:28 +00:00

deavmi started working 2022-12-16 13:11:54 +00:00

deavmi referenced this issue

2022-12-16 13:12:59 +00:00

Function definitions #42

deavmi changed reference from ~~vardec_varass_dependency~~ to function_parameters_fix

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

deavmi commented

2022-12-16 13:25:21 +00:00

Looks like we will need to look at the typechecking stuff, I am getting a checkContainerCollision() error it seems:

[INFO] AssertFail Check: Function (Name: apple, ReturnType: int, Args: arg1, arg2)
[INFO] AssertFail Check: Function (Name: apple, ReturnType: int, Args: arg1, arg2)
[INFO] Entity "simple_function_decls.apple.h" is allowed to be defined within container "simple_function_decls.apple"
core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(126): Assertion failure

Looks like we will need to look at the typechecking stuff, I am getting a `checkContainerCollision()` error it seems: ``` [INFO] AssertFail Check: Function (Name: apple, ReturnType: int, Args: arg1, arg2) [INFO] AssertFail Check: Function (Name: apple, ReturnType: int, Args: arg1, arg2) [INFO] Entity "simple_function_decls.apple.h" is allowed to be defined within container "simple_function_decls.apple" core.exception.AssertError@source/tlang/compiler/typecheck/resolution.d(126): Assertion failure ```

deavmi commented

2022-12-16 13:31:41 +00:00

I wonder, is it not related to maybe theese not having a container?

Line 126 is the following in the resolver:

assert(cast(Entity) currentEntity.parentOf());

And because these are run through by the typechecker in collision detection stage (because we have added these VariableParameter[] to the bodyStatements/getStatements(),therefore they will be checked and resolution too).

Note Resolution is NOT required but occurs as we have the lines(in the checkCollision() method):

string fullPath = resolver.generateName(modulle, entity);
string containerNameFullPath = resolver.generateName(modulle, containerEntity);

gprintln("Entity \"" ~ fullPath
                        ~ "\" is allowed to be defined within container \""
                        ~ containerNameFullPath ~ "\"");

It requires that we have a container as parentOf() is called by resolver as we want a full path resolution and hence the VariableParemeters need a parent so resolver can climb and generate the full path.

I wonder, is it not related to maybe theese not having a container? Line 126 is the following in the resolver: ```d assert(cast(Entity) currentEntity.parentOf()); ``` And because these are run through by the typechecker in collision detection stage (because we have added these `VariableParameter[]` to the `bodyStatements`/`getStatements()`,therefore they will be checked and resolution too). **Note** Resolution is NOT required but occurs as we have the lines(in the `checkCollision()` method): ```d string fullPath = resolver.generateName(modulle, entity); string containerNameFullPath = resolver.generateName(modulle, containerEntity); gprintln("Entity \"" ~ fullPath ~ "\" is allowed to be defined within container \"" ~ containerNameFullPath ~ "\""); ``` --- It requires that we have a container as `parentOf()` is called by resolver as we want a full path resolution and hence the `VariableParemeter`s need a parent so resolver can climb and generate the full path.

deavmi commented

2022-12-16 15:22:22 +00:00

We are getting an error, where the node is not visited as it is pooled on the spot in the branch:

/**
* Variable asignments
*/
else if(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());

    /* Pool the variable */
    DNode varDecDNode = pool(variable);

The reason for this is the variable declaration Variable pass only happens later and should be pooled first. This is easy fix, we won't do weird weight changes, although we could. In fact the better option is this:

// Add the parameters first THEN the function's body statements
// because they must be available before other statements
// which may reference them, Secondly they must be added (the VariableParameter(s))
// such that they are lookup-able.
addStatements(cast(Statement[])params);

// Add the funciton's body
addStatements(bodyStatements);

We are getting an error, where the node is not visited as it is pooled on the spot in the branch: ```d /** * Variable asignments */ else if(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()); /* Pool the variable */ DNode varDecDNode = pool(variable); ``` The reason for this is the variable declaration `Variable` pass only happens later and should be pooled first. This is easy fix, we won't do weird weight changes, although we could. In fact the better option is this: ```d // Add the parameters first THEN the function's body statements // because they must be available before other statements // which may reference them, Secondly they must be added (the VariableParameter(s)) // such that they are lookup-able. addStatements(cast(Statement[])params); // Add the funciton's body addStatements(bodyStatements); ```

deavmi commented

2022-12-16 15:26:28 +00:00

Now that that works, we need to add a seperate DNode branch thing to not add dnodes for the VariableParameter(s)

Now that that works, we need to add a seperate DNode branch thing to not add dnodes for the `VariableParameter`(s)

deavmi commented

2022-12-16 15:34:34 +00:00

Currently this returns null in the dependency/core.d:

if(variable)
{
	/* Pool the node */
	// FIXME: This is returning null for parameter variables
	VariableNode varDecNode = poolT!(VariableNode, Variable)(variable);

Currently this returns `null` in the `dependency/core.d`: ```d if(variable) { /* Pool the node */ // FIXME: This is returning null for parameter variables VariableNode varDecNode = poolT!(VariableNode, Variable)(variable); ```

deavmi commented

2022-12-16 15:44:22 +00:00

Aight the above is fixed with using the correct variable type in poolt to spwawn the correct DNode kind-of (must be VariableNode as that is what expressionPass() will cast too and if not, like just straight DNode then the cats fails and null is returned in that expressionPass() code):

DNode dnode = poolT!(VariableNode, Variable)(varParamDec);
dnode.markVisited();

Aight the above is fixed with using the correct variable type in poolt to spwawn the correct `DNode` kind-of (must be `VariableNode` as that is what `expressionPass()` will cast too and if not, like just straight `DNode` then the cats fails and `null` is returned in that `expressionPass()` code): ```d DNode dnode = poolT!(VariableNode, Variable)(varParamDec); dnode.markVisited(); ```

deavmi commented

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

Next is the symbol mapping of the arguments in the function signature, this must be done in the emitting process...

deavmi added the

emit

label 2022-12-16 15:45:02 +00:00

deavmi commented

2022-12-16 15:49:35 +00:00

A simple update to generateSignature(Function) worked:

Variable currentParameter = parameters[parIdx];

// Generate the symbol-mapped names for the parameters
Variable typedEntityVariable = cast(Variable)typeChecker.getResolver().resolveBest(func, currentParameter.getName()); //TODO: Remove `auto`
string typedEntityVariableName = typeChecker.getResolver().generateName(func, typedEntityVariable);
string renamedSymbol = SymbolMapper.symbolLookup(func, typedEntityVariableName);

// Generate <type> <parameter-name (symbol mapped)>
parameterString~=currentParameter.getType()~" "~renamedSymbol;

A simple update to `generateSignature(Function)` worked: ```d Variable currentParameter = parameters[parIdx]; // Generate the symbol-mapped names for the parameters Variable typedEntityVariable = cast(Variable)typeChecker.getResolver().resolveBest(func, currentParameter.getName()); //TODO: Remove `auto` string typedEntityVariableName = typeChecker.getResolver().generateName(func, typedEntityVariable); string renamedSymbol = SymbolMapper.symbolLookup(func, typedEntityVariableName); // Generate <type> <parameter-name (symbol mapped)> parameterString~=currentParameter.getType()~" "~renamedSymbol; ```

deavmi commented

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

This has all been implemented with commit 8972a277f3f4b718667e457755cbed16f93c59b9/functions_parameters_fix on branch functions_parameters_fix

This has all been implemented with commit `8972a277f3f4b718667e457755cbed16f93c59b9/functions_parameters_fix` on branch `functions_parameters_fix`

deavmi closed this issue

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

deavmi stopped working 2022-12-16 16:06:27 +00:00

2 hours 54 minutes

deavmi reopened this issue

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

deavmi started working 2022-12-16 16:14:58 +00:00

deavmi canceled time tracking 2022-12-16 16:15:01 +00:00