Juspreet - New Fixpoint Graph Builder - Immutable Only#3
Juspreet - New Fixpoint Graph Builder - Immutable Only#3jssandh2 wants to merge 20 commits intoshravanrn:RefinementTypesfrom
Conversation
…ock Modification and exposed functions
…rrors. Compilation woks.
| namespace liquid | ||
| { | ||
|
|
||
| class PhiNodeObligation |
There was a problem hiding this comment.
PhiNodeObligation is a DS used to automatically infer phi nodes for mutable code - not needed here
|
|
||
| std::map<std::string, std::set<std::string>> variablesValuesPerBlock; | ||
| std::set<std::string> finishedBlocks; | ||
| std::map<std::string, std::vector<PhiNodeObligation>> phiNodeObligations; |
|
@shravanrn : PhiNodeObligation removed. A |
|
|
||
| // Ensure that : | ||
| // 1) forall blocks in PreviousFinishedBlocks, dominate(blocks, block), forall block in PreviousUnfinishedblocks | ||
| for (const auto& previousUnfinishedBlock : previousFinishedBlocks) |
There was a problem hiding this comment.
previousUnfinishedBlock : previousFinishedBlocks is definitely wrong
| bool first = true; | ||
|
|
||
| for (auto& previousBlock : previousBlocks) | ||
| { |
| } | ||
|
|
||
| ResultType VariablesEnvironmentImmutable::validateFinishedAndUnfinishedBlockStates( | ||
| const std::vector<std::string>& previousFinishedBlocks, |
There was a problem hiding this comment.
The upcoming commit may not immediately remove this function. I am first removing redundant variables, and making sure the new logic is not broken because of removals. This will come up next.
|
|
||
| bool first = true; | ||
|
|
||
| for (auto& previousBlock : previousBlocks) |
There was a problem hiding this comment.
Impose the linear order by checking ancestors - Juspreet
|
|
||
| for (auto& previousBlock : previousBlocks) | ||
| { | ||
| if (first) |
There was a problem hiding this comment.
Before accessing variablesMappingsPerBlock[previousBlock] there should be a check to make sure variablesMappingsPerBlock[previousBlock] is actually filled in... else the order of iteration is broken
| return ResultType::Success(); | ||
| } | ||
|
|
||
| ResultType VariablesEnvironmentImmutable::createPhiNodeWithoutCreatedBinders(const std::string& variable, const std::string& mappedVariableName, const FixpointType& type, const std::vector<std::string>& sourceVariableNames, const std::vector<std::string>& previousBlocks) |
There was a problem hiding this comment.
createPhiNodeAssumingNoFutureBinders
There was a problem hiding this comment.
I reversed this, as this was wrong. We were not using createPhiNodeAssumingNoFutureBinders and were actually using createPhiNodeWithoutCreatedBinders. The functionality wasn't different, as there is a statement check for undeclared variables that we will never go into (save for the sake of for loops).
|
|
||
| predecessorTransitionGuards.emplace_back("("s + predecessorEntryGuard + " && "s + transitionGuardName + ")"s); | ||
| } | ||
|
|
There was a problem hiding this comment.
Throw error if more than 2 predecessors
| if (!getPredRes.Succeeded) { return getPredRes; } | ||
| } | ||
|
|
||
| std::vector<std::string> predecessorTransitionGuards; |
There was a problem hiding this comment.
predecessors.size() > 2 => Not good.
| } | ||
|
|
||
| //@TODO:: (juspreet) - The recursive call here isn't clear. Verify. | ||
| ResultType VariablesEnvironmentImmutable::getBlockGuard(const std::string& blockName, std::string& blockGuard) |
There was a problem hiding this comment.
Replace with fixpoint algorithm
| } | ||
| } | ||
|
|
||
| for (const auto& blockName : blockNames) |
There was a problem hiding this comment.
This is on the next part of the TODO list - Replacing blockGuards with a purely Fixpoint approach.
There was a problem hiding this comment.
This is on the next part of the TODO list - Replacing blockGuards with a purely Fixpoint approach.
| const FunctionBlockGraph& functionBlockGraph; | ||
|
|
||
| std::map<std::string, FixpointType> variableTypes; | ||
| std::map<std::string, std::map<std::string, std::string>> variablesMappingsPerBlock; |
| std::map<std::string, FixpointType> variableTypes; | ||
| std::map<std::string, std::map<std::string, std::string>> variablesMappingsPerBlock; | ||
|
|
||
| std::map<std::string, std::set<std::string>> variablesValuesPerBlock; |
There was a problem hiding this comment.
I've changed this to a vector, and added a RefinementUtils function to get a set from a vector. As a result, I'm wondering if this should be changed in the first place ?
| } | ||
|
|
||
| // Get the name of the variable | ||
| std::string VariablesEnvironmentImmutable::GetVariableName(const std::string variableName) |
| std::map<std::string, std::set<std::string>> variablesValuesPerBlock; | ||
| std::set<std::string> finishedBlocks; | ||
|
|
||
| std::set<std::string> outputVariables; |
|
|
||
| std::set<std::string> outputVariables; | ||
|
|
||
| std::map<std::string, std::string> cachedBlockGuards; |
| ResultType endBlock(const std::string& blockName); | ||
|
|
||
| public: | ||
| Counter FreshState; |
|
|
There are still ~3 things to discuss (though, for the sake of practicality and incremental work, this PR can be merged as it is verified to work):
|
VariablesEnvironmentImmutable.hVariablesEnvironmentImmutable.cppVariablesEnvLibEImmutable.cppstartBlock,getCommonVariables,validateFinishedAndUnfinishedBlocks, Phi Node Logic is correctcc : @shravanrn