From 86c90d77dd116694c1c8ec442ff227aa26c0e6ae Mon Sep 17 00:00:00 2001 From: Mark Date: Sun, 16 Jul 2023 20:42:40 -0600 Subject: [PATCH 1/2] do a better job handling EOF in read_term (#1887) --- src/machine/machine_state.rs | 31 ++++++----- src/machine/streams.rs | 102 ++++++++++++++++++++++++---------- src/machine/system_calls.rs | 105 +++++++++++++++++------------------ src/read.rs | 21 ++++++- 4 files changed, 162 insertions(+), 97 deletions(-) diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs index 499dce55..c8a9cbe0 100644 --- a/src/machine/machine_state.rs +++ b/src/machine/machine_state.rs @@ -616,7 +616,7 @@ impl MachineState { unreachable!("Stream must be a Stream::Readline(_)") } - pub fn read_term(&mut self, stream: Stream, indices: &mut IndexStore) -> CallResult { + pub fn read_term(&mut self, mut stream: Stream, indices: &mut IndexStore) -> CallResult { self.check_stream_properties( stream, StreamType::Text, @@ -637,22 +637,27 @@ impl MachineState { match self.read(stream, &indices.op_dir) { Ok(term_write_result) => return self.read_term_body(term_write_result), Err(err) => { - match err { + match &err { CompilationError::ParserError(e) if e.is_unexpected_eof() => { - self.eof_action( - self.registers[2], - stream, - atom!("read_term"), - 3, - )?; + if stream.at_end_of_stream() { + unify!(self, self.registers[2], atom_as_cell!(atom!("end_of_file"))); + return Ok(()); + } else if stream.past_end_of_stream() { + self.eof_action( + self.registers[2], + stream, + atom!("read_term"), + 3, + )?; - if stream.options().eof_action() == EOFAction::Reset { - if self.fail == false { - continue; + if stream.options().eof_action() == EOFAction::Reset { + if self.fail == false { + continue; + } } - } - return Ok(()); + return Ok(()); + } } _ => {} } diff --git a/src/machine/streams.rs b/src/machine/streams.rs index c1b4678e..0ea8591d 100644 --- a/src/machine/streams.rs +++ b/src/machine/streams.rs @@ -884,19 +884,38 @@ impl PartialEq for Stream { impl Eq for Stream {} +fn cursor_position(past_end_of_stream: &mut bool, cursor: &Cursor, cursor_len: u64) -> AtEndOfStream { + let position = cursor.position(); + + let at_end_of_stream = match position.cmp(&cursor_len) { + Ordering::Equal => AtEndOfStream::At, + Ordering::Greater => { + *past_end_of_stream = true; + AtEndOfStream::Past + } + Ordering::Less => AtEndOfStream::Not, + }; + + at_end_of_stream +} + impl Stream { #[inline] pub(crate) fn position(&mut self) -> Option<(u64, usize)> { // returns lines_read, position. let result = match self { + Stream::Byte(byte_stream_layout) => { + Some(byte_stream_layout.stream.get_ref().0.position()) + } + Stream::StaticString(string_stream_layout) => { + Some(string_stream_layout.stream.stream.position()) + } Stream::InputFile(file_stream) => { file_stream.position() } - Stream::NamedTcp(..) - | Stream::NamedTls(..) - | Stream::Readline(..) - | Stream::StaticString(..) - | Stream::Byte(..) => Some(0), + Stream::NamedTcp(..) | Stream::NamedTls(..) | Stream::Readline(..) => { + Some(0) + } _ => None, }; @@ -971,38 +990,61 @@ impl Stream { return AtEndOfStream::Past; } - if let Stream::InputFile(stream_layout) = self { - let position = stream_layout.position(); + match self { + Stream::Byte(stream_layout) => { + let StreamLayout { + past_end_of_stream, + stream, + .. + } = &mut **stream_layout; - let StreamLayout { - past_end_of_stream, - stream, - .. - } = &mut **stream_layout; + let cursor_len = stream.get_ref().0.get_ref().len() as u64; + cursor_position(past_end_of_stream, &stream.get_ref().0, cursor_len) + } + Stream::StaticString(stream_layout) => { + let StreamLayout { + past_end_of_stream, + stream, + .. + } = &mut **stream_layout; - match stream.get_ref().file.metadata() { - Ok(metadata) => { - if let Some(position) = position { - return match position.cmp(&metadata.len()) { - Ordering::Equal => AtEndOfStream::At, - Ordering::Less => AtEndOfStream::Not, - Ordering::Greater => { - *past_end_of_stream = true; - AtEndOfStream::Past + let cursor_len = stream.stream.get_ref().len() as u64; + cursor_position(past_end_of_stream, &stream.stream, cursor_len) + } + Stream::InputFile(stream_layout) => { + let position = stream_layout.position(); + + let StreamLayout { + past_end_of_stream, + stream, + .. + } = &mut **stream_layout; + + match stream.get_ref().file.metadata() { + Ok(metadata) => { + if let Some(position) = position { + match position.cmp(&metadata.len()) { + Ordering::Equal => AtEndOfStream::At, + Ordering::Less => AtEndOfStream::Not, + Ordering::Greater => { + *past_end_of_stream = true; + AtEndOfStream::Past + } } - }; - } else { + } else { + *past_end_of_stream = true; + AtEndOfStream::Past + } + } + _ => { *past_end_of_stream = true; AtEndOfStream::Past } } - _ => { - *past_end_of_stream = true; - AtEndOfStream::Past - } } - } else { - AtEndOfStream::Not + _ => { + AtEndOfStream::Not + } } } @@ -1306,7 +1348,7 @@ impl MachineState { match eof_action { EOFAction::Error => { stream.set_past_end_of_stream(true); - return Err(self.open_past_eos_error(stream, caller, arity)); + Err(self.open_past_eos_error(stream, caller, arity)) } EOFAction::EOFCode => { let end_of_stream = if stream.options().stream_type() == StreamType::Binary { diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index f18a5464..86c4c1bd 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -5796,72 +5796,71 @@ impl Machine { self.machine_st.read_term(stream, &mut self.indices) } + #[inline(always)] + fn read_term_and_write_to_heap( + &mut self, + atom_or_string: AtomOrString, + ) -> Result, MachineStub> { + let string = match atom_or_string { + AtomOrString::Atom(atom) if atom == atom!("[]") => "".to_owned(), + _ => atom_or_string.to_string(), + }; + + let chars = CharReader::new(ByteStream::from_string(string)); + let mut parser = Parser::new(chars, &mut self.machine_st); + let op_dir = CompositeOpDir::new(&self.indices.op_dir, None); + + let term_write_result = parser.read_term(&op_dir, Tokens::Default) + .map_err(|err| error_after_read_term(err, 0, &parser)) + .and_then(|term| { + write_term_to_heap( + &term, + &mut self.machine_st.heap, + &mut self.machine_st.atom_tbl, + ) + }); + + match term_write_result { + Ok(term_write_result) => Ok(Some(term_write_result)), + Err(CompilationError::ParserError(e)) if e.is_unexpected_eof() => { + let value = self.machine_st.registers[2]; + self.machine_st.unify_atom(atom!("end_of_file"), value); + + Ok(None) + } + Err(e) => { + let stub = functor_stub(atom!("read_term_from_chars"), 3); + let e = self.machine_st.session_error(SessionError::from(e)); + + Err(self.machine_st.error_form(e, stub)) + } + } + } + #[inline(always)] pub(crate) fn read_from_chars(&mut self) -> CallResult { if let Some(atom_or_string) = self.machine_st.value_to_str_like(self.machine_st.registers[1]) { - let chars = CharReader::new(ByteStream::from_string(atom_or_string.to_string())); - let mut parser = Parser::new(chars, &mut self.machine_st); - let op_dir = CompositeOpDir::new(&self.indices.op_dir, None); + if let Some(term_write_result) = self.read_term_and_write_to_heap(atom_or_string)? { + let result = heap_loc_as_cell!(term_write_result.heap_loc); + let var = self.deref_register(2).as_var().unwrap(); - let term_write_result = parser.read_term(&op_dir, Tokens::Default) - .map_err(CompilationError::from) - .and_then(|term| { - write_term_to_heap( - &term, - &mut self.machine_st.heap, - &mut self.machine_st.atom_tbl, - ) - }); + self.machine_st.bind(var, result); + } - let term_write_result = match term_write_result { - Ok(term_write_result) => term_write_result, - Err(e) => { - let stub = functor_stub(atom!("read_from_chars"), 2); - let e = self.machine_st.session_error(SessionError::from(e)); - - return Err(self.machine_st.error_form(e, stub)); - } - }; - - let result = heap_loc_as_cell!(term_write_result.heap_loc); - let var = self.deref_register(2).as_var().unwrap(); - - self.machine_st.bind(var, result); + Ok(()) } else { unreachable!() } - - Ok(()) } #[inline(always)] pub(crate) fn read_term_from_chars(&mut self) -> CallResult { if let Some(atom_or_string) = self.machine_st.value_to_str_like(self.machine_st.registers[1]) { - let chars = CharReader::new(ByteStream::from_string(atom_or_string.to_string())); - let mut parser = Parser::new(chars, &mut self.machine_st); - let op_dir = CompositeOpDir::new(&self.indices.op_dir, None); - - let term_write_result = parser.read_term(&op_dir, Tokens::Default) - .map_err(CompilationError::from) - .and_then(|term| { - write_term_to_heap( - &term, - &mut self.machine_st.heap, - &mut self.machine_st.atom_tbl, - ) - }); - - let term_write_result = match term_write_result { - Ok(term_write_result) => term_write_result, - Err(e) => { - let stub = functor_stub(atom!("read_term_from_chars"), 3); - let e = self.machine_st.session_error(SessionError::from(e)); - - return Err(self.machine_st.error_form(e, stub)); - } - }; - - self.machine_st.read_term_body(term_write_result) + if let Some(term_write_result) = self.read_term_and_write_to_heap(atom_or_string)? { + self.machine_st.read_term_body(term_write_result) + } else { + Ok(()) + } } else { unreachable!() } diff --git a/src/read.rs b/src/read.rs index 9a191dad..dfba5147 100644 --- a/src/read.rs +++ b/src/read.rs @@ -36,6 +36,25 @@ pub(crate) fn devour_whitespace<'a, R: CharRead>(parser: &mut Parser<'a, R>) -> } } +pub(crate) fn error_after_read_term( + err: ParserError, + prior_num_lines_read: usize, + parser: &Parser, +) -> CompilationError { + if err.is_unexpected_eof() { + let line_num = parser.lexer.line_num; + let col_num = parser.lexer.col_num; + + // rough overlap with errors 8.14.1.3 k) & l) of the ISO standard here + if !(line_num == prior_num_lines_read && col_num == 0) { + return CompilationError::from(ParserError::IncompleteReduction(line_num, col_num)); + } + } + + CompilationError::from(err) +} + + impl MachineState { pub(crate) fn read( &mut self, @@ -50,7 +69,7 @@ impl MachineState { parser.add_lines_read(prior_num_lines_read); let term = parser.read_term(&op_dir, Tokens::Default) - .map_err(CompilationError::from)?; + .map_err(|err| error_after_read_term(err, prior_num_lines_read, &parser))?; // CompilationError::from (term, parser.lines_read() - prior_num_lines_read) }; From a154a34f8746c4af0244a6d5ceb953ae39783fb1 Mon Sep 17 00:00:00 2001 From: Mark Date: Sun, 16 Jul 2023 22:22:44 -0600 Subject: [PATCH 2/2] omit anonymous variables from read_term variable_names and singletons lists --- src/machine/machine_indices.rs | 27 ++++++++++++++++++++++++++- src/machine/machine_state.rs | 12 ++++++++---- src/machine/mock_wam.rs | 7 ++++++- src/read.rs | 24 ++++++++++++++---------- 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/machine/machine_indices.rs b/src/machine/machine_indices.rs index 3e358db9..7389caca 100644 --- a/src/machine/machine_indices.rs +++ b/src/machine/machine_indices.rs @@ -227,7 +227,32 @@ impl CodeIndex { } } -pub(crate) type HeapVarDict = IndexMap; +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum VarKey { + AnonVar(usize), + VarPtr(VarPtr), +} + +impl VarKey { + #[inline] + pub(crate) fn to_string(&self) -> String { + match self { + VarKey::AnonVar(h) => format!("_{}", h), + VarKey::VarPtr(var) => var.borrow().to_string(), + } + } + + #[inline(always)] + pub(crate) fn is_anon(&self) -> bool { + if let VarKey::AnonVar(_) = self { + true + } else { + false + } + } +} + +pub(crate) type HeapVarDict = IndexMap; pub(crate) type GlobalVarDir = IndexMap), FxBuildHasher>; diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs index c8a9cbe0..f7237836 100644 --- a/src/machine/machine_state.rs +++ b/src/machine/machine_state.rs @@ -486,13 +486,13 @@ impl MachineState { pub fn read_term_body(&mut self, mut term_write_result: TermWriteResult) -> CallResult { fn push_var_eq_functors<'a>( heap: &mut Heap, - iter: impl Iterator, + iter: impl Iterator, atom_tbl: &mut AtomTable, ) -> Vec { let mut list_of_var_eqs = vec![]; for (var, binding) in iter { - let var_atom = atom_tbl.build_with(&var.borrow().to_string()); + let var_atom = atom_tbl.build_with(&var.to_string()); let h = heap.len(); heap.push(atom_as_cell!(atom!("="), 2)); @@ -542,7 +542,11 @@ impl MachineState { let singleton_var_list = push_var_eq_functors( &mut self.heap, - term_write_result.var_dict.iter().filter(|(_, binding)| { + term_write_result.var_dict.iter().filter(|(var_name, binding)| { + if var_name.is_anon() { + return false; + } + if let Some(r) = binding.as_var() { *singleton_var_set.get(&r).unwrap_or(&false) } else { @@ -565,7 +569,7 @@ impl MachineState { let list_of_var_eqs = push_var_eq_functors( &mut self.heap, - var_list.iter().map(|(var_name, var,_)| (var_name,var)), + var_list.iter().filter_map(|(var_name, var,_)| if var_name.is_anon() { None } else { Some((var_name,var)) }), &mut self.atom_tbl, ); diff --git a/src/machine/mock_wam.rs b/src/machine/mock_wam.rs index 70264ac9..257735c6 100644 --- a/src/machine/mock_wam.rs +++ b/src/machine/mock_wam.rs @@ -71,7 +71,12 @@ impl MockWAM { printer.var_names = term_write_result .var_dict .into_iter() - .map(|(var, cell)| (cell, var)) + .map(|(var, cell)| { + match var { + VarKey::VarPtr(var) => (cell, var.clone()), + VarKey::AnonVar(_) => (cell, VarPtr::from(var.to_string())) + } + }) .collect(); Ok(printer.print().result()) diff --git a/src/read.rs b/src/read.rs index dfba5147..fac2e4e8 100644 --- a/src/read.rs +++ b/src/read.rs @@ -259,9 +259,9 @@ impl CharRead for ReadlineStream { } #[inline] -pub(crate) fn write_term_to_heap( - term: &Term, - heap: &mut Heap, +pub(crate) fn write_term_to_heap<'a, 'b>( + term: &'a Term, + heap: &'b mut Heap, atom_tbl: &mut AtomTable, ) -> Result { let term_writer = TermWriter::new(heap, atom_tbl); @@ -294,7 +294,7 @@ impl<'a, 'b> TermWriter<'a, 'b> { } #[inline] - fn modify_head_of_queue(&mut self, term: &TermRef<'a>, h: usize) { + fn modify_head_of_queue(&mut self, term: &TermRef, h: usize) { if let Some((arity, site_h)) = self.queue.pop_front() { self.heap[site_h] = self.term_as_addr(term, h); @@ -310,7 +310,7 @@ impl<'a, 'b> TermWriter<'a, 'b> { self.heap.push(heap_loc_as_cell!(h)); } - fn term_as_addr(&mut self, term: &TermRef<'a>, h: usize) -> HeapCellValue { + fn term_as_addr(&mut self, term: &TermRef, h: usize) -> HeapCellValue { match term { &TermRef::Cons(..) => list_loc_as_cell!(h), &TermRef::AnonVar(_) | &TermRef::Var(..) => heap_loc_as_cell!(h), @@ -329,7 +329,7 @@ impl<'a, 'b> TermWriter<'a, 'b> { } } - fn write_term_to_heap(mut self, term: &'a Term) -> Result { + fn write_term_to_heap(mut self, term: &Term) -> Result { let heap_loc = self.heap.len(); for term in breadth_first_iter(term, RootIterationPolicy::Iterated) { @@ -383,17 +383,19 @@ impl<'a, 'b> TermWriter<'a, 'b> { self.push_stub_addr(); } } - &TermRef::AnonVar(Level::Root) | &TermRef::Literal(Level::Root, ..) => { + &TermRef::AnonVar(Level::Root) | TermRef::Literal(Level::Root, ..) => { let addr = self.term_as_addr(&term, h); self.heap.push(addr); } &TermRef::Var(Level::Root, _, ref var_ptr) => { let addr = self.term_as_addr(&term, h); - self.var_dict.insert(var_ptr.clone(), heap_loc_as_cell!(h)); + self.var_dict.insert(VarKey::VarPtr(var_ptr.clone()), addr); self.heap.push(addr); } &TermRef::AnonVar(_) => { if let Some((arity, site_h)) = self.queue.pop_front() { + self.var_dict.insert(VarKey::AnonVar(h), heap_loc_as_cell!(site_h)); + if arity > 1 { self.queue.push_front((arity - 1, site_h + 1)); } @@ -422,10 +424,12 @@ impl<'a, 'b> TermWriter<'a, 'b> { } &TermRef::Var(_, _, ref var) => { if let Some((arity, site_h)) = self.queue.pop_front() { - if let Some(addr) = self.var_dict.get(var).cloned() { + let var_key = VarKey::VarPtr(var.clone()); + + if let Some(addr) = self.var_dict.get(&var_key).cloned() { self.heap[site_h] = addr; } else { - self.var_dict.insert(var.clone(), heap_loc_as_cell!(site_h)); + self.var_dict.insert(var_key, heap_loc_as_cell!(site_h)); } if arity > 1 {