From 417497632ddfa2bdc17688703c24ca3fd60af318 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Fri, 15 Nov 2019 09:27:04 +0200
Subject: Improve {}-imbalance diagnostics in cc::parser and make it warning

---
 libbuild2/cc/compile-rule.cxx              |  6 ++++--
 libbuild2/cc/parser+module.test.testscript |  8 ++++----
 libbuild2/cc/parser.cxx                    | 29 +++++++++++++++++++++++++----
 3 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/libbuild2/cc/compile-rule.cxx b/libbuild2/cc/compile-rule.cxx
index f3b2c0d..6e21a7e 100644
--- a/libbuild2/cc/compile-rule.cxx
+++ b/libbuild2/cc/compile-rule.cxx
@@ -4013,13 +4013,15 @@ namespace build2
 
       otype ot (li.type);
 
-      // If things go wrong give the user a bit extra context.
+      // If things go wrong give the user a bit extra context. Let's call it
+      // "scanning" instead of "parsing" since this has become an established
+      // term.
       //
       auto df = make_diag_frame (
         [&src](const diag_record& dr)
         {
           if (verb != 0)
-            dr << info << "while parsing " << src;
+            dr << info << "while scanning " << src;
         });
 
       // For some compilers (GCC, Clang) the preporcessed output is only
diff --git a/libbuild2/cc/parser+module.test.testscript b/libbuild2/cc/parser+module.test.testscript
index e935180..d305125 100644
--- a/libbuild2/cc/parser+module.test.testscript
+++ b/libbuild2/cc/parser+module.test.testscript
@@ -80,7 +80,7 @@ EOO
 
 : brace-missing
 :
-$* <<EOI 2>>EOE != 0
+$* <<EOI 2>>EOE
 export
 {
   class foo
@@ -89,12 +89,12 @@ export
   module foo;
 }
 EOI
-<stdin>:8:1: error: {}-imbalance detected
+<stdin>:8:1: warning: missing '}'
 EOE
 
 : brace-stray
 :
-$* <<EOI 2>>EOE != 0
+$* <<EOI 2>>EOE
 export
 {
   class foo
@@ -103,7 +103,7 @@ export
 }
 module foo;
 EOI
-<stdin>:6:1: error: {}-imbalance detected
+<stdin>:6:1: warning: extraneous '}'
 EOE
 
 : import-missing-name
diff --git a/libbuild2/cc/parser.cxx b/libbuild2/cc/parser.cxx
index 690f41c..706dc48 100644
--- a/libbuild2/cc/parser.cxx
+++ b/libbuild2/cc/parser.cxx
@@ -30,8 +30,16 @@ namespace build2
       // issue warnings. But the problem with this approach is that they are
       // easy to miss. So for now we fail. And it turns out we don't mis-
       // parse much.
+
+      // We keep a {}-balance and skip everything at depth 1 and greater.
+      // While after P1703 and P1857 everything that we are interested in
+      // (module and import declarations) are treated as pseudo-pp-directives
+      // and recognized everywhere, they are illegal everywhere execept at
+      // depth 0. So we are going to skip for performance reasons and expect
+      // the compiler to complain about the syntax rather than, say, module
+      // BMI not being found.
       //
-      size_t bb (0); // {}-balance.
+      int64_t bb (0);
 
       token t;
       for (bool n (true); (n ? l_->next (t) : t.type) != type::eos; )
@@ -50,7 +58,7 @@ namespace build2
           }
         case type::rcbrace:
           {
-            if (bb-- == 0)
+            if (--bb < 0)
               break; // Imbalance.
 
             continue;
@@ -60,9 +68,9 @@ namespace build2
             // Constructs we need to recognize:
             //
             //           module                              ;
+            // [export]  module <module-name> [<attributes>] ;
             // [export]  import <module-name> [<attributes>] ;
             // [export]  import <header-name> [<attributes>] ;
-            // [export]  module <module-name> [<attributes>] ;
             //
             // Additionally, when include is translated to an import, it's
             // normally replaced with the special __import keyword since it
@@ -100,8 +108,21 @@ namespace build2
         break;
       }
 
+      // We used to issue an error here but the diagnostics and, especially,
+      // the location are not very helpful. While the compilers don't do a
+      // much better job at it, there are often other semantic errors that are
+      // more helpful and which we cannot match. So now we warn and let the
+      // compiler fail.
+      //
+      // Another option would have been to postpone this diagnostics until
+      // after the compiler fails (and thus also confirming that it indeed has
+      // failed) but that would require propagating this state from apply() to
+      // perform_update() and also making sure we issue this diagnostics even
+      // if anything in between fails (probably by having it sitting in a
+      // diag_frame). So let's keep it simple for now.
+      //
       if (bb != 0)
-        /*warn*/ fail (t) << "{}-imbalance detected";
+        warn (t) << (bb > 0 ? "missing '}'" : "extraneous '}'");
 
       if (module_marker_ && u.module_info.name.empty ())
         fail (*module_marker_) << "module declaration expected after "
-- 
cgit v1.1