Implement #warning and #error by harlanhaskins · Pull Request #14048 · swiftlang/swift

harlanhaskins · 2018-01-22T00:38:43Z

This PR is an implementation of #warning and #error, which ~~is currently being pitched to swift-evolution~~ was accepted for Swift 5.

harlanhaskins · 2018-01-22T01:35:32Z

@swift-ci please smoke test

harlanhaskins · 2018-01-23T18:25:27Z

This currently doesn't handle directives in the middle of switch statements, a la:

switch 4 {
#warning "boo"
default: break
}

nkcsgexi · 2018-01-23T18:38:19Z

Finally... @harlanhaskins

harlanhaskins · 2018-02-02T01:58:24Z

@swift-ci please smoke test

harlanhaskins · 2018-02-02T03:24:47Z

@swift-ci please smoke test

harlanhaskins · 2018-02-02T12:03:48Z

lib/AST/ASTPrinter.cpp

+    Printer << tok::pound_warning; 
+  }
+
+  Printer << '"' << PDD->getMessage()->getValue() << '"';


Before I forget, I need to add parens to this.

harlanhaskins · 2018-02-02T16:25:51Z

@swift-ci please smoke test

skagedal · 2018-02-02T16:36:02Z

lib/AST/ASTWalker.cpp

  }

+  bool visitPoundDiagnosticDecl(PoundDiagnosticDecl *PDD) {
+    // By default, ignore #erorr/#warning.


Typo on "erorr" :)

jrose-apple · 2018-02-02T16:42:36Z

lib/Parse/ParseDecl.cpp

+
+  auto string = parseExprStringLiteral();
+  if (string.isNull())
+    return makeParserError();


Suggestion: continue looking for a close-paren if it occurs before the end of the line.

👍

I originally tried checking for #warning “Foo” and FixIt-ing in the surrounding parens. Should I continue with that?

That's also good, but I'm mostly thinking about #warning(Foo) and worse #warning(Foo is bad, yanno?), where the fix is probably to add quotes.

Also a good idea!

jrose-apple · 2018-02-02T16:43:46Z

lib/AST/ASTDumper.cpp


+    void visitPoundDiagnosticDecl(PoundDiagnosticDecl *PDD) {
+      const char *name = PDD->isError() ?
+        "pound_error_decl" : "pound_warning_decl";


Nitpick: seems like having a common pound_diagnostic_decl as the kind and then printing an attribute for error vs. warning would be more inline with the other AST nodes.

jrose-apple · 2018-02-02T16:45:09Z

lib/Sema/TypeCheckDecl.cpp


+  void visitPoundDiagnosticDecl(PoundDiagnosticDecl *PDD) {
+    if (PDD->hasBeenEmitted()) { return; }
+    PDD->markEmitted();


I suspect this can never happen more than once per phase, so you may be able to get rid of hasBeenEmitted.

I’ll try it and see what happens!

Turns out it does happen twice. ☹️

jrose-apple · 2018-02-02T16:47:09Z

test/Sema/pound_diagnostics.swift

+  switch 34 {
+  #warning("warnings can be nested in switch statements") // expected-warning {{warnings can be nested in switch statements}}
+  #if true
+  #error("errors can be nested in if-configs inside switch statements too") // expected-error {{errors can be nested in if-configs inside switch statements too}}


There's a slightly different test where the #error is immediately after a case.

Oops, sorry, I guess that's just part of the body of the case. It's where the #if is immediately after a case, but contains cases itself.

Okay, I’ll nest a case inside the existing if config and then put a #error in there 👍

harlanhaskins · 2018-02-02T22:45:44Z

@jrose-apple Okay, this latest commit will fix-it:

#warning "foo"
#warning test 123
#warning(test 123)

harlanhaskins · 2018-02-02T22:46:58Z

@swift-ci please smoke test

harlanhaskins · 2018-02-02T22:55:00Z

@swift-ci please smoke test

jrose-apple · 2018-02-02T23:38:18Z

lib/Parse/ParseDecl.cpp

+    // Catch #warning(oops, forgot the quotes)
+    SourceLoc wordsStartLoc = Tok.getLoc();
+
+    while (!Tok.isAtStartOfLine() && Tok.isNot(tok::r_paren)) {


Don't forget to consume the r_paren!

Ack! Thanks!

rintaro · 2018-02-03T01:51:17Z

Could you add check for trailing tokens on the same line? e.g.

#warning("foo bar") var x = 1 // expected-error {{extra tokens following warning directive}}
#error("foo bar"); // expected-error {{extra tokens following error directive}}

I'm not sure this was discussed, but we don't allow this for any directives so far.

harlanhaskins · 2018-02-03T02:30:38Z

@swift-ci please smoke test

harlanhaskins · 2018-02-03T04:10:18Z

Build timed out?

@swift-ci please smoke test Linux platform

DougGregor

This looks fantastic

harlanhaskins · 2018-02-03T23:07:00Z

🚢

natecook1000 · 2018-02-03T23:13:15Z

utils/vim/syntax/swift.vim


 syn match swiftPreproc /#\(\<file\>\|\<line\>\|\<function\>\)/
-syn match swiftPreproc /^\s*#\(\<if\>\|\<else\>\|\<elseif\>\|\<endif\>\)/
+syn match swiftPreproc /^\s*#\(\<if\>\|\<else\>\|\<elseif\>\|\<endif\>\<error\>\|\<warning\>\|\)/


This is what I call fit and finish 💯

DevAndArtist · 2018-02-06T08:37:41Z

Is this supported?

#warning("""
  This is a very long warning \
  in a codebase that has 80 \
  character line width. ...
  """)

harlanhaskins · 2018-02-06T14:49:31Z

Not in the current implementation, but it absolutely should be. I’ll make a PR with that soon. Thanks for noticing!

harlanhaskins · 2018-02-06T17:31:53Z

Actually, spoke too soon. Multi line string literals are definitely supported already -- I mistakenly thought they were a different token.

DougGregor · 2018-03-09T17:46:29Z

Hey @harlanhaskins , mind adding a ChangeLog entry for this?

harlanhaskins · 2018-03-09T17:56:49Z

@DougGregor Submitted as #15110. Thanks for the reminder!

* Implement #warning and #error * Fix #warning/#error in switch statements * Fix AST printing for #warning/#error * Add to test case * Add extra handling to ParseDeclPoundDiagnostic * fix dumping * Consume the right paren even in the failure case * Diagnose extra tokens on the same line after a diagnostic directive

harlanhaskins added the swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review label Jan 22, 2018

harlanhaskins force-pushed the caution-tape branch from d6fd1d2 to 3ca6ec6 Compare January 22, 2018 01:35

harlanhaskins mentioned this pull request Jan 22, 2018

Compiler Diagnostic Directives swiftlang/swift-evolution#783

Merged

harlanhaskins requested review from DougGregor and jrose-apple and removed request for DougGregor and jrose-apple January 22, 2018 19:09

harlanhaskins force-pushed the caution-tape branch from 3ca6ec6 to 76c1eed Compare February 1, 2018 20:31

harlanhaskins added swift evolution approved Flag → feature: A feature that was approved through the Swift evolution process and removed swift evolution pending discussion Flag → feature: A feature that has a Swift evolution proposal currently in review labels Feb 2, 2018

harlanhaskins changed the title ~~[Do Not Merge] Implement #warning and #error~~ [WIP] Implement #warning and #error Feb 2, 2018

Implement #warning and #error

ed5dbd3

harlanhaskins force-pushed the caution-tape branch from 80b24e5 to ed5dbd3 Compare February 2, 2018 01:57

harlanhaskins requested review from DougGregor and jrose-apple February 2, 2018 01:57

Fix #warning/#error in switch statements

3848ec0

harlanhaskins commented Feb 2, 2018

View reviewed changes

Fix AST printing for #warning/#error

ffcefa7

harlanhaskins force-pushed the caution-tape branch from 81ea7c5 to ffcefa7 Compare February 2, 2018 16:08

skagedal reviewed Feb 2, 2018

View reviewed changes

jrose-apple reviewed Feb 2, 2018

View reviewed changes

harlanhaskins added 2 commits February 2, 2018 12:10

Add to test case

6fb75a9

Add extra handling to ParseDeclPoundDiagnostic

f921dcd

harlanhaskins force-pushed the caution-tape branch from 7ac669f to f921dcd Compare February 2, 2018 22:44

fix dumping

c0de994

harlanhaskins force-pushed the caution-tape branch from 77f2946 to c0de994 Compare February 2, 2018 22:54

jrose-apple reviewed Feb 2, 2018

View reviewed changes

harlanhaskins changed the title ~~[WIP] Implement #warning and #error~~ Implement #warning and #error Feb 3, 2018

Consume the right paren even in the failure case

d4fe20f

harlanhaskins force-pushed the caution-tape branch from 80d0cb7 to d4fe20f Compare February 3, 2018 01:23

Diagnose extra tokens on the same line after a diagnostic directive

27c1ffe

harlanhaskins force-pushed the caution-tape branch from a306bee to 27c1ffe Compare February 3, 2018 02:29

DougGregor approved these changes Feb 3, 2018

View reviewed changes

harlanhaskins merged commit 5e02d2a into swiftlang:master Feb 3, 2018

natecook1000 reviewed Feb 3, 2018

View reviewed changes

harlanhaskins deleted the caution-tape branch February 3, 2018 23:27

harlanhaskins mentioned this pull request Mar 6, 2018

Add no-op visitor in SILGenFunction for #warning/#error #15008

Merged

kastiglione mentioned this pull request Aug 27, 2018

Fix swiftPreproc regex in swift.vim #18994

Merged

Conversation

harlanhaskins commented Jan 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harlanhaskins commented Jan 22, 2018

Uh oh!

harlanhaskins commented Jan 23, 2018

Uh oh!

nkcsgexi commented Jan 23, 2018

Uh oh!

harlanhaskins commented Feb 2, 2018

Uh oh!

harlanhaskins commented Feb 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harlanhaskins commented Feb 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

harlanhaskins commented Feb 2, 2018

Uh oh!

harlanhaskins commented Feb 2, 2018

Uh oh!

harlanhaskins commented Feb 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rintaro commented Feb 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harlanhaskins commented Feb 3, 2018

Uh oh!

harlanhaskins commented Feb 3, 2018

Uh oh!

DougGregor left a comment

Choose a reason for hiding this comment

Uh oh!

harlanhaskins commented Feb 3, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DevAndArtist commented Feb 6, 2018

Uh oh!

harlanhaskins commented Feb 6, 2018

Uh oh!

harlanhaskins commented Feb 6, 2018

Uh oh!

DougGregor commented Mar 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

harlanhaskins commented Mar 9, 2018

harlanhaskins commented Jan 22, 2018 •

edited

Loading

rintaro commented Feb 3, 2018 •

edited

Loading

DougGregor commented Mar 9, 2018 •

edited

Loading