[SYCL] Refactor sycl-post-link for better error handling and code simplicity#21579
Open
maksimsab wants to merge 3 commits intointel:syclfrom
Open
[SYCL] Refactor sycl-post-link for better error handling and code simplicity#21579maksimsab wants to merge 3 commits intointel:syclfrom
maksimsab wants to merge 3 commits intointel:syclfrom
Conversation
…plicity This commit improves error handling and simplifies code in sycl-post-link without changing functionality: **Error handling improvements:** - Convert functions to return Error instead of void - Propagate errors up to the stack to comply with LLVM guidelines - Introduce ExitOnError pattern for consistent error handling - Replace error() and checkError() calls with ExitOnErr() for cleaner flow **Code refactoring:** - Refactor simple utility functions reusing LLVM functionality **Code simplification:** - Remove unnecessary includes - Simplify SmallVector declarations by removing MAX_COLUMNS_IN_FILE_TABLE constant and using default template parameter These changes prepare the code for extraction to a library component while improving maintainability and following LLVM error handling conventions. All refactored functions maintain the same functionality but with proper error propagation.
Contributor
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
There was a problem hiding this comment.
Question: was AI used to generate substantial amount of content of this PR?
Contributor
Author
|
What AI did:
|
Contributor
sounds good, thanks. I just wanted to make sure we follow this policy: https://llvm.org/docs/AIToolPolicy.html |
| Error saveModuleSymbolTable(const module_split::ModuleDesc &MD, | ||
| const StringRef Filename) { | ||
| return writeToOutput(Filename, [&](raw_ostream &OS) -> Error { | ||
| OS << Table; |
Contributor
There was a problem hiding this comment.
Does the code compiles after this change?
YuriPlyakhin
requested changes
Mar 24, 2026
Contributor
YuriPlyakhin
left a comment
YuriPlyakhin
left a comment
There was a problem hiding this comment.
the code is unlikely to compile...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit improves error handling and simplifies code in sycl-post-link without changing functionality:
Error handling improvements:
Code refactoring:
Code simplification:
These changes prepare the code for extraction to a library component while improving maintainability and following LLVM error handling conventions. All refactored functions maintain the same functionality but with proper error propagation.