<div dir="ltr"><div>The lines with paths looked like this:</div><div> <string>C:\src\llvm-project\clang\test\Analysis\unix-fns.c</string><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 10, 2019 at 6:20 PM Hubert Tong <<a href="mailto:hubert.reinterpretcast@gmail.com">hubert.reinterpretcast@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Jun 10, 2019 at 7:29 PM Reid Kleckner <<a href="mailto:rnk@google.com" target="_blank">rnk@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">I reverted this series of patches in r363007, since there were multiple issues with the new approach on Windows:<div>- The heuristic for detecting path strings was incomplete, since it didn't account for drive letter style paths.</div><div>- The -w option appeared to be necessary for ignoring CRLF.</div><div>- The '</string>$' portion of the new pattern doesn't appear to match CRLF lines.</div><div><br></div><div>This was enough that fixing forward was prohibitively difficult, so I reverted them.</div></div></blockquote><div>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.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Apr 2, 2019 at 1:08 PM Hubert Tong via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">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.<br></div><div dir="ltr"><br></div><div dir="ltr">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.<br><br>Replace the RUN line:<br><font size="2"><span style="font-family:monospace,monospace">-// RUN: cat %t.plist | %diff_plist %S/Inputs/expected-plists/</span></font><font size="2"><span style="font-family:monospace,monospace">unix-fns.c.plist -<br>+// RUN: %normalize_plist <%S/Inputs/expected-plists/</span></font><font size="2"><span style="font-family:monospace,monospace">unix-fns.c.plist >%t.expected.sed.plist && echo >>%t.plist && %normalize_plist <%t.plist | diff -u %t.expected.sed.plist -</span></font><br><br>Replace the lit substitution:<br><span style="font-family:monospace,monospace">-# Diff command used by Clang Analyzer tests (when comparing .plist files<br>+# Filtering command used by Clang Analyzer tests (when comparing .plist files<br> # with reference output)<br>-config.substitutions.append((</span><span style="font-family:monospace,monospace">'%diff_plist',<br>- 'diff -u -w -I "<string>/" -I "<string>.:" -I "version"'))<br>+config.substitutions.append((</span><span style="font-family:monospace,monospace">'%normalize_plist',<br>+ "grep -Ev '%s|%s|%s'" %<br>+ ('^[[:space:]]*<string>.* version .*</string>$',<br>+ '^[[:space:]]*<string>/.*</</span><span style="font-family:monospace,monospace">string>$',<br>+ '^[[:space:]]*<string>.:.*</</span><span style="font-family:monospace,monospace">string>$')))</span><br><br>Why not use more `RUN` lines?<br>The output being checked is line-number sensitive. Keeping the same number of `RUN` lines minimizes unnecessary noise and risk of errors.<br><br>Why not use `sed`?<br>`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.<br>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.<br><br>Why remove `-w` from `diff`?<br>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.<br><br>Why not add the newline via `cat`?<br>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.<br></div></div>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>
</blockquote></div></div>
</blockquote></div>