Skip to content

[Clang] Optimize tok::isLiteral with range-based condition #153228

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 4 commits into from
Aug 13, 2025

Conversation

Thibault-Monnier
Copy link
Contributor

This commit optimizes tok::isLiteral by replacing a succession of 13 conditions with a range-based check.

I am not sure whether this is allowed. I believe it is done nowhere else in the codebase ; however, I have seen range-based conditions being used with other enums.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 12, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 12, 2025

@llvm/pr-subscribers-clang

Author: Thibault Monnier (Thibault-Monnier)

Changes

This commit optimizes tok::isLiteral by replacing a succession of 13 conditions with a range-based check.

I am not sure whether this is allowed. I believe it is done nowhere else in the codebase ; however, I have seen range-based conditions being used with other enums.


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

1 Files Affected:

  • (modified) clang/include/clang/Basic/TokenKinds.h (+1-4)
diff --git a/clang/include/clang/Basic/TokenKinds.h b/clang/include/clang/Basic/TokenKinds.h
index 1b133dde89587..f9927182d3484 100644
--- a/clang/include/clang/Basic/TokenKinds.h
+++ b/clang/include/clang/Basic/TokenKinds.h
@@ -95,10 +95,7 @@ inline bool isStringLiteral(TokenKind K) {
 /// Return true if this is a "literal" kind, like a numeric
 /// constant, string, etc.
 inline bool isLiteral(TokenKind K) {
-  return K == tok::numeric_constant || K == tok::char_constant ||
-         K == tok::wide_char_constant || K == tok::utf8_char_constant ||
-         K == tok::utf16_char_constant || K == tok::utf32_char_constant ||
-         isStringLiteral(K) || K == tok::header_name || K == tok::binary_data;
+  return K >= tok::numeric_constant && K <= tok::utf32_string_literal;
 }
 
 /// Return true if this is any of tok::annot_* kinds.

@Thibault-Monnier Thibault-Monnier changed the title Optimize tok::isLiteral with range-based condition [clang] Optimize tok::isLiteral with range-based condition Aug 12, 2025
@Thibault-Monnier Thibault-Monnier changed the title [clang] Optimize tok::isLiteral with range-based condition [clang:frontend] Optimize tok::isLiteral with range-based condition Aug 12, 2025
@cor3ntin cor3ntin changed the title [clang:frontend] Optimize tok::isLiteral with range-based condition [Clang] Optimize tok::isLiteral with range-based condition Aug 12, 2025
K == tok::wide_char_constant || K == tok::utf8_char_constant ||
K == tok::utf16_char_constant || K == tok::utf32_char_constant ||
isStringLiteral(K) || K == tok::header_name || K == tok::binary_data;
return K >= tok::numeric_constant && K <= tok::utf32_string_literal;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add a comment in TokenKinds.def (before numeric_constant) such that we guarantee someone does not change the order or add an assert that tests for equality?
I don't think we currently rely on, or guarantee an order here @AaronBallman

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will add an assert.

@Thibault-Monnier
Copy link
Contributor Author

@cor3ntin I am done.

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Corentin Jabot <[email protected]>
Copy link

github-actions bot commented Aug 13, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@Thibault-Monnier
Copy link
Contributor Author

Thanks @cor3ntin! I do not have commit access, could you please commit for me?

@AaronBallman AaronBallman merged commit b75896b into llvm:main Aug 13, 2025
9 checks passed
@Thibault-Monnier Thibault-Monnier deleted the token-optimization branch August 13, 2025 12:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants