Skip to content

Commit

Permalink
Apply errexit in expansion::Error::handle
Browse files Browse the repository at this point in the history
The `apply_errexit` function in `yash-env` is removed as it did not
abstract the semantics of the option in an intuitive way. Instead, the
`handle` method of `expansion::Error` now returns a result that reflects
the semantics of the `ErrExit` option.
  • Loading branch information
magicant committed Apr 21, 2024
1 parent 25acd21 commit 362b7c9
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 72 deletions.
10 changes: 9 additions & 1 deletion yash-env/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@ All notable changes to `yash-env` will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.1.1] - Unreleased
## [0.2.0] - Unreleased

### Added

- `Env::errexit_is_applicable`

### Removed

- `semantics::apply_errexit`

### Fixed

Expand Down
19 changes: 13 additions & 6 deletions yash-env/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -400,16 +400,23 @@ impl Env {
variable
}

pub(crate) fn errexit_is_applicable(&self) -> bool {
/// Tests whether the [`ErrExit`] option is applicable in the current context.
///
/// This function returns true if and only if:
/// - the [`ErrExit`] option is on, and
/// - the current stack has no [`Condition`] frame.
///
/// [`Condition`]: Frame::Condition
pub fn errexit_is_applicable(&self) -> bool {
self.options.get(ErrExit) == On && !self.stack.contains(&Frame::Condition)
}

/// Returns a `Divert` if the shell should exit because of the `ErrExit`
/// [shell option](self::option::Option).
/// Returns a `Divert` if the shell should exit because of the [`ErrExit`]
/// shell option.
///
/// The function returns `Break(Divert::Exit)` if the `ErrExit` option is
/// on, the current `self.exit_status` is non-zero, and the current stack
/// has no `Condition` [frame](Frame); otherwise, `Continue(())`.
/// The function returns `Break(Divert::Exit(None))` if the [`errexit`
/// option is applicable](Self::errexit_is_applicable) and the current
/// `self.exit_status` is non-zero. Otherwise, it returns `Continue(())`.
pub fn apply_errexit(&self) -> ControlFlow<Divert> {
if !self.exit_status.is_successful() && self.errexit_is_applicable() {
Break(Divert::Exit(None))
Expand Down
57 changes: 1 addition & 56 deletions yash-env/src/semantics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,9 @@
//! Type definitions for command execution.

use crate::system::Errno;
use crate::Env;
use nix::sys::signal::Signal;
use std::ffi::c_int;
use std::ops::ControlFlow::{self, Break};
use std::ops::ControlFlow;
use std::process::ExitCode;
use std::process::Termination;
use yash_syntax::source::Location;
Expand Down Expand Up @@ -230,63 +229,9 @@ impl Divert {
/// next.
pub type Result<T = ()> = ControlFlow<Divert, T>;

/// Applies the `ErrExit` shell option to the result.
///
/// If the `ErrExit` option is on in `env.options`, `env.stack` contains no
/// `Frame::Condition`, and the `result` is `Divert::Interrupt`, then the result
/// is converted to `Divert::Exit`.
pub fn apply_errexit<T>(result: Result<T>, env: &Env) -> Result<T> {
match result {
Break(Divert::Interrupt(exit_status)) if env.errexit_is_applicable() => {
Break(Divert::Exit(exit_status))
}

other => other,
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::option::Option::ErrExit;
use crate::option::State::On;
use crate::stack::Frame;

#[test]
fn apply_errexit_applicable() {
let mut env = Env::new_virtual();
env.options.set(ErrExit, On);
let subject: Result = Break(Divert::Interrupt(Some(ExitStatus(42))));
let result = apply_errexit(subject, &env);
assert_eq!(result, Break(Divert::Exit(Some(ExitStatus(42)))));
}

#[test]
fn apply_errexit_to_non_interrupt() {
let mut env = Env::new_virtual();
env.options.set(ErrExit, On);
let subject: Result = Break(Divert::Return(None));
let result = apply_errexit(subject, &env);
assert_eq!(result, subject);
}

#[test]
fn apply_errexit_in_condition_context() {
let mut env = Env::new_virtual();
env.options.set(ErrExit, On);
let env = env.push_frame(Frame::Condition);
let subject: Result = Break(Divert::Interrupt(Some(ExitStatus(42))));
let result = apply_errexit(subject, &env);
assert_eq!(result, subject);
}

#[test]
fn apply_errexit_with_disabled_option() {
let env = Env::new_virtual();
let subject: Result = Break(Divert::Interrupt(Some(ExitStatus(42))));
let result = apply_errexit(subject, &env);
assert_eq!(result, subject);
}

#[test]
fn signal_try_from_exit_status() {
Expand Down
7 changes: 7 additions & 0 deletions yash-semantics/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ All notable changes to `yash-semantics` will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [0.2.0] - Unreleased

### Changed

- `<expansion::Error as handle::Handle>::handle` now returns `Divert::Exit`
instead of `Divert::Interrupt` when the `ErrExit` shell option is applicable.

## [0.1.0] - 2024-04-13

### Added
Expand Down
5 changes: 2 additions & 3 deletions yash-semantics/src/command/compound_command/case.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use crate::xtrace::XTrace;
use crate::Handle;
use std::fmt::Write;
use std::ops::ControlFlow::Continue;
use yash_env::semantics::apply_errexit;
use yash_env::semantics::ExitStatus;
use yash_env::semantics::Result;
use yash_env::Env;
Expand Down Expand Up @@ -56,15 +55,15 @@ fn config() -> Config {
pub async fn execute(env: &mut Env, subject: &Word, items: &[CaseItem]) -> Result {
let subject = match expand_word(env, subject).await {
Ok((expansion, _exit_status)) => expansion,
Err(error) => return apply_errexit(error.handle(env).await, env),
Err(error) => return error.handle(env).await,
};
trace_subject(env, &subject.value).await;

'outer: for item in items {
for pattern in &item.patterns {
let mut pattern = match expand_word_attr(env, pattern).await {
Ok((expansion, _exit_status)) => expansion,
Err(error) => return apply_errexit(error.handle(env).await, env),
Err(error) => return error.handle(env).await,
};

// Unquoted backslashes should act as quoting, as required by POSIX XCU 2.13.1
Expand Down
7 changes: 3 additions & 4 deletions yash-semantics/src/command/compound_command/for_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use crate::xtrace::XTrace;
use crate::Handle;
use std::fmt::Write;
use std::ops::ControlFlow::{Break, Continue};
use yash_env::semantics::apply_errexit;
use yash_env::semantics::Divert;
use yash_env::semantics::Field;
use yash_env::semantics::Result;
Expand All @@ -48,13 +47,13 @@ pub async fn execute(
) -> Result {
let (name, _) = match expand_word(env, name).await {
Ok(word) => word,
Err(error) => return apply_errexit(error.handle(env).await, env),
Err(error) => return error.handle(env).await,
};

let values = if let Some(words) = values {
match expand_words(env, words).await {
Ok((fields, _)) => fields,
Err(error) => return apply_errexit(error.handle(env).await, env),
Err(error) => return error.handle(env).await,
}
} else {
env.variables
Expand Down Expand Up @@ -92,7 +91,7 @@ pub async fn execute(
});
let location = name.origin;
let error = Error { cause, location };
return apply_errexit(error.handle(env).await, env);
return error.handle(env).await;
}
};
}
Expand Down
15 changes: 13 additions & 2 deletions yash-semantics/src/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,16 +46,27 @@ impl Handle for yash_syntax::parser::Error {
}
}

/// Prints an error message and sets the exit status to non-zero.
/// Prints an error message and returns a divert result indicating a non-zero
/// exit status.
///
/// This implementation handles the error by printing an error message to the
/// standard error and returning `Divert::Interrupt(Some(ExitStatus::ERROR))`.
/// If the [`ErrExit`] option is set, `Divert::Exit(Some(ExitStatus::ERROR))` is
/// returned instead.
///
/// Note that other POSIX-compliant implementations may use different non-zero
/// exit statuses.
///
/// [`ErrExit`]: yash_env::option::Option::ErrExit
impl Handle for crate::expansion::Error {
async fn handle(&self, env: &mut Env) -> super::Result {
print_message(env, self).await;
Break(Divert::Interrupt(Some(ExitStatus::ERROR)))

if env.errexit_is_applicable() {
Break(Divert::Exit(Some(ExitStatus::ERROR)))
} else {
Break(Divert::Interrupt(Some(ExitStatus::ERROR)))
}
}
}

Expand Down

0 comments on commit 362b7c9

Please sign in to comment.