[PATCH] D28911: [LinkerScript] Implement `MEMORY` command

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 23 01:39:04 PST 2017


grimar added inline comments.


================
Comment at: ELF/LinkerScript.cpp:2055
+    uint32_t Flag = 0;
+    switch (C) {
+    case '!':
----------------
I would probably split this into separate function and rewrite to something like:

```

static uint32_t convertToFlags(char Attr) {
  if (Attr == 'w' || Attr == 'W')
    return SHF_WRITE;
  if (Attr == 'x' || Attr == 'X')
    return SHF_EXECINSTR;
  if (Attr == 'a' || Attr == 'A')
    return SHF_ALLOC;
  if (Attr != 'r' && Attr != 'R')
    setError("invalid memory region attribute: " + Attr);
return 0;
} 

std::pair<uint32_t, uint32_t> ScriptParser::readMemoryAttributes() {
  uint32_t Flags = 0;
  uint32_t NotFlags = 0;
  bool Invert = false;
  for (auto C : next()) {
    if (C == '!)
      Invert = !Invert;
    else if (Invert)
      NotFlags |= convertToFlags(C);
    else
      Flags |= convertToFlags(C);
  }
  return {Flags, NotFlags};
}

```


================
Comment at: ELF/LinkerScript.cpp:2078
+    }
+    if (Error)
+      break;
----------------
Looks you do not really need that break.


https://reviews.llvm.org/D28911





More information about the llvm-commits mailing list