[ELF] Remove consumeLabel in ScriptLexer#99567
Conversation
This commit removes `consumeLabel` since we can just use consume function to have the same functionalities.
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: Hongyu Chen (yugier) ChangesThis commit removes Full diff: https://github.com/llvm/llvm-project/pull/99567.diff 3 Files Affected:
diff --git a/lld/ELF/ScriptLexer.cpp b/lld/ELF/ScriptLexer.cpp
index 14f39ed10e17c..5ac1c15043ce6 100644
--- a/lld/ELF/ScriptLexer.cpp
+++ b/lld/ELF/ScriptLexer.cpp
@@ -289,18 +289,6 @@ bool ScriptLexer::consume(StringRef tok) {
return false;
}
-// Consumes Tok followed by ":". Space is allowed between Tok and ":".
-bool ScriptLexer::consumeLabel(StringRef tok) {
- if (consume((tok + ":").str()))
- return true;
- if (tokens.size() >= pos + 2 && tokens[pos] == tok &&
- tokens[pos + 1] == ":") {
- pos += 2;
- return true;
- }
- return false;
-}
-
void ScriptLexer::skip() { (void)next(); }
void ScriptLexer::expect(StringRef expect) {
diff --git a/lld/ELF/ScriptLexer.h b/lld/ELF/ScriptLexer.h
index 7919e493fa28b..b4d2ab8627a91 100644
--- a/lld/ELF/ScriptLexer.h
+++ b/lld/ELF/ScriptLexer.h
@@ -30,7 +30,6 @@ class ScriptLexer {
void skip();
bool consume(StringRef tok);
void expect(StringRef expect);
- bool consumeLabel(StringRef tok);
std::string getCurrentLocation();
MemoryBufferRef getCurrentMB();
diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index db46263115242..830c4f315a5a8 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -1692,11 +1692,11 @@ ScriptParser::readSymbols() {
while (!errorCount()) {
if (consume("}"))
break;
- if (consumeLabel("local")) {
+ if (consume("local:") || (consume("local") && consume(":"))) {
v = &locals;
continue;
}
- if (consumeLabel("global")) {
+ if (consume("global:") || (consume("global") && consume(":"))) {
v = &globals;
continue;
}
|
lld/ELF/ScriptParser.cpp
Outdated
| continue; | ||
| } | ||
| if (consumeLabel("global")) { | ||
| if (consume("global:") || (consume("global") && consume(":"))) { |
There was a problem hiding this comment.
We should check local: and global: below at StringRef tok = next();
There was a problem hiding this comment.
Sorry I am confused here why we need check local: and global below StringRef tok = next(); ? if the lexer didn't consume global:, global : , local:, local : , or extern, we do not need do other things for the symbol right? Please correct me if I understand here wrong. Thank you!
There was a problem hiding this comment.
- Manually inline the two
consumeLabelcalls - Move the expanded body to below, just above
next()
There was a problem hiding this comment.
I committed the change but I think it's not what you were trying to say? If it is not could you please specify it more?
lld/ELF/ScriptParser.cpp
Outdated
| SmallVector<SymbolVersion, 0> ext = readVersionExtern(); | ||
| v->insert(v->end(), ext.begin(), ext.end()); | ||
| } else { | ||
| if (consume("local:") || (consume("local") && consume(":"))) { |
There was a problem hiding this comment.
I think this introduces a bug that a symbol named local or global will not be accepted.
The "local" and "global" name check should be performed after next().
There was a problem hiding this comment.
It makes a lot of sense now thank you very much! I just updated it again.
hongyu-dev
changed the title
consumeLabel in ScriptLexerconsumeLabel in ScriptLexer
Summary: This commit removes `consumeLabel` since we can just use consume function to have the same functionalities. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250691
(cherry picked from commit 026972a)
(cherry picked from commit 026972a)
(cherry picked from commit 026972a)
3 participants
This commit removes
consumeLabelsince we can just use consume function to have the same functionalities.