Skip to content

Commit

Permalink
[oil-language] Expression errors produce error status 3 in a command.
Browse files Browse the repository at this point in the history
So then errexit can take over.  This fixes uncaught IndexError,
KeyError, ZeroDivisionError, etc.

We still don't have a good story for TypeError.  There is a one-off
around one EvalExpr.

Addresses #942.
  • Loading branch information
Andy C committed Apr 30, 2022
1 parent a6e6d23 commit 10fefce
Show file tree
Hide file tree
Showing 11 changed files with 329 additions and 16 deletions.
3 changes: 3 additions & 0 deletions core/error.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,6 @@ class ErrExit(FatalRuntime):
Travels between WordEvaluator and CommandEvaluator.
"""

class Expr(FatalRuntime):
""" e.g. KeyError, IndexError, ZeroDivisionError """
13 changes: 13 additions & 0 deletions cpp/core_error.h
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,19 @@ class ErrExit : public _ErrorWithLocation {
}
};

// Stub: the parts that raise aren't translated
class Expr : public _ErrorWithLocation {
#if 0

This comment has been minimized.

Copy link
@AngryCoder1

AngryCoder1 May 19, 2022

ув

public:
Expr(Str* user_str, int span_id)
: _ErrorWithLocation(user_str, span_id) {
}
Expr(Str* user_str, Token* token)
: _ErrorWithLocation(user_str, token) {
}
#endif
};

// Stub
class Runtime : public _ErrorWithLocation {
public:
Expand Down
3 changes: 2 additions & 1 deletion cpp/frontend_flag_spec.cc
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,8 @@ Tuple2<args::_Attributes*, args::Reader*> ParseCmdVal(

// With optional arg
Tuple2<args::_Attributes*, args::Reader*> ParseCmdVal(
Str* spec_name, runtime_asdl::cmd_value__Argv* cmd_val, bool accept_typed_args) {
Str* spec_name, runtime_asdl::cmd_value__Argv* cmd_val,
bool accept_typed_args) {
// TODO: disallow typed args!
ParseCmdVal(spec_name, cmd_val);
}
Expand Down
3 changes: 2 additions & 1 deletion cpp/frontend_flag_spec.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,8 @@ Tuple2<args::_Attributes*, args::Reader*> ParseCmdVal(

// With optional arg
Tuple2<args::_Attributes*, args::Reader*> ParseCmdVal(
Str* spec_name, runtime_asdl::cmd_value__Argv* cmd_val, bool accept_typed_args);
Str* spec_name, runtime_asdl::cmd_value__Argv* cmd_val,
bool accept_typed_args);

Tuple2<args::_Attributes*, args::Reader*> ParseLikeEcho(
Str* spec_name, runtime_asdl::cmd_value__Argv* cmd_val);
Expand Down
10 changes: 6 additions & 4 deletions doc/error-handling.md
Original file line number Diff line number Diff line change
Expand Up @@ -429,14 +429,16 @@ the rules for Oil's new command types:
- `proc`. As with defining shell functions, defining a `proc` never fails. It
always exits `0`.
- `var`, `const`, `setvar`, and the `_` keyword. If an exception occurs during
expression evaluation, the status is `1`. Otherwise it's `0`.
expression evaluation, the status is `3`. Otherwise it's `0`.

Similarly, an expression sub like like `echo $[1 / 0]` will raise an internal
exception, and the status of `echo` will be `1`. (This is similar to what
exception, and the status of `echo` will be `3`. (This is similar to what
happens when a redirect fails.)

TODO: implement these rules. Maybe consider status 3?

Note: The status `3` is an arbitrary non-zero status that's not used by other
shells. Status `2` is generally for syntax errors and status `1` is for most
runtime failures.

## Summary

Oil uses three mechanisms to fix error handling once and for all.
Expand Down
29 changes: 24 additions & 5 deletions oil_lang/expr_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
lvalue, value, value_e, scope_e,
)
from asdl import runtime
from core import error
from core.pyerror import e_die, log
from frontend import consts
from oil_lang import objects
Expand Down Expand Up @@ -311,9 +312,13 @@ def EvalExpr(self, node):
if node.op.id == Id.Arith_Star:
return left * right
if node.op.id == Id.Arith_Slash:
# NOTE: from __future__ import division changes 5/2!
# But just make it explicit.
return float(left) / right # floating point division
# NOTE: does not depend on from __future__ import division
try:
result = float(left) / right # floating point division
except ZeroDivisionError:
raise error.Expr('divide by zero', token=node.op)

return result

if node.op.id == Id.Expr_DSlash:
return left // right # integer divison
Expand Down Expand Up @@ -554,7 +559,16 @@ def _gen():
if node.tag == expr_e.Subscript:
obj = self.EvalExpr(node.obj)
index = self._EvalIndices(node.indices)
return obj[index]
try:
result = obj[index]
except KeyError:
# TODO: expr.Subscript has no error location
raise error.Expr('dict entry not found', span_id=runtime.NO_SPID)
except IndexError:
# TODO: expr.Subscript has no error location
raise error.Expr('index out of range', span_id=runtime.NO_SPID)

return result

# Note: This is only for the obj.method() case. We will probably change
# the AST and get rid of getattr().
Expand All @@ -568,7 +582,12 @@ def _gen():

if id_ == Id.Expr_RArrow: # d->key is like d['key']
name = node.attr.val
return o[name]
try:
result = o[name]
except KeyError:
raise error.Expr('dict entry not found', token=node.op)

return result

if id_ == Id.Expr_DColon: # StaticName::member
raise NotImplementedError(id_)
Expand Down
3 changes: 3 additions & 0 deletions osh/cmd_eval.py
Original file line number Diff line number Diff line change
Expand Up @@ -1403,6 +1403,9 @@ def _Execute(self, node):
try:
status = self._Dispatch(node, cmd_st)
check_errexit = cmd_st.check_errexit
except error.Expr as e:
self.errfmt.PrettyPrintError(e, prefix='expression: ')
status = 3
except error.FailGlob as e:
if not e.HasLocation(): # Last resort!
e.span_id = self.mem.CurrentSpanId()
Expand Down
186 changes: 186 additions & 0 deletions spec/fatal-errors.test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
#
# Some shell errors are unrecoverable! Like divide by zero (except in bash.
#
# Any others?


#### Unrecoverable: divide by zero in redirect word

$SH -c '
echo hi > file$(( 42 / 0 )) in
echo inside=$?
'
echo outside=$?

## STDOUT:
outside=1
## END

## OK dash/ash STDOUT:
outside=2
## END

# bash makes the command fail
## OK bash STDOUT:
inside=1
outside=0
## END:


#### Unrecoverable: divide by zero in conditional word

$SH -c '
if test foo$(( 42 / 0 )) = foo; then
echo true
else
echo false
fi
echo inside=$?
'
echo outside=$?

echo ---

$SH -c '
if test foo$(( 42 / 0 )) = foo; then
echo true
fi
echo inside=$?
'
echo outside=$?

## STDOUT:
outside=1
---
outside=1
## END

## OK dash/ash STDOUT:
outside=2
---
outside=2
## END

# bash makes the command fail
## OK bash STDOUT:
inside=1
outside=0
---
inside=1
outside=0
## END:

# weird difference in zsh!

## BUG zsh STDOUT:
outside=1
---
outside=0
## END


#### Unrecoverable: divide by zero in case

$SH -c '
case $(( 42 / 0 )) in
(*) echo hi ;;
esac
echo inside=$?
'
echo outside=$?

echo ---

$SH -c '
case foo in
( $(( 42 / 0 )) )
echo hi
;;
esac
echo inside=$?
'
echo outside=$?

## STDOUT:
outside=1
---
outside=1
## END

## OK dash/ash STDOUT:
outside=2
---
outside=2
## END

## OK bash STDOUT:
inside=1
outside=0
---
inside=1
outside=0
## END:

## BUG zsh STDOUT:
outside=0
---
outside=0
## END


#### Unrecoverable: ${undef?message}

$SH -c '
echo ${undef?message}
echo inside=$?
'
echo outside=$?

$SH -c '
case ${undef?message} in
(*) echo hi ;;
esac
echo inside=$?
'
echo outside=$?

## STDOUT:
outside=1
outside=1
## END
## OK dash STDOUT:
outside=2
outside=2
## END
## OK bash STDOUT:
outside=127
outside=127
## END

#### ${undef} with nounset

$SH -c '
set -o nounset
case ${undef} in
(*) echo hi ;;
esac
echo inside=$?
'
echo outside=$?

## STDOUT:
outside=1
## END

## OK dash STDOUT:
outside=2
## END

## OK bash STDOUT:
outside=127
## END

## BUG zsh STDOUT:
outside=0
## END

46 changes: 45 additions & 1 deletion spec/oil-builtin-error.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,57 @@ process sub failed: 2 2
## END

#### try can handled failed var, setvar, etc.
shopt --set parse_brace

try {
echo hi
var x = 1 / 0
echo 'should not get here'
}
echo div $_status

try {
var a = []
setvar item = a[1]
echo 'should not get here'
}
echo index $_status

try {
var d = {}
setvar item = d['mykey']
echo 'should not get here'
}
echo key $_status

try {
setvar item = d->mykey
echo 'should not get here'
}
echo arrow $_status

## STDOUT:
hi
div 3
index 3
key 3
arrow 3
## END

#### try can handled failed expr sub
shopt --set parse_brace

try {
echo hi

var d = {}
echo "result = $[d->BAD]"
echo 'should not get here'
}
echo _status=$_status
## STDOUT:
_status=1
hi
_status=3
## END

#### boolstatus with external command
Expand Down
Loading

0 comments on commit 10fefce

Please sign in to comment.