Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

#1491 ECL lexer regular expression detection code appears incorrect

Bug
closed-fixed
Neil Hodgson
5
2013-07-21
2013-06-27
Neil Hodgson
No

In the ECL lexer, the IsSpaceEquiv function generates warnings from code checkers as the expression contains redundant clauses. The first subclause (state <= SCE_ECL_COMMENTDOC) covers all the later ones.

return (state <= SCE_ECL_COMMENTDOC) ||
    // including SCE_ECL_DEFAULT, SCE_ECL_COMMENT, SCE_ECL_COMMENTLINE
    (state == SCE_ECL_COMMENTLINEDOC) || (state == SCE_ECL_COMMENTDOCKEYWORD) ||
    (state == SCE_ECL_COMMENTDOCKEYWORDERROR)

where

SCE_ECL_COMMENTDOC=23
SCE_ECL_COMMENTLINEDOC=15
SCE_ECL_COMMENTDOCKEYWORD=17
SCE_ECL_COMMENTDOCKEYWORDERROR=18

Unsure of what the correct fix would be.

Discussion

  • Gordon Smith
    Gordon Smith
    2013-06-27

    Its been a while...

    As you probably worked out, this code is based on the cpp lexer (which I am guessing has/had the same warning), in the ECL case SCE_ECL_COMMENTDOC seems to cover all the ECL states (except for a few used in a compare tool).

    The safe thing todo would be to change it to: return (state <= SCE_ECL_COMMENTDOC); since that is what it is logically doing anyway, I will investigate what this call is supposed to be achieving.

     
  • Neil Hodgson
    Neil Hodgson
    2013-06-27

    For C++, the first subclause just hits 4 states 0..3: DEFAULT / COMMENT / COMMENTLINE / COMMENTDOC. The styles were renumbered for ECL, so the expression should probably just be a list of the comment styles and the default style. Maybe:

    switch (state) {
        case SCE_ECL_DEFAULT:
        case SCE_ECL_COMMENT:
        case SCE_ECL_COMMENTLINE:
        case SCE_ECL_COMMENTLINEDOC:
        case SCE_ECL_COMMENTDOCKEYWORD:
        case SCE_ECL_COMMENTDOCKEYWORDERROR:
        case SCE_ECL_COMMENTDOC:
            return true;
    }
    
     
  • Gordon Smith
    Gordon Smith
    2013-06-27

    Looking at the code, I think it is only there to handle the special case of an inline regex expression (a JavaScript quirk - is the cpp lexer used for javascript also?).

    Based on initial testing your proposed fix would be fine.

     
  • Neil Hodgson
    Neil Hodgson
    2013-06-27

    Yes, the C++ lexer also does JavaScript.

    Committed as [17badf].

     

    Related

    Commit: [17badf]

  • Neil Hodgson
    Neil Hodgson
    2013-06-30

    • status: open --> open-fixed
     
  • Neil Hodgson
    Neil Hodgson
    2013-07-21

    • status: open-fixed --> closed-fixed