Skip to content

Handle vendor string in match for clang version string #145612

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

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

dbartol
Copy link
Contributor

@dbartol dbartol commented Jun 24, 2025

#145455 failed to account for the optional "vendor string" that precedes the clang version in the Clang version string metadata. I just updated it to use a regex to allow arbitrary content before the clang version. Tested this both with and without a vendor string, and it seems to work as expected.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Jun 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 24, 2025

@llvm/pr-subscribers-clang

Author: Dave Bartolomeo (dbartol)

Changes

#145455 failed to account for the optional "vendor string" that precedes the clang version in the Clang version string metadata. I just updated it to use a regex to allow arbitrary content before the clang version. Tested this both with and without a vendor string, and it seems to work as expected.


Full diff: https://github.com/llvm/llvm-project/pull/145612.diff

1 Files Affected:

  • (modified) clang/test/CodeGen/pragma-comment.c (+1-1)
diff --git a/clang/test/CodeGen/pragma-comment.c b/clang/test/CodeGen/pragma-comment.c
index 861fba9aece3b..aa3aba18b9b2c 100644
--- a/clang/test/CodeGen/pragma-comment.c
+++ b/clang/test/CodeGen/pragma-comment.c
@@ -34,4 +34,4 @@
 // ELF-NOT: foo
 // This following match prevents the clang version metadata from matching the forbidden 'foo' and 'bar' tokens.
 // This can happen if the clang version string contains a Git repo URL that includes one of those substrings.
-// ELF-LABEL: !"clang version
+// ELF-LABEL: {{\!\".*clang version}}

@dbartol
Copy link
Contributor Author

dbartol commented Jun 24, 2025

@AaronBallman

@dbartol
Copy link
Contributor Author

dbartol commented Jun 24, 2025

@Sharjeel-Khan This should handle any vendor string before clang version, so I expect it to fix the Android break as well.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM!

@AaronBallman AaronBallman merged commit 54953b9 into llvm:main Jun 25, 2025
9 checks passed
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
llvm#145455 failed to account for the optional "vendor string" that precedes
the `clang version` in the Clang version string metadata. I just updated
it to use a regex to allow arbitrary content before the `clang version`.
Tested this both with and without a vendor string, and it seems to work
as expected.
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
llvm#145455 failed to account for the optional "vendor string" that precedes
the `clang version` in the Clang version string metadata. I just updated
it to use a regex to allow arbitrary content before the `clang version`.
Tested this both with and without a vendor string, and it seems to work
as expected.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants