[Lldb-commits] [PATCH] D149213: [lldb] Add basic support to Rust enums in TypeSystemClang

[Alex Langford via Phabricator via lldb-commits]

bulbazord added a comment.

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 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.



================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2526-2527
 
+ConstString VariantMember::GetName() const {
+  return ConstString(this->variant_name);
+}
----------------
Store `variant_name` as a ConstString instead of creating one each time. Creating a `ConstString` (even if the string is in the StringPool) has a non-trivial creation cost.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2530
+
+bool VariantMember::IsDefault() const { return !discr_value.has_value(); }
+
----------------
`return !discr_value;`

`has_value` does the same thing as `operator bool` for `std::optional`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149213/new/

https://reviews.llvm.org/D149213