Searchfox diagrams need to account for context/contextsym now including lambdas since bug 1856762
Categories
(Webtools :: Searchfox, enhancement)
Tracking
(Not tracked)
People
(Reporter: asuth, Assigned: asuth)
References
Details
As noted in https://bugzilla.mozilla.org/show_bug.cgi?id=1856762#c8 the landing of bug 1856762 caused a regression in the (pre-alpha, unsupported) searchfox diagram functionality where any calls made from lambdas break the cmd_traverse traversal because we now emit the lambda as the context(sym) but we don't emit structured information for the lambdas.
If we did emit structured information for the lambdas, cmd_traverse would also need to understand the relationship between its lambda and the containing function/method. The appropriate initial heuristic would likely be to just elide the detail of the lambda. An expansion on that could be to use the record presentation we use for aggregated classes and methods to aggregate lambdas as ~tree children of their method.
If we don't emit structured information for the lambdas, we could adjust the cmd_traverse heuristic to try and manipulate the pretty "context", but a complication right now is that we currently seem to include the full function signature inclusive of arguments in that case which is not something our "split on ::" logic can handle, so we still would need to clean up the emitted "context".
Aside: There's some blue sky thinking on how we might better understand the relationships of lambdas in bug 1790683 but that's very out of scope for this and I think we'd want the clang static analyzer (CSA) hookup done or fully abandoned before trying that.
|
Assignee | |
Updated•8 months ago
|
|
|
Assignee | |
Comment 1•8 months ago
|
||
It's unclear if things changed after my brief investigation in https://bugzilla.mozilla.org/show_bug.cgi?id=1856762#c8 (a bunch of commits landed that could have changed things) or I didn't look thoroughly enough at the time, but using the example of this lambda from that comment based on the use for the symbol in this symbol search we are emitting structured records:
1, over the "actor" in StrongWorkerRef::Create(aWorkerPrivate, "FetchChild", [actor]() {
{
"loc": "00255:61-66",
"structured": 1,
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::(anonymous)",
"sym": "F_<T_888a80f064fe6e82>_",
"type_pretty": null,
"kind": "field",
"subsystem": null,
"parentsym": "T_888a80f064fe6e82",
"implKind": "",
"sizeBytes": null,
"alignmentBytes": null,
"ownVFPtrBytes": null,
"bindingSlots": [],
"ontologySlots": [],
"supers": [],
"methods": [],
"fields": [],
"overrides": [],
"props": [],
"variants": []
}
2, over the opening [ of the lambda in StrongWorkerRef::Create(aWorkerPrivate, "FetchChild", [actor]() { which is the same for all the rest, corresponds to the CXXRecordDecl that wraps the CXXMethodDecl that the operator() gets placed on, and holds the captures as fields, plus exciting things like the (defaulted) destructor to clean up the captures,
{
"loc": "00255:60-61",
"structured": 1,
"pretty": "(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61)",
"sym": "T_888a80f064fe6e82",
"type_pretty": null,
"kind": "class",
"subsystem": null,
"implKind": "",
"sizeBytes": 8,
"alignmentBytes": 8,
"ownVFPtrBytes": null,
"bindingSlots": [],
"ontologySlots": [],
"supers": [],
"methods": [
{
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::operator()",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEENK3$_0clEv",
"props": [
"instance",
"user",
"constexpr"
],
"args": []
},
{
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61)",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0C1ERKSB_",
"props": [
"instance",
"defaulted"
],
"args": []
},
{
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61)",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0C1EOSB_",
"props": [
"instance",
"defaulted"
],
"args": []
},
{
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::operator=",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0aSERKSB_",
"props": [
"instance",
"defaulted",
"deleted"
],
"args": []
},
{
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::~(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61)",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0D1Ev",
"props": [
"instance",
"defaulted"
],
"args": []
}
],
"fields": [
{
"lineRange": "#255",
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::(anonymous)",
"sym": "F_<T_888a80f064fe6e82>_",
"type": "class RefPtr<class mozilla::dom::FetchChild>",
"typesym": "T_RefPtr",
"offsetBytes": 0,
"bitPositions": null,
"sizeBytes": 8
}
],
"overrides": [],
"props": [],
"variants": []
}
3: capture constructor on the class?
{
"loc": "00255:60-61",
"structured": 1,
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61)",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0C1EOSB_",
"type_pretty": null,
"kind": "method",
"subsystem": null,
"parentsym": "T_888a80f064fe6e82",
"implKind": "",
"sizeBytes": null,
"alignmentBytes": null,
"ownVFPtrBytes": null,
"bindingSlots": [],
"ontologySlots": [],
"supers": [],
"methods": [],
"fields": [],
"overrides": [],
"props": [
"instance",
"defaulted"
],
"variants": [],
"args": [
{
"name": "",
"type": "(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61) &&",
"typesym": "T_888a80f064fe6e82"
}
]
}
4: capture constructor on the class?
{
"loc": "00255:60-61",
"structured": 1,
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::(lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61)",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0C1ERKSB_",
"type_pretty": null,
"kind": "method",
"subsystem": null,
"parentsym": "T_888a80f064fe6e82",
"implKind": "",
"sizeBytes": null,
"alignmentBytes": null,
"ownVFPtrBytes": null,
"bindingSlots": [],
"ontologySlots": [],
"supers": [],
"methods": [],
"fields": [],
"overrides": [],
"props": [
"instance",
"defaulted"
],
"variants": [],
"args": [
{
"name": "",
"type": "const (lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61) &",
"typesym": "T_888a80f064fe6e82"
}
]
}
5: operator= on the class
{
"loc": "00255:60-61",
"structured": 1,
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::operator=",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEEN3$_0aSERKSB_",
"type_pretty": null,
"kind": "method",
"subsystem": null,
"parentsym": "T_888a80f064fe6e82",
"implKind": "",
"sizeBytes": null,
"alignmentBytes": null,
"ownVFPtrBytes": null,
"bindingSlots": [],
"ontologySlots": [],
"supers": [],
"methods": [],
"fields": [],
"overrides": [],
"props": [
"instance",
"defaulted",
"deleted"
],
"variants": [],
"args": [
{
"name": "",
"type": "const (lambda at /builds/worker/checkouts/gecko/dom/fetch/FetchChild.cpp:255:61) &",
"typesym": "T_888a80f064fe6e82"
}
]
}
6: operator() on the class
{
"loc": "00255:60-61",
"structured": 1,
"pretty": "mozilla::dom::FetchChild::CreateForWorker::(anonymous)::operator()",
"sym": "_ZZN7mozilla3dom10FetchChild15CreateForWorkerEPNS0_13WorkerPrivateE6RefPtrINS0_7PromiseEES4_INS0_15AbortSignalImplEES4_INS0_13FetchObserverEEENK3$_0clEv",
"type_pretty": null,
"kind": "method",
"subsystem": null,
"parentsym": "T_888a80f064fe6e82",
"implKind": "",
"sizeBytes": null,
"alignmentBytes": null,
"ownVFPtrBytes": null,
"bindingSlots": [],
"ontologySlots": [],
"supers": [],
"methods": [],
"fields": [],
"overrides": [],
"props": [
"instance",
"user",
"constexpr"
],
"variants": [],
"args": []
}
|
|
Assignee | |
Comment 2•8 months ago
|
||
In my WIP thus far I'd been looking at introducing a new structured "kind" of "lambda" so where we might have a pretty that looked like SomeClass::someMethod::(anonymous)::operator() it could instead look like SomeClass::someMethod::(lambda0). But maybe it's better to instead just leave the individual methods intact as-is and just turn the (anonymous) class into (lambdaN). Combined with a change to use our getQualifiedName consistently and have us always build the qualified name rather than having a fast-path for isFunctionOrMethod, this would improve our pretty situation. We could then also do something similar for our getMangledName so that we can provide a more readable and stable synthetic symbol to describe the lambda class which also potentially matters as we start to do more clever things with templates like care about their instantiations which the location-based-hash does not help with.
And rather than change the kind for the anonymous class from "class" to "lambda", maybe we'd add synthetic "props" entries to the class and operator(), something like lambda{} and lambda() (or ideally something better). The key thing would be that cmd_traverse could identify from the function's structured info that it's a lambda and apply the heuristic to ignore the lambda and walk up to its containing defining context.
Description
•