Added Natvis for some internal datalibrary types #179
Conversation
src/dl.cpp
Outdated
| #ifdef DL_DEBUGGING_ENABLED | ||
| // This is just for natvis, not to be used elsewhere | ||
| dl_ctx_t g_DlDebuggingContext = nullptr; | ||
| const dl_ctx_t* dl_type_desc::ctx = &g_DlDebuggingContext; |
There was a problem hiding this comment.
I am no fan of all of these globals but I can see that it can't really be done in natvis without at least one global :(
I would like the define to be DL_NATVIS_ENABLED as this relates to natvis and not some general debugging.
It should also be enough with one g_DlActiveNatvisCtx so that we don't pollute all structs with this CTX.
The comment above might need a bit of expansion as well on the fact that this context will be used when inspecting values via natvis.
One thing that could be worth considering is to add a DL_NATVIS_SCOPE(ctx) that can be put in the top-level functions, that with a tls might automate the setup of the global? Not sure how much "muck" that would add to the codebase however?
There was a problem hiding this comment.
I can't access "global scope variables", only "type scoped globals" from natvis. In other words I need all the globals except g_DlDebuggingContext. I added g_DlDebuggingContext just so I can change a single pointer in the debugger when I want to inspect a different context, instead of manually having to change all five places.
I will improve the documentation about this and rename them to your liking.
There was a problem hiding this comment.
About making the natvis context TLS (or FLS) aware, this seems very exotic to me. If a developer uses different contexts per thread or fiber then I think they should make that adaptation in their own local version of the library. This is an open source library where such adaptations are easy to do, and thus I would prefer to only provide basic and common use cases.
There was a problem hiding this comment.
I was expecting globals to work as this doc says that it should work https://learn.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2022. But I also trust you on this one, just not a fan of polluting the structs with all these statics :(
There was a problem hiding this comment.
Do you mean a scope which can declare the global as extern and then assign it? Or something else?
There was a problem hiding this comment.
I'm not a fan of the statics neither. I guess it could be solved by adding a dll based natvis or something similar. But that is far past my ambitions in this case.
Now the types
dl_substr,dl_array,dl_context,dl_type_desc,dl_member_desc,dl_enum_desc,dl_enum_value_descanddl_enum_alias_descwill show the meaningful data they contain instead of raw offsets.There are still plenty of improvements which can be made, but this batch already makes me much more efficient so I'm sharing it early instead of in a few years when it maybe, possibly is better than now...