[cfe-dev] [PATCH] add regex/globbing to -verify diagnostics

[mike-m]

Hi Chris, thanks for taking a look at this patch.

Patch is updated to your specs and attached. Summary of changes since last iteration:

- removed globbing support
- invalid regex now reports proper diagnostic instead of using assert mechanism
- reworked "\\n" -> '\n' substitution with StringRef
- parameterized various verify diagnostic error definitions for { string | regex } differentiation

Existing lit tests show no measurable performance impact.

--mike-m
-------------- next part --------------
A non-text attachment was scrubbed...
Name: verify.regex2.patch
Type: application/octet-stream
Size: 22466 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100427/4d469548/attachment.obj>
-------------- next part --------------


On 2010-04-26, at 5:41 PM, Chris Lattner wrote:

> On Apr 25, 2010, at 10:36 PM, mike-m wrote:
>> Patch update uses suggested '-re' and '-glob' syntax, patch attached.
>> 
>> - lit tests pass with clang-debug, clang-release and selfclang-debug builds
>> - no measurable performance impact on lit tests
> 
> I also prefer the "-re" suffix, thanks for doing that.
> 
> This is really cool, but I think it's a bit overkill.  I'd rather not support glob at all, it adds a bunch of complexity for no added advantage.
> 
> You converted some loops like this:
> 
> -    std::string Msg(CommentStart, ExpectedEnd);
> -    std::string::size_type FindPos;
> -    while ((FindPos = Msg.find("\\n")) != std::string::npos)
> -      Msg.replace(FindPos, 2, "\n");
> -    // Add is possibly multiple times.
> -    for (int i = 0; i < Times; ++i)
> -      ExpectedDiags.push_back(std::make_pair(Pos, Msg));
> 
> to explicit for loops with switches in them.  If you're going to change them, please convert them to using the StringRef API and its algorithms.
> 
> It looks like using an invalid regex in an directive line will cause an assertion, pleasem ake it be an error from -verify mode.
> 
> Thanks for working on this!
> 
> -Chris