#1242 [PATCH] Lexer for Nim

[Jad Altahan]

Finished what I started in ticket #1220. A new lexer for the Nim language is finished.

[Jad Altahan]

Apparently, I missed the Numerical constants section of the manual, which specifes the numeric prefixes and the case-sensitivity of suffixes. Thus, I have made the following changes:

• Removed 0c/0C prefix in IsNumOctal(). Not supported.
• Added upper-case type suffixes in SCE_NIM_NUMBER. They are case-insensitive.

[Neil Hodgson]

The new lexer code is mostly great.

There were a couple of minor issues. In win32/scintilla.mak, LexNim.cxx is specified to produce LexNimrod.obj instead of LexNim.obj. There is an extraneous space at the end of #define SCLEX_NIM 126 in SciLexer.h causing automatic regeneration to produce a difference. Running scite/scripts/RegenerateSource.py fixes both of these.

Both gcc and clang would like some clarifying parentheses. If you are on Windows, its worthwhile installing a MinGW gcc distribution (like https://nuwen.net/mingw.html) to be able to check for porting issues.

./../lexers/LexNim.cxx: In member function 'virtual void LexerNim::Lex(Sci_PositionU, Sci_Position, int, Scintilla::IDocument*)':
./../lexers/LexNim.cxx:305:43: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
                     if (sc.chPrev != '\'' && sc.ch == 'e' || sc.ch == 'E') {
                         ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
./../lexers/LexNim.cxx:467:56: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
             if (IsADigit(sc.ch) || IsADigit(sc.chNext) && sc.ch == '.') {
                                    ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~

../lexers/LexNim.cxx:305:43: warning: '&&' within '||' [-Wlogical-op-parentheses]
                    if (sc.chPrev != '\'' && sc.ch == 'e' || sc.ch == 'E') {
                        ~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~ ~~
../lexers/LexNim.cxx:305:43: note: place parentheses around the '&&' expression to silence this warning
                    if (sc.chPrev != '\'' && sc.ch == 'e' || sc.ch == 'E') {
                                          ^
                        (                                )
../lexers/LexNim.cxx:467:56: warning: '&&' within '||' [-Wlogical-op-parentheses]
            if (IsADigit(sc.ch) || IsADigit(sc.chNext) && sc.ch == '.') {
                                ~~ ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
../lexers/LexNim.cxx:467:56: note: place parentheses around the '&&' expression to silence this warning
            if (IsADigit(sc.ch) || IsADigit(sc.chNext) && sc.ch == '.') {
                                                       ^
                                   (                                  )

[Jad Altahan]

Hi Neil. Welcome back from your vacation.

I ran the the RegenerateSource script and it made the changes you mentioned.

I have also fixed the parenthesis warnings as specified in your reply. Being a C# pleb, I am not familiar with C++ compilers at all. I installed GCC and, following the README instructions, I managed suceessfully compileScintilla with it. There doesn't seem to be any more warnings to deal with.

Link to the commit on my fork:
https://github.com/xv/scintilla/commit/623e31d439151edb2faa5bf6fa00532e75277c54

[Neil Hodgson]

A small problem has been found in the Nim lexer. Some of the functions and variables that are local to the lexer have global linkage so can clash with other lexers with similar problems. The common way to fix this is to declare most local elements in an unnamed namespace leaving the final line declaring the LexerModule globally visible. See LexCPP.cxx for an example using an unnamed namespace.

[Jad Altahan]

Working on it right now. But I got a question: should I remove the static keyword from the functions I just moved? Make them inline? Or keep everything the same?

static int GetNumStyle(int numType)
static bool IsAWordChar(const int ch)
static int IsNumHex(StyleContext &sc)
static int IsNumBinary(StyleContext &sc)
static int IsNumOctal(StyleContext &sc)
static bool IsNewline(const int ch)
static bool IsTripleLiteral(const int style)
static bool IsLineComment(const int style)
static bool IsStreamComment(const int style)

[Neil Hodgson]

I'd remove the static as it adds nothing beyond the unnamed namespace. Also probably make the simpler functions like IsTripleLiteral both constexpr and noexcept, as that could help optimization and pass StyleContext as const & to functions that are just readers.

However, its OK not to do these if you have other goals. I don't want to be policing coding style for lexers beyond avoiding bugs and compiler/lint warnings.

[Jad Altahan]

The changes have been made.

[Jad Altahan]

Hey Neil. Please update the properties file to the attached one. I've done plenty of work on it, and that includes:

• Reorganized for better readability.
• Removed property shbang.nim
• Added property *source.patterns.nim
• Removed atomic keyword. Not in use.
• Removed properties block.start & block.end. Not needed.
• Added value "#" to property comment.box.middle.nim
• Removed whitespace value from property comment.box.end.nim
• Added command.help property.
• Added "Compile Release Version" command to the Tools menu.

Thank you.

Edit - Jan 4th:

• Fixed typo in property fold.comment. It should be fold.compact

[Neil Hodgson]

Committed with changes as [112d46].

The global folding properties were removed as they affect all languages so should not be in a language-specific file.

Once a feature request has been committed, its better to create a new issue for further changes as that makes tracking its progress easier and means it is less likely to be overlooked.

[Jad Altahan]

--ignore--

[Jad Altahan]

Understood.

Could you please close this ticket then?

[Neil Hodgson]

Tickets are closed once the change has been included in a release.