Skip to content

[SYCLomatic][Python Migration][Bug] Fix case_010_include_rule_file lit case#2861

Merged
zhimingwang36 merged 3 commits into
oneapi-src:SYCLomaticfrom
TejaX-Alaghari:py_mig_bug
Jun 5, 2025
Merged

[SYCLomatic][Python Migration][Bug] Fix case_010_include_rule_file lit case#2861
zhimingwang36 merged 3 commits into
oneapi-src:SYCLomaticfrom
TejaX-Alaghari:py_mig_bug

Conversation

@TejaX-Alaghari

@TejaX-Alaghari TejaX-Alaghari commented May 29, 2025

Copy link
Copy Markdown
Contributor

This PR fixes the flaky Python rule file inclusion bug which fails when including rule files

The root cause of this issue is due to using llvm::StringRef objects for tracking the processed YAML files in a llvm::DensetSet.
The contents of the llvm::StringRef objects change for each call to readYAMLFile() function.
And at times, the string data pointed by the llvm::StringRef objects stored in the llvm::DenseSet are altered unintentionally causing rule files to be skipped from including.

This issue is fixed by using std::unordered_set<std::string> in place of llvm::DenseSet<llvm::StringRef> where std::string objects have the advantage of owning the memory of char array they store.

@TejaX-Alaghari TejaX-Alaghari requested a review from a team as a code owner May 29, 2025 10:00
@TejaX-Alaghari TejaX-Alaghari changed the title Added debug condition for checking failure case of python inc rule file case [SYCLomatic][Python Migration][Bug] Fix case_010_include_rule_file lit case May 30, 2025
@TejaX-Alaghari TejaX-Alaghari force-pushed the py_mig_bug branch 2 times, most recently from 8baff5a to 413275b Compare May 31, 2025 05:59

@tomflinda tomflinda left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@zhimingwang36 zhimingwang36 merged commit 3006eef into oneapi-src:SYCLomatic Jun 5, 2025
7 checks passed
@TejaX-Alaghari TejaX-Alaghari deleted the py_mig_bug branch June 5, 2025 12:37
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