Merge pull request #4535 from criemen/jump-to-def

C++: Extend jump-to-def support to template instantiations.

Author: jbj

cpp/ql/src/definitions.ql

@@ -1,13 +1,18 @@
 /**
  * @name Jump-to-definition links
  * @description Generates use-definition pairs that provide the data
- *              for jump-to-definition in the code viewer.
+ *              for jump-to-definition in the code viewer of LGTM.
  * @kind definitions
  * @id cpp/jump-to-definition
  */
 
 import definitions
 
 from Top e, Top def, string kind
-where def = definitionOf(e, kind)
+where
+  def = definitionOf(e, kind) and
+  // We need to exclude definitions for elements inside template instantiations,
+  // as these often lead to multiple links to definitions from the same source location.
+  // LGTM does not support this bevaviour.
+  not e.isFromTemplateInstantiation(_)
 select e, def, kind

cpp/ql/src/definitions.qll

@@ -124,6 +124,7 @@ private predicate constructorCallTypeMention(ConstructorCall cc, TypeMention tm)
 
 /**
  * Gets an element, of kind `kind`, that element `e` uses, if any.
+ * Attention: This predicate yields multiple definitions for a single location.
  *
  * The `kind` is a string representing what kind of use it is:
  *  - `"M"` for function and method calls
@@ -196,15 +197,7 @@ Top definitionOf(Top e, string kind) {
     not e.(Element).isInMacroExpansion() and
     // exclude nested macro invocations, as they will overlap with
     // the top macro invocation.
-    not exists(e.(MacroAccess).getParentInvocation()) and
-    // exclude results from template instantiations, as:
-    // (1) these dependencies will often be caused by a choice of
-    // template parameter, which is non-local to this part of code; and
-    // (2) overlapping results pointing to different locations will
-    // be very common.
-    // It's possible we could allow a subset of these dependencies
-    // in future, if we're careful to ensure the above don't apply.
-    not e.isFromTemplateInstantiation(_)
+    not exists(e.(MacroAccess).getParentInvocation())
   ) and
   // Some entities have many locations. This can arise for an external
   // function that is frequently declared but not defined, or perhaps

cpp/ql/src/localDefinitions.ql

@@ -1,7 +1,7 @@
 /**
  * @name Jump-to-definition links
  * @description Generates use-definition pairs that provide the data
- *              for jump-to-definition in the code viewer.
+ *              for jump-to-definition in the code viewer of VSCode.
  * @kind definitions
  * @id cpp/ide-jump-to-definition
  * @tags ide-contextual-queries/local-definitions

cpp/ql/src/localReferences.ql

@@ -1,7 +1,7 @@
 /**
  * @name Find-references links
  * @description Generates use-definition pairs that provide the data
- *              for find-references in the code viewer.
+ *              for find-references in the code viewer of VSCode.
  * @kind definitions
  * @id cpp/ide-find-references
  * @tags ide-contextual-queries/local-references

cpp/ql/test/query-tests/definitions/locationInfo.ql

@@ -40,7 +40,9 @@ string issues(Link e) {
     filepath1 = filepath2 and
     not end1 < begin2 and
     not begin1 > end2 and
-    e != e2
+    e != e2 and
+    not e.isFromTemplateInstantiation(_) and
+    not e2.isFromTemplateInstantiation(_)
   ) and
   result = "overlaps another link"
 }