Dead code eliminator: use default if other actions won't be executed #56
Conversation
|
Fixes #54. |
| if (defaultAction != nullptr) { | ||
| auto decl = getActionDecl(refMap, *defaultAction); | ||
| if (decl.has_value() && !decl.value()->body->components.empty()) { | ||
| printInfo("Replacing table apply with default action %1%", defaultAction); |
There was a problem hiding this comment.
Well, we can still add some form of eliminated node here. Otherwise we do not track that we have modified the table here. What do you think?
There was a problem hiding this comment.
In the sense of eliminatedNodes.push_back(stmt); or something similar. Our current ref files do not quite capture this ambiguity.
There was a problem hiding this comment.
Yes. I think a better way is to create another vector to store these because they are not simply eliminated. What do you think?
There was a problem hiding this comment.
You could make it a vector of pairs, with the original node and the replaced node.
There is a question on the performance impact when we keep adding nodes. Recall that this code may run on every control-plane update message. But I do not think it will have much impact because we only invoke the elimination when the semantics have changed. Still, it's something we should keep in mind.
There was a problem hiding this comment.
You could make it a vector of pairs, with the original node and the replaced node.
That sounds good. I will do it soon.
And yes, I agree on the performance thing.
RabbitWhite1
force-pushed
the
zhanghan/default-action
branch
from
d0ad039
to
5de49df
Compare
RabbitWhite1
force-pushed
the
zhanghan/default-action
branch
from
cb62e69
to
0a4d536
Compare
|
Support recording replacement, and updated reference_checker and reference files accordingly. PTAL. |
| << node->getSourceInfo().toPosition().sourceLine << ": " | ||
| << sourceFragment; | ||
| } else { | ||
| // FIXME: any elegant solution to just print first line of `replaced->toString()`? |
There was a problem hiding this comment.
Do not use toString it only gives incomplete node information, use the source fragment. I think it is fine to have multiple lines. But we need to check how that looks like. Let's fix that in a separate PR.
Also make sure the source info is valid. It should be by necessity of construction, but it is good to check that this invariant holds.
There was a problem hiding this comment.
Thanks. Will do that.
No description provided.