Skip to content
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

Revert "[DebugInfo] Fix handling of @_originallyDefinedIn types" #78324

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Member

Reverts #78104

This is potentially breaking the round trip of symbol decoration.

@compnerd
Copy link
Member Author

@swift-ci please smoke test

@compnerd compnerd requested a review from augusto2112 December 20, 2024 18:14
@augusto2112
Copy link
Contributor

@compnerd do you know which type is triggering the assertion failure?

@augusto2112
Copy link
Contributor

There's no way to do the round trip when the type is annotated with @_originallyDefinedIn. I added some code to skip the round tripping for types affected by that attribute, but maybe I missed some cases.

@compnerd
Copy link
Member Author

$s11SwiftFormat24DocumentationCommentTextV13extractedFromACSg08CompilerA6Syntax6TriviaV_tcfc4LineL_VD is one of the two that we are seeing.

@augusto2112
Copy link
Contributor

What project are you compiling? I don't see any @_originallyDefinedIn in swift-format or swift-syntax. Would you mind posting the entire error?

@hjyamauchi
Copy link
Contributor

hjyamauchi commented Dec 20, 2024

Reverting this PR seems to fix this issue: #78326 CC @augusto2112

@augusto2112
Copy link
Contributor

I was able to reproduce it, let me work on a fix

@compnerd
Copy link
Member Author

@augusto2112 can we revert the change if you cannot get the fix in before the freeze tonight?

@augusto2112
Copy link
Contributor

@compnerd sure, I have a fix here: #78328

@augusto2112
Copy link
Contributor

@compnerd in case mine fails in some other unexpected way you can open a PR reverting ce1a9d565f537f81b3d40e67666c91e9cc178938 and 15bb8ecccd8b371b9b33a876d8b04e8f8aa42eea on llvm-project, and cross testing with this one, to get it ready to merge just in case.

@compnerd
Copy link
Member Author

Please test with following PRs:
swiftlang/llvm-project#9785

@swift-ci please smoke test

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