Rewrite core.demangle to avoid exception handling for normal control flow

[denizzzka]

Description

core.demangle uses exception handling for normal control flow: https://issues.dlang.org/show_bug.cgi?id=15504

What are rough milestones of this project?

Remove exception handling from normal control flow.

How does this project help the D community?

Code will be more quality. Also I suspect what current implementation masks memory overuse error. More info in comments: https://github.com/dlang/druntime/pull/3262

Recommended skills

D, make

(If applicable, e.g. GSoC/SAoC)

What can students expect to get out of doing this project?

Experience in code refactoring.

Point of Contact

## References

[ibuclaw]

I have no problems with people using the implementation in the liberty library, even if they it's only as a point of reference/for inspiration.

https://github.com/gcc-mirror/gcc/blob/master/libiberty/d-demangle.c

[ljmf00]

@wilzbach is this applicable to GSoC 2021?

[wilzbach]

I don't think it would be a good fit as the time for GSoC is three months whereas this looks like it would be doable in a few days at most.

[ljmf00]

I don't think it would be a good fit as the time for GSoC is three months whereas this looks like it would be doable in a few days at most.

Yeah, I didn't know the complexity of it. I'll maybe take a look at this. Apart from the spec, there's anything else that could be helpful for this?

[maxhaton]

I am currently having a stab at this by trying to transition it to a simple Maybe monad. The transition is more difficult than it seems but nonetheless tractable - the main issue isn't really the use of exceptions but rather that the exceptions are used in a really stupid way: Again, a solvable problem but this isn't really refactoring as much as fundamentally altering the control flow of the code (if the aim is to make it nice rather than merely nothrow)

The code has rotted pretty badly unfortunately however in line with the other comments if this were to be an SAOC (Which I would probably vote against since it's not all that important as it currently works) the task would have to be rewriting it entirely to be generally a bit nicer (even a translation of the GNU implementation)

[ljmf00]

I'm currently doing a rewrite based on both GNU implementation and DRuntime. I found some things that are not specified on the spec. I'm not sure if the parser is wrong, the spec is wrong or I'm wrong.

https://github.com/dlang/druntime/blob/master/src/core/demangle.d#L1241

I think the grammar doesn't specify this particular case. If I'm right, I can do a PR to fix it.

I would say that spec could be more explicit about back referencing and how it works here, but I managed to understand it with the existing code.

Anyway, I came up with a simple way to handle the errors and should make the code way more maintainable and a better control flow.

[maxhaton]

What are you doing with the Exceptions? (remember that they aren't actually errors)

Also, I recommend just copying the GNU one as the druntime implementation is just old rotted and rubbish.

[Geod24]

https://github.com/dlang/druntime/blob/728f1d9c3b7a37eba4d59ee2637fb924053cba6d/src/core/demangle.d#L1241

(Protip: Press y to get a permalink)

This one is actually a bit more complicated. Currently, in ref is valid. When -preview=in is used, in ref is an error, but the compiler will still encode in where ref is used as in ref (to avoid mismatches in ABI).

[maxhaton]

https://github1s.com/dlang/druntime/blob/728f1d9c3b7a37eba4d59ee2637fb924053cba6d/src/core/demangle.d add 1s for a sometimes useful way of jumping about on a computer without a proper editor

[ljmf00]

What are you doing with the Exceptions? (remember that they aren't actually errors)

I'm getting rid of Exception entirely. The public API is nothrow, so I'm using the function return to deal with errors, like in C.

Also, I recommend just copying the GNU one as the druntime implementation is just old rotted and rubbish.

Yes, I'm just based on the behavior, not the codebase itself. For example, I spotted the problem related below because of that. Also, there's somethings on GNU implementation that are removed in the current druntime implementation because it's deprecated. For example, there's no support for extern(Pascal) on the druntime, because it was removed. I can add it, but I do believe its not the intention, right?

dlang/druntime@728f1d9/src/core/demangle.d#L1241

(Protip: Press y to get a permalink)

Thanks :)

This one is actually a bit more complicated. Currently, in ref is valid. When -preview=in is used, in ref is an error, but the compiler will still encode in where ref is used as in ref (to avoid mismatches in ABI).

Ok. So I should add it anyway. It's not in the spec for some reason? AFAIK, preview stuff is on the spec.

[maxhaton]

What are you doing with the Exceptions? (remember that they aren't actually errors)

I'm getting rid of Exception entirely. The public API is nothrow, so I'm using the function return to deal with errors, like in C.

Also, I recommend just copying the GNU one as the druntime implementation is just old rotted and rubbish.

Yes, I'm just based on the behavior, not the codebase itself. For example, I spotted the problem related below because of that. Also, there's somethings on GNU implementation that are removed in the current druntime implementation because it's deprecated. For example, there's no support for extern(Pascal) on the druntime, because it was removed. I can add it, but I do believe its not the intention, right?

dlang/druntime@728f1d9/src/core/demangle.d#L1241

(Protip: Press y to get a permalink)

Thanks :)

This one is actually a bit more complicated. Currently, in ref is valid. When -preview=in is used, in ref is an error, but the compiler will still encode in where ref is used as in ref (to avoid mismatches in ABI).

Ok. So I should add it anyway. It's not in the spec for some reason? AFAIK, preview stuff is on the spec.

Quite a few of the functions that throw also return values on success

[ljmf00]

Quite a few of the functions that throw also return values on success

Yes. The C convention for error handling is perfect here. I marked the parameters as out and return values are just for success or failure. Everything I need to write on, I just use references.

[maxhaton]

DemangleMaybe!size_t decodeNumber( scope const(char)[] num ) scope nothrow
{
    debug(trace) printf( "decodeNumber+\n" );
    debug(trace) scope(success) printf( "decodeNumber-\n" );

    size_t val = 0;

    foreach ( c; num )
    {
        import core.checkedint : mulu, addu;

        bool overflow = false;
        val = mulu(val, 10, overflow);
        val = addu(val, c - '0',  overflow);
        if (overflow)
            return typeof(return).error();
    }
    return typeof(return).valid(val);
}

I can't be bothered to finish it now, but I just wrote a Maybe type to handle it for me. Just be careful because currently the exceptions are basically being used to unwind the entire state machine, so you need to check every function call a throwing function makes then return immediately if it fails.

[RazvanN7]

@ljmf00 any progres on this?

[ljmf00]

I haven't dedicated much time on this, but I have some demangling working without exceptions here: https://github.com/ljmf00/demangler/tree/basic-implementation

[RazvanN7]

I don't think that there is enough material for this idea to be considered a project proposal. It is a bug report and should be treated as such. In the interest of cleaning this list, I'm going to close this project.

[maxhaton]

Writing a full demangler for D is definitely a project, what more do you want?

[ljmf00]

:eyes: