bulbazord added a comment. In D149213#4491889 <https://reviews.llvm.org/D149213#4491889>, @VladimirMakaev wrote:
> In D149213#4491594 <https://reviews.llvm.org/D149213#4491594>, @bulbazord > wrote: > >> I'm curious to know why you don't try and implement some kind of >> `TypeSystemRust`? I assume it's going to be a lengthy process, but >> eventually you (or somebody else) is going to want to implement a >> `TypeSystemRust` in some form and they'll have to revisit all of >> rust-specific things we've tacked onto various parts of LLDB (e.g. this >> functionality with DWARFASTParserClang). I suppose the question I'm trying >> to answer is "How much do we add to LLDB before actually adding proper rust >> support?" > > I think there was an attempt in the past to build TypeSystemRust. Rust > Project had a fork of LLDB with this implemented by Tom Tromey and CodeLLDB > maintainer(vadimcn) has one. Both of them were not able to merge that into > LLDB codebase. A relevant link to a discussion > https://rust-lang.zulipchat.com/#narrow/stream/317568-t-compiler.2Fwg-debugging/topic/upstreaming.20rust.20support.20into.20lldb > > Apart from just implementing type system itself (which is much bigger scope > than this change) there are other non-trivial issues: > > 1. There is no "compiler-as-a-service" in Rust so getting expressions to work > is non-trivial. An interpreter of some sort needs to be built with subset of > Rust support > 2. Infra changes (DEVOPS) changes are required to start building Rust > inferiors to run tests on CI > 3. LLVM / rustc cross repository logistics. Rustc probably needs to live in > LLVM to make this work well. > > Given the fact that the majority of the things "just work" when debugging > Rust with LLDB (using Clang declarations effectively as of today) I kinda > believe this is not a bad solution to try to do a bit of extra care for Rust > types in TypeSystemClang and the debugging experience will be great for Rust > users. As long as LLDB emits enough information a conversion can be made in > synthetic provider to display it nicely in debugger. Tests can be written to > make sure it doesn't rot. This is definitely something I want to improve in > this PR too. > > I think if Rust Project Debugging WG decides to invest into native > TypeSystemRust I think having these fixes around isn't going to slow down the > progress but quite the contrary (in my opinion). And cleaning them up will > also be trivial since they're specifically guarded by CU check. Thanks for giving that background, I was aware there were conversations about this somewhere but I didn't have the full context. >> I was also curious about the test. I'm not sure how the test is supposed to >> work here, is the rust source file actually built or is that just to show >> how the yaml file was generated? There's no rust support in our testing >> infrastructure AFAICT so I assume this just makes sure that something >> already generated doesn't get broken in the future. > > I'm gonna work on the test more and try to make assertions against global > SBValue. Greg suggested this might work. So since we don't have access to > rustc in the testing setup I've generated object file from main.rs manually > and converted it to yaml. This makes the test portable. (main.rs is kept for > the reference) Sounds good. I added a comment inline about that. ================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:3184 + case DW_TAG_variant_part: + ParseVariantPart(die, parent_die, class_clang_type, default_accessibility, + layout_info); ---------------- clayborg wrote: > clayborg wrote: > > I would add a check here for the Rust language. We might also want to > > rename "ParseVariantPart" to "ParseRustVariant" to indicate this is > > specific to the Rust language. > Something like: > > ``` > if (die.GetCU()->GetDWARFLanguageType() == eLanguageTypeRust) > ParseRustVariant(die, parent_die, class_clang_type, default_accessibility, > layout_info); > break; > ``` +1, I'm partial to the `ParseRustVariantPart` name myself. IMO, I think it would be helpful knowing that this functionality was written with rust in mind (especially since it's a clang-based codepath). That way if we need to support this in the future for something in clang (or any other language), it makes it easy to see at a glance that `ParseVariantPart` is intended for rust. ================ Comment at: lldb/test/API/lang/rust/enum-structs/main.rs:1-4 +#![no_std] +#![no_main] +use core::ptr::NonNull; + ---------------- VladimirMakaev wrote: > clayborg wrote: > > Is this file used at all? I don't see any references to it. I am guessing > > that the main.yaml is expected to reference this file? > It's not used I've just put it as a reference of the program that the object > file was taken from. I used rustc to produce object file from this program In that case, could you include the exact invocation of rustc (and any other follow-up commands) in a comment at the beginning of this file? That way it can be rebuilt if the test needs adjustment for some reason. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149213/new/ https://reviews.llvm.org/D149213 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
