-
Notifications
You must be signed in to change notification settings - Fork 39
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
Swati/internal function #329
Conversation
src/transformers/visitors/circuitInternalFunctionCallVisitor.ts
Outdated
Show resolved
Hide resolved
@@ -213,6 +213,9 @@ const internalCallVisitor = { | |||
file.nodes.forEach(childNode => { | |||
if(childNode.nodeType === 'FunctionDefinition') { | |||
childNode.parameters.modifiedStateVariables = joinWithoutDupes(childNode.parameters.modifiedStateVariables, state.newParametersList); | |||
const modifiedNodes = childNode.parameters.modifiedStateVariables.map(node => node.name); | |||
if(state.decNode && !(modifiedNodes.includes(state.decNode.declarations[0].name))) | |||
childNode.body.preStatements.splice(1, 0, state.decNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you leave a comment here, explaining this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/transformers/visitors/circuitInternalFunctionCallVisitor.ts
Outdated
Show resolved
Hide resolved
src/transformers/visitors/circuitInternalFunctionCallVisitor.ts
Outdated
Show resolved
Hide resolved
@@ -306,22 +312,33 @@ const internalCallVisitor = { | |||
childNode.body.statements[id-1] = statenode; | |||
node.body.statements.forEach(kidNode =>{ | |||
if(kidNode.nodeType === 'ExpressionStatement'&& kidNode.expression.name === state.internalFncName[index]) { | |||
kidNode.expression = Object.assign(kidNode.expression,statenode.initialValue); | |||
if (kidNode.expression.operator) { | |||
const newExpressionNode = Object.assign(cloneDeep(kidNode.expression), statenode.initialValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still dont' get exactly what is happening here? What does kidNode.expression.operator correspond to? Isn't this normally defined? kidNode remains in node.body.statements so doesn't this lead to a duplicate?
src/transformers/visitors/orchestrationInternalFunctionCallVisitor.ts
Outdated
Show resolved
Hide resolved
src/transformers/visitors/orchestrationInternalFunctionCallVisitor.ts
Outdated
Show resolved
Hide resolved
}); | ||
}); | ||
deletedIndexes.forEach(index => parameterList.splice(index, 1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to be careful here as well. Say we have indexes (2,5) stored in deleted indexes, and we delete index 2 first then the element from index 5 is now in index 4 so the wrong index gets deleted. We need to go through the indexes in reverse order from highest to lowest.
// if the function doesn't modifies the returned variable don't include it | ||
if(state.decNode && !(modifiedNodes.includes(decNode.declarations[0].name))) | ||
childNode.body.preStatements.splice(decIndex+1, 0, decNode); | ||
}): ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The name decNode is used twice here in the for loop and also declared on line 218, could we use a different name for one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed the name
}); | ||
|
||
if(state.expNode) state.newStatementList.push(state.expNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this now obsolete?
@@ -91,10 +91,6 @@ export default function codeGenerator(node: any, options: any = {}): any { | |||
if (node.declarations[0].isStruct) return `\n let ${codeGenerator(node.declarations[0])} = {}; \n${codeGenerator(node.initialValue)};`; | |||
return `\nlet ${codeGenerator(node.initialValue)};`; | |||
} else if (node.declarations[0].isAccessed && !node.declarations[0].isSecret) { | |||
const obj = {} | |||
if(Object.keys(node.initialValue).length > 1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just checking, should 95-97 be removed?
newNode.expression = state.initNode; | ||
} | ||
|
||
parent._newASTPointer.interactsWithSecret ? state.returnPara = returnPara : ' '; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have the same issue as elsewhere here with state.returnPara being overwritten?
No description provided.