[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked an inline comment as done. jdenny added inline comments. 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: >

[PATCH] D79276: [FileCheck] Support comment directives

This revision was automatically updated to reflect the committed changes. Closed by commit rG9a9a5f9893c8: [FileCheck] Support comment directives (authored by jdenny). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/

[PATCH] D79276: [FileCheck] Support comment directives

jhenderson added inline comments. 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. + jdenny wrote: > jhenderson wrote: > > jdenny wrote: > > >

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked 2 inline comments as done. jdenny added a comment. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D79276: [FileCheck] Support comment directives

thopre accepted this revision. thopre added a comment. LGTM. Sorry for the late review. Comment at: llvm/test/FileCheck/comment/after-words.txt:1 +# Comment prefixes are not recognized at ends of words. + jdenny wrote: > jdenny wrote: > > thopre wrote: > > >

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/test/FileCheck/comment/after-words.txt:1 +# Comment prefixes are not recognized at ends of words. + jdenny wrote: > thopre wrote: > > How about characters that cannot be part

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked an inline comment as done. jdenny added inline comments. Comment at: llvm/test/FileCheck/comment/after-words.txt:1 +# Comment prefixes are not recognized at ends of words. + thopre wrote: > How about characters that cannot be part of an identifier?

[PATCH] D79276: [FileCheck] Support comment directives

thopre added inline comments. Comment at: llvm/test/FileCheck/comment/after-words.txt:1 +# Comment prefixes are not recognized at ends of words. + How about characters that cannot be part of an identifier? E.g. would "@COM:" be interpreted as a comment? Should

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked 5 inline comments as done. jdenny added a comment. In D79276#2024411 , @jhenderson wrote: > LGTM, but might not hurt to have someone else looking at it. I'll probably wait until Monday to give others more time to participate. Thanks for

[PATCH] D79276: [FileCheck] Support comment directives

jhenderson accepted this revision. jhenderson added a comment. This revision is now accepted and ready to land. LGTM, but might not hurt to have someone else looking at it. Comment at: llvm/test/FileCheck/comment/bad-comment-prefix.txt:40 +RUN:

[PATCH] D79276: [FileCheck] Support comment directives

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

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 262406. jdenny marked 8 inline comments as done. jdenny added a comment. Addressed remaining reviewer comments except one, which is still under discussion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276

[PATCH] D79276: [FileCheck] Support comment directives

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

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 262185. jdenny added a comment. My last update accidentally included D79375 . This strips it back out. Sorry about that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276

[PATCH] D79276: [FileCheck] Support comment directives

jdenny added inline comments. Comment at: llvm/lib/Support/FileCheck.cpp:1928 static const char *DefaultCheckPrefixes[] = {"CHECK"}; +static const char *DefaultCommentPrefixes[] = {"COM", "RUN"}; jhenderson wrote: > Similar to what I mentioned in D79375,

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 262141. jdenny marked 14 inline comments as done. jdenny added a comment. Applied most reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Files: clang/test/CodeGen/default-address-space.c

[PATCH] D79276: [FileCheck] Support comment directives

jhenderson 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:"

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 261998. jdenny added a comment. Fixed a missed rename in the test code in the last update. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Files: clang/test/CodeGen/default-address-space.c

[PATCH] D79276: [FileCheck] Support comment directives

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:"

[PATCH] D79276: [FileCheck] Support comment directives

jdenny updated this revision to Diff 261962. jdenny marked 11 inline comments as done. jdenny added a comment. Addressed most reviewer comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79276/new/ https://reviews.llvm.org/D79276 Files: clang/test/CodeGen/default-address-space.c

[PATCH] D79276: [FileCheck] Support comment directives

jdenny added a comment. In D79276#2017742 , @jdenny wrote: > Thanks for the review. I've replied to a few comments, and I'll address the > rest later. > > In D79276#2017101 , @jhenderson > wrote: > > > I

[PATCH] D79276: [FileCheck] Support comment directives

jdenny marked 4 inline comments as done. jdenny added a comment. Thanks for the review. I've replied to a few comments, and I'll address the rest later. In D79276#2017101 , @jhenderson wrote: > I hesitate to suggest this, but is it worth using `REM` as

[PATCH] D79276: [FileCheck] Support comment directives

jhenderson added a comment. I hesitate to suggest this, but is it worth using `REM` as a comment prefix? It's the comment marker for Windows batch files, so has some precedence. Comment at: llvm/docs/CommandGuide/FileCheck.rst:57 + By default, FileCheck ignores any occurrence

[PATCH] D79276: [FileCheck] Support comment directives

jdenny created this revision. jdenny added reviewers: probinson, thopre, hfinkel, jhenderson, jroelofs, jdoerfert. Herald added subscribers: cfe-commits, hiraditya, arichardson, Anastasia. Herald added projects: clang, LLVM. Sometimes you want to disable a FileCheck directive without removing it