On 11/18/13 12:30 PM, Jonathan M Davis wrote:
> On Monday, November 18, 2013 07:56:15 Andrei Alexandrescu wrote:
>> On 11/18/13 12:59 AM, Jacob Carlborg wrote:
>>> I like the empty newlines as well. Otherwise it feels like reading a
>>> text without paragraphs.
>>
>> But three newlines for four lines? That's not paragraphs, it's a grocery
>> list.
>
> Yeah. It falls apart on some level when there are so few lines. It's just that
> it's also ugly to mush them all together. So, to me at least, it seems like
> it's ugly either way, and in the past, I've just stuck with my normal pattern
> of having the blank lines when that's the case. Now (in part, thanks to your
> complaints about it), I'm more likely to mush the lines together when putting
> the blank lines would result in a bunch of one liners. But I don't really feel
> like there's a good solution when the function is so short.

Well you must agree at some level that since you don't like short functions with or without newlines inserted, the set of options is considerably reduced.

1. You search for an editor that offers half lines.

2. You shun short functions altogether.

3. You adjust your style.

Style is not something immutable. Paradoxically, when I was younger I used to see it as less of a variable than now. Improving and adapting one's style to various contexts is a lifelong and beneficial process.


Andrei

On 11/18/2013 2:16 AM, Daniel Murphy wrote:
> Yeah.  On the other hand, if we decide assert(0) means 'assume unreachable'
> we can optimize out the try-catch in release mode, among other things.
>
> try { s } catch { assert(0); } -> s
> if (e) assert(0); else s; -> e; s;

I seriously object to this, as assert(0) is there for when you really do want an assert in release builds.

I.e.:

    assert(!e);         // removed in release builds
    if (e) assert(0);   // stays in release builds

It's a valid D idiom, not a bug or oversight.

On 11/18/2013 4:23 AM, Jacob Carlborg wrote:
> Does all architectures support the HLT instruction or equivalent? The spec
> explicitly says HLT is used on x86.

Most architectures I've used supported a HLT, and there's always something the back end could construct if it had to, even:

     ...
     call halt
     ...

On 2013-11-18 16:56, Andrei Alexandrescu wrote:

> But three newlines for four lines? That's not paragraphs, it's a grocery
> list.

As Jonathan said, this example is quite bad in the empty lines vs code lines ratio.

But I general I do put an empty newline before and after each statement.

BTW, I don't have any empty newlines in my grocery list :) . It's more of bullet point list. Or I use an app on my phone which handles all that for me.

-- 
/Jacob Carlborg

On 2013-11-18 20:44, Andrei Alexandrescu wrote:

> I thought about this some more. So std.datetime accounts for > 1/3 of
> all try statements in Phobos. (It also holds a number of other odd
> records - talking from allegations: largest module in source code,
> requires most memory to build for unittest, most unittest lines per line
> of working code. It's great that these are being worked on.) That must
> mean something. The question is what. It may as well mean that the rest
> of Phobos is sloppy and does not add nothrow in places where it should,
> which would make the rest of it insert a bunch of try/catch in the same
> pattern as std.datetime. An argument based on sense and sensibility
> would suggest that something ought to be done about that idiom in
> std.datetime. But the argument "the more nothrow the merrier" is also
> sensible.

I really don't understand the point in having some functions thrown and others assert.

-- 
/Jacob Carlborg

On 2013-11-18 21:49, Walter Bright wrote:

> Most architectures I've used supported a HLT, and there's always
> something the back end could construct if it had to, even:
>
>       ...
>       call halt
>       ...

In that case, would we want it to be legal to throw AssertError instead of using the halt instructions? Because that's what the docs says.

-- 
/Jacob Carlborg

On Monday, November 18, 2013 11:44:46 Andrei Alexandrescu wrote:
> That 7 issues with as many try/catch instances, i.e. 13% of the total 54. You may think I cherry-picked them; in fact, they are the FIRST 7 instances I found by simply searching the file for 'try'. I have no reason to believe I won't find similar issues with most or all of them.
> 
> I think std.datetime needs a pass with an eye for this idiom and obviating it wherever possible.

Clearly, I need to take a look at this. I don't think that I ever made an attempt to avoid the try-catch-assert(0) idiom. I used exceptions where they made sense and then used try-catch-assert(0) where it was required in order to mark functions as nothrow when I knew that they logically could/should be nothrow. On the whole, I think that the implementaton of std.datetime is good, but my primary focus was always on the API, and it was a lot of code for people to review, so it's not entirely surprising if some things could/should be improved. There were a _lot_ of improvements thanks to the review process, but that doesn't mean that everything was caught. It's also the case that we've all learned quite a bit in the years since, and the compiler and libraries have improved as well, changing what does and doesn't work in some cases (e.g. any function with format is currently impure in std.datetime, but it now may be able to be pure). So, it should now be possible to improve some of std.datetime's implementation beyond what we could do when it was first introduced.

Once I've finished splitting std.datetime (which is mostly done, but I've been too busy lately to finish quite yet), I'll look into reducing the number of instances try-catch-assert(0) in std.datetime as well as looking for any other implementation improvements which can be made. And that's one thing that the plethora of unit tests makes easier, because the risk of breaking stuff and not catching it is fairly low (in fact, Martin Nowak made improvements to a function in core.time recently and expressed that he was glad for how much easier it made it to catch mistakes in his refactor because of how thorough the tests were).

- Jonathan M Davis

On Monday, November 18, 2013 12:18:31 Walter Bright wrote:
> I agree. But I'll add that it would be even better to redesign FracSec.from so it never throws at all - have it assert(0) for invalid values being passed to it.
> 
> Exceptions should not be thrown for invalid arguments. Data validation should be handled separately.

I think that that depends. In some cases, it would just mean duplicating work if the API expected the caller to verify the arguments' validity first. A prime example of this would be functions which parse strings. It would be highly inefficient for a function like fromISOExtString to assert instead of enforcing. The caller would have to do the exact work that the function has to do in order to do the check, so it's just wasteful to expect the caller to ensure the validity of the input.

Constructors for types like SysTime or DateTime are in a bit more of a grey area in that in most cases what's passed is likely to be valid - e.g. hopefully, when someone gives a date, they have some clue that the date is valid ahead of time. However, there are cases where the caller really has no way of knowing ahead of time whether a date is going to be valid or not - e.g. is February 29th a valid date in year X? A function could be provided to check the validity ahead of time, but that puts a greater burden on the caller and ends up duplicating the checks. It also would have created more code duplication in std.datetime in some places, because the checks would have to be put in more places rather than having them in a centralized location (e.g. Date's constructor can check the validity of the values, and DateTime can just take advantage of that rather than having to duplicate the check). I do acknowledge however that it's a grey area.

In general, std.datetime takes a defensive programming approach rather than the DbC approach with the idea that it's often likely that the input is going to be originate from a user or file and that many of the checks have to be done regardless, so asking the programmer to do them just adds overhead. In some cases, it arguably should have used DbC, but there were enough places that it should have been using defensive programming already that it was more consistent to use defensive programming in any of the grey areas.

- Jonathan M Davis

On Monday, November 18, 2013 12:47:32 Walter Bright wrote:
> On 11/18/2013 2:16 AM, Daniel Murphy wrote:
> > Yeah. On the other hand, if we decide assert(0) means 'assume
> > unreachable'
> > we can optimize out the try-catch in release mode, among other things.
> > 
> > try { s } catch { assert(0); } -> s
> > if (e) assert(0); else s; -> e; s;
> 
> I seriously object to this, as assert(0) is there for when you really do want an assert in release builds.
> 
> I.e.:
> 
> assert(!e); // removed in release builds
> if (e) assert(0); // stays in release builds
> 
> It's a valid D idiom, not a bug or oversight.

Then what would you suggest for dealing with cases where a nothrow function calls a function which can throw in general but can't throw with the given arguments? Completely separate from the issue of whether std.datetime should need to be calling a throwing function from a nothrow function as often as it does, it's still an issue which must be dealt with in general. At present, any nothrow function which wants to call a throwing function needs to use a try- catch block to wrap the throwing function, and it makes good sense to use assert(0) to catch the case where the caller screwed up and the function can indeed throw. However, given that that throwing function should never throw, it's desirable to be able to optimize out the try-catch-assert(0). How would we do that without taking advantage of the fact that assert(0) indicates that that line of code should be unreachable?

If this were @safe, we would use @trusted, but this is nothrow, and there is no trusted-nothrow.

- Jonathan M Davis

On Monday, November 18, 2013 12:44:43 Andrei Alexandrescu wrote:
> Well you must agree at some level that since you don't like short functions with or without newlines inserted, the set of options is considerably reduced.
> 
> 1. You search for an editor that offers half lines.
> 
> 2. You shun short functions altogether.
> 
> 3. You adjust your style.
> 
> Style is not something immutable. Paradoxically, when I was younger I used to see it as less of a variable than now. Improving and adapting one's style to various contexts is a lifelong and beneficial process.

I agree. My style has adjusted over time. It's just that I don't like any of the options in this case, so I'm divided on how to format the code. But given how others dislike the empty lines, I now lean more towards going with fewer lines in this case. At least then other folks will like the formatting more even if both ways bug may. But maybe it'll grow on me and bug me less.

- Jonathan M Davis