[llvm-commits] PATCH: FileCheck to allow referencing vars defined on the same line

[Eli Bendersky]

Dmitri, thanks for the review, comments below.

> +  /// VariableDefs - Maps definitions of variables to their parenthesized
> +  /// capture numbers.
> +  /// E.g. for the pattern "foo[[bar:.*]]baz", VariableDefs will map
> "bar" to 1.
>
> Please don't duplicate function or class name inside the comment.
> Please also don't duplicate the comment in .cpp file.  Old code is
> written that way, but current coding style advises not to duplicate.
>
> This link may be helpful:
> <http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments>.
>

I'm well familiar with this document. Specifically, with the big
boldly emphasized section at its top:

"If you are extending, enhancing, or bug fixing already implemented
code, use the style that is already being used so that the source is
uniform and easy to follow."

The code I added follows the style in FileCheck.cpp

> Please also don't duplicate the comment in .cpp file.

I'm not sure what this refers to, could you clarify?


> +  std::map<StringRef, unsigned> VariableDefs;
>
> StringMap?

I tried with StringMap first, but since it doesn't have a functional
copy constructor it didn't work.

>
> +          std::string backref = std::string("\\") +
>
> This should be 'Backref'.  Also trailing whitespace.
>

OK, will fix.

> +void Pattern::AddBackrefToRegEx(StringRef Backref) {
> +  RegExStr += Backref;
> +}
>
> Maybe this should take an 'unsigned' parameter?

This is a good idea, I'll look into it.

>
>> If this looks alright to people, I will also update the FileCheck
>> documentation and probably add some more tests.
>
> Please add tests that check that we count parentheses in user's
> regexes correctly (i.e., that CurParen is correct).
>

I'm not sure I understand what you mean here, could you give an
example? Is this testing the added feature or existing functionality?

Eli