TY&RES: implement hygiene & infer types in macros #4094

vlad20012 · 2019-07-04T15:39:15Z

Now we take into account any custom macros in type inference

vlad20012 · 2019-07-12T23:12:24Z

Fixed tests

Undin · 2019-08-02T08:02:32Z

src/main/kotlin/org/rust/lang/core/types/infer/TypeInferenceWalker.kt

+        val tailExpr = expandedStmts.lastOrNull()
+            ?.let { it as? RsExpr }
+            ?.takeIf { e ->
+                e.expandedFromSequence.all {


Comment why we need this.
As we discussed in person it's very unclear place

Undin · 2019-08-15T11:18:59Z

src/test/kotlin/org/rust/RsTestBase.kt

+    protected inline fun <reified T : PsiElement> findElementInEditor(marker: String = "^"): T =
+        findElementInEditor(T::class.java, marker)
+
+    protected fun <T : PsiElement> findElementInEditor(psiClass: Class<T>, marker: String): T {


Do we really need these changes?
I remember your concerns about performance when too large code is inlined but it's still test code

It makes debugging & stack trace analyzing more complicated. As I remember, I made these changes exactly when I too tired to debug findElementsWithDataAndOffsetInEditor function (you can see some changes there) when it is inlined. I think that we really need these changes, especially in tests, because it's a something that usually fails and we are often forced to analyze stack traces from there or debug it, so it should be as simple as possible.

Undin · 2019-08-15T11:25:39Z

src/main/kotlin/org/rust/lang/core/resolve/NameResolution.kt

@@ -1176,14 +1180,15 @@ private fun processLexicalDeclarations(
    scope: RsElement,
    cameFrom: PsiElement,
    ns: Set<Namespace>,
+    hygieneFilter: (RsPatBinding) -> Boolean,


Let's add a comment/javadoc why and when we need this filter

Undin · 2019-08-15T11:30:34Z

src/main/kotlin/org/rust/lang/core/resolve/NameResolution.kt

@@ -1343,6 +1359,33 @@ fun processNestedScopesUpwards(
    return false
 }

+private fun makeHygieneFilter(anchor: PsiElement): (RsPatBinding) -> Boolean {


I think the main explanation what we want to archive by hygiene filter should be located in javadoc here

Undin · 2019-08-15T11:33:20Z

src/main/kotlin/org/rust/lang/core/resolve/NameResolution.kt

+        val nameIdentifier = element.nameIdentifier ?: return false
+        val hygienicScope2 = (nameIdentifier.findElementExpandedFrom() ?: nameIdentifier).containingFile
+            .unwrapCodeFragments()
+        return hygienicScope == hygienicScope2


Is it possible to come up with more self-explained names for these variables?

Undin · 2019-08-15T12:07:34Z

src/test/kotlin/org/rust/RsTestBase.kt

@@ -256,14 +275,25 @@ abstract class RsTestBase : LightPlatformCodeInsightFixtureTestCase(), RsTestCas
            val previousLine = LogicalPosition(markerPosition.line - 1, markerPosition.column)
            val elementOffset = myFixture.editor.logicalPositionToOffset(previousLine)
            val elementAtMarker = myFixture.file.findElementAt(elementOffset)!!
-            val element = elementAtMarker.ancestorOrSelf<T>()
+
+            if (followMacroExpansions) {


Do we really need this property?
Can we always follow macro expansions if it's possible?

We can't follow expansions at least in RsMacroCallReferenceDelegationTest. We can either turn off followMacroExpansions for this test (and turn on by default) or turn it on where it is needed. For now, I decided to turn it on explicitly

src/test/kotlin/org/rust/lang/core/resolve/RsResolveTestBase.kt

Undin · 2019-08-15T12:23:59Z

src/main/kotlin/org/rust/lang/core/macros/RsExpandedElement.kt

 * macro call, i.e. outside of any expansion.
 */
+fun PsiElement.findElementExpandedFromChecked(): PsiElement? {


Hm, change looks quite suspicious
Previous findElementExpandedFrom became findElementExpandedFromChecked (if take into account javadoc) but not all previous usages of findElementExpandedFrom replaced by findElementExpandedFromChecked

Also, I think the previous name was better
Let's keep old name and come up with a better name for new function

Undin · 2019-08-15T12:25:19Z

src/main/kotlin/org/rust/lang/core/macros/RsExpandedElement.kt

+}
+
+/**
+ * If receiver element is inside a macro expansion, returns the leaf element inside the macro call


What this function should return if element is not inside macro expansion?

Undin · 2019-08-15T13:53:55Z

src/test/kotlin/org/rust/lang/core/type/RsTypificationTestBase.kt

@@ -84,3 +89,7 @@ abstract class RsTypificationTestBase : RsTestBase() {
        }
    }
 }
+
+private val BUILTIN_MACRO_NAMES = listOf(
+    "env", "option_env", "concat", "line", "column", "file", "stringify", "module_path", "cfg"


Do I understand correctly that this list consists of builtin macros except some ones that have special rules in our grammar?

looks like compile_error is not in this list
Also, check concat - we have special grammar rule for it now

It's exactly this list: https://github.com/intellij-rust/intellij-rust/blob/master/src/main/kotlin/org/rust/lang/core/types/infer/TypeInferenceWalker.kt#L1077

We need it because such built-in macros actually resolve to dummy definitions that expand to empty block expr ({ }) that we don't take into account during type inference.

vlad20012 · 2019-08-19T19:12:36Z

bors r=undin

4094: TY&RES: implement hygiene & infer types in macros r=undin a=vlad20012 Now we take into account any custom macros in type inference Co-authored-by: vlad20012 <beskvlad@gmail.com>

bors · 2019-08-19T19:34:26Z

Build failed

continuous-integration/appveyor/branch

vlad20012 · 2019-08-19T19:35:06Z

bors retry

3996: INT & ANN: Implement intention and fix for struct fields expansion instead of `..` r=vlad20012 a=shevtsiv  In this pull request I intend to do the following: 1) Create E0023 and E0027 error classes and implement analysis for them. 2) Create fix for those errors. 3) Create intention for struct fields expansion. 4) Create a module with shared code for using it in the fix and intention. Once these steps are complete I will squash all the commits and #3928 should be closed after merge. 4094: TY&RES: implement hygiene & infer types in macros r=undin a=vlad20012 Now we take into account any custom macros in type inference Co-authored-by: shevtsiv <rostykshevtsiv@gmail.com> Co-authored-by: vlad20012 <beskvlad@gmail.com>

bors · 2019-08-19T20:46:18Z

Build failed (retrying...)

continuous-integration/appveyor/branch

4094: TY&RES: implement hygiene & infer types in macros r=undin a=vlad20012 Now we take into account any custom macros in type inference Co-authored-by: vlad20012 <beskvlad@gmail.com>

bors · 2019-08-19T22:28:56Z

Build succeeded

4330: TY: fix random type inference fails caused by STUB usage instead of AST r=vlad20012 a=vlad20012 The bug was introduced in #4094 (because we used a stubbed statement list) and led to the fact that sometimes all types in some file was not inferred until you type some text there Co-authored-by: vlad20012 <beskvlad@gmail.com>

vlad20012 force-pushed the hygiene branch 3 times, most recently from c8d6d47 to c01cd05 Compare July 5, 2019 00:11

mchernyavsky added this to the v102 milestone Jul 8, 2019

vlad20012 force-pushed the hygiene branch 3 times, most recently from dba24a9 to 15f945b Compare July 12, 2019 15:28

vlad20012 assigned Undin Jul 17, 2019

mchernyavsky removed this from the v102 milestone Jul 20, 2019

Undin added the feature label Jul 21, 2019

vlad20012 force-pushed the hygiene branch 3 times, most recently from 9ff5e3c to 794179e Compare July 29, 2019 16:02

Undin reviewed Aug 15, 2019

View reviewed changes

vlad20012 force-pushed the hygiene branch 3 times, most recently from 2780687 to 88bf672 Compare August 19, 2019 16:14

TY&RES: implement hygiene & infer types in macros

3246664

vlad20012 force-pushed the hygiene branch from 88bf672 to 3246664 Compare August 19, 2019 16:28

bors bot added a commit that referenced this pull request Aug 19, 2019

Merge #4094

7a86084

4094: TY&RES: implement hygiene & infer types in macros r=undin a=vlad20012 Now we take into account any custom macros in type inference Co-authored-by: vlad20012 <beskvlad@gmail.com>

bors bot added a commit that referenced this pull request Aug 19, 2019

Merge #4094

b657706

4094: TY&RES: implement hygiene & infer types in macros r=undin a=vlad20012 Now we take into account any custom macros in type inference Co-authored-by: vlad20012 <beskvlad@gmail.com>

bors bot merged commit 3246664 into intellij-rust:master Aug 19, 2019

Undin added this to the v104 milestone Aug 24, 2019

vlad20012 mentioned this pull request Aug 30, 2019

TY: fix random type inference fails caused by STUB usage instead of AST #4330

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TY&RES: implement hygiene & infer types in macros #4094

TY&RES: implement hygiene & infer types in macros #4094

vlad20012 commented Jul 4, 2019

vlad20012 commented Jul 12, 2019

Undin Aug 2, 2019

Undin Aug 15, 2019

vlad20012 Aug 15, 2019 •

edited

Undin Aug 15, 2019

Undin Aug 15, 2019

Undin Aug 15, 2019

Undin Aug 15, 2019

vlad20012 Aug 15, 2019 •

edited

Undin Aug 15, 2019

Undin Aug 15, 2019

Undin Aug 15, 2019

Undin Aug 15, 2019

Undin Aug 15, 2019

vlad20012 Aug 19, 2019

vlad20012 commented Aug 19, 2019

bors bot commented Aug 19, 2019

vlad20012 commented Aug 19, 2019

bors bot commented Aug 19, 2019

bors bot commented Aug 19, 2019

TY&RES: implement hygiene & infer types in macros #4094

TY&RES: implement hygiene & infer types in macros #4094

Conversation

vlad20012 commented Jul 4, 2019

vlad20012 commented Jul 12, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlad20012 Aug 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlad20012 Aug 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vlad20012 commented Aug 19, 2019

bors bot commented Aug 19, 2019

Build failed

vlad20012 commented Aug 19, 2019

bors bot commented Aug 19, 2019

Build failed (retrying...)

bors bot commented Aug 19, 2019

Build succeeded

vlad20012 Aug 15, 2019 •

edited

vlad20012 Aug 15, 2019 •

edited