diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ea9519d..7397c7ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ or footers - The DLS will no longer fail to parse files with unicode characters in cblock header or footers, or in multiline comments +- The DLS will now report when an ambiguous default call is made +- Improvements and clarifications to connections between symbols and references, + for details, see [USAGE.md](USAGE.md). +-- Method declared in unrelated templates in an allowed way will now have their + references correctly resolved. +-- Goto-reference on default calls will now go to the methods that may be called. +-- Goto-implementations on templates will now go to all places where they are + instantiated. +-- Goto-implementations on objects will now go to all the 'in each' declarations + which apply to that object. +- Added parser support for provisional 'explicit\_method\_decls' +- The DLS will now correctly report missing template names in 'in each' constructs ## 0.9.17 - Fixed linter wrongly throwing an error on space after `defined` keyword @@ -21,7 +33,7 @@ ## 0.9.15 - Added support for line length and breaking rules regarding line-breaks after opening parentheses, method output arguments, conditional expressions and binary operands. -- Added support for indendation rule indent_continuation_line. +- Added support for indendation rule indent\_continuation\_line. - Optimizations in how the server resolves file paths, should reduce time-to-ready for the server when first starting by about 50%, depending on the complexity of the include tree. diff --git a/USAGE.md b/USAGE.md index 0d397092..ab1d23ad 100644 --- a/USAGE.md +++ b/USAGE.md @@ -12,6 +12,78 @@ details consult the documentation of the client. As this file is currently a work in progress, relevant details may be missing or incomplete. +## Clarification of semantics of Symbol Lookups +DML is at its core a _declarative_ language centered around a Device, +and declarations are merged together according to the hierarchical +structure. Thus, the meaning of `goto-definition`, `goto-declaration`, +`goto-implementations`, and `goto-references` may not be similar to how +they behave in other languages. +Here is a clarification of what each operation means for each object +kind in DML. It is worth noting that unless otherwise specified, a goto- +operation on a reference is equivalent to the same operation on each symbol +that it could refer to. + +*NOTE:* Due to limitations in the DLS, a matching from references to +declarations is not always possible. `goto-references` may provide incomplete +info and `goto-definition` and similar on a reference may fail in some cases. +See issues [#13](https://github.com/intel/dml-language-server/issues/13), +[#23](https://github.com/intel/dml-language-server/issues/23), +[#26](https://github.com/intel/dml-language-server/issues/26), +[#30](https://github.com/intel/dml-language-server/issues/30), and +[#65](https://github.com/intel/dml-language-server/issues/65). + +### On Composite Objects (banks, registers, implements, etc.) +`goto-declaration` and `goto-definition` on a composite object are equivalent. +They will find the locations of all object declarations that may be +merged with the one at the name. + +`goto-implementations` on objects will find all `in each` declaration block that +applies to that object. + +`goto-references` will go to any location where the symbol is referred to +directly. + +### On Methods +`goto-declaration` on a method will find the most-overridden declaration +or definition of the method, this will usually be a 'default' or +abstract declaration. + +`goto-definition` on a method reference will find all definitions of that method +that could be called from that reference, or the declaration of the method if it +has no non-abstract definitions. Note that this will NOT point to +method declarations that are entirely overridden. `goto-definition` on a method +name will go to the nearest definition of the method. For non-abstract methods +this is a no-op. + +`goto-implementations` on a method will find all method declarations that +could override it. + +`goto-references` will go to any location where the method is referred to +directly (including call sites). + +### On Parameters +`goto-declaration` on a parameter will find the most-overridden declaration of +this parameter, usually a 'default' or abstract declaration. + +`goto-definition` on a parameter will find all definitions that are used within +a method. + +`goto-implementations` on a parameter is currently unused. + +`goto-references` will go to any location where the parameter is referred to +directly. + +### On Templates +`goto-declaration` and `goto-definition` are equivalent, and will both give the +template declaration site. + +`goto-implementations` will give each location where the template is directly +instantiated. + +`goto-references` will go to any location where the template is referred to, +including direct instantiation sites (so this is a super-set of +`goto-implementations`) + ## In-Line Linting Configuration It may be desireable to control linting on a per-file basis, rather than relying on the linting configuration. This can be done with in-line diff --git a/src/actions/analysis_queue.rs b/src/actions/analysis_queue.rs index 2ffd15c5..c190bf74 100644 --- a/src/actions/analysis_queue.rs +++ b/src/actions/analysis_queue.rs @@ -257,7 +257,7 @@ impl AnalysisQueue { || !device_lock.is_empty() || !isolated_lock.is_empty(); if has_work { - debug!("Queue still has work ({:?}, {:?}, {:?})", + trace!("Queue still has work ({:?}, {:?}, {:?})", queue_lock, device_lock, isolated_lock); } has_work @@ -296,7 +296,7 @@ impl AnalysisQueue { for job in queue_lock.iter() { if let QueuedJob::IsolatedAnalysisJob(ijob) = job { if paths.contains(&ijob.path) { - debug!("Detected there is still isolated \ + trace!("Detected there is still isolated \ work in queue on {:?}", &ijob.path); return true; @@ -308,7 +308,7 @@ impl AnalysisQueue { let isolated_lock = self.isolated_tracker.lock().unwrap(); for (_, path) in isolated_lock.iter() { if paths.contains(path) { - debug!("Detected there is still isolated \ + trace!("Detected there is still isolated \ work in-flight on {:?}", path); return true; @@ -326,7 +326,7 @@ impl AnalysisQueue { for job in queue_lock.iter() { if let QueuedJob::DeviceAnalysisJob(ijob) = job { if paths.contains(&ijob.root.path) { - debug!("Detected there is still device \ + trace!("Detected there is still device \ work in queue on {:?}", &ijob.root.path); return true; @@ -338,7 +338,7 @@ impl AnalysisQueue { let device_lock = self.device_tracker.lock().unwrap(); for (path, _) in device_lock.values() { if paths.contains(path) { - debug!("Detected there is still device \ + trace!("Detected there is still device \ work in-flight on {:?}", path); return true; @@ -613,7 +613,7 @@ impl LinterJob { self.file.clone())).ok(); }, Err(e) => { - trace!("Failed to create isolated linter analysis: {}", e); + debug!("Failed to create isolated linter analysis: {}", e); } } } diff --git a/src/actions/analysis_storage.rs b/src/actions/analysis_storage.rs index 34853b04..5f5b6edf 100644 --- a/src/actions/analysis_storage.rs +++ b/src/actions/analysis_storage.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 and MIT //! Stores currently completed analysis. -use log::{debug, trace, info}; +use log::{debug, info, trace}; use crossbeam::channel; @@ -14,13 +14,12 @@ use std::sync::Mutex; use std::time::{Duration, SystemTime}; use crate::actions::ContextDefinition; -use crate::analysis::scope::{ContextedSymbol, ContextKey}; +use crate::analysis::scope::ContextKey; use crate::analysis::structure::objects::Import; -use crate::analysis::{IsolatedAnalysis, DeviceAnalysis, DMLError}; +use crate::analysis::{DMLError, DeviceAnalysis, IsolatedAnalysis}; use crate::lsp_data::*; use crate::analysis::parsing::tree::{ZeroSpan, ZeroFilePosition}; -use crate::analysis::reference::Reference; use crate::server::ServerToHandle; use crate::lint::LinterAnalysis; @@ -211,27 +210,6 @@ impl AnalysisStorage { } } - pub fn context_symbol_at_pos<'t>(&'t self, pos: &ZeroFilePosition) -> - Result>, AnalysisLookupError> { - let canon_path = CanonPath::from_path_buf(pos.path()) - .ok_or(AnalysisLookupError::NoFile)?; - let analysis = self.get_isolated_analysis(&canon_path)?; - let mut context = analysis.lookup_context_symbol(pos); - // Patch out leading 'device' context, unneeded - if let Some(ref mut ic) = context { - ic.remove_head_context(); - } - Ok(context) - } - - pub fn reference_at_pos(&self, pos: &ZeroFilePosition) -> - Result, AnalysisLookupError> { - let canon_path = CanonPath::from_path_buf(pos.path()) - .ok_or(AnalysisLookupError::NoFile)?; - let analysis = self.get_isolated_analysis(&canon_path)?; - Ok(analysis.lookup_reference(pos)) - } - pub fn first_context_at_pos(&self, pos: &ZeroFilePosition) -> Result, AnalysisLookupError> { let canon_path = CanonPath::from_path_buf(pos.path()) @@ -306,7 +284,7 @@ impl AnalysisStorage { }; contexts.insert(None); - debug!("Full contexts for {:?} are {:?}", path, contexts); + trace!("Full contexts for {:?} are {:?}", path, contexts); if self.get_isolated_analysis(path).map_or( false, |a|a.is_device_file()) { @@ -489,7 +467,7 @@ impl AnalysisStorage { pub fn update_analysis(&mut self, resolver: &PathResolver) { let mut device_analysis = vec![]; let mut results_holder = vec![]; - debug!("Updating stored analysises"); + trace!("Updating stored analysises"); for r in self.results.try_iter() { results_holder.push(r); } @@ -566,7 +544,7 @@ impl AnalysisStorage { |i|!timestamp_is_newer(timestamp, i.timestamp)) .unwrap_or(false)) { - debug!("was not invalidated by recent \ + trace!("was not invalidated by recent \ isolated analysis"); self.device_analysis.insert(canon_path, TimestampedStorage { @@ -628,7 +606,7 @@ impl AnalysisStorage { fn update_last_use(&self, path: &CanonPath) { if let Some(mut_lock) = self.last_use.get(path) { let now = SystemTime::now(); - debug!("Updated last-use of {} to {:?}", path.as_str(), now); + trace!("Updated last-use of {} to {:?}", path.as_str(), now); *mut_lock.lock().unwrap() = now; } } @@ -677,7 +655,7 @@ impl AnalysisStorage { if let Some(device_timestamp) = self.device_analysis.get(path) .map(|device|device.timestamp) { - debug!("Timestamp is {:?}", device_timestamp); + trace!("Timestamp is {:?}", device_timestamp); for dependee_path in self.all_dependencies(path, Some(path)) { // NOTE: This means that calling this function with a missing // isolated analysis will not tell you the device needs to be diff --git a/src/actions/hover.rs b/src/actions/hover.rs index 7fddb367..e149bbd0 100644 --- a/src/actions/hover.rs +++ b/src/actions/hover.rs @@ -24,6 +24,6 @@ pub fn tooltip( let hover_span = ctx.convert_pos_to_span(hover_file_path, params.position); // TODO: sort out how to handle hovers, and what info they should provide let contents = vec![]; - debug!("tooltip: contents.len: {}", contents.len()); + trace!("tooltip: contents.len: {}", contents.len()); Ok(Tooltip { contents, range: hover_span.range }) } diff --git a/src/actions/mod.rs b/src/actions/mod.rs index 1791e727..ed91df54 100644 --- a/src/actions/mod.rs +++ b/src/actions/mod.rs @@ -92,6 +92,7 @@ pub mod notifications; pub mod requests; pub mod progress; pub mod work_pool; +pub mod semantic_lookup; /// Persistent context shared across all requests and notifications. pub enum ActionContext { @@ -1183,10 +1184,10 @@ impl InitActionContext { response: sender, }; if self.check_wait(&wait) { - debug!("Wait {:?} was immediately completed", wait); + trace!("Wait {:?} was immediately completed", wait); Ok(self.check_wait_satisfied(&wait)) } else { - debug!("Wait {:?} needs to wait", wait); + trace!("Wait {:?} needs to wait", wait); self.add_state_wait(wait); loop { match receiver.recv() { @@ -1198,7 +1199,7 @@ impl InitActionContext { } fn check_wait(&self, wait: &AnalysisStateWaitDefinition) -> bool { - debug!("Checking wait {:?}", wait); + trace!("Checking wait {:?}", wait); let mut wait_done = true; let analysis = self.analysis.lock().unwrap(); let queue = &self.analysis_queue; @@ -1258,9 +1259,9 @@ impl InitActionContext { } if wait_done { - debug!("{:?} was done", wait); + trace!("{:?} was done", wait); } else { - debug!("{:?} not done", wait); + trace!("{:?} not done", wait); } wait_done } diff --git a/src/actions/requests.rs b/src/actions/requests.rs index 20bf9ede..b4e2f651 100644 --- a/src/actions/requests.rs +++ b/src/actions/requests.rs @@ -15,10 +15,8 @@ use crate::actions::{AnalysisProgressKind, AnalysisWaitKind, ContextDefinition, InitActionContext, rpc_error_code}; use crate::actions::notifications::ContextDefinitionKindParam; -use crate::analysis::{Named, DeclarationSpan, LocationSpan, DLSLimitation, - SymbolRef, ZeroFilePosition, ZeroSpan, - isolated_template_limitation}; -use crate::analysis::reference::ReferenceKind; +use crate::actions::semantic_lookup::{DLSLimitation, declarations_at_fp, definitions_at_fp, implementations_at_fp, references_at_fp}; +use crate::analysis::{Named, DeclarationSpan, LocationSpan}; use crate::analysis::symbols::SimpleSymbol; use crate::config::Config; @@ -127,136 +125,6 @@ where } } - -pub fn type_semantic_limitation() -> DLSLimitation { - DLSLimitation { - issue_num: 65, - description: "The DLS does not currently support semantic analysis of \ - types, including reference finding".to_string(), - } -} - -// TODO: This function is getting bloated, refactor into several smaller ones -fn fp_to_symbol_refs - (fp: &ZeroFilePosition, - ctx: &InitActionContext, - relevant_limitations: &mut HashSet) - -> Result, AnalysisLookupError> -{ - let analysis = ctx.analysis.lock().unwrap(); - // This step-by-step approach could be folded into analysis_storage, - // but I keep it as separate here so that we could, perhaps, - // returns different information for "no symbols found" and - // "no info at pos" - debug!("Looking up symbols/references at {:?}", fp); - let (context_sym, reference) = (analysis.context_symbol_at_pos(fp)?, - analysis.reference_at_pos(fp)?); - debug!("Got {:?} and {:?}", context_sym, reference); - let canon_path = CanonPath::from_path_buf(fp.path()) - .ok_or(AnalysisLookupError::NoFile)?; - // Rather than holding the lock throughout the request, clone - // the filter - let filter = Some(ctx.device_active_contexts.lock().unwrap().clone()); - let mut definitions = vec![]; - let analysises = analysis.all_device_analysises_containing_file( - &canon_path); - if analysises.is_empty() { - return Err(AnalysisLookupError::NoDeviceAnalysis); - } - match (context_sym, reference) { - (None, None) => { - debug!("No symbol or reference at point"); - return Ok(vec![]); - }, - (Some(sym), refer) => { - if refer.is_some() { - error!("Obtained both symbol and reference at {:?}\ - (reference is {:?}), defaulted to symbol", - &fp, refer); - } - for device in analysis.filtered_device_analysises_containing_file( - &canon_path, - filter.as_ref()) { - definitions.extend( - device.lookup_symbols_by_contexted_symbol( - &sym, relevant_limitations) - .into_iter()); - } - }, - (None, Some(refr)) => { - debug!("Mapping {:?} to symbols", refr.loc_span()); - // Should be guaranteed by the context reference lookup above - // (isolated analysis does exist) - if refr.reference_kind() == ReferenceKind::Type { - relevant_limitations.insert(type_semantic_limitation()); - } - - let first_context = analysis.first_context_at_pos(fp).unwrap(); - let mut any_template_used = false; - for device in analysis.filtered_device_analysises_containing_file( - &canon_path, - filter.as_ref()) { - // NOTE: This ends up being the correct place to warn users - // about references inside uninstantiated templates, - // but we have to perform some extra work to find out we are - // in that case - if let Some(ContextKey::Template(ref sym)) = first_context { - if device.templates.templates.get(sym.name_ref()) - .and_then(|t|t.location.as_ref()) - .and_then( - |loc|device.template_object_implementation_map.get(loc)) - .map_or(false, |impls|!impls.is_empty()) { - any_template_used = true; - } - } - - definitions.append( - &mut device.symbols_of_ref(*refr.loc_span())); - } - if let Some(ContextKey::Template(templ)) = first_context { - if !any_template_used { - relevant_limitations.insert( - isolated_template_limitation(templ.name.as_str()) - ); - } - } - }, - } - Ok(definitions) -} - -fn handle_default_remapping - (ctx: &InitActionContext, - symbols: Vec, - fp: &ZeroFilePosition) -> HashSet -{ - let analysis = ctx.analysis.lock().unwrap(); - let refr_opt = analysis.reference_at_pos(fp); - // NOTE: Because the call to this is preceded by a symbol lookup, - // it is probably safe to discard an error here - if let Ok(Some(refr)) = refr_opt { - if refr.to_string().as_str() == "default" { - // If we are at a defaut reference, - // remap symbol references to methods - // to the correct decl site, leave others as-is - return symbols.into_iter() - .flat_map(|d|'sym: { - let sym = d.lock().unwrap(); - if sym.kind == DMLSymbolKind::Method { - if let Some(loc) = sym.default_mappings - .get(refr.loc_span()) { - break 'sym vec![*loc]; - } - } - sym.declarations.clone() - }).collect(); - } - } - symbols.into_iter() - .flat_map(|d|d.lock().unwrap().declarations.clone()) - .collect() -} - /// The result of a deglob action for a single wildcard import. /// /// The `location` is the position of the wildcard. @@ -548,19 +416,13 @@ impl RequestAction for GotoImplementation { wait_for_device_path!(ctx, canon_path); let mut limitations = HashSet::new(); - match fp_to_symbol_refs(&fp, &ctx, &mut limitations) { - Ok(symbols) => { - let mut unique_locations: HashSet - = HashSet::default(); - for symbol in symbols { - for implementation in &symbol.lock().unwrap().implementations { - unique_locations.insert(*implementation); - } - } - let lsp_locations: Vec<_> = unique_locations.into_iter() + + match implementations_at_fp(&ctx, &fp, &mut limitations) { + Ok(locs) => { + let lsp_locations: Vec<_> = locs.into_iter() .map(|l|ls_util::dls_to_location(&l)) .collect(); - trace!("Requested implementations are {:?}", lsp_locations); + debug!("Requested implementations are {:?}", lsp_locations); Ok(response_maybe_with_limitations( // NOTE: this ends up being the client-path, which is // actually what we want @@ -571,8 +433,7 @@ impl RequestAction for GotoImplementation { }, Err(lookuperror) => { let main_file_name = fp.path(); - warn_miss_lookup(lookuperror, - main_file_name.to_str()); + warn_miss_lookup(lookuperror, main_file_name.to_str()); Self::fallback_response() }, } @@ -614,12 +475,9 @@ impl RequestAction for GotoDeclaration { wait_for_device_path!(ctx, canon_path); let mut limitations = HashSet::new(); - match fp_to_symbol_refs(&fp, &ctx, &mut limitations) { - Ok(symbols) => { - let unique_locations = handle_default_remapping(&ctx, - symbols, - &fp); - let lsp_locations = unique_locations.into_iter() + match declarations_at_fp(&ctx, &fp, &mut limitations) { + Ok(locs) => { + let lsp_locations = locs.into_iter() .map(|l|ls_util::dls_to_location(&l)) .collect(); trace!("Requested declarations are {:?}", lsp_locations); @@ -674,13 +532,9 @@ impl RequestAction for GotoDefinition { wait_for_device_path!(ctx, canon_path); let mut limitations = HashSet::new(); - match fp_to_symbol_refs(&fp, &ctx, &mut limitations) { - Ok(symbols) => { - let unique_locations: HashSet = - symbols.into_iter() - .flat_map(|d|d.lock().unwrap() - .definitions.clone()).collect(); - let lsp_locations: Vec<_> = unique_locations.into_iter() + match definitions_at_fp(&ctx, &fp, &mut limitations) { + Ok(locs) => { + let lsp_locations: Vec<_> = locs.into_iter() .map(|l|ls_util::dls_to_location(&l)) .collect(); Ok(response_maybe_with_limitations( @@ -735,13 +589,9 @@ impl RequestAction for References { wait_for_device_path!(ctx, canon_path); let mut limitations = HashSet::new(); - match fp_to_symbol_refs(&fp, &ctx, &mut limitations) { - Ok(symbols) => { - let unique_locations: HashSet = - symbols.into_iter() - .flat_map(|d|d.lock().unwrap() - .references.clone()).collect(); - let lsp_locations: Vec<_> = unique_locations.into_iter() + match references_at_fp(&ctx, &fp, &mut limitations) { + Ok(locs) => { + let lsp_locations: Vec<_> = locs.into_iter() .map(|l|ls_util::dls_to_location(&l)) .collect(); trace!("Requested references are {:?}", lsp_locations); diff --git a/src/actions/semantic_lookup.rs b/src/actions/semantic_lookup.rs new file mode 100644 index 00000000..413d7a6f --- /dev/null +++ b/src/actions/semantic_lookup.rs @@ -0,0 +1,399 @@ +// © 2024 Intel Corporation +// SPDX-License-Identifier: Apache-2.0 and MIT +//! Stores currently completed analysis. + +use log::{debug, error}; + +use std::collections::HashSet; +use std::{fmt, mem}; +use std::sync::Arc; + +use crate::actions::analysis_storage::{AnalysisLookupError, AnalysisStorage}; +use crate::actions::{ContextDefinition, InitActionContext}; +use crate::analysis::scope::{ContextedSymbol, ContextKey}; +use crate::analysis::structure::objects::MaybeAbstract; +use crate::analysis::symbols::DMLSymbolKind; +use crate::analysis::{DeviceAnalysis, IsolatedAnalysis, LocationSpan, SymbolRef}; + +use crate::analysis::parsing::tree::{ZeroSpan, ZeroFilePosition}; +use crate::analysis::reference::{Reference, ReferenceKind}; +use crate::file_management::CanonPath; +use crate::server::Output; + +// The issue number is used as the _unique identifier_ for this +// limitation, and should be the number of an open GITHUB +// issue. +// Example: +// DLSLimitation { +// issue_num: 42, +// description: "Example of a DLS limitation".to_string(), +// }; + +#[derive(Clone, Debug, Eq, PartialEq, Hash,)] +pub struct DLSLimitation { + pub issue_num: u64, + pub description: String, +} + +impl fmt::Display for DLSLimitation { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} (issue#{})", self.description, self.issue_num) + } +} + +pub fn type_semantic_limitation() -> DLSLimitation { + DLSLimitation { + issue_num: 65, + description: "The DLS does not currently support semantic analysis of \ + types, including reference finding".to_string(), + } +} + +pub fn isolated_template_limitation(template_name: &str) -> DLSLimitation { + DLSLimitation { + issue_num: 31, + description: + format!("References from, and definitions inside, a template \ + cannot be evaluated \ + without an instantiating object. Open a device file \ + that uses the template '{}' to obtain such information.", + template_name), + } +} + +// Because symbols need to be tied to their source analysis that result is +// a [(DeviceAnalysis, [SymbolRef])] list +// The reference comes from an isolated analysis, and thus is disconnected from a device +// context +type DeviceSymbols<'t> = Vec<(&'t DeviceAnalysis, Vec)>; +enum SymbolsOrReference<'t> { + Symbols(DeviceSymbols<'t>), + Reference(Reference), + Nothing, +} + +struct AnalysisInfo <'a> { + pub isolated_analysis: &'a IsolatedAnalysis, + pub device_analysises: Vec<&'a DeviceAnalysis>, +} + +#[allow(dead_code)] +struct SemanticLookup<'t> { + pub stored_symbols: DeviceSymbols<'t>, + pub found_ref: Option, + pub analysis_info: AnalysisInfo<'t>, + pub recognized_limitations: HashSet, +} + +impl <'t> SemanticLookup<'t> { + pub fn create_lookup(fp: &ZeroFilePosition, + analysis_lock: &'t AnalysisStorage, + ctx: &'t InitActionContext) + -> Result { + let active_filter = ctx.device_active_contexts.lock().unwrap().clone(); + let analysis_info = analysises_at_fp(analysis_lock, fp, Some(&active_filter))?; + let mut recognized_limitations = HashSet::new(); + + let lookup_result = get_refs_and_syms_at_fp( + fp, + &analysis_info, + &mut recognized_limitations, + )?; + + let mut stored_symbols = vec![]; + let mut found_ref = None; + match lookup_result { + SymbolsOrReference::Symbols(symbol_refs) => stored_symbols = symbol_refs, + SymbolsOrReference::Reference(reference) => { + found_ref = Some(reference.clone()); + stored_symbols = get_symbols_of_ref( + &reference, + &analysis_info, + &mut recognized_limitations) + }, + SymbolsOrReference::Nothing => (), + } + + Ok(SemanticLookup { + stored_symbols, + found_ref, + analysis_info, + recognized_limitations, + }) + } + + pub fn symbols(&'t self) -> Vec { + self.stored_symbols.iter().flat_map(|(_, syms)|syms) + .map(|s|Arc::clone(s)).collect::>() + } +} + +fn analysises_at_fp<'t>( + analysis_storage: &'t AnalysisStorage, + fp: &ZeroFilePosition, + active_filter: Option<&HashSet>) + -> Result, AnalysisLookupError> { + let canon_path = CanonPath::from_path_buf(fp.path()) + .ok_or(AnalysisLookupError::NoFile)?; + let isolated_analysis = analysis_storage.get_isolated_analysis(&canon_path)?; + let analysises = analysis_storage.filtered_device_analysises_containing_file(&canon_path, active_filter); + if analysises.is_empty() { + return Err(AnalysisLookupError::NoDeviceAnalysis); + } + Ok(AnalysisInfo { + isolated_analysis, + device_analysises: analysises, + }) +} + +fn get_refs_and_syms_at_fp<'t>( + fp: &ZeroFilePosition, + analysis_info: &AnalysisInfo<'t>, + relevant_limitations: &mut HashSet) + -> Result, AnalysisLookupError> { + debug!("Looking up references and symbols at position {:?}", fp); + let ref_at_pos = analysis_info.isolated_analysis.lookup_reference(fp); + + let context_sym_at_pos = context_symbol_at_pos(analysis_info.isolated_analysis, fp); + let symbols_at_fp = context_sym_at_pos.map(|cs| { + analysis_info.device_analysises.iter().map(|a|(*a, a.lookup_symbols(&cs, relevant_limitations))).collect::>() + }); + if let Some(syms) = symbols_at_fp { + if ref_at_pos.is_some() { + error!("Obtained both symbol and reference at {:?}\ + (reference is {:?}), defaulted to symbol", + &fp, ref_at_pos); + } + return Ok(SymbolsOrReference::Symbols(syms)); + } + if let Some(refr) = ref_at_pos { + if refr.reference_kind() == ReferenceKind::Type { + relevant_limitations.insert(type_semantic_limitation()); + } + } + if let Some(refr) = ref_at_pos { + Ok(SymbolsOrReference::Reference(refr.clone())) + } else { + debug!("No symbol or reference at point"); + Ok(SymbolsOrReference::Nothing) + } +} + +fn get_symbols_of_ref<'t>(reference: &Reference, + analysis_info: &AnalysisInfo<'t>, + relevant_limitations: &mut HashSet) + -> DeviceSymbols<'t> { + debug!("Mapping {:?} to symbols", reference.loc_span()); + // Should be guaranteed by the context reference lookup above + // (isolated analysis does exist) + let mut symbols = vec![]; + let first_context = analysis_info.isolated_analysis + .lookup_first_context(&reference.loc_span() + .start_position()); + let mut any_template_used = false; + for device in &analysis_info.device_analysises { + // NOTE: This ends up being the correct place to warn users + // about references inside uninstantiated templates, + // but we have to perform some extra work to find out we are + // in that case + if let Some(ContextKey::Template(ref sym)) = first_context { + if device.templates.templates.get(sym.name_ref()) + .and_then(|t|t.location.as_ref()) + .and_then(|loc|device.template_object_implementation_map.get(loc)) + .map_or(false, |impls|!impls.is_empty()) { + any_template_used = true; + } + } + let syms = device.symbols_of_ref(*reference.loc_span()); + debug!("Mapped {:?} to symbols {:?} in device analysis of {}", + reference.loc_span(), + syms.iter().map(|s|s.lock().unwrap().medium_debug_info()).collect::>(), + device.clientpath.display()); + symbols.push((*device, syms)); + } + if let Some(ContextKey::Template(templ)) = first_context { + if !any_template_used { + relevant_limitations.insert( + isolated_template_limitation(templ.name.as_str()) + ); + } + } + + symbols +} + +fn context_symbol_at_pos<'t>(isolated_analysis: &'t IsolatedAnalysis, pos: &ZeroFilePosition) -> + Option> { + let mut context = isolated_analysis.lookup_context_symbol(pos); + // Patch out leading 'device' context, unneeded + if let Some(ref mut ic) = context { + ic.remove_head_context(); + } + context +} + +fn symbol_implementations_of_symbol<'t>(symbol: &'t SymbolRef, + analysis: &'t DeviceAnalysis) + -> Vec { + let symbol_lock = symbol.lock().unwrap(); + + // Special case for methods, recursively follow the implementations to find + // all methods + if symbol_lock.kind == DMLSymbolKind::Method { + // Re-dropping the lock here is sensible, as we want to iterate over non-locked symbols + drop(symbol_lock); + #[allow(clippy::mutable_key_type)] + let mut syms: HashSet::<&'t SymbolRef> = HashSet::default(); + let mut next_iteration = vec![symbol]; + while let Some(next) = next_iteration.pop() { + let next_lock = next.lock().unwrap(); + if !syms.contains(next) { + let parent = if let Some(meth_source) = next_lock.source.as_method() { + if !meth_source.1.is_abstract() { + syms.insert(next); + } + meth_source.0 + } else { + internal_error!("Expected method symbol source of symbol iterated over by\ + implementations_of_symbol, symbol is {:?}", next_lock); + continue; + }; + + for impl_ref in &next_lock.implementations { + // Note: We only need to find the implementations of the variant of this method + // under parent. Other variants are handled by finding multiple symbols at + // the ref location + if let Some(impl_sym) = analysis.symbol_info.method_symbols.get(impl_ref) + .and_then(|m|m.get(parent)) { + next_iteration.push(impl_sym); + } else { + internal_error!("Expected method implementation symbol to exist in \ + analysis, symbol is {:?}, implementation ref is {:?}", + next_lock, impl_ref); + } + } + } + } + syms.into_iter().map(Arc::clone).collect() + } else { + vec![] + } +} + +// NOTE: It'd be nice for some of these methods to return hashsets of symbolrefs. +// however, SymbolRefs can in fact be modified while these requests are running, +// due to symbol->reference caching, so it would break hashset invariance +// Instead, we will rely on actions.rs to make locations unique before passing to client +fn extra_implementations_of_method_symbols<'t>(semantic_lookup: &SemanticLookup<'t>) + -> Vec { + let mut full_method_symbol_set: Vec = vec![]; + for (_device_analysis, symbols) in &semantic_lookup.stored_symbols { + for symbol in symbols { + full_method_symbol_set.extend( + symbol_implementations_of_symbol(symbol, _device_analysis).into_iter()); + } + } + full_method_symbol_set +} + +pub fn implementations_at_fp(context: &InitActionContext, + fp: &ZeroFilePosition, + relevant_limitations: &mut HashSet) + -> Result, AnalysisLookupError> { + let analysis_lock = context.analysis.lock().unwrap(); + let mut semantic_lookup = SemanticLookup::create_lookup( + fp, + &analysis_lock, + context)?; + let method_symbols = extra_implementations_of_method_symbols(&semantic_lookup); + mem::swap(relevant_limitations, &mut semantic_lookup.recognized_limitations); + Ok(semantic_lookup.symbols() + .into_iter() + .filter(|s|s.lock().unwrap().kind != DMLSymbolKind::Method) + .flat_map(|s|s.lock().unwrap().implementations.clone()) + .chain(method_symbols.into_iter() + .map(|s|s.lock().unwrap().loc)) + .collect()) +} + +fn definitions_of_symbol<'t>(symbol: &'t SymbolRef, refr: Option<&Reference>) + -> Vec { + let symbol_lock = symbol.lock().unwrap(); + if symbol_lock.kind == DMLSymbolKind::Method { + if let Some((_, methsrc)) = symbol_lock.source.as_method() { + let mut to_return = vec![]; + // If we goto-def directly on an abstract method, + // give its first-level overrides + if methsrc.is_abstract() && refr.is_none() { + to_return = symbol_lock.implementations.iter() + .cloned().collect(); + } + if to_return.is_empty() { + to_return = symbol_lock.definitions.clone(); + } + if to_return.is_empty() { + to_return = symbol_lock.declarations.clone(); + } + to_return + } else { + internal_error!("Expected method symbol source for method symbol {:?}", symbol_lock); + vec![] + } + } else { + symbol_lock.definitions.clone() + } +} + +pub fn definitions_at_fp(context: &InitActionContext, + fp: &ZeroFilePosition, + relevant_limitations: &mut HashSet) + -> Result, AnalysisLookupError> { + let analysis_lock = context.analysis.lock().unwrap(); + let mut semantic_lookup = SemanticLookup::create_lookup( + fp, + &analysis_lock, + context)?; + mem::swap(relevant_limitations, &mut semantic_lookup.recognized_limitations); + Ok(semantic_lookup.symbols() + .into_iter() + .flat_map(|s|definitions_of_symbol(&s, semantic_lookup.found_ref.as_ref())) + .collect()) +} + +pub fn declarations_at_fp(context: &InitActionContext, + fp: &ZeroFilePosition, + relevant_limitations: &mut HashSet) + -> Result, AnalysisLookupError> { + let analysis_lock = context.analysis.lock().unwrap(); + let mut semantic_lookup = SemanticLookup::create_lookup( + fp, + &analysis_lock, + context)?; + mem::swap(relevant_limitations, &mut semantic_lookup.recognized_limitations); + Ok(semantic_lookup.symbols() + .into_iter() + .flat_map(|s|{ + let symlock = s.lock().unwrap(); + if matches!(symlock.kind, DMLSymbolKind::Method | DMLSymbolKind::Parameter){ + symlock.bases.clone() + } else { + symlock.declarations.clone() + }}) + .collect()) +} + +pub fn references_at_fp(context: &InitActionContext, + fp: &ZeroFilePosition, + relevant_limitations: &mut HashSet) + -> Result, AnalysisLookupError> { + let analysis_lock = context.analysis.lock().unwrap(); + let mut semantic_lookup = SemanticLookup::create_lookup( + fp, + &analysis_lock, + context)?; + mem::swap(relevant_limitations, &mut semantic_lookup.recognized_limitations); + Ok(semantic_lookup.symbols() + .into_iter() + .flat_map(|s|s.lock().unwrap().references.clone()) + .collect()) +} \ No newline at end of file diff --git a/src/analysis/mod.rs b/src/analysis/mod.rs index b60e1231..eff48152 100644 --- a/src/analysis/mod.rs +++ b/src/analysis/mod.rs @@ -26,12 +26,10 @@ use rayon::prelude::*; use crate::actions::SourcedDMLError; use crate::actions::analysis_storage::TimestampedStorage; -use crate::analysis::symbols::{SimpleSymbol, DMLSymbolKind, SymbolMaker, - SymbolContainer, StructureSymbol, SymbolSource}; +use crate::actions::semantic_lookup::{DLSLimitation, isolated_template_limitation}; +use crate::analysis::symbols::{DMLSymbolKind, SimpleSymbol, StructureSymbol, SymbolContainer, SymbolMaker, SymbolSource}; pub use crate::analysis::symbols::SymbolRef; -use crate::analysis::reference::{Reference, - GlobalReference, VariableReference, - ReferenceKind, NodeRef}; +use crate::analysis::reference::{GlobalReference, NodeRef, Reference, ReferenceKind, ReferenceVariant, VariableReference}; use crate::analysis::scope::{Scope, SymbolContext, ContextKey, ContextedSymbol}; use crate::analysis::parsing::parser::{FileInfo, FileParser}; @@ -42,8 +40,7 @@ pub use crate::analysis::parsing::tree:: {ZeroRange, ZeroSpan, ZeroPosition, ZeroFilePosition}; use crate::analysis::parsing::tree::{MissingToken, MissingContent, TreeElement}; -use crate::analysis::structure::objects::{Template, Import, CompObjectKind, - ParamValue}; +use crate::analysis::structure::objects::{CompObjectKind, Import, MaybeAbstract, ParamValue, Template}; use crate::analysis::structure::statements::{ForPre, Statement, StatementKind}; use crate::analysis::structure::toplevel::{ObjectDecl, TopLevel}; use crate::analysis::structure::types::DMLType; @@ -60,8 +57,7 @@ use crate::analysis::templating::objects::{make_device, DMLObject, use crate::analysis::templating::topology::{RankMaker, rank_templates, create_templates_traits}; -use crate::analysis::templating::methods::{MethodDeclaration, DMLMethodRef, - DMLMethodArg}; +use crate::analysis::templating::methods::{DMLMethodArg, DMLMethodRef, DefaultCallReference, MethodDeclaration}; use crate::analysis::templating::traits::{DMLTemplate, TemplateTraitInfo}; use crate::analysis::templating::types::DMLResolvedType; @@ -79,26 +75,6 @@ pub struct FileSpec<'a> { pub const IMPLICIT_IMPORTS: [&str; 2] = ["dml-builtins.dml", "simics/device-api.dml"]; -// The issue number is used as the _unique identifier_ for this -// limitation, and should be the number of an open GITHUB -// issue. -// Example: -// DLSLimitation { -// issue_num: 42, -// description: "Example of a DLS limitation".to_string(), -// }; - -#[derive(Clone, Debug, Eq, PartialEq, Hash,)] -pub struct DLSLimitation { - pub issue_num: u64, - pub description: String, -} - -impl fmt::Display for DLSLimitation { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{} (issue#{})", self.description, self.issue_num) - } -} // For things whose names are in one spot as a dmlstring pub trait DMLNamed { @@ -358,7 +334,9 @@ pub struct SymbolStorage { pub param_symbols: HashMap<(ZeroSpan, String), HashMap>, pub object_symbols: HashMap, - pub method_symbols: HashMap, + // This is doubly-indexed, by decl location and then + // by parent object key + pub method_symbols: HashMap>, // constants, sessions, saveds, hooks, method args pub variable_symbols: HashMap, } @@ -368,7 +346,7 @@ impl SymbolStorage { self.template_symbols.values() .chain(self.param_symbols.values().flat_map(|h|h.values())) .chain(self.object_symbols.values()) - .chain(self.method_symbols.values()) + .chain(self.method_symbols.values().flat_map(|h|h.values())) .chain(self.variable_symbols.values()) } } @@ -395,58 +373,102 @@ pub struct DeviceAnalysis { pub clientpath: PathBuf, } +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum ReferenceMatchKind { + Found, + MismatchedFind, + NotFound, +} + #[derive(Debug, Clone)] -pub enum ReferenceMatches { - Found(HashSet), - WrongType(SymbolRef), - NotFound(HashSet), +pub struct ReferenceMatches { + pub kind: ReferenceMatchKind, + // How these references are interpreted depends on 'kind', + // for NotFound and MismatchedFind they are generally suggestions + // whereas for Found they are actual matches + pub references: HashSet, + // These will be reported as-is, regardless of kind + pub messages: Vec, } impl Default for ReferenceMatches { fn default() -> Self { - Self::NotFound(HashSet::default()) + ReferenceMatches { + kind: ReferenceMatchKind::NotFound, + references: HashSet::default(), + messages: vec![], + } } } impl ReferenceMatches { pub fn add_match(&mut self, reference: SymbolRef) { - if !matches!(self, Self::WrongType(_)) { - if let Self::Found(ref mut references) = self { - references.insert(reference); - } else { - *self = Self::Found(HashSet::from([reference])); - } + if self.kind != ReferenceMatchKind::MismatchedFind { + self.kind = ReferenceMatchKind::Found; + self.references.insert(reference); } } pub fn add_suggestion(&mut self, reference: SymbolRef) { - if let Self::NotFound(ref mut references) = self { - references.insert(reference); + if !matches!(self.kind, ReferenceMatchKind::Found + | ReferenceMatchKind::MismatchedFind) { + self.references.insert(reference); } } - pub fn set_wrongtype(&mut self, reference: SymbolRef) { - *self = Self::WrongType(reference); + pub fn set_mismatched(&mut self, reference: SymbolRef) { + self.kind = ReferenceMatchKind::MismatchedFind; + self.references.insert(reference); } pub fn new() -> Self { Self::default() } - pub fn has_matches(&self) -> bool { - matches!(self, Self::Found(_)) + pub fn as_matches(&self) -> Option> { + if self.kind == ReferenceMatchKind::Found { + Some(self.references.clone()) + } else { + None + } + } + + pub fn as_suggestions(&self) -> Option> { + if self.kind == ReferenceMatchKind::NotFound { + Some(self.references.clone()) + } else { + None + } + } + + pub fn as_mismatched(&self) -> Option> { + if self.kind == ReferenceMatchKind::MismatchedFind { + Some(self.references.clone()) + } else { + None + } + } + + pub fn add_message(&mut self, message: DMLError) { + self.messages.push(message); } pub fn merge_with(&mut self, other: Self) { - match other { - Self::Found(syms) => for sym in syms { - self.add_match(sym); - }, - Self::NotFound(syms) => for sym in syms { - self.add_suggestion(sym); - }, - Self::WrongType(sym) => self.set_wrongtype(sym), + match other.kind { + ReferenceMatchKind::MismatchedFind => + for sym in other.references { + self.set_mismatched(sym); + }, + ReferenceMatchKind::Found => + for sym in other.references { + self.add_match(sym); + }, + ReferenceMatchKind::NotFound => + for sym in other.references { + self.add_suggestion(sym); + }, } + self.messages.extend(other.messages); } } @@ -554,19 +576,22 @@ fn gather_scopes<'c>(next_scopes: Vec<&'c dyn Scope>, } } -pub fn isolated_template_limitation(template_name: &str) -> DLSLimitation { - DLSLimitation { - issue_num: 31, - description: - format!("References from, and definitions inside, a template \ - cannot be evaluated \ - without an instantiating object. Open a device file \ - that uses the template '{}' to obtain such information.", - template_name), +impl DeviceAnalysis { + pub fn lookup_symbols<'t>(&self, context_sym: &ContextedSymbol<'t>, limitations: &mut HashSet) + -> Vec { + trace!("Looking up symbols at {:?}", context_sym); + + // Special handling for methods, since each method decl has its + // own symbol + if context_sym.kind() == DMLSymbolKind::Method { + return self.symbol_info.method_symbols + .get(context_sym.loc_span()) + .map(|m|m.values().cloned()) + .into_iter().flatten().collect(); + } + self.lookup_symbols_by_contexted_symbol(context_sym, limitations) } -} -impl DeviceAnalysis { pub fn get_device_obj(&self) -> &DMLObject { &self.device_obj } @@ -802,7 +827,11 @@ impl DeviceAnalysis { DMLResolvedObject::ShallowObject(shallow) => match &shallow.variant { DMLShallowObjectVariant::Method(m) => - self.symbol_info.method_symbols.get(m.location()), + // For resolutions, a methods symbols is just the symbol + // of the method we resolved to + self.symbol_info.method_symbols + .get(m.location()) + .and_then(|m|m.get(&parent.key)), DMLShallowObjectVariant::Session(s) | DMLShallowObjectVariant::Saved(s) => self.symbol_info.variable_symbols.get(s.loc_span()), @@ -1018,10 +1047,11 @@ impl DeviceAnalysis { } } - pub fn lookup_symbols_by_contexted_symbol<'t>( - &self, - sym: &ContextedSymbol<'t>, - limitations: &mut HashSet) -> Vec { + // TODO/NOTE: This method seems slightly misplaced, consider moving to semantic_lookup + pub fn lookup_symbols_by_contexted_symbol<'t>(&self, + sym: &ContextedSymbol<'t>, + limitations: &mut HashSet) + -> Vec { if matches!(sym.symbol.kind, DMLSymbolKind::Template | DMLSymbolKind::Typedef | DMLSymbolKind::Extern) { @@ -1042,13 +1072,15 @@ impl DeviceAnalysis { }; if let Some(objs) = mb_objs { - debug!("Found {:?}", objs); + trace!("Found {:?}", objs); let mut refs = ReferenceMatches::new(); let _: Vec<_> = objs.into_iter() .map(|o|self.lookup_def_in_obj(&o, sym.symbol, &mut refs)) .collect(); - if let ReferenceMatches::Found(syms) = refs { - syms.into_iter().collect() + // We can ignore messages from lookup defs here, as this lookup is + // live and reporting additional things from here makes no sense + if let Some(matches) = refs.as_matches() { + matches.into_iter().collect() } else { vec![] } @@ -1077,7 +1109,10 @@ impl DeviceAnalysis { self.resolve_noderef_in_obj(&obj_copy, node, method_structure, - ref_matches) + ref_matches); + }, + SymbolSource::Method(key, method) => { + self.resolve_noderef_in_method(key, method, node, method_structure, ref_matches); }, // TODO: Cannot be resolved without constant folding SymbolSource::MethodArg(_method, _name) => (), @@ -1089,37 +1124,106 @@ impl DeviceAnalysis { } } + fn get_method_symbol(&self, + method: &Arc, + parent_obj_key: &StructureKey) + -> Option<&SymbolRef> { + let to_ret = self.symbol_info.method_symbols.get(method.location()) + .and_then(|m|m.get(parent_obj_key)); + if to_ret.is_none() { + internal_error!("Missing method symbol for method ref {:?}", method); + } + to_ret + } + fn resolve_simple_noderef_in_method<'c>(&'c self, - obj: &DMLShallowObject, + parent_key: &StructureKey, meth: &Arc, node: &DMLString, _type_hint: Option<()>, method_structure: &HashMap , ref_matches: &mut ReferenceMatches) { - // TODO: is this 100% true? - // We cannot lookup things from within a method without - // the reference being contained within the span of the method + // When resolving a noderef from an overridden method, meth here will be + // a bottom-most overriding method. So instead find the correct method + // by recursing to parent if !meth.span().contains_pos(&node.span.start_position()) { + match meth.get_default() { + Some(DefaultCallReference::Valid(defmeth)) => { + if let Some(defmeth_sym) = self.get_method_symbol(defmeth, + parent_key) { + self.resolve_noderef_in_symbol( + defmeth_sym, + &NodeRef::Simple(node.clone()), + method_structure, + ref_matches); + } + }, + Some(DefaultCallReference::Ambiguous(defmeths)) => { + for defmeth in defmeths { + if let Some(defmeth_sym) = self.get_method_symbol(defmeth, + parent_key) { + self.resolve_noderef_in_symbol( + defmeth_sym, + &NodeRef::Simple(node.clone()), + method_structure, + ref_matches); + } + } + }, + _ => { + trace!("Fell through recursive method noderef resolution for {:?} in method {:?}", node, meth); + } + } return; } - + trace!("Resolving simple noderef {:?} in method {:?}", node, meth); match node.val.as_str() { "this" => ref_matches.add_match(Arc::clone( self.symbol_info.object_symbols.get( - &obj.parent).unwrap())), + parent_key).unwrap())), "default" => - // NOTE: This is part of the hack that maps default - // references in methods to the corret method decl. - // Here, we simply check if the method has any - // default call, and if so map the reference to the - // method symbol - if meth.get_default().is_some() { - if let Some(s) = self.symbol_info.method_symbols - .get(obj.location()) { - ref_matches.add_match(Arc::clone(s)) + // NOTE: Here we match a default ref to the symbol of the method decl + // that we directly overrode. Meaning that further lookups on that symbol + // will be specialized for that overriding chain + if let Some(defref) = meth.get_default() { + match defref { + DefaultCallReference::Valid(refr) => { + if let Some(s) = self.get_method_symbol(refr, parent_key) { + ref_matches.add_match(Arc::clone(s)); + } + }, + DefaultCallReference::Ambiguous(refs) => { + // TODO/NOTE: These are currently unused, but since we want to mark a mismatch anyway may as well + // pass all these references in + for reference in refs { + if let Some(s) = self.get_method_symbol(reference, parent_key) { + ref_matches.set_mismatched(Arc::clone(s)); + } + } + let ambiguous_desc: &'static str + = "Ambiguous default call, you may need to clarify the template ordering or use a template-qualified-method-implementation-call"; + // TODO: curren + ref_matches.add_message(DMLError { + span: *node.span(), + description: ambiguous_desc.to_string(), + severity: Some(DiagnosticSeverity::ERROR), + related: refs + .iter() + .map(|d|(*d.location(), + format!("Possible candidate for default call{}", + d.get_disambiguation_name() + .map_or_else(||"".to_string(), + |n|format!(", can be explicitly called as 'this.{}'", n))) + )) + .collect(), + }); + } + _ => { + // 'default' when overriding nothing is an error, similar to the else-case below } + } } else { // fairly sure 'default' cannot be a // reference otherwise @@ -1144,12 +1248,50 @@ impl DeviceAnalysis { } } + fn resolve_noderef_in_method<'c>(&'c self, + parent_key: &StructureKey, + method: &Arc, + node: &NodeRef, + method_structure: &HashMap, + ref_matches: &mut ReferenceMatches) { + match node { + NodeRef::Simple(simple) => { + self.resolve_simple_noderef_in_method(parent_key, method, simple, + None, + method_structure, + ref_matches); + }, + NodeRef::Sub(subnode, simple, _) => { + let mut intermediate_matches = ReferenceMatches::new(); + self.resolve_noderef_in_method(parent_key, + method, + subnode, + method_structure, + &mut intermediate_matches); + if let Some(syms) = intermediate_matches.as_matches() { + let wrapped_simple = NodeRef::Simple(simple.clone()); + for sym in syms { + self.resolve_noderef_in_symbol( + &sym, + &wrapped_simple, + method_structure, + ref_matches); + } + } else { + ref_matches.merge_with(intermediate_matches); + } + } + } + } + fn resolve_noderef_in_obj<'c>(&'c self, obj: &DMLObject, node: &NodeRef, method_structure: &HashMap, ref_matches: &mut ReferenceMatches) { + trace!("Resolving noderef {:?} in obj {:?}", node, obj); match node { NodeRef::Simple(simple) => { let resolvedobj = obj.resolve(&self.objects); @@ -1159,7 +1301,9 @@ impl DeviceAnalysis { .. }) = resolvedobj { - self.resolve_simple_noderef_in_method(shallow, m, simple, + self.resolve_simple_noderef_in_method(&shallow.parent, + m, + simple, None, method_structure, ref_matches); @@ -1176,18 +1320,18 @@ impl DeviceAnalysis { subnode, method_structure, &mut intermediate_matches); - match intermediate_matches { - ReferenceMatches::Found(syms) => { - let wrapped_simple = NodeRef::Simple(simple.clone()); - for sym in syms { - self.resolve_noderef_in_symbol( - &sym, - &wrapped_simple, - method_structure, - ref_matches); - } - }, - other => ref_matches.merge_with(other), + + if let Some(syms) = intermediate_matches.as_matches() { + let wrapped_simple = NodeRef::Simple(simple.clone()); + for sym in syms { + self.resolve_noderef_in_symbol( + &sym, + &wrapped_simple, + method_structure, + ref_matches); + } + } else { + ref_matches.merge_with(intermediate_matches); } } } @@ -1234,6 +1378,8 @@ impl DeviceAnalysis { method_structure, ref_matches); } + } else { + debug!("Failed to obtain obj from context {:?}", context_chain); } } @@ -1310,7 +1456,7 @@ impl DeviceAnalysis { reference, method_structure, reference_matches); - if !reference_matches.has_matches() { + if reference_matches.as_matches().is_none() { let (_, new_chain) = context_chain.split_last().unwrap(); let sub_matches = self.find_target_for_reference(new_chain, reference, @@ -1320,11 +1466,20 @@ impl DeviceAnalysis { } } + fn handle_symbol_ref(symbol: &SymbolRef, + reference: &Reference) { + let mut sym = symbol.lock().unwrap(); + sym.references.insert(*reference.loc_span()); + if sym.kind == DMLSymbolKind::Template && reference.extra_info.was_instantiation { + sym.implementations.insert(*reference.loc_span()); + } + } + #[allow(clippy::ptr_arg)] fn match_references_in_scope<'c>( &'c self, scope_chain: Vec<&'c dyn Scope>, - _report: &mut Vec, + report: &mut Vec, method_structure: &HashMap, reference_cache: &Mutex, status: &AliveStatus) { @@ -1332,22 +1487,23 @@ impl DeviceAnalysis { let context_chain: Vec = scope_chain .iter().map(|s|s.create_context()).collect(); // NOTE: chunk number is arbitrarily picked that benches well - current_scope.defined_references().par_chunks(25).for_each(|references|{ + report.extend(current_scope.defined_references().par_chunks(25).flat_map(|references|{ status.assert_alive(); + let mut local_reports = vec![]; for reference in references { - debug!("In {:?}, Matching {:?}", context_chain, reference); - let symbol_lookup = match &reference { - Reference::Variable(var) => self.find_target_for_reference( + trace!("In {:?}, Matching {:?}", context_chain, reference); + let symbol_lookup = match &reference.variant { + ReferenceVariant::Variable(var) => self.find_target_for_reference( context_chain.as_slice(), var, method_structure, reference_cache), - Reference::Global(glob) => + ReferenceVariant::Global(glob) => self.lookup_global_from_ref(glob), }; - match symbol_lookup { - ReferenceMatches::NotFound(_suggestions) => + match symbol_lookup.kind { + ReferenceMatchKind::NotFound => // TODO: report suggestions? // TODO: Uncomment reporting of errors here when // semantics are strong enough that they are rare @@ -1362,32 +1518,18 @@ impl DeviceAnalysis { // This maps symbols->references, this is later // used to create the inverse map // (not done here because of ownership issues) - ReferenceMatches::Found(symbols) => - for symbol in &symbols { - let mut sym = symbol.lock().unwrap(); - sym.references.insert(*reference.loc_span()); - if let Some(meth) = sym.source - .as_object() - .and_then(DMLObject::as_shallow) - .and_then(|s|s.variant.as_method()) { - if let Some(default_decl) = meth.get_default() { - if let Some(var) = reference.as_variable_ref() { - if var.reference.to_string().as_str() - == "default" { - sym.default_mappings.insert( - *var.loc_span(), - *default_decl.location()); - } - } - } - } + ReferenceMatchKind::Found => + for symbol in &symbol_lookup.references { + Self::handle_symbol_ref(symbol, reference); }, - ReferenceMatches::WrongType(_) => + ReferenceMatchKind::MismatchedFind => //TODO: report mismatch, (), } + local_reports.extend(symbol_lookup.messages); } - }) + local_reports.into_par_iter() + }).collect::>()); } } @@ -1439,7 +1581,7 @@ impl fmt::Display for IsolatedAnalysis { |mut s, path|{ write!(s, "\t\t{:?}\n, ", path) .expect("write string error"); - s + s }))?; writeln!(f, "\ttoplevel: {}\n}}", self.toplevel)?; Ok(()) @@ -1617,7 +1759,7 @@ fn objects_to_symbols(maker: &SymbolMaker, for obj in objects.values() { let new_symbol: SymbolRef = new_symbol_from_object(maker, obj); - debug!("Comp obj symbol is: {:?}", new_symbol); + debug!("Created comp obj symbol: {:?}", new_symbol); storage.object_symbols.insert(obj.key, new_symbol); for subobj in obj.components.values() { // Non-shallow objects will be handled by the iteration @@ -1676,6 +1818,8 @@ fn new_symbol_from_object(maker: &SymbolMaker, SymbolSource::DMLObject(DMLObject::CompObject(object.key)), definitions = all_decl_defs.clone(), declarations = all_decl_defs.clone(), + implementations = object.used_ineach_locs.clone().into_iter().collect(), + // TODO: this does not follow from the new definition of bases bases = all_decl_defs) } @@ -1693,14 +1837,13 @@ fn new_symbol_from_arg(maker: &SymbolMaker, bases = bases, definitions = definitions, declarations = declarations - ) } fn log_non_same_insert(map: &mut HashMap, key: K, val: SymbolRef) -> bool -where K: std::hash::Hash + Eq + Clone, +where K: std::hash::Hash + Eq + Clone + std::fmt::Debug, { // NOTE: We should not need to do these comparisons, when // object symbol creation is properly guided by structural AST @@ -1714,14 +1857,49 @@ where K: std::hash::Hash + Eq + Clone, if !old.lock().unwrap().equivalent( &map.get(&key).unwrap().lock().unwrap()) { internal_error!( - "Overwrote previous symbol {:?} with non-similar symbol {:?}", - old, map.get(&key)); + "Overwrote previous symbol {:?} at {:?} with non-similar symbol {:?}", + old, key, map.get(&key)); return true; } } false } +// The current strategy for method symbols is: +// Create a symbol for each level of overriding for each object where the method +// is actualized. We then end up with many symbols for the same decl location, +// and leave it to requests to collect the aggregate information at the point +fn add_new_symbol_from_method(maker: &SymbolMaker, parent_obj_key: &StructureKey, method_ref: &Arc, errors: &mut Vec, storage: &mut SymbolStorage, method_structure: &mut HashMap) { + let (bases, definitions, declarations) = ( + method_ref.get_bases().iter().map(|b|*b.location()).collect(), + vec![*method_ref.get_decl().location()], + vec![*method_ref.get_decl().location()], + ); + debug!("Made symbol for method {:?}", method_ref); + let new_sym = symbol_ref!( + maker, + *method_ref.location(), + DMLSymbolKind::Method, + SymbolSource::Method(*parent_obj_key, Arc::clone(method_ref)), + bases = bases, + definitions = definitions, + declarations = declarations); + let insert_at = storage.method_symbols.entry(*method_ref.location()) + .or_default(); + if !log_non_same_insert(insert_at, *parent_obj_key, new_sym) { + for arg in method_ref.args() { + let new_argsymbol = new_symbol_from_arg(maker, method_ref, arg); + log_non_same_insert(&mut storage.variable_symbols, *arg.loc_span(), new_argsymbol); + } + add_method_scope_symbols(maker, method_ref, method_structure, storage, errors); + if let Some(defaults) = method_ref.get_default() { + for default in defaults.flat_refs() { + add_new_symbol_from_method(maker, parent_obj_key, default, errors, storage, method_structure); + } + } + } +} + #[allow(clippy::ptr_arg)] fn add_new_symbol_from_shallow(maker: &SymbolMaker, shallow: &DMLShallowObject, @@ -1731,15 +1909,13 @@ fn add_new_symbol_from_shallow(maker: &SymbolMaker, ) { let (bases, definitions, declarations) = match &shallow.variant { DMLShallowObjectVariant::Parameter(param) => - (vec![*param.get_likely_declaration().loc_span()], + (vec![*param.get_last_declaration().loc_span()], param.used_definitions.iter() - .map(|(_, def)|*def.loc_span()).collect(), + .map(|(_, def)|*def.loc_span()).collect(), param.declarations.iter() - .map(|(_, def)|*def.loc_span()).collect()), + .map(|(_, def)|*def.loc_span()).collect()), DMLShallowObjectVariant::Method(method_ref) => - (vec![*method_ref.get_base().location()], - method_ref.get_all_defs(), - method_ref.get_all_decls()), + return add_new_symbol_from_method(maker, &shallow.parent, method_ref, errors, storage, method_structure), DMLShallowObjectVariant::Constant(constant) => (vec![*constant.loc_span()], vec![*constant.loc_span()], @@ -1754,7 +1930,7 @@ fn add_new_symbol_from_shallow(maker: &SymbolMaker, vec![*hook.loc_span()], vec![*hook.loc_span()]), }; - debug!("Made symbol for {:?}", shallow); + let new_sym = symbol_ref!( maker, *shallow.location(), @@ -1766,7 +1942,7 @@ fn add_new_symbol_from_shallow(maker: &SymbolMaker, bases = bases, definitions = definitions, declarations = declarations); - + debug!("Made shallow symbol {:?}", new_sym); match &shallow.variant { DMLShallowObjectVariant::Parameter(_) => { log_non_same_insert(storage.param_symbols.entry( @@ -1776,21 +1952,9 @@ fn add_new_symbol_from_shallow(maker: &SymbolMaker, shallow.parent, new_sym); }, - DMLShallowObjectVariant::Method(method_ref) => - if !log_non_same_insert(&mut storage.method_symbols, - *shallow.location(), - new_sym) { - for arg in method_ref.args() { - let new_argsymbol = new_symbol_from_arg(maker, - method_ref, - arg); - log_non_same_insert(&mut storage.variable_symbols, - *arg.loc_span(), - new_argsymbol); - } - add_method_scope_symbols(maker, method_ref, method_structure, - storage, errors); - }, + DMLShallowObjectVariant::Method(method_ref) => { + internal_error!("Unreachable method_ref case reached, ignored. ({:?})", method_ref); + }, DMLShallowObjectVariant::Constant(_) | DMLShallowObjectVariant::Session(_) | DMLShallowObjectVariant::Saved(_) | @@ -1813,15 +1977,14 @@ fn add_method_scope_symbols(maker: &SymbolMaker, symbols: HashMap::default(), sub_ranges: vec![], }; - if !matches!(&*method.get_decl().body, StatementKind::Compound(_)) { - error!("Internal Error: Method body was not a compound statement"); + if matches!(&*method.get_decl().body, StatementKind::Compound(_)) { + add_new_method_scope_symbols(maker, + method, + &method.get_decl().body, + errors, + storage, + &mut entry); } - add_new_method_scope_symbols(maker, - method, - &method.get_decl().body, - errors, - storage, - &mut entry); if !entry.is_empty() { method_structure.insert( *method.get_decl().location(), @@ -2082,6 +2245,8 @@ impl DeviceAnalysis { info!("Match references"); let reference_cache: Mutex = Mutex::default(); for scope_chain in all_scopes(bases) { + debug!("Got scope at {:?}", scope_chain.last() + .map(|s|s.span().start_position())); self.match_references_in_scope(scope_chain, errors, method_structure, @@ -2231,6 +2396,10 @@ impl DeviceAnalysis { &container, &mut errors, &mut method_structure); + // This needs to be done after all symbols are created, because method + // symbol order is not correlated to the object iteration order + bind_method_implementations(&mut symbol_info.method_symbols); + status.assert_alive(); // TODO: how do we store type info? extend_with_templates(&maker, &mut symbol_info, &tt_info); @@ -2335,3 +2504,41 @@ pub fn from_device_and_bases<'a>(_device: &'a IsolatedAnalysis, toplevels.iter().flat_map( |tl|&tl.templates).collect() } + +fn bind_method_implementations(method_symbols: &mut HashMap>) { + debug!("Bind method implementations"); + for (parent, method_symbol) in method_symbols.values().flat_map(|m|m.iter()) { + debug!("Binding for method {} under {:?}", method_symbol.lock().unwrap().medium_debug_info(), parent); + + // Cloning the arc is not strictly necessary, but avoiding holding the lock is good practice + let method = match &method_symbol.lock().unwrap().source { + SymbolSource::Method(_, methref) => Arc::clone(methref), + _ => { + internal_error!("Method symbol {:?} did not have method symbol source", method_symbol); + continue; + } + }; + let default_decls = method.get_default().into_iter() + .flat_map(|d|d.flat_refs().into_iter().cloned().collect::>()) + .collect::>(); + debug!("Default decls are {:?}", default_decls); + let overridden_methods = default_decls.into_iter() + .filter_map(|d| + if let Some(parent_syms) = method_symbols.get(d.location()) { + Some(parent_syms) + } else { + internal_error!("Method symbol {:?} did not have method symbol source", d); + None + }) + .filter_map(|ps|ps.get(parent)); + for overridden_method in overridden_methods { + debug!("Inserted into parent {}", overridden_method.lock().unwrap().medium_debug_info()); + if method.is_abstract() { + internal_error!("Unexpectedly bound abstract method {:?} as implementation of {:?}", + method, overridden_method); + } else { + overridden_method.lock().unwrap().implementations.insert(*method.location()); + } + } + } +} diff --git a/src/analysis/parsing/parser.rs b/src/analysis/parsing/parser.rs index db64bfbd..466bb756 100644 --- a/src/analysis/parsing/parser.rs +++ b/src/analysis/parsing/parser.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 and MIT use logos::Lexer; use regex::Regex; -use log::debug; +use log::trace; use lazy_static::lazy_static; use crate::span::{Range, ZeroIndexed, Position}; @@ -129,13 +129,13 @@ impl ParseContext { let peek = fileparser.peek(); let (endpos, endtok) = match &peek { Some(tok) => { - debug!("Ended context at {:?}: got {}, expected {}", + trace!("Ended context at {:?}: got {}, expected {}", tok.range.end(), tok.kind.description(), description); (Some(tok.range.start()), Some(*tok)) }, None => { - debug!("Ended context at {:?}: got EOF, expected {}", + trace!("Ended context at {:?}: got EOF, expected {}", fileparser.get_position(), description); (Some(fileparser.get_position()), None) }, @@ -193,7 +193,7 @@ impl ParseContext { } else { // If nobody could've handled this token, // skip it and retry - debug!("Skipped token {} at {:?}, expected {}", + trace!("Skipped token {} at {:?}, expected {}", token.kind.description(), token.range, description); fileparser.skip(description); @@ -241,7 +241,7 @@ impl ParseContext { } else { // If nobody could've handled this token, // skip it and retry - debug!("Skipped token {} at {:?}, expected {}", + trace!("Skipped token {} at {:?}, expected {}", token.kind.description(), token.range, description); fileparser.skip(description); @@ -278,7 +278,7 @@ impl ParseContext { } else { // If nobody could've handled this token, // skip it and retry - debug!("Skipped token {} at {:?}, expected {}", + trace!("Skipped token {} at {:?}, expected {}", token.kind.description(), token.range, description); fileparser.skip(description); diff --git a/src/analysis/parsing/structure.rs b/src/analysis/parsing/structure.rs index ee9e647b..c0b9104b 100644 --- a/src/analysis/parsing/structure.rs +++ b/src/analysis/parsing/structure.rs @@ -97,6 +97,7 @@ pub struct MethodContent { pub returns: Option, pub throws: Option, pub default: Option, + pub colon: Option, pub statements: Statement, } @@ -327,12 +328,14 @@ fn parse_method(modifier: Option, }; let throws = pre_statements_context.next_if_kind( stream, TokenKind::Throws); + let colon = pre_statements_context.next_if_kind( + stream, TokenKind::Colon); let default = pre_statements_context.next_if_kind( stream, TokenKind::Default); let statements = Statement::parse(&outer, stream, file_info); DMLObjectContent::Method(MethodContent { modifier, independent, startup, memoized, method, name, lparen, - arguments, rparen, returns, throws, default, statements + arguments, rparen, returns, throws, colon, default, statements }).into() } @@ -628,9 +631,10 @@ impl TreeElement for Instantiation { |(tok,_)|tok).collect(), }; - let refs = toks.into_iter().filter_map( - |tok|Reference::global_from_token( - tok, file, ReferenceKind::Template)); + let refs = toks.into_iter() + .filter_map(|tok|Reference::global_from_token( + tok, file, ReferenceKind::Template)) + .map(|mut r|{r.extra_info.was_instantiation = true; r}); accumulator.extend(refs); } } diff --git a/src/analysis/provisionals.rs b/src/analysis/provisionals.rs index 14d46ea1..a921ac7c 100644 --- a/src/analysis/provisionals.rs +++ b/src/analysis/provisionals.rs @@ -14,6 +14,7 @@ use crate::analysis::FileSpec; #[strum(serialize_all = "snake_case")] pub enum Provisional { // TODO: implement the neccessary checks for explicit params + ExplicitMethodDecls, ExplicitParamDecls, SimicsUtilVect, } @@ -70,6 +71,7 @@ mod test { fn test_provisionals_parsing() { for (s, p) in [ ("explicit_param_decls", Provisional::ExplicitParamDecls), + ("explicit_method_decls", Provisional::ExplicitMethodDecls), ("simics_util_vect", Provisional::SimicsUtilVect), ] { assert_eq!(Provisional::from_str(s), Ok(p)); diff --git a/src/analysis/reference.rs b/src/analysis/reference.rs index a7c0f470..6321e0d8 100644 --- a/src/analysis/reference.rs +++ b/src/analysis/reference.rs @@ -102,11 +102,25 @@ pub enum ReferenceKind { } #[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] -pub enum Reference { +pub enum ReferenceVariant { Variable(VariableReference), Global(GlobalReference), } +// NOTE: Since this information is likely to be sparse, use +// boxes for any non-trivially small fields. If fields get to be more +// than a few, the entire info should probably be boxed instead +#[derive(Debug, Default, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct ReferenceInfo { + pub was_instantiation: bool, +} + +#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] +pub struct Reference { + pub variant: ReferenceVariant, + pub extra_info: ReferenceInfo, +} + // NOTE: The locationspan of a reference is the actual source range its // considered to be selectable at // For example: @@ -116,17 +130,17 @@ pub enum Reference { // and the declarationspan covering the full reference impl LocationSpan for Reference { fn loc_span(&self) -> &ZeroSpan { - match self { - Reference::Variable(var) => var.loc_span(), - Reference::Global(glob) => glob.loc_span(), + match &self.variant { + ReferenceVariant::Variable(var) => var.loc_span(), + ReferenceVariant::Global(glob) => glob.loc_span(), } } } impl DeclarationSpan for Reference { fn span(&self) -> &ZeroSpan { - match self { - Reference::Variable(var) => var.span(), - Reference::Global(glob) => glob.span(), + match &self.variant { + ReferenceVariant::Variable(var) => var.span(), + ReferenceVariant::Global(glob) => glob.span(), } } } @@ -134,35 +148,41 @@ impl DeclarationSpan for Reference { impl std::fmt::Display for Reference { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { - match self { - Reference::Variable(var) => var.fmt(f), - Reference::Global(glob) => glob.fmt(f), + match &self.variant { + ReferenceVariant::Variable(var) => var.fmt(f), + ReferenceVariant::Global(glob) => glob.fmt(f), } } } impl Reference { pub fn as_variable_ref(&self) -> Option<&VariableReference> { - match self { - Reference::Variable(var) => Some(var), + match &self.variant { + ReferenceVariant::Variable(var) => Some(var), _ => None, } } pub fn from_noderef(node: NodeRef, kind: ReferenceKind) -> Reference { - Reference::Variable(VariableReference { - reference: node, - kind, - }) + Reference { + variant: ReferenceVariant::Variable(VariableReference { + reference: node, + kind, + }), + extra_info: ReferenceInfo::default(), + } } pub fn global_from_string(name: String, loc: ZeroSpan, kind: ReferenceKind) -> Reference { - Reference::Global(GlobalReference { - name, - loc, - kind, - }) + Reference { + variant: ReferenceVariant::Global(GlobalReference { + name, + loc, + kind, + }), + extra_info: ReferenceInfo::default(), + } } pub fn global_from_token<'a>(token: &LeafToken, file: FileSpec<'a>, @@ -175,9 +195,9 @@ impl Reference { kind)) } pub fn reference_kind(&self) -> ReferenceKind { - match self { - Reference::Variable(r) => r.kind, - Reference::Global(r) => r.kind, + match &self.variant { + ReferenceVariant::Variable(r) => r.kind, + ReferenceVariant::Global(r) => r.kind, } } } diff --git a/src/analysis/scope.rs b/src/analysis/scope.rs index 63fdd164..e82e4eb4 100644 --- a/src/analysis/scope.rs +++ b/src/analysis/scope.rs @@ -8,7 +8,7 @@ use crate::analysis::reference::{Reference}; use crate::analysis::structure::objects::Method; -use log::{debug}; +use log::{debug, trace}; pub trait Scope : DeclarationSpan + Named { // Need this to create default mappings @@ -24,17 +24,17 @@ pub trait Scope : DeclarationSpan + Named { fn reference_at_pos_inside(&self, pos: ZeroPosition) -> Option<&Reference> { self.defined_references().iter() - .find(|r|{debug!("Considering {:?}", r); + .find(|r|{trace!("Considering {:?}", r); r.loc_span().range.contains_pos(pos)}) } fn reference_at_pos(&self, pos: &ZeroFilePosition) -> Option<&Reference> { - debug!("Searching for references inside {:?}", self.create_context()); + trace!("Searching for references inside {:?}", self.create_context()); if let Some(our_ref) = self.reference_at_pos_inside(pos.position) { - debug!("Got {:?}", our_ref); + trace!("Got {:?}", our_ref); Some(our_ref) } else { - debug!("Recursed"); + trace!("Recursed"); for scope in &self.defined_scopes() { if scope.span().contains_pos(pos) { return scope.reference_at_pos(pos); @@ -219,7 +219,7 @@ impl <'t> ContextedSymbol<'t> { impl SymbolContext { pub fn lookup_symbol<'t>(&'t self, pos: &ZeroFilePosition) -> Option> { - debug!("Starting lookup in {:?}", self.get_name()); + debug!("Starting file-position lookup in {:?}", self.get_name()); let mut acc = vec![]; self.lookup_symbol_aux(pos, &mut acc).map( |sub|ContextedSymbol { @@ -234,7 +234,7 @@ impl SymbolContext { -> Option<&'t SimpleSymbol> { acc.push(&self.context); self.subsymbols.iter() - .find(|sub|{debug!("Considering {:?}, contains pos? {:?}", + .find(|sub|{trace!("Considering {:?}, contains pos? {:?}", sub, sub.contains_pos(pos)); sub.contains_pos(pos)}) diff --git a/src/analysis/structure/objects.rs b/src/analysis/structure/objects.rs index b69be6a0..4d7d5376 100644 --- a/src/analysis/structure/objects.rs +++ b/src/analysis/structure/objects.rs @@ -325,6 +325,7 @@ impl Import { #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct InEach { + pub loc: ZeroSpan, pub span: ZeroSpan, pub spec: Vec, pub statements: Statements, @@ -399,6 +400,10 @@ impl ToStructure for InEach { content.spec.references(&mut references, file); content.statements.references(&mut references, file); Some(InEach { + loc: ZeroSpan::from_range( + ZeroRange::combine(content.intok.range(), + content.each.range()), + file.path), span, spec, statements, @@ -1014,6 +1019,12 @@ impl StructureSymbol for Method { } } +impl MaybeAbstract for Method { + fn is_abstract(&self) -> bool { + matches!(self.body.as_ref(), StatementKind::Empty(_)) + } +} + impl Scope for Method { fn create_context(&self) -> ContextKey { ContextKey::Method(SimpleSymbol::make(self, self.kind())) diff --git a/src/analysis/symbols.rs b/src/analysis/symbols.rs index 487faf4f..18192038 100644 --- a/src/analysis/symbols.rs +++ b/src/analysis/symbols.rs @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 and MIT use std::sync::Arc; -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use std::sync::Mutex; use crate::analysis::{Named, LocationSpan}; @@ -11,7 +11,7 @@ use crate::analysis::parsing::tree::ZeroSpan; use crate::analysis::structure::objects::{CompObjectKind}; use crate::analysis::structure::expressions::DMLString; -use crate::analysis::templating::objects::DMLObject; +use crate::analysis::templating::objects::{DMLObject, DMLNamedMember, StructureKey}; use crate::analysis::templating::methods::{DMLMethodRef}; use crate::analysis::templating::types::DMLResolvedType; use crate::analysis::templating::traits::DMLTemplate; @@ -112,6 +112,10 @@ impl SimpleSymbol { #[derive(Debug, Clone, Eq, PartialEq)] pub enum SymbolSource { DMLObject(DMLObject), + // (key of containing object, method ref) + // TODO: same principle for shared methods? likely not, there is + // no object for the topmost method + Method(StructureKey, Arc), MethodArg(Arc, DMLString), MethodLocal(Arc, DMLString), // TODO: RC this if it's expensive @@ -126,6 +130,13 @@ impl SymbolSource { _ => None } } + pub fn as_method(&self) -> Option<(&StructureKey, &Arc)> { + match self { + Self::Method(key, methref) => Some((key, methref)), + _ => None + } + } + pub fn as_metharg(&self) -> Option<(&Arc, &DMLString)> { match self { Self::MethodArg(arg, name) => Some((arg, name)), @@ -153,6 +164,17 @@ impl SymbolSource { (_, _) => self == other } } + + pub fn maybe_name(&self) -> Option<&str> { + match self { + SymbolSource::DMLObject(obj) => obj.as_shallow().map(|o| o.identity()), + SymbolSource::Method(_, methref) => Some(methref.identity()), + SymbolSource::MethodArg(_, name) => Some(&name.val), + SymbolSource::MethodLocal(_, name) => Some(&name.val), + SymbolSource::Type(_) => None, + SymbolSource::Template(templ) => Some(&templ.name), + } + } } // Intentionally omitted implementation of clone, should not be done @@ -162,22 +184,24 @@ pub struct Symbol { pub id: SymbolID, pub loc: ZeroSpan, pub kind: DMLSymbolKind, + // Bases are the topmost declarations of this symbol + pub bases: Vec, + // Definitions are the sites which define at least part of this symbol pub definitions: Vec, + // Declarations are the sites which declare this symbol pub declarations: Vec, + // References are all locations that may refer to this symbol pub references: HashSet, // NOTE: The meaning of 'implementation' varies with symbol kind - // For methods and interfaces, this is straightforward - // For templates, it will give all declarations for all objects that - // directly or indirectly implement the template + // - For interfaces this is straightforward + // - For templates it will give all direct instantiations of the template + // - For methods, it gives all methods that directly override this method, + // note that when presented to the user, we give both direct and indirect overrides // TODO: For objects, we could give the locations of in-eachs that affect it // there is no way to go to these specs without finding definitions of // some param or method set in them - pub implementations: Vec, - pub bases: Vec, + pub implementations: HashSet, pub source: SymbolSource, - // Used for method symbols only, maps default references - // to the method_decl they should resolve to - pub default_mappings: HashMap, // TODO: RC or box this if it's expensive pub typed: Option, } @@ -286,10 +310,21 @@ impl Symbol { definitions: Vec::default(), declarations: Vec::default(), references: HashSet::default(), - implementations: Vec::default(), + implementations: HashSet::default(), bases: Vec::default(), - default_mappings: HashMap::default(), typed: None, } } + + // Prints appropriate info about a method without clogging the output + // with minutiae for symbols that may contain lots of sub-data (methods) + pub fn medium_debug_info(&self) -> String { + format!( + "Symbol({}, {:?} at loc: {:?})", + self.source.maybe_name() + .map_or(format!("id={}", self.id), + |name| format!("{} (id={})", name, self.id)), + self.kind, + self.loc) + } } diff --git a/src/analysis/templating/methods.rs b/src/analysis/templating/methods.rs index b22b69e3..bf57a198 100644 --- a/src/analysis/templating/methods.rs +++ b/src/analysis/templating/methods.rs @@ -163,6 +163,7 @@ impl DeclarationSpan for MethodDecl { pub trait MethodDeclaration : DMLNamedMember + MaybeAbstract { fn is_shared(&self) -> bool; + fn is_default(&self) -> bool; fn throws(&self) -> bool; fn fully_typed(&self) -> bool { for arg in self.args() { @@ -175,12 +176,27 @@ pub trait MethodDeclaration : DMLNamedMember + MaybeAbstract { fn args(&self) -> &Vec; fn returns(&self) -> &Vec; + // TODO: Let this take multiple overridden methods, as there are allowed cases + // of that and we can provide better (fewer, more compact) reports, bundling + // together similar ones fn check_override(&self, overridden: &T, report: &mut Vec) where T : MethodDeclaration, { + if self.is_shared() && !overridden.is_shared() { + report.push(DMLError { + span: *self.location(), + description: + "Shared method cannot override non-shared method".to_string(), + related: vec![( + *overridden.location(), + "Overridden method declared here".to_string())], + severity: Some(DiagnosticSeverity::ERROR), + }); + } + if self.throws() && !overridden.throws() { report.push(DMLError { span: *self.location(), @@ -257,6 +273,10 @@ impl MethodDeclaration for MethodDecl { matches!(self.modifier, MethodModifier::Shared) } + fn is_default(&self) -> bool { + self.default + } + fn throws(&self) -> bool { self.throws } @@ -287,141 +307,155 @@ impl MethodDecl { } } +// Note: A method can have a template ref even if not shared, it just means +// it was declared top-level within a template #[derive(Clone, Debug, PartialEq, Eq)] -pub enum DMLMethodRef { - TraitMethod(Arc, MethodDecl), - ConcreteMethod(DMLConcreteMethod), +pub struct DMLMethodRef { + pub template_ref: Option>, + pub concrete_decl: DMLConcreteMethod, } impl DMLMethodRef { + pub fn get_disambiguation_name(&self) -> Option { + self.template_ref.as_ref().map( + |trt|format!("templates.{}.{}", + trt.name, self.identity())) + } + pub fn get_decl(&self) -> &MethodDecl { - match self { - Self::TraitMethod(_, decl) => decl, - Self::ConcreteMethod(conc) => &conc.decl, - } + &self.concrete_decl.decl } - pub fn get_default(&self) -> Option> { - match self { - Self::TraitMethod(_, _) => None, - Self::ConcreteMethod(decl) => decl.default_call - .as_ref().map(Arc::clone), - } + pub fn get_default(&self) -> Option<&DefaultCallReference> { + self.concrete_decl.default_call.as_ref() } pub fn get_all_defs(&self) -> Vec { - match self { - DMLMethodRef::TraitMethod(_, decl) => if !decl.is_abstract() { - vec![*decl.location()] - } else { - vec![] - }, - DMLMethodRef::ConcreteMethod(concmeth) => concmeth.get_all_defs(), - } + self.concrete_decl.get_all_defs() } pub fn get_all_decls(&self) -> Vec { - match self { - DMLMethodRef::TraitMethod(_, decl) => if decl.is_abstract() { - vec![*decl.location()] - } else { - vec![] - }, - DMLMethodRef::ConcreteMethod(concmeth) => concmeth.get_all_defs(), - } + self.concrete_decl.get_all_decls() } - pub fn get_base(&self) -> MethodDecl { - match self { - DMLMethodRef::TraitMethod(_, decl) => { - decl.clone() - }, - DMLMethodRef::ConcreteMethod(meth) => { - if let Some(default) = &meth.default_call { - default.get_base() - } else { - meth.decl.clone() - } - } - } + // Invariant: Return is non-empty + pub fn get_bases(&self) -> Vec { + log::trace!("Getting bases of {:?}", self); + let to_return = if let Some(default_refs) = &self.concrete_decl.default_call { + default_refs.get_bases() + } else { + vec![self.concrete_decl.decl.clone()] + }; + assert!(!to_return.is_empty()); + to_return } } impl MaybeAbstract for DMLMethodRef { fn is_abstract(&self) -> bool { - match self { - DMLMethodRef::TraitMethod(_, decl) => decl.is_abstract(), - DMLMethodRef::ConcreteMethod(decl) => decl.is_abstract(), - } + self.concrete_decl.is_abstract() } } impl MethodDeclaration for DMLMethodRef { fn is_shared(&self) -> bool { - match self { - DMLMethodRef::TraitMethod(_, decl) => decl.is_shared(), - DMLMethodRef::ConcreteMethod(decl) => decl.is_shared(), - } + self.concrete_decl.is_shared() + } + + fn is_default(&self) -> bool { + self.concrete_decl.is_default() } fn throws(&self) -> bool { - match self { - DMLMethodRef::TraitMethod(_, decl) => decl.throws(), - DMLMethodRef::ConcreteMethod(decl) => decl.throws(), - } + self.concrete_decl.throws() } fn args(&self) -> &Vec { - match self { - DMLMethodRef::TraitMethod(_, decl) => decl.args(), - DMLMethodRef::ConcreteMethod(decl) => decl.args(), - } + self.concrete_decl.args() } fn returns(&self) -> &Vec { - match self { - DMLMethodRef::TraitMethod(_, decl) => decl.returns(), - DMLMethodRef::ConcreteMethod(decl) => decl.returns(), - } + self.concrete_decl.returns() } } impl DMLNamedMember for DMLMethodRef { fn identity(&self) -> &str { + self.concrete_decl.identity() + } + + fn kind_name(&self) -> &'static str { + self.concrete_decl.kind_name() + } + + fn location(&self) -> &ZeroSpan { + self.concrete_decl.location() + } +} + +impl DeclarationSpan for DMLMethodRef { + fn span(&self) -> &ZeroSpan { + self.concrete_decl.span() + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DefaultCallReference { + Abstract(Arc), + Valid(Arc), + Ambiguous(Vec>), +} + +impl DefaultCallReference { + pub fn get_bases(&self) -> Vec { match self { - DMLMethodRef::TraitMethod(_, decl) => decl.identity(), - DMLMethodRef::ConcreteMethod(decl) => decl.identity(), + DefaultCallReference::Abstract(method) | + DefaultCallReference::Valid(method) => method.get_bases(), + DefaultCallReference::Ambiguous(methods) => + methods.iter().flat_map(|m|m.get_bases()).collect(), } } - fn kind_name(&self) -> &'static str { + pub fn get_all_decls(&self) -> Vec { match self { - DMLMethodRef::TraitMethod(_, _) => "shared method", - DMLMethodRef::ConcreteMethod(_) => "method", + DefaultCallReference::Abstract(method) => vec![*method.location()], + DefaultCallReference::Valid(method) => method.get_all_decls(), + DefaultCallReference::Ambiguous(methods) => + methods.iter().flat_map(|m|m.get_all_decls()).collect(), } } - fn location(&self) -> &ZeroSpan { + pub fn get_all_defs(&self) -> Vec { match self { - DMLMethodRef::TraitMethod(_, decl) => decl.location(), - DMLMethodRef::ConcreteMethod(decl) => decl.location(), + DefaultCallReference::Abstract(_) => vec![], + DefaultCallReference::Valid(method) => method.get_all_defs(), + DefaultCallReference::Ambiguous(methods) => + methods.iter().flat_map(|m|m.get_all_defs()).collect(), } } -} -impl DeclarationSpan for DMLMethodRef { - fn span(&self) -> &ZeroSpan { + pub fn as_valid(&self) -> Option<&Arc> { + match self { + DefaultCallReference::Valid(method) => Some(method), + DefaultCallReference::Ambiguous(_) => None, + DefaultCallReference::Abstract(_) => None, + } + } + + pub fn flat_refs(&self) -> Vec<&Arc> { match self { - DMLMethodRef::TraitMethod(_, decl) => decl.span(), - DMLMethodRef::ConcreteMethod(decl) => decl.span(), + DefaultCallReference::Abstract(method) | + DefaultCallReference::Valid(method) => vec![method], + DefaultCallReference::Ambiguous(methods) => + methods.iter().collect() } } - } +} // This is roughly equivalent with a non-codegenned method in DMLC #[derive(Debug, Clone, PartialEq, Eq)] pub struct DMLConcreteMethod { pub decl: MethodDecl, - // where the default call goes (None if invalid) - pub default_call: Option>, + // where the default call goes (None if nothing was overriding) + pub default_call: Option, } impl DMLConcreteMethod { @@ -464,6 +498,10 @@ impl MethodDeclaration for DMLConcreteMethod { self.decl.is_shared() } + fn is_default(&self) -> bool { + self.decl.default + } + fn throws(&self) -> bool { self.decl.throws() } diff --git a/src/analysis/templating/objects.rs b/src/analysis/templating/objects.rs index f8a516ea..bdf764fd 100644 --- a/src/analysis/templating/objects.rs +++ b/src/analysis/templating/objects.rs @@ -28,18 +28,15 @@ use crate::analysis::structure::toplevel::{ExistCondition, ObjectDecl, use crate::analysis::symbols::{DMLSymbolKind}; use crate::analysis::templating::Declaration; use crate::analysis::templating::types::eval_type; -use crate::analysis::templating::methods::{DMLMethodRef, - MethodDecl, MethodDeclaration, - DMLConcreteMethod}; +use crate::analysis::templating::methods::{DMLConcreteMethod, DMLMethodRef, DefaultCallReference, MethodDecl, MethodDeclaration}; use crate::analysis::templating::topology::{InEachStruct, InferiorVariant, Rank, RankDesc, RankDescKind, RankMaker, topsort}; use crate::analysis::templating::traits::{DMLTemplate, DMLTrait, - get_impls, merge_impl_maps, - TraitMemberKind, TemplateTraitInfo}; + get_impls, TraitMemberKind, TemplateTraitInfo}; use crate::analysis::{LocationSpan, DeclarationSpan, combine_vec_of_decls}; -type InEachSpec = HashMap, Arc)>>; +type InEachSpec = HashMap, (ZeroSpan, Arc))>>; pub type StructureKey = DefaultKey; pub type StructureContainer = SlotMap; @@ -184,7 +181,7 @@ fn create_spec<'t>(loc: ZeroSpan, if let Some((first, rest)) = ineach.obj.spec.split_first() { if let Some(in_each_spec) = in_each_specs.get(ineach) { let to_add = (rest.iter().map(|t|t.val.clone()).collect(), - Arc::clone(in_each_spec)); + (ineach.obj.loc, Arc::clone(in_each_spec))); if let Some(e) = in_eachs.get_mut(&first.val) { e.push(to_add); } else { @@ -332,7 +329,7 @@ pub fn make_device<'t>(path: &str, container, report); let device_obj = container.get(obj_key).unwrap(); - debug!("Device components are: {:?}", device_obj.components); + trace!("Device components are: {:?}", device_obj.components); export_invariants(toplevel, device_obj, container, report); device_obj } @@ -749,13 +746,16 @@ impl DMLAmbiguousDef { } } - pub fn get_likely_declaration(&self) -> &T { - if let Some(def) = self.declarations.first() { + pub fn get_last_declaration(&self) -> &T { + if let Some(decl) = self.declarations.last() { + &decl.1 + } else if let Some(def) = self.definitions.last() { &def.1 } else { - &self.used_definitions.first().unwrap().1 + &self.used_definitions.last().unwrap().1 } } + pub fn get_unambiguous_def(&self) -> Option<&T> { if self.used_definitions.len() == 1 { self.used_definitions.first().map(|t|&t.1) @@ -822,6 +822,8 @@ pub struct DMLCompositeObject { // None: 'none' for size in arraydim here means an _unknown_ size pub arraydimvars: Vec, pub components: HashMap, + // Used for goto-implementation on composite objects + pub used_ineach_locs: Vec, } impl DMLCompositeObject { @@ -920,8 +922,10 @@ impl DMLCompositeObject { // we need to add) // NOTE: new objectspecs are pathed with the full condition chain required // to instantiate them +// Returns all spans of final actually used ineachspecs, needed for +// implementations tracking for composite objects fn add_template_specs(obj_specs: &mut Vec>, - source_each_stmts: &InEachSpec) { + source_each_stmts: &InEachSpec) -> Vec{ let mut each_stmts = source_each_stmts.clone(); // TODO: We need to handle conditional imports and is-es here, as these are // allowed in _some_ cases. For now, we merely pretend the conditions do not @@ -948,6 +952,8 @@ fn add_template_specs(obj_specs: &mut Vec>, // true in whatever context it would work in let mut used_templates = HashSet::::default(); + let mut used_ineach_spans = vec![]; + while let Some((_, tpl)) = queue.pop() { // TODO: here we would have to consider cond when conditional // is/imports exist @@ -958,31 +964,35 @@ fn add_template_specs(obj_specs: &mut Vec>, tpl.name); used_templates.insert(tpl.name.to_string()); let mut modifications = vec![]; - if let Some(templ_specs) = each_stmts.get(&tpl.name) { - for (needed_templates, spec) in templ_specs { - let mut can_add = true; - for templ in needed_templates { - if !used_templates.contains(templ.as_str()) { - // We will need to re-add a check for this template, - // in case it will appear later in the queue - modifications.push((templ.clone(), - needed_templates.clone(), - Arc::clone(spec))); - can_add = false; - break; + { + if let Some(templ_specs) = each_stmts.get(&tpl.name) { + for (needed_templates, (loc, spec)) in templ_specs { + let mut can_add = true; + for templ in needed_templates { + if !used_templates.contains(templ.as_str()) { + // We will need to re-add a check for this template, + // in case it will appear later in the queue + modifications.push( + (templ.clone(), + needed_templates.clone(), + (*loc, Arc::clone(spec)))); + can_add = false; + break; + } + } + if can_add { + queue.extend(spec.instantiations.values() + .flat_map(|v|v.iter()) + .map(|t|(spec.condition.clone(), + Arc::clone(t)))); + obj_specs.push(Arc::clone(spec)); + used_ineach_spans.push(*loc); } - } - if can_add { - queue.extend(spec.instantiations.values() - .flat_map(|v|v.iter()) - .map(|t|(spec.condition.clone(), - Arc::clone(t)))); - obj_specs.push(Arc::clone(spec)); } } } - for (name, templs, spec) in modifications { - let to_insert = (templs, spec); + for (name, templs, (loc, spec)) in modifications { + let to_insert = (templs, (loc, spec)); if let Some(e) = each_stmts.get_mut(&name) { e.push(to_insert); } else { @@ -998,6 +1008,8 @@ fn add_template_specs(obj_specs: &mut Vec>, queue.push((d.cond.clone(), Arc::clone(v))); } } + + used_ineach_spans } // Simply adds the in_eachs specified in object specs to the ineachspec @@ -1339,6 +1351,7 @@ fn create_object_instance(loc: Option, declloc: loc.unwrap_or(identity.span), all_decls, identity: identity.clone(), + used_ineach_locs: vec![], key: StructureKey::null(), kind, definitions: vec![], @@ -1764,45 +1777,55 @@ fn merge_composite_subobjs<'c>(parent_each_stmts: &InEachSpec, // TODO: Check collisions of 'name' parameters } -// return a tuple (default_map, order) -// where 'default_map' maps method declarations to declarations they directly -// override +// Maps a method decl to the method definitions it directly overrides, and the method declarations it directly overrides +type DeclMap<'t> = HashMap<&'t MethodDecl, HashSet<&'t MethodDecl>>; + +// return a tuple (default_map, order, declaration_map) +// where 'default_map' maps method declarations to definitions they directly override // and 'order' is a order of methods, with lowest rank being last +// and 'declaration_map' maps method declarations to the abstract method declarations they directly override fn sort_method_decls<'t>(decls: &[(&Rank, &'t MethodDecl)]) -> - (HashMap<&'t MethodDecl, HashSet<&'t MethodDecl>>, Vec<&'t MethodDecl>) { - trace!("Sorting method declarations: {:?}", decls); - let mut rank_to_method: HashMap<&Rank, &MethodDecl> + (DeclMap<'t>, Vec<&'t MethodDecl>, DeclMap<'t>) { + debug!("Sorting method decls {:?}", decls); + let mut rank_to_method_def: HashMap<&Rank, Vec<&MethodDecl>> = HashMap::default(); - let mut conflicting_decls: HashMap<&MethodDecl, Vec<&MethodDecl>> + let mut rank_to_method_decl: HashMap<&Rank, Vec<&MethodDecl>> = HashMap::default(); + for (rank, decl) in decls { - if let Some(conflict) = rank_to_method.get(rank) { - // Conflicting declarations within one block - conflicting_decls.entry(conflict) - .and_modify(|e|e.push(decl)) - .or_insert_with(||vec![decl]); + if decl.is_abstract() { + rank_to_method_decl.entry(rank).or_default().push(*decl); } else { - rank_to_method.insert(rank, *decl); + rank_to_method_def.entry(rank).or_default().push(*decl); } } - - trace!("Conflicting declarations are: {:?}", conflicting_decls); - - let ranks: HashSet<&Rank> = rank_to_method.keys() - .cloned().collect(); - - let ancestry: HashMap<&Rank, Vec<&Rank>> = ranks.iter() - .map(|r|(*r, ranks.iter().filter( + trace!("rank-to-method-def is: {:?}, rank-to-method-decl is: {:?}", rank_to_method_def, rank_to_method_decl); + + // Ancestry of defs->defs and defs->decls + #[allow(clippy::type_complexity)] + let (def_ancestry, decl_ancestry): (HashMap<&Rank, Vec<&Rank>>, + HashMap<&Rank, Vec<&Rank>>) = { + let def_ranks: HashSet<&Rank> = rank_to_method_def.keys() + .cloned().collect(); + let decl_ranks: HashSet<&Rank> = rank_to_method_decl.keys() + .cloned().collect(); + let def_ancestry = def_ranks.iter().map(|r|(*r, def_ranks.iter().filter( |r2|r.inferior.contains(&r2.id)).cloned().collect())) - .collect(); + .collect(); + let decl_ancestry = def_ranks.iter().map(|r|(*r, decl_ranks.iter().filter( + |r2|r.inferior.contains(&r2.id)).cloned().collect())) + .collect(); + (def_ancestry, decl_ancestry) + }; + trace!("Def ancestry is {:?}", def_ancestry); let mut minimal_ancestry: HashMap<&Rank, Vec<&Rank>> = HashMap::default(); - // Get ancestors that are not the ancestor of any ancestor - for (rank, ancestors) in &ancestry { + // For each rank, map to its first-level ancestry + for (rank, ancestors) in &def_ancestry { let mut minimal_ancestors = vec![]; for ancestor in ancestors { - if !ancestors.iter().flat_map(|a|ancestry.get(a).unwrap()) + if !ancestors.iter().flat_map(|a|def_ancestry.get(a).unwrap()) .any(|r|r == ancestor) { minimal_ancestors.push(*ancestor); } @@ -1810,33 +1833,58 @@ fn sort_method_decls<'t>(decls: &[(&Rank, &'t MethodDecl)]) -> minimal_ancestry.insert(rank, minimal_ancestors); } - trace!("Minimal ancestors are {:?}", minimal_ancestry); + // Map method definitions to abstract decls they override, which none of their minimal ancestors override + let mut method_map_decls: DeclMap<'t> = HashMap::default(); + for (rank, ancestors) in &decl_ancestry { + for def in rank_to_method_def.get(rank).unwrap() { + method_map_decls.entry(def).or_default(); + for ancestor in ancestors { + if !minimal_ancestry.get(rank).unwrap().iter().any( + |ma|decl_ancestry.get(ma).unwrap().iter().any(|a|a == ancestor)) { + if let Some(decls) = rank_to_method_decl.get(ancestor) { + method_map_decls.entry(def).or_default().extend(decls.iter()); + } + } + } + // Decls at the same rank are also directly overridden, by definition + if let Some(decls) = rank_to_method_decl.get(rank) { + method_map_decls.entry(def).or_default().extend(decls.iter()); + } + trace!("Method {:?} at rank {:?} directly overrides decls {:?}", + def, rank, method_map_decls.get(def).unwrap()); + } + } - // TODO: there is an error that is missed here, - // see L463 of traits.py in dmlc source - // (or, if it has moved, the EAMBINH report in the - // sort_method_implementations function in the same - // file) + trace!("Minimal ancestry is {:?}", minimal_ancestry); + // Map method definitions to those that they directly override + let mut method_map_defs: DeclMap<'t> = HashMap::default(); - let method_map: HashMap<&'t MethodDecl, HashSet<&'t MethodDecl>> - = rank_to_method.iter() - .map(|(r, m)|(*m, - minimal_ancestry[r].iter() - .map(|subrank|&rank_to_method[*subrank]) - .cloned().collect())) - .collect(); - trace!("Default map is {:?}", method_map); - let method_order = topsort(&method_map).unwrap_sorted(); + for (r, defs) in &rank_to_method_def { + for def in defs { + trace!("Handling method {:?} at rank {:?}", def, r); + // Ensure the entry exists for later unwraps + let mdefs = method_map_defs.entry(def).or_default(); + for subrank in &minimal_ancestry[r] { + let subdefs = rank_to_method_def.get(subrank).unwrap(); + trace!("Set as overriding {:?}", subdefs); + mdefs.extend(subdefs.iter()); + } + } + } + + trace!("Default map is {:?}", method_map_defs); + trace!("Abstract override map is {:?}", method_map_decls); + let method_order = topsort(&method_map_defs).unwrap_sorted(); trace!("Method order is {:?}", method_order); - (method_map, method_order) + (method_map_defs, method_order, method_map_decls) } fn add_methods(obj: &mut DMLCompositeObject, methods: MethodMapping, - method_map: &HashMap>, + decl_to_trait_map: &HashMap>, report: &mut Vec) { debug!("Adding methods to {}", obj.identity()); - trace!("Shared methods are {:?}", method_map); + for (name, (used, regular_decls)) in methods { if !used { trace!("Skipped {} name, declaration unused", name); @@ -1844,63 +1892,56 @@ fn add_methods(obj: &mut DMLCompositeObject, } trace!("Handling method {}, which is declared by {:?}", name, regular_decls); - let mut all_decls: Vec<(&Rank, &MethodDecl)> + let all_decls: Vec<(&Rank, &MethodDecl)> = regular_decls.iter().map(|(a, b)|(a, b)).collect(); - // TODO: If we obtain a shared method that is not in a trait - // does this mean it was declared in a non-shared context, - // and if so, do we need to report it? - if let Some(trait_impl) = method_map.get(&name) { - if let Some(trait_decl) = trait_impl.get_method(&name) { - all_decls.push((&trait_impl.rank, trait_decl)); - } - } - trace!("All possible declarations are: {:?}", all_decls); - let (default_map, method_order) = sort_method_decls(&all_decls); - trace!("Method order is: {:?}", method_order); + + let (default_map, method_order, decl_map) = sort_method_decls(&all_decls); let mut decl_to_method: HashMap> = HashMap::default(); - for method in method_order.iter().rev() { - debug!("Handling overrides of {:?}", method); - // Guaranteed, every val in method order is a key in default_map - let default_decls = default_map.get(method).unwrap(); - if method.is_shared() { - trace!("Was a shared method, deferred"); - let conflicting_decls : Vec<&MethodDecl> = default_decls.iter() - .filter(|d|!d.is_shared()).cloned().collect(); - if !conflicting_decls.is_empty() { - report.push(DMLError { - span: method.name.span, - description: - "Shared method cannot override non-shared method" - .to_string(), - related: conflicting_decls.iter().map( - |d|(d.name.span, - "Overrides this non-shared method".to_string())) - .collect(), - severity: Some(DiagnosticSeverity::ERROR), - }); - } - // The actual shared method "codegenning" is done later - // since it is not stored with the containing object - let new_method = Arc::new( - DMLMethodRef::TraitMethod( - Arc::clone(method_map.get(method.identity()).unwrap()), - (*method).clone())); - trace!("Inserted dependent methoddecl {:?}", new_method); - decl_to_method.insert((*method).clone(), new_method); + // Create abstract methodrefs for declarations first, order does not matter + for method in decl_map.values().flatten() { + if decl_to_method.contains_key(method) { continue; } + let template_ref = decl_to_trait_map + .get(method) + .map(Arc::clone); + + let new_method = Arc::new( + DMLMethodRef { + template_ref, + concrete_decl: DMLConcreteMethod { + decl: (*method).clone(), + default_call: None, + }, + } + ); + trace!("Inserted dependent methoddecl {:?}", new_method); + decl_to_method.insert((*method).clone(), new_method); + } + trace!("Abstract decl methods are {:?}", decl_to_method); + for method in method_order.iter().rev() { + trace!("Handling overrides of {:?}", method); + // Guaranteed by topsort let defaults = default_map.get(method).unwrap(); - let default = if defaults.len() == 1 { - let decl = defaults.iter().next().unwrap(); - trace!("Default call decl is {:?}", decl); - trace!("And the current map is {:?}", decl_to_method); - Some(decl_to_method.get(decl).unwrap()) - } else { - None + let default = match defaults.len() { + 1 => { + let decl = defaults.iter().next().unwrap(); + Some(DefaultCallReference::Valid(Arc::clone(decl_to_method.get(decl).unwrap()))) + }, + 0 => decl_map.get(method).and_then(|decls| { + match decls.len() { + 1 => Some(DefaultCallReference::Abstract( + Arc::clone(decls.iter().next().and_then(|d|decl_to_method.get(d)).unwrap()))), + // The case of conflicting abstract declarations is handled elsewhere + _ => None, + } + }), + _ => Some(DefaultCallReference::Ambiguous( + defaults.iter().map(|d| Arc::clone(decl_to_method.get(d).unwrap())).collect())), }; trace!("Default call would be {:?}", default); - for decl in default_map.get(method).unwrap() { + for decl in defaults.iter().chain(decl_map.get(method).unwrap_or(&HashSet::default())) { // TODO: I suspect we could improve this error message in cases // where several similar incorrect overrides are made over one // method @@ -1919,21 +1960,28 @@ fn add_methods(obj: &mut DMLCompositeObject, }); } - if !decl.is_shared() { - method.check_override(*decl, report); - } + method.check_override(*decl, report); } - let new_method = Arc::new(DMLMethodRef::ConcreteMethod( - DMLConcreteMethod { - decl: (*method).clone(), - default_call: default.cloned(), - })); + let template_ref = decl_to_trait_map + .get(method) + .map(Arc::clone); + + let new_method = Arc::new( + DMLMethodRef { + template_ref, + concrete_decl: DMLConcreteMethod { + decl: (*method).clone(), + default_call: default, + }, + } + ); trace!("Inserted dependent methoddecl {:?}", new_method); decl_to_method.insert((*method).clone(), new_method); } trace!("Complete decl-to-method map is: {:?}", decl_to_method); - let to_add = decl_to_method.get(method_order.first().unwrap()).unwrap(); + // If there are no definitions, skip to next method (declarations are separately added above) + let Some(to_add) = method_order.first().map(|m|decl_to_method.get(m).unwrap()) else { continue; }; debug!("Added {:?}", to_add.identity()); obj.add_shallow_component(DMLShallowObjectVariant::Method( Arc::clone(to_add))); @@ -1944,7 +1992,7 @@ fn check_trait_overrides(obj: &DMLCompositeObject, container: &StructureContainer, report: &mut Vec) { debug!("Checking traits overrides on {:?}", obj.identity); - debug!("all symbols are: {:?}", + trace!("all symbols are: {:?}", obj.components.keys().collect::>()); fn report_collision(name: &str, srcloc: ZeroSpan, @@ -2014,7 +2062,13 @@ fn check_trait_overrides(obj: &DMLCompositeObject, variant: DMLShallowObjectVariant::Method(m), .. }) = collision { - m.check_override(meth, report); + // Special case, we are allowed conflict with + // an abstract method if we can override it + if m.is_abstract() { + meth.check_override(m.get_decl(), report); + } else { + m.check_override(meth, report); + } } } } else if let TraitMemberKind::Method(meth) = member { @@ -2184,10 +2238,10 @@ pub fn make_object(loc: ZeroSpan, debug!("Making object {}", identity.val); let mut each_stmts = parent_each_stmts.clone(); - add_template_specs(&mut obj_specs, &each_stmts); + let used_ineach_locs = add_template_specs(&mut obj_specs, &each_stmts); add_template_ineachs(&obj_specs, &mut each_stmts); - debug!("Has specs at {:?}", obj_specs.iter().map(|rc|rc.loc) + trace!("Has specs at {:?}", obj_specs.iter().map(|rc|rc.loc) .collect::>()); trace!("Complete specs are: {:?}", @@ -2205,7 +2259,7 @@ pub fn make_object(loc: ZeroSpan, saveds, sessions, methods, hooks, subobjs) = collect_symbols(¶meters, &obj_specs, report); - debug!("All local symbols are: {:?}", + trace!("All local symbols are: {:?}", symbols.keys().map(|k|k.as_str()).collect::>()); let new_obj_key = create_object_instance(Some(loc), @@ -2220,6 +2274,7 @@ pub fn make_object(loc: ZeroSpan, { let new_obj = container.get_mut(new_obj_key).unwrap(); new_obj.definitions = obj_specs.clone(); + new_obj.used_ineach_locs = used_ineach_locs; add_templates(new_obj, &obj_specs); add_constants(new_obj, constants); add_parameters(new_obj, parameters); @@ -2232,10 +2287,12 @@ pub fn make_object(loc: ZeroSpan, add_subobjs(new_obj_key, subobj_keys, container); { let new_obj = container.get_mut(new_obj_key).unwrap(); - let trait_method_map = merge_impl_maps(&identity.val, &loc, - new_obj.templates.values().map( - |t|&t.traitspec), - report); + let trait_method_map = new_obj.templates.values() + .flat_map(|t|t.traitspec.methods.values() + .map(|m|(m.decl.clone(), Arc::clone(&t.traitspec))) + .collect::>()) + .collect(); + add_methods(new_obj, methods, &trait_method_map, report); } check_trait_overrides(container.get(new_obj_key).unwrap(), diff --git a/src/analysis/templating/topology.rs b/src/analysis/templating/topology.rs index 484d152b..7a1b0c75 100644 --- a/src/analysis/templating/topology.rs +++ b/src/analysis/templating/topology.rs @@ -309,7 +309,7 @@ pub fn dependencies<'t>(statements: &'t StatementSpec, InferiorVariant::Import(imp) => { let name = imp_map.get(&imp.obj) .map_or_else(||imp.obj.imported_name(), |s|s.as_str()); - debug!("Mapped {:?} to {:?}", imp.obj, name); + debug!("Mapped import {:?} to {:?}", imp.obj, name); inferior.insert(name, InferiorVariant::Import(imp)); if imp.cond == ExistCondition::Always { @@ -612,7 +612,7 @@ pub fn rank_templates_aux<'t>(mut templates: HashMap<&'t str, DMLError { span: *span, description: format!( - "No template; '{}'", + "No template named '{}'", missing_template_name), related: vec![], severity: Some( @@ -627,6 +627,42 @@ pub fn rank_templates_aux<'t>(mut templates: HashMap<&'t str, debug!("Implicit import of '{}' missing and intentionally ignored", missing_template_name); }, + InferiorVariant::InEach(decl) => { + if !BUILTIN_TEMPLATES.iter().any( + |name|name==missing_template_name) { + // The 'is' may contain more names than we + // are looking for, find the particular name + // that matches and use that span + // Because the same name could be used multiple + // times, but this only causes inferior binding + // report one error per such span + + let spans: Vec<_> = decl.obj.spec.iter() + .filter(|dmlname| + &dmlname.val == missing_template_name) + .map(|dmlname|dmlname.span()) + .collect(); + if spans.is_empty() { + internal_error!("Unexpectedly no name \ + matching missing template \ + in {:?} (wanted {})", + decl, missing_template_name); + continue; + } + for span in spans { + report.push( + DMLError { + span: *span, + description: format!( + "No template; '{}'", + missing_template_name), + related: vec![], + severity: Some( + DiagnosticSeverity::ERROR), + }); + } + } + }, inf => { internal_error!( "Unexpected template dependency through {:?}", diff --git a/src/analysis/templating/traits.rs b/src/analysis/templating/traits.rs index d28b01bb..ab2eaed7 100644 --- a/src/analysis/templating/traits.rs +++ b/src/analysis/templating/traits.rs @@ -7,14 +7,14 @@ use std::iter; use std::sync::Arc; use crate::analysis::parsing::tree::ZeroSpan; -use crate::analysis::structure; use crate::analysis::structure::objects::{MaybeAbstract}; use crate::analysis::structure::expressions::DMLString; use crate::analysis::structure::toplevel::{ObjectDecl, ExistCondition}; use crate::analysis::DMLError; -use crate::analysis::templating::methods::{MethodDecl, MethodDeclaration}; +use crate::analysis::templating::methods::{DMLConcreteMethod, DMLMethodRef, DefaultCallReference, MethodDecl, MethodDeclaration}; use crate::analysis::templating::objects::{CoarseObjectKind, + DMLNamedMember, DMLCoarseObjectKind, ObjectSpec, Spec}; use crate::analysis::templating::Declaration; @@ -42,14 +42,18 @@ pub struct DMLTrait { pub ancestors: HashMap>, // This is the final specification of a trait in analysis. So // lock in the objects here and move them to owned - // for further analysis later. Not that these are + // for further analysis later. Note that these are // _not_ objectdecls as conditional declarations are not // part of template type (this is IMO weird but what can you do) pub sessions: HashMap, pub saveds: HashMap, pub params: HashMap, - pub methods: HashMap, + + pub abstract_methods: HashMap, + // These are now concrete method declarations, since we need to be able + // to handle them as such for purposes of TQMIC resolutions or default calls + pub methods: HashMap, // Maps a symbol name to the ancestor implementing it // does NOT include symbols defined in this trait @@ -67,23 +71,36 @@ pub enum TraitMemberKind<'t> { Method(&'t MethodDecl), } -impl <'t> TraitMemberKind<'t> { - pub fn location(&self) -> &ZeroSpan { +impl <'t> DMLNamedMember for TraitMemberKind<'t> { + fn location(&self) -> &'t ZeroSpan { match self { TraitMemberKind::Session(d) | TraitMemberKind::Saved(d) | TraitMemberKind::Parameter(d) => &d.name.span, - TraitMemberKind::Method(m) => &m.name.span, + TraitMemberKind::Method(m) => m.location(), } } - pub fn name(&self) -> &str { + + fn identity(&self) -> &'t str { match self { TraitMemberKind::Session(d) | TraitMemberKind::Saved(d) | TraitMemberKind::Parameter(d) => &d.name.val, - TraitMemberKind::Method(m) => &m.name.val, + TraitMemberKind::Method(m) => m.identity() + } + } + + fn kind_name(&self) -> &'static str { + match self { + TraitMemberKind::Session(_) => "session", + TraitMemberKind::Saved(_) => "saved", + TraitMemberKind::Parameter(_) => "parameter", + TraitMemberKind::Method(_) => "method", } } +} + +impl <'t> TraitMemberKind<'t> { pub fn as_method(&self) -> Option<&'t MethodDecl> { match self { TraitMemberKind::Method(m) => Some(m), @@ -134,6 +151,7 @@ pub fn get_impls(me: &Arc) -> HashMap> { .chain(me.saveds.keys()) .chain(me.params.keys()) .chain(me.methods.keys()) + .chain(me.abstract_methods.keys()) .map(|name|(name.to_string(), Arc::clone(me))) .chain(me.ancestor_map.iter().map(|(n, rc)|(n.clone(), Arc::clone(rc)))) .collect() @@ -148,6 +166,18 @@ pub enum ImplementRelation { } impl DMLTrait { + pub fn get_defined_method<'t, T>(&'t self, method_name: &T) + -> Option<&'t DMLConcreteMethod> + where T: Borrow + ?Sized { + self.methods.get(method_name.borrow()) + } + + pub fn get_method<'t, T>(&'t self, method_name: &T) + -> Option<&'t MethodDecl> + where T: Borrow + ?Sized { + self.get_member(method_name)?.as_method() + } + pub fn implement_relation(&self, other: &T) -> ImplementRelation where T: Borrow { @@ -168,13 +198,7 @@ impl DMLTrait { } } - pub fn get_method<'t, T>(&'t self, method_name: &T) - -> Option<&'t MethodDecl> - where T: Borrow + ?Sized { - self.get_member(method_name)?.as_method() - } - - // Lookup a definiton or declaration of a member in THIS trait, does not + // Lookup a definition or declaration of a member in THIS trait, does not // examine ancestors pub fn get_member<'t, T>(&'t self, member_name: &T) -> Option> @@ -194,6 +218,10 @@ impl DMLTrait { return Some(TraitMemberKind::Parameter(param)); } if let Some(method) = self.methods.get(member_name.borrow()) { + trace!("Got as {:?}", method); + return Some(TraitMemberKind::Method(&method.decl)); + } + if let Some(method) = self.abstract_methods.get(member_name.borrow()) { trace!("Got as {:?}", method); return Some(TraitMemberKind::Method(method)); } @@ -232,13 +260,12 @@ impl DMLTrait { traits: &HashMap>, report: &mut Vec) -> Arc { - debug!("Processing trait for {}", - traits.keys().fold( + debug!("Processing trait for {}", name); + trace!("other traits are {:?}", traits.keys().fold( "".to_string(), |mut s,k|{write!(s, "{}, ", k) .expect("write string error"); s})); - trace!("other traits are {:?}", name,); // Skip analysis for dummy trait if maybeloc.is_none() { trace!("Dummy trait, skipped"); @@ -251,6 +278,7 @@ impl DMLTrait { sessions: HashMap::default(), saveds: HashMap::default(), params: HashMap::default(), + abstract_methods: HashMap::default(), methods: HashMap::default(), ancestor_map: HashMap::default(), reserved_symbols: HashMap::default(), @@ -376,26 +404,71 @@ impl DMLTrait { trace!("params are: {:?}", params); // Map methods into non-colliding method declarations - let methods: HashMap = spec.methods.iter() + let abstract_methods: HashMap = spec.methods.iter() .filter_map(discard_conditional) - .filter(|method|method.modifier == - structure::objects::MethodModifier::Shared) + .filter(|method|method.is_abstract()) .filter_map(|m| { let new_m = MethodDecl::from_content(&m, report); if maybe_report_collision( &mut used_names, - new_m.name.val.clone(), - new_m.name.span, + new_m.identity().to_string(), + *new_m.location(), report) { Some(new_m) } else { None }}).map(|m|(m.name.val.clone(), m)).collect(); - trace!("methods are: {:?}", methods); - - // Check that our overrides are sound - for meth in methods.values() { - if let Some(ancestor) = impl_map.get(&meth.name.val) { + trace!("abstract methods are: {:?}", abstract_methods); + + let mut defined_methods: HashMap = HashMap::default(); + for method in spec.methods.iter() + .filter_map(discard_conditional) + .filter(|method|!method.is_abstract()) { + let new_m = MethodDecl::from_content(&method, report); + // special case, methods are allowed to conflict + // with abstract methods + // NOTE: Right now this means we'd report the collision + // with non-method members from abstract decl only + // when the definition also collides, which is probably fine + if abstract_methods.contains_key(new_m.name.val.as_str()) + || (maybe_report_collision( + &mut used_names, + new_m.identity().to_string(), + *new_m.location(), + report)) { + // Need one more check here, if we have multiple + // non-shared decls and one or more shared decls, + // we'd miss the extra non-shared ones otherwise + if defined_methods.contains_key(new_m.identity()) { + report.push(DMLError { + span: *new_m.location(), + description: + format!("Name collision on '{}'", + new_m.identity()), + severity: Some(DiagnosticSeverity::ERROR), + related: vec![ + (*defined_methods[new_m.identity()].location(), + "Previously defined here".to_string())], + }); + } else { + defined_methods.insert( + new_m.identity().to_string(), new_m.clone()); + } + } + } + + trace!("defined methods are: {:?}", defined_methods); + + let mut methods: HashMap = HashMap::default(); + + // Check that our overrides are sound and construct our concrete methods + // NOTE: Override order is already checked in merge_impl_maps, so we can use + // impl_map to get the next in order + for meth in defined_methods.values().chain(abstract_methods.values()) { + // TODO: Fairly sure this does not handle ambiguous overrides correctly. + // Odds are: they are reported correctly from merge_impl_maps, but + // we do not collect all of them here + let default_call = if let Some(ancestor) = impl_map.get(&meth.name.val) { if let Some(coll) = ancestor.get_member( meth.name.val.as_str()) { match coll { @@ -408,24 +481,30 @@ impl DMLTrait { override another method".into(), severity: Some(DiagnosticSeverity::ERROR), related: vec![ - (undermeth.name.span, + (*undermeth.location(), "Overridden definition here".into())], }); } - if !undermeth.default && !undermeth.is_abstract() { + if !undermeth.is_default() && !undermeth.is_abstract() { report.push(DMLError { span: meth.name.span, description: "This method attempts to override \ a non-default method".into(), related: vec![ - (undermeth.name.span, + (*undermeth.location(), "Attempted to override this \ declaration".into())], severity: Some(DiagnosticSeverity::ERROR), }); } meth.check_override(undermeth, report); + ancestor.get_defined_method(meth.identity()) + .map(|dmeth|DefaultCallReference::Valid( + Arc::new(DMLMethodRef { + template_ref: Some(Arc::clone(ancestor)), + concrete_decl: dmeth.clone(), + }))) }, TraitMemberKind::Saved(invalid) | TraitMemberKind::Session(invalid) | @@ -440,15 +519,29 @@ impl DMLTrait { (invalid.name.span, "Previously defined here".into())], }); + None } } } else { - error!("Invalid internal state: trait {} was indicated \ - as defining {} by {}, but didn't", - ancestor.name, - meth.name.val, - name); + internal_error!("trait {} was indicated \ + as defining {} by {}, but didn't", + ancestor.name, + meth.name.val, + name); + None } + } else { + None + }; + if !meth.is_abstract() { + methods.insert( + meth.name.val.clone(), + DMLConcreteMethod { + // TODO: This clone is slightly inefficient + decl: meth.clone(), + default_call, + } + ); } } @@ -490,6 +583,7 @@ impl DMLTrait { sessions, saveds, params, + abstract_methods, methods, ancestor_map: impl_map, reserved_symbols, @@ -536,7 +630,7 @@ where // Methods can override each other on trait level, // other declarations cannot match (decl, edecl) { - (TraitMemberKind::Method(_), TraitMemberKind::Method(_)) + (TraitMemberKind::Method(m1), TraitMemberKind::Method(m2)) => match relation { ImplementRelation::Implements => { map.insert(name, implr); @@ -549,14 +643,20 @@ where // analysis. // A minor improvement would be to pick both ImplementRelation::Unrelated => { - map.insert(name.clone(), Arc::clone(&mimplr)); - if let Some(ambdef) = - ambiguous_defs.get_mut(&name) { - ambdef.push(implr); - } - else { - ambiguous_defs.insert(name, - vec![mimplr, implr]); + // This conflict is allowed as long as exactly 1 + // is abstract + if m1.is_abstract() && !m2.is_abstract() { + map.insert(name, mimplr); + + } else if !m1.is_abstract() && m2.is_abstract() { + map.insert(name, implr); + } else { + // Both abstract or both non-abstract + // ambiguous + map.insert(name.clone(), Arc::clone(&implr)); + ambiguous_defs.entry(name) + .or_default() + .push(mimplr); } }, _ => unreachable!("should be covered by if clause"), @@ -593,7 +693,7 @@ where related: traits.iter().map( |t|{ let decl = t.get_method(ambiguous_name).unwrap(); - (decl.name.span, + (*decl.location(), format!("Conflicting definition here in {}", t.name)) }).collect(), diff --git a/src/file_management.rs b/src/file_management.rs index 672218d5..cd2de599 100644 --- a/src/file_management.rs +++ b/src/file_management.rs @@ -138,7 +138,7 @@ impl PathResolver { fn try_path(path: PathBuf) -> Option { if let Ok(info) = fs::metadata(&path) { if info.is_file() { - debug!("Resolved {:?} to {:?}", path, + trace!("Resolved {:?} to {:?}", path, // Fairly sure this can never fail fs::canonicalize(&path).unwrap()); return CanonPath::from_path_buf(path); diff --git a/src/server/mod.rs b/src/server/mod.rs index c4243d63..582a9efd 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -330,7 +330,7 @@ impl LsService { let output = client_reader_output; let send = client_reader_send; loop { - debug!("Awaiting message"); + trace!("Awaiting message"); let msg_string = match msg_reader.read_message() { Some(m) => m, None => { @@ -346,11 +346,11 @@ impl LsService { trace!("Read a message `{}`", msg_string); match RawMessageOrResponse::try_parse(&msg_string) { Ok(RawMessageOrResponse::Message(rm)) => { - debug!("Parsed a message: {}", rm.method); + trace!("Parsed a message: {}", rm.method); send.send(ServerToHandle::ClientMessage(rm)).ok(); }, Ok(RawMessageOrResponse::Response(rr)) => { - debug!("Parsed a response: {}", rr.id); + trace!("Parsed a response: {}", rr.id); send.send(ServerToHandle::ClientResponse(rr)).ok(); }, Err(e) => { @@ -394,7 +394,7 @@ impl LsService { ServerToHandle::ExitCode(code) => return code, ServerToHandle::IsolatedAnalysisDone(path, context, requests) => { - debug!("Received isolated analysis of {:?}", path); + trace!("Received isolated analysis of {:?}", path); if let ActionContext::Init(ctx) = &mut self.ctx { // hack where we try to activate a device context // as early as we possibly can, unless device context @@ -445,14 +445,14 @@ impl LsService { } }, ServerToHandle::DeviceAnalysisDone(path) => { - debug!("Received device analysis of {:?}", path); + trace!("Received device analysis of {:?}", path); if let ActionContext::Init(ctx) = &mut self.ctx { ctx.report_errors(&self.output); ctx.check_state_waits(); } }, ServerToHandle::LinterDone(path) => { - debug!("Received linter analysis of {:?}", path); + trace!("Received linter analysis of {:?}", path); if let ActionContext::Init(ctx) = &mut self.ctx { ctx.report_errors(&self.output); } @@ -460,7 +460,7 @@ impl LsService { ServerToHandle::AnalysisRequest(importpath, context, source) => { if let ActionContext::Init(ctx) = &mut self.ctx { if !ctx.config.lock().unwrap().to_owned().suppress_imports { - debug!("Analysing imported file {}", + trace!("Analysing imported file {}", &importpath.to_str().unwrap()); ctx.isolated_analyze( &importpath, Some(source), context, &self.output); @@ -479,14 +479,14 @@ impl LsService { blocking_requests: $($br_action: ty),*; requests: $($request: ty),*; ) => { - debug!("Handling `{}`", $method); + trace!("Handling `{}`", $method); match $method.as_str() { $( <$n_action as LSPNotification>::METHOD => { let notification: Notification<$n_action> = msg.parse_as_notification()?; if let Ok(mut ctx) = self.ctx.inited() { - debug!("Notified: {}", $method); + trace!("Notified: {}", $method); if notification.dispatch(&mut ctx, self.output.clone()).is_err() { debug!("Error handling notification: {:?}", msg); } @@ -531,7 +531,7 @@ impl LsService { <$request as LSPRequest>::METHOD => { let request: Request<$request> = msg.parse_as_request()?; if let Ok(ctx) = self.ctx.inited() { - debug!("Unblockingly Requested: {}", $method); + trace!("Unblockingly Requested: {}", $method); self.dispatcher.dispatch(request, ctx); } else { diff --git a/src/span/mod.rs b/src/span/mod.rs index 18a979f4..f2b4eefc 100644 --- a/src/span/mod.rs +++ b/src/span/mod.rs @@ -528,7 +528,7 @@ impl Span { impl Span { pub fn contains_pos(&self, pos: &FilePosition) -> bool { - self.range.contains_pos(pos.position) + self.file == pos.file && self.range.contains_pos(pos.position) } }