[SourceKit] Add request to expand macros syntactically #66295

rintaro · 2023-06-02T05:39:52Z

Expand macros in the specified source file syntactically (without any module imports, nor typechecking).

Request would look like:

{
  key.compilerargs: [...]
  key.sourcefile: <file name>
  key.sourcetext: <source text> (optional)
  key.expansions: [<expansion specifier>...]
}

key.compilerargs are used for getting plugins search paths. If key.sourcetext is not specified, it's loaded from the file system.
Each <expansion sepecifier> is

{
  key.offset: <offset>
  key.modulename: <plugin module name>
  key.typename: <macro typename>
  key.macro_roles: [<macro role UID>...]
}

Clients have to provide the module and type names because that's semantic.

Response is a CategorizedEdits just like (semantic) "ExpandMacro" refactoring. But without key.buffer_name. Nested expansions are not supported at this point.

rintaro · 2023-06-05T16:37:02Z

@swift-ci Please smoke test

rintaro · 2023-06-05T16:44:18Z

I will add nested macros and "expanded" macros test cases, either in this PR or follow ups

tools/SourceKit/tools/sourcekitd/lib/Service/Requests.cpp

utils/gyb_sourcekit_support/UIDs.py

bnbarham · 2023-06-05T17:27:19Z

tools/SourceKit/include/SourceKit/Core/LangSupport.h

@@ -220,6 +220,60 @@ struct RawCharSourceRange {
  unsigned Length;
 };

+enum class MacroRole : uint8_t {


Unfortunate we need another of these 😅

bnbarham · 2023-06-05T18:08:42Z

lib/IDETool/SyntacticMacroExpansion.cpp

+  for (auto attr : macro->getAttrs().getAttributes<MacroRoleAttr>()) {
+    roles -= attr->getMacroRole();
+  }


When would we want to add roles after we've already synthesized the macro?

When the client didn't send the all roles in previous requests.
I don't think it happens practically, but different modules may declare the macro decl differently for the same macro.

To be clear, this could still result in weird behavior:

First the client sends the macro with an attached member role -> we expand it as a member

Now the client sends the macro with only an attached conformance role -> we expand it as both a member and a conformance macro

So, there is state being carried between the requests. I think we should either

Set the macro roles to only those that are declared in the current request or

Not re-use a MacroDecl if it has different roles than in a previous invocation.

Since I removed caching, we don't carry any state between requests.
The only possible case this hits is that the client sends inconsistent roles within a single request. Which... is not great, but I think it's not worth to handle it.

Ah, I see. I thought that MacroDecls was cached more persistently.

bnbarham · 2023-06-05T18:11:38Z

lib/IDETool/SyntacticMacroExpansion.cpp

+
+  StableHasher hasher = StableHasher::defaultHasher();
+
+  // Macro name.


Should this be the typename in the underlying module instead?

module/typename of the macro still doesn't make the expansion unique

@MyMacro("foo") @MyMacro("bar") func fn()

These two MyMacro expansions should have unique discriminators. So I decided to combine 2 locations for attached macros (target decl location and the attribute location), instead of the macro name or macro implementation name.

lib/IDETool/SyntacticMacroExpansion.cpp

bnbarham · 2023-06-05T18:35:35Z

lib/IDETool/SyntacticMacroExpansion.cpp

+  // Note that this finds the generated source file that was created in the
+  // previous expansion requests.


IIUC this will only work up until another request is received that has different arguments. Maybe that's okay, but it does mean that all requests have to be sequential for now, ie. you couldn't request for expansions in different files concurrently.

Different files still usually share the same compiler arguments as long as they are in the same module.
But yes, if compiler arguments are different, concurrent requests for nested expansion may not work.

Do we care about any arguments other than the plugin search ones? Could we just filter to that?

We currently use -module-name too as far as I remember. I don't think it's a good idea to "sanitize" compiler arguments here. Affecting options might change overtime, but I'm skeptical about anyone changing the compiler options or CompilerInvocation correctly updates this request.

This is less sanitizing and more picking the ones we care about.

anyone changing the compiler options or CompilerInvocation correctly updates this request

Unless it impacts it, they shouldn't need to.

They don't know if their change affects this. Like, a new plugin search option might be added in the future.

I removed the caching logic from this PR for now.

lib/IDETool/SyntacticMacroExpansion.cpp

rintaro · 2023-06-05T23:42:35Z

@swift-ci Please smoke test

rintaro · 2023-06-07T15:54:02Z

@swift-ci Please smoke test

bnbarham · 2023-06-07T18:47:31Z

lib/IDETool/SyntacticMacroExpansion.cpp

+  bool hasError = false;
+  for (const auto &expansion : expansions) {
+    hasError |= getExpansion(SF, expansion, consumer);
+  }
+  return hasError;


We don't do anything with this error at the moment, so I'm not sure it's worth having. If we wanted to keep it I'd prefer it was an llvm::Error, though right now the only error is if we fail to find the expansion.

Also, could both getExpansions and getExpansion be something like expandAll and expand?

Removed the return value. Renamed the methods

bnbarham · 2023-06-07T18:49:56Z

lib/IDETool/SyntacticMacroExpansion.cpp

+  std::vector<unsigned> bufferIDs;
+
+  SmallString<32> discriminator;
+  discriminator.append("macro_");


Maybe a little more unique for these? Eg. __syntactic_macro_?

bnbarham · 2023-06-07T18:52:53Z

utils/gyb_sourcekit_support/UIDs.py

@@ -272,6 +275,8 @@ def __init__(self, internal_name, external_name):
    REQUEST('Diagnostics', 'source.request.diagnostics'),
    REQUEST('Compile', 'source.request.compile'),
    REQUEST('CompileClose', 'source.request.compile.close'),
+    REQUEST('ExpandMacroSyntactically',
+            'source.request.expand.macro.syntactically'),


We are... quite inconsistent with these. How about source.request.syntactic_macro_expansion?

test/SourceKit/Macros/syntactic_expansion.swift

tools/SourceKit/include/SourceKit/Core/LangSupport.h

ahoppen · 2023-06-07T17:26:52Z

tools/SourceKit/include/SourceKit/Core/LangSupport.h

+      RawCharSourceRange range;
+      unsigned parameterIndex;


I suppose that a Replacement works as follows.

range references some part of code in the expanded macro definition (which is the part after the =). The source code of range will be replaced by the parameterIndex-th argument of the macro.

If that’s correct, could you add documentation that describes it?

Also, does this work if the macro defines variadic parameters or parameter packs? Because in those cases a single parameter might have multiple arguments.

Your understanding is correct. I will add some comment about it.
As for variadics, I am not sure tbh. This just follows the compiler's implementation.

Also, since "expanded" is not fully supported for attached macro, I don't think we can fully support it.
That's why I didn't mention it in the description.

Looks like there is some issue with variadic macros defined in terms of other macros in general. Filed rdar://110419352

tools/SourceKit/tools/sourcekitd-test/sourcekitd-test.cpp

include/swift/IDETool/SyntacticMacroExpansion.h

lib/IDETool/SyntacticMacroExpansion.cpp

rintaro · 2023-06-07T20:26:14Z

@swift-ci Please smoke test

rintaro · 2023-06-07T20:41:07Z

@swift-ci Please smoke test

Expand macros in the specified source file syntactically (without any module imports, nor typechecking). Request would look like: ``` { key.compilerargs: [...] key.sourcefile: <file name> key.sourcetext: <source text> (optional) key.expansions: [<expansion specifier>...] } ``` `key.compilerargs` are used for getting plugins search paths. If `key.sourcetext` is not specified, it's loaded from the file system. Each `<expansion sepecifier>` is ``` { key.offset: <offset> key.modulename: <plugin module name> key.typename: <macro typename> key.macro_roles: [<macro role UID>...] } ``` Clients have to provide the module and type names because that's semantic. Response is a `CategorizedEdits` just like (semantic) "ExpandMacro" refactoring. But without `key.buffer_name`. Nested expnasions are not supported at this point.

rintaro · 2023-06-07T21:27:08Z

@swift-ci Please smoke test

rintaro force-pushed the sourcekit-syntactic-macroexpansion branch 4 times, most recently from ee434b3 to 8294db0 Compare June 5, 2023 16:35

rintaro marked this pull request as ready for review June 5, 2023 16:37

rintaro requested review from ahoppen, bnbarham, hborla, slavapestov and xedin as code owners June 5, 2023 16:37

rintaro removed request for xedin, slavapestov and hborla June 5, 2023 16:37

bnbarham reviewed Jun 5, 2023

View reviewed changes

rintaro mentioned this pull request Jun 6, 2023

[5.9][SourceKit] Add request to expand macros syntactically #66383

Closed

rintaro force-pushed the sourcekit-syntactic-macroexpansion branch 2 times, most recently from 49c08e9 to 6f315f8 Compare June 7, 2023 15:52

bnbarham reviewed Jun 7, 2023

View reviewed changes

ahoppen reviewed Jun 7, 2023

View reviewed changes

rintaro force-pushed the sourcekit-syntactic-macroexpansion branch from 6f315f8 to f6dd5d1 Compare June 7, 2023 20:21

rintaro force-pushed the sourcekit-syntactic-macroexpansion branch from f6dd5d1 to de41f45 Compare June 7, 2023 20:38

ahoppen approved these changes Jun 7, 2023

View reviewed changes

bnbarham approved these changes Jun 7, 2023

View reviewed changes

rintaro force-pushed the sourcekit-syntactic-macroexpansion branch from de41f45 to c3d1304 Compare June 7, 2023 21:26

bnbarham mentioned this pull request Jun 7, 2023

[5.9] Macro refactorings and fixes #66426

Merged

rintaro merged commit 46e98c1 into swiftlang:main Jun 8, 2023

rintaro deleted the sourcekit-syntactic-macroexpansion branch June 8, 2023 05:44

fwcd mentioned this pull request Oct 11, 2023

Add LSP extension request for retrieving macro expansions swiftlang/sourcekit-lsp#892

Closed

6 tasks

lokesh-tr mentioned this pull request Jun 5, 2024

Add LSP support for showing Macro Expansions swiftlang/sourcekit-lsp#1436

Merged


		StableHasher hasher = StableHasher::defaultHasher();

		// Macro name.

		// Note that this finds the generated source file that was created in the
		// previous expansion requests.

[SourceKit] Add request to expand macros syntactically #66295

[SourceKit] Add request to expand macros syntactically #66295

Uh oh!

Conversation

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

Uh oh!

Uh oh!

rintaro commented Jun 5, 2023

Uh oh!

rintaro commented Jun 5, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

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!

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!

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!

Uh oh!

rintaro commented Jun 5, 2023

Uh oh!

rintaro commented Jun 7, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rintaro commented Jun 7, 2023

Uh oh!

rintaro commented Jun 2, 2023 •

edited

Loading