Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regarding "-Werror" and how the proposals are examined by the committee #262

Closed
Kleidukos opened this issue Mar 24, 2024 · 17 comments
Closed
Labels
approved Approved by CLC vote meta General questions on CLC rules and policies

Comments

@Kleidukos
Copy link
Member

Kleidukos commented Mar 24, 2024

Hello committee,

In light of recent proposals that were discussed with deprecation and warning mechanisms at their core, I see a strange phenomenon of pleading for users of -Werror, that is to say the flag that turns every warning, no matter how benign, and including -Wcompat, into fatal errors.

This is quite surprising because users of -Werror voluntarily checked out of the ability to be programmatically warned ahead of time for breaking changes that would happen in a couple of major releases. By not being selective on which warning they want to turn into errors, they prefer to have things break early, compromising neither on the "break", nor on the "early".

Hence from my point of view, it is quite peculiar to see arguments opposed to future breaking changes with a progressive deprecation phase, in order to protect the users who so dearly wish things to break early.

I would like to ask the committee to solve the matter of -Werror once and for all, preferring solutions like warning groups, that allow granularity, instead of never ever emitting any kind of warning at all.

@angerman
Copy link

This shouldn't even be a discussion. If you enabled -Werror you are asking for early fatal breakage. And that's what you get. We should not entertain the idea that -Werror need to be considered when deprecating. The whole point is to warn people for upcoming changes. If you run with -Werror you get the warning as fatal error early. Not a bit later when it fatally breaks. Deprecations allow you to also get some more context as to why. And you can often specifically opt out of a specific warning at the module level if you so need to.

@hasufell
Copy link
Member

I think what sparked this thread is that these types of discussions are constantly coming up. So the question is if we as a CLC should encode common singular principles we all agree on in a document, so that contributors know what they're getting into and we can just link to the document whenever it comes up again.

Such a document wouldn't really be set in stone and could as well be discarded by a future CLC.

But I believe it makes things a bit easier.

This is far less ambitious than coming up with a "stability policy". I see this really about avoiding repetitive discussions.

@parsonsmatt
Copy link

I agree. If you choose to build with -Werror then you are signing up for breaking changes + you can always disable particular warning classes or demote them back to warnings.

@Bodigrim
Copy link
Collaborator

I agree, and mind you I'm addicted to -Werror -Wall -Wcompat stuff.

@mixphix
Copy link
Collaborator

mixphix commented Mar 26, 2024

It's almost like the committee members already have a reasonable grasp on the solutions at hand, and are not the ones causing problems and repeating discussions on purpose....

@Kleidukos
Copy link
Member Author

@mixphix Don't feel targeted if you're not guilty of this. This sort of principle also applies to submitters and third-party reviewers on the proposals.

@michaelpj
Copy link

GHC's new stability principles explicitly say that they will not keep warnings stable: https://github.com/ghc-proposals/ghc-proposals/blob/master/principles.rst?plain=1#L397

(Obviously the CLC can do something different to GHC, but it's a reference point)

@Bodigrim
Copy link
Collaborator

Bodigrim commented Apr 19, 2024

@Kleidukos what kind of a formal resolution besides personal statements from CLC members are you looking for if any?

@Kleidukos
Copy link
Member Author

@Bodigrim A note in Impact Assessments would be the end goal, so that the opinion of the CLC doesn't get lost in this ticket when it is closed.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Apr 20, 2024

@Kleidukos would you like to suggest a specific wording? How about the following?

"Note that additional or new warnings and deprecations are not classified as breaking changes. We assume that users who enable -Werror make a conscious and informed choice to break their builds as early and as fatal as possible."

Such wording still allows CLC members to have individual opinions whether a given warning / deprecation is worth the trouble. It's not like anyone can incur as many warnings as they like, but they do not count towards breakage and do not require a proposer to write patches to fix them.

Upd.: This is not really anything new, historically we (at least the current incarnation of CLC) never treated warnings as breakage.

@Kleidukos
Copy link
Member Author

@Bodigrim yep, the wording is perfect.

@Bodigrim Bodigrim added the meta General questions on CLC rules and policies label Apr 25, 2024
@Bodigrim
Copy link
Collaborator

OK, let's vote on this.

Dear CLC members, the proposal is to extend Impact Assessments section of PROPOSALS.md with a paragraph

Note that additional or new warnings and deprecations are not classified as breaking changes. We assume that users who enable -Werror make a conscious and informed choice to break their builds as early and as fatal as possible.

@angerman @hasufell @tomjaguarpaw @parsonsmatt @velveteer @mixphix


+1 from me, see my comment above.

@Bodigrim Bodigrim added the vote-in-progress The CLC vote is in progress label Apr 25, 2024
@parsonsmatt
Copy link

+1

3 similar comments
@velveteer
Copy link
Contributor

+1

@hasufell
Copy link
Member

+1

@tomjaguarpaw
Copy link
Member

+1

@Bodigrim
Copy link
Collaborator

Thanks all, that's enough votes to approve. I'll implement the change in a moment.

@Bodigrim Bodigrim added approved Approved by CLC vote and removed vote-in-progress The CLC vote is in progress labels Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved by CLC vote meta General questions on CLC rules and policies
Projects
None yet
Development

No branches or pull requests

9 participants