Skip to content

fix: make hintmap label lowercase as all strings are mad lowercase#252

Merged
chvamshi-xilinx merged 1 commit intoXilinx:main-gefrom
HimanshuChoudhary-Xilinx:bugfix12
Mar 26, 2026
Merged

fix: make hintmap label lowercase as all strings are mad lowercase#252
chvamshi-xilinx merged 1 commit intoXilinx:main-gefrom
HimanshuChoudhary-Xilinx:bugfix12

Conversation

@HimanshuChoudhary-Xilinx
Copy link
Collaborator

Problem solved by the commit

Bug / issue (if any) fixed, which PR introduced the bug, how it was discovered

How problem was solved, alternative solutions (if any) and why they were rejected

Risks (if any) associated the changes in the commit

What has been tested and how, request additional testing if necessary

Documentation impact (if any)

Signed-off-by: Himanshu Choudhary <Himanshu.Choudhary@amd.com>
Copilot AI review requested due to automatic review settings March 25, 2026 13:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR normalizes PREEMPT hintmap labels to lowercase during ASM parsing so the internally tracked hintmap keys match the lowercased argument strings used elsewhere in the preprocessor pipeline.

Changes:

  • Lowercase the optional PREEMPT hintmap label (args[3]) after stripping a leading @.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +274 to +276
if (!hintmap_label.empty()) {
std::transform(hintmap_label.begin(), hintmap_label.end(), hintmap_label.begin(),
[](unsigned char c) { return static_cast<char>(std::tolower(c)); });
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

This new lowercasing logic relies on std::transform and std::tolower, but this translation unit doesn't explicitly include <algorithm> / <cctype>. It currently compiles only if those headers are pulled in transitively; please add the direct includes to make this file self-contained and portable across toolchains.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@chvamshi-xilinx chvamshi-xilinx merged commit 3794e3a into Xilinx:main-ge Mar 26, 2026
10 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.

3 participants