Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 8765 - Assert should show failing expression. + Issue 9255. #1426

Closed
wants to merge 1 commit into from
Closed

Issue 8765 - Assert should show failing expression. + Issue 9255. #1426

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 1, 2013

#1203 had to be closed because Github has some bugs with rebasing. Unlike the previous pull this one has unittests.

Fixes Issue 8765 - Assert should print failing expression.
Fixes Issue 9255 - Assert should print file name rather than module name, to be consistent with other Exception types.

Note that fixing 9255 is automatic after Fixing 8765 due to the change in code paths (a message now always exists and therefore the if statement is always taken).

@ghost ghost closed this Jan 2, 2013
@ghost ghost reopened this Jan 5, 2013
@ghost ghost closed this Jan 5, 2013
@ghost
Copy link
Author

ghost commented Jan 7, 2013

@9rnsr: Can I ask you for some advice? Edit: Removed long comment, issue is solved now.

@9rnsr
Copy link
Contributor

9rnsr commented Jan 8, 2013

@AndrejMitrovic At least, all semantic processing should be done before entering glue layer. So calling Expression::semanticwith sc == NULL is logically incorrect in basic
Therefore you should make custom messages in AssertExp::semantic.

@ghost
Copy link
Author

ghost commented Jan 8, 2013

Of course! I have to remind myself always to do this sort of thing in the front-end and never in the middle layer or backend. Thanks for the help Kenji.

@dnadlinger
Copy link
Member

As a bonus, things will also be available in GDC/LDC without extra work necessary. ;)

@ghost ghost reopened this Jan 8, 2013
@ghost
Copy link
Author

ghost commented Jan 8, 2013

Ok let's see what the tester says, it ran ok on my machine.

@ghost
Copy link
Author

ghost commented Jan 13, 2013

I've had these 64bit failures with my last pull #1203, where after a few days the autotester went green again, and then back to red. I really don't know what's causing it.

@ghost
Copy link
Author

ghost commented Nov 24, 2013

Well, after 11 months I've rebased it. Let's see if it works.

msg = resolveProperties(sc, msg);
msg = msg->implicitCastTo(sc, Type::tchar->constOf()->arrayOf());
msg = msg->optimize(WANTvalue);
if (msg->isLvalue()) // runtime value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, to tell if it's a known compile-time value call optimize then check op == TOKstring.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@ghost
Copy link
Author

ghost commented Nov 26, 2013

Hmm still no word from autotester, I guess it's still down?

@yebblies
Copy link
Member

No, the autotester is running.

@braddr
Copy link
Member

braddr commented Nov 26, 2013

It's running, but the rate of new pulls and merges to master are high enough that the tester fleet is not able to make much headway into the backlog of things to test. More hardware would help. A shift of focus from creating pulls to merging pulls for a while would help too. Changing how the builds are prioritized would also help.

@ghost
Copy link
Author

ghost commented Nov 27, 2013

I seem to be getting the same x64 failures as almost a year ago. https://d.puremagic.com/test-results/pull-history.ghtml?projectid=1&repoid=1&pullid=1426

@yebblies: Any idea what could be causing it?

@yebblies
Copy link
Member

That's very strange. Does it still fail without the e2ir change?

}
else // no message -> use assert expression as msg
{
OutBuffer buf;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dance with the OutBuffer is just mem.strdup(toChars())

@ghost
Copy link
Author

ghost commented Nov 27, 2013

That's very strange. Does it still fail without the e2ir change?

I'll add a test-commit for the autotester.

@ghost
Copy link
Author

ghost commented Nov 27, 2013

Nope, still fails.

msg = msg->semantic(sc);
msg = resolveProperties(sc, msg);
msg = msg->implicitCastTo(sc, Type::tchar->constOf()->arrayOf());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe this line. You should move this to after the last call to ensureValidMessage. I'm just guessing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, it's inside ensureValidMessage. Scratch that.

8765 - Assert should print failing expression.
9255 - Assert should print file name rather than module name, to be
consistent with other Exception types.
@andralex
Copy link
Member

yah this would be nice!

@nordlow
Copy link
Contributor

nordlow commented Apr 21, 2014

Is anybody working on printing sub-expression values of failing asserts aswell?

For details see my last post at: https://issues.dlang.org/show_bug.cgi?id=12480

@Safety0ff
Copy link
Contributor

I updated the phobos bitmanip unittests and sadly this still fails in std.numeric.

@quickfur
Copy link
Member

ping

@andralex
Copy link
Member

ping

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
9 participants