Smarter identifier resolution in conditions involving strings

Conditions comparing strings behaved rather erratically: while wing=m
was smartly interpreted as wing='m' (because m is ambiguous as plane
selector), wing=e was not so smartly interpreted as wing=effic
(because e unambiguously identifies effic; this was then rejected due
to incompatible types), and wing=g was even less smartly interpreted
as wing=group.

Address this by a redesign of the identifier resolution rules in
nstr_comp(): If the condition contains just one identifier, it names a
selector.  If it contains two, try to intepret either as selector or,
if the other can be interpreted as selector, as value for that.
String selectors accept any identifier as value, numeric selectors
only the ones listed in their table.

Interpret both identifiers as selectors only if their types are
compatible (makes rpath=gu work for ships) and their tables, if any,
match (makes type=spy work for land units).

If more than one interpretation makes sense, drop any value
interpretations of identifiers that are unabbreviated selector names,
and selector interpretations of those that are not (makes wing=w work
for planes).

Change nstr_match_val() to accept any identifier as value for a string
selector.  Replace nstr_mkselval() by nstr_resolve_val(), to resolve
string values in addition to symbolic ones.  Remove resolution to
string from nstr_resolve_id(), drop its last parameter.  Remove unused
nstr_string_ok().  New nstr_is_name_of_ca(), nstr_ca_comparable().

Examples for conditions that are now interpreted differently:

    condition    old interpretation  new interpretation
    wing=g       wing=group          wing='g'
    w=g          wing=group          rejected as ambiguous (1)
    w=e          wing=effic (2)      wing='e'

(1) because both wing='g' and 'w'=group make sense
(2) rejected as incomparable later on

diff --git a/src/lib/subs/nstr.c b/src/lib/subs/nstr.c
index f5ab19726cc228a9dd72a633663059a91ae5005d..8beb07094a612284655d506e467ae6e530346ce4 100644 (file)
--- a/src/lib/subs/nstr.c
+++ b/src/lib/subs/nstr.c
@@ -44,11 +44,12 @@
 
 static char *nstr_parse_val(char *, struct valstr *);
 static int nstr_match_ca(struct valstr *, struct castr *);
+static int nstr_is_name_of_ca(struct valstr *, struct castr *, int);
+static int nstr_ca_comparable(struct castr *, int, int);
 static int nstr_match_val(struct valstr *, struct castr *, int);
-static int nstr_string_ok(struct castr *ca, int idx);
-static struct valstr *nstr_resolve_id(struct valstr *, struct castr *, int, int);
+static struct valstr *nstr_resolve_id(struct valstr *, struct castr *, int);
 static struct valstr *nstr_resolve_sel(struct valstr *, struct castr *);
-static struct valstr *nstr_mkselval(struct valstr *, int, struct castr *);
+static struct valstr *nstr_resolve_val(struct valstr *, int, struct castr *);
 static int nstr_optype(enum nsc_type, enum nsc_type);
 
 /*
@@ -69,6 +70,7 @@ nstr_comp(struct nscstr *np, int len, int type, char *str)
     struct nscstr dummy;
     int lft_caidx, rgt_caidx;
     int lft_val, rgt_val;
+    int two_sels;
 
     cond = str;
     for (i = 0; ; ++i, ++np) {
@@ -97,49 +99,80 @@ nstr_comp(struct nscstr *np, int len, int type, char *str)
        /*
         * Resolve identifiers
         *
-        * An identifier can name a selector or, if the other operand
-        * is a selector, a value for that.  The condition is
-        * ambiguous if both selector x value and value x selector are
-        * possible.  Example: n<n for sectors could mean newdes<n or
-        * n<newdes.
+        * If just one operand is an identifier, it names a selector.
+        * If both operands are identifiers, things get complicated:
+        * either can then name a selector or a symbolic value for the
+        * selector named by the other operand.
         */
-       lft_val = nstr_match_val(&np->lft, ca, rgt_caidx);
-       rgt_val = nstr_match_val(&np->rgt, ca, lft_caidx);
-       /*
-        * if lft_val >= 0, then rhs names a selector and lhs names
-        * one of its values.  Likewise for rgt_val.
-        */
-       if (lft_val >= 0 && rgt_val >= 0) {
-           pr("%.*s -- condition ambiguous\n", (int)(tail-cond), cond);
-           return -1;
-       } else if (rgt_val >= 0) {
-           /* selector x value */
-           if (!nstr_resolve_sel(&np->lft, &ca[lft_caidx]))
-               return -1;
-           nstr_mkselval(&np->rgt, rgt_val, &ca[lft_caidx]);
-       } else if (lft_val >= 0) {
-           /* value x selector */
-           nstr_mkselval(&np->lft, lft_val, &ca[rgt_caidx]);
-           if (!nstr_resolve_sel(&np->rgt, &ca[rgt_caidx]))
-               return -1;
-       } else {
+       if (np->lft.val_cat == NSC_ID && np->rgt.val_cat == NSC_ID) {
+           lft_val = nstr_match_val(&np->lft, ca, rgt_caidx);
+           rgt_val = nstr_match_val(&np->rgt, ca, lft_caidx);
+           two_sels = nstr_ca_comparable(ca, lft_caidx, rgt_caidx);
            /*
-            * Neither side works as selector value; any identifiers
-            * must name selectors or strings.
+            * If lft_val >= 0 interpreting rgt as a selector and lft
+            * as one of its values works.  Likewise for rgt_val >= 0.
+            * If two_sels, interpreting both lft and rgt as selector
+            * works.
             */
-           if (!nstr_resolve_id(&np->lft, ca, lft_caidx,
-                                nstr_string_ok(ca, rgt_caidx)))
-               return -1;
-           if (!nstr_resolve_id(&np->rgt, ca, rgt_caidx,
-                                nstr_string_ok(ca, lft_caidx)))
+           switch ((lft_val >= 0) + (rgt_val >= 0) + !!two_sels) {
+           case 0:             /* no interpretation */
+               if (lft_caidx >= 0 && rgt_caidx >= 0) {
+                   /*
+                    * Both identifiers name selectors.  Since
+                    * !two_sels, they can't be comparable.
+                    * Example: type=civil.
+                    */
+                   pr("%.*s -- not comparable\n", (int)(tail-cond), cond);
+                   return -1;
+               }
+               /*
+                * At least one identifier doesn't name a selector,
+                * and nstr_resolve_id() will fail for it below
+                */
+               break;
+           case 1:             /* one unambigous interpretation */
+               break;
+           default:            /* multiple interpretations */
+               /*
+                * Last-resort disambiguation: if the identifier is
+                * the unabbreviated name of a selector, discard
+                * value, else discard selector interpretation.
+                * Example: resolve wing=g to wing='g', not wing=group
+                * or 'wing'=group.
+                */
+               if (nstr_is_name_of_ca(&np->lft, ca, lft_caidx))
+                   lft_val = -1;
+               else
+                   two_sels = 0;
+               if (nstr_is_name_of_ca(&np->rgt, ca, rgt_caidx))
+                   rgt_val = -1;
+               else
+                   two_sels = 0;
+               if ((lft_val >= 0) + (rgt_val >= 0) + !!two_sels == 1)
+                   break;      /* last-resort disambiguation worked */
+               /*
+                * Example: n<n for sectors could mean newdes<n or
+                * n<newdes.
+                */
+               pr("%.*s -- condition ambiguous\n", (int)(tail-cond), cond);
                return -1;
+           }
+           /* resolve identifiers naming values */
+           if (lft_val >= 0)
+               nstr_resolve_val(&np->lft, lft_val, &ca[rgt_caidx]);
+           if (rgt_val >= 0)
+               nstr_resolve_val(&np->rgt, rgt_val, &ca[lft_caidx]);
        }
+       /* remaining identifiers name selectors */
+       if (!nstr_resolve_id(&np->lft, ca, lft_caidx))
+           return -1;
+       if (!nstr_resolve_id(&np->rgt, ca, rgt_caidx))
+           return -1;
 
        /* find operator type */
        np->optype = nstr_optype(np->lft.val_type, np->rgt.val_type);
        if (np->optype == NSC_NOTYPE) {
-           pr("%.*s -- condition operand type mismatch\n",
-              (int)(tail-cond), cond);
+           pr("%.*s -- not comparable\n", (int)(tail-cond), cond);
            return -1;
        }
 
@@ -311,6 +344,37 @@ nstr_match_ca(struct valstr *val, struct castr *ca)
                  sizeof(struct castr));
 }
 
+/*
+ * Is identifier VAL the name of the selector given by CA and IDX?
+ * Return non-zero if and only if IDX is non-negative and VAL is the
+ * name of CA[IDX].
+ * IDX must have been obtained from nstr_match_ca(VAL, CA).
+ */
+static int
+nstr_is_name_of_ca(struct valstr *val, struct castr *ca, int idx)
+{
+    if (CANT_HAPPEN(val->val_cat != NSC_ID && idx >= 0))
+       return 0;
+    return idx >= 0 && strlen(ca[idx].ca_name) == val->val_as.str.maxsz;
+}
+
+/*
+ * Do we have two comparable selectors?
+ * Check selector descriptors CA[LFT_IDX] (unless LFT_IDX is negative)
+ * and CA[RGT_IDX] (unless RGT_IDX is negative).  CA may be null when
+ * both are negative.
+ */
+static int
+nstr_ca_comparable(struct castr *ca, int lft_idx, int rgt_idx)
+{
+    if (lft_idx < 0 || rgt_idx < 0)
+       return 0;
+    if (ca[lft_idx].ca_table != ca[rgt_idx].ca_table)
+       return 0;               /* Example: land type=spy */
+    return nstr_optype(ca[lft_idx].ca_type, ca[rgt_idx].ca_type)
+       != NSC_NOTYPE;          /* Example: ship name=effic */
+}
+
 /*
  * Match VAL in a selector's values, return its (non-negative) value.
  * Match values of selector descriptor CA[IDX], provided IDX is not
@@ -322,71 +386,52 @@ static int
 nstr_match_val(struct valstr *val, struct castr *ca, int idx)
 {
     char id[32];
+    enum nsc_type type;
 
-    if (val->val_cat != NSC_ID || val->val_as.str.maxsz >= sizeof(id))
+    if (val->val_cat != NSC_ID || idx < 0)
        return M_NOTFOUND;
 
-    if (idx < 0 || ca[idx].ca_table == EF_BAD)
-       return M_NOTFOUND;
-    if (CANT_HAPPEN(nstr_promote(ca[idx].ca_type) != NSC_LONG))
+    type = nstr_promote(ca[idx].ca_type);
+    if (type == NSC_STRING)
+       return 0;
+
+    if (ca[idx].ca_table == EF_BAD || CANT_HAPPEN(type != NSC_LONG))
        return M_NOTFOUND;
 
+    if (val->val_as.str.maxsz >= sizeof(id))
+       return M_NOTFOUND;
     memcpy(id, val->val_as.str.base, val->val_as.str.maxsz);
     id[val->val_as.str.maxsz] = 0;
-
     return ef_elt_byname(ca[idx].ca_table, id);
 }
 
 /*
- * Can CA[IDX] be compared to a string?
- * Return 0 for negative IDX.
- */
-static int
-nstr_string_ok(struct castr *ca, int idx)
-{
-    return idx >= 0 && nstr_promote(ca[idx].ca_type) == NSC_STRING;
-}
-
-/*
- * Change VAL to resolve identifier to selector or string.
+ * Change VAL to resolve identifier to selector.
  * Return VAL on success, NULL on error.
- * No change if VAL is not an identifier.  Otherwise, change it as
- * follows.
- * Error if IDX == M_NOTUNIQUE or IDX == M_NOTFOUND and !STRING_OK.
- * Change into string if IDX == M_NOTFOUND and STRING_OK.
- * Change into selector CA[IDX] if IDX >= 0.
+ * No change if VAL is not an identifier.
+ * Else change VAL into symbolic value for selector CA[IDX] if IDX >=
+ * 0, and error if not.
  */
 static struct valstr *
-nstr_resolve_id(struct valstr *val, struct castr *ca, int idx, int string_ok)
+nstr_resolve_id(struct valstr *val, struct castr *ca, int idx)
 {
     if (val->val_cat != NSC_ID)
        return val;
 
-    if (idx == M_NOTUNIQUE && !string_ok) {
+    if (idx == M_NOTUNIQUE) {
        pr("%.*s -- ambiguous name\n",
           (int)val->val_as.str.maxsz, val->val_as.str.base);
        val->val_cat = NSC_NOCAT;
        return NULL;
     }
 
-    if (idx == M_NOTFOUND && !string_ok) {
+    if (idx == M_NOTFOUND) {
        pr("%.*s -- unknown name\n",
           (int)val->val_as.str.maxsz, val->val_as.str.base);
        val->val_cat = NSC_NOCAT;
        return NULL;
     }
 
-    if (idx < 0) {
-       CANT_HAPPEN(!string_ok);
-       /* interpret unresolvable identifier as string */
-       val->val_type = NSC_STRING;
-       val->val_cat = NSC_VAL;
-       /* map identifier ~ to empty string, like some commands do */
-       if (val->val_as.str.maxsz == 1 && val->val_as.str.base[0] == '~')
-           val->val_as.str.maxsz = 0;
-       return val;
-    }
-
     return nstr_resolve_sel(val, &ca[idx]);
 }
 
@@ -415,13 +460,30 @@ nstr_resolve_sel(struct valstr *val, struct castr *ca)
 }
 
 /*
- * Initialize VAL to value SELVAL for selector CA, return VAL.
+ * Change VAL to resolve identifier to value SELVAL for selector CA.
+ * Return VAL.
+ * VAL must be an identifier, and SELVAL must have been obtained from
+ * nstr_match_val(VAL, CA0, IDX), where CA = &CA0[IDX].
  */
 static struct valstr *
-nstr_mkselval(struct valstr *val, int selval, struct castr *ca)
+nstr_resolve_val(struct valstr *val, int selval, struct castr *ca)
 {
     enum nsc_type type = nstr_promote(ca->ca_type);
-    
+
+    if (CANT_HAPPEN(val->val_cat != NSC_ID)) {
+       val->val_cat = NSC_NOCAT;
+       return val;
+    }
+
+    if (type == NSC_STRING) {
+       val->val_type = NSC_STRING;
+       val->val_cat = NSC_VAL;
+       /* map identifier ~ to empty string, like some commands do */
+       if (val->val_as.str.maxsz == 1 && val->val_as.str.base[0] == '~')
+           val->val_as.str.maxsz = 0;
+       return val;
+    }
+
     if (CANT_HAPPEN(type != NSC_LONG || ca->ca_table == EF_BAD)) {
        val->val_type = NSC_NOTYPE;
        val->val_cat = NSC_NOCAT;
@@ -464,7 +526,7 @@ nstr_comp_val(char *str, struct valstr *val, int type)
 {
     struct castr *ca = ef_cadef(type);
     char *tail = nstr_parse_val(str, val);
-    if (!nstr_resolve_id(val, ca, nstr_match_ca(val, ca), 0))
+    if (!nstr_resolve_id(val, ca, nstr_match_ca(val, ca)))
        return NULL;
     return tail;
 }