On 16/05/2020 01:42, Witold Baryluk via Gcc-patches wrote:
> Adds support for DMGL_RET_POSTFIX in D demangler, so it shows the type
> of the declared variable, or function return type. Postfix notation is
> used with space.
>
Hi,
Thanks for your contribution, it would be good to have a confirmation on
whether you have assigned copyrights to the FSF.
I've left some comments about the change itself below.
> libiberty/ChangeLog:
>
> * d-demangle.c: Add DMGL_RET_POSTFIX support.
> * testsuite/d-demangle-expected: Add new tests.
>
You can make the skeleton for the changelog entry using contrib/mklog, each
individual function and type change should be listed.
> index f2d6946ec..318c21c8d 100644
> --- a/libiberty/d-demangle.c
> +++ b/libiberty/d-demangle.c
> @@ -1553,6 +1555,15 @@ dlang_parse_mangle (string *decl, const char *mangled,
> struct dlang_info *info)
>
> string_init (&type);
> mangled = dlang_type (&type, mangled, info);
> +
> + /* If requested return the return type of a function or type of
> + a variable in postfix notation with space as separator. */
> + if ((info->option & DMGL_RET_POSTFIX) != 0)
> + {
> + string_append (decl, " ");
> + string_appendn (decl, type.b, string_length (&type));
> + }
> +
One concern here is that dlang_parse_mangle is called recursively, so you'll
need to test and unset DMGL_RET_POSTFIX at the beginning of this function,
otherwise you'll get return types popping up within the demangled result, not
just at the end.
For example:
_D4test__T2fnS_DQo7aliasfnFiZiZQvFZv
Should demangle as
test.fn!(test.aliasfn(int)).fn() void
And not
test.fn!(test.aliasfn(int) int).fn() void
> --- a/libiberty/testsuite/d-demangle-expected
> +++ b/libiberty/testsuite/d-demangle-expected
> @@ -1406,3 +1406,87 @@
> std.algorithm.iteration.FilterResult!(std.typecons.Tuple!(int, "a", int, "b",
> in
> --format=dlang
>
> _D3std3uni__T6toCaseS_DQvQt12toLowerIndexFNaNbNiNewZtVii1043S_DQCjQCi10toLowerTabFNaNbNiNemZwSQDo5ascii7toLowerTAyaZQDzFNaNeQmZ14__foreachbody2MFNaNeKmKwZ14__foreachbody3MFNaNeKwZi
> std.uni.toCase!(std.uni.toLowerIndex(dchar), 1043,
> std.uni.toLowerTab(ulong), std.ascii.toLower,
> immutable(char)[]).toCase(immutable(char)[]).__foreachbody2(ref ulong, ref
> dchar).__foreachbody3(ref dchar)
> +#
> +--format=dlang
> +_D3std8datetime9stopwatch9StopWatch6__ctorMFNbNcNiNfEQBz8typecons__T4FlagVAyaa9_6175746f5374617274ZQBfZSQDyQDxQDrQDk
> +std.datetime.stopwatch.StopWatch.this(std.typecons.Flag!("autoStart").Flag)
> +#
> +--format=dlang
> +_D4core4time8Duration__T8opBinaryVAyaa1_2bTSQBqQBoQBmZQBeMxFNaNbNiNfQzZQBc
> +core.time.Duration.opBinary!("+",
> core.time.Duration).opBinary(core.time.Duration) const
> +#
> +--format=dlang
> +_D4core4sync5mutex5Mutex__T12lock_nothrowTOCQBqQBoQBmQBjZQBeMOFNbNiNeZv
> +core.sync.mutex.Mutex.lock_nothrow!(shared(core.sync.mutex.Mutex)).lock_nothrow()
> shared
Shouldn't all these tests be using --ret-postfix? The only example where both
with and without make sense are for symbols with embedded symbols within, to
ensure that their return types aren't emitted, as per the example I gave above.
Regards
Iain
