Skip to content

[CodeComplete] Offer completion suggestions for default argument values inside the macro declaration + argument label inside #externalMacro #65526

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

ahoppen
Copy link
Member

@ahoppen ahoppen commented Apr 29, 2023

We were not type checking the default argument initializer because MacroDecl is not an AbstractFucntionDecl. Add MacroDecl to the list of nodes we know how to get parameters from.

Pretty much unrelated, but also a small change in the same test file: Continue checking a #externalMacro declaration in code completion mode even if the argument labels don’t match. This way we get completion results for #externalMacro argument labels and when competing after # when externalMacro is already specified.

rdar://108163564
rdar://108163809
rdar://108681394

@ahoppen ahoppen requested a review from rintaro April 29, 2023 01:25
@ahoppen ahoppen changed the title [CodeComplete] Offer completion suggestions for default argument values inside the macro declaration [CodeComplete] Offer completion suggestions for default argument values inside the macro declaration + argument label inside #externalMacro Apr 29, 2023
@ahoppen
Copy link
Member Author

ahoppen commented Apr 29, 2023

@swift-ci Please smoke test

@ahoppen ahoppen force-pushed the ahoppen/complete-in-macro-default-arg branch from f17e5b9 to 057b42f Compare April 29, 2023 15:02
} else if (auto *MD = dyn_cast<MacroDecl>(defaultArg->getParent())) {
Param = MD->getParameterList()->get(defaultArg->getIndex());
} else if (auto *SD = dyn_cast<SubscriptDecl>(defaultArg->getParent())) {
Param = SD->getIndices()->get(defaultArg->getIndex());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this use getParameterAt instead? swift::getParameterList already handles MacroDecl too.

…es inside the macro declaration

We were not type checking the default argument initializer because `MacroDecl` is not an `AbstractFucntionDecl`. Add `MacroDecl` to the list of nodes we know how to get parameters from.

rdar://108163564
@ahoppen ahoppen force-pushed the ahoppen/complete-in-macro-default-arg branch from 057b42f to ce957a7 Compare May 1, 2023 20:48
@ahoppen
Copy link
Member Author

ahoppen commented May 1, 2023

@swift-ci Please smoke test

@@ -3959,7 +3959,8 @@ TypeConverter::getLoweredLocalCaptures(SILDeclRef fn) {
fn.getAsRegularLocation(), Context);

if (auto *afd = dyn_cast<AbstractFunctionDecl>(curFn.getDecl())) {
auto *param = getParameterAt(afd, curFn.defaultArgIndex);
auto *param = getParameterAt(static_cast<ValueDecl *>(afd),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why's the static_cast needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because otherwise it’s ambiguous whether this should call getParameterAt(DeclContext *, …) or getParameterAt(ValueDecl *, …)

// DEFAULT_ARG: Decl[GlobalVar]/CurrModule/TypeRelation[Convertible]: globalVar[#Int#]; name=globalVar

@freestanding(expression)
macro externalMacro() = ##^EXTERNAL_MACRO^#externalMacro
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the test just below this one, I assume this isn't meant to have the trailing externalMacro?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you’re right


@freestanding(expression)
macro externalMacroCallPattern() = #externalMacro(#^EXTERNAL_MACRO_CALL_PATTERN^#)
// EXTERNAL_MACRO_CALL_PATTERN: Pattern/None/Flair[ArgLabels]/TypeRelation[Convertible]: ['(']{#module: String#}, {#type: String#}[')'][#Void#]; name=module:type:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's module:type: there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s how the call pattern shows up in code completion, I think. It’s the same that shows up for normal function calls

$ cat /tmp/a.swift
func foo(module: String, type: String) {}

foo(#^COMPLETE^#)
$ swift-ide-complete /tmp/a.swift
+ /Users/alex/sbin/swift-ide-test -code-completion -source-filename /private/tmp/a.swift -code-completion-token COMPLETE
found code completion token COMPLETE at offset 47
Begin completions, 1 items
Decl[FreeFunction]/CurrModule/Flair[ArgLabels]: ['(']{#module: String#}, {#type: String#}[')'][#Void#]; name=module:type:
End completions

@ahoppen ahoppen force-pushed the ahoppen/complete-in-macro-default-arg branch from ce957a7 to 7e4b88b Compare May 1, 2023 23:10
@ahoppen
Copy link
Member Author

ahoppen commented May 1, 2023

@swift-ci Please smoke test

@ahoppen
Copy link
Member Author

ahoppen commented May 2, 2023

@swift-ci Please smoke test macOS

1 similar comment
@ahoppen
Copy link
Member Author

ahoppen commented May 2, 2023

@swift-ci Please smoke test macOS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants