Skip to content

[lldb] Re-use clang's keyword enable/disable mechanism in CPlusPlusNameParser.cpp #164284

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.

Sign up for GitHub

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
+68 −61
Open

[lldb] Re-use clang's keyword enable/disable mechanism in CPlusPlusNameParser.cpp #164284

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e1175e6
[lldb] Re-use clang's keyword enable/disable mechanism in CPlusPlusNa…
dsandersllvm Oct 18, 2025
0275d00
fixup: use `using namespace clang` instead
dsandersllvm Oct 27, 2025
f221e49
Revert "[lldb] Re-use clang's keyword enable/disable mechanism in CPl…
dsandersllvm Oct 27, 2025
7af1e87
[clang] Refactor clang's keyword enable/disable mechanism to allow ll…
dsandersllvm Oct 18, 2025
e03585e
[lldb] Re-use clang's keyword enable/disable mechanism in CPlusPlusNa…
dsandersllvm Oct 27, 2025
8e0c1a7
[clang] Refactor clang's keyword enable/disable mechanism to allow ll…
dsandersllvm Oct 18, 2025
2c7391f
fixup: formatting
dsandersllvm Oct 27, 2025
af4a46f
fixup: Move getKeywordStatus's comment to the header
dsandersllvm Nov 4, 2025
6a80842
fixup: Restore comment for TokenKey
dsandersllvm Nov 5, 2025
b836bc6
Merge branch 'lldb-parse-identifiers-keywords-clang-only' into lldb-p…
dsandersllvm Nov 5, 2025
3acac96
fixup: Move using
dsandersllvm Nov 5, 2025
72e9d76
fixup: Test Class::constant in CPlusPlusLanguageTest.cpp
dsandersllvm Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions clang/include/clang/Basic/IdentifierTable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,59 @@ class LangOptions;
class MultiKeywordSelector;
class SourceLocation;

// This is an inline namespace to allow both clang and lldb to use them without
// namespace prefixes (via `using namespace` in lldb's case).
inline namespace TokenKeyEnumerators {
Copy link
Member

@Michael137 Michael137 Oct 20, 2025

Choose a reason for hiding this comment

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

Let's make the Clang changes in a separate PR. I don't think you need the inline namespace either. Can we just follow the convention of the other enums in this header?

Copy link
Collaborator Author

@dsandersllvm dsandersllvm Oct 20, 2025

Choose a reason for hiding this comment

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

I don't mind splitting it but that PR won't make sense without the context of this PR

The reason we need the inline namespace is because of this code:

#define KEYWORD(NAME, FLAGS)                                                   \
  AddResult = getKeywordStatus(LangOpts, FLAGS);                               \
  ...
#include "clang/Basic/TokenKinds.def"

I can't reference FLAGS as clang::FLAGS because some of the KEYWORD() declaration's in TokenKinds.def are the | of multiple values such as BOOLSUPPORT|KEYC23 and I'd need the namespace to appear on each identifier. enum class is ruled out for the same reason, and enum class combined with using enum ... requires C++20 and therefore can't be used.

inline namespace allows lldb to import them with using namespace ... for use as bare names without changing how clang uses them

Copy link
Member

@Michael137 Michael137 Oct 21, 2025

Choose a reason for hiding this comment

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

I don't mind splitting it but that PR won't make sense without the context of this PR

I don't have a huge preference. Typically we split out cross-project changes into separate PRs. It makes the area maintainers more likely to review it in time (just based filtering email subjects) and has the benefit of making the main non-NFC PR smaller. You can always just reference this PR as a motivating example.

I can't reference FLAGS as clang::FLAGS because some of the KEYWORD() declaration's in TokenKinds.def are the | of multiple values such as BOOLSUPPORT|KEYC23 and I'd need the namespace to appear on each identifier. enum class is ruled out for the same reason, and enum class combined with using enum ... requires C++20 and therefore can't be used.

Can't we just do using namespace clang inside that LLDB function then?

Copy link
Collaborator Author

@dsandersllvm dsandersllvm Oct 27, 2025

Choose a reason for hiding this comment

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

Separated out the clang portion into #165323. The commits are still in this PR as it's a dependency for this to work

Can't we just do using namespace clang inside that LLDB function then?

I've switched it to using namespace clang. I assume lldb is generally not doing that because of name collisions and wanting to know when it's using other components but this is at least scoped to a small function so it should still be fine from that perspective

enum TokenKey : unsigned {
KEYC99 = 0x1,
KEYCXX = 0x2,
KEYCXX11 = 0x4,
KEYGNU = 0x8,
KEYMS = 0x10,
BOOLSUPPORT = 0x20,
KEYALTIVEC = 0x40,
KEYNOCXX = 0x80,
KEYBORLAND = 0x100,
KEYOPENCLC = 0x200,
KEYC23 = 0x400,
KEYNOMS18 = 0x800,
KEYNOOPENCL = 0x1000,
WCHARSUPPORT = 0x2000,
HALFSUPPORT = 0x4000,
CHAR8SUPPORT = 0x8000,
KEYOBJC = 0x10000,
KEYZVECTOR = 0x20000,
KEYCOROUTINES = 0x40000,
KEYMODULES = 0x80000,
KEYCXX20 = 0x100000,
KEYOPENCLCXX = 0x200000,
KEYMSCOMPAT = 0x400000,
KEYSYCL = 0x800000,
KEYCUDA = 0x1000000,
KEYZOS = 0x2000000,
KEYNOZOS = 0x4000000,
KEYHLSL = 0x8000000,
KEYFIXEDPOINT = 0x10000000,
KEYMAX = KEYFIXEDPOINT, // The maximum key
KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
KEYALL = (KEYMAX | (KEYMAX - 1)) & ~KEYNOMS18 & ~KEYNOOPENCL &
~KEYNOZOS // KEYNOMS18, KEYNOOPENCL, KEYNOZOS are excluded.
};
} // namespace TokenKeyEnumerators

/// How a keyword is treated in the selected standard. This enum is ordered
/// intentionally so that the value that 'wins' is the most 'permissive'.
enum KeywordStatus {
KS_Unknown, // Not yet calculated. Used when figuring out the status.
KS_Disabled, // Disabled
KS_Future, // Is a keyword in future standard
KS_Extension, // Is an extension
KS_Enabled, // Enabled
};

KeywordStatus getKeywordStatus(const LangOptions &LangOpts,
unsigned Flags);

enum class ReservedIdentifierStatus {
NotReserved = 0,
StartsWithUnderscoreAtGlobalScope,
Expand Down
53 changes: 1 addition & 52 deletions clang/lib/Basic/IdentifierTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,57 +77,6 @@ IdentifierTable::IdentifierTable(const LangOptions &LangOpts,
// Language Keyword Implementation
//===----------------------------------------------------------------------===//

// Constants for TokenKinds.def
namespace {

enum TokenKey : unsigned {
KEYC99 = 0x1,
KEYCXX = 0x2,
KEYCXX11 = 0x4,
KEYGNU = 0x8,
KEYMS = 0x10,
BOOLSUPPORT = 0x20,
KEYALTIVEC = 0x40,
KEYNOCXX = 0x80,
KEYBORLAND = 0x100,
KEYOPENCLC = 0x200,
KEYC23 = 0x400,
KEYNOMS18 = 0x800,
KEYNOOPENCL = 0x1000,
WCHARSUPPORT = 0x2000,
HALFSUPPORT = 0x4000,
CHAR8SUPPORT = 0x8000,
KEYOBJC = 0x10000,
KEYZVECTOR = 0x20000,
KEYCOROUTINES = 0x40000,
KEYMODULES = 0x80000,
KEYCXX20 = 0x100000,
KEYOPENCLCXX = 0x200000,
KEYMSCOMPAT = 0x400000,
KEYSYCL = 0x800000,
KEYCUDA = 0x1000000,
KEYZOS = 0x2000000,
KEYNOZOS = 0x4000000,
KEYHLSL = 0x8000000,
KEYFIXEDPOINT = 0x10000000,
KEYMAX = KEYFIXEDPOINT, // The maximum key
KEYALLCXX = KEYCXX | KEYCXX11 | KEYCXX20,
KEYALL = (KEYMAX | (KEYMAX - 1)) & ~KEYNOMS18 & ~KEYNOOPENCL &
~KEYNOZOS // KEYNOMS18, KEYNOOPENCL, KEYNOZOS are excluded.
};

/// How a keyword is treated in the selected standard. This enum is ordered
/// intentionally so that the value that 'wins' is the most 'permissive'.
enum KeywordStatus {
KS_Unknown, // Not yet calculated. Used when figuring out the status.
KS_Disabled, // Disabled
KS_Future, // Is a keyword in future standard
KS_Extension, // Is an extension
KS_Enabled, // Enabled
};

} // namespace

// This works on a single TokenKey flag and checks the LangOpts to get the
// KeywordStatus based exclusively on this flag, so that it can be merged in
// getKeywordStatus. Most should be enabled/disabled, but some might imply
Expand Down Expand Up @@ -222,7 +171,7 @@ static KeywordStatus getKeywordStatusHelper(const LangOptions &LangOpts,

/// Translates flags as specified in TokenKinds.def into keyword status
/// in the given language standard.
static KeywordStatus getKeywordStatus(const LangOptions &LangOpts,
KeywordStatus clang::getKeywordStatus(const LangOptions &LangOpts,
unsigned Flags) {
// KEYALL means always enabled, so special case this one.
if (Flags == KEYALL) return KS_Enabled;
Expand Down
20 changes: 13 additions & 7 deletions lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,9 +719,8 @@ CPlusPlusNameParser::ParseFullNameImpl() {
}
start_position.Remove();
return result;
} else {
return std::nullopt;
}
return std::nullopt;
}

llvm::StringRef CPlusPlusNameParser::GetTextForRange(const Range &range) {
Expand Down Expand Up @@ -755,12 +754,19 @@ static const clang::LangOptions &GetLangOptions() {
return g_options;
}

static const llvm::StringMap<tok::TokenKind> &GetKeywordsMap() {
static llvm::StringMap<tok::TokenKind> g_map{
#define KEYWORD(Name, Flags) {llvm::StringRef(#Name), tok::kw_##Name},
static const llvm::StringMap<tok::TokenKind> GetKeywordsMap() {
using namespace clang::TokenKeyEnumerators;
llvm::StringMap<tok::TokenKind> g_map;

auto LangOpts = GetLangOptions();

clang::KeywordStatus AddResult;
#define KEYWORD(NAME, FLAGS) \
AddResult = getKeywordStatus(LangOpts, FLAGS); \
if (AddResult != clang::KS_Disabled) \
g_map[llvm::StringRef(#NAME)] = tok::kw_##NAME;
#include "clang/Basic/TokenKinds.def"
#undef KEYWORD
};
return g_map;
}

Expand All @@ -769,7 +775,7 @@ void CPlusPlusNameParser::ExtractTokens() {
return;
clang::Lexer lexer(clang::SourceLocation(), GetLangOptions(), m_text.data(),
m_text.data(), m_text.data() + m_text.size());
const auto &kw_map = GetKeywordsMap();
const auto kw_map = GetKeywordsMap();
Copy link
Member

@Michael137 Michael137 Oct 28, 2025

Choose a reason for hiding this comment

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

It's a bit unfortunate we have to reconstruct this StringMap every time. Do you have an intuition for how expensive that is?

Why can't we construct the map statically? The LangOptions never change right? So you could just wrap all of this new logic in an immediately-invoked-lambda and still make the g_map static. Wdyt?

Copy link
Collaborator Author

@dsandersllvm dsandersllvm Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'll see if I can measure it. I doubt it's noticeable for interactive use but it might add up in scripts

The LangOptions never change right?

At the moment they don't but I think that's a bug so I didn't add the call_once wrapper to it. I think they ought to depend on the active stack frame. For example:
target variable Class::requires
ought to work in a C++17 stack frame but not a C++20 stack frame. I'm not sure if it should work in an ObjC frame, I don't think it would at the moment but it's only supposed to be a keyword when it follows an @.

Along the same lines, if you have a C++20 frame call an OpenCL frame, this (admittedly contrived) example
target variable Class::_Atomic
should work in the OpenCL frame but not the C++20 frame. The reason I call this one contrived is because identifiers matching /^_[A-Z]/ are reserved by the standard.

clang::Token token;
for (lexer.LexFromRawLexer(token); !token.is(clang::tok::eof);
lexer.LexFromRawLexer(token)) {
Expand Down
Loading