Martin~ Sorry about the bad patch. I work on C++ at Google. We built a check for clang-tidy that identifies errors of this form and discovered the error here as part of our search. I am just trying to be a good citizen and upstream a fix, but I must have gotten sloppy as I was doing a bunch of these.
Thanks for fixing it and finding the a test for it, I actually had no idea how to trigger this codepath and have never used R. If you are curious, the upstream check for clang-tidy is http://reviews.llvm.org/D18766 You may consider running some of the other clang-tidy checks on your source base, they will likely find other bugs. Cheers, Matt On Thu, Apr 7, 2016 at 6:46 AM, Martin Maechler <maech...@stat.math.ethz.ch> wrote: > >>>>> Matthew Fowles Kulukundis <matt.fow...@gmail.com> > >>>>> on Tue, 5 Apr 2016 11:17:30 -0400 writes: > > > All~ > > CHECK_this_length macro expands to multiple statements making it > unsafe to > > use in a single line `if` statement (as is happening near line > 335). This > > fixes the macro using the standard `do { } while (0)` macro trick. > > yes, but you forgot the closing '}' ... so you could not even > have compiled R after applying your patch. > > Also, it would be nice to contrive a minimal example where the > change makes a difference. This "fails" to trigger : > > -------------------------------- > as.double.foo <- function(x) x[FALSE] > x <- structure(3, class="foo") > as.numeric(x) # numeric(0) > > sprintf("%d !", x)# "3 !" instead of giving an error > -------------------------------- > > Thank you, Matt, in any case; this (with the "{" !) has now gone > into the source. > > Martin > > > Matt > > x[DELETED ATTACHMENT external: r-sprintf.patch, text/x-patch] > > ______________________________________________ > > R-devel@r-project.org mailing list > > https://stat.ethz.ch/mailman/listinfo/r-devel > [[alternative HTML version deleted]] ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
