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 updated this revision to Diff 7778.May 29 2016, 11:55 PM
sgillespie retitled this revision from to Fix #12099: Remove bogus flags.
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
bgamari edited edge metadata.

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
austin edited edge metadata.

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

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

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
bgamari edited edge metadata.

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
sgillespie edited edge metadata.
  • 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.

bgamari accepted this revision.Jun 8 2016, 3:49 AM
bgamari edited edge metadata.

Thanks @sgillespie!

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.