Depends on D102578
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | 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?) |
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | The intention is to match Rust language syntax which requires a comma after one element tuples to disambiguate them from parenthesized types. |
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | 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 |
Rebase
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | 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). |
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | 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? |
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | It would be (), (T1,), and then (T1, T2,) instead of (T1, T2). |
llvm/lib/Demangle/RustDemangle.cpp | ||
---|---|---|
452–453 | Oh, right. Thanks for setting me straight! |
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?)