[PATCH] D88810: [clangd] Add isKeyword function.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1425c7223676: [clangd] Add isKeyword function. (authored by
hokein).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88810/new/
https://reviews.llvm.org/D88810
Files:
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
}
+TEST(SourceCodeTests, isKeywords) {
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus20 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
+ EXPECT_TRUE(isKeyword("return", LangOpts));
+ EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+ // these are identifiers (not keywords!) with special meaning in some
+ // contexts.
+ EXPECT_FALSE(isKeyword("final", LangOpts));
+ EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
const LangOptions );
};
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions );
+
/// Heuristically determine namespaces visible at a point, without parsing
Code.
/// This considers using-directives and enclosing namespace-declarations that
/// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
return Ranges;
}
+bool isKeyword(llvm::StringRef NewName, const LangOptions ) {
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
namespace {
struct NamespaceEvent {
enum {
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
}
+TEST(SourceCodeTests, isKeywords) {
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus20 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
+ EXPECT_TRUE(isKeyword("return", LangOpts));
+ EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+ // these are identifiers (not keywords!) with special meaning in some
+ // contexts.
+ EXPECT_FALSE(isKeyword("final", LangOpts));
+ EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
const LangOptions );
};
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions );
+
/// Heuristically determine namespaces visible at a point, without parsing Code.
/// This considers using-directives and enclosing namespace-declarations that
/// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
return Ranges;
}
+bool isKeyword(llvm::StringRef NewName, const LangOptions ) {
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
namespace {
struct NamespaceEvent {
enum {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88810: [clangd] Add isKeyword function.
hokein added inline comments. Comment at: clang-tools-extra/clangd/SourceCode.cpp:638 + // Keywords are initialized in constructor. + clang::IdentifierTable KeywordsTable(LangOpts); + return KeywordsTable.find(NewName) != KeywordsTable.end(); sammccall wrote: > this builds a pretty decent sized hashtable for each call, because LangOpts > isn't constant. > > We could consider exposing getKeywordStatus from IdentifierTable.h, then we > could build a StringMap (once, statically) and then call > getKeywordStatus to apply the per-langopts logic. > In fact that StringMap might also be generally nice to expose in > TokenKinds.h... as long as there are no conflicts. > > Or we could have cache map from LangOpts to IdentifierTable, since we won't > see many sets in practice. > > Or maybe it doesn't matter, I suppose! > Or maybe it doesn't matter, I suppose! this is my impression as well, there are ~120 keywords for C++, I think the number is not too big. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88810/new/ https://reviews.llvm.org/D88810 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88810: [clangd] Add isKeyword function.
hokein updated this revision to Diff 296168.
hokein marked an inline comment as done.
hokein added a comment.
address code comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88810/new/
https://reviews.llvm.org/D88810
Files:
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
}
+TEST(SourceCodeTests, isKeywords) {
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus20 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
+ EXPECT_TRUE(isKeyword("return", LangOpts));
+ EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+ // these are identifiers (not keywords!) with special meaning in some
+ // contexts.
+ EXPECT_FALSE(isKeyword("final", LangOpts));
+ EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
const LangOptions );
};
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions );
+
/// Heuristically determine namespaces visible at a point, without parsing
Code.
/// This considers using-directives and enclosing namespace-declarations that
/// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
return Ranges;
}
+bool isKeyword(llvm::StringRef NewName, const LangOptions ) {
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
namespace {
struct NamespaceEvent {
enum {
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,19 @@
EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
}
+TEST(SourceCodeTests, isKeywords) {
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus20 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
+ EXPECT_TRUE(isKeyword("return", LangOpts));
+ EXPECT_TRUE(isKeyword("co_await", LangOpts));
+
+ // these are identifiers (not keywords!) with special meaning in some
+ // contexts.
+ EXPECT_FALSE(isKeyword("final", LangOpts));
+ EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
const LangOptions );
};
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions );
+
/// Heuristically determine namespaces visible at a point, without parsing Code.
/// This considers using-directives and enclosing namespace-declarations that
/// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
return Ranges;
}
+bool isKeyword(llvm::StringRef NewName, const LangOptions ) {
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
namespace {
struct NamespaceEvent {
enum {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88810: [clangd] Add isKeyword function.
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: clang-tools-extra/clangd/SourceCode.cpp:638
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
this builds a pretty decent sized hashtable for each call, because LangOpts
isn't constant.
We could consider exposing getKeywordStatus from IdentifierTable.h, then we
could build a StringMap (once, statically) and then call
getKeywordStatus to apply the per-langopts logic.
In fact that StringMap might also be generally nice to expose in
TokenKinds.h... as long as there are no conflicts.
Or we could have cache map from LangOpts to IdentifierTable, since we won't see
many sets in practice.
Or maybe it doesn't matter, I suppose!
Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:794
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus11 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
demonstrate some sensitivity to langopts?
e.g. set CPlusPlus20 to true then check co_await
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88810/new/
https://reviews.llvm.org/D88810
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D88810: [clangd] Add isKeyword function.
hokein created this revision.
hokein added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: clang.
hokein requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.
This will be used in rename for doing basic name validation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D88810
Files:
clang-tools-extra/clangd/SourceCode.cpp
clang-tools-extra/clangd/SourceCode.h
clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,18 @@
EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
}
+TEST(SourceCodeTests, isKeywords) {
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus11 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
+ EXPECT_TRUE(isKeyword("return", LangOpts));
+
+ // these are identifiers (not keywords!) with special meaning in some
+ // contexts.
+ EXPECT_FALSE(isKeyword("final", LangOpts));
+ EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
const LangOptions );
};
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions );
+
/// Heuristically determine namespaces visible at a point, without parsing
Code.
/// This considers using-directives and enclosing namespace-declarations that
/// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
return Ranges;
}
+bool isKeyword(llvm::StringRef NewName, const LangOptions ) {
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
namespace {
struct NamespaceEvent {
enum {
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -789,6 +789,18 @@
EXPECT_TRUE(isHeaderFile("header.h", LangOpts));
}
+TEST(SourceCodeTests, isKeywords) {
+ LangOptions LangOpts;
+ LangOpts.CPlusPlus11 = true;
+ EXPECT_TRUE(isKeyword("int", LangOpts));
+ EXPECT_TRUE(isKeyword("return", LangOpts));
+
+ // these are identifiers (not keywords!) with special meaning in some
+ // contexts.
+ EXPECT_FALSE(isKeyword("final", LangOpts));
+ EXPECT_FALSE(isKeyword("override", LangOpts));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -248,6 +248,10 @@
const LangOptions );
};
+/// Return true if the \p TokenName is in the list of reversed keywords of the
+/// language.
+bool isKeyword(llvm::StringRef TokenName, const LangOptions );
+
/// Heuristically determine namespaces visible at a point, without parsing Code.
/// This considers using-directives and enclosing namespace-declarations that
/// are visible (and not obfuscated) in the file itself (not headers).
Index: clang-tools-extra/clangd/SourceCode.cpp
===
--- clang-tools-extra/clangd/SourceCode.cpp
+++ clang-tools-extra/clangd/SourceCode.cpp
@@ -633,6 +633,12 @@
return Ranges;
}
+bool isKeyword(llvm::StringRef NewName, const LangOptions ) {
+ // Keywords are initialized in constructor.
+ clang::IdentifierTable KeywordsTable(LangOpts);
+ return KeywordsTable.find(NewName) != KeywordsTable.end();
+}
+
namespace {
struct NamespaceEvent {
enum {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
