Skip to content

Conversation

hnrklssn
Copy link
Contributor

This adds some dynamically togglable debug logs for the swiftify function in the ClangImporter under -Xllvm -debug-only=safe-interop-wrappers, and for the _SwiftifyImport macro under -Xllvm -debug-only=swiftify-import

@hnrklssn
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@j-hui j-hui left a comment

Choose a reason for hiding this comment

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

This LGTM, but I'm not sure familiar with what conventions are around inserting normally-inert debug statements like this in the Swift compiler source, so I advise waiting for at least one more approval from someone with more experience in this code base than me.

@hnrklssn
Copy link
Contributor Author

@swift-ci please smoke test

@hnrklssn hnrklssn force-pushed the swiftify-debug-logging branch from b5cb83e to cc4435a Compare May 24, 2025 00:14
@hnrklssn
Copy link
Contributor Author

@swift-ci please smoke test

spanAvailability: String? = nil,
typeMappings: [String: String] = [:]) =
typeMappings: [String: String] = [:],
debug: Bool = false) =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DougGregor do you think including conditionally enableable debug logging in the macro is reasonable, or is that a no-no in the stdlib?

Copy link
Member

Choose a reason for hiding this comment

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

I don't really love the idea of putting the debug parameter into the macro itself, and I don't know that it even really works to have the macro printing things to standard output. For this functionality (what's the input to the macro?), I'd rather have that as part of the compiler's debugging flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh that's a good point, that doesn't need to be specific to this macro

These can be enabled with `-Xcc -mllvm -debug-only=macros`.
This is a pure refactor to enable using PrintAST in the next commit.
@hnrklssn
Copy link
Contributor Author

@swift-ci please smoke test

The argument list is going to be mostly empty if PrintExprs is not
enabled. Always trying to print the arguments also broke quite a few
tests, because it would result in a mismatch between the swift interface
before and after serialization, since CustomAttr arguments are not
serialized.
@hnrklssn
Copy link
Contributor Author

@swift-ci please smoke test

@hnrklssn hnrklssn enabled auto-merge September 12, 2025 20:02
@hnrklssn hnrklssn merged commit 8e802be into swiftlang:main Sep 13, 2025
3 checks passed
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.

4 participants