[cfe-dev] Replacing the non-portable diff invocation coded as %diff_plist (and friends)

[Reid Kleckner via cfe-dev]

The lines with paths looked like this:
  <string>C:\src\llvm-project\clang\test\Analysis\unix-fns.c</string>

On Mon, Jun 10, 2019 at 6:20 PM Hubert Tong <
hubert.reinterpretcast at gmail.com> wrote:

> On Mon, Jun 10, 2019 at 7:29 PM Reid Kleckner <rnk at google.com> wrote:
>
>> I reverted this series of patches in r363007, since there were multiple
>> issues with the new approach on Windows:
>> - The heuristic for detecting path strings was incomplete, since it
>> didn't account for drive letter style paths.
>> - The -w option appeared to be necessary for ignoring CRLF.
>> - The '</string>$' portion of the new pattern doesn't appear to match
>> CRLF lines.
>>
>> This was enough that fixing forward was prohibitively difficult, so I
>> reverted them.
>>
> Thanks. Would you mind sending me an example path string, and I'll look
> into updating the path string detection. The `-b` option appears to take
> care of CRLF, and I'll add '[[:space:]]*' before '$' to account for CRLF.
>
>
>>
>> On Tue, Apr 2, 2019 at 1:08 PM Hubert Tong via cfe-dev <
>> cfe-dev at lists.llvm.org> wrote:
>>
>>> Various tests in `clang/test/Analysis/` invoke `diff` with a
>>> non-portable `-I` option. I do not believe that a requirement for a `diff`
>>> utility that supports such an extension is intended by the community. I
>>> believe that replacing `diff -I` with `diff` on files that have been
>>> normalized using `grep -v` is a reasonable solution. A minor detail is that
>>> the output being checked is produced without a newline at the end of the
>>> file. As part of the proposed solution, the file is rehabilitated as a text
>>> file (as required by `grep`) by adding such a newline. The same is done for
>>> the reference expected files that are missing a newline at the end of the
>>> file. The solution is sketched out below for feedback.
>>>
>>> The normalization is in many cases encoded in a lit substitution
>>> `%diff_plist`. A sample application of the proposed change would look like
>>> the following. See further below for additional rationale on the
>>> particulars.
>>>
>>> Replace the RUN line:
>>> -// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/unix-fns.c.plist
>>> -
>>> +// RUN: %normalize_plist <%S/Inputs/expected-plists/unix-fns.c.plist
>>> >%t.expected.sed.plist && echo >>%t.plist && %normalize_plist <%t.plist |
>>> diff -u %t.expected.sed.plist -
>>>
>>> Replace the lit substitution:
>>> -# Diff command used by Clang Analyzer tests (when comparing .plist files
>>> +# Filtering command used by Clang Analyzer tests (when comparing .plist
>>> files
>>>  # with reference output)
>>> -config.substitutions.append(('%diff_plist',
>>> -    'diff -u -w -I "<string>/" -I "<string>.:" -I "version"'))
>>> +config.substitutions.append(('%normalize_plist',
>>> +    "grep -Ev '%s|%s|%s'" %
>>> +        ('^[[:space:]]*<string>.* version .*</string>$',
>>> +         '^[[:space:]]*<string>/.*</string>$',
>>> +         '^[[:space:]]*<string>.:.*</string>$')))
>>>
>>> Why not use more `RUN` lines?
>>> The output being checked is line-number sensitive. Keeping the same
>>> number of `RUN` lines minimizes unnecessary noise and risk of errors.
>>>
>>> Why not use `sed`?
>>> `sed` can be used to normalize the lines that are expected to vary;
>>> however, the some of the expected-output files have these lines omitted.
>>> The `grep` replacement is more compatible with the behaviour of `diff
>>> -I`. The filtered files are nevertheless named with "sed" for the
>>> connotation that substitution took place.
>>>
>>> Why remove `-w` from `diff`?
>>> It appears that the `-w` option was being used for its effect (in some
>>> implementations) of ignoring the difference between a file that ends with a
>>> newline and one which does not. Following normalization of both files to
>>> end in a newline, this is no longer necessary. Since the effect is
>>> non-portable, keeping the option (and thus allowing the non-portable effect
>>> to occur, and perhaps become relied upon during development) is potentially
>>> harmful.
>>>
>>> Why not add the newline via `cat`?
>>> The newline can be added (in this context) using `echo | cat file -` as
>>> opposed to using `echo >>file`; however, the `lit` implementation of `cat`
>>> does not operate as expected on the standard input.
>>> _______________________________________________
>>> cfe-dev mailing list
>>> cfe-dev at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190610/a288a4e3/attachment.html>