[PATCH] Change semantics of regex expectations in the diagnostic verifier

Richard Smith richard at metafoo.co.uk
Wed Dec 11 15:08:52 PST 2013



================
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:254-274
@@ +253,23 @@
+  // level is found. When true, P marks begin-position of CloseBrace.
+  bool SearchClosingBrace(StringRef OpenBrace, StringRef CloseBrace) {
+    unsigned Depth = 1;
+    P = C;
+    while (P < End) {
+      StringRef S(P, End - P);
+      if (S.startswith(OpenBrace)) {
+        ++Depth;
+        P += OpenBrace.size();
+      } else if (S.startswith(CloseBrace)) {
+        --Depth;
+        if (Depth == 0) {
+          PEnd = P + CloseBrace.size();
+          return true;
+        }
+        P += CloseBrace.size();
+      } else {
+        ++P;
+      }
+    }
+    return false;
+  }
+
----------------
Hans Wennborg wrote:
> Richard Smith wrote:
> > I don't think this does the right thing for cases like "\{{" and "\}}".
> These directives don't really support any escaping (except for \n apparently, and it will get \\n "wrong"). I'd rather hold off adding support for escaping if you don't think this is important.
Fine by me. We've always had this problem, and no-one has ever cared.

================
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:880
@@ -825,4 +879,3 @@
                              unsigned Min, unsigned Max) {
-  if (RegexKind)
-    return new RegexDirective(DirectiveLoc, DiagnosticLoc, Text, Min, Max);
-  return new StandardDirective(DirectiveLoc, DiagnosticLoc, Text, Min, Max);
+  if (!RegexKind || Text.find("{{") == StringRef::npos)
+    return new StandardDirective(DirectiveLoc, DiagnosticLoc, Text, Min, Max);
----------------
Hans Wennborg wrote:
> Richard Smith wrote:
> > Should we diagnose a `-re` match that doesn't contain a `'{{'`? Either that, or drop the `-re` form entirely and always allow regexps?
> My first idea was actually to drop the -re forms, as you say. However, double curly braces come up in diagnostic output that we want to match, which seemed like a good reason not to do this. For example  test\SemaCXX\constexpr-printing.cpp:41.
> 
> I'll add a diagnostic.
How common are these `}}`-in-expected-text issues? If it's just `constexpr-printing.cpp`, I'd be OK changing those to say `{{[{][{]}}blah{{[}][}]}}` or some similar ugliness. But I'm also fine with this as-is; it seems like a strict improvement to me.


http://llvm-reviews.chandlerc.com/D2388



More information about the cfe-commits mailing list