test/[: binary operators: fix '<' and add '=~'; some more cleanups

In ksh88, the test/[ built-in supported both the '<' and '>'
lexical sorting comparison operators, same as in [[. However, in
every version of ksh93, '<' does not work though '>' still does!

Still, the code for both is present in test_binop():

src/cmd/ksh93/bltins/test.c
548:		case TEST_SGT:
549:			return(strcoll(left, right)>0);
550:		case TEST_SLT:
551:			return(strcoll(left, right)<0);

Analysis: The binary operators are looked up in shtab_testops[] in
data/testops.c using a macro called sh_lookup, which expands to a
sh_locate() call. If we examine that function in sh/string.c, it's
easy to see that on systems using ASCII (i.e. all except IBM
mainframes), it assumes the table is sorted in ASCII order.

src/cmd/ksh93/sh/string.c
64:	while((c= *tp->sh_name) && (CC_NATIVE!=CC_ASCII || c <= first))

The problem was that the '<' operator was not correctly sorted in
shtab_testops[]; it was sorted immediately before '>', but after
'='. The ASCII order is: < (60), = (61), > (62). This caused '<' to
never be found in the table.

The test_binop() function is also used by [[, yet '<' always worked
in that. This is because the parser has code that directly checks
for '<' and '>' within [[ (in sh/parse.c, lines 1949-1952).

This commit also adds '=~' to 'test', which took three lines of
code and allowed eliminating error handling in test_binop() as
test/[ and [[ now support the same binary ops. (re: fc2d5a60)

src/cmd/ksh93/*/*.[ch]:
- Rename a couple of very misleadingly named macros in test.h:
  . For == and !=, the TEST_PATTERN bit is off for pattern compares
    and on for literal string compares! Rename to TEST_STRCMP.
  . The TEST_BINOP bit does not denote all binary operators, but
    only the logical -a/-o ops in test/[. Rename to TEST_ANDOR.

src/cmd/ksh93/bltins/test.c: test_binop():
- Add support for =~. This is only used by test/[. The method is
  implemented in two lines that convert the ERE to a shell pattern
  by prefixing it with ~(E), then call test_strmatch with that
  temporary string to match the ERE and update ${.sh.match}.
- Since all binary ops from shtab_testops[] are now accounted for,
  remove unknown op error handling from this function.

src/cmd/ksh93/data/testops.c:
- shtab_testops[]:
  . Correctly sort the '<' (TEST_SLT) entry.
  . Remove ']]' (TEST_END). It's not an op and doesn't belong here.
- Update sh_opttest[] documentation with =~, \<, \>.
- Remove now-unused e_unsupported_op[] error message.

src/cmd/ksh93/sh/lex.c: sh_lex():
- Check for ']]' directly instead of relying on the removed
  TEST_END entry from shtab_testops[].

src/cmd/ksh93/tests/bracket.sh:
- Add relevant tests.

src/cmd/ksh93/tests/builtins.sh:
- Fix an old test that globally deleted the 'test' builtin. Delete
  it within the command substitution subshell only.
- Remove the test for non-support of =~ in test/[.
- Update the test for invalid test/[ op to use test directly.

NEWS

@@ -5,6 +5,10 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 
 2021-11-13:
 
+- The test/[ built-in command now supports the '<' and '=~' operators from [[.
+  As of now, test/[ supports the same operators as [[ except for the different
+  and/or operators. Note: test/[ remains deprecated due to its many pitfalls.
+
 - The test/[ built-in command is fixed so that the binary -a (and) and -o (or)
   operators, as in [ "$a" -a "$b" ] or [ "$a" -o "$b" ], work even if "$a" is
   '!' or '('. To avoid breaking backwards compatibility with the nonstandard

src/cmd/ksh93/bltins/test.c

@@ -174,7 +174,7 @@ int b_test(int argc, char *argv[],Shbltin_t *context)
 		case 4:
 		{
 			register int op = sh_lookup(cp=argv[2],shtab_testops);
-			if(op&TEST_BINOP)
+			if(op&TEST_ANDOR)
 				break;
 			if(!op)
 			{
@@ -309,7 +309,7 @@ static int e3(struct test *tp)
 	register int op;
 	char *binop;
 	arg=nxtarg(tp,0);
-	if(sh_isoption(SH_POSIX) && tp->ap + 1 < tp->ac && ((op=sh_lookup(tp->av[tp->ap],shtab_testops)) & TEST_BINOP))
+	if(sh_isoption(SH_POSIX) && tp->ap + 1 < tp->ac && ((op=sh_lookup(tp->av[tp->ap],shtab_testops)) & TEST_ANDOR))
 	{	/*
 		 * In POSIX mode, makes sure standard binary -a/-o takes precedence
 		 * over nonstandard unary -a/-o if the lefthand expression is "!" or "("
@@ -374,7 +374,7 @@ static int e3(struct test *tp)
 	}
 skip:
 	op = sh_lookup(binop=cp,shtab_testops);
-	if(!(op&TEST_BINOP))
+	if(!(op&TEST_ANDOR))
 		cp = nxtarg(tp,0);
 	if(!op)
 	{
@@ -522,6 +522,10 @@ int test_unop(Shell_t *shp,register int op,register const char *arg)
 	}
 }
 
+/*
+ * This function handles binary operators for both the
+ * test/[ built-in and the [[ ... ]] compound command
+ */
 int test_binop(Shell_t *shp,register int op,const char *left,const char *right)
 {
 	register double lnum = 0, rnum = 0;
@@ -551,6 +555,9 @@ int test_binop(Shell_t *shp,register int op,const char *left,const char *right)
 			return(strcmp(left, right)==0);
 		case TEST_SNE:
 			return(strcmp(left, right)!=0);
+		case TEST_REP:
+			sfprintf(stkstd, "~(E)%s", right);
+			return(test_strmatch(shp, left, stkfreeze(stkstd, 1))>0);
 		case TEST_EF:
 			return(test_inode(left,right));
 		case TEST_NT:
@@ -569,15 +576,8 @@ int test_binop(Shell_t *shp,register int op,const char *left,const char *right)
 			return(lnum>=rnum);
 		case TEST_LE:
 			return(lnum<=rnum);
-		default:
-		{
-			/* fallback for operators not supported by the test builtin */
-			int i=0;
-			while(shtab_testops[i].sh_number && shtab_testops[i].sh_number != op)
-				i++;
-			errormsg(SH_DICT, ERROR_exit(2), op==TEST_END ? e_badop : e_unsupported_op, shtab_testops[i].sh_name);
-		}
 	}
+	/* all possible binary operators should be covered above */
 	UNREACHABLE();
 }
 

src/cmd/ksh93/data/testops.c

@@ -31,6 +31,7 @@
 
 /*
  * This is the list of binary test and [[ ... ]] operators
+ * It must be sorted in ascending ASCII order
  */
 
 const Shtable_t shtab_testops[] =
@@ -47,12 +48,11 @@ const Shtable_t shtab_testops[] =
 		"-nt",		TEST_NT,
 		"-o",		TEST_OR,
 		"-ot",		TEST_OT,
+		"<",		TEST_SLT,
 		"=",		TEST_SEQ,
 		"==",		TEST_SEQ,
 		"=~",           TEST_REP,
-		"<",		TEST_SLT,
 		">",		TEST_SGT,
-		"]]",		TEST_END,
 		"",		0
 };
 
@@ -134,6 +134,9 @@ const char sh_opttest[] =
 	"[+\astring1\a = \astring2\a?\astring1\a is equal to \astring2\a.]"
 	"[+\astring1\a == \astring2\a *?Same as \b=\b.]"
 	"[+\astring1\a != \astring2\a?\astring1\a is not equal to \astring2\a.]"
+	"[+\astring1\a =~ \aere\a *?\astring1\a matches the extended regular expression \aere\a.]"
+	"[+\astring1\a \\< \astring2\a *?\astring1\a lexically sorts before \astring2\a.]"
+	"[+\astring1\a \\> \astring2\a *?\astring1\a lexically sorts after \astring2\a.]"
 	"[+\anum1\a -eq \anum2\a?\anum1\a is numerically equal to \anum2\a.]"
 	"[+\anum1\a -ne \anum2\a?\anum1\a is not numerically equal to \anum2\a.]"
 	"[+\anum1\a -lt \anum2\a?\anum1\a is less than \anum2\a.]"
@@ -167,6 +170,5 @@ const char test_opchars[]	= "HLNRSVOGCaeohrwxdcbfugkv"
 const char e_argument[]		= "argument expected";
 const char e_missing[]		= "%s missing";
 const char e_badop[]		= "%s: unknown operator";
-const char e_unsupported_op[]	= "%s: operator not supported; use [[ ... ]]";
 const char e_tstbegin[]		= "[[ ! ";
 const char e_tstend[]		= " ]]\n";

src/cmd/ksh93/include/test.h

@@ -31,12 +31,13 @@
 #include	"defs.h"
 #include	"shtable.h"
 /*
- *  These are the valid test operators
+ *  These are the binary test operators
+ *  See also shtab_testops[] in data/testops.c
  */
 
 #define TEST_ARITH	040	/* arithmetic operators */
-#define TEST_BINOP	0200	/* binary operator */
-#define TEST_PATTERN	0100	/* turn off bit for pattern compares */
+#define TEST_ANDOR	0200	/* logical operators: -a, -o */
+#define TEST_STRCMP	0100	/* literal string comparison; turn off bit for pattern matching */
 
 #define TEST_NE		(TEST_ARITH|9)
 #define TEST_EQ		(TEST_ARITH|4)
@@ -44,10 +45,10 @@
 #define TEST_GT		(TEST_ARITH|6)
 #define TEST_LE		(TEST_ARITH|7)
 #define TEST_LT		(TEST_ARITH|8)
-#define TEST_OR		(TEST_BINOP|1)
-#define TEST_AND	(TEST_BINOP|2)
-#define TEST_SNE	(TEST_PATTERN|1)
-#define TEST_SEQ	(TEST_PATTERN|14)
+#define TEST_OR		(TEST_ANDOR|1)
+#define TEST_AND	(TEST_ANDOR|2)
+#define TEST_SNE	(TEST_STRCMP|TEST_PNE)
+#define TEST_SEQ	(TEST_STRCMP|TEST_PEQ)
 #define TEST_PNE	1
 #define TEST_PEQ	14
 #define TEST_EF		3
@@ -55,7 +56,6 @@
 #define TEST_OT		12
 #define TEST_SLT	16
 #define TEST_SGT	17
-#define TEST_END	8
 #define TEST_REP	20
 
 extern int test_unop(Shell_t*,int, const char*);
@@ -67,7 +67,6 @@ extern const char	test_opchars[];
 extern const char	e_argument[];
 extern const char	e_missing[];
 extern const char	e_badop[];
-extern const char	e_unsupported_op[];
 extern const char	e_tstbegin[];
 extern const char	e_tstend[];
 

src/cmd/ksh93/sh.1

@@ -7456,9 +7456,8 @@ to those specified for the \f3[[\fP compound command under
 .I Conditional Expressions
 above, but with several important differences. The \f3=\fP, \f3==\fP and
 \f3!=\fP operators test for string (in)equality without pattern matching;
-the \f3==\fP variant is nonstandard and should not be used. The \f3=\(ap\fP,
-\f3<\fP, \f3>\fP, \f3&&\fP and \f3||\fP operators are not available. Instead
-of \f3&&\fP and \f3||\fP, the \f3-a\fP and \f3-o\fP binary operators can be
+\f3==\fP is nonstandard and unportable. The f3&&\fP and \f3||\fP operators are
+not available. Instead, the \f3-a\fP and \f3-o\fP binary operators can be
 used, but they are fraught with pitfalls due to grammatical ambiguities and
 therefore deprecated in favor of invoking separate \f3test\fP commands. Most
 importantly, as \f3test\fP and \f3[\fP are simple regular commands, field

src/cmd/ksh93/sh/lex.c

@@ -1416,15 +1416,17 @@ breakloop:
 			return(lp->token);
 		}
 		lp->lex.incase = 0;
-		c = sh_lookup(state,shtab_testops);
-		switch(c)
+		if(state[0]==']' && state[1]==']' && !state[2])
 		{
-		case TEST_END:
+			/* end of [[ ... ]] */
 			lp->lex.testop2 = lp->lex.intest = 0;
 			lp->lex.reservok = 1;
 			lp->token = ETESTSYM;
 			return(lp->token);
-
+		}
+		c = sh_lookup(state,shtab_testops);
+		switch(c)
+		{
 		case TEST_SEQ:
 			if(lp->lexd.warn && state[1]==0)
 				errormsg(SH_DICT,ERROR_warn(0),e_lexobsolete3,shp->inlineno);
@@ -1434,7 +1436,7 @@ breakloop:
 			{
 				if(lp->lexd.warn && (c&TEST_ARITH))
 					errormsg(SH_DICT,ERROR_warn(0),e_lexobsolete4,shp->inlineno,state);
-				if(c&TEST_PATTERN)
+				if(c&TEST_STRCMP)
 					lp->lex.incase = 1;
 				else if(c==TEST_REP)
 					lp->lex.incase = TEST_RE;

src/cmd/ksh93/sh/parse.c

@@ -1970,10 +1970,11 @@ static Shnode_t *test_primary(Lex_t *lexp)
 #endif /* SHOPT_KIA */
 		if(sh_lex(lexp))
 			sh_syntax(lexp);
-		if(num&TEST_PATTERN)
+		if(num&TEST_STRCMP)
 		{
+			/* If the argument is unquoted, enable pattern matching */
 			if(lexp->arg->argflag&(ARG_EXP|ARG_MAC))
-				num &= ~TEST_PATTERN;
+				num &= ~TEST_STRCMP;
 		}
 		t = getnode(tstnod);
 		t->lst.lsttyp = TTST|TTEST|TBINARY|(num<<TSHIFT);

src/cmd/ksh93/tests/bracket.sh

@@ -434,5 +434,20 @@ then	set -o posix -o trackall
 	set +o posix
 fi
 
+# =====
+# test should support '<' as well as '>'; before 2021-11-13, ksh supported
+# only '>' due to '<' being missorted in shtab_testops[] in data/testops.c
+[ foo \< bar ] 2>/dev/null
+(($?==1)) || err_exit '[ foo \< bar ] not working'
+[ foo \> bar ] 2>/dev/null
+(($?==1)) || err_exit '[ foo \> bar ] not working'
+
+# as of 2021-11-13, test also supports =~
+[ att_ =~ '(att|cus)_.*' ] 2>/dev/null || err_exit 'test/[: =~ ERE not working'
+[ abc =~ 'a(b)c' ] 2>/dev/null || err_exit "[ abc =~ 'a(b)c' ] fails"
+[ abc =~ '\babc\b' ] 2>/dev/null || err_exit "[ abc =~ '\\babc\\b' ] fails"
+[ AATAAT =~ '(AAT){2}' ] 2>/dev/null || err_exit "[ AATAAT =~ '(AAT){2}' ] does not match"
+[ AATAATCCCAATAAT =~ '(AAT){2}CCC(AAT){2}' ] || err_exit "[ AATAATCCCAATAAT =~ '(AAT){2}CCC(AAT){2}' ] does not match"
+
 # ======
 exit $((Errors<125?Errors:125))

src/cmd/ksh93/tests/builtins.sh

@@ -240,8 +240,7 @@ fi
 if	[[ $(LC_MESSAGES=C type test) != 'test is a shell builtin' ]]
 then	err_exit 'whence -v test not a builtin'
 fi
-builtin -d test
-if	[[ $(type test) == *builtin* ]]
+if	[[ $(builtin -d test; type test) == *builtin* ]]
 then	err_exit 'whence -v test after builtin -d incorrect'
 fi
 typeset -Z3 percent=$(printf '%o\n' "'%'")
@@ -1070,21 +1069,10 @@ then	got=$( { "$SHELL" -c '
 fi
 
 # ==========
-# Verify that the POSIX 'test' builtin complains loudly when the '=~' operator is used rather than
-# failing silently. See https://github.com/att/ast/issues/1152.
-actual=$($SHELL -c 'test foo =~ foo' 2>&1)
-actual_status=$?
-actual=${actual#*: }
-expect='test: =~: operator not supported; use [[ ... ]]'
-expect_status=2
-[[ "$actual" = "$expect" ]] || err_exit "test =~ failed (expected $expect, got $actual)"
-[[ "$actual_status" = "$expect_status" ]] ||
-    err_exit "test =~ failed with the wrong exit status (expected $expect_status, got $actual_status)"
-
-# Invalid operators 'test' and '[[ ... ]]' both reject should also cause an error with exit status 2.
+# Verify that the POSIX 'test' builtin exits with status 2 when given an invalid binary operator.
 for operator in '===' ']]'
 do
-	actual="$($SHELL -c "test foo $operator foo" 2>&1)"
+	actual=$(test foo "$operator" foo 2>&1)
 	actual_status=$?
 	actual=${actual#*: }
 	expect="test: $operator: unknown operator"