[cfe-commits] [Patch 1 of 7] -verify fixes and enhancement

[Jordan Rose]

I'm reviewing these in order, so I apologize if my comments don't make sense in the larger context.


+#include <climits>

+    /// Constant representing one or more matches aka regex "+".
+    static const unsigned OneOrMoreCount =  UINT_MAX;

I assume this'll go away anyway in a later patch (when you allow "2+"), at which point you can drop the #include again. :-)


+  public:
+    static Directive* Create(bool RegexKind, const SourceLocation &Location,
+                             const std::string &Text, unsigned Count);

The "Text" argument should be a StringRef rather than std::string &.

Also, LLVM conventions says to attach the star to the "Create"...and also, really, that "Create" and "Match" should be lowerCamelCase, but I guess it's okay to leave them for now to avoid touching their code.


+        for (DirectiveList::iterator I = L->begin(), E = L->end(); I != E; ++I)
+          delete *I;

We have a nice helper function in STLExtras.h for this.


+typedef VerifyDiagnosticConsumer::Directive Directive;
+typedef VerifyDiagnosticConsumer::DirectiveList DirectiveList;
+typedef VerifyDiagnosticConsumer::ExpectedData ExpectedData;

I'm not sure if it's better to use 'typedef' or 'using' here. In header files we tend to prefer 'using' at namespace scope, but I'm not sure about .cpp files.


Other than those little points, seems like a reasonable base for later patches. Nothing really changed here anyway.
Jordan