Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 10.7.1.1
    • Fix Version/s: 10.7.1.1
    • Component/s: Tools
    • Labels:
      None

      Description

      When building with "ant -q", I see this warning when JavaCC processes ij.jj:

      [java] Warning: Lookahead adequacy checking not being performed since option LOOKAHEAD is more than 1. Set option FORCE_LA_CHECK to true to force checking.

      1. ij.diff
        7 kB
        Knut Anders Hatlen
      2. ij.diff
        7 kB
        Knut Anders Hatlen

        Activity

        Hide
        Knut Anders Hatlen added a comment -

        If we do as the warning says, and set FORCE_LA_CHECK to true, we get even more warnings, since it then warns us about everything that would have been a problem if LOOKAHEAD had been 1 (it is 2). But I think it should be fairly straightforward to massage the grammar into a shape in which it's safe to lower the lookahead to 1.

        Getting rid of the warning isn't just about reducing noise in the build. If we'd had lookahead adequacy checking, which the warning says was disabled, we would for example have been warned about mistakes like this one in ij.jj -> ijStatement(), where FirstStatement appears twice in the grammar:

        r=ExecuteStatement()
        r=FirstStatement()
        r=FirstStatement()
        r=JBMSPreparedStatementExec()
        Show
        Knut Anders Hatlen added a comment - If we do as the warning says, and set FORCE_LA_CHECK to true, we get even more warnings, since it then warns us about everything that would have been a problem if LOOKAHEAD had been 1 (it is 2). But I think it should be fairly straightforward to massage the grammar into a shape in which it's safe to lower the lookahead to 1. Getting rid of the warning isn't just about reducing noise in the build. If we'd had lookahead adequacy checking, which the warning says was disabled, we would for example have been warned about mistakes like this one in ij.jj -> ijStatement(), where FirstStatement appears twice in the grammar: r=ExecuteStatement() r=FirstStatement() r=FirstStatement() r=JBMSPreparedStatementExec()
        Hide
        Knut Anders Hatlen added a comment -

        Attaching a patch that reduces the lookahead in ij.jj from 2 to 1, thereby enabling lookahead adequacy checking and silencing the warning. The following grammar changes were needed to make it work with lookahead 1:

        1) Remove the duplicate reference to FirstStatement in the grammar

        2) Refactor the ConnectStatement rule so that it doesn't branch until after the common prefix <CONNECT> has been seen

        3) dynamicConnection/attributeList: Add a lookahead hint in attributeList to avoid ambiguity when the attribute list is empty and followed by AS (as in CONNECT '...' ATTRIBUTES AS C1, where a lookahead of 1 wouldn't be enough to determine whether AS was a property name or a keyword)

        4) Merge the rules for qualified and unqualified table names in DescTableStatement

        5) Factor out the common prefix <SHOW> in ShowStatement

        6) Merge the rules IllegalStatementName and PrepareStatement into a single rule with the common prefix <PREPARE> factored out

        7) Merge the three rules JBMSPreparedStatementExec, F2KExecuteProcedure and ExecuteStatement into a single rule, with the common prefix <EXECUTE> factored out

        Show
        Knut Anders Hatlen added a comment - Attaching a patch that reduces the lookahead in ij.jj from 2 to 1, thereby enabling lookahead adequacy checking and silencing the warning. The following grammar changes were needed to make it work with lookahead 1: 1) Remove the duplicate reference to FirstStatement in the grammar 2) Refactor the ConnectStatement rule so that it doesn't branch until after the common prefix <CONNECT> has been seen 3) dynamicConnection/attributeList: Add a lookahead hint in attributeList to avoid ambiguity when the attribute list is empty and followed by AS (as in CONNECT '...' ATTRIBUTES AS C1, where a lookahead of 1 wouldn't be enough to determine whether AS was a property name or a keyword) 4) Merge the rules for qualified and unqualified table names in DescTableStatement 5) Factor out the common prefix <SHOW> in ShowStatement 6) Merge the rules IllegalStatementName and PrepareStatement into a single rule with the common prefix <PREPARE> factored out 7) Merge the three rules JBMSPreparedStatementExec, F2KExecuteProcedure and ExecuteStatement into a single rule, with the common prefix <EXECUTE> factored out
        Hide
        Knut Anders Hatlen added a comment -

        All the regression tests ran cleanly with the patch.

        Show
        Knut Anders Hatlen added a comment - All the regression tests ran cleanly with the patch.
        Hide
        Knut Anders Hatlen added a comment -

        Updated patch with a small formatting change that I had forgotten (changes whitespace only).

        Show
        Knut Anders Hatlen added a comment - Updated patch with a small formatting change that I had forgotten (changes whitespace only).
        Hide
        Dag H. Wanvik added a comment -

        Good cleanup, thanks for getting rid of the noise in our build. Looks good to me. +1

        Show
        Dag H. Wanvik added a comment - Good cleanup, thanks for getting rid of the noise in our build. Looks good to me. +1
        Hide
        Knut Anders Hatlen added a comment -

        Thanks for reviewing the patch, Dag.
        Committed revision 1029970.

        Show
        Knut Anders Hatlen added a comment - Thanks for reviewing the patch, Dag. Committed revision 1029970.

          People

          • Assignee:
            Knut Anders Hatlen
            Reporter:
            Knut Anders Hatlen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development