[Debug] Add _DebugDescription macro #69626

kastiglione · 2023-11-02T22:50:10Z

Implementation of the DebugDescription macro pitched on the forums: https://forums.swift.org/t/pitch-debug-description-macro/67711. In this initial commit, the macro is named _DebugDescription to indicate it's internal use at this time, pending Swift Evolution.

rdar://115180949

kastiglione · 2023-11-02T22:52:41Z

@swift-ci test

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

ahmednour25689

LGTM few nits

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

kastiglione · 2023-11-04T01:10:48Z

@swift-ci test

kastiglione · 2023-11-06T18:51:06Z

@swift-ci test windows

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

DougGregor · 2023-11-08T20:18:24Z

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

+        //   1. The body must have a single item
+        //   2. The single item must be a string literal
+        //   3. Later on, the interpolation in the string literal will be validated.
+        guard let codeBlock = descriptionProperty.accessorBlock?.accessors.as(CodeBlockItemListSyntax.self),


Hmm, this will be a bit of an issue. A macro applied to a type shouldn't be able to see the bodies of functions defined within that type, because (for example) we need to be able to determine the members and layout of a type without evaluating the bodies of functions. Unfortunately, @hborla and I discussed this and (IIRC) it's described in a proposal, but the compiler fails to enforce it.

I think what we'll end up needing to do here is collect information about the type itself in a member-attribute macro, then use it to add an attribute to the "description" property which can look at the body. I think that can be made to work.

because (for example) we need to be able to determine the members and layout of a type without evaluating the bodies of functions

does this apply to a peer macro? Since a peer macro doesn't introduce any members, the compiler could determine the members and layout independent of expansion of the peer macro.

I think what we'll end up needing to do here is collect information about the type itself in a member-attribute macro, then use it to add an attribute to the "description" property which can look at the body. I think that can be made to work.

I think this makes sense except for one missing piece to the puzzle. The macro attached do description won't have the ability to emit a (global) constant to a custom section. Not currently. Perhaps a change to the compiler could allow a macro attached to description to emit data to a custom section. Thoughts?

correction: it is now possible to emit static properties to custom sections, thanks to this recent change: #69192

See e2b3659

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

ahoppen

Just a few minor style comments

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

ahoppen · 2023-12-05T22:54:41Z

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

+    public static func expansion<DeclGroup, DeclSyntax, Context>(
+        of node: AttributeSyntax,
+        attachedTo declaration: DeclGroup,
+        providingAttributesFor member: DeclSyntax,
+        in context: Context
+    )
+    throws -> [AttributeSyntax]
+    where DeclGroup: DeclGroupSyntax, DeclSyntax: DeclSyntaxProtocol, Context: MacroExpansionContext


Can be simplified to

Suggested change

public static func expansion<DeclGroup, DeclSyntax, Context>(

of node: AttributeSyntax,

attachedTo declaration: DeclGroup,

providingAttributesFor member: DeclSyntax,

in context: Context

)

throws -> [AttributeSyntax]

where DeclGroup: DeclGroupSyntax, DeclSyntax: DeclSyntaxProtocol, Context: MacroExpansionContext

public static func expansion(

of node: AttributeSyntax,

attachedTo declaration: some DeclGroupSyntax,

providingAttributesFor member: some DeclSyntaxProtocol,

in context: some MacroExpansionContext

)

throws -> [AttributeSyntax]

I will add a comment. The reason it's written this way is because of an lldb bug which doesn't yet support some style generics. The way it's written allows me to po swift-syntax values.

In that case, I would prefer to not use DeclSyntax as a generic type because it shadows the DeclSyntax type from SwiftSyntax.

I updated this PR to use the some syntax, and kept my local working copy with the generic syntax.

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

ahoppen · 2023-12-05T23:03:47Z

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

