Skip to content

[lld][ELF] removed peek2 in linker script lexer #99539

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.

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

Closed
wants to merge 0 commits into from

Conversation

hongyu-dev
Copy link
Contributor

This removes peek2 in lexer and updates peek for a default value ll2 = false unless we need to look ahead 2 tokens in readSectionAddressType.

@llvmbot
Copy link
Member

llvmbot commented Jul 18, 2024

@llvm/pr-subscribers-lld

@llvm/pr-subscribers-lld-elf

Author: Hongyu Chen (yugier)

Changes

This removes peek2 in lexer and updates peek for a default value ll2 = false unless we need to look ahead 2 tokens in readSectionAddressType.


Full diff: https://github.com/llvm/llvm-project/pull/99539.diff

3 Files Affected:

  • (modified) lld/ELF/ScriptLexer.cpp (+4-10)
  • (modified) lld/ELF/ScriptLexer.h (+1-2)
  • (modified) lld/ELF/ScriptParser.cpp (+3-3)
diff --git a/lld/ELF/ScriptLexer.cpp b/lld/ELF/ScriptLexer.cpp
index 14f39ed10e17c..d4f1b11f458fb 100644
--- a/lld/ELF/ScriptLexer.cpp
+++ b/lld/ELF/ScriptLexer.cpp
@@ -264,20 +264,14 @@ StringRef ScriptLexer::next() {
   return tokens[pos++];
 }
 
-StringRef ScriptLexer::peek() {
-  StringRef tok = next();
-  if (errorCount())
-    return "";
-  pos = pos - 1;
-  return tok;
-}
+StringRef ScriptLexer::peek(bool ll2) {
+  if (ll2)
+    skip();
 
-StringRef ScriptLexer::peek2() {
-  skip();
   StringRef tok = next();
   if (errorCount())
     return "";
-  pos = pos - 2;
+  pos = ll2 ? pos - 2 : pos - 1;
   return tok;
 }
 
diff --git a/lld/ELF/ScriptLexer.h b/lld/ELF/ScriptLexer.h
index 7919e493fa28b..7a4c1a5d14576 100644
--- a/lld/ELF/ScriptLexer.h
+++ b/lld/ELF/ScriptLexer.h
@@ -25,8 +25,7 @@ class ScriptLexer {
   StringRef skipSpace(StringRef s);
   bool atEOF();
   StringRef next();
-  StringRef peek();
-  StringRef peek2();
+  StringRef peek(bool ll2 = false);
   void skip();
   bool consume(StringRef tok);
   void expect(StringRef expect);
diff --git a/lld/ELF/ScriptParser.cpp b/lld/ELF/ScriptParser.cpp
index db46263115242..75a0b6b00888e 100644
--- a/lld/ELF/ScriptParser.cpp
+++ b/lld/ELF/ScriptParser.cpp
@@ -898,14 +898,14 @@ bool ScriptParser::readSectionDirective(OutputSection *cmd, StringRef tok1, Stri
 void ScriptParser::readSectionAddressType(OutputSection *cmd) {
   // Temporarily set inExpr to support TYPE=<value> without spaces.
   bool saved = std::exchange(inExpr, true);
-  bool isDirective = readSectionDirective(cmd, peek(), peek2());
+  bool isDirective = readSectionDirective(cmd, peek(), peek(/*ll2=*/true));
   inExpr = saved;
   if (isDirective)
     return;
 
   cmd->addrExpr = readExpr();
-  if (peek() == "(" && !readSectionDirective(cmd, "(", peek2()))
-    setError("unknown section directive: " + peek2());
+  if (peek() == "(" && !readSectionDirective(cmd, "(", peek(/*ll2=*/true)))
+    setError("unknown section directive: " + peek(/*ll2=*/true));
 }
 
 static Expr checkAlignment(Expr e, std::string &loc) {

@MaskRay
Copy link
Member

MaskRay commented Jul 18, 2024

We should not add the peek2 ability to peek. Instead, the call site should check whether the first token is (, and if yes, peek another token to check NOLOAD/COPY/INFO/TYPE. Basically, we move some "(" code from readSectionDirective to the caller.

@MaskRay
Copy link
Member

MaskRay commented Jul 18, 2024

In conventional commit messages, the first word after the tag in the subject is a capitalized verb: [ELF] Remove ...

Some people add [lld] but I don't. I am fine whether or not [lld] is added.

@hongyu-dev
Copy link
Contributor Author

hongyu-dev commented Jul 18, 2024

We should not add the peek2 ability to peek. Instead, the call site should check whether the first token is (, and if yes, peek another token to check NOLOAD/COPY/INFO/TYPE. Basically, we move some "(" code from readSectionDirective to the caller.

I wanted to do this but it seems like once isDirective == false we need call readExpr and readExpr calls readPrimary . If we consume ( for readSectionDirective then we might have to change readExpr and readPrimary.

This would be relatively easier to resolve once we have a more stateful lexer I believe?

@hongyu-dev hongyu-dev closed this Jul 20, 2024
@hongyu-dev hongyu-dev force-pushed the lld-elf-lexer-peek-update branch from d2788c1 to efa833d Compare July 20, 2024 21:38
@hongyu-dev hongyu-dev deleted the lld-elf-lexer-peek-update branch July 20, 2024 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants