jhenderson added inline comments.
================
Comment at: llvm/lib/Support/FileCheck.cpp:1928
static const char *DefaultCheckPrefixes[] = {"CHECK"};
+static const char *DefaultCommentPrefixes[] = {"COM", "RUN"};
----------------
jdenny wrote:
> jhenderson wrote:
> > Similar to what I mentioned in D79375, perhaps it would make more sense for
> > COM and RUN to be defined by the tool itself rather than the library to
> > allow users to customise their respective front-ends as they wish.
> I suggested there that we handle the check prefix side of that change in a
> separate patch. Perhaps the same patch could handle the comment prefix side
> of it.
>
> Do you see a reason to handle this before committing the current patches?
I don't, as there's no current use-case for it. Let's leave it as-is for now.
================
Comment at: llvm/test/FileCheck/comment/bad-comment-prefix.txt:36
+RUN: %ProtectFileCheckOutput not FileCheck /dev/null < /dev/null 2>&1 \
+RUN: -check-prefixes=FOO,COM | \
+RUN: FileCheck -check-prefix=CHECK-PREFIX-DUP-COMMENT %s
----------------
Perhaps worth checking against RUN as well as COM?
================
Comment at: llvm/test/FileCheck/comment/blank-comments.txt:9
+
+CHECK: .chk:2:8: remark: CHECK: expected string found in input
----------------
I'm assuming that FileCheck treats the entire line after the first `CHECK:`
here as part of the CHECK, and doesn't try to start a second CHECK directive?
================
Comment at: llvm/test/FileCheck/comment/suffixes.txt:1-2
+# Comment prefixes plus check directive suffixes are not comment directives
+# and are treated as plain text.
+
----------------
I don't think you should change anything here, but if I'm following this right,
this leads to the amusing limitation that you can "comment out" (via
--comment-prefixes) any CHECK lines that use a suffix (CHECK-NEXT, CHECK-NOT
etc) but not those that don't without changing --check-prefixes value!
By the way, do we need to have a test-case for that? I.e. that
--comment-prefixes=CHECK-NEXT disables the CHECK-NEXT lines (assuming it does
of course)?
================
Comment at: llvm/test/FileCheck/comment/suppresses-checks.txt:1
+Comment directives successfully comment out check directives.
+
----------------
Missing '#'
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
