Opened 13 months ago

Last modified 2 months ago

#12822 new task

Cleanup GHC verbosity flags

Reported by: hsyl20 Owned by:
Priority: low Milestone:
Component: Compiler Version: 8.0.1
Keywords: newcomer, flags Cc: simonmar, captaintrunky
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: Other Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s):
Wiki Page:

Description

1) In the discussion of Phab:D2679#77898, Simon Marlow suggested that we make verbosity levels (-v0, -v1...) work just like optimization levels (-O0, -O1...), i.e., that we make each verbosity level a collection of verbosity flags.

Currently the verbosity level is tested against explicitly in several places in GHC's code.

2) Homogenize verbosity flags: currently, flags that change GHC's reporting have various prefixes:

  • -fprint-*
  • -fshow-*
  • -dshow-passes
  • -ddump-*
  • -dppr-*
  • -dverbose-*

Maybe we should homogenize (some of) them by using a common -v prefix as in -vshow-source-paths.

Attachments (1)

fix12822.patch (5.3 KB) - added by captaintrunky 2 months ago.
First implementation of unified verbosity flags.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 13 months ago by goldfire

+1 for efforts in this direction.

comment:2 Changed 13 months ago by simonmar

Cc: simonmar added

comment:3 Changed 3 months ago by captaintrunky

I'd like to work on this ticket. Regarding 2), is it enough just to rename corresponding flags or it could be better to add a new type, e.g. 'data ReportFlag'?

comment:4 Changed 3 months ago by bgamari

Let's discuss a concrete proposal before discussing implementation strategy. Which of the two parts listed in the ticket description would you like to take up? Both? If so, what concrete renaming are you proposing? For how long of a deprecation window do we want to keep the current flags around?

Changes like these tend to inflict a small amount of pain on a lot of users for a long time, so I think we should be careful and deliberate in approaching this. By all means let's discuss it though! Perhaps ultimately this should go through the proposals process.

Last edited 3 months ago by bgamari (previous) (diff)

comment:5 Changed 3 months ago by captaintrunky

I'm ready to work with both parts. It feels like the first part is somewhat simpler and straightforward, so I can start with it. As for the second part, I like the renaming with '-v' prefix. Implementation wise, I think that we can add a new constructor for DynFlag:

  data DynFlags = DynFlags {
    ...
  } |
  VerbocityFlag {
    ...
  }

As it's painful change, I think that it should be marked as deprecated for quite a lot of time, maybe until the end of 2018?

comment:6 Changed 3 months ago by bgamari

Let's first define specifically which flags we are talking about changing here; there are currently several broad categories that one might consider "verbosity" flags,

  • the dump (-d*) flags; these are currently handled uniformly within just and are represented by the DynFlags.DumpFlags type.
  • the -fprint-* flags
  • the -fshow-options flag
  • the -v<n> flags
  • the as-of-yet non-existent flags which denote the individual effects currently controlled by the -v<n> flags

What concretely would you like to do with these flags? It's not entirely clear to me that, for instance, unifying the -d* flags with the -fprint-* flags is a good idea; afterall, the former are meant for developers while the latter are for users.

Regarding the refactoring bit of (1): There should be no need to add a constructor to DynFlags (which is essentially just a product of all of the user-specified configuration GHC needs). You should have a look at how the -On flags are currently implemented; namely look at references to optLevel in DynFlags. Of particular interest will be DynFlags.optLevelFlags.

comment:7 Changed 2 months ago by bgamari

I wrote this in response to captaintrunky's query on ghc-devs,

Sergey Bykov <captaintrunky@…> writes:

Hi, I'm working with the #12822 task, which is a refactoring for the verbosity flag. It should be reimplemented in a way, similar to the 'optimization' flag. After studying the codebase, specifically *optLevelFlags*, I'm stuck with the following questions:

  1. Should I add a new data 'VerbosityFlag' similar to GeneralFlag, DumpFlag, etc or should I extend any of existing data types?
  2. How to determine a set of verbosity options to implement? Is grepping through all the codebase and adding corresponding options a good approach?

As I understand it, the task is to split up the current -v<n> flags into distinct flags. The current role of -v is described roughly in Note [Verbosity levels] (although it references -ddump-most and -ddump-all, which don't exist anymore).

Grepping the source tree (e.g. for "verbosity dflags") would indeed be a good way to find the various places it's used. For now let's just add the new flags to GeneralFlag. If there are enough that GeneralFlags becomes bloated we can refactor later.

comment:8 Changed 2 months ago by bgamari

For the record, here are the relevant uses of verbosity that I was able to quickly find,

$ ag "verbosity .*dflags"
compiler/backpack/DriverBkp.hs
514:                | verbosity (hsc_dflags hsc_env) >= 2 -> showMsg "Skipping  " ""

compiler/ghci/Linker.hs
1463:    = when (verbosity dflags > 1) $

compiler/main/Finder.hs
659:        | verbosity dflags < 3 =
775:        | verbosity dflags < 3 =

compiler/main/DynFlags.hs
1998:             || (verbosity dflags >= 4 && enableIfVerbose f)
2203:  | verbosity dflags >= 4 = ["-v"]

compiler/main/HscMain.hs
810:            | verbosity (hsc_dflags hsc_env) >= 2 -> showMsg "Skipping  " ""

compiler/main/ErrUtils.hs
554:  | verbosity dflags >= val = act
613:       if verbosity dflags >= 2
684: = do   { let verb = verbosity dflags

ghc/Main.hs
233:  case verbosity dflags6 of
736:   let verb = verbosity dflags

ghc/GHCi/UI.hs
553:                when (isNothing maybe_exprs && verbosity dflags > 0) $
588:  let show_prompt = verbosity dflags > 0 || is_tty
618:  liftIO $ when (verbosity dflags > 0) $ putStrLn "Leaving GHCi."
1818:  when (verbosity dflags > 0) $
2634:          when (verbosity dflags2 > 0) $

Changed 2 months ago by captaintrunky

Attachment: fix12822.patch added

First implementation of unified verbosity flags.

comment:9 Changed 2 months ago by captaintrunky

I've done some preliminary fixes for this task in DynFlags. Is that the right approach? Next step is to refactor some uses of 'verbosity' in some files mentioned above.

comment:10 Changed 2 months ago by captaintrunky

Cc: captaintrunky added
Note: See TracTickets for help on using tickets.