We’re rewarding the question askers & reputations are being recalculated! Read more.
15

One small comment about your API. public void Tokenize() I would expect a Tokenize method to return a stream (IEnumerable) of Tokens rather than modify state. This should be a pure and idempotent function in my opinion.


13

The docstrings are vague. Documentation needs to be precise and complete. It should answer questions like "what arguments should I pass?" and "what does it return?" For example, "go through the AST and count lines" should become something like, "Return the number of nodes in the AST rooted at node, and update the set line_nos with the line numbers of these ...


12

#ifndef __JMCOMP_LEXER_H__ That is a symbol reserved for use by the implementation. Writing your this in a way that mimics what you see in the compiler-supplied headers is exactly wrong! For this and other reasons I suggest leaving that out completely and just use #pragma once. If you ever find a platform were this pragma is not supported, a script can ...


12

Nitpick private boolean exausthed = false; Pedantic, but this should be spelled exhausted. Class vs. Object private Set<Character> blankChars = new HashSet<Character>(); And then later in the constructor blankChars.add('\r'); blankChars.add('\n'); blankChars.add((char) 8); blankChars.add((char) 9); ...


11

Your code looks very clean and organized. Only some comments (without understanding the code in depth) You would probably use the regex option RegexOptions.Compiled to improve performance The Tokenize method has lots of similar code fragments. You could use a list of SyntaxKind elements like you did with LiteralTokens. Here you call MatchesDefition twice: ...


10

This looks well structured, with responsibilities nicely separated, but as they say: 'the devil is in the details'. Let's have a look: CharStream This class contains some surprising behavior: Peek(int count) returns null if there are less than count characters left, instead of returning only the remaining characters. This is something that I expect to be ...


8

A few other things: cursor += 2; and cursor++; are all over the place in your Tokenizer. This looks very error-prone. You need a class that will move cursor automatically when you read a substring. Something similar to StringReader, but designed to better fit your task. Maybe: //instead of returning strings you can return a complex object, //that would ...


8

Great job overall, the code looks quite clean. There are only few things that I would mention: you can calculate count using sum() one-liner if len(children) == 0: can be replaced with if not children: and, we don't actually need to handle this base case, since we are going to use sum with +1, and in case of children being an empty list, we'll have 1 as a ...


8

Looks pretty tidy, a few things: Consider reversing the check for an empty source in CharStream: if (string.IsNullOrEmpty(source)) { EOF = true; return; } Lines = Preprocessor.Utilities.GenerateList(source); This deals with that special condition immediately and doesn't disrupt the logic flow (as opposed to - do something if valid, then handle ...


6

1. Review There's no docstring. What does the function do? What does it return? The result includes the spaces: >>> tokenize('1 + 2') ['1', ' ', '+', ' ', '2'] but it seems unlikely that the spaces are significant. One of the useful things a tokenizer can do is to discard whitespace. The tokens are collected into a list and returned. This is ...


6

You can use verbatim strings to reduce the slash chaos: { SyntaxKind.RootOperationToken, new Regex(@"\/\/") }, You can also use IReadOnlyDictionary to prevent your dictionaries from being modified. The readonly flag tells that no one can write something like Keywords = null but currently it still can be modified, e.g. Keywords.Clear();. private ...


6

Much has already been said, but I think I've still got something to add: Whitespace Returning a separate token for each whitespace character is probably not very useful. If whitespace isn't significant in your language then you can simply ignore it (no tokens). Otherwise, why not combine consecutive whitespace characters into a single token? Your token ...


5

I have some notes in addition to answer of Jesse C. Slicer. In IsKeyword method the first line is if (!char.IsLetter(PeekChar())) return 0; Looks like C-style. If method returns enum return enum. Define a field in TokenType like NoToken = 0 and return it. And then compare result in all ifs with TokenType.NoToken rather than with zero. Do this for all ...


5

These are just some quick thoughts off the top of my head. Is there any particular functional reason for TokenType to be ushort? In general, you don't modify from the base type unless it's intended to be passed to a native API. Make the comments on your enum values XML comments: PrintKeyword = 1, // PRINT becomes /// <summary> /// PRINT /// </...


5

I've never learned about lexers, so I can only answer 1-4 confidently. I hope this helps to answer some of your questions. I found your code quite easy to follow. I was able to read from top to bottom, and jumping to the other method definitions and back. It seems very well organized. Yes, having all the right sections of code nestled into their own ...


5

It is very hard to review a lexer without formal definition of the language (honestly, I have very vague understanding how the nested comments are supposed to be structured). However, even without such definition, certain things are surely bugs. For example, in case '%': token = token_new(lexer, _mod); token->str = "%"; ...


5

Overall, the code is quite clean and understandable. Though, I have never done anything like you are doing, but here are some notes about the code: the Token and TokenTypes classes may use "__slots__" for faster attribute access and memory savings the char in builtin_map.keys() can and should be replaced with just char in builtin_map to avoid creating the ...


5

Issues with EBNF string ::= '"' {(any_character | string_escape)} '"'; Here for "any_character" you probably meant any character except " but that is not made explicit. number ::= [('+' | '-')] {digit} ('.' [digit] {digit}); ^^^A^^^ ^^^^^^^B^^^^^^^^^^^^ For A: This means zero or more digits. Thats fine. For B: Thats zero or one ...


4

private static readonly Dictionary<string, TokenKind> KeywordToTokenKind = new Dictionary<string, TokenKind>(StringComparer.OrdinalIgnoreCase) { ["and"] = TokenKind.AndKeyword, .. }; I would create this and other similar dictionaries automatically with reflection by decorating the enums with some ...


4

Include what you use You use isdigit, isalpha, etc. -- but you haven't included ctype.h or cctype to assure that they're declared. Broken Code bool startsIdentifier(char character) { return std::isalpha(character) || character == '_'); } I'm not sure if you accidentally messed this up while cutting and pasting to the browser, but this won't even ...


3

Sorting the properties alphabetically does not yield "the same code". Properties that appear later in a rule override properties that occur earlier in the same rule. Mozilla Developer Network gives an example of a tricky edge case: A value which is not specified is set to its initial value. That sounds anecdotal, but it really means that it overrides ...


3

Observation First observation use Flex/Bison. Learn it once it makes all this stuff trivial. OK. Got that out the way. Looking at the current code. Style You treat tokenizing and analyzing as two distinct phases. In most compilers this is not how it works. Normally the analyzer will read a stream of tokens (not from a list of prepared tokens). The ...


3

I would start by replacing the tuple (int Line, int Column) location with an immutable struct named SourceLocation. Here are some of the reasons why I would do this. The concept already exists in several places. TokenFactory, Token, and IContainsLocation know about this tuple. Adding a FileName field to the tuple would require changes to all three types. ...


3

I have implemented several lexers, mostly in C. Since the C language doesn't have regular expressions, and since regular expression engines need to evaluate the string character-by-character anyhow, checking each character by hand makes sense in C. This is probably why tutorials like this approach. Even in C, though, there are tools for writing lexers using ...


3

Same Lexer writtern in Lex. Lexer.l startsIdentifier [a-zA-Z_] isInIdentifier {startsIdentifier}|[0-9] isHexChar [0-9A-Fa-f] isOctal [0-7] isDigit [0-9] isWhitespace [ \n\t] skipWhitespace {isWhitespace}* isHexNumber 0x{isHexChar}+...


3

Looks very nice - clean code, readable, very good names. You're sometimes using brackets, sometimes not (compare input and nextChar, e.g.). I suggest choosing one of these and sticking to it. Range primitives could simplify and improve parts of your code, and is considered very idiomatic D. a. Your code can be made Unicode compatible simply by using std....


3

IMO the use of OneOfBase as a kind of imitation of discriminated unions makes a lot of noise in your code and makes it more difficult to read than it has to be. Instead I would define a more traditional class structure, where each class holds information about each pattern or token type: abstract class LikeToken { public abstract bool Match(string ...


2

I don't see your sentiment analysis code in your tweet_sentiment.py, so I will not comment on that. And if I understand correctly, you are trying to build a classifier for text classification based on Twitter data. So, depending on your data size, here are two things you could try. Small data size (several tens of thousands or lower): Tdidf + Support ...


2

A Makefile review Your object files should not each depend on all the headers. You'll find that doing so will not be a problem now, but will waste a lot of your time as your project grows. Instead, generate the dependencies automatically - e.g. following Paul D. Smith's paper. Use CXX and CXXFLAGS for C++ compilation, not CC and CFLAGS. It's best to run ...


2

This is a great exercise! Parsing can be a very useful skill to learn. Once you've done it yourself, you should definitely look up lex and yacc (or flex and bison, if you prefer). They make these sorts of things pretty easy. Refactoring As you mentioned, there are some repeated sections that you could refactor. I would take the sections where you are ...


Only top voted, non community-wiki answers of a minimum length are eligible