Miscellaneous parser cleanup (#429)

- Give unary `-` and `not` the same precedence as in C++
- Add detail to parse error messages, and make the --trace flag also enable parser debug tracing
- Make any new shift-reduce conflicts into build errors
- Use `%precedence` rather than `%nonassoc` where possible, in order to catch more grammar bugs at build time

executable_semantics/main.cpp

@@ -12,8 +12,6 @@
 #include "llvm/Support/CommandLine.h"
 
 int main(int argc, char* argv[]) {
-  // yydebug = 1;
-
   using llvm::cl::desc;
   using llvm::cl::opt;
   opt<bool> trace_option("trace", desc("Enable tracing"));

executable_semantics/syntax/parse.cpp

@@ -29,7 +29,11 @@ auto parse(const std::string& input_file_name)
   std::optional<AST> parsed_input = std::nullopt;
   ParseAndLexContext context(input_file_name);
 
-  auto syntax_error_code = yy::parser(parsed_input, context)();
+  auto parser = yy::parser(parsed_input, context);
+  if (tracing_output) {
+    parser.set_debug_level(1);
+  }
+  auto syntax_error_code = parser();
   if (syntax_error_code != 0) {
     return syntax_error_code;
   }

executable_semantics/syntax/parser.ypp

@@ -19,6 +19,12 @@
 // ‘make_YYEOF’, for the end of input.
 %define api.token.constructor
 
+// Make parse error messages more detailed
+%define parse.error verbose
+
+// Enable support for parser debugging
+%define parse.trace true
+
 //
 // Parameters to the parser and lexer
 //
@@ -31,6 +37,18 @@
 // "inout" parameter passed to both the parser and the lexer.
 %param {Carbon::ParseAndLexContext& context}
 
+// The following shift-reduce conflicts are expected; any others should be
+// treated as errors:
+// - The "dangling else" ambiguity: `if (b) if (c) x = 1; else x = 2;`
+//   could parse as either `if (b) { if (c) x = 1; else x = 2;}` or
+//   `if (b) { if (c) x = 1; } else x = 2;`. Following C++, we want Carbon
+//   to choose the first option. Resolving this by restructuring the grammar
+//   would make it harder to read, and resolving it by assigning precedence to
+//   `if` and `else` could mask other ambiguities, especially if we allow
+//   `if`/`else` in expressions, so we allow Bison to resolve it through its
+//   default behavior of preferring to shift rather than reduce.
+%expect 1
+
 // -----------------------------------------------------------------------------
 
 %code top {
@@ -145,13 +163,15 @@ void yy::parser::error(
   COLON ":"
 ;
 
-%nonassoc "{" "}"
-%nonassoc ":" "," DBLARROW
+%precedence "{" "}"
+%precedence ":" "," DBLARROW
 %left OR AND
-%nonassoc EQUAL_EQUAL NOT
+%nonassoc EQUAL_EQUAL
 %left "+" "-"
+%precedence NOT UNARY_MINUS
 %left "." ARROW
-%nonassoc "(" ")" "[" "]"
+%precedence "(" ")" "[" "]"
+
 %start input
 %locations
 %%
@@ -200,7 +220,7 @@ expression:
     { $$ = Carbon::MakeBinOp(yylineno, Carbon::Operator::Or, $1, $3); }
 | NOT expression
     { $$ = Carbon::MakeUnOp(yylineno, Carbon::Operator::Not, $2); }
-| "-" expression
+| "-" expression %prec UNARY_MINUS
     { $$ = Carbon::MakeUnOp(yylineno, Carbon::Operator::Neg, $2); }
 | expression tuple
     { $$ = Carbon::MakeCall(yylineno, $1, $2); }