[commons-jexl] branch master updated: JEXL-369: Blind fix for elusive lexical scope cleaning pb;

[he...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git


The following commit(s) were added to refs/heads/master by this push:
     new fe04b47c JEXL-369: Blind fix for elusive lexical scope cleaning pb;
fe04b47c is described below

commit fe04b47c981d59fc6d175ecd4b1006af5bf55759
Author: henrib <he...@apache.org>
AuthorDate: Tue Aug 30 19:07:26 2022 +0200

    JEXL-369: Blind fix for elusive lexical scope cleaning pb;
---
 .../commons/jexl3/internal/LexicalScope.java       | 36 +++++++++++++++-------
 .../java/org/apache/commons/jexl3/LexicalTest.java |  8 +++++
 2 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
index 2f983e01..7a89d373 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/LexicalScope.java
@@ -21,8 +21,8 @@ import java.util.BitSet;
 /**
  * The set of symbols declared in a lexical scope.
  * <p>The symbol identifiers are determined by the functional scope.</p>
- * <p>We use 2 bits per symbol; bit 0 sets the actual symbol as lexical (let), bit 1 as a const.
- * There are actually only 4 used states: 0, 1, 3</p>
+ * <p>We use 2 bits per symbol s; bit (s*2)+0 sets the actual symbol as lexical (let), bit (s*2)+1 as a const.
+ * There are actually only 2 used states: 1 and 3</p>
  */
 public class LexicalScope {
     /**
@@ -31,14 +31,19 @@ public class LexicalScope {
     protected static final int LONGBITS = 64;
     /**
      * Bits per symbol.
-     * Declared, const, defined.
+     * let (b + 0) + const (b + 1).
      */
     protected static final int BITS_PER_SYMBOL = 2;
+    /**
+     * Bits per symbol.
+     * Declared, const, defined.
+     */
+    protected static final int SYMBOL_SHIFT = 1;
     /**
      * Bitmask for symbols.
      * Declared, const, defined.
      */
-    protected static final long SYMBOL_MASK = (1L << BITS_PER_SYMBOL) - 1; // 3, as 1+2, 2 bits
+    protected static final long SYMBOL_MASK = (1L << (BITS_PER_SYMBOL - 1)) - 1; // 3, as 1+2, 2 bits
     /**
      * Number of symbols.
      */
@@ -122,7 +127,7 @@ public class LexicalScope {
      * @return true if declared, false otherwise
      */
     public boolean hasSymbol(final int symbol) {
-        final int bit = symbol << BITS_PER_SYMBOL;
+        final int bit = symbol << SYMBOL_SHIFT;
         return isSet(bit);
     }
 
@@ -133,7 +138,7 @@ public class LexicalScope {
      * @return true if declared as constant, false otherwise
      */
     public boolean isConstant(final int symbol) {
-        final int bit = (symbol << BITS_PER_SYMBOL) | 1;
+        final int bit = (symbol << SYMBOL_SHIFT) | 1;
         return isSet(bit);
     }
 
@@ -144,7 +149,7 @@ public class LexicalScope {
      * @return true if registered, false if symbol was already registered
      */
     public boolean addSymbol(final int symbol) {
-        final int bit = (symbol << BITS_PER_SYMBOL) ;
+        final int bit = (symbol << SYMBOL_SHIFT) ;
         if (set(bit)) {
             count += 1;
             return true;
@@ -159,7 +164,11 @@ public class LexicalScope {
      * @return true if registered, false if symbol was already registered
      */
     public boolean addConstant(final int symbol) {
-        final int bit = (symbol << BITS_PER_SYMBOL) | 1;
+        final int letb = (symbol << SYMBOL_SHIFT) ;
+        if (!isSet(letb)) {
+            throw new IllegalStateException("symbol not declared before const " + symbol);
+        }
+        final int bit = (symbol << SYMBOL_SHIFT) | 1;
         return set(bit);
     }
 
@@ -176,15 +185,20 @@ public class LexicalScope {
                 final int s = Long.numberOfTrailingZeros(clean);
                 // call clean for symbol definition (3 as a mask for 2 bits,1+2)
                 clean &= ~(SYMBOL_MASK << s);
-                cleanSymbol.accept(s >> BITS_PER_SYMBOL);
+                cleanSymbol.accept(s >> SYMBOL_SHIFT);
             }
         }
         symbols = 0L;
         if (moreSymbols != null) {
             if (cleanSymbol != null) {
                 // step by bits per symbol
-                for (int s = moreSymbols.nextSetBit(0); s != -1; s = moreSymbols.nextSetBit(s + BITS_PER_SYMBOL)) {
-                    cleanSymbol.accept(s + LONGBITS);
+                for (int s = moreSymbols.nextSetBit(0);
+                     s != -1;
+                     s = moreSymbols.nextSetBit(s + BITS_PER_SYMBOL)) {
+                    // skip const bit indicator
+                    if ((s & 1) == 0) {
+                        cleanSymbol.accept((s >> SYMBOL_SHIFT) + LONGBITS);
+                    }
                 }
             }
             moreSymbols.clear();
diff --git a/src/test/java/org/apache/commons/jexl3/LexicalTest.java b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
index 43d0fd9f..ad6f5a7b 100644
--- a/src/test/java/org/apache/commons/jexl3/LexicalTest.java
+++ b/src/test/java/org/apache/commons/jexl3/LexicalTest.java
@@ -401,6 +401,14 @@ public class LexicalTest {
             Assert.assertTrue(scope.hasSymbol(i));
             Assert.assertFalse(scope.hasSymbol(i + 1));
         }
+        for(int i = 0; i < 128; i += 2) {
+            Assert.assertTrue(scope.addConstant(i));
+            Assert.assertFalse(scope.addConstant(i));
+        }
+        for(int i = 0; i < 128; i += 2) {
+            Assert.assertTrue(scope.hasSymbol(i));
+            Assert.assertFalse(scope.hasSymbol(i + 1));
+        }
     }
 
     @Test