As a follow-up to

https://github.com/D-Programming-Language/phobos/pull/3207#issuecomment-144073495

I starting digging in DMD for logic controlling behaviour of assert(), especially whether it's possible to add automatic printing of `lhs` and `rhs` upon assertion failure if `AssertExp` is a binary expression say `lhs == rhs`.

After grepping for `AssertExp` the only possible place I could think of was

ToElemVisitor::visit(AssertExp *ae)

inside

elem *toElem(Expression *e, IRState *irs)

in file e2ir.c.

Questions:

1. Is this the right place where this lhs-rhs-printing logic should be added? If so could somebody tell me how to make this happen?

2. Is it possible to from within DMD generate expressions that do

`import std.stdio : write`

and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?

On Tuesday, 29 September 2015 at 21:02:42 UTC, Nordlöw wrote:
> As a follow-up to
>
> https://github.com/D-Programming-Language/phobos/pull/3207#issuecomment-144073495
>
> I starting digging in DMD for logic controlling behaviour of assert(), especially whether it's possible to add automatic printing of `lhs` and `rhs` upon assertion failure if `AssertExp` is a binary expression say `lhs == rhs`.
>
> After grepping for `AssertExp` the only possible place I could think of was
>
> ToElemVisitor::visit(AssertExp *ae)
>
> inside
>
> elem *toElem(Expression *e, IRState *irs)
>
> in file e2ir.c.
>
> Questions:
>
> 1. Is this the right place where this lhs-rhs-printing logic should be added? If so could somebody tell me how to make this happen?
>
> 2. Is it possible to from within DMD generate expressions that do
>
> `import std.stdio : write`
>
> and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?

Someone will write something like this:

assert(plainPassword == plainPassword.toLower());

and plaintext passwords will end up in stderr.

On Tue, Sep 29, 2015 at 09:02:40PM +0000, Nordlöw via Digitalmars-d wrote: [...]
> 2. Is it possible to from within DMD generate expressions that do
> 
> `import std.stdio : write`
> 
> and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?

I dunno about right or wrong approaches, but something like this would make the compiler dependent on Phobos, which is a big no-no for those embedded OS people who need to be able to plug in their own runtime without being forced to implement what Phobos implements, which may lead to prohibitive bloat for embedded software.

The most that the compiler can depend on is druntime, which, fortunately, does depend on the C library (so far), so perhaps you could get away with `import std.c.stdio : printf;` and using C-style printf() calls to print the output. The existing stacktrace dump functions may already be using printf from the C library, and the standard C library seems common enough even on embedded platforms that you can probably get away with it.

Alternatively, maybe what you really want is for the compiler to emit a call to a druntime function that does the printing, then the people who need to replace druntime with their own lightweight version can simply implement this function, and things would Just Work.


T

-- 
Любишь кататься - люби и саночки возить.

On Tue, Sep 29, 2015 at 09:13:55PM +0000, John Colvin via Digitalmars-d wrote:
> On Tuesday, 29 September 2015 at 21:02:42 UTC, Nordlöw wrote:
[...]
> >2. Is it possible to from within DMD generate expressions that do
> >
> >`import std.stdio : write`
> >
> >and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?
> 
> Someone will write something like this:
> 
> assert(plainPassword == plainPassword.toLower());
> 
> and plaintext passwords will end up in stderr.

That's an instance of assert abuse, and we probably don't have to be responsible for it.  Such checking belongs in enforce, or an explicit check with an exception throw, NOT an assert.


T

-- 
If blunt statements had a point, they wouldn't be blunt...

On Tuesday, 29 September 2015 at 21:22:43 UTC, H. S. Teoh wrote:
> On Tue, Sep 29, 2015 at 09:13:55PM +0000, John Colvin via Digitalmars-d wrote:
>> On Tuesday, 29 September 2015 at 21:02:42 UTC, Nordlöw wrote:
> [...]
>> >2. Is it possible to from within DMD generate expressions that do
>> >
>> >`import std.stdio : write`
>> >
>> >and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?
>> 
>> Someone will write something like this:
>> 
>> assert(plainPassword == plainPassword.toLower());
>> 
>> and plaintext passwords will end up in stderr.
>
> That's an instance of assert abuse, and we probably don't have to be responsible for it.  Such checking belongs in enforce, or an explicit check with an exception throw, NOT an assert.
>
>
> T

Not necessarily. It could just be a defensive assert for something that should already have been verified/cleaned/caught earlier.

auto pass = getPassword();
pass.clean();
assert(pass == pass.toLower());
//and on we go ...

On Tuesday, 29 September 2015 at 21:26:00 UTC, John Colvin wrote:
> On Tuesday, 29 September 2015 at 21:22:43 UTC, H. S. Teoh wrote:
>> On Tue, Sep 29, 2015 at 09:13:55PM +0000, John Colvin via Digitalmars-d wrote:
>>> On Tuesday, 29 September 2015 at 21:02:42 UTC, Nordlöw wrote:
>> [...]
>>> >2. Is it possible to from within DMD generate expressions that do
>>> >
>>> >`import std.stdio : write`
>>> >
>>> >and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?
>>> 
>>> Someone will write something like this:
>>> 
>>> assert(plainPassword == plainPassword.toLower());
>>> 
>>> and plaintext passwords will end up in stderr.
>>
>> That's an instance of assert abuse, and we probably don't have to be responsible for it.  Such checking belongs in enforce, or an explicit check with an exception throw, NOT an assert.
>>
>>
>> T
>
> Not necessarily. It could just be a defensive assert for something that should already have been verified/cleaned/caught earlier.
>
> auto pass = getPassword();
> pass.clean();
> assert(pass == pass.toLower());
> //and on we go ...

It could even be in an `in { }` block

On 9/29/15, John Colvin via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> Someone will write something like this:
>
> assert(plainPassword == plainPassword.toLower());
>
> and plaintext passwords will end up in stderr.

If you have plaintext passwords stored anywhere you are already screwed. ;)

On Tue, Sep 29, 2015 at 11:32:14PM +0200, Andrej Mitrovic via Digitalmars-d wrote:
> On 9/29/15, John Colvin via Digitalmars-d <digitalmars-d@puremagic.com> wrote:
> > Someone will write something like this:
> >
> > assert(plainPassword == plainPassword.toLower());
> >
> > and plaintext passwords will end up in stderr.
> 
> If you have plaintext passwords stored anywhere you are already screwed. ;)

Point. :-)


T

-- 
There is no gravity. The earth sucks.

On Tuesday, 29 September 2015 at 21:02:42 UTC, Nordlöw wrote:
> As a follow-up to
>
> https://github.com/D-Programming-Language/phobos/pull/3207#issuecomment-144073495
>
> I starting digging in DMD for logic controlling behaviour of assert(), especially whether it's possible to add automatic printing of `lhs` and `rhs` upon assertion failure if `AssertExp` is a binary expression say `lhs == rhs`.
>
> After grepping for `AssertExp` the only possible place I could think of was
>
> ToElemVisitor::visit(AssertExp *ae)
>
> inside
>
> elem *toElem(Expression *e, IRState *irs)
>
> in file e2ir.c.
>
> Questions:
>
> 1. Is this the right place where this lhs-rhs-printing logic should be added? If so could somebody tell me how to make this happen?
>
> 2. Is it possible to from within DMD generate expressions that do
>
> `import std.stdio : write`
>
> and then calls write on the `lhs` and `rsh`...or this a completely wrong approach to solving this problem?

Several years ago, I proposed a really nice function called assertPred which did all this kind of stuff for you, and it supported a bunch of different operations - e.g. assertPred!"=="(a, b), assertPred!"+=(a, b, result), assertPred!"opCmp"(a, b). But it was ultimately rejected with the decision being that we'd improve assert to do this instead (which lost out on some of the fancier overloads of assertPred but would be great for the common case). However, after some work was actually done to implement that, it was finally decided that it was too expensive to put it into assert:

https://issues.dlang.org/show_bug.cgi?id=5547

So, the proposed alternative to assertPred which got it rejected ultimately got rejected itself.

But as great as assertPred was, it turns out that turning all of your checks in unit tests into templated functions rather than just using assert, results in a lot of template bloat, and unit tests require more memory and take longer to compile. So, I've pretty much gotten to the point where I think that it's just simplest to use assert as-is and then go back and tweak your unit test to print out the information you need if there's a failure. Any information in a unit test that isn't reproducible should already have been being printed on failure anyway (e.g. the seed value for the random number generator), so it really shouldn't be a big deal to tweak a test to print out what's going on. As nice as it might be to have the assertion print out better information, I don't think that it's worth adding a function just for that. It's just too expensive in terms of compilation time and memory.

- Jonathan M Davis

On 09/29/2015 05:02 PM, Nordlöw wrote:
> As a follow-up to
>
> https://github.com/D-Programming-Language/phobos/pull/3207#issuecomment-144073495
>
>
> I starting digging in DMD for logic controlling behaviour of assert(),

Great idea. While at it could you please make sure the format is compatible with that of compile-time errors? Errors during compilation and unit testing are close in the edit-build-test continuum. Thanks! -- Andrei