Page MenuHomePhabricator

[ELF] - Fix (partial) for bug 28843 - Make sure we handle options with opposing meanings.
ClosedPublic

Authored by grimar on Aug 25 2016, 5:37 AM.

Details

Summary

As stated in PR28843:

we should handle command lines with

  • -target1-rel -target1-abs
  • --demangle --no-demangle

Patch implements this for specified options.
There are also others conflicting options, so fix is "partial".

Diff Detail

Repository
rL LLVM

Event Timeline

grimar updated this revision to Diff 69226.Aug 25 2016, 5:37 AM
grimar retitled this revision from to [ELF] - Fix (partial) for bug 28843 - Make sure we handle options with opposing meanings..
grimar updated this object.
grimar added reviewers: ruiu, rafael.
grimar added a subscriber: llvm-commits.
ruiu added inline comments.Aug 29 2016, 3:24 PM
ELF/Driver.cpp
347–357 ↗(On Diff #69226)

I can see the same pattern here. How about this?

static bool getArg(opt::InputArgList &Args, unsigned K1, unsigned K2, bool Default) {
  if (auto *Arg = Args.getLastArg(K1, K2))
    return Arg->getOption().getID() == K1;
  return Default;
}
grimar added inline comments.Aug 30 2016, 7:40 AM
ELF/Driver.cpp
347–357 ↗(On Diff #69226)

I tried the same when wrote this. Finished with my version because find it slightly more readable (too much arguments imo).
No real preference here though. Lets go with your version if you prefer it.

grimar updated this revision to Diff 69680.Aug 30 2016, 7:46 AM
  • Addressed comments.

I think I still slightly prefer diff1 :)

rafael accepted this revision.Aug 30 2016, 10:56 AM
rafael edited edge metadata.

Second one LGTM. Maybe just rename K1 and K2 to more descriptive names.

This revision is now accepted and ready to land.Aug 30 2016, 10:56 AM
ruiu accepted this revision.Aug 30 2016, 12:45 PM
ruiu edited edge metadata.

LGTM

This revision was automatically updated to reflect the committed changes.