[PATCH] D72756: [LLD][ELF] Add support for INPUT_SECTION_FLAGS

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/ScriptParser.cpp:709
+    StringRef tok = next();
+    if (tok == "INPUT_SECTION_FLAGS") {
+      std::tie(withFlags, withoutFlags) = readInputSectionFlags();
----------------
I think more natural way is to use `consume`:

```
if (consume("INPUT_SECTION_FLAGS"))
  std::tie(withFlags, withoutFlags) = readInputSectionFlags();

StringRef filePattern = next();
...
```


================
Comment at: lld/ELF/ScriptParser.cpp:804
+      std::tie(withFlags, withoutFlags) = readInputSectionFlags();
+      tok = next();
+    }
----------------
The same.


================
Comment at: lld/ELF/ScriptParser.cpp:863
       readInclude();
+    } else if (tok == "INPUT_SECTION_FLAGS") {
+        std::tie(withFlags, withoutFlags) = readInputSectionFlags();
----------------
We parse "KEEP" in `readInputSectionDescription`, and you handle "INPUT_SECTION_FLAGS" for it right there.
Seems you can remove handling of "INPUT_SECTION_FLAGS" from here and move it to `readInputSectionDescription` too
(for the no-KEEP case).

This should allow avoid having `withFlags = withoutFlags = 0;` code (and variables itself) around.


================
Comment at: lld/ELF/ScriptParser.cpp:1146
+  };
+#undef ENUM_ENT
+  if (llvm::Optional<uint64_t> asInt = parseInt(tok))
----------------
May be use `llvm::StringSwitch` instead?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72756/new/

https://reviews.llvm.org/D72756