Categorization of warnings in Swift

Ideally I can see wanting to have multiple overlapping categorization schemes. It’s pretty much axiomatic than any classification strategy involves tradeoffs, so allowing more than one is the only approach that makes sense. I may well want to turn off all warnings introduced after Swift 2.2, or enable only warnings for which Fix-It improvements are provided, or any other orthogonal conceptual collection.

Not that every conceivable collection should be supported, but the general notion that there could be overlapping categories seems worth considering.

I agree that this is useful. The main concern I have is coming up with a good scheme to compose these categories. With GCC and Clang, the order of -W and -Wno flags on the command line determine the resolution of enabled/disabled categories. Some people think that is a good design. Others a horrible one. I'm of mixed opinions. It works well from the command line, as the order is clear, but a long command line can make it incomprehensible. In an IDE, which may have a UI to control which categories are enabled, the composition of what is enabled and what isn't can become really hard to reason about if the categories can overlap.

···

On Jan 13, 2016, at 10:20 AM, Kate Stone via swift-dev <swift-dev@swift.org> wrote:

Kate Stone k8stone@apple.com
 Xcode Low Level Tools

On Jan 12, 2016, at 5:15 PM, Michael Ilseman via swift-dev <swift-dev@swift.org> wrote:

By keeping it in mind, do you mean to allow for more categories to be added in the future, or does this affect the fundamental design?

On Jan 12, 2016, at 5:06 PM, Michael Gottesman <mgottesman@apple.com> wrote:

On Jan 12, 2016, at 9:44 AM, Michael Ilseman via swift-dev <swift-dev@swift.org> wrote:

Hello, I'm interested in enabling finer-grained control over warning/error
reporting ala Clang. I've started to put in some infrastructure to
DiagnosticEngine, and now I'm at the point of determining categorization.

I'd like some input (and maybe even some bikeshedding!) on the community's
thoughts and preferences here. Here's my take on the issue:

When it comes to defining the categorization, I can see a few approaches:
Categorize based on broad language-feature/compiler-area. E.g. "Availability" or “CommandLineArguments”
Categorize based on the kind of warning. E.g. "Deprecated" or “Uninitialized"
Categorize based on severity or specificity of warnings. E.g. "Pedantic" or "UnusedValue" or "NullDereference"

And, of course, I think that preference should be given to how people would
actively like to use the categories to control warnings. For example, there's
a handful of warnings that makes less sense in a REPL or rapid experimentation
environment, such as variable_never_mutated.

Here's a straw-man proposal of some categorization. Of course, one developer's
annoying pedantic warning is another's life-saving code-smell detector. I am not
personally tied to these categorizations at all, I'm just interested in there
being something simple and basic. Below is a list of every warning in Swift with
an attempt to put it under a category.

Deprecated:
    var_not_allowed_in_pattern
        "Use of '%select{var|let}0' binding here is deprecated and will be "
        "removed in a future version of Swift"
    deprecated_c_style_for_stmt
        "C-style for statement is deprecated and will be removed in a future "
        "version of Swift"
    deprecated_convention_attribute
        "'@%0' attribute is deprecated; '@convention(%1)' should be used "
        "instead"
    availability_deprecated
        "%0 %select{is|%select{is|was}3}1 deprecated"
        "%select{| %select{on|in}3 %2%select{| %4}3}1"
    availability_deprecated_msg
        "%0 %select{is|%select{is|was}3}1 deprecated"
        "%select{| %select{on|in}3 %2%select{| %4}3}1: %5"
    availability_deprecated_rename
        "%0 %select{is|%select{is|was}3}1 deprecated"
        "%select{| %select{on|in}3 %2%select{| %4}3}1: renamed to '%5'"
    parameter_curry_syntax_removed
        "curried function declaration syntax will be removed in a future "
        "version of Swift; use a single parameter list"

Unsupported:
    warning_parallel_execution_not_supported
        "parallel execution not supported; falling back to serial execution"
    unsupported_synthesize_init_variadic
        "synthesizing a variadic inherited initializer for subclass %0 is "
        "unsupported"

Stylistic/Pedantic/Cleanliness:
    pbd_never_used_stmtcond
        "value %0 was defined but never used; consider replacing "
        "with boolean test"
    pbd_never_used
        "initialization of %select{variable|immutable value}1 %0 was never used"
        "; consider replacing with assignment to '_' or removing it"
    capture_never_used
        "capture %0 was never used",
    variable_never_used
        "%select{variable|immutable value}1 %0 was never used; "
        "consider replacing with '_' or removing it"
    variable_never_mutated
        "%select{variable|parameter}1 %0 was never mutated; "
        "consider changing to 'let' constant"
    variable_never_read
        "%select{variable|parameter}1 %0 was written to, but never read"
    expression_unused_result
        "result of call to %0 is unused"
    expression_unused_init_result
        "result of initializer is unused", ())
    expression_unused_result_message
        "result of call to %0 is unused: %1"
    expression_unused_result_nonmutating
        "result of call to non-mutating function %0 is unused; "
        "use %1 to mutate in-place"
    expression_unused_optional_try
        "result of 'try?' is unused"
    non_trailing_closure_before_default_args
        "closure parameter prior to parameters with default arguments will "
        "not be treated as a trailing closure"
    parameter_extraneous_double_up
        "extraneous duplicate parameter name; %0 already has an argument "
        "label"
    parameter_extraneous_empty_name
        "extraneous '_' in parameter: %0 has no keyword argument name"
    escaped_parameter_name
        "keyword '%0' does not need to be escaped in argument list"

CodeSmell/StrongStylisticHints:
    guard_always_succeeds
        "'guard' condition is always true, body is unreachable"
    warn_unqualified_access
        "use of %0 treated as a reference to %1 in %2 %3"
    var_pattern_didnt_bind_variables
        "'%0' pattern has no effect; sub-pattern didn't bind any variables"
    type_inferred_to_undesirable_type
        "%select{variable|constant}2 %0 inferred to have type %1, "
        "which may be unexpected"
    no_throw_in_try
        "no calls to throwing functions occur within 'try' expression"
    no_throw_in_do_with_catch
        "'catch' block is unreachable because no errors are thrown in 'do' block"
    required_initializer_override_keyword
        "'override' is implied when overriding a required initializer"
    if_always_true
        "'if' condition is always true"
    while_always_true
        "'while' condition is always true"
    warn_protocol_witness_optionality
        "%select{type|result|parameter|parameters|"
        "result and parameters}0 of %1 %select{has|has|has|have|have|}0"
        " different optionality than expected by protocol %2"
    optional_req_nonobjc_near_match
        "non-@objc %select{initializer %1|method %1|property %1|subscript}0 "
        "cannot satisfy optional requirement of @objc protocol %2"
    override_unnecessary_IUO
        "overriding %0 parameter of type %1 with implicitly unwrapped optional "
        "type %2",
    override_unnecessary_result_IUO
        "overriding %0 optional result type %1 with implicitly unwrapped "
        "optional type %2",
    inject_forced_downcast, sema_tce, none,
        "treating a forced downcast to %0 as optional will never produce 'nil'"
    recursive_accessor_reference, tce_sema, none,
        "attempting to %select{access|modify}1 %0 within its own "
        "%select{getter|setter}1",
    store_in_willset, tce_sema, none,
        "attempting to store to property %0 within its own willSet, which is "
        "about to be overwritten by the new value",
    isa_is_always_true
        "'%0' test is always true",
    conditional_downcast_coercion
        "conditional cast from %0 to %1 always succeeds"
    downcast_to_unrelated, sema_tcc, none,
        "cast from %0 to unrelated type %1 always fails"
    forced_downcast_noop
        "forced cast of %0 to same type has no effect"
    forced_downcast_coercion
        "forced cast from %0 to %1 always succeeds; did you mean to use 'as'?"
    extraneous_default_args_in_call
        "call to %0 has extraneous arguments that could use defaults"
    unreachable_code
        "will never be executed"
    unreachable_code_after_stmt
        "code after '%select{return|break|continue|throw}0' will never "
        "be executed"
    unreachable_case
        "%select{case|default}0 will never be executed"
    switch_on_a_constant
        "switch condition evaluates to a constant"
    integer_conversion_overflow_warn
        "integer overflows when converted from %0 to %1"
    integer_literal_overflow_warn
        "integer literal overflows when stored into %0"
    trailing_closure_excess_newlines
        "trailing closure is separated from call site by multiple newlines"
    lex_nul_character, lexing, none
        "nul character embedded in middle of file"
    unindented_code_after_return
        "expression following 'return' is treated as an argument of "
        "the 'return'"
    lex_editor_placeholder_in_playground
        "editor placeholder in source file"

Attributes:
    attr_availability_unknown_platform
        "unknown platform '%0' for attribute '%1'"
    attr_warn_unused_result_expected_name
        "expected parameter 'message' or 'mutable_variant'"
    attr_warn_unused_result_duplicate_parameter
        "duplicate '%0' parameter; previous value will be ignored"
    attr_warn_unused_result_unknown_parameter
        "unknown parameter '%0' in 'warn_unused_result' attribute"
    attr_migration_id_expected_name
        "expected parameter 'pattern'"
    attr_migration_id_unknown_parameter
        "unknown parameter '%0' in '_migration_id' attribute"
    attr_migration_id_duplicate_parameter
        "duplicate '%0' parameter; previous value will be ignored"
    invalid_swift_name_method
        "too %select{few|many}0 parameters in swift_name attribute (expected %1; "
        "got %2)"

Availability:
    availability_query_useless_min_deployment
        "unnecessary check for '%0'; minimum deployment target ensures guard "
        "will always be true"
    availability_query_useless_enclosing_scope
        "unnecessary check for '%0'; enclosing scope ensures guard "
        "will always be true"

And the below I'm either struggling to think about how to categorize them
(perhaps no category at first), or un-familiar with what they're targeting
    warning_from_clang:
        "%0"
    could_not_rewrite_bridging_header, none, none,
        "failed to serialize bridging header; "
        "target may not be debuggable outside of its original project"
    omit_needless_words
        "%0 could be named %1 [-Womit-needless-words]"
    unused_compiler_version_component
        "the second version component is not used for comparison"
    unknown_build_config
        "unknown %0 for build configuration '%1'"
    sema_import_current_module
        "this file is part of module %0; ignoring import"
    sema_import_current_module_with_file
        "file '%0' is part of module %1; ignoring import"
    access_control_member_more
        "declaring %select{PRIVATE|an internal|a public}0 %1 for "
        "%select{a private|an internal|PUBLIC}2 %3"
    access_control_ext_member_more
        "declaring %select{PRIVATE|an internal|a public}0 %1 in "
        "%select{a private|an internal|PUBLIC}2 extension"
    emit_reference_dependencies_without_primary_file
        "ignoring -emit-reference-dependencies (requires -primary-file)"
    warning_no_such_sdk
        "no such SDK: '%0'"
    warn_cannot_stat_input
        "unable to determine when '%0' was last modified: %1"
    warning_unnecessary_repl_mode
        "unnecessary option '%0'; this is the default for '%1' "
        "with no input files"
    incremental_requires_output_file_map
        "ignoring -incremental (currently requires an output file map)"
    incremental_requires_build_record_entry
        "ignoring -incremental; output file map has no master dependencies "
        "entry (\"%0\" under \"\")"

I haven’t gotten to how to expose this to the user, and I’ll defer to the community for suggestions in that area.

Just an FYI: We have also talked about performance related warnings at the SIL level (that could potentially be a hard error). We do not have any such thing implemented right now, but keep it in mind.

Michael

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Upon further discussion with Jordan and others offline, I’m not sure it makes sense at this point in Swift to go about doing categorization. Before wrapping up in this area, I’m going to pursue:

Clean up some existing code, where we have unused categories assigned to all diagnostics (and those categories are arguably useless)
Expose frontend options to treat all warnings as errors as well as options to ignore all warnings

I opened PR [Diagnostics] Introduce -suppress-warnings and -warnings-as-errors flags by milseman · Pull Request #980 · apple/swift · GitHub to perform this.

Interesting future work could be along the lines of addressing https://bugs.swift.org/browse/SR-529\. After there’s unique identifiers, then we can re-explore finer grained control.

As far as front-end options, any preferences on the command-line switches? I don’t see a need to keep consistency with GCC/Clang here, so perhaps “-suppress-warnings” and “-warnings-as-errors”?

Hi, Michael. As one of the people who's been a strong believer of "warning flags result in style dialects", I think it's important to establish a use case here. What will people actually do with warning categories? What warnings will we allow turning off? Under what contexts?

For the "variable never mutated" warning, you mentioned that this doesn't make sense in a rapid experimentation environment. I'd say more specifically that it doesn't make sense in code you're actively changing. But Live Issues should be able to know what code you're actively changing, and only suppress the warning there.

(We do have some ad hoc categorization today, including "REPL mode" as you mentioned. I'm fine with making that something more general.)

I’ll look more at REPL mode and see how to better generalize that. It’s more in line with what I’m trying to accomplish, and it may not make sense to categorize all the warnings in Swift so much as call out limited sub-sets. If that’s the case, and doing so is more so the exception than the rule, then I’m more amenable to tags and/or Kate’s suggestions.

I guess I'd rather avoid eagerly classifying warnings, and I'll continue to argue against -W* and -Wno-* flags for the time being.

What about global flags, such as “-Werr” or equivalent? Do you have any thoughts about Dmitri’s point on multi-platform libraries and how they sometimes can trigger strict stylistic warnings excessively?

I'd rather come up with good answers to if and/or easy, idiomatic ways to silence most warnings (like assigning to _) over flags and diagnostic regions (Clang's pragmas).

Jordan

_______________________________________________
swift-dev mailing list
swift-dev@swift.org
https://lists.swift.org/mailman/listinfo/swift-dev

Hi, Michael. As one of the people who's been a strong believer of "warning flags result in style dialects", I think it's important to establish a use case here. What will people actually do with warning categories? What warnings will we allow turning off? Under what contexts?

For the "variable never mutated" warning, you mentioned that this doesn't make sense in a rapid experimentation environment. I'd say more specifically that it doesn't make sense in code you're actively changing. But Live Issues should be able to know what code you're actively changing, and only suppress the warning there.

(We do have some ad hoc categorization today, including "REPL mode" as you mentioned. I'm fine with making that something more general.)

I’ll look more at REPL mode and see how to better generalize that. It’s more in line with what I’m trying to accomplish, and it may not make sense to categorize all the warnings in Swift so much as call out limited sub-sets. If that’s the case, and doing so is more so the exception than the rule, then I’m more amenable to tags and/or Kate’s suggestions.

I guess I'd rather avoid eagerly classifying warnings, and I'll continue to argue against -W* and -Wno-* flags for the time being.

What about global flags, such as “-Werr” or equivalent? Do you have any thoughts about Dmitri’s point on multi-platform libraries and how they sometimes can trigger strict stylistic warnings excessively?

I'd rather come up with good answers to if and/or easy, idiomatic ways to silence most warnings (like assigning to _) over flags and diagnostic regions (Clang's pragmas).

Our crop of migration-to-Swift-3 warnings is a perfect example of a case where these approaches don’t work well. For example, it is completely reasonable to want to suppress the just-committed ‘typealias’ to ‘associatedtype' warning if you want to keep your code base compiling with Swift 2.[01]. There’s no sensible idiom there, and having to use diagnostic regions would be annoying at best.

  - Doug

Doug, do you think it’s worth pursuing a strategy to handle the migration warnings right now?

···

On Jan 13, 2016, at 4:51 PM, Michael Ilseman via swift-dev <swift-dev@swift.org> wrote:

On Jan 13, 2016, at 2:28 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On Jan 13, 2016, at 13:51, Michael Ilseman <milseman@apple.com <mailto:milseman@apple.com>> wrote:

On Jan 13, 2016, at 1:43 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

On Jan 14, 2016, at 8:32 AM, Douglas Gregor <dgregor@apple.com> wrote:

On Jan 13, 2016, at 2:28 PM, Jordan Rose via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

On Jan 13, 2016, at 13:51, Michael Ilseman <milseman@apple.com <mailto:milseman@apple.com>> wrote:

On Jan 13, 2016, at 1:43 PM, Jordan Rose <jordan_rose@apple.com <mailto:jordan_rose@apple.com>> wrote:

There may not be a need for consistency, but there may be value in being consistent.

If you go with a cohesive model where there are warning categories and separate categories can be enabled, disabled, promoted to errors, etc., then there are merits to GCC/Clang's warning style that I don't think should just be dismissed out of hand. Deviating can provide negative value because it a different scheme that is, well, just different, but conceptually similar to something many developers already know.

Before dismissing -Werror and -w, consider the taxonomy that they are a part of in the GCC/Clang world:

  -Werror-<warning>
  -Wno-error-<warning>
  -Wno-<warning>

This is a very general scheme for controlling warnings. Perhaps "-w" and "-Werror" are a bit special case, so you could always do something like:

  -Wno-all
  -Werror-all

And just have a warning category for "all warnings".

If you are going to go down the route of having any control for warnings from the command line --- which I realize is a contentious topic on this thread --- I personally believe that the flags should be simple, consistent, and generalized. Having special case flags like "-suppress-warnings" and "-warnings-as-errors" feels more even more special case, and I just have to know what they are.

I'm not married to the -W flags scheme, but it is something that is understood. If we are going to have flags, I'd prefer something that was equally (or more) understandable with a general and simple scheme for controlling warnings.

···

On Jan 13, 2016, at 4:51 PM, Michael Ilseman via swift-dev <swift-dev@swift.org> wrote:

As far as front-end options, any preferences on the command-line switches? I don’t see a need to keep consistency with GCC/Clang here, so perhaps “-suppress-warnings” and “-warnings-as-errors”?

As far as front-end options, any preferences on the command-line switches? I don’t see a need to keep consistency with GCC/Clang here, so perhaps “-suppress-warnings” and “-warnings-as-errors”?

There may not be a need for consistency, but there may be value in being consistent.

If you go with a cohesive model where there are warning categories and separate categories can be enabled, disabled, promoted to errors, etc., then there are merits to GCC/Clang's warning style that I don't think should just be dismissed out of hand. Deviating can provide negative value because it a different scheme that is, well, just different, but conceptually similar to something many developers already know.

That’s a very good point. If we want to offer individual or categorical control in the future, we should design something consistent.

Before dismissing -Werror and -w, consider the taxonomy that they are a part of in the GCC/Clang world:

-Werror-<warning>
-Wno-error-<warning>
-Wno-<warning>

This is a very general scheme for controlling warnings. Perhaps "-w" and "-Werror" are a bit special case, so you could always do something like:

-Wno-all
-Werror-all

And just have a warning category for "all warnings".

If you are going to go down the route of having any control for warnings from the command line --- which I realize is a contentious topic on this thread --- I personally believe that the flags should be simple, consistent, and generalized. Having special case flags like "-suppress-warnings" and "-warnings-as-errors" feels more even more special case, and I just have to know what they are.

I'm not married to the -W flags scheme, but it is something that is understood. If we are going to have flags, I'd prefer something that was equally (or more) understandable with a general and simple scheme for controlling warnings.

I don’t mind any particulars, just so long as it’s something reasonable and consistent, and provides a user story that is not too confusing. The -Wno-error-<warning> flags does seem confusing, in that it presupposes that a prior flag ended up treating it as an error, while it might actually be off-by-default. I would think that there are 5 potential states a diagnostic/category can be in WRT user control: “unspecified”, “default”, “error”, “explicitly on”, “explicitly off”. (Perhaps “unspecified” and “default” can be folded in together, but we might want to expose explicitly setting as default to the user).

Rather than rely on order on command line, I’d rather have a very consistent order of precedence that fits intuition. See below for my proposed order of precedence.

The main things I’m trying to avoid in all this is:
Complicating compiler maintenance without sufficient benefit
Complicating the user story
Painting ourselves into a compatibility corner
Categorization merely as an exercise (taxonomy without use case)

Perhaps I'm the outlier here, but I personally think that "warning flags result in style dialects" is not an anathema. There is some code where stylistic enforcements need to be stricter for a variety of reasons. While I think we should aim for a common set of warnings that are enabled to establish good hygiene in Swift code, I do think there will be cases where more warnings/errors are desired beyond what the core language defines.

I agree we don't need to eagerly classify warnings, but I'd also like to understand your criteria for why -W flags are bad thing in general and what you'd prefer to see as a better direction. I certainly don't see it that way. My fear is that if we are overly conservative about not having precise control over warnings is that we may hold back on adding useful warnings to the compiler because they will not be appropriate for everyone.

I also don't see if as a solution for warning control, as you indicated in another message, as I see legitimate cases where more aggressive warnings may be desirable to enable in certain contexts. There is also Chris's comments about other contents, like Playgrounds, where some pedantic warnings that may be on by default provide less value.

I would propose the following precedence, which matches basic intuition:
Individual control
Use case control
Categorical control
Global control
E.g. If someone bothered to single out a warning, that should take precedence over others. If there’s a conflict at a precedence level, we error.

By “use case” control, for lack of a better name off the top of my head (please suggest one!), I mean situations in which diagnostics are not grouped by kind or any other logical grouping. Rather, they appear together in situations that commonly might want to be controlled. These are often diagnostics which would be cross-cutting of any hierarchical categorization by kind. Some examples:
Warnings about features that are currently in transition, which many users don’t want to see if they want to be able to compile warning-free on multiple compiler minor versions.
Opting into extra-stringent stylistic warnings that we wouldn’t normally want to thrust upon normal users (but, not style dialects, just a spectrum of stringency).
The kinds of diagnostics that users like Playgrounds just shouldn’t care about, e.g. unused definitions.
The aim being that there’s a specific user or use-case in mind when defining one, as opposed to having them as an organizational grouping. I don’t feel like these would naturally tend to have much, if any, overlap with each other and perhaps we can treat them as non-overlapping. They may, of course, crosscut categories, should we ever decide to implement categories.

Clang’s categories are sometimes “use case” here, e.g. “c++98-compat”, but often are categorical, e.g. "duplicate-decl-specifier”. I think that for the current state of Swift and Swift for the near future, it makes sense to tackle “use case” soon and categorical later if at all. I feel like the more categorical approach is also more likely to be a source of stylistic dialect splitting and of limited value to the user. Power users can use individual control.

I’d prefer to tackle unique-identifiers before tackling individual control, to help with maintenance and not face a compatibility problem. If this is seems very valuable to Swift, we can also pursue automatic and hyperlinked documentation produced from the diagnostic definitions, etc.

···

On Jan 19, 2016, at 10:21 PM, Ted kremenek <kremenek@apple.com> wrote:

On Jan 13, 2016, at 4:51 PM, Michael Ilseman via swift-dev <swift-dev@swift.org> wrote:

As far as front-end options, any preferences on the command-line switches? I don’t see a need to keep consistency with GCC/Clang here, so perhaps “-suppress-warnings” and “-warnings-as-errors”?

There may not be a need for consistency, but there may be value in being consistent.

If you go with a cohesive model where there are warning categories and separate categories can be enabled, disabled, promoted to errors, etc., then there are merits to GCC/Clang's warning style that I don't think should just be dismissed out of hand. Deviating can provide negative value because it a different scheme that is, well, just different, but conceptually similar to something many developers already know.

That’s a very good point. If we want to offer individual or categorical control in the future, we should design something consistent.

Before dismissing -Werror and -w, consider the taxonomy that they are a part of in the GCC/Clang world:

-Werror-<warning>
-Wno-error-<warning>
-Wno-<warning>

This is a very general scheme for controlling warnings. Perhaps "-w" and "-Werror" are a bit special case, so you could always do something like:

-Wno-all
-Werror-all

And just have a warning category for "all warnings".

If you are going to go down the route of having any control for warnings from the command line --- which I realize is a contentious topic on this thread --- I personally believe that the flags should be simple, consistent, and generalized. Having special case flags like "-suppress-warnings" and "-warnings-as-errors" feels more even more special case, and I just have to know what they are.

I'm not married to the -W flags scheme, but it is something that is understood. If we are going to have flags, I'd prefer something that was equally (or more) understandable with a general and simple scheme for controlling warnings.

I don’t mind any particulars, just so long as it’s something reasonable and consistent, and provides a user story that is not too confusing. The -Wno-error-<warning> flags does seem confusing, in that it presupposes that a prior flag ended up treating it as an error, while it might actually be off-by-default. I would think that there are 5 potential states a diagnostic/category can be in WRT user control: “unspecified”, “default”, “error”, “explicitly on”, “explicitly off”. (Perhaps “unspecified” and “default” can be folded in together, but we might want to expose explicitly setting as default to the user).

To get the ball rolling towards getting a consistent set of flags soon, here’s a proposal (the details of which I’m not particularly attached to):

-Werror=<warning/all/category/...>
-Wenable=<warning/all/category/...>
-Wdisable=<warning/all/category/...>
-Wdefault=<warning/all/category/...>

And, we add support for “all” while we figure out what we want to do for everything else. I could also consider “-Won=” and “-Woff=“ for brevity, though perhaps at the cost of some clarity. Similarly for any command-line aliases to ease brevity, e.g. “-W<warning>” expands to “-Wenable=<warning>”, etc.

···

On Jan 20, 2016, at 11:23 AM, Michael Ilseman via swift-dev <swift-dev@swift.org> wrote:

On Jan 19, 2016, at 10:21 PM, Ted kremenek <kremenek@apple.com <mailto:kremenek@apple.com>> wrote:

On Jan 13, 2016, at 4:51 PM, Michael Ilseman via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote:

Rather than rely on order on command line, I’d rather have a very consistent order of precedence that fits intuition. See below for my proposed order of precedence.

The main things I’m trying to avoid in all this is:
Complicating compiler maintenance without sufficient benefit
Complicating the user story
Painting ourselves into a compatibility corner
Categorization merely as an exercise (taxonomy without use case)

Perhaps I'm the outlier here, but I personally think that "warning flags result in style dialects" is not an anathema. There is some code where stylistic enforcements need to be stricter for a variety of reasons. While I think we should aim for a common set of warnings that are enabled to establish good hygiene in Swift code, I do think there will be cases where more warnings/errors are desired beyond what the core language defines.

I agree we don't need to eagerly classify warnings, but I'd also like to understand your criteria for why -W flags are bad thing in general and what you'd prefer to see as a better direction. I certainly don't see it that way. My fear is that if we are overly conservative about not having precise control over warnings is that we may hold back on adding useful warnings to the compiler because they will not be appropriate for everyone.

I also don't see if as a solution for warning control, as you indicated in another message, as I see legitimate cases where more aggressive warnings may be desirable to enable in certain contexts. There is also Chris's comments about other contents, like Playgrounds, where some pedantic warnings that may be on by default provide less value.

I would propose the following precedence, which matches basic intuition:
Individual control
Use case control
Categorical control
Global control
E.g. If someone bothered to single out a warning, that should take precedence over others. If there’s a conflict at a precedence level, we error.

By “use case” control, for lack of a better name off the top of my head (please suggest one!), I mean situations in which diagnostics are not grouped by kind or any other logical grouping. Rather, they appear together in situations that commonly might want to be controlled. These are often diagnostics which would be cross-cutting of any hierarchical categorization by kind. Some examples:
Warnings about features that are currently in transition, which many users don’t want to see if they want to be able to compile warning-free on multiple compiler minor versions.
Opting into extra-stringent stylistic warnings that we wouldn’t normally want to thrust upon normal users (but, not style dialects, just a spectrum of stringency).
The kinds of diagnostics that users like Playgrounds just shouldn’t care about, e.g. unused definitions.
The aim being that there’s a specific user or use-case in mind when defining one, as opposed to having them as an organizational grouping. I don’t feel like these would naturally tend to have much, if any, overlap with each other and perhaps we can treat them as non-overlapping. They may, of course, crosscut categories, should we ever decide to implement categories.

Clang’s categories are sometimes “use case” here, e.g. “c++98-compat”, but often are categorical, e.g. "duplicate-decl-specifier”. I think that for the current state of Swift and Swift for the near future, it makes sense to tackle “use case” soon and categorical later if at all. I feel like the more categorical approach is also more likely to be a source of stylistic dialect splitting and of limited value to the user. Power users can use individual control.

I’d prefer to tackle unique-identifiers before tackling individual control, to help with maintenance and not face a compatibility problem. If this is seems very valuable to Swift, we can also pursue automatic and hyperlinked documentation produced from the diagnostic definitions, etc.

_______________________________________________
swift-dev mailing list
swift-dev@swift.org <mailto:swift-dev@swift.org>
https://lists.swift.org/mailman/listinfo/swift-dev

As far as front-end options, any preferences on the command-line switches? I don’t see a need to keep consistency with GCC/Clang here, so perhaps “-suppress-warnings” and “-warnings-as-errors”?

There may not be a need for consistency, but there may be value in being consistent.

If you go with a cohesive model where there are warning categories and separate categories can be enabled, disabled, promoted to errors, etc., then there are merits to GCC/Clang's warning style that I don't think should just be dismissed out of hand. Deviating can provide negative value because it a different scheme that is, well, just different, but conceptually similar to something many developers already know.

That’s a very good point. If we want to offer individual or categorical control in the future, we should design something consistent.

Before dismissing -Werror and -w, consider the taxonomy that they are a part of in the GCC/Clang world:

-Werror-<warning>
-Wno-error-<warning>
-Wno-<warning>

This is a very general scheme for controlling warnings. Perhaps "-w" and "-Werror" are a bit special case, so you could always do something like:

-Wno-all
-Werror-all

And just have a warning category for "all warnings".

If you are going to go down the route of having any control for warnings from the command line --- which I realize is a contentious topic on this thread --- I personally believe that the flags should be simple, consistent, and generalized. Having special case flags like "-suppress-warnings" and "-warnings-as-errors" feels more even more special case, and I just have to know what they are.

I'm not married to the -W flags scheme, but it is something that is understood. If we are going to have flags, I'd prefer something that was equally (or more) understandable with a general and simple scheme for controlling warnings.

I don’t mind any particulars, just so long as it’s something reasonable and consistent, and provides a user story that is not too confusing. The -Wno-error-<warning> flags does seem confusing, in that it presupposes that a prior flag ended up treating it as an error, while it might actually be off-by-default.

I agree with you completely. I have nothing against deviating from them for good reason — but those reasons should be well understood and deliberate.

I would think that there are 5 potential states a diagnostic/category can be in WRT user control: “unspecified”, “default”, “error”, “explicitly on”, “explicitly off”. (Perhaps “unspecified” and “default” can be folded in together, but we might want to expose explicitly setting as default to the user).

Hmm. I’m trying to understand what “unspecified” would mean in practice. The diagnostic system would need to do something with an “unspecified” warning, which I assume would mean the “default”.

Rather than rely on order on command line, I’d rather have a very consistent order of precedence that fits intuition. See below for my proposed order of precedence.

Seems like a great goal. The main advantage of the command line is that it allows the ability to easily provide overrides if the default behavior is articulated in terms of a set of command line options you inherited elsewhere from the build system.

For example, in some cases being able to pass “additional flags” on a per-file basis has been historically a useful way for a specific file to provide overrides to the default project warning settings.

The main things I’m trying to avoid in all this is:
Complicating compiler maintenance without sufficient benefit
Complicating the user story
Painting ourselves into a compatibility corner
Categorization merely as an exercise (taxonomy without use case)

These sound great, but also a bit abstract. I’m also not so worried about the latter. The categorization we are talking about here is about the ability to enable/disable any warning and to change the policy on each warning. Whether or not warnings are grouped into logical (and useful) groups really comes down to whether or not you want to turn on/off groups of warnings at a time that are naturally related.

Perhaps I'm the outlier here, but I personally think that "warning flags result in style dialects" is not an anathema. There is some code where stylistic enforcements need to be stricter for a variety of reasons. While I think we should aim for a common set of warnings that are enabled to establish good hygiene in Swift code, I do think there will be cases where more warnings/errors are desired beyond what the core language defines.

I agree we don't need to eagerly classify warnings, but I'd also like to understand your criteria for why -W flags are bad thing in general and what you'd prefer to see as a better direction. I certainly don't see it that way. My fear is that if we are overly conservative about not having precise control over warnings is that we may hold back on adding useful warnings to the compiler because they will not be appropriate for everyone.

I also don't see if as a solution for warning control, as you indicated in another message, as I see legitimate cases where more aggressive warnings may be desirable to enable in certain contexts. There is also Chris's comments about other contents, like Playgrounds, where some pedantic warnings that may be on by default provide less value.

I would propose the following precedence, which matches basic intuition:
Individual control
Use case control
Categorical control
Global control
E.g. If someone bothered to single out a warning, that should take precedence over others. If there’s a conflict at a precedence level, we error.

By “use case” control, for lack of a better name off the top of my head (please suggest one!), I mean situations in which diagnostics are not grouped by kind or any other logical grouping. Rather, they appear together in situations that commonly might want to be controlled. These are often diagnostics which would be cross-cutting of any hierarchical categorization by kind. Some examples:
Warnings about features that are currently in transition, which many users don’t want to see if they want to be able to compile warning-free on multiple compiler minor versions.
Opting into extra-stringent stylistic warnings that we wouldn’t normally want to thrust upon normal users (but, not style dialects, just a spectrum of stringency).
The kinds of diagnostics that users like Playgrounds just shouldn’t care about, e.g. unused definitions.
The aim being that there’s a specific user or use-case in mind when defining one, as opposed to having them as an organizational grouping. I don’t feel like these would naturally tend to have much, if any, overlap with each other and perhaps we can treat them as non-overlapping. They may, of course, crosscut categories, should we ever decide to implement categories.

I hate to say it, but “use case control” naturally fits with a cross-cutting warning category.

Clang’s categories are sometimes “use case” here, e.g. “c++98-compat”, but often are categorical, e.g. "duplicate-decl-specifier”. I think that for the current state of Swift and Swift for the near future, it makes sense to tackle “use case” soon and categorical later if at all. I feel like the more categorical approach is also more likely to be a source of stylistic dialect splitting and of limited value to the user. Power users can use individual control.

I’d prefer to tackle unique-identifiers before tackling individual control, to help with maintenance and not face a compatibility problem. If this is seems very valuable to Swift, we can also pursue automatic and hyperlinked documentation produced from the diagnostic definitions, etc.

One possibility is to make “use case control” and “categorical control” distinctly different in a fundamental way. For example, perhaps all warnings can be divided up into hierarchical groups (groups only overlap if they are a subset) based on categorical control, and those groups can have defaults that are defined in terms of the leaves. The “use case” control has no defaults, but provides a knob to explicitly turn a group of warnings on or off, essentially overriding the other behavior.

What does “global control” mean in your taxonomy?

···

On Jan 20, 2016, at 11:23 AM, Michael Ilseman <milseman@apple.com> wrote:

On Jan 19, 2016, at 10:21 PM, Ted kremenek <kremenek@apple.com <mailto:kremenek@apple.com>> wrote:

On Jan 13, 2016, at 4:51 PM, Michael Ilseman via swift-dev <swift-dev@swift.org <mailto:swift-dev@swift.org>> wrote: