[PATCH] D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 27 01:10:59 PST 2018


grimar added inline comments.


================
Comment at: ELF/ScriptParser.cpp:388
 
-std::pair<ELFKind, uint16_t> ScriptParser::readBfdName() {
+std::tuple<ELFKind, uint16_t, bool> ScriptParser::readBfdName() {
   StringRef S = unquote(next());
----------------
grimar wrote:
> I would suggest doing something like:
> 
> ```
> namespace {
>   struct BfdArchKind {
>     ELFKind BfdEKind;
>     uint16_t BfdEMachine;
>     bool IsMipsN32Abi;
>   };
> };
> 
> static BfdArchKind parseBfdName(StringRef Name) {
> ...
> ```
> 
> i.e. use a struct instead of `std::tuple` and a static helper instead of the member function.
It seems a bit more readable because it is sometimes unobvious what pair/tuple do contain.
Here the third boolean argument made me wonder what is contained there.
Using a struct with named members can help.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54920





More information about the llvm-commits mailing list