Skip to content

Commit

Permalink
StaticValidator: When a keyword is misused as a function, say which one.
Browse files Browse the repository at this point in the history
Also make the GdlPool's list of keywords immutable.
  • Loading branch information
AlexLandau committed Nov 9, 2013
1 parent 3312d98 commit cc4bb00
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 52 deletions.
80 changes: 40 additions & 40 deletions src/org/ggp/base/util/gdl/grammar/GdlPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.TreeMap;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

import com.google.common.collect.ImmutableSet;
import com.google.common.collect.MapMaker;

public final class GdlPool
Expand All @@ -25,8 +25,8 @@ public final class GdlPool
private static final ConcurrentMap<String, GdlVariable> variablePool = new ConcurrentHashMap<String, GdlVariable>();
private static final ConcurrentMap<String, GdlConstant> constantPool = new ConcurrentHashMap<String, GdlConstant>();
private static final Map<String,String> constantCases = new TreeMap<String,String>(String.CASE_INSENSITIVE_ORDER);
private static final Map<String,String> variableCases = new TreeMap<String,String>(String.CASE_INSENSITIVE_ORDER);
private static final Map<String,String> variableCases = new TreeMap<String,String>(String.CASE_INSENSITIVE_ORDER);

// Controls whether we normalize the case of incoming constants and variables.
public static boolean caseSensitive = true;

Expand All @@ -37,8 +37,8 @@ public final class GdlPool
// as if one had attempted to create the GdlConstant "true", regardless of whether the
// game-specific constants are case-sensitive or not. These special keywords are never
// sent over the network in PLAY requests and responses, so this should be safe.
public static final HashSet<String> keywords = new HashSet<String>(Arrays.asList(
new String[] {"init","true","next","role","does","goal","legal","terminal","base","input","_"}));
public static final ImmutableSet<String> KEYWORDS = ImmutableSet.of(
"init","true","next","role","does","goal","legal","terminal","base","input","_");
public static final GdlConstant BASE = getConstant("base");
public static final GdlConstant DOES = getConstant("does");
public static final GdlConstant GOAL = getConstant("goal");
Expand All @@ -58,20 +58,20 @@ public final class GdlPool
/**
* Drains the contents of the GdlPool. Useful to control memory usage
* once you have finished playing a large game.
*
*
* WARNING: Should only be called *between games*.
*/
public static void drainPool() {
public static void drainPool() {
distinctPool.clear();
functionPool.clear();
notPool.clear();
orPool.clear();
propositionPool.clear();
relationPool.clear();
rulePool.clear();
variablePool.clear();
variablePool.clear();
variableCases.clear();

// When draining the pool between matches, we still need to preserve the keywords
// since there are global references to them. For example, the Prover state machine
// has a reference to the GdlConstant "true", and that reference must still point
Expand All @@ -80,25 +80,25 @@ public static void drainPool() {
// are set aside and returned to the pool after all of the other constants (which
// were game-specific) have been drained.
Map<String, GdlConstant> keywordConstants = new HashMap<String, GdlConstant>();
for (String keyword : keywords) {
for (String keyword : KEYWORDS) {
keywordConstants.put(keyword, GdlPool.getConstant(keyword));
}
constantPool.clear();
constantCases.clear();
constantCases.clear();
for (Map.Entry<String,GdlConstant> keywordEntry : keywordConstants.entrySet()) {
constantCases.put(keywordEntry.getKey(), keywordEntry.getKey());
constantPool.put(keywordEntry.getKey(), keywordEntry.getValue());
}
}

/**
* If the pool does not have a mapping for the given key, adds a mapping from key to value
* to the pool.
*
*
* Note that even if you've checked to make sure that the pool doesn't contain the key,
* you still shouldn't assume that this method actually inserts the given value, since
* this class is accessed by multiple threads simultaneously.
*
*
* @return the value mapped to by key in the pool
*/
private static <K,V> V addToPool(K key, V value, ConcurrentMap<K, V> pool) {
Expand All @@ -111,7 +111,7 @@ private static <K,V> V addToPool(K key, V value, ConcurrentMap<K, V> pool) {

public static GdlConstant getConstant(String value)
{
if (keywords.contains(value.toLowerCase())) {
if (KEYWORDS.contains(value.toLowerCase())) {
value = value.toLowerCase();
}
if (!caseSensitive) {
Expand All @@ -121,13 +121,13 @@ public static GdlConstant getConstant(String value)
constantCases.put(value, value);
}
}
GdlConstant ret = constantPool.get(value);

GdlConstant ret = constantPool.get(value);
if(ret == null)
ret = addToPool(value, new GdlConstant(value), constantPool);
ret = addToPool(value, new GdlConstant(value), constantPool);
return ret;
}

public static GdlVariable getVariable(String name)
{
if (!caseSensitive) {
Expand All @@ -136,24 +136,24 @@ public static GdlVariable getVariable(String name)
} else {
variableCases.put(name, name);
}
}
GdlVariable ret = variablePool.get(name);
}

GdlVariable ret = variablePool.get(name);
if(ret == null)
ret = addToPool(name, new GdlVariable(name), variablePool);
return ret;
}
}

public static GdlDistinct getDistinct(GdlTerm arg1, GdlTerm arg2)
{
ConcurrentMap<GdlTerm, GdlDistinct> bucket = distinctPool.get(arg1);
if(bucket == null)
bucket = addToPool(arg1, new ConcurrentHashMap<GdlTerm, GdlDistinct>(), distinctPool);

GdlDistinct ret = bucket.get(arg2);
if(ret == null)
ret = addToPool(arg2, new GdlDistinct(arg1, arg2), bucket);

return ret;
}

Expand Down Expand Up @@ -181,7 +181,7 @@ public static GdlFunction getFunction(GdlConstant name, List<GdlTerm> body)
body = getImmutableCopy(body);
ret = addToPool(body, new GdlFunction(name, body), bucket);
}

return ret;
}

Expand All @@ -190,7 +190,7 @@ public static GdlNot getNot(GdlLiteral body)
GdlNot ret = notPool.get(body);
if(ret == null)
ret = addToPool(body, new GdlNot(body), notPool);

return ret;
}

Expand All @@ -206,7 +206,7 @@ public static GdlOr getOr(List<GdlLiteral> disjuncts)
disjuncts = getImmutableCopy(disjuncts);
ret = addToPool(disjuncts, new GdlOr(disjuncts), orPool);
}

return ret;
}

Expand All @@ -215,7 +215,7 @@ public static GdlProposition getProposition(GdlConstant name)
GdlProposition ret = propositionPool.get(name);
if(ret == null)
ret = addToPool(name, new GdlProposition(name), propositionPool);

return ret;
}

Expand Down Expand Up @@ -243,7 +243,7 @@ public static GdlRelation getRelation(GdlConstant name, List<GdlTerm> body)
body = getImmutableCopy(body);
ret = addToPool(body, new GdlRelation(name, body), bucket);
}

return ret;
}

Expand All @@ -263,16 +263,16 @@ public static GdlRule getRule(GdlSentence head, List<GdlLiteral> body)
ConcurrentMap<List<GdlLiteral>, GdlRule> bucket = rulePool.get(head);
if(bucket == null)
bucket = addToPool(head, new ConcurrentHashMap<List<GdlLiteral>, GdlRule>(), rulePool);

GdlRule ret = bucket.get(body);
if(ret == null) {
body = getImmutableCopy(body);
ret = addToPool(body, new GdlRule(head, body), bucket);
}

return ret;
}

private static <T> List<T> getImmutableCopy(List<T> list) {
return Collections.unmodifiableList(new ArrayList<T>(list));
}
Expand All @@ -297,7 +297,7 @@ public static Gdl immerse(Gdl foreignGdl) {
List<GdlLiteral> rval = new ArrayList<GdlLiteral>();
for(int i=0; i<or.arity(); i++)
{
rval.add((GdlLiteral) immerse(or.get(i)));
rval.add((GdlLiteral) immerse(or.get(i)));
}
return GdlPool.getOr(rval);
} else if(foreignGdl instanceof GdlProposition) {
Expand All @@ -307,26 +307,26 @@ public static Gdl immerse(Gdl foreignGdl) {
List<GdlTerm> rval = new ArrayList<GdlTerm>();
for(int i=0; i<rel.arity(); i++)
{
rval.add((GdlTerm) immerse(rel.get(i)));
}
rval.add((GdlTerm) immerse(rel.get(i)));
}
return GdlPool.getRelation((GdlConstant)immerse(rel.getName()), rval);
} else if(foreignGdl instanceof GdlRule) {
GdlRule rule = (GdlRule)foreignGdl;
List<GdlLiteral> rval = new ArrayList<GdlLiteral>();
for(int i=0; i<rule.arity(); i++)
{
rval.add((GdlLiteral) immerse(rule.get(i)));
rval.add((GdlLiteral) immerse(rule.get(i)));
}
return GdlPool.getRule((GdlSentence) immerse(rule.getHead()), rval);
return GdlPool.getRule((GdlSentence) immerse(rule.getHead()), rval);
} else if(foreignGdl instanceof GdlConstant) {
return GdlPool.getConstant(((GdlConstant) foreignGdl).getValue());
} else if(foreignGdl instanceof GdlFunction) {
GdlFunction func = (GdlFunction)foreignGdl;
List<GdlTerm> rval = new ArrayList<GdlTerm>();
for(int i=0; i<func.arity(); i++)
{
rval.add((GdlTerm) immerse(func.get(i)));
}
rval.add((GdlTerm) immerse(func.get(i)));
}
return GdlPool.getFunction((GdlConstant) immerse(func.getName()), rval);
} else if(foreignGdl instanceof GdlVariable) {
return GdlPool.getVariable(((GdlVariable) foreignGdl).getName());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ private String getRandomWord() {
}

private static boolean shouldMap(String token) {
if (GdlPool.keywords.contains(token.toLowerCase())) {
if (GdlPool.KEYWORDS.contains(token.toLowerCase())) {
return false;
}
try {
Expand Down
15 changes: 4 additions & 11 deletions src/org/ggp/base/validator/StaticValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -488,17 +488,10 @@ private static void testPredefinedArities(
}

//Look for function arities with these names
if(functionArities.containsKey(ROLE)
|| functionArities.containsKey(TERMINAL)
|| functionArities.containsKey(GOAL)
|| functionArities.containsKey(LEGAL)
|| functionArities.containsKey(DOES)
|| functionArities.containsKey(INIT)
|| functionArities.containsKey(TRUE)
|| functionArities.containsKey(NEXT)
|| functionArities.containsKey(BASE)
|| functionArities.containsKey(INPUT)) {
throw new ValidatorException("Probable error: Misuse of a keyword as a function");
for (GdlConstant functionName : functionArities.keySet()) {
if (GdlPool.KEYWORDS.contains(functionName)) {
throw new ValidatorException("The keyword " + functionName + " is being used as a function. It should only be used as the name of a sentence.");
}
}
}

Expand Down

0 comments on commit cc4bb00

Please sign in to comment.