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

[Simon Atanasyan via Phabricator via llvm-commits]

atanasyan marked an inline comment as done.
atanasyan 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:
> 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.
I like an idea to use a helper structure, but unfortunately we cannot make `parseBfdName` a static helper because it calls private `ScriptParser::next()`.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54920