Depends on D102729
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Could you split this up - one patch to add dyn-bounds with zero dyn-traits, then add dyn-traits with zero dyn-trait-assoc-binding, then add those? (takes me a while to check the test cases/understand exactly what they're doing - so having really narrow patches makes it easier to focus on the difference between one patch and the next)
Parse dyn-bounds without any dyn-traits
At language level at least one trait is required, so visually this doesn't look
quite right without any traits and trailing whitespace at the end.
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
485–492 | keep thinking it'd be nice if this were wrapped up in some (albeit parameterized for different situations you mentioned) "demangleLifetime" to match the syntax description more closely | |
554–555 | According to the grammar/spec this is <binder> = "G" <base-62-number> rather than <binder> = ["G" <base-62-number>]? | |
555–556 | Not sure how consistently this is done throughout the code - but maybe including "optional" when a feature is optional (so it's clear it's implementing [<binder>] not <binder>) would be good? | |
llvm/test/Demangle/rust.test | ||
243–244 | Could this test be simplified down to: CHECK: trait::<dyn for<'b> + 'a> _RIC5traitDG_EL_E |
Rebase
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
485–492 | I am still somewhat reluctant to change this, since I would have to introduce three different variants and each would be used only once. For example, in variant here, when lifetime is printed it should be preceded by + . | |
555–556 | I renamed the function in parent revision to demangleOptionalBinder. | |
llvm/test/Demangle/rust.test | ||
243–244 | In "D" <dyn-bounds> <lifetime> production, <lifetime> is outside the binder from <dyn-bounds>. Additionally since erased lifetimes are not printed here, test has to use another binder. |
keep thinking it'd be nice if this were wrapped up in some (albeit parameterized for different situations you mentioned) "demangleLifetime" to match the syntax description more closely