-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[Serialization] Reword xref errors and show notes for common issues #36431
Conversation
I'm still looking for a way to test this as this error should only be display for configuration errors that can't be recovered from. @swift-ci Please smoke test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea, though of course it needs tests. (Maybe you could arrange for -I %t
to be passed to the compiler and add new headers there between writing a .swiftmodule file and reading it back in?)
// with serialized SIL and a client. | ||
auto line = "'" + declNameStr + "' in module '" + | ||
baseModule->getName().str() + | ||
"' was filtered out."; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This note won't mean anything to an end user. It might be worth the effort to modify filterValues()
so it preserves information about why a declaration was filtered out so we can describe it here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll revisit this the next time I see an issue with the filtering. We can probably combine the details of the decls that were filtered out with the reason why and it will be more helpful. I still think most users will have to refer to us because this crash shouldn't happen with supported configurations, in theory at least.
return llvm::make_error<XRefError>("top-level value not found", pathTrace, | ||
getXRefDeclNameForError()); | ||
declName, notes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to make this change to other XRefErrors too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "top-level value not found" is the main error point that I've been seeing reporting such failures. I don't like so much the complexity this adds even currently so I'd prefer to make sure it's useful on other error points before adding more complexity.
The other XRefErrors above are more for Swift specific concepts like operators and precedence groups so modularization is more reliable with them. The errors below are for nested declarations so the search in other modules doesn't apply (I think) but the notes on filtering could be useful there.
@swift-ci Please smoke test |
Try to give more information to the user for likely project issues causing XRef errors.
…n failures Previous output: ``` could not deserialize type for ‘foo(_:)': top-level value not found Cross-reference to module ‘Bar_Private' ... Baz ``` New output: ``` Could not deserialize type for ‘foo(_:)' Caused by: top-level value not found Cross-reference to ‘Baz' in module 'Bar_Private' ```
@swift-ci Please smoke test |
@swift-ci Please smoke test Linux |
Try to improve a common deserialization failure error message and provide possible solution paths.
For a given old message:
This is the new message: