From: Mark Thom Date: Mon, 22 Feb 2021 02:18:06 +0000 (-0700) Subject: issue singleton variable warnings from loader.pl (#812) X-Git-Tag: v0.9.0~150^2~23 X-Git-Url: https://git.sagredo.dev/?a=commitdiff_plain;h=0ef5f7f9b1ef7ebbf6622b8830ec970d644159b3;p=scryer-prolog.git issue singleton variable warnings from loader.pl (#812) --- diff --git a/crates/prolog_parser/src/parser.rs b/crates/prolog_parser/src/parser.rs index a623f9f4..0fa142f6 100644 --- a/crates/prolog_parser/src/parser.rs +++ b/crates/prolog_parser/src/parser.rs @@ -965,6 +965,12 @@ impl<'a, R: Read> Parser<'a, R> { self.lexer.eof() } + #[inline] + pub fn num_lines_read(&self) -> usize { + self.lexer.line_num + } + + // on success, returns the parsed term and the number of lines read. pub fn read_term(&mut self, op_dir: &CompositeOpDir) -> Result { self.tokens = read_tokens(&mut self.lexer)?; @@ -998,18 +1004,4 @@ impl<'a, R: Read> Parser<'a, R> { )), } } - - pub fn read(&mut self, op_dir: &CompositeOpDir) -> Result, ParserError> { - let mut terms = Vec::new(); - - loop { - terms.push(self.read_term(op_dir)?); - - if self.lexer.eof()? { - break; - } - } - - Ok(terms) - } } diff --git a/src/clause_types.rs b/src/clause_types.rs index 382dd6ae..70736282 100644 --- a/src/clause_types.rs +++ b/src/clause_types.rs @@ -422,6 +422,9 @@ impl SystemClauseType { &SystemClauseType::REPL(REPLCodePtr::RemoveModuleExports) => { clause_name!("$remove_module_exports") } + &SystemClauseType::REPL(REPLCodePtr::AddNonCountedBacktracking) => { + clause_name!("$add_non_counted_backtracking") + } &SystemClauseType::Close => clause_name!("$close"), &SystemClauseType::CopyToLiftedHeap => clause_name!("$copy_to_lh"), &SystemClauseType::DeleteAttribute => clause_name!("$del_attr_non_head"), @@ -778,6 +781,9 @@ impl SystemClauseType { ("$remove_module_exports", 2) => { Some(SystemClauseType::REPL(REPLCodePtr::RemoveModuleExports)) } + ("$add_non_counted_backtracking", 3) => { + Some(SystemClauseType::REPL(REPLCodePtr::AddNonCountedBacktracking)) + } ("$variant", 2) => Some(SystemClauseType::Variant), ("$wam_instructions", 4) => Some(SystemClauseType::WAMInstructions), ("$write_term", 7) => Some(SystemClauseType::WriteTerm), diff --git a/src/lib/builtins.pl b/src/lib/builtins.pl index 975a07c1..abf650c2 100644 --- a/src/lib/builtins.pl +++ b/src/lib/builtins.pl @@ -1516,16 +1516,21 @@ peek_char(S, C) :- '$peek_char'(S, C). +is_stream_position(position_and_lines_read(P, L)) :- + ( var(P) ; integer(P), P >= 0 ), + ( var(L) ; integer(L), L >= 0 ), + !. + +check_stream_property(D, direction, D) :- + ( var(D) -> true ; lists:member(D, [input, output, input_output]), ! ). check_stream_property(file_name(F), file_name, F) :- ( var(F) -> true ; atom(F) ). check_stream_property(mode(M), mode, M) :- ( var(M) -> true ; lists:member(M, [read, write, append]) ). -check_stream_property(D, direction, D) :- - ( var(D) -> true ; lists:member(D, [input, output, input_output]), ! ). check_stream_property(alias(A), alias, A) :- ( var(A) -> true ; atom(A) ). check_stream_property(position(P), position, P) :- - ( var(P) -> true ; integer(P), P >= 0 ). + ( var(P) -> true ; is_stream_position(P)). check_stream_property(end_of_stream(E), end_of_stream, E) :- ( var(E) -> true ; lists:member(E, [not, at, past]) ). check_stream_property(eof_action(A), eof_action, A) :- @@ -1576,8 +1581,8 @@ at_end_of_stream :- set_stream_position(S_or_a, Position) :- ( var(Position) -> throw(error(instantiation_error, set_stream_position/2)) - ; integer(Position), Position >= 0 -> - true + ; Position = position_and_lines_read(P, _), + is_stream_position(Position) -> + '$set_stream_position'(S_or_a, P) ; throw(error(domain_error(stream_position, Position))) - ), - '$set_stream_position'(S_or_a, Position). + ). diff --git a/src/loader.pl b/src/loader.pl index 5f5a1031..5975dddd 100644 --- a/src/loader.pl +++ b/src/loader.pl @@ -110,14 +110,55 @@ load(Stream) :- false. %% Clear the heap. load(_). + +print_comma_separated_list([VN=_]) :- + write(VN), + !. +print_comma_separated_list([VN=_, VNEq | VNEqs]) :- + write(VN), + write(', '), + print_comma_separated_list([VNEq | VNEqs]). + + +filter_anonymous_vars([], []). +filter_anonymous_vars([VN=V | VNEqs0], VNEqs) :- + ( atom_concat('_', _, VN) -> + filter_anonymous_vars(VNEqs0, VNEqs) + ; VNEqs = [VN=V | VNEqs1], + filter_anonymous_vars(VNEqs0, VNEqs1) + ). + +warn_about_singletons([], _). +warn_about_singletons([Singleton|Singletons], LinesRead) :- + ( LinesRead =:= -1 -> + true + ; filter_anonymous_vars([Singleton|Singletons], VarEqs), + VarEqs \== [] -> + write('Warning: singleton variables '), + print_comma_separated_list(VarEqs), + write(' at line '), + write(LinesRead), + write(' of '), + load_context(Module), + write(Module), + nl + ; true + ). + + load_loop(Stream, Evacuable) :- + ( stream_property(Stream, position(position_and_lines_read(_, LinesRead))) -> + true + ; LinesRead = -1 + ), read_term(Stream, Term, [singletons(Singletons)]), ( Term == end_of_file -> close(Stream), '$conclude_load'(Evacuable) ; var(Term) -> instantiation_error(load/1) - ; compile_term(Term, Evacuable), + ; warn_about_singletons(Singletons, LinesRead), + compile_term(Term, Evacuable), load_loop(Stream, Evacuable) ). @@ -308,6 +349,13 @@ compile_declaration(initialization(Goal), Evacuable) :- assertz(Module:'$initialization_goals'(Goal)). compile_declaration(set_prolog_flag(Flag, Value), _) :- set_prolog_flag(Flag, Value). +compile_declaration(non_counted_backtracking(Name/Arity), Evacuable) :- + must_be(atom, Name), + must_be(integer, Arity), + ( Arity >= 0 -> + '$add_non_counted_backtracking'(Name, Arity, Evacuable) + ; domain_error(not_less_than_zero, Arity, load/1) + ). compile_clause((Target:Head :- Body), Evacuable) :- diff --git a/src/machine/loader.rs b/src/machine/loader.rs index d4b5e662..afa193d3 100644 --- a/src/machine/loader.rs +++ b/src/machine/loader.rs @@ -1947,6 +1947,18 @@ impl Machine { self.restore_load_state_payload(result, evacuable_h); } + pub(crate) fn add_non_counted_backtracking(&mut self) { + let key = self + .machine_st + .read_predicate_key(self.machine_st[temp_v!(1)], self.machine_st[temp_v!(2)]); + + let (mut loader, evacuable_h) = self.loader_from_heap_evacuable(temp_v!(3)); + loader.non_counted_bt_preds.insert(key); + + let result = LiveTermStream::evacuate(loader); + self.restore_load_state_payload(result, evacuable_h); + } + pub(crate) fn meta_predicate_property(&mut self) { let module_name = atom_from!( self.machine_st, diff --git a/src/machine/machine_indices.rs b/src/machine/machine_indices.rs index 43264a3f..336ab16c 100644 --- a/src/machine/machine_indices.rs +++ b/src/machine/machine_indices.rs @@ -449,6 +449,7 @@ pub enum REPLCodePtr { IsConsistentWithTermQueue, FlushTermQueue, RemoveModuleExports, + AddNonCountedBacktracking, } #[derive(Debug, Clone, PartialEq)] diff --git a/src/machine/machine_state.rs b/src/machine/machine_state.rs index ab372549..b7ac7499 100644 --- a/src/machine/machine_state.rs +++ b/src/machine/machine_state.rs @@ -27,6 +27,7 @@ use std::fmt; use std::io::Write; use std::mem; use std::ops::{Index, IndexMut}; +use std::rc::Rc; #[derive(Debug)] pub struct Ball { @@ -299,6 +300,30 @@ pub struct MachineState { impl MachineState { pub(crate) fn read_term(&mut self, mut stream: Stream, indices: &mut IndexStore) -> CallResult { + fn push_var_eq_functors<'a>( + heap: &mut Heap, + iter: impl Iterator, &'a Addr)>, + op_dir: &OpDir, + atom_tbl: TabledData, + ) -> Vec { + let mut list_of_var_eqs = vec![]; + + for (var, binding) in iter { + let var_atom = clause_name!(var.to_string(), atom_tbl); + + let h = heap.h(); + let spec = fetch_atom_op_spec(clause_name!("="), None, op_dir); + + heap.push(HeapCellValue::NamedStr(2, clause_name!("="), spec)); + heap.push(HeapCellValue::Atom(var_atom, None)); + heap.push(HeapCellValue::Addr(*binding)); + + list_of_var_eqs.push(Addr::Str(h)); + } + + list_of_var_eqs + } + self.check_stream_properties( &mut stream, StreamType::Text, @@ -327,46 +352,39 @@ impl MachineState { return Ok(()); } - let mut list_of_var_eqs = vec![]; - - for (var, binding) in term_write_result.var_dict.into_iter() { - let var_atom = clause_name!(var.to_string(), self.atom_tbl); - - let h = self.heap.h(); - let spec = fetch_atom_op_spec(clause_name!("="), None, &indices.op_dir); - - self.heap - .push(HeapCellValue::NamedStr(2, clause_name!("="), spec)); - self.heap.push(HeapCellValue::Atom(var_atom, None)); - self.heap.push(HeapCellValue::Addr(binding)); + let list_of_var_eqs = push_var_eq_functors( + &mut self.heap, + term_write_result.var_dict.iter(), + &indices.op_dir, + self.atom_tbl.clone(), + ); - list_of_var_eqs.push(Addr::Str(h)); - } - - let mut var_set: IndexMap = IndexMap::new(); + let mut singleton_var_set: IndexMap = IndexMap::new(); + let mut var_list = vec![]; for addr in self.acyclic_pre_order_iter(term) { if let Some(var) = addr.as_var() { - if !var_set.contains_key(&var) { - var_set.insert(var, true); + if !singleton_var_set.contains_key(&var) { + singleton_var_set.insert(var, true); + var_list.push(addr); } else { - var_set.insert(var, false); + singleton_var_set.insert(var, false); } } } - let mut var_list = vec![]; - let mut singleton_var_list = vec![]; - - for addr in self.acyclic_pre_order_iter(term) { - if let Some(var) = addr.as_var() { - if var_set.get(&var) == Some(&true) { - singleton_var_list.push(var.as_addr()); + let singleton_var_list = push_var_eq_functors( + &mut self.heap, + term_write_result.var_dict.iter().filter(|(_, binding)| { + if let Some(r) = binding.as_var() { + *singleton_var_set.get(&r).unwrap_or(&false) + } else { + false } - - var_list.push(var.as_addr()); - } - } + }), + &indices.op_dir, + self.atom_tbl.clone(), + ); let singleton_addr = self[temp_v!(3)]; let singletons_offset = diff --git a/src/machine/mod.rs b/src/machine/mod.rs index b9247176..6e3bd4eb 100644 --- a/src/machine/mod.rs +++ b/src/machine/mod.rs @@ -513,6 +513,9 @@ impl Machine { REPLCodePtr::RemoveModuleExports => { self.remove_module_exports(); } + REPLCodePtr::AddNonCountedBacktracking => { + self.add_non_counted_backtracking(); + } } self.machine_st.p = CodePtr::Local(p); diff --git a/src/machine/streams.rs b/src/machine/streams.rs index f4c182ce..4c138a49 100644 --- a/src/machine/streams.rs +++ b/src/machine/streams.rs @@ -202,6 +202,7 @@ struct InnerStream { options: StreamOptions, stream_inst: StreamInstance, past_end_of_stream: bool, + lines_read: usize, } #[derive(Debug, Clone)] @@ -214,7 +215,9 @@ impl WrappedStreamInstance { InnerStream { options: StreamOptions::default(), stream_inst, - past_end_of_stream } + past_end_of_stream, + lines_read: 0, + } ))) } } @@ -363,6 +366,11 @@ impl Stream { ptr as *const u8 } + #[inline] + pub(crate) fn add_lines_read(&mut self, incr_num_lines_read: usize) { + self.stream_inst.0.borrow_mut().lines_read += incr_num_lines_read; + } + #[inline] pub(crate) fn options(&self) -> std::cell::Ref<'_, StreamOptions> { std::cell::Ref::map( @@ -380,11 +388,18 @@ impl Stream { } #[inline] - pub(crate) fn position(&mut self) -> Option { - match self.stream_inst.0.borrow_mut().stream_inst { + pub(crate) fn position(&mut self) -> Option<(u64, usize)> { // returns lines_read, position. + let result = match self.stream_inst.0.borrow_mut().stream_inst { StreamInstance::InputFile(_, ref mut file) => file.seek(SeekFrom::Current(0)).ok(), + StreamInstance::TcpStream(..) | + StreamInstance::TlsStream(..) | + StreamInstance::ReadlineStream(..) | + StreamInstance::StaticStr(..) | + StreamInstance::Bytes(..) => Some(0), _ => None, - } + }; + + result.map(|position| (position, self.stream_inst.0.borrow().lines_read)) } #[inline] @@ -588,6 +603,7 @@ impl Stream { // returns true on success. #[inline] pub(super) fn reset(&mut self) -> bool { + self.stream_inst.0.borrow_mut().lines_read = 0; self.stream_inst.0.borrow_mut().past_end_of_stream = false; loop { diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs index 12039b02..3c30de06 100644 --- a/src/machine/system_calls.rs +++ b/src/machine/system_calls.rs @@ -4534,8 +4534,17 @@ impl MachineState { } } "position" => { - if let Some(position) = stream.position() { - HeapCellValue::Addr(Addr::Usize(position as usize)) + if let Some((position, lines_read)) = stream.position() { + let h = self.heap.h(); + + let position_term = functor!( + "position_and_lines_read", + [integer(position), integer(lines_read)] + ); + + self.heap.extend(position_term.into_iter()); + + HeapCellValue::Addr(Addr::HeapCell(h)) } else { self.fail = true; return Ok(()); @@ -4543,7 +4552,6 @@ impl MachineState { } "end_of_stream" => { let end_of_stream_pos = stream.position_relative_to_end(); - HeapCellValue::Atom(clause_name!(end_of_stream_pos.as_str()), None) } "eof_action" => HeapCellValue::Atom( diff --git a/src/read.rs b/src/read.rs index b55849a9..4cb8e779 100644 --- a/src/read.rs +++ b/src/read.rs @@ -184,15 +184,19 @@ impl MachineState { ) -> Result { let mut stream = parsing_stream(inner.clone())?; - let term = { + let (term, num_lines_read) = { let mut parser = Parser::new(&mut stream, atom_tbl, self.flags); - parser.read_term(&CompositeOpDir::new(op_dir, None))? + let term = parser.read_term(&CompositeOpDir::new(op_dir, None))?; + + (term, parser.num_lines_read()) }; // 'pausing' the stream saves the pending top buffer // created by the parsing stream, which was created in this // scope and is about to be destroyed in it. + inner.add_lines_read(num_lines_read); + let buf = stream.take_buf(); inner.pause_stream(buf)?; diff --git a/src/write.rs b/src/write.rs index 52263c2f..e98b550f 100644 --- a/src/write.rs +++ b/src/write.rs @@ -85,6 +85,8 @@ impl fmt::Display for REPLCodePtr { write!(f, "REPLCodePtr::FlushTermQueue"), REPLCodePtr::RemoveModuleExports => write!(f, "REPLCodePtr::RemoveModuleExports"), + REPLCodePtr::AddNonCountedBacktracking => + write!(f, "REPLCodePtr::AddNonCountedBacktracking"), } } }