[lldb] Add unittests for SwiftLanguageRuntime demangling functions #9422
Conversation
| switch (node->getFirstChild()->getKind()) { | ||
| case Node::Kind::Static: | ||
| case Node::Kind::ExplicitClosure: | ||
| while (llvm::is_contained({Node::Kind::Static, Node::Kind::ExplicitClosure}, |
There was a problem hiding this comment.
suggestion: only support nested ExplicitClosure.
as far as I know, there's no case where Static nodes are nested. While there's no harm in supporting nested Static nodes, I'd prefer to see the code match the reality. Purely for expressiveness. I'd rather not have a reader wonder "why does this need to handle nested Static nodes"?
There was a problem hiding this comment.
Is this a quadratic algorithm?
It's linear in the depth of the tree: one node in each level of the tree is compared against two possibilities, i.e. it performs a constant number of operations on each level of the tree.
as far as I know, there's no case where Static nodes are nested.
So is it safe to say that if there is a Static node, it must be at the root of the tree?
There was a problem hiding this comment.
Updated the PR with suggestions
felipepiovezan
force-pushed
the
felipe/add_unittests_demangler
branch
5 times, most recently
from
e16f58a to
dca54e1
Compare
This should make it easier to make changes and ensure they are correct. For example, as commented in the tests, there are known issues with the async function detection.
felipepiovezan
force-pushed
the
felipe/add_unittests_demangler
branch
from
dca54e1 to
3e6402b
Compare
These produce nested Node objects with the Kind::ExplicitClosure annotation, all of which must be peeled in order to find the Function node.
felipepiovezan
force-pushed
the
felipe/add_unittests_demangler
branch
from
3e6402b to
4dac4d3
Compare
|
Updated wording of a comment |
This should make it easier to make changes and ensure they are correct. For example, as commented in the tests, there are known issues with the async function detection.