jdenny added inline comments.
================ Comment at: llvm/test/FileCheck/comment/blank-comments.txt:9 + +CHECK: .chk:2:8: remark: CHECK: expected string found in input ---------------- jhenderson wrote: > 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? That's right. ================ 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. + ---------------- jhenderson wrote: > 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)? > 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! That's right, but check prefixes have this problem too. That is, you can do things like `-check-prefixes=FOO,FOO-NOT` so that `FOO-NOT` is not negative. `ValidatePrefixes` should be extended to catch such cases, I think. But in a separate patch. > 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)? Hmm. I think it's behavior we don't want to support. Maybe the test case should be added when extending `ValidatePrefixes` as I described above? 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
