jdenny added inline comments.

================
Comment at: llvm/docs/CommandGuide/FileCheck.rst:57
+ By default, FileCheck ignores any occurrence in ``match-filename`` of any 
check
+ prefix if it is preceded on the same line by "``COM:``" or "``RUN:``".  See 
the
+ section `The "COM:" directive`_ for usage details.
----------------
jhenderson wrote:
> Nit: the rest of this document uses single spaces after full stops. Please do 
> that in the new text too.
It doesn't matter to me which we use, especially given that it doesn't seem to 
affect how the HTML renders.  However, the majority of periods (ignoring `..` 
marking blocks) in this document that are followed by at least one space are 
followed by two spaces. Does that change your suggestion?


================
Comment at: llvm/lib/Support/FileCheck.cpp:1930
 
+bool FileCheck::ValidateCheckPrefixes() {
+  StringSet<> PrefixSet;
----------------
jhenderson wrote:
> It looks to me like the changes related to this function could be done 
> separately (perhaps first). Is that the case, and if so, could you do so? 
> It's a little hard to follow what's just refactoring of the existing stuff 
> and what is new, with the extra comment stuff thrown in.
Good idea: D79375


================
Comment at: llvm/test/FileCheck/comment.txt:38-43
+// RUN: echo 'foo COM: bar'        > %t.in
+// RUN: echo 'CHECK: foo COM: bar' > %t.chk
+// RUN: %ProtectFileCheckOutput FileCheck -vv %t.chk < %t.in 2>&1 \
+// RUN: | FileCheck -check-prefix=WITHIN-CHECKS %s
+//
+// WITHIN-CHECKS: .chk:1:8: remark: CHECK: expected string found in input
----------------
jhenderson wrote:
> I'm struggling a little with this case. Firstly, why the '8' in the column 
> count in the remark? Secondly, if COM: was being treated as a genuine comment 
> here, then the check directive would become `CHECK: foo` which would still be 
> a match of the input, if I'm not mistaken? I guess the two might somehow be 
> related, but I don't know how if so.
> Firstly, why the '8' in the column count in the remark?

Are you asking why it's not `1`?  Here, FileCheck gives the location of the 
first character of the pattern from the check file.

> Secondly, if COM: was being treated as a genuine comment here, then the check 
> directive would become CHECK: foo which would still be a match of the input, 
> if I'm not mistaken?

Ah, thanks.  Fixed.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79276/new/

https://reviews.llvm.org/D79276



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to