CTFE improved error handling + ref param bug 6934 #522
Conversation
Minor simplification. Previously NULL was used to mean failure, and was also used for void return. It now returns EXP_CANT_INTERPRET on failure, and EXP_VOID_INTERPRET if void. Builtins were already using this convention.
'cantInterpret' doesn't do anything useful any more.
CTFE and templates both have backtraces which print directly to stderr. Add a new type of error message 'errorSupplemental', and use it for these cases where the message is supplementary information for a previously reported error, rather than a new error.
If CTFE fails inside a static assert, the static assert error is just giving context.
Errors in functionParameters should cause the whole CallExp to return an ErrorExp.
Need to use rvalue interpreting of the array, not lvalue, if getting the length.
It just wasn't interpreting the 'case'.
The semantic passes aren't yet robust enough to allow CTFE to continue in all cases, but this change removes a third of the errors, and makes it more comprehensible.
Turn it from segfault into rejects-valid.
Doesn't do anything yet.
Binary only (refcount is only 0 or 1). This is enough to drastically reduce the number of times array and struct literals are interpreted.
It was copying literals too many times. This also gives an improvement to speed and memory consumption.
Use ref counting to prevent modification of string literals, even on D1.
|
I think the refCount should be in Expression, not distributed around other nodes. I also don't see where it is incremented and decremented. Should it get an addRef() and Release() ? |
|
I found where it was inc/dec'd. But I think the inc and dec should be encapsulated in a proper addRef() and Release(). I also think that conflating the reference count with "has it been interpreted yet" is a mistake. The idea is that new Expression() should do the addRef() inside the constructor, and then a Release() should free the Expression. The CTFE can do the Release()'s on the ones it allocates, the rest can just harmlessly leak for now. |
|
I found where it was inc/dec'd. But I think the inc and dec should be encapsulated in a proper addRef() and Release().
The idea is that new Expression() should do the addRef() inside the constructor, and then a Release() should free the Expression. The CTFE can do the Release()'s on the ones it allocates, the rest can just harmlessly leak for now. |
What I called 'refcount' is really an ownership flag. Proper refcount will be added later.
|
In reality what I was calling 'ctfeRefCount' was really just an ownership flag, "have all elements been interpreted yet?", needed only for aggregate types. I've changed the name, and reset all the flags when CTFE completes. |
CTFE improved error handling + ref param bug 6934
|
Oddly, this works on D1 on Windows, but fails on D1 on Linux with: test42.d(3427): Error: CTFE fails for structs with destructors (Bugzilla 6933) --- errorlevel 256 |
|
I checked in the D1 merge; could you please look into why it fails for D1. |
|
I think I found it; testing now. |
|
valgrind for the win! |
Additional fix for pull #522
Do not call `getcwd` (or other `base`) in `absolutePath` if not needed.
Unfortunately this pull request is a bit complicated because one bug (6934) involved multiple problems.
There are two parts in this pull request.
(1) a cleanup to the CTFE backtraces.
In the backtrace for both CTFE and templates, it was writing directly to stderr.
I added a new error message type "errorSupplemental" to standardize this.
I made progress to fixing 6984 "CTFE generates a torrent of spurious errors, if there was a previous error" by improving the CTFE error handling in funcDeclaration::interpret and CallExp, and a improved error handling in static assert.
(2) initial refcounting for aggregate types (string, array, AA, and struct literals)
I had to do this to fix the serious bug 6934. It also gives a minor improvement to speed and memory consumption, even though it never releases any memory.
Bugs fixed:
6934 [CTFE] can't use $ in a slice of an array passed by ref
6964 Error message with __error: static assert(undefined+1)
6985 [CTFE] Non-constant case expressions can't be interpreted
Also mitigates bug 6933 from segfault without line number, into rejects-valid.