On Aug 18, 2010, at 11:30 PM, Lars Tandle Kyllingstad wrote:

> On Wed, 2010-08-18 at 20:09 -0400, Michel Fortin wrote:
>> Le 2010-08-18 ? 19:08, Jonathan M Davis a ?crit :
>> 
>>> The ideal situation would be to use asserts in all cases where it's going to be bugs in the program rather than bad user input and that they go away in release mode.
>> 
>> I'd say the ideal situation would be to have the 'in' contracts checked on the caller's side. That way a program compiled in debug mode would still check the input it sends to Phobos, and whether Phobos was compiled in debug or release mode would only affect its internal checks, not the checks about its inputs.
> 
> I wonder if it would be feasible to have two versions of the Phobos lib, libphobos2.a and libphobos2-debug.a, distributed with DMD.  The former would be linked in when you compile with -release, otherwise the latter would be used.  The Phobos makefile already builds both versions, so it would just be a small compiler change.

The compiler already has -defaultlib and -debuglib flags also.  But what's really needed are checked and non-checked libraries (ie. no flags and -release) not... something and -debug.  It's been a while, but I think -debuglib applies to appls built with -debug set.

Of course it would be necessary to add them to the config file, but that should be easier than changing the compiler.?

On 19 Aug, 2010,at 03:49 PM, Simen Kjaeraas <simen.kjaras at gmail.com> wrote:

> Jacob <doob at me.com> wrote:
>
> > That would be the -defaultlib and -debuglib options if I'm not misstaken.
>
> Not quite. -debuglib is only chosen if used with -g. Also, unless sc.ini/dmd.conf gets -debuglib=phobos-debug added to DFLAGS, it's not gonna matter all that much, as it won't be standard.
>
> -- 
> Simen
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/phobos/attachments/20100819/a0462fca/attachment.html>

This is only true for instantiations that aren't invoked in the std lib. Any future instantiations that match ones in the std lib will use the version compiled into Phobos. It makes for very inconsistent results. I personally think that until enforce doesn't prevent inlining it should not be used anywhere in ranges since much of the range concept's charter is for inlining performance advantages.

Sent from my iPhone

On Aug 18, 2010, at 7:36 PM, David Simcha <dsimcha at gmail.com> wrote:

> A valid criticism in general.  However, all of the ranges in question are templates, so each instantiation is compiled in whatever mode your program is compiled in.
> 
> On 8/18/2010 7:08 PM, Jonathan M Davis wrote:
>> The ideal situation would be to use asserts in all cases where it's going to be bugs in the program rather than bad user input and that they go away in release mode. The problem is the fact that Phobos is compiled as an external library rather than being compiled in with your code when you compile. And since there's no way (at least no standard way) to have a release and debug version of the Phobos libraries, with the release version being used in release mode and the debug version being used in debug mode, that pretty much means that you _always_ compile your code with the release version of Phobos. This makes assert _useless_ for users of the library. If you want checks beyond within Phobos code, you _have_ to use enforce(), and since enforce has issues with inlining and it doesn't go away in release mode, it's like forcing a portion of Phobos into debug mode.
>> 
>> _Ideally_, we would find some sort of standard way to make Phobos used in debug mode when compiling in debug mode and release mode when compiling in release mode. That way, asserts could be used in a lot of these cases since a lot of them really shouldn't be checked in release mode. Personally, I _really_ don't like the fact that the libraries aren't necessarily in the same mode as your code, though I'm not sure that there's a good way to deal with that other than having some standard naming scheme for debug vs release libraries which Phobos uses and dmd understands so that you get a debug version when compiling your code in debug mode, and you get a release version when compiling in release mode. Barring that however...
>> 
>> You'd almost have to take it on a case by case basis. In some cases, you're going to need checks and it would be bad not to have them, while in others, it would just cost too much to leave them in. It's a tough call. If anything though, I think that it's a prime example of needing to find a better way to deal with debug vs release and libraries. If we had a standard way to have both available, then dmd could use both with user code, and then we can use asserts like they should be used. As it stands though, we're probably going to have to use enforce() where we _need_ checks for safety and asserts otherwise. But since using asserts is almost pointless.... Bleh. It's just ugly.
>> 
>> - Jonathan M Davis
>> _______________________________________________
>> phobos mailing list
>> phobos at puremagic.com
>> http://lists.puremagic.com/mailman/listinfo/phobos
>> 
>> 
> 
> _______________________________________________
> phobos mailing list
> phobos at puremagic.com
> http://lists.puremagic.com/mailman/listinfo/phobos

On 8/20/2010 12:39 AM, Steven Schveighoffer wrote:
> This is only true for instantiations that aren't invoked in the std lib. Any future instantiations that match ones in the std lib will use the version compiled into Phobos. It makes for very inconsistent results. I personally think that until enforce doesn't prevent inlining it should not be used anywhere in ranges since much of the range concept's charter is for inlining performance advantages.
>
> 

And ranges will likely never stop affecting inlining.  It's not just a DMD optimizer sucking issue.  It's also the amount of code an enforce() call generates.  Read the disassembly to one and you'll see what I'm talking about.

It seems like the consensus here is that, for range primitives, assert() is the way to go.  I'd like to commit the changes in time for the next release, but I'd like to have Andrei's nod first b/c he's been quiet throughout this discussion and he's the main designer of std.range and std.algorithm.

On 8/18/2010 6:49 PM, David Simcha wrote:
> While I was debugging std.range and std.algorithm I noticed that there's tons of inconsistency about when enforce() vs. assert() is used for checking input to range primitives.  I think this needs to be made consistent, and there are good arguments for both.
>
> enforce() is arguably the way to go because we're dealing with an external API, not internal consistency checks.  According to the guidelines in TDPL input external to an API should be checked even in release mode.
>
> On the other hand, assert() is arguably the way to go because ranges are supposed to be a very efficient abstraction.  Having enforce() in range primitives that look like they should be very fast, inlined functions is just bad practice.  For example, some benchmarks I did awhile back showed that the use of enforce() and its effect on inlining can make using ranges slower than using opApply. Furthermore, if you're stacking several higher-order ranges, i.e. filter!"a > 0"(map!"a * a"(retro(someArray))) the checks may be redundant.  Lastly, since arrays are the canonical range, omitting release mode checks in library-defined ranges would make them consistent with arrays.
>
> Generally when I was debugging I tried to be locally consistent, i.e. use whatever was used elsewhere in the same range.  This meant being globally inconsistent.  Now that I've thought about it, my vote is for using assert() consistently for range primitives.  If ranges are filled with tons of safety checks that have significant performance costs and can't be disabled even in release mode, they will have failed in their goal of providing a lightweight, efficient abstraction.  enforce() can be used for things that are intended to be called much less frequently, like constructors.  Comments?

On 08/21/2010 01:52 PM, David Simcha wrote:
> It seems like the consensus here is that, for range primitives, assert() is the way to go. I'd like to commit the changes in time for the next release, but I'd like to have Andrei's nod first b/c he's been quiet throughout this discussion and he's the main designer of std.range and std.algorithm.

Thanks - I feel like a mobster who's being given respect :o). I thought of this a lot, and talked with Walter as well. Will get back with an opinion today.

Andrei

On 8/21/2010 2:29 PM, Andrei Alexandrescu wrote:
> On 08/21/2010 01:52 PM, David Simcha wrote:
>> It seems like the consensus here is that, for range primitives, assert() is the way to go. I'd like to commit the changes in time for the next release, but I'd like to have Andrei's nod first b/c he's been quiet throughout this discussion and he's the main designer of std.range and std.algorithm.
> 
> Thanks - I feel like a mobster who's being given respect :o). I thought of this a lot, and talked with Walter as well. Will get back with an opinion today.
> 
> Andrei

The part I dislike the most is that this decision is being heavily influenced by a dmd quality of implementation issue.  Primarily, the inlinability of enforce vs assert.

Brad Roberts wrote:
>
>
> The part I dislike the most is that this decision is being heavily influenced by a dmd quality of implementation issue.  Primarily, the inlinability of enforce vs assert.
>
> 

It should be assert, regardless.

An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/phobos/attachments/20100821/4d7f969b/attachment.html>

On Saturday 21 August 2010 16:34:02 David Simcha wrote:
>  I disagree for two reasons:
> 
>  1.  enforce() generates so much code that it's likely that very few
> compilers would inline it, and then also inline the function calling it.

I thought that the issue wasn't so much that enforce() isn't inlined as that a function which called enforce() isn't inlined.

- Jonathan M Davis