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

[enh] Issue 8765 - Assert should print expression when no message is present #1203

Closed
wants to merge 11 commits into from
Closed

Conversation

ghost
Copy link

@ghost ghost commented Oct 21, 2012

This makes this code:

assert(1 == 2);

Print:

core.exception.AssertError@test.d(2): assert(1 == 2)

But I can't figure out a way to write a test-case because the test is compilable but it's expected to fail at runtime. I need to compare the runtime message just like diag-style tests in the fail_compilation folder. Any help there is appreciated.

@braddr
Copy link
Member

braddr commented Oct 22, 2012

I like more informative assert output, but I don't think that the .di files should be impacted. Your changes are too early in the lifecycle and instead it should be moved to very late, nearer code generation.

@andralex
Copy link
Member

I think we should also print the failing expression when the error message is present.

@ghost
Copy link
Author

ghost commented Oct 22, 2012

@braddr: Agreed.
@andralex: Good idea, now it's implemented:

int x;
assert(x == 1);
assert(x == 1, "x is not 1")
core.exception.AssertError@test.d(6): assert(x == 1)
core.exception.AssertError@test.d(7): assert(x == 1), "x is not 1"

I think pulling this will avoid having to write assert messages in most cases since they will be much more informative by default.

@ghost
Copy link
Author

ghost commented Oct 22, 2012

I have no idea why these two fail:

http://d.puremagic.com/test-results/pull.ghtml?runid=338676
http://d.puremagic.com/test-results/pull.ghtml?runid=338675

make[1]: *** [generated/linux/debug/64/unittest/std/conv] Error 134
make[1]: *** [generated/osx/debug/32/unittest/std/conv] Abort trap: 6

The tests worked on my win32 machine.

@ghost
Copy link
Author

ghost commented Oct 24, 2012

I'm gonna have to get a vbox machine for a x64 linux system, I don't know why some platforms are failing (mostly x64 + OSX 32bit).

@ghost
Copy link
Author

ghost commented Oct 24, 2012

I can't build Phobos on x64 posix, the build either runs indefinitely or runs out of memory (in vbox). If someone has the time and skill set to figure out what's wrong I'd kindly ask if they could have a look (test report: http://d.puremagic.com/test-results/pull.ghtml?runid=340630), thanks.

@@ -2004,6 +2004,26 @@ elem *AssertExp::toElem(IRState *irs)
Type *t1 = e1->type->toBasetype();

//printf("AssertExp::toElem() %s\n", toChars());
OutBuffer buf;
Expression* msgExpr;
Copy link
Member

Choose a reason for hiding this comment

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

Should be Expression *

Copy link
Member

Choose a reason for hiding this comment

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

And this declaration should be moved down to where it's initialized.

@yebblies
Copy link
Member

It's possible your bug is using OutBuffer::toChars when you should be using OutBuffer::extractData. OutBuffer's destructor will free the buffer.

@ghost
Copy link
Author

ghost commented Oct 27, 2012

Maybe, but it only fails on x64 systems. I'll try your suggestion though.

Edit: Actually wrong it also failed on x32. We'll see what the autotester says soon.

@ghost
Copy link
Author

ghost commented Oct 31, 2012

It seems to almost consistently fail with x64 (-m64) binaries (except OSX which fails all the time).
http://d.puremagic.com/test-results/pull-history.ghtml?repoid=1&pullid=1203

@WalterBright: Sorry to bother you, do you have any idea what could be causing this?

@ghost
Copy link
Author

ghost commented Dec 3, 2012

And now it's working again, how strange.

Let's see if it works again -- restarting tester.

9rnsr and others added 10 commits December 27, 2012 12:49
…re function

`hasMutableIndirectionParams()` is based on the old `TypeFunction::purityLevel()` (before fixing issue 8408). So its result is consistent with `TypeFunction::purityLevel() != PUREstrong` in 2.060. Then it *fixes* the regression.
Issue 9177 - Wrong `alias func this` incorrectly reports error message
Also updated test cases. By a recent change in 2.061 (4b2767e), direct returning of NewExp sometimes avoids immutable implicit conversion. So, test5081 should return mutable array values through local variables.
Issue 9230 - Incorrect implicit immutable conversion occurs in pure function
Issue 9234 - DMD confuses string template parameter with function
@ghost ghost closed this Jan 1, 2013
specified.

Fixes Issue 9255 - Assert needs use file name for the AssertError .file
field.
@ghost ghost reopened this Jan 1, 2013
@ghost ghost closed this Jan 1, 2013
@ghost
Copy link
Author

ghost commented Jan 1, 2013

Uhhhh what is up with Github? I see one thing in git diff head~1 and a completely different diff on Github. (this is after a forced push)

@ghost
Copy link
Author

ghost commented Jan 1, 2013

I'll try to open a new pull request.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants