Difference between revisions of "Review/std.d.lexer"

From D Wiki
Jump to: navigation, search
m (→‎Related links)
 
(2 intermediate revisions by the same user not shown)
Line 12: Line 12:
 
== Current state ==
 
== Current state ==
  
−
Being polished before putting on vote.
+
Voting has finished.
 +
Rejected in its current implementation.
  
 
== Review 1 ==
 
== Review 1 ==
Line 50: Line 51:
 
=== Clarifications about concerns ===
 
=== Clarifications about concerns ===
  
−
TODO:
+
    Renaming tokens:
 +
        I feel that this is consistent with Phobos' style guidelines and does not
 +
        need to be changed. Members of the D community are free to vote no on
 +
        inclusion of this module if they disagree strongly enough.
 +
   
 +
    Defining tokens in terms of templates:
 +
        I'm not sure how this would impact performance of the lexer. Given the
 +
        amount of effort that went in to optimizing the lexer on the part of Dmitry,
 +
        myself, and others, I'm not willing to try changing something like this.
 +
        If someone else is strongly in favor of the Tok!"=" style, they can
 +
        implement it and benchmark it against the current implementation.
 +
   
 +
    Splitting the lexer module into smaller sub-modules:
 +
        I don't feel that this is worth the complexity that would be added.
 +
   
 +
    Using slices of a giant string constant to speed things up:
 +
        This didn't actually speed things up.
 +
   
 +
    Changing the lexer to throw a ParseException instead of Exception:
 +
        This can be revisited when Phobos has a standard exception heirarchy.
 +
   
 +
    Creating a parser generator:
 +
        This is not in the scope of a lexer module review.
 +
 
 +
(c) Brian
 +
 
 +
== Voting ==
 +
 
 +
=== Voting thread ===
 +
 
 +
http://forum.dlang.org/post/eeenynxifropasqcufdg@forum.dlang.org
 +
 
 +
=== Stats ===
 +
 
 +
==== Yes ====
 +
* André
 +
* Jacob Carlborg
 +
* Øivind
 +
* Justin Whear
 +
* Daniel Kozak
 +
* Mike Parker
 +
* Chris
 +
* Namespace
 +
* Dicebot
 +
* nazriel
 +
* Sönke Ludwig
 +
 
 +
==== Yes, If ====
 +
* Dejan Lekic (needs to use specific Exception class)
 +
* Tove (Must be CTFE'able)
 +
* deadalnix (Must be able to reuse identifier pool)
 +
 
 +
==== No ====
 +
* Martin Nowak *
 +
* Jakob Ovrum
 +
* ilya-stromberg
 +
* Andrei Alexandrescu *
 +
* Jonathan M Davis *
 +
* Volcz
 +
* Dmitry Olshansky
 +
 
 +
People marked with asterisk belong to this list : https://github.com/D-Programming-Language?tab=members
 +
 
 +
=== Summary ===
 +
 
 +
    Total    : 21
 +
    Yes      : 11
 +
    Yes, If  : 3
 +
    No      : 7
 +
 
 +
    Among D developers : 0/0/3
 +
 
 +
=== Verdict ===
 +
 
 +
Rejected in its current implementation

Latest revision as of 12:49, 13 October 2013

Description

std.d.lexer is standard module for lexing D code, written by Brian Schott

Related links

  1. Official lexing specification
  2. Documentation
  3. Example project that uses std.d.lexer

Current state

Voting has finished. Rejected in its current implementation.

Review 1

Result

There were no critical concerns raised (by review manager opinion) but Brian has decided to address some of smaller ones before putting it on vote.

Description

Probably longest part of discussion was about requiring lookahead in lexer and related complexity/performance implications. Outcome of discussion wasn't really clear.

Other comments/proposals:

  1. using _ suffix for keywords (listed separately as raised some discussion)
  2. various other naming issues
  3. general lack of documentation
  4. TokenType definition (performance concerns)
  5. providing some benchmark for comparing with other lexerout of the box
  6. splitting it and converting into package
  7. tests need more coverage / comments
  8. sharing identifier pool between multiple lexers


Changes so far

https://github.com/Hackerpilot/phobos/commit/9bdb7f97bb8021f3b0d0291896b8fe21a6fead23#std/d/lexer.d :

  • There are a few more unit tests now
  • bitAnd renamed to amp
  • slice rename to dotdot
  • Much more cross-referencing in the doc comments
  • Start line and column can be specified in the lexer config

https://github.com/Hackerpilot/phobos/compare/D-Programming-Language:df38839...master

Clarifications about concerns

   Renaming tokens:
       I feel that this is consistent with Phobos' style guidelines and does not
       need to be changed. Members of the D community are free to vote no on
       inclusion of this module if they disagree strongly enough.
   
   Defining tokens in terms of templates:
       I'm not sure how this would impact performance of the lexer. Given the
       amount of effort that went in to optimizing the lexer on the part of Dmitry,
       myself, and others, I'm not willing to try changing something like this.
       If someone else is strongly in favor of the Tok!"=" style, they can
       implement it and benchmark it against the current implementation.
   
   Splitting the lexer module into smaller sub-modules:
       I don't feel that this is worth the complexity that would be added.
   
   Using slices of a giant string constant to speed things up:
       This didn't actually speed things up.
   
   Changing the lexer to throw a ParseException instead of Exception:
       This can be revisited when Phobos has a standard exception heirarchy.
   
   Creating a parser generator:
       This is not in the scope of a lexer module review.

(c) Brian

Voting

Voting thread

http://forum.dlang.org/post/eeenynxifropasqcufdg@forum.dlang.org

Stats

Yes

  • André
  • Jacob Carlborg
  • Øivind
  • Justin Whear
  • Daniel Kozak
  • Mike Parker
  • Chris
  • Namespace
  • Dicebot
  • nazriel
  • Sönke Ludwig

Yes, If

  • Dejan Lekic (needs to use specific Exception class)
  • Tove (Must be CTFE'able)
  • deadalnix (Must be able to reuse identifier pool)

No

  • Martin Nowak *
  • Jakob Ovrum
  • ilya-stromberg
  • Andrei Alexandrescu *
  • Jonathan M Davis *
  • Volcz
  • Dmitry Olshansky

People marked with asterisk belong to this list : https://github.com/D-Programming-Language?tab=members

Summary

   Total    : 21
   Yes      : 11
   Yes, If  : 3
   No       : 7

   Among D developers : 0/0/3

Verdict

Rejected in its current implementation

Retrieved from "https://wiki.dlang.org/?title=Review/std.d.lexer&oldid=3016"