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 <hen...@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