On Friday, 21 February 2014 at 22:14:36 UTC, Andrej Mitrovic wrote:
> Hmm, maybe I should make a pull request upstream?

You should.

On Friday, 21 February 2014 at 22:13:30 UTC, Andrej Mitrovic wrote:
> On 2/21/14, Joseph Cassman <jc7919@outlook.com> wrote:
>> 1. StringCache is a custom hash table. It looks like it's primary
>> role is to reduce some sort of duplication.
>
> It's used for string interning.

But that still doesn't explain why a custom hash table implementation is necessary. Maybe a lightweight wrapper around built-in AAs is sufficient?

(I also apologize if this has already been asked and answered in the last review thread, which is unfortunately too long to read in a short time.)

Brian, do you have benchmarks for this proposal similar to ones you have provided in old review threads? (vs DMD frontend lexer)

On Friday, 21 February 2014 at 12:12:17 UTC, Dicebot wrote:
> http://wiki.dlang.org/Review/std.lexer
>
> This is follow-up by Brian to his earlier proposal (http://wiki.dlang.org/Review/std.d.lexer). This time proposed module focuses instead on generic lexer generation as discussed in matching voting thread.
>
> Docs: http://hackerpilot.github.io/experimental/std_lexer/phobos/lexer.html
> Code: https://github.com/Hackerpilot/Dscanner/blob/master/stdx/lexer.d
>
> Example topics to evaluate during review:
>  - Feasibility of overall design and concept
>  - Quality of documentation
>  - Consistency with existing Phobos modules
>  - Overall quality of implementation
>
> Initial review will end on March the 8th.

No criticism should stop this module being accepted, as we do not have any other lexer in the runtime anyway. Therefore I suggest we accept std.lexer until a better solution comes up. Naturally anyone should be encouraged to provide a better solution by submitting a pull request to Phobos developers...

So far I haven't seen a better lexer for D source than Brian's std.lexer. If anyone has, please let me know.

On Monday, 24 February 2014 at 22:14:34 UTC, Dejan Lekic wrote:
> No criticism should stop this module being accepted, as we do not have any other lexer in the runtime anyway. Therefore I suggest we accept std.lexer until a better solution comes up. Naturally anyone should be encouraged to provide a better solution by submitting a pull request to Phobos developers...

The problem is that this is what has been done before, and now we are more or less stuck with outdated, sometimes poorly-written, often buggy modules (std.signals being one example).

On Mon, 24 Feb 2014 14:32:53 -0800, Meta <jared771@gmail.com> wrote:

> On Monday, 24 February 2014 at 22:14:34 UTC, Dejan Lekic wrote:
>> No criticism should stop this module being accepted, as we do not have any other lexer in the runtime anyway. Therefore I suggest we accept std.lexer until a better solution comes up. Naturally anyone should be encouraged to provide a better solution by submitting a pull request to Phobos developers...
>
> The problem is that this is what has been done before, and now we are more or less stuck with outdated, sometimes poorly-written, often buggy modules (std.signals being one example).

Well, we keep voting down replacement candidates, which incidentally, is exactly what happened with the std.signals replacement, so I view this as an orthogonal issue to whether or not it should be included after passing a review. I don't think the fact that a module might not be perfect after review should stop us from approving a module that offers completely new functionality AND passed a review. Handling the problems after inclusion is what bugzilla is for.

-- 
Adam Wilson
GitHub/IRC: LightBender
Aurora Project Coordinator

On Monday, 24 February 2014 at 23:07:07 UTC, Adam Wilson wrote:
> Well, we keep voting down replacement candidates, which incidentally, is exactly what happened with the std.signals replacement, so I view this as an orthogonal issue to whether or not it should be included after passing a review. I don't think the fact that a module might not be perfect after review should stop us from approving a module that offers completely new functionality AND passed a review. Handling the problems after inclusion is what bugzilla is for.

I guess std.signals was a bad example, as there *was* a proposed replacement. However, there were real problems with the replacement that made it not suitable for inclusion. If I recall, these were largely API issues, which are the hardest to change. If we had've accepted the new std.signals despite the issues raised, several years down the road it might turn out to be as broken as the old implementation (no offense to the author, this is just for the sake of argument), and we are unable to replace it for fear of breaking code. There are then 2 options: support the old API with its broken behaviour in the same module as the new API, or introduce std.signals2 or the like, which people have shown resistance to in the past. I think that it's very important to be careful as to what goes into Phobos at this point, as it's going to become increasingly difficult to change anything.

On Mon, 24 Feb 2014 15:36:43 -0800, Meta <jared771@gmail.com> wrote:

> On Monday, 24 February 2014 at 23:07:07 UTC, Adam Wilson wrote:
>> Well, we keep voting down replacement candidates, which incidentally, is exactly what happened with the std.signals replacement, so I view this as an orthogonal issue to whether or not it should be included after passing a review. I don't think the fact that a module might not be perfect after review should stop us from approving a module that offers completely new functionality AND passed a review. Handling the problems after inclusion is what bugzilla is for.
>
> I guess std.signals was a bad example, as there *was* a proposed replacement. However, there were real problems with the replacement that made it not suitable for inclusion. If I recall, these were largely API issues, which are the hardest to change. If we had've accepted the new std.signals despite the issues raised, several years down the road it might turn out to be as broken as the old implementation (no offense to the author, this is just for the sake of argument), and we are unable to replace it for fear of breaking code. There are then 2 options: support the old API with its broken behaviour in the same module as the new API, or introduce std.signals2 or the like, which people have shown resistance to in the past. I think that it's very important to be careful as to what goes into Phobos at this point, as it's going to become increasingly difficult to change anything.

Ok. Then by that standpoint we have two remaining options. A) Don't ever change existing code because you might breaking someone else's depending code on accident. Even without an API change, a change in how a function does it's processing can result in incorrect behavior in down-stream code. B) Never introduce new modules because we are terrified that the API might not be right in five years.

It is unrealistic in the extreme to demand that a new module in Phobos meet some arbitrary future compatibility bar. We routinely make changes to Phobos that break people's code in subtle ways that don't produce compiler errors and we never hear about it because no sane programmer expects a standard library to be a static thing that will never ever ever change in any way what so ever so that they can expect the exact same behavior five years from now. No, they go fix it and move on.

"OMG! We can't add this! It might not be the right API in the future!" By that reasoning we should halt all work on Phobos ever. After all, we might break something or a new API might not be perfect in five years after a new language feature allows an obviously better way of implementing that API. As reasoning goes, the best word I am come up with to describe it is "naive". It is a purely fear based response based on some unspecified and unknown future circumstance. Newsflash, you can't predict the future, which incidentally, is why API's change!

The reason std.signals replacement got voted down was because it wasn't API compatible with the original. This is despite the fact that nobody stood up to say that they use the current implementation and like it. Anybody who did use it said that they are desperate for a replacement. No the current std.signals is universally reviled and almost completely unused but we can't change it because of the mythical silent user who might be out there coding under a rock with no Internet and isn't voicing their opinion. Well, guess what, if you don't speak-up you have no right to blame it on the people who did and decided to changed it. You had your chance, stop whining and go update your code.

We are programmers. It is our JOB to deal with changes in the API/ABI/ISA. We don't have the RIGHT to whine about change because dealing with change is what we DO. If we aren't dealing with change we're working on something that isn't used at all and therefore pointless. I'm all for pointless pursuits, but don't hamstring me and my non-pointless pursuits with them.

I wanted std.signals in the new form badly, but now I can't have it, and Aurora is going to suffer mightily for it. Now I get to go off and make my own substandard iteration of Signals, because some people whined about how the API wasn't backwards compatible, even though it's widely acknowledged that almost nobody uses it and those that do would've been happy to move to the new signals.

Note, I am not talking about voting down modules that are obviously poorly implemented, I am talking about modules that work well and can do the task they purport to do with as few bugs as possible. If the module can do what it says it can, and the API isn't horrible, then we should default to adding it. Griping about the API five years from now is just bikeshedding, write up a pull and get that reviewed, chances are high it'll get rejected because the changes are worse anyways.

The only difficulty with changing anything is in your head. In practice, programmers who actually need to get things done expect it.

-- 
Adam Wilson
GitHub/IRC: LightBender
Aurora Project Coordinator

On Tuesday, 25 February 2014 at 00:28:26 UTC, Adam Wilson wrote:
>[SNIP]

You're throwing what I said way out of proportion. I was replying to the statement:

"No criticism should stop this module being accepted, as we do not
have any other lexer in the runtime anyway. Therefore I suggest
we accept std.lexer until a better solution comes up."

I don't agree with this. Obviously std.lexer is well-written and has been through a few rounds of iteration, but that doesn't mean we should accept it without due diligence to ensure that we won't be regretting some overlooked, poorly-designed or badly-named piece of functionality down the road. "Good enough because we don't yet have anything better" is a bad idea. It seems to me that what Brian has written is much better than "good enough", but I don't think that it should be accepted into Phobos *solely* because we don't have anything else. If the community decides that it is a worthwhile addition, then great, but that must not happen *until* it has passed rigorous review, just like every other recent Phobos module.

On Mon, 24 Feb 2014 17:22:32 -0800, Meta <jared771@gmail.com> wrote:

> On Tuesday, 25 February 2014 at 00:28:26 UTC, Adam Wilson wrote:
>> [SNIP]
>
> You're throwing what I said way out of proportion. I was replying to the statement:
>
> "No criticism should stop this module being accepted, as we do not
> have any other lexer in the runtime anyway. Therefore I suggest
> we accept std.lexer until a better solution comes up."
>
> I don't agree with this. Obviously std.lexer is well-written and has been through a few rounds of iteration, but that doesn't mean we should accept it without due diligence to ensure that we won't be regretting some overlooked, poorly-designed or badly-named piece of functionality down the road. "Good enough because we don't yet have anything better" is a bad idea. It seems to me that what Brian has written is much better than "good enough", but I don't think that it should be accepted into Phobos *solely* because we don't have anything else. If the community decides that it is a worthwhile addition, then great, but that must not happen *until* it has passed rigorous review, just like every other recent Phobos module.

Fair enough. I guess I am just still touchy after the way std.signals was shot down. There weren't great technical arguments for shooting it down and so I feel that a good piece of code that would've been immediately useful and accepted by the community was rejected over some pretty silly fears.

Note that I as badly as I want std.lexer to be included I want it to pass a rigorous review. This review (and, is passing, subsequent inclusion) has opened up an opportunity to start using D at work that I did not expect and so I am kind of excited about it.

-- 
Adam Wilson
GitHub/IRC: LightBender
Aurora Project Coordinator