[CodeCompletion] Suggest synthesized declarations from macros #66821

rintaro · 2023-06-21T18:57:55Z

Don't invalidate the lookup cache in getOrCreateSynthesizedFile() Adding a synthesized file itself doesn't introduce any decls. Instead, we should invalidate it right after the actual declrations are added in the file
Remove SourceLookupCache::invalidate() method. It was just used right before the destruction. It was just not necessary
Include auxiliary decls in SourceLookupCache::lookupVisibleDecls() Previously, global symbol completion didn't include decls synthesized by peer macros or freestanding decl macros
Include "auxiliary decls" in visible member lookup, and visible local decl lookup
Hide macro unique names

rdar://110535113

rintaro · 2023-06-21T19:00:29Z

@swift-ci Please smoke test

rintaro · 2023-06-21T18:59:07Z

lib/Sema/LookupVisibleDecls.cpp

-  for (auto *member : NTD->getMembers()) {
-    if (!ctx.evaluator.hasActiveRequest(ExpandPeerMacroRequest{member})) {
-      (void)evaluateOrDefault(
-          ctx.evaluator,
-          ExpandPeerMacroRequest{member},
-          {});


This is not necessary because visitAuxiliaryDecls() calls it lazily.

What calls visitAuxiliaryDecls for members in regular type checking?

I think it's populateLookupTableEntryFromMacroExpansions called from DirectLookupRequest::evaluate()

rintaro · 2023-06-21T21:23:07Z

@swift-ci Please smoke test

rintaro · 2023-06-21T22:56:51Z

@swift-ci Please smoke test

bnbarham · 2023-06-22T00:09:20Z

lib/AST/Decl.cpp

@@ -3827,6 +3827,10 @@ bool ValueDecl::shouldHideFromEditor() const {
      getBaseIdentifier().str().startswith("$__"))
    return true;

+  // Macro unique names are only intended to be used inside the expanded code.
+  if (MacroDecl::isUniqueMacroName(getBaseName()))


bnbarham · 2023-06-22T00:15:26Z

lib/Sema/LookupVisibleDecls.cpp

-  for (auto *member : NTD->getMembers()) {
-    if (!ctx.evaluator.hasActiveRequest(ExpandPeerMacroRequest{member})) {
-      (void)evaluateOrDefault(
-          ctx.evaluator,
-          ExpandPeerMacroRequest{member},
-          {});


What calls visitAuxiliaryDecls for members in regular type checking?

bnbarham · 2023-06-22T00:24:05Z

test/IDE/complete_macros_expanded.swift

+// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/plugins/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/../Macros/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath
+
+// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t -plugin-path %t/plugins -parse-as-library 
+


Since arbitrary goes down fairly different paths to named, possibly worth adding tests for it as well?

Apparently arbitrary at top-level is not really supported:
If we add names: arbitrary

error: 'declaration' macros are not allowed to introduce arbitrary names at global scope

error: 'peer' macros are not allowed to introduce arbitrary names at global scope

And the macro decl becomes isInvalid(), and the expansion is not performed.

If we omit names from the macro decl:

error: declaration name '<name>' is not covered by macro '<macro name>'

Introduced values are found and type checked, but since it's error, it doesn't built.

I feel like we should remove

unexpandedDecls.append(TopLevelArbitraryMacros.begin(), TopLevelArbitraryMacros.end());

From lookupVisibleDecls()

I was thinking within a struct

For non top level cases, there is not much different code paths between arbitrary and named, but sure.

ahoppen

Thank you 🙏🏽

lib/AST/NameLookup.cpp

ahoppen · 2023-06-22T10:08:51Z

lib/AST/NameLookup.cpp

+    if (auto *E = elem.dyn_cast<Expr *>()) {
+      // 'MacroExpansionExpr' at code-item position may introduce value decls.
+      // NOTE: the expression must be type checked.
+      // FIXME: In code-completion local expressions are _not_ type checked.


How do we get completions from macro expansion declarations if the corresponding MacroExpansionExpr hasn’t been type-checked?

We can't. So we need to typecheck MacroExpansionExpr before here, or here.

lib/SILOptimizer/Differentiation/LinearMapInfo.cpp

ahoppen · 2023-06-22T10:11:16Z

test/IDE/complete_macros_expanded.swift

+  @PeerWithSuffix func localFunc() {}
+
+  do {
+    #^LOCAL?skip=FIXME^#


Change this to an xfail so we don’t forget to change it if it’s fixed?

Suggested change

#^LOCAL?skip=FIXME^#

#^LOCAL?xfail=FIXME^#

I deliberately make this skip for now. There's two FIXMEs here.

MacroExpansionExpr is not type-checked.

Macro expansion in function bodies in fast-completion.

If we fix (1) but not (2) this test goes nondeterministic because it succeeds only when LOCAL token is tested first.

Ah, OK. Makes sense.

* Don't invalidate the lookup cache in 'getOrCreateSynthesizedFile()' Adding a synthesized file itself doesn't introduce any decls. Instead, we should invalidate the right after the actual declrations are added in the file * Remove 'SourceLookupCache::invalidate()' method. It was just used right before the destruction. It was just not necessary * Include auxiliary decls in 'SourceLookupCache::lookupVisibleDecls()' Previously, global symbol completion didn't include decls synthesized by peer macros or freestanding decl macros * Include "auxiliary decls" in visible member lookup, and visible local decl lookup * Hide macro unique names rdar://110535113

rintaro · 2023-06-22T20:21:54Z

@swift-ci Please smoke test

rintaro requested review from ahoppen, bnbarham, hborla, slavapestov and xedin as code owners June 21, 2023 18:57

rintaro commented Jun 21, 2023

View reviewed changes

rintaro force-pushed the completion-macro-expanded-rdar110535113 branch from 824439f to f88f181 Compare June 21, 2023 21:17

rintaro force-pushed the completion-macro-expanded-rdar110535113 branch from f88f181 to 34c0f61 Compare June 21, 2023 22:56

bnbarham reviewed Jun 22, 2023

View reviewed changes

ahoppen approved these changes Jun 22, 2023

View reviewed changes

rintaro force-pushed the completion-macro-expanded-rdar110535113 branch from 34c0f61 to 4699899 Compare June 22, 2023 19:14

rintaro force-pushed the completion-macro-expanded-rdar110535113 branch from 4699899 to 096d8ea Compare June 22, 2023 20:18

xedin removed their request for review June 23, 2023 00:00

rintaro merged commit 196ea79 into swiftlang:main Jun 23, 2023

rintaro mentioned this pull request Jun 23, 2023

[5.9][CodeCompletion] Suggest synthesized declarations from macros #66888

Merged

		// RUN: %host-build-swift -swift-version 5 -emit-library -o %t/plugins/%target-library-name(MacroDefinition) -module-name=MacroDefinition %S/../Macros/Inputs/syntax_macro_definitions.swift -g -no-toolchain-stdlib-rpath

		// RUN: %target-swift-ide-test -batch-code-completion -source-filename %s -filecheck %raw-FileCheck -completion-output-dir %t -plugin-path %t/plugins -parse-as-library

[CodeCompletion] Suggest synthesized declarations from macros #66821

[CodeCompletion] Suggest synthesized declarations from macros #66821

Uh oh!

Conversation

rintaro commented Jun 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rintaro commented Jun 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro commented Jun 21, 2023

Uh oh!

rintaro commented Jun 21, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro Jun 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro commented Jun 22, 2023

Uh oh!

Uh oh!

rintaro commented Jun 21, 2023 •

edited

Loading

rintaro Jun 22, 2023 •

edited

Loading

rintaro Jun 22, 2023 •

edited

Loading