Allow `#warning` in more places

Evolution Discussion
1

The #warning compiler directive, from SE–196, is useful for marking lines of code that need to be changed. However, it is currently parsed as a statement, which means it can only appear in statement position.

Sometimes the line of code that needs to be marked is located where a statement cannot appear, such as within the body of an expression. It could be an element of an array, an argument to a function call, or a parameter in a declaration.

In all those situations, the #warning directive today results in a compiler error related to finding a statement where an expression was expected. This occurs regardless of whether the directive appears on a line by itself or trailing some other code. For example these are both errors:

foo(
  a: 123,
  #warning("change this")    // currently an error
  b: 456,
  c: 789
)

let x = [
  123,
  456,    #warning("change this")    // currently an error
  789
]

I propose that #warning should instead be treated like whitespace, similar to the way that comments are, and allowed to appear in these positions.

• • •

Interestingly, within an if expression, #warning can already appear on a line by itself even though other statements (such as print) cannot:

Example 
let x = if true {
  #warning("change this")    // okay, not currently an error
  123
} else {
  456
}

let y = if true {
  print("change this")    // currently an error
  123
} else {
  456
}

let z = if true {
  123    #warning("change this")    // currently an error
} else {
  456
}

I don’t know exactly what’s going on there, but it would get subsumed by this proposal.

• • •

The precise details of how this should work have a few possibilities:

  1. Allow #warning on any line by itself, with no other code (except comments).

  2. Allow both (1) and the use of #warning in trailing position, after all other code on a line, similar to //... comments (though those could still come after).

  3. Allow #warning in any position, including the middle of a line of code, similar to /*...*/ comments.

I personally lean toward option 2, though I’m not tied to that choice. The important thing is to enable the use of #warning within multi-line expressions and declarations, where it is currently an error.

(For consistency, I suppose I ought to propose the same thing for #error, even though it would just trade one error for another. So let’s tack that on as well.)

7 Likes
2

I think you've described useful functionality, but as the question has already been raised with respect to #if as a compiler directive in some of the same places, my sense is that we ought to tackle this holistically rather than one at a time. Yes, we've been kicking the can down the road for a decade, but it feels like at some point we really do need to subsume these case-by-case ideas and make a solid argument for a lexical approach across the board, rather than reviewing one-off use cases (cf. SE-0330).

I really like your idea of thinking about how comments are treated. If we can settle on a model where (all? some?) non-paired compiler directives are treated as moral equivalents of /* ... */ and paired conditional compiler directives (#if...#endif) are parsed "both ways" (i.e., there must be valid code whether or not the stuff inside is also considered part of the comment), then I think all the desired functionality might fall out automatically:

let x = [
  1,
#if BLAH
  2, #warning("AHHH")
#endif
  3,
  4, #error("AHHH")
]

// parsed as...

let x = [
  1,
/* comment */
  2, /* comment */
/* comment */
  3,
  4, /* comment */
]

// ...and as...

let x = [
  1,
/* comment
  2, /* comment */
   comment */
  3,
  4, /* comment */
]
11 Likes
3

This could probably be solved with tooling, e.g.:

foo(
  a: 123,
  b: 456, // TODO: change this
  c: 789
)

Some linters already have on option of issuing warnings for TODO comments (swift-format doesn't do this as of now).

4

Swift Testing has an @__testing(warning:) pragma-like attribute macro we use to generate warnings in attribute position. Would be nice if we could just use #warning(_:) there instead.

5

I agree that parsing #warning and #error as trivia seems like it would be a big win, since they don't provide any other semantic information. Just let the compiler emit the diagnostics when it sees them after the parse and then nothing else further down the chain has to worry about them.

I'm less sure how well that works for #if blocks. It's not so much parsing it "both ways" as it is parsing it "N ways" when you have #if/#elseif/#elseif/.../#else, where N is the total number of branch combinations in the file. You'd either have to present the user with N total independent trees up front (a lot more complexity/memory usage than the current implementation, and what would tools that want to operate on/transform a single view of the file like swift-format do with that?), or you would have to restructure how basically all nodes are represented in the tree to present just one tree but let the user inspect the different branches. I don't think it falls out as automatically as you suggest, and it would be a regression to say that the user has to provide a build configuration in order to get a single syntax tree out of the parser.

From the POV of someone who's written a ton of syntax-based rules, it's definitely very easy to forget to handle IfConfigDecls wherever they might appear. I would love to have a way to prevent that at the API level. But while the current representation has drawbacks, it's fairly predictable and lightweight. Solving this in general is a lot more complicated of a design problem. I think we would need to overhaul the APIs to basically force the user to consider the possibility of an #if in any position to make it harder for someone to forget to consider those situations (e.g., every node property becomes something like an Either<ActualNodeType, IfBlock>, or some sort of mapping from conditions to nodes), and that would most certainly make the syntax APIs used to query and rewrite trees a lot gnarlier to work with.

1 Like