[PATCH] D54920: [ELF][MIPS] Handle mips in the OUTPUT_FORMAT directive
Simon Atanasyan via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 27 02:06:29 PST 2018
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
More information about the llvm-commits
mailing list