[PATCH] D17499: [ELF] - replaced std::function with raw pointers in LinkerScript.cpp

[Rui Ueyama via llvm-commits]

On Mon, Feb 22, 2016 at 2:08 AM, George Rimar <grimar at accesssoftek.com>
wrote:

> grimar created this revision.
> grimar added reviewers: dblaikie, ruiu, rafael.
> grimar added subscribers: llvm-commits, grimar.
>
> Little change as was proposed by David Blaikie.
>
> Rui, Rafal, what do you think ?
>
>
> http://reviews.llvm.org/D17499
>
> Files:
>   ELF/LinkerScript.cpp
>
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -83,21 +83,11 @@
>  }
>
>  class elf2::ScriptParser {
> +  typedef void (ScriptParser::*CmdHandler)();
>

I'd name just Handler.


> +
>  public:
>    ScriptParser(BumpPtrAllocator *A, StringRef S, bool B)
> -      : Saver(*A), Tokens(tokenize(S)), IsUnderSysroot(B) {
> -    Cmd["ENTRY"] = std::mem_fn(&ScriptParser::readEntry);
> -    Cmd["EXTERN"] = std::mem_fn(&ScriptParser::readExtern);
> -    Cmd["GROUP"] = std::mem_fn(&ScriptParser::readGroup);
> -    Cmd["INCLUDE"] = std::mem_fn(&ScriptParser::readInclude);
> -    Cmd["INPUT"] = std::mem_fn(&ScriptParser::readGroup);
> -    Cmd["OUTPUT"] = std::mem_fn(&ScriptParser::readOutput);
> -    Cmd["OUTPUT_ARCH"] = std::mem_fn(&ScriptParser::readOutputArch);
> -    Cmd["OUTPUT_FORMAT"] = std::mem_fn(&ScriptParser::readOutputFormat);
> -    Cmd["SEARCH_DIR"] = std::mem_fn(&ScriptParser::readSearchDir);
> -    Cmd["SECTIONS"] = std::mem_fn(&ScriptParser::readSections);
> -    Cmd[";"] = [](ScriptParser &) {};
> -  }
> +      : Saver(*A), Tokens(tokenize(S)), IsUnderSysroot(B) {}
>
>    void run();
>
> @@ -127,7 +117,18 @@
>
>    StringSaver Saver;
>    std::vector<StringRef> Tokens;
> -  llvm::StringMap<std::function<void(ScriptParser &)>> Cmd;
> +  llvm::StringMap<CmdHandler> Cmd{
> +      {"ENTRY", &ScriptParser::readEntry},
> +      {"EXTERN", &ScriptParser::readExtern},
> +      {"GROUP", &ScriptParser::readGroup},
> +      {"INCLUDE", &ScriptParser::readInclude},
> +      {"INPUT", &ScriptParser::readGroup},
> +      {"OUTPUT", &ScriptParser::readOutput},
> +      {"OUTPUT_ARCH", &ScriptParser::readOutputArch},
> +      {"OUTPUT_FORMAT", &ScriptParser::readOutputFormat},
> +      {"SEARCH_DIR", &ScriptParser::readSearchDir},
> +      {"SECTIONS", &ScriptParser::readSections},
> +      {";", nullptr}};
>

I think adding a nullptr is too subtle. Can you define readNothing and use
that as a handler for ';'?


>    size_t Pos = 0;
>    bool IsUnderSysroot;
>    bool Error = false;
> @@ -138,8 +139,9 @@
>      StringRef Tok = next();
>      auto It = Cmd.find(Tok);
>      if (It != Cmd.end()) {
> -      std::function<void(ScriptParser &)> &Handler = It->second;
> -      Handler(*this);
> +      CmdHandler Handler = It->second;
> +      if (Handler)
> +        (this->*Handler)();
>

Then you can do

if (Handler Fn = Cmd.lookup(Tok))
 (this->*Fn)();
else
  setError("unknown directive "...);


>      } else {
>        setError("unknown directive: " + Tok);
>      }
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160222/92b1e79b/attachment.html>