shafik updated this revision to Diff 252941.
shafik marked 4 inline comments as done.
shafik added a comment.
Addressing comments:
- Adding more detailed comments
- Adding test for cases that currently fail b/c we don't enable C++20
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76168/new/
https://reviews.llvm.org/D76168
Files:
lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===================================================================
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@
"std::vector<Class, std::allocator<Class>>",
"_M_emplace_back_aux<Class const&>"},
{"`anonymous namespace'::foo", "`anonymous namespace'", "foo"},
- {"`operator<<A>'::`2'::B<0>::operator>",
- "`operator<<A>'::`2'::B<0>",
+ {"`operator<<A>'::`2'::B<0>::operator>", "`operator<<A>'::`2'::B<0>",
"operator>"},
{"`anonymous namespace'::S::<<::__l2::Foo",
- "`anonymous namespace'::S::<<::__l2",
- "Foo"}};
+ "`anonymous namespace'::S::<<::__l2", "Foo"},
+ // These cases are idiosyncratic to how we generate debug info for names
+ // when we have template parameters. They are not valid C++ names but if
+ // we fix this we need to support them for older compilers.
+ {"A::operator><A::B>", "A", "operator><A::B>"},
+ {"operator><A::B>", "", "operator><A::B>"},
+ {"A::operator<<A::B>", "A", "operator<<A::B>"},
+ {"operator<<A::B>", "", "operator<<A::B>"},
+ {"A::operator<<<A::B>", "A", "operator<<<A::B>"},
+ {"operator<<<A::B>", "", "operator<<<A::B>"},
+ };
llvm::StringRef context, basename;
for (const auto &test : test_cases) {
@@ -169,6 +177,12 @@
"abc::", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"f<A<B><C>>", context, basename));
+
+ // We expect these cases to fail until we turn on C++2a
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "A::operator<=><A::B>", context, basename));
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "operator<=><A::B>", context, basename));
}
static std::set<std::string> FindAlternate(llvm::StringRef Name) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,37 @@
}
const auto &token = Peek();
+
+ // When we generate debug info we add template parameters to names.
+ // Since we don't add a space between the name and the template parameter in
+ // some cases we are not generating valid C++ names e.g.:
+ //
+ // operator<<A::B>
+ //
+ // In some cases we with then parse incorrectly. This fixes the issue by
+ // detecting this case and inserting tok::less in place of tok::lessless
+ // and returning successfully that we consumed the operator.
+ if (token.getKind() == tok::lessless) {
+ // Make sure we have more tokens before attempting to look ahead one more
+ if (m_next_token_index + 1 < m_tokens.size()) {
+ // Look ahead two tokens
+ clang::Token n_token = m_tokens[m_next_token_index + 1];
+ // If we find ( or < then this is indeed operator<< no need for fix
+ if (n_token.getKind() != tok::l_paren && n_token.getKind() != tok::less)
{
+ clang::Token tmp_tok;
+
+ tmp_tok.setLength(1);
+ tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
+ tmp_tok.setKind(tok::less);
+
+ m_tokens[m_next_token_index] = tmp_tok;
+
+ start_position.Remove();
+ return true;
+ }
+ }
+ }
+
switch (token.getKind()) {
case tok::kw_new:
case tok::kw_delete:
Index: lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
===================================================================
--- lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
+++ lldb/unittests/Language/CPlusPlus/CPlusPlusLanguageTest.cpp
@@ -140,12 +140,20 @@
"std::vector<Class, std::allocator<Class>>",
"_M_emplace_back_aux<Class const&>"},
{"`anonymous namespace'::foo", "`anonymous namespace'", "foo"},
- {"`operator<<A>'::`2'::B<0>::operator>",
- "`operator<<A>'::`2'::B<0>",
+ {"`operator<<A>'::`2'::B<0>::operator>", "`operator<<A>'::`2'::B<0>",
"operator>"},
{"`anonymous namespace'::S::<<::__l2::Foo",
- "`anonymous namespace'::S::<<::__l2",
- "Foo"}};
+ "`anonymous namespace'::S::<<::__l2", "Foo"},
+ // These cases are idiosyncratic to how we generate debug info for names
+ // when we have template parameters. They are not valid C++ names but if
+ // we fix this we need to support them for older compilers.
+ {"A::operator><A::B>", "A", "operator><A::B>"},
+ {"operator><A::B>", "", "operator><A::B>"},
+ {"A::operator<<A::B>", "A", "operator<<A::B>"},
+ {"operator<<A::B>", "", "operator<<A::B>"},
+ {"A::operator<<<A::B>", "A", "operator<<<A::B>"},
+ {"operator<<<A::B>", "", "operator<<<A::B>"},
+ };
llvm::StringRef context, basename;
for (const auto &test : test_cases) {
@@ -169,6 +177,12 @@
"abc::", context, basename));
EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
"f<A<B><C>>", context, basename));
+
+ // We expect these cases to fail until we turn on C++2a
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "A::operator<=><A::B>", context, basename));
+ EXPECT_FALSE(CPlusPlusLanguage::ExtractContextAndIdentifier(
+ "operator<=><A::B>", context, basename));
}
static std::set<std::string> FindAlternate(llvm::StringRef Name) {
Index: lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
===================================================================
--- lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
+++ lldb/source/Plugins/Language/CPlusPlus/CPlusPlusNameParser.cpp
@@ -329,6 +329,37 @@
}
const auto &token = Peek();
+
+ // When we generate debug info we add template parameters to names.
+ // Since we don't add a space between the name and the template parameter in
+ // some cases we are not generating valid C++ names e.g.:
+ //
+ // operator<<A::B>
+ //
+ // In some cases we with then parse incorrectly. This fixes the issue by
+ // detecting this case and inserting tok::less in place of tok::lessless
+ // and returning successfully that we consumed the operator.
+ if (token.getKind() == tok::lessless) {
+ // Make sure we have more tokens before attempting to look ahead one more
+ if (m_next_token_index + 1 < m_tokens.size()) {
+ // Look ahead two tokens
+ clang::Token n_token = m_tokens[m_next_token_index + 1];
+ // If we find ( or < then this is indeed operator<< no need for fix
+ if (n_token.getKind() != tok::l_paren && n_token.getKind() != tok::less) {
+ clang::Token tmp_tok;
+
+ tmp_tok.setLength(1);
+ tmp_tok.setLocation(token.getLocation().getLocWithOffset(1));
+ tmp_tok.setKind(tok::less);
+
+ m_tokens[m_next_token_index] = tmp_tok;
+
+ start_position.Remove();
+ return true;
+ }
+ }
+ }
+
switch (token.getKind()) {
case tok::kw_new:
case tok::kw_delete:
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
