[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

This revision was automatically updated to reflect the committed changes. Closed by commit rL369049: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug (authored by jdenny, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61466/new/ https://reviews.llvm.org/D61466 ___ cfe-commits mailing list cf

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny added a comment. In D61466#1613571 , @jdenny wrote: > Note the reproducer in the implementation's FIXME. The other FIXME points > there, so I figure it's best not to duplicate the note. @jkorous, is this change sufficient? Thanks. CHANGES SIN

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny updated this revision to Diff 213197. jdenny added a comment. Note the reproducer in the implementation's FIXME. The other FIXME points there, so I figure it's best not to duplicate the note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61466/new/ https://reviews.llvm.org/D614

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jkorous added a comment. In D61466#1602928 , @jdenny wrote: > In D61466#1602917 , @jkorous wrote: > > > > > > In an inline comment, you also mentioned the alternative of replacing > `EXPECT_EQ` with `EXPECT_NE`. N

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny added a comment. In D61466#1602917 , @jkorous wrote: > Hi @jdenny, thanks for the warning! Hi. Thanks for the review. > I think having the test disabled is fine - the main upside I see is that we > won't check it fails over and over on our CI s

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jkorous added a comment. Hi @jdenny, thanks for the warning! I think having the test disabled is fine - the main upside I see is that we won't check it fails over and over on our CI systems. Could you please mention the test/reproducer in those FIXMEs? Otherwise LGTM. Comment

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61466/new/ https://reviews.llvm.org/D61466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny added a comment. Ping. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61466/new/ https://reviews.llvm.org/D61466 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61466: [Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug

jdenny updated this revision to Diff 208795. jdenny retitled this revision from "[Rewrite][NFC] Add FIXME about RemoveLineIfEmpty" to "[Rewrite][NFC] Add FIXMEs and tests for RemoveLineIfEmpty bug". jdenny edited the summary of this revision. jdenny added a comment. Rebase, and add a unit test t