[PATCH] D102579: [Demangle][Rust] Parse tuples
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed May 19 11:48:57 PDT 2021
dblaikie added inline comments.
================
Comment at: llvm/lib/Demangle/RustDemangle.cpp:450-451
+ }
+ if (I == 1)
+ print(",");
+ print(")");
----------------
tmiasko wrote:
> dblaikie wrote:
> > tmiasko wrote:
> > > dblaikie wrote:
> > > > Looks a bit awkward to print a comma after a single argument (`(_,)`) - is that necessary? (does it disambiguate this with some other case that has only a list of 1 between parentheses?)
> > > The intention is to match Rust language syntax which requires a comma after one element tuples to disambiguate them from parenthesized types.
> > Cool - thanks for the context!
> >
> > And stylistically, that trailing comma is usually written without a space after it, unlike separating commas?
> >
> > Pity, this could come out more naturally from the algorithm if that was the case (always printing ", " after every type - rather than having to special case)
> >
> > I guess you could do it this way:
> > ```
> > for (...) {
> > if (I > 0)
> > print(" ");
> > demangleType();
> > print(",");
> > }
> > ```
> > Marginally less special casing
> Looking at rustfmt, rustc type printer and other demanglers, they are consistent writing trailing comma without a space. They also include the trailing comma only when necessary, i.e., for one element tuple only (so I retained the special case).
Cool, I still wonder if it might be marginally more neatly written the way I suggested - it still produces the same output, but with one fewer "if", I think?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D102579/new/
https://reviews.llvm.org/D102579
More information about the llvm-commits
mailing list