+        let arguments = argumentList.map(\.expression)
+        guard let typeIdentifier = String(expr: arguments[0]),
+              let computedProperties = Array<String>(expr: arguments[1]) else {
+            assertionFailure("_DebugDescriptionProperty called incorrectly")


In general, I would prefer to emit errors here instead of hitting assertionFailures. As I mentioned above, somebody might redeclare _DebugDescriptionPropertyMacro.

I'm not sure what scenario would result in broken internals being used for real. Also, that these hypothetical errors would not be in the code written by a user.

I’m just thinking that in general it’s nicer to throw errors than to crash because they tend to be more actionable for the user. Either because they re-declared the macro or because there was some hole that we missed. But up to you.

I took your suggestion. No more asserts, only diagnostics.

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

kastiglione · 2023-12-06T18:41:24Z

@swift-ci test

kastiglione · 2023-12-06T20:45:56Z

@swift-ci test

kastiglione · 2023-12-07T01:03:15Z

Test failures are due to use of String.contains and CI sets the deployment target to 10.13 where contains isn't available. I will push an update to fix it.

kastiglione · 2023-12-07T03:59:05Z

@swift-ci test

DougGregor

This is looking great!

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

DougGregor · 2023-12-07T19:59:49Z

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift

+
+extension MemberAccessExprSyntax {
+  /// Parse a member access consisting only of property references. Any other syntax throws an error.
+  fileprivate func propertyChain() throws -> [DeclReferenceExprSyntax] {


It's somewhat stylistic, but you could consider putting this on ExprSyntaxProtocol and dealing with both the DeclReferenceExprSyntax and MemberAccessExprSyntax cases there. Then the client of this function wouldn't have to separate them out.

I like it, thanks! Done in e54634c.

stdlib/public/core/DebuggerSupport.swift

kastiglione · 2023-12-08T00:12:40Z

@swift-ci test

kastiglione · 2023-12-08T17:03:20Z

@swift-ci test

finagolfin · 2023-12-09T04:20:22Z

This pull broke building the stdlib with a compiler with SWIFT_BUILD_SWIFT_SYNTAX disabled, as macros are unavailable then:

stdlib/public/core/DebuggerSupport.swift:36:14: error: macros are not supported in this compiler
public macro _DebugDescription() =
             ^
/home/ubuntu/jenkins/workspace/oss-swift-RA-linux-ubuntu-16.04-android/swift/stdlib/public/core/DebuggerSupport.swift:42:14: error: macros are not supported in this compiler
public macro _DebugDescriptionProperty(_ debugIdentifier: String, _ computedProperties: [String]) =
             ^

kastiglione · 2023-12-09T04:43:45Z

Thanks. It seems I should restore the #if $Macros that existed in b2c4c3c.

kastiglione · 2023-12-09T04:48:33Z

@finagolfin #70351

Make `_DebugDescription` macro conditional upon macro availability in the build. The `Macros` feature is based on the value of `SWIFT_BUILD_SWIFT_SYNTAX`. See #69626 (comment)

Implementation of the DebugDescription macro pitched on the forums: https://forums.swift.org/t/pitch-debug-description-macro/67711. In this initial commit, the macro is named `_DebugDescription` to indicate it's internal use at this time, pending Swift Evolution. rdar://115180949

) Make `_DebugDescription` macro conditional upon macro availability in the build. The `Macros` feature is based on the value of `SWIFT_BUILD_SWIFT_SYNTAX`. See swiftlang#69626 (comment)

beccadax · 2024-07-23T20:27:15Z

test/Macros/DebugDescription/either_description.swift

+struct MyStruct1: CustomStringConvertible {
+  var description: String { "thirty" }
+}
+// CHECK: static let _lldb_summary = (


It would be a good idea for the macro to explicitly declare the type of the _lldb_summary property:

Suggested change

// CHECK: static let _lldb_summary = (

// CHECK: static let _lldb_summary: (UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8) =

While Swift allows you to infer a property's type from its initial value expression, that can impact compiler performance by forcing the compiler to type-check the initial value expression when it's compiling other files. We're generating these declarations anyway, so we might as well generate an explicit type when we do.

Thanks. This was mentioned on the SE discussion just the other day. I assumed, incorrectly, that the way it was generated would be fine. I did a couple benchmarks with reasonable sized tuples and there was a moderate differences in wall time. Do you have any opinion on whether to introduce a local type alias for the tuple (which was suggested in the SE discussion)? I think benefit would be a slightly more readable macro expansion output, but a tuple of bytes isn't particularly readable in the first place.

[Debug] Add DebugDescription macro

0185d06

kastiglione requested review from DougGregor, adrian-prantl and hborla November 2, 2023 22:53

kastiglione changed the title ~~[Debug] Add DebugDescription macro~~ [Debug] Add _DebugDescription macro Nov 2, 2023

augusto2112 reviewed Nov 2, 2023

View reviewed changes

ahmednour25689 reviewed Nov 3, 2023

View reviewed changes

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift Outdated Show resolved Hide resolved

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift Outdated Show resolved Hide resolved

lib/Macros/Sources/SwiftMacros/DebugDescriptionMacro.swift Outdated Show resolved Hide resolved

adrian-prantl reviewed Nov 3, 2023

View reviewed changes

kastiglione added 2 commits November 3, 2023 17:40

Fix ULEB encoding

bf7f423

Add docstrings, and minor code cleanup

21c6c47

kastiglione requested a review from ahoppen November 8, 2023 19:52

DougGregor reviewed Nov 8, 2023

View reviewed changes

ahoppen reviewed Nov 9, 2023

View reviewed changes

Factor into two macros (@DougGregor); Apply feedback (@ahoppen)

e2b3659

kastiglione requested review from DougGregor, ahoppen, augusto2112 and adrian-prantl December 5, 2023 18:14

ahoppen reviewed Dec 5, 2023

View reviewed changes

kastiglione added 3 commits December 6, 2023 10:07

Address more @ahoppen review feedback

7b9b1a3

Reformat to 2 space indent

d5508bb

Use some Protocol syntax

3c45cd3

kastiglione requested a review from ahoppen December 6, 2023 18:09

Declare _DebugDescription in stdlib

b2c4c3c

kastiglione requested a review from a team as a code owner December 6, 2023 20:45

Support targets outside of macOS 13+

0f51f30

DougGregor approved these changes Dec 7, 2023

View reviewed changes

Apply @DougGregor's feedback

e54634c

kastiglione merged commit ad585b3 into main Dec 8, 2023

kastiglione deleted the dl/Debug-Add-DebugDescription-macro branch December 8, 2023 23:19

kastiglione mentioned this pull request Dec 9, 2023

[Debug] Conditional _DebugDescription macro declaration #70351

Merged

beccadax reviewed Jul 23, 2024

View reviewed changes

	// CHECK: static let _lldb_summary = (
	// CHECK: static let _lldb_summary: (UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8, UInt8) =

[Debug] Add _DebugDescription macro #69626

[Debug] Add _DebugDescription macro #69626

Uh oh!

Conversation

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

Uh oh!

Uh oh!

kastiglione commented Nov 2, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahmednour25689 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kastiglione commented Nov 4, 2023

Uh oh!

kastiglione commented Nov 6, 2023

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kastiglione commented Dec 6, 2023

Uh oh!

kastiglione commented Dec 6, 2023

Uh oh!

kastiglione commented Nov 2, 2023 •

edited

Loading