Improvements to symbol resolutions

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.

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

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);
             }
         }
     }

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<Option<ContextedSymbol<'t>>, 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<Option<&Reference>, 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<Option<ContextKey>, 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

src/actions/hover.rs

@@ -24,6 +24,6 @@ pub fn tooltip<O: Output>(
     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 })
 }

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<O: Output> {
@@ -1183,10 +1184,10 @@ impl <O: Output> InitActionContext<O> {
                 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 <O: Output> InitActionContext<O> {
         }
 
     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 <O: Output> InitActionContext<O> {
         }
 
         if wait_done {
-            debug!("{:?} was done", wait);
+            trace!("{:?} was done", wait);
         } else {
-            debug!("{:?} not done", wait);
+            trace!("{:?} not done", wait);
         }
         wait_done
     }