Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct type name for associative arrays and useful Debug Information #2869

Merged
merged 12 commits into from
Nov 11, 2018
Merged

Correct type name for associative arrays and useful Debug Information #2869

merged 12 commits into from
Nov 11, 2018

Conversation

void-995
Copy link
Contributor

@void-995 void-995 commented Oct 5, 2018

No description provided.

@kinke
Copy link
Member

kinke commented Oct 5, 2018

Thanks for working on this!

@void-995
Copy link
Contributor Author

void-995 commented Oct 5, 2018

@kinke, this one is actually stopping me from adequately using VS Code in terms of debugging. I'm planning 2 phase patch for LDC2. First one is adding required data for being able to parse associative arrays in debugger and showing up real name of a type. Which is this PR, that, I hope, will be useful for others as well. Second is more tricky. I actually would like to use Microsoft's C/C++ plug-in for VS Code to debug. The trick is that mentioned debugger may use Natvis for showing up actual data from slices and associative data and working correctly if types was named like they came from C++. Meaning that for best experience Natvis require some kind of mechanism that translates names of D types to C++ types. I've already did that part as well, tested it and it works like a charm. So the second phase I would like to do is to add new optional command line argument to LDC2 that will translate D names to C++ ones and debugging information will be written like that was C++ application. That would allow best experience to work with D from VS Code on Windows. If I would able to add missing tag support for MIEngine then we would have same painless debugging process on Linux and Mac from VS Code as well. VS Code taking a huge part right now, especially if we will talk about cross-platform development, and I think that Code D plug-in is playing a big role in popularization of D programming language. All I aim here is just making debugging process as painless as possible there, which will boost my interest in D way further and will allow me to actually finish my project with it.

@kinke
Copy link
Member

kinke commented Oct 5, 2018

Sounds great. Have you seen #2826? It also makes the type names appear C++-like due to proper scoping, but is currently stalled due to an apparent LLVM CodeView bug with enabled inlining (at least with LLVM 6).

@void-995
Copy link
Contributor Author

void-995 commented Oct 5, 2018

My solution is purely fancy string parsing/translation. What we have here is sufficient for moving forward. I will definitely check PR you mentioned later.

@rainers
Copy link
Contributor

rainers commented Oct 5, 2018

Would be nice if this works with mago, too. __key_t and __val_t are already compatible with dmd output, but the typename is always dAssocArray in dmd. I suspect natvis cannot deal with D's syntax for associative arrays anyway, so for the non-C++-imitating version, dAssocArray or something else that can be identified by mago would be helpful.

@kinke
Copy link
Member

kinke commented Oct 5, 2018

Can one of you guys pls shed some light on Natvis and mago? Where/when are they used?

I noticed that the VS debugger is recently able to expand a D slice in the tree view, which is extremely cool - I guess that's due to VisualD being installed? For clarification - I usually debug in VS by taking an arbitrary C++ project in the LDC solution and setting its debug cmdline appropriately (to some D executable), no VisualD projects involved...

@rainers
Copy link
Contributor

rainers commented Oct 6, 2018

The mago Concord debugger extension adds a D expression evaluator to the VS debug engine for files that have the source language set to D in the debug info (supported by dmd and LLVM). You can see the language in the call stack. This expression evaluator also populates the slice elements in the tree.

Natvis is a macro-language of the C++ expression evaluator that allows to customize displaying variables depending on their C++ type, e.g. to avoid showing the raw members of a std::string, but the actual text only.

@void-995
Copy link
Contributor Author

void-995 commented Oct 6, 2018

@rainers, it won't be hard to change name and and value type name to match DMD at all. I would better make DMD to name associative array type with real name and, or at least, make Mago match AA by suffix, like you did with slices. Because it was dArray for slices before, isn't it? So for AA it would be simply last symbol is ], but previous before it isn't [. Also if you will catch slices by suffix first - you wouldn't even need to match previous before last.

The problem is when I tried to do it with DMD, it began to crash while slices that had same code path for taking real type name in - worked without problems. DMD's backend a bit hard to unwrap for someone who didn't worked with it for some period of time.

So even if I would like to change __value_t to __val_t and impl to ptr, I'm more for saving real AA type anyway with fixing that bug that doesn't allow saving real AA type in DMD later.

@rainers
Copy link
Contributor

rainers commented Oct 6, 2018

Just matching trailing ] without preceding [ might work indeed. What about static arrays? I guess these are never UDTs.

The problem is when I tried to do it with DMD, it began to crash

I suspect you've been populating TYPE.Tident, but this is in a union with TYPE.Tkey!

So even if I would like to change __value_t to __val_t and impl to ptr

I think there no real reason to deviate here (ptr is unused in mago).

@void-995
Copy link
Contributor Author

void-995 commented Oct 6, 2018

@rainers, you have no idea how much I'm enlighten and raging at the same time after I heard about union... So all I have to do is to double check static arrays situation and change __value_t to __val_t then? Good catch by the way, will try that in my naming converting routine.

@void-995
Copy link
Contributor Author

void-995 commented Oct 8, 2018

VS Code, Code D, MS C/C++ and Natvis

@kinke, check this one out, that's what I meant by imitating C++ type names for debugger in action:

cpp-imitating-naming-0001

cpp-imitating-naming-0002

Successfully converting without any problems as complex names as:

Alpha!(Bravo[], Charlie!(Delta, Echo!Foxtrot, Golf[42]), Hotel[India]) delegate(ref Alpha!(Bravo[], Charlie!(Delta, Echo!Foxtrot, Golf[42]), Hotel[India]) input) pure nothrow @nogc @safe
Alpha<slice<Bravo>, Charlie<Delta, Echo<Foxtrot>, Golf[42]>, associative_array<India, Hotel>> delegate(ref Alpha<slice<Bravo>, Charlie<Delta, Echo<Foxtrot>, Golf[42]>, associative_array<India, Hotel>> input) pure nothrow @nogc @safe

driver/cl_options.cpp Outdated Show resolved Hide resolved
gen/dibuilder.cpp Outdated Show resolved Hide resolved
gen/dibuilder.cpp Outdated Show resolved Hide resolved
@kinke
Copy link
Member

kinke commented Oct 9, 2018

Wrt. C++-like names, #2826 will map Module.Aggregate.Type to Module::Aggregate::Type (for CodeView at least). IIRC, we don't emit any DI templates (with proper DI template params) yet; I assume that if we did so, we'd have the C++ <T> in the CodeView names too (Rainer: would that be an issue for Mago?).

It would certainly be nice to have a solution fitting both VisualD and the VS code plugin when targeting Windows without extra cmdline option complexity. I guess the only way to do so is working on Mago?

Anyway, the screenshots are very promising, nice job. 👍

@void-995
Copy link
Contributor Author

@kinke, to make it work with VS Code there are next options:

  1. CodeView and C++ name imitating (via some command line option)
  2. DWARF and GDB on Windows (via some command line option)
  3. MI interface for Mago, there's already project called mago-mi somewhere, but it's broken right now

So first one looked like the simplest one from my point of view. Second variant is interesting as well. Third - I'm not sure how much resource and time that might take.

@kinke
Copy link
Member

kinke commented Oct 16, 2018

Thx for rebasing/polishing. It may be a good idea to change the Dwarf language tag here too if the cmdline option is enabled; what does @rainers think?

Edit: Well, maybe we could just reuse existing -gc ('optimize for non D debuggers' => global.params.symdebug == 2) to control this C++ naming.

@void-995
Copy link
Contributor Author

It was my original idea with DMD to expand on -gc. But all that -gc was doing is replacing . (dot) with @ at DI type names, while templates were written as is, dynamic arrays as is, associative arrays as dAssocArray and so on. As you might guess, that wasn't that much helpful for C++ debugger, so I came up with my own implementation of something similar. Plus, -gc is now deprecated anyway, so I gave that a new name.

And I'm also curious, @kinke, is it possible to write DWARF instead of CodeView for Windows? Maybe that would be more interesting approach? I would able to write Pretty Printer in Python for GDB then, which will work on Linux out of the box without any additional moves. Of course it should be only as command line option on Windows if that's possible. Just throwing ideas while trying to reach original goal - making debugging in VS Code easy and comfortable.

@kinke
Copy link
Member

kinke commented Oct 16, 2018

-gc isn't deprecated by LDC (just not shown in LDMD's help); AFAIK, it has never had any effect (i.e., just -g) except for that DWARF language tag given to LLVM. No idea what effect that language tag has in LLVM; I assume it only affects CodeView output language markers, i.e., no name changes (Rainer added something to LLVM's CodeView output IIRC, probably just the file marker for mago).


Emitting DWARF for Windows/MSVC targets should be possible according to https://metricpanda.com/rival-fortress-update-27-compiling-with-clang-on-windows (-gdwarf switch for clang + LLD linker).

@void-995
Copy link
Contributor Author

@kinke, seems like new Module thing broke Mago in Visual D hard.

@kinke
Copy link
Member

kinke commented Oct 20, 2018

More details please. I performed a quick check with https://github.com/ldc-developers/ldc/blob/master/tests/debuginfo/args_cdb.d, and it was looking pretty good. The only thing I noticed were base classes not showing up properly ('unable to evaluate' or so), but those weren't really usable before either, crashing VS when trying to expand Object etc.

@void-995
Copy link
Contributor Author

Now classes instead of module.submodule.ClassName!TemplateArgument appear as module.submodule::ClassName.

@kinke
Copy link
Member

kinke commented Oct 20, 2018

Ah, so template params are missing in the names; that's most likely not CodeView specific and a general issue. We should probably try to emit appropriate template DI metadata.

@void-995
Copy link
Contributor Author

Ah, so template params are missing in the names; that's most likely not CodeView specific and a general issue. We should probably try to emit appropriate template DI metadata.

Not only that, notice :: instead of . as well.

@kinke
Copy link
Member

kinke commented Oct 20, 2018

Common, that's hardly a dealbreaker, is it? Restoring the dot as D-specific namespace delimiter would need some investigation, i.e., whether that's a CodeView limitation or whether we could teach LLVM's CodeView output about it. Mago could translate too if need be.

@kinke
Copy link
Member

kinke commented Nov 9, 2018

Rebased + revised. @void-995: Please re-verify when you find some time.

@kinke
Copy link
Member

kinke commented Nov 9, 2018

[Expanding an AA, e.g., the one in args_cdb.d, still doesn't work with mago in the VS debugger.]

@kinke
Copy link
Member

kinke commented Nov 10, 2018

Would be nice if this works with mago, too. __key_t and __val_t are already compatible with dmd output, but the typename is always dAssocArray in dmd.

Added now, works like a charm, thx!

@rainers
Copy link
Contributor

rainers commented Nov 10, 2018

Added now, works like a charm, thx!

Thanks. The next version of mago will also allow working with toPrettyChars, that's what dmd emits since 2.083.

I haven't looked closely at the D to C++ symbol conversion, but my first reaction is that post-processing the result string of toPrettyChars might be a bit bittle when types get complicated (e.g. with complex literals as template arguments). I would have expected a new visitor generating the C++ type from scratch, but maybe that's going too far.

@rainers
Copy link
Contributor

rainers commented Nov 10, 2018

It may be a good idea to change the Dwarf language tag here too if the cmdline option is enabled; what does @rainers think?

Edit: Well, maybe we could just reuse existing -gc ('optimize for non D debuggers' => global.params.symdebug == 2) to control this C++ naming.

Yes, source language D should not be emitted, otherwise mago will try to interpret the C++ info in VS and probably will get confused.

@void-995
Copy link
Contributor Author

Would be nice if this works with mago, too. __key_t and __val_t are already compatible with dmd output, but the typename is always dAssocArray in dmd.

Added now, works like a charm, thx!

I've already handled that in DMD and @rainers handled that in Mago. There is no need to make special change like that, it actually kills the whole purpose of what have been started. Any reasons for why you have deleted .h and .cpp files? I saw you moved a lot into DIBuilder itself.

@kinke
Copy link
Member

kinke commented Nov 10, 2018

I've already handled that in DMD and @rainers handled that in Mago. There is no need to make special change like that, it actually kills the whole purpose of what have been started.

Well, noone left a comment here about that, and as I said, expanding it didn't work in VS (the type name is okay with mago, it doesn't show dAssocArray). If DMD 2.083 already emits the pretty name, LDC 1.13 should too and we can drop the last commit - hopefully there's a new VisualD version with an updated mago soon, so that LDC users actually see a benefit of this PR when debugging in VS (edit: okay, float[int] as typename instead of unsigned char * is already some improvement).

Any reasons for why you have deleted .h and .cpp files? I saw you moved a lot into DIBuilder itself.

They seemed overkill to me - a dedicated .cpp module for a cmdline option, a D function declaration and basically a one-liner C++ function. That's definitely not 'a lot' (and when dropping the cmdline option for existing -gc, we're talking about a handful of lines).

@kinke
Copy link
Member

kinke commented Nov 10, 2018

hopefully there's a new VisualD version with an updated mago soon

v0.48-beta3 released 6 hours ago ;) - thx Rainer. Tested it, found it working nicely with the pretty name => last commit dropped.

@JohanEngelen
Copy link
Member

(fyi, Clang has an option to tune debug information for whatever debugger you choose to use: https://clang.llvm.org/docs/ClangCommandLineReference.html#debugger-to-tune-debug-information-for . We could add the same.)

@kinke
Copy link
Member

kinke commented Nov 10, 2018

LLVM exposes that (or a similar one) directly, it's -debugger-tune=<gdb|lldb|sce> for LDC. No idea what effect it has, but most likely none for CodeView.

@kinke
Copy link
Member

kinke commented Nov 11, 2018

Merging this eagerly as I'm preparing more CodeView fixes, and that would lead to merge conflicts I'd rather avoid.

@kinke kinke merged commit 540f19c into ldc-developers:master Nov 11, 2018
@void-995 void-995 deleted the better-debugging branch December 7, 2018 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants