Fix #12099: Remove bogus flags
ClosedPublic

Authored by sgillespie on May 29 2016, 11:55 PM.

Details

Summary

Remove -fwarn- and -fno-warn- from flagsForCompletion

Testcase: Fix linter error on T12099

For Issue Trac #12099

Diff Detail

Repository
rGHC Glasgow Haskell Compiler
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sgillespie retitled this revision from to Fix #12099: Remove bogus flags.May 29 2016, 11:55 PM
sgillespie updated this object.
sgillespie edited the test plan for this revision. (Show Details)
sgillespie updated the Trac tickets for this revision.
bgamari accepted this revision.May 30 2016, 2:40 AM

Indeed, good catch. Thanks for the test!

This revision is now accepted and ready to land.May 30 2016, 2:40 AM
thomie added inline comments.May 30 2016, 3:52 AM
compiler/main/DynFlags.hs
2997

-W by itself doesn't do anything either. I think you can also hide it.

austin accepted this revision.May 30 2016, 3:12 PM

LGTM, and I think @thomie is right as well.

sgillespie updated this revision to Diff 7796.May 31 2016, 5:06 PM

I've updated this to set them all as hidden flags. There are still a few problems

  • -W is still in the output of --show-options, but now it only once. 8.0.1 shows it twice, so I guess it's an improvement
  • It still suggests -fwarn- and -fno-warn-. Output below.
./inplace/bin/ghc-stage2 -fwarn-
ghc-stage2: unrecognised flag: -fwarn-
did you mean one of:
  -fwarn-
  -Wwarn

Okay, so it looks to me like -W by itself DOES something (please correct me if I'm reading this wrong). If I go to line ~2788, I see this:

, make_ord_flag defFlag "W"       (NoArg (mapM_ setWarningFlag minusWOpts))

Which appears to be turning on all minusWOpts. However, I'm going to leave it the way it is right now, since it returns -W only once (before it listed it twice).

thomie added a comment.Jun 3 2016, 6:48 AM

This patch looks weird now (arc/phab issue).

Can you run something like arc diff HEAD^^^ --update D2281.

The number of '^' you use signals the number of commits you want to include in this Diff.

It still suggests -fwarn- and -fno-warn-.

Are you still trying to fix this, or is this patch ready to go?

bgamari requested changes to this revision.Jun 3 2016, 2:41 PM

Indeed, it seems like there is something wrong with the patch. I suspect @thomie is right that simply refreshing it should help.

This revision now requires changes to proceed.Jun 3 2016, 2:41 PM
sgillespie updated this revision to Diff 7839.Jun 3 2016, 7:33 PM
  • Remove bogus flags from suggestions (Trac #12099)
  • Fix arc diff

That was 100% my fault. I didn't know what I was doing. A few notes:

  • Set [-fwarn-, -fno-warn-] as deprecated so they don't show up as suggestions
  • Left -W in --show-options, since it looks like it adds extra warnings

Please let me know if I need to make any more changes.

thomie accepted this revision.Jun 3 2016, 8:10 PM
thomie added a reviewer: thomie.

Looks good to me.

This revision is now accepted and ready to land.Jun 8 2016, 3:49 AM
This revision was automatically updated to reflect the committed changes.