SE-0196 — Compiler Diagnostic Directives

Evolution Proposal Reviews
1

The review of SE 0196 — Compiler Diagnostic Directives begins now and runs through January 30, 2018.

The (original) proposal is available here:

https://github.com/apple/swift-evolution/blob/ab0c22a2340be9bfcb82e6f237752b4d959a93b7/proposals/0196-diagnostic-directives.md

Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to the review manager. When emailing the review manager directly, please keep the proposal link at the top of the message.

What goes into a review of a proposal?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.

When reviewing a proposal, here are some questions to consider:

  • What is your evaluation of the proposal?

  • Is the problem being addressed significant enough to warrant a change to Swift?

  • Does this proposal fit well with the feel and direction of Swift?

  • If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Thanks,
Ted Kremenek
Review Manager

Addendum

The proposal was accepted with revision. The latest version of the proposal is here:

https://github.com/apple/swift-evolution/blob/master/proposals/0196-diagnostic-directives.md

9 Likes
2

+1. Something I've been missing in Swift for a long time.

3

What is your evaluation of the proposal?

This is a small addition but one that we could really use in our codebases, big +1 on this.

Is the problem being addressed significant enough to warrant a change to Swift?

I believe so, another example where this feature could come in handy is code generation:

  • when we use Sourcery to generate code we sometimes run into situations that we can't generate proper code for, since silently skipping the generation of code for the unsupported condition would be the worst thing to do (people can miss it and rely on the code working as normal), what we do is we just write uncompilable Swift to raise the error to the user. It would be much cleaner to just generate #error "Type Foo doesn't support X"

Does this proposal fit well with the feel and direction of Swift?

I think its important we get this functionality in, its not that common to use since this isn't really a language keyword but more of a compiler hint, but one that we could really use.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

It follows standard syntax that I've used in other languages, e.g. C#, C++

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read proposal and the discussion that happened in the pitch phase, also thought about it before as the need for it came up in my development of products

1 Like
4

Technically, you can already do that, since “#error” does indeed produce a compiler error. :-)

2 Likes
5

Technically, you can already do that, since “error” does indeed produce a compiler error. :-)

yeah its one of the options I meant by the uncompilable Swift expression, once this proposal is implemented we'll have proper diagnostic. Right now you can just as well generate something like "THIS IS WRONG IDEA" and it will fail build, not the nicest thing to do though ¯_(ツ)_/¯

6

+1, improves Swift's ergonomicity in a meaningful way.

7

I'm in favor of these directives, but I would go the #directive(...) route from the beginning. Here is why I think we should use the approach from the alternatives rather than the proposed one:

  • At least for #warning directive I think it doesn't have to be a compile time only warning. Similar to how thread sanitizer in Xcode emits warnings at runtime, I think we should also be able to emit runtime warnings in our projects if at runtime something unexpected happened. Ideally this is a better way to log issues during development and testing, where I'd also assume that the warning is removed by the compiler for a release build automatically.

    • #warning("....") is the same as #warning("....", at: .compiletime)
    • Additionally we'd have #warning("...", at: .runtime)

Here is a small peace of code that I took from my current project, where I could make use of a runtime warning rather than printing the issue and filtering all logs to find it.

private func on<TriggerObservable>(
  completable: Completable,
  handleError: ErrorHandler<TriggerObservable>? = nil,
  recover: RecoveryHandler? = nil,
  complete: CompletionHandler? = nil,
  dispose key: UnleashedDisposeKey
) where TriggerObservable : ObservableType {
  var completable = completable
  // Add error handler if needed
  if let handleError = handleError {
    completable = completable.retryWhen(handleError)
  }

  let dispose: () -> Void = { self.disposeCache[key] = nil }

  let recoverAndDispose: RecoveryHandler = { error in
    if let recover = recover {
      DispatchQueue.main.async { recover(error) }
    } else {
      #warning("""
        Unhandled error 🆘:
          - \(error)
          - Emitted on a completable sequence\n
        """, at: .runtime)
      )
    }
    dispose()
  }

  let completeAndDispose: CompletionHandler = {
    if let complete = complete {
      DispatchQueue.main.async(execute: complete)
    }
    dispose()
  }

  completable
    .subscribe(onCompleted: completeAndDispose, onError: recoverAndDispose)
    .disposed(by: disposeCache, key: key)
}

The issue here is that the error handling in implemented in a different place and simply cannot be included into the else branch of the if statement. Rather than printing the warning, I'd love to see the warnings emitted by the IDE which would provide me an easy way to jump to the source code that emitted the warning and also show more informations defined in the custom string.

I also have a few questions related to #warning:

  • Are the emitted warnings bound to a module?
  • If not, how can we mute those warnings emitted by an integrated 3rd party module?

EDIT:

  • Only runtime warnings can have string interpolations, build-time warnings must be composed out of a constant string (single or multi-lined version are both allowed).

  • We could also allow for symmetrical consistency runtime errors which will behave as a simple wrapper over fatalError.

8

What is your evaluation of the proposal?

I think this feature could be very useful and is a good addition to the language.

I would prefer a syntax with parentheses for the simple reason that you might want to emit a warning that appears on the same line as the code being warned about:

return 0     #warning("Incomplete implementation: return number of rows")

I'm also interested in the idea of converting <#foo#> markers in Swift code into warnings which trap when executed, but that is a severable proposal with distinct use cases from this one.

Is the problem being addressed significant enough to warrant a change to Swift?

It's a minor problem, but significant.

Does this proposal fit well with the feel and direction of Swift?

Yes, I think so. We already use # for compile-time features like this, and this design isn't laden with legacy elements like #pragma or anything.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

This feature is pretty useful in Objective-C.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A glance.

2 Likes
9

Are the emitted warnings bound to a module?

  • No, #warning is purely build-time, and so should not be serialized in modules. That said, SwiftPM dependencies building from source will necessarily build their #warning/#error directives during a swift build run. Silencing those warnings is a possible direction we could (and probably should) take for SwiftPM. More broadly, I foresee #warning as being more useful for user code, and #error more useful for library code.
10

Well in CocoaTouch there is no SPM yet and as for CocoaPods you'll get those build-time warnings as well. Sure CocoaPods allow to silent them, but still I'm concerned about modules #warning("Fix this for the next major release.")

And I still think we should provide buil-time and run-time warning derective where as for the error derective we already have fatalError and Never at runtime.

11

I’m not quite sure I follow. If it’s a binary framework, that won’t show up, because the #warning wouldn’t have been serialized. If it’s a module you’re building from source, then it will show up unless you inhibit warnings using CocoaPods’s inhibit_all_warnings!, or SwiftPM’s as-yet-unproposed feature.

12

That's exactly what I was asking about, for non-binary frameworks. If this is accepted someone has to push a follow up proposal to add this to SPM as well.

What is your opinion about the runtime warnings I described in my first post in this thread? Those does not have to be added with this proposal but the door would still be open if we'd use the parentheses version.

13

What is your evaluation of the proposal?

I'm in favor of adding #warning and #error directives, but not using the grammar described in the proposal. In my opinion, it should be consistent with other compiler directives available in Swift, i.e. use parentheses:

// like this:

#warning("foo")
#error("bar")

// instead of:

#warning "foo"
#error "bar"

Using parentheses would allow to extend this directive in the future, e.g. adding runtime warning support mentioned by @DevAndArtist.

Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

Does this proposal fit well with the feel and direction of Swift?

The idea – yes but execution – not so much, in my opinion. The proposed syntax feels out-of-place in comparison to other compiler directives, like #available and #canImport.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Yes, Objective-C.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Fully read the proposal and this thread.

14

I’m definitely not opposed to it, and I’m also not opposed to the parenthesized version of warning.

1 Like
15

What is your evaluation of the proposal?

I am very supportive of adding a feature along these lines to the language but I am not fully convinced of the specific details.

I have used a generic TODO function that calls fatalError in the past. I believe I have also used similar functions with names describing other use cases. It is possible to force a function like this to produce a warning using @available(*, deprecated). Of course it is also possible to write a function warning that produces warning using the same technique.

With this existing capability in mind, there are three primary reasons I can think of to add a feature to the language. First, the technique described above is an abuse of @available that does not allow control over the content of the warning message. Second, a directive is able to produce a warning in a lexical context where expressions are not allowed. Third, I don't believe a similar technique is available to force an error.

However, the current proposal sacrifices one of the benefits of the custom TODO function approach that I really appreciate. The custom TODO function is an expression that can be used as a placeholder for a value of any type. This is sometimes very useful. It allows code to compile (and even run) before it is complete. This provides a very flexible workflow for writing code and interacting with the type checker. That can be especially useful when writing very generic code.

I would like to see consideration of a mechanism for defining warning producing expressions which offer control over the warning message that is produced. I'm not sure exactly what that might look like or exactly how it would relate to a directive that can be used in contexts where an expression is not allowed but I believe it is relevant to consideration of this proposal.

Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

Does this proposal fit well with the feel and direction of Swift?

Mostly, although the parenthesized version suggested by others seems to fit a little better.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Yes, this was useful in Objective-C.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick read, although I have used features like this in the past and have wanted them to exist in Swift.

16

What is your evaluation of the proposal?

I'm generally in favor of this proposal, but with the #warning("text") syntax. This will allow easier extensions to this type of directive in the future, like the #warning("warning", at:) example mentioned above.

Is the problem being addressed significant enough to warrant a change to Swift?

Yes, it's a slight a problem, but not insignificant.

Does this proposal fit well with the feel and direction of Swift?

Yes, compiler directives are a natural part of the Swift language.

If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

None.

How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read the proposal and thread.

17
  • Does this proposal fit well with the feel and direction of Swift?

No. The proposed syntax is inconsistent with other compiler directives.

#warning "This is a warning message"

The #sourceLocation directive for example, looks like a function call:
#sourceLocation(file: "abc.swift", line: 42)

When sourceLocation went through the evolution process the original proposal was to use a syntax that is compatible with the C preprocessor (`#line "abc.swift" 42). At the time swift-evolution decided that we should use a more swifty syntax that is decidedly different from the C preprocessor.

Applying the same rationale, I would expect the syntax for #warning to be

#warning(message: "This is a warning message.")

-- adrian

18

+1. I think that this would be really useful.

19

I have no problem with that syntax, though message: seems excessive.

1 Like
20

I agree that message: doesn't add any value and is cumbersome to type. Removing it doesn't make the syntax any less swifty.

2 Likes