On Friday, 21 February 2014 at 12:12:17 UTC, Dicebot wrote:
http://wiki.dlang.org/Review/std.lexer
First of all, thank you for the great work. This is a very
important project.
I'll begin with reviewing the documentation.
Summary
Some simple explanation of the terminology and concepts would be
nice. At least a link to Wikipedia.
Create the string array costants for your language.
Typo ("costants")
Examples:
An inline complete example of a very simple language would be
nice.
A lexer for D is available here.
Although good to have, this is too much to take in all at once,
for documentation purposes.
A lexer for Lua is available here.
Nary a comment in sight. This might serve as the example lexer if
only it was better commented. The comments can be copy-pasted
from the module documentation, even that would make the code much
easier to grok.
Template Parameter Definitions
What does this mean? Parameters to what template?
Can this section be moved to inside the documentation of Lexer,
and Lexer be moved to the first documented symbol in the file?
A function that serves as the default token lexing function.
For most languages this will be the identifier lexing function.
Should the function signature and contracts be explained here?
This function must return bool and take a single size_t
argument representing the number of bytes to skip over before
looking for a separating character.
I think it's better to describe the signature in D syntax rather
than English.
A listing of tokens whose value is variable, such as
whitespace, identifiers, number literals, and string literals.
No mention of how the list is represented (is it an array? what
type of elements should the array have? how are the array values
used?), the reader is left to figure that out from the example
below.
Template for determining the type used for a token type.
Selects the smallest unsigned integral type that is able to
hold the value staticTokens.length + dynamicTokens.length +
possibleDefaultTokens.length. For example if there are 20
static tokens, 30 dynamic tokens, and 10 possible default
tokens, this template will alias itself to ubyte, as 20 + 30 +
10 < ubyte.max.
Should this be documented? I understand that this will be
instantiated only once, by std.lexer.
Utility declarations should preferably be at the end of the
module, so that they appear last in the documentation.
Generates the token type identifier for the given symbol. There
are two special cases:
Are these magic constants necessary? Why not declare them as
enums?
In all cases this template will alias itself to a constant of
type IdType. This template will fail at compile time if symbol
is not one of the staticTokens, dynamicTokens, or
possibleDefaultTokens.
Style nit: D symbols should be wrapped in the $D(...) macro.
== overload for the the token type.
Is it really necessary to document opEquals?
But since it's here: how does it interact with extraFields?
The Column number at which this token occurs.
There was a lot of bikeshedding regarding the correct terminology
to use when adding -vcolumns to DMD (
https://github.com/D-Programming-Language/dmd/pull/3077 ). I
think the documentation should at least mention what exactly it
is counting.
A function that serves as the default token lexing function.
For most languages this will be the identifier lexing function.
What should the function's name be? How will it interact with
Lexer? (It's not clear that this refers to the
defaultTokenFunction parameter, especially after the previous
list item, popFront, is a different piece of the puzzle.)
The documentation for Lexer's arguments seem to be thrown all
around the module. I suggest to document them only once, all in
Lexer's DDoc, add example signatures, and move Lexer to the top
of the module.
Examples:
struct CalculatorLexer
I think this should be expanded into a full, well-documented
example featured in the module DDoc.
_popFront();
Where did that come from? Perhaps you meant Lexer's DDoc to say
"which should call this mixin's _popFront()", and the DDoc
escaped the _ character? If so, why not use a named mixin to
disambiguate instead of _?
struct LexerRange;
struct StringCache;
These are thoroughly documented, but will they be used by
anything other than std.lexer?
