Remove broken compile-time overload system (#9677)

# Description

This PR removes the compile-time overload system. Unfortunately, this
system never worked correctly because in a gradual type system where
types can be `Any`, you don't have enough information to correctly
resolve function calls with overloads. These resolutions must be done at
runtime, if they're supported.

That said, there's a bit of work that needs to go into resolving
input/output types (here overloads do not execute separate commands, but
the same command and each overload explains how each output type
corresponds to input types).

This PR also removes the type scope, which would give incorrect answers
in cases where multiple subexpressions were used in a pipeline.

# User-Facing Changes

Finishes removing compile-time overloads. These were only used in a few
places in the code base, but it's possible it may impact user code. I'll
mark this as breaking change so we can review.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
This commit is contained in:
JT
2023-07-14 07:05:03 +12:00
committed by GitHub
parent 99329f14a3
commit 30904bd095
7 changed files with 118 additions and 445 deletions

View File

@ -621,7 +621,7 @@ impl EngineState {
for overlay_frame in self.active_overlays(removed_overlays).rev() {
visibility.append(&overlay_frame.visibility);
if let Some(decl_id) = overlay_frame.get_decl(name, &Type::Any) {
if let Some(decl_id) = overlay_frame.get_decl(name) {
if visibility.is_decl_id_visible(&decl_id) {
return Some(decl_id);
}
@ -638,7 +638,7 @@ impl EngineState {
visibility.append(&overlay_frame.visibility);
if visibility.is_decl_id_visible(&decl_id) {
for ((name, _), id) in overlay_frame.decls.iter() {
for (name, id) in overlay_frame.decls.iter() {
if id == &decl_id {
return Some(name);
}
@ -714,12 +714,12 @@ impl EngineState {
for overlay_frame in self.active_overlays(&[]).rev() {
for decl in &overlay_frame.decls {
if overlay_frame.visibility.is_decl_id_visible(decl.1) && predicate(&decl.0 .0) {
if overlay_frame.visibility.is_decl_id_visible(decl.1) && predicate(decl.0) {
let command = self.get_decl(*decl.1);
if ignore_deprecated && command.signature().category == Category::Deprecated {
continue;
}
output.push((decl.0 .0.clone(), Some(command.usage().to_string())));
output.push((decl.0.clone(), Some(command.usage().to_string())));
}
}
}
@ -790,7 +790,7 @@ impl EngineState {
}
let mut decls: Vec<(Vec<u8>, DeclId)> =
decls_map.into_iter().map(|(v, k)| (v.0, k)).collect();
decls_map.into_iter().map(|(v, k)| (v, k)).collect();
decls.sort_by(|a, b| a.0.cmp(&b.0));
decls.into_iter()
@ -953,7 +953,6 @@ pub struct StateWorkingSet<'a> {
pub permanent_state: &'a EngineState,
pub delta: StateDelta,
pub external_commands: Vec<Vec<u8>>,
pub type_scope: TypeScope,
/// Current working directory relative to the file being parsed right now
pub currently_parsed_cwd: Option<PathBuf>,
/// All previously parsed module files. Used to protect against circular imports.
@ -963,54 +962,6 @@ pub struct StateWorkingSet<'a> {
pub parse_errors: Vec<ParseError>,
}
/// A temporary placeholder for expression types. It is used to keep track of the input types
/// for each expression in a pipeline
pub struct TypeScope {
/// Layers that map the type inputs that are found in each parsed block
outputs: Vec<Vec<Type>>,
/// The last know output from a parsed block
last_output: Type,
}
impl Default for TypeScope {
fn default() -> Self {
Self {
outputs: Vec::new(),
last_output: Type::Any,
}
}
}
impl TypeScope {
pub fn get_previous(&self) -> &Type {
match self.outputs.last().and_then(|v| v.last()) {
Some(input) => input,
None => &Type::Nothing,
}
}
pub fn get_last_output(&self) -> Type {
self.last_output.clone()
}
pub fn add_type(&mut self, input: Type) {
if let Some(v) = self.outputs.last_mut() {
v.push(input)
} else {
self.outputs.push(vec![input])
}
}
pub fn enter_scope(&mut self) {
self.outputs.push(Vec::new())
}
pub fn exit_scope(&mut self) -> Option<Vec<Type>> {
self.last_output = self.get_previous().clone();
self.outputs.pop()
}
}
/// A delta (or change set) between the current global state and a possible future global state. Deltas
/// can be applied to the global state to update it to contain both previous state and the state held
/// within the delta.
@ -1141,7 +1092,6 @@ impl<'a> StateWorkingSet<'a> {
delta: StateDelta::new(permanent_state),
permanent_state,
external_commands: vec![],
type_scope: TypeScope::default(),
currently_parsed_cwd: permanent_state.currently_parsed_cwd.clone(),
parsed_module_files: vec![],
search_predecls: true,
@ -1197,13 +1147,11 @@ impl<'a> StateWorkingSet<'a> {
pub fn add_decl(&mut self, decl: Box<dyn Command>) -> DeclId {
let name = decl.name().as_bytes().to_vec();
let input_type = decl.signature().get_input_type();
self.delta.decls.push(decl);
let decl_id = self.num_decls() - 1;
self.last_overlay_mut()
.insert_decl(name, input_type, decl_id);
self.last_overlay_mut().insert_decl(name, decl_id);
decl_id
}
@ -1212,7 +1160,7 @@ impl<'a> StateWorkingSet<'a> {
let overlay_frame = self.last_overlay_mut();
for (name, decl_id) in decls {
overlay_frame.insert_decl(name, Type::Any, decl_id);
overlay_frame.insert_decl(name, decl_id);
overlay_frame.visibility.use_decl_id(&decl_id);
}
}
@ -1249,7 +1197,7 @@ impl<'a> StateWorkingSet<'a> {
let overlay_frame = self.last_overlay_mut();
if let Some(decl_id) = overlay_frame.predecls.remove(name) {
overlay_frame.insert_decl(name.into(), Type::Any, decl_id);
overlay_frame.insert_decl(name.into(), decl_id);
return Some(decl_id);
}
@ -1279,7 +1227,7 @@ impl<'a> StateWorkingSet<'a> {
visibility.append(&overlay_frame.visibility);
if let Some(decl_id) = overlay_frame.get_decl(name, &Type::Any) {
if let Some(decl_id) = overlay_frame.get_decl(name) {
if visibility.is_decl_id_visible(&decl_id) {
// Hide decl only if it's not already hidden
overlay_frame.visibility.hide_decl_id(&decl_id);
@ -1298,7 +1246,7 @@ impl<'a> StateWorkingSet<'a> {
{
visibility.append(&overlay_frame.visibility);
if let Some(decl_id) = overlay_frame.get_decl(name, &Type::Any) {
if let Some(decl_id) = overlay_frame.get_decl(name) {
if visibility.is_decl_id_visible(&decl_id) {
// Hide decl only if it's not already hidden
self.last_overlay_mut().visibility.hide_decl_id(&decl_id);
@ -1467,7 +1415,7 @@ impl<'a> StateWorkingSet<'a> {
None
}
pub fn find_decl(&self, name: &[u8], input: &Type) -> Option<DeclId> {
pub fn find_decl(&self, name: &[u8]) -> Option<DeclId> {
let mut removed_overlays = vec![];
let mut visibility: Visibility = Visibility::new();
@ -1493,7 +1441,7 @@ impl<'a> StateWorkingSet<'a> {
}
}
if let Some(decl_id) = overlay_frame.get_decl(name, input) {
if let Some(decl_id) = overlay_frame.get_decl(name) {
if visibility.is_decl_id_visible(&decl_id) {
return Some(decl_id);
}
@ -1509,7 +1457,7 @@ impl<'a> StateWorkingSet<'a> {
{
visibility.append(&overlay_frame.visibility);
if let Some(decl_id) = overlay_frame.get_decl(name, input) {
if let Some(decl_id) = overlay_frame.get_decl(name) {
if visibility.is_decl_id_visible(&decl_id) {
return Some(decl_id);
}
@ -1549,7 +1497,7 @@ impl<'a> StateWorkingSet<'a> {
for scope_frame in self.delta.scope.iter().rev() {
for overlay_frame in scope_frame.active_overlays(&mut removed_overlays).rev() {
for decl in &overlay_frame.decls {
if decl.0 .0.starts_with(name) {
if decl.0.starts_with(name) {
return true;
}
}
@ -1562,7 +1510,7 @@ impl<'a> StateWorkingSet<'a> {
.rev()
{
for decl in &overlay_frame.decls {
if decl.0 .0.starts_with(name) {
if decl.0.starts_with(name) {
return true;
}
}
@ -1768,14 +1716,13 @@ impl<'a> StateWorkingSet<'a> {
let overlay_frame = scope_frame.get_overlay(*overlay_id);
for decl in &overlay_frame.decls {
if overlay_frame.visibility.is_decl_id_visible(decl.1) && predicate(&decl.0 .0)
{
if overlay_frame.visibility.is_decl_id_visible(decl.1) && predicate(decl.0) {
let command = self.get_decl(*decl.1);
if ignore_deprecated && command.signature().category == Category::Deprecated
{
continue;
}
output.push((decl.0 .0.clone(), Some(command.usage().to_string())));
output.push((decl.0.clone(), Some(command.usage().to_string())));
}
}
}
@ -1908,7 +1855,7 @@ impl<'a> StateWorkingSet<'a> {
let overlay_frame = self.permanent_state.get_overlay(overlay_id);
for (decl_key, decl_id) in &overlay_frame.decls {
result.insert(decl_key.0.to_owned(), *decl_id);
result.insert(decl_key.to_owned(), *decl_id);
}
}
@ -1917,7 +1864,7 @@ impl<'a> StateWorkingSet<'a> {
let overlay_frame = scope_frame.get_overlay(overlay_id);
for (decl_key, decl_id) in &overlay_frame.decls {
result.insert(decl_key.0.to_owned(), *decl_id);
result.insert(decl_key.to_owned(), *decl_id);
}
}
}

View File

@ -1,7 +1,5 @@
use crate::{DeclId, ModuleId, OverlayId, Type, Value, VarId};
use std::borrow::Borrow;
use crate::{DeclId, ModuleId, OverlayId, Value, VarId};
use std::collections::HashMap;
use std::hash::{Hash, Hasher};
pub static DEFAULT_OVERLAY_NAME: &str = "zero";
@ -181,7 +179,7 @@ pub struct OverlayFrame {
pub vars: HashMap<Vec<u8>, VarId>,
pub constants: HashMap<VarId, Value>,
pub predecls: HashMap<Vec<u8>, DeclId>, // temporary storage for predeclarations
pub decls: HashMap<(Vec<u8>, Type), DeclId>,
pub decls: HashMap<Vec<u8>, DeclId>,
pub modules: HashMap<Vec<u8>, ModuleId>,
pub visibility: Visibility,
pub origin: ModuleId, // The original module the overlay was created from
@ -202,82 +200,16 @@ impl OverlayFrame {
}
}
pub fn insert_decl(&mut self, name: Vec<u8>, input: Type, decl_id: DeclId) -> Option<DeclId> {
self.decls.insert((name, input), decl_id)
pub fn insert_decl(&mut self, name: Vec<u8>, decl_id: DeclId) -> Option<DeclId> {
self.decls.insert(name, decl_id)
}
pub fn insert_module(&mut self, name: Vec<u8>, module_id: ModuleId) -> Option<ModuleId> {
self.modules.insert(name, module_id)
}
pub fn get_decl(&self, name: &[u8], input: &Type) -> Option<DeclId> {
if let Some(decl) = self.decls.get(&(name, input) as &dyn DeclKey) {
Some(*decl)
} else {
// then fallback to not using the input type
for decl_key in self.decls.keys() {
if decl_key.0 == name {
// FIXME: this fallback may give bad type information
// in the case where no matching type is found. But, at
// least we treat it as a found internal command rather
// than an external command, which would cause further issues
return Some(
*self
.decls
.get(decl_key)
.expect("internal error: found decl not actually found"),
);
}
}
None
}
}
}
trait DeclKey {
fn name(&self) -> &[u8];
fn input(&self) -> &Type;
}
impl Hash for dyn DeclKey + '_ {
fn hash<H: Hasher>(&self, state: &mut H) {
self.name().hash(state);
self.input().hash(state);
}
}
impl PartialEq for dyn DeclKey + '_ {
fn eq(&self, other: &Self) -> bool {
self.name() == other.name() && self.input() == other.input()
}
}
impl Eq for dyn DeclKey + '_ {}
impl<'a> DeclKey for (&'a [u8], &Type) {
fn name(&self) -> &[u8] {
self.0
}
fn input(&self) -> &Type {
self.1
}
}
impl DeclKey for (Vec<u8>, Type) {
fn name(&self) -> &[u8] {
&self.0
}
fn input(&self) -> &Type {
&self.1
}
}
impl<'a> Borrow<dyn DeclKey + 'a> for (Vec<u8>, Type) {
fn borrow(&self) -> &(dyn DeclKey + 'a) {
self
pub fn get_decl(&self, name: &[u8]) -> Option<DeclId> {
self.decls.get(name).cloned()
}
}