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

[Richard Smith]

================
Comment at: include/clang/Frontend/VerifyDiagnosticConsumer.h:124-127
@@ -122,6 +123,6 @@
 ///
-///   // expected-error-re {{variable has has type 'struct .'}}
-///   // expected-error-re {{variable has has type 'struct .*'}}
-///   // expected-error-re {{variable has has type 'struct (.*)'}}
-///   // expected-error-re {{variable has has type 'struct[[:space:]](.*)'}}
+///   // expected-error-re {{variable has has type 'struct {{.}}'}}
+///   // expected-error-re {{variable has has type 'struct {{.*}}'}}
+///   // expected-error-re {{variable has has type 'struct {{(.*)}}'}}
+///   // expected-error-re {{variable has has type 'struct{{[[:space:]](.*)}}'}}
 /// \endcode
----------------
"has has"? =)

================
Comment at: lib/Frontend/VerifyDiagnosticConsumer.cpp:848
@@ +847,3 @@
+// Add the characters from FixedStr to RegexStr, escaping as needed.  This
+// avoids "leaning toothpicks" in common patterns.
+static void AddFixedStringToRegEx(StringRef FixedStr, std::string &RegexStr) {
----------------
Took me a second to figure out what you meant here. Maybe instead say this avoids the need for backslash-escaping?

================
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);
----------------
Should we diagnose a `-re` match that doesn't contain a `'{{'`? Either that, or drop the `-re` form entirely and always allow regexps?

================
Comment at: test/Analysis/analyzer-stats.c:5
@@ -4,3 +4,3 @@
 
-int test() { // expected-warning-re{{test -> Total CFGBlocks: [0-9]+ \| Unreachable CFGBlocks: 0 \| Exhausted Block: no \| Empty WorkList: yes}}
+int test() { // expected-warning-re{{{{test -> Total CFGBlocks: [0-9]+ \| Unreachable CFGBlocks: 0 \| Exhausted Block: no \| Empty WorkList: yes}}}}
   int a = 1;
----------------
`{{{{`? :(

Can we write this as:

  // expected-warning-re{{test -> Total CFGBlocks: {{[0-9]+}} | Unreachable CFGBlocks: 0 | Exhausted Block: no | Empty WorkList: yes}}

?

================
Comment at: test/Misc/verify.c:13-14
@@ +12,3 @@
+struct s r3; // expected-error-re {{tentative definition has type '{{(.*)[[:space:]]*(.*)}}' that is never completed}}
+struct s r4; // expected-error-re {{{{^tentative}}}}
+struct s r5; // expected-error-re {{{{completed$}}}}
----------------
Maybe:

  {{{{^}}tentative}}
  {{completed{{$}}}}

================
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;
+  }
+
----------------
I don't think this does the right thing for cases like "\{{" and "\}}".


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