Improved error recovery for _Pragma

[rcraik]

Summary

Currently, if the argument to _Pragma is not a parenthesised string literal, the bad token will be consumed, as well as the ')', if present. If additional bad tokens are passed to the _Pragma, this results in extra error messages which may distract from the true problem. For example:

temp.c:1:1: error: _Pragma takes a parenthesized string literal
_Pragma(unroll 1)
^
temp.c:1:16: error: expected identifier or '('
_Pragma(unroll 1)
               ^
2 errors generated.
}

The proposed patch causes all tokens to be consumed until the closing ')' or a new line, whichever is reached first.

[hubert.reinterpretcast]

The old code would have handled this okay:

_Pragma(
BAD_TOKEN
)

At the same time, the case could have looked like this:

_Pragma( void bar() {
  foo();
}
/**EOF**/

Both of these could be handled by using a matched parentheses approach: it eats the _Pragma in the first, and erases the rest of the file in the second (avoiding spurious messages following the _Pragma).

[rcraik]

In regards to the previous comment, a BalancedDelimiterTracker could (in theory) be used instead to eat all tokens until the closing ')', if it exists. However, _Pragma is handled by the preprocessor, and I believe BalancedDelimiterTracker can only used from within the parser.

With this patch I have tried to follow the style used elsewhere in the preprocessor.

[rcraik]

I've updated the patch to address the first case mentioned by Hubert. The behaviour with this patch will now be the same as the current trunk

[rcraik]

Ping

[rsmith]

LGTM

This looks like a strict improvement on what we did before, even though it doesn't perfectly handle all of Hubert's testcases.

lib/Lex/Pragma.cpp

if (Tok.isNot(tok::l_paren)) {
     Diag(PragmaLoc, diag::err__Pragma_malformed);
     return _PragmaLexing.failed();
   }
 
   // Read the '"..."'.
   Lex(Tok);
   if (!tok::isStringLiteral(Tok.getKind())) {
     Diag(PragmaLoc, diag::err__Pragma_malformed);
-    // Skip this token, and the ')', if present.
+    // Skip bad tokens, and the ')', if present.
     if (Tok.isNot(tok::r_paren) && Tok.isNot(tok::eof))
       Lex(Tok);
+    while (Tok.isNot(tok::r_paren) &&
+           !Tok.isAtStartOfLine() &&
+           Tok.isNot(tok::eof))
+      Lex(Tok);
     if (Tok.is(tok::r_paren))
       Lex(Tok);
     return _PragmaLexing.failed();
   }
 
   if (Tok.hasUDSuffix()) {
     Diag(Tok, diag::err_invalid_string_udl);
     // Skip this token, and the ')', if present.

test/Preprocessor/_Pragma.c

 // RUN: %clang_cc1 %s -verify -Wall
 
 _Pragma ("GCC system_header")  // expected-warning {{system_header ignored in main file}}
 
 // rdar://6880630
 _Pragma("#define macro")    // expected-warning {{unknown pragma ignored}}
 
 _Pragma("") // expected-warning {{unknown pragma ignored}}
 _Pragma("message(\"foo \\\\\\\\ bar\")") // expected-warning {{foo \\ bar}}
 
 #ifdef macro
 #error #define invalid
 #endif
 
+_Pragma(unroll 1 // expected-error{{_Pragma takes a parenthesized string literal}}
+
+_Pragma(clang diagnostic push) // expected-error{{_Pragma takes a parenthesized string literal}}
+
 _Pragma( // expected-error{{_Pragma takes a parenthesized string literal}}