Skip to content

Commit 29e9f26

Browse files
cmticecmtice
committed
Address more review comments:
- Use std::move on std::string & std::vector in constructor initializers. - Remove some unnecessary code. - Update ExtractTokenData (helper function in unit tests) to set up the lexer and to the lexing inside the function; return an llvm::Expected value. - Add 'using namespace lldb_private::dil;' to unit tests; clean up tests accordingly. - Minor code cleanups in the unit tests.
1 parent ccf5203 commit 29e9f26

File tree

3 files changed

+72
-109
lines changed

3 files changed

+72
-109
lines changed

‎lldb/include/lldb/ValueObject/DILLexer.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class Token {
3434
};
3535

3636
Token(Kind kind, std::string spelling, uint32_t start)
37-
: m_kind(kind), m_spelling(spelling), m_start_pos(start) {}
37+
: m_kind(kind), m_spelling(std::move(spelling)), m_start_pos(start) {}
3838

3939
Kind GetKind() const { return m_kind; }
4040

@@ -88,7 +88,8 @@ class DILLexer {
8888
if (m_tokens_idx + N < m_lexed_tokens.size())
8989
return m_lexed_tokens[m_tokens_idx + N];
9090

91-
return m_eof_token;
91+
// Last token should be an 'eof' token.
92+
return m_lexed_tokens.back();
9293
}
9394

9495
/// Return the index for the 'current' token being handled by the DIL parser.
@@ -106,8 +107,8 @@ class DILLexer {
106107

107108
private:
108109
DILLexer(llvm::StringRef dil_expr, std::vector<Token> lexed_tokens)
109-
: m_expr(dil_expr), m_lexed_tokens(lexed_tokens), m_tokens_idx(UINT_MAX),
110-
m_eof_token(Token(Token::eof, "", 0)) {}
110+
: m_expr(dil_expr), m_lexed_tokens(std::move(lexed_tokens)),
111+
m_tokens_idx(UINT_MAX), m_eof_token(Token(Token::eof, "", 0)) {}
111112

112113
static llvm::Expected<Token> Lex(llvm::StringRef expr,
113114
llvm::StringRef &remainder);

‎lldb/source/ValueObject/DILLexer.cpp‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -108,21 +108,17 @@ llvm::Expected<Token> DILLexer::Lex(llvm::StringRef expr,
108108

109109
uint32_t position = cur_pos - expr.begin();
110110
std::optional<llvm::StringRef> maybe_word = IsWord(expr, remainder);
111-
if (maybe_word) {
112-
llvm::StringRef word = *maybe_word;
113-
return Token(Token::identifier, word.str(), position);
114-
}
111+
if (maybe_word)
112+
return Token(Token::identifier, maybe_word->str(), position);
115113

116114
constexpr std::pair<Token::Kind, const char *> operators[] = {
117115
{Token::l_paren, "("},
118116
{Token::r_paren, ")"},
119117
{Token::coloncolon, "::"},
120118
};
121119
for (auto [kind, str] : operators) {
122-
if (remainder.consume_front(str)) {
123-
cur_pos += strlen(str);
120+
if (remainder.consume_front(str))
124121
return Token(kind, str, position);
125-
}
126122
}
127123

128124
// Unrecognized character(s) in string; unable to lex it.

‎lldb/unittests/ValueObject/DILLexerTests.cpp‎

Lines changed: 64 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -14,179 +14,145 @@
1414

1515
using llvm::StringRef;
1616

17-
std::vector<std::pair<lldb_private::dil::Token::Kind, std::string>>
18-
ExtractTokenData(lldb_private::dil::DILLexer &lexer) {
19-
std::vector<std::pair<lldb_private::dil::Token::Kind, std::string>> data;
17+
using namespace lldb_private::dil;
18+
19+
llvm::Expected<std::vector<std::pair<Token::Kind, std::string>>>
20+
ExtractTokenData(llvm::StringRef input_expr) {
21+
22+
llvm::Expected<DILLexer> maybe_lexer = DILLexer::Create(input_expr);
23+
if (!maybe_lexer)
24+
return maybe_lexer.takeError();
25+
DILLexer lexer(*maybe_lexer);
26+
2027
if (lexer.NumLexedTokens() == 0)
21-
return data;
28+
return llvm::createStringError("No lexed tokens");
2229

2330
lexer.ResetTokenIdx(UINT_MAX);
31+
std::vector<std::pair<Token::Kind, std::string>> data;
2432
do {
2533
lexer.Advance();
26-
lldb_private::dil::Token tok = lexer.GetCurrentToken();
34+
Token tok = lexer.GetCurrentToken();
2735
data.push_back(std::make_pair(tok.GetKind(), tok.GetSpelling()));
28-
} while (data.back().first != lldb_private::dil::Token::eof);
36+
} while (data.back().first != Token::eof);
2937
return data;
3038
}
3139

3240
TEST(DILLexerTests, SimpleTest) {
3341
StringRef input_expr("simple_var");
34-
uint32_t tok_len = 10;
35-
llvm::Expected<lldb_private::dil::DILLexer> maybe_lexer =
36-
lldb_private::dil::DILLexer::Create(input_expr);
42+
llvm::Expected<DILLexer> maybe_lexer = DILLexer::Create(input_expr);
3743
ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
38-
lldb_private::dil::DILLexer lexer(*maybe_lexer);
39-
lldb_private::dil::Token token =
40-
lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
41-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::unknown);
44+
DILLexer lexer(*maybe_lexer);
45+
Token token = Token(Token::unknown, "", 0);
46+
EXPECT_EQ(token.GetKind(), Token::unknown);
4247

4348
lexer.Advance();
4449
token = lexer.GetCurrentToken();
45-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
50+
EXPECT_EQ(token.GetKind(), Token::identifier);
4651
EXPECT_EQ(token.GetSpelling(), "simple_var");
47-
EXPECT_EQ(token.GetSpelling().size(), tok_len);
4852
lexer.Advance();
4953
token = lexer.GetCurrentToken();
50-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
54+
EXPECT_EQ(token.GetKind(), Token::eof);
5155
}
5256

5357
TEST(DILLexerTests, TokenKindTest) {
54-
StringRef input_expr("namespace");
55-
llvm::Expected<lldb_private::dil::DILLexer> maybe_lexer =
56-
lldb_private::dil::DILLexer::Create(input_expr);
57-
ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
58-
lldb_private::dil::DILLexer lexer(*maybe_lexer);
59-
lldb_private::dil::Token token =
60-
lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
61-
lexer.Advance();
62-
token = lexer.GetCurrentToken();
58+
Token token = Token(Token::identifier, "ident", 0);
6359

64-
EXPECT_TRUE(token.Is(lldb_private::dil::Token::identifier));
65-
EXPECT_FALSE(token.Is(lldb_private::dil::Token::l_paren));
66-
EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::eof,
67-
lldb_private::dil::Token::identifier));
68-
EXPECT_FALSE(token.IsOneOf(
69-
lldb_private::dil::Token::l_paren, lldb_private::dil::Token::r_paren,
70-
lldb_private::dil::Token::coloncolon, lldb_private::dil::Token::eof));
60+
EXPECT_TRUE(token.Is(Token::identifier));
61+
EXPECT_FALSE(token.Is(Token::l_paren));
62+
EXPECT_TRUE(token.IsOneOf(Token::eof, Token::identifier));
63+
EXPECT_FALSE(token.IsOneOf(Token::l_paren, Token::r_paren, Token::coloncolon,
64+
Token::eof));
7165
}
7266

7367
TEST(DILLexerTests, LookAheadTest) {
7468
StringRef input_expr("(anonymous namespace)::some_var");
75-
llvm::Expected<lldb_private::dil::DILLexer> maybe_lexer =
76-
lldb_private::dil::DILLexer::Create(input_expr);
69+
llvm::Expected<DILLexer> maybe_lexer = DILLexer::Create(input_expr);
7770
ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
78-
lldb_private::dil::DILLexer lexer(*maybe_lexer);
79-
lldb_private::dil::Token token =
80-
lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
81-
uint32_t expect_loc = 23;
71+
DILLexer lexer(*maybe_lexer);
72+
Token token = Token(Token::unknown, "", 0);
8273
lexer.Advance();
8374
token = lexer.GetCurrentToken();
8475

8576
// Current token is '('; check the next 4 tokens, to make
8677
// sure they are the identifier 'anonymous', the identifier 'namespace'
8778
// ')' and '::', in that order.
88-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::l_paren);
89-
EXPECT_EQ(lexer.LookAhead(1).GetKind(), lldb_private::dil::Token::identifier);
79+
EXPECT_EQ(token.GetKind(), Token::l_paren);
80+
EXPECT_EQ(lexer.LookAhead(1).GetKind(), Token::identifier);
9081
EXPECT_EQ(lexer.LookAhead(1).GetSpelling(), "anonymous");
91-
EXPECT_EQ(lexer.LookAhead(2).GetKind(), lldb_private::dil::Token::identifier);
82+
EXPECT_EQ(lexer.LookAhead(2).GetKind(), Token::identifier);
9283
EXPECT_EQ(lexer.LookAhead(2).GetSpelling(), "namespace");
93-
EXPECT_EQ(lexer.LookAhead(3).GetKind(), lldb_private::dil::Token::r_paren);
94-
EXPECT_EQ(lexer.LookAhead(4).GetKind(), lldb_private::dil::Token::coloncolon);
84+
EXPECT_EQ(lexer.LookAhead(3).GetKind(), Token::r_paren);
85+
EXPECT_EQ(lexer.LookAhead(4).GetKind(), Token::coloncolon);
9586

9687
// Our current index should still be 0, as we only looked ahead; we are still
9788
// officially on the '('.
98-
EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)0);
89+
EXPECT_EQ(lexer.GetCurrentTokenIdx(), 0u);
9990

10091
// Accept the 'lookahead', so our current token is '::', which has the index
10192
// 4 in our vector of tokens (which starts at zero).
10293
lexer.Advance(4);
10394
token = lexer.GetCurrentToken();
104-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::coloncolon);
105-
EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)4);
95+
EXPECT_EQ(token.GetKind(), Token::coloncolon);
96+
EXPECT_EQ(lexer.GetCurrentTokenIdx(), 4u);
10697

10798
lexer.Advance();
10899
token = lexer.GetCurrentToken();
109-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
100+
EXPECT_EQ(token.GetKind(), Token::identifier);
110101
EXPECT_EQ(token.GetSpelling(), "some_var");
111-
EXPECT_EQ(lexer.GetCurrentTokenIdx(), (uint32_t)5);
112-
// Verify we've advanced our position counter (lexing location) in the
113-
// input 23 characters (the length of '(anonymous namespace)::'.
114-
EXPECT_EQ(token.GetLocation(), expect_loc);
102+
EXPECT_EQ(lexer.GetCurrentTokenIdx(), 5u);
103+
EXPECT_EQ(token.GetLocation(), strlen("(anonymous namespace)::"));
115104

116105
lexer.Advance();
117106
token = lexer.GetCurrentToken();
118-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
107+
EXPECT_EQ(token.GetKind(), Token::eof);
119108
}
120109

121110
TEST(DILLexerTests, MultiTokenLexTest) {
122-
StringRef input_expr("This string has (several ) ::identifiers");
123-
llvm::Expected<lldb_private::dil::DILLexer> maybe_lexer =
124-
lldb_private::dil::DILLexer::Create(input_expr);
125-
ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
126-
lldb_private::dil::DILLexer lexer(*maybe_lexer);
127-
lldb_private::dil::Token token =
128-
lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
129-
130-
std::vector<std::pair<lldb_private::dil::Token::Kind, std::string>>
131-
lexer_tokens_data = ExtractTokenData(lexer);
132-
133-
EXPECT_THAT(
134-
lexer_tokens_data,
135-
testing::ElementsAre(
136-
testing::Pair(lldb_private::dil::Token::identifier, "This"),
137-
testing::Pair(lldb_private::dil::Token::identifier, "string"),
138-
testing::Pair(lldb_private::dil::Token::identifier, "has"),
139-
testing::Pair(lldb_private::dil::Token::l_paren, "("),
140-
testing::Pair(lldb_private::dil::Token::identifier, "several"),
141-
testing::Pair(lldb_private::dil::Token::r_paren, ")"),
142-
testing::Pair(lldb_private::dil::Token::coloncolon, "::"),
143-
testing::Pair(lldb_private::dil::Token::identifier, "identifiers"),
144-
testing::Pair(lldb_private::dil::Token::eof, "")));
145-
lexer.Advance();
146-
token = lexer.GetCurrentToken();
147-
EXPECT_EQ(token.GetSpelling(), "");
148-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::eof);
111+
EXPECT_THAT_EXPECTED(
112+
ExtractTokenData("This string has (several ) ::identifiers"),
113+
llvm::HasValue(
114+
testing::ElementsAre(testing::Pair(Token::identifier, "This"),
115+
testing::Pair(Token::identifier, "string"),
116+
testing::Pair(Token::identifier, "has"),
117+
testing::Pair(Token::l_paren, "("),
118+
testing::Pair(Token::identifier, "several"),
119+
testing::Pair(Token::r_paren, ")"),
120+
testing::Pair(Token::coloncolon, "::"),
121+
testing::Pair(Token::identifier, "identifiers"),
122+
testing::Pair(Token::eof, ""))));
149123
}
150124

151125
TEST(DILLexerTests, IdentifiersTest) {
152126
std::vector<std::string> valid_identifiers = {
153-
"$My_name1", "$pc", "abcd", "ab cd", "_", "_a", "_a_",
154-
"a_b", "this", "self", "a", "MyName", "namespace"};
127+
"$My_name1", "$pc", "abcd", "_", "_a", "_a_",
128+
"a_b", "this", "self", "a", "MyName", "namespace"};
155129
std::vector<std::string> invalid_identifiers = {"234", "2a", "2",
156130
"$", "1MyName", ""};
157131

158132
// Verify that all of the valid identifiers come out as identifier tokens.
159133
for (auto &str : valid_identifiers) {
160134
SCOPED_TRACE(str);
161-
llvm::Expected<lldb_private::dil::DILLexer> maybe_lexer =
162-
lldb_private::dil::DILLexer::Create(str);
163-
ASSERT_THAT_EXPECTED(maybe_lexer, llvm::Succeeded());
164-
lldb_private::dil::DILLexer lexer(*maybe_lexer);
165-
lldb_private::dil::Token token =
166-
lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
167-
lexer.Advance();
168-
token = lexer.GetCurrentToken();
169-
EXPECT_EQ(token.GetKind(), lldb_private::dil::Token::identifier);
135+
EXPECT_THAT_EXPECTED(ExtractTokenData(str),
136+
llvm::HasValue(testing::ElementsAre(
137+
testing::Pair(Token::identifier, str),
138+
testing::Pair(Token::eof, ""))));
170139
}
171140

172141
// Verify that none of the invalid identifiers come out as identifier tokens.
173142
for (auto &str : invalid_identifiers) {
174143
SCOPED_TRACE(str);
175-
llvm::Expected<lldb_private::dil::DILLexer> maybe_lexer =
176-
lldb_private::dil::DILLexer::Create(str);
144+
llvm::Expected<DILLexer> maybe_lexer = DILLexer::Create(str);
177145
if (!maybe_lexer) {
178146
llvm::consumeError(maybe_lexer.takeError());
179147
// In this case, it's ok for lexing to return an error.
180148
} else {
181-
lldb_private::dil::DILLexer lexer(*maybe_lexer);
182-
lldb_private::dil::Token token =
183-
lldb_private::dil::Token(lldb_private::dil::Token::unknown, "", 0);
149+
DILLexer lexer(*maybe_lexer);
150+
Token token = Token(Token::unknown, "", 0);
184151
// We didn't get an error; make sure we did not get an identifier token.
185152
lexer.Advance();
186153
token = lexer.GetCurrentToken();
187-
EXPECT_TRUE(token.IsNot(lldb_private::dil::Token::identifier));
188-
EXPECT_TRUE(token.IsOneOf(lldb_private::dil::Token::unknown,
189-
lldb_private::dil::Token::eof));
154+
EXPECT_TRUE(token.IsNot(Token::identifier));
155+
EXPECT_TRUE(token.IsOneOf(Token::unknown, Token::eof));
190156
}
191157
}
192158
}

0 commit comments

Comments
 (0)