Thanks, I agree with the course of action, especially given literanger seems to be the only casualty. I'm just making note that the policy doesn't necessarily generate the outcome we want. No policy ever will! I 100% appreciate CRAN volunteers' efforts.
In this case, the compiler is generating a false positive - I'm reasonably sure of this having looked at the assembly generated. The kludge doesn't make the library perform 'as expected' on more systems => the kludge means the _compiler_ performs 'as expected' (on more systems). With the kludge: the code will perform marginally worse at run time => we are trading some runtime performance to avoid a (build-time) bug confined to the build tools, and it takes additional effort to maintain. Such is life. On Tue, May 13, 2025 at 11:34 AM Kevin Ushey <kevinus...@gmail.com> wrote: > > Hi Stephen, > > I still believe your best option is still to just submit a version of > your package that includes a workaround for this compiler issue. > > You could, in theory, try to contact the CRAN maintainers at > c...@r-project.org, and either (1) request an exception for your > package, or (2) request that they update the compilers used so that > this issue no longer occurs. However, the repository policy states: > > The time of the volunteers is CRAN’s most precious resource > > and so you're unlikely to be successful in petitioning for their time, > even if your diagnosis of the issue is correct. Especially so now, > since you have a workaround for the issue in question. (Of course, I > could be wrong; I obviously cannot speak for them.) > > I would strongly recommend just biting your tongue and submitting a > version of your package with the workaround. > > My own personal view: we must unfortunately accept that "good" code is > not necessarily neat nor concise code, and I say this as someone who > also hates having to write these sorts of kludges. However, what > ultimately matters is whether your software works as expected on the > systems where it is run; from that perspective, your kludge achieves > that goal on more systems than your code would otherwise. > > Best, > Kevin > > On Mon, May 12, 2025 at 5:32 PM Stephen Wade <stephematic...@gmail.com> wrote: > > > > After (a lot) more work, including looking for MWE (see > > https://godbolt.org/z/38nnaEcrf), I am confident this is a bug which > > has already been resolved: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111415. > > > > The bug only occurs when optimisation is enabled (-O1 to -O3 at > > least), on GCC 12.4, 13.1--13.3, and 14.1--14.2. > > > > The question is "what is the correct remedy for the package on CRAN"? > > Making the code _unnecessarily_ complicated compels GCC to avoid an > > optimisation pattern that is causing the warning, e.g. > > > > inline std::vector<double> adjust_pvalues(const std::vector<double> & > > unadjusted) { > > > > const size_t n_pvalue = unadjusted.size(); > > if (n_pvalue < 2) { > > #if ( __GNUC__ == 12 && __GNUC_MINOR__ > 3 && __GNUC_MINOR__ < 5 ) || \ > > ( __GNUC__ == 13 && __GNUC_MINOR__ < 4 ) || \ > > ( __GNUC__ == 14 && __GNUC_MINOR__ < 3 ) > > std::vector<double> kludge(n_pvalue); > > kludge = unadjusted; > > return kludge; /* no warning produced */ > > #else > > return unadjusted; > > #endif > > } > > > > std::vector<double> adjusted(n_pvalue, 0); > > /* Do some other stuff */ > > return adjusted; > > } > > > > That is _not_ "good" code practice (in my view). I would only put this > > in a `cran_release` branch of the repository because it's not > > something I'd like to see in `main`. That means a (manual) merge into > > `cran_release` each time I release the package for the next few years > > until CRAN uses compilers that don't produce that warning. Annoying, > > but not a deal-breaker. All this tells me (not that it is a bad > > policy) that the CRAN policy isn't always going to produce the best > > outcome in terms of code practise (again, in my view). > > > > On Mon, May 12, 2025 at 10:02 PM Tomas Kalibera > > <tomas.kalib...@gmail.com> wrote: > > > > > > > > > On 5/9/25 03:09, Stephen Wade wrote: > > > > The literanger package is no longer passing on CRAN > > > > (https://CRAN.R-project.org/package=literanger) due to array-bound > > > > warnings in GCC 13.3 and 14.2 (more details below). > > > > > > > > This _looks_ to me like one of either a) a compiler bug, b) a false > > > > positive, or c) (very unlikely) something in the standard library > > > > implementation. > > > > > > > > Have others seen warnings like this recently, and if so, what have you > > > > done about them? The warning did not appear in clang, nor with GCC > > > > 15.1.0 on CRAN's Fedora test service. > > > > > > > > Firstly, the relevant code snippet: > > > > > > > > /** Compute adjusted p-values using Benjamini/Hochberg method */ > > > > inline std::vector<double> adjust_pvalues(const std::vector<double> & > > > > unadjusted) { > > > > > > > > const size_t n_pvalue = unadjusted.size(); > > > > if (n_pvalue < 2) return unadjusted; /* <-- WARNING HERE */ > > > > > > > > std::vector<double> adjusted(n_pvalue, 0); > > > > /* Do some other stuff */ > > > > return adjusted; > > > > > > > > } > > > > > > > > > > > > Secondly, the warning (on my own machine, using GCC 13.2.0, which also > > > > has this problem): > > > > > > > > inlined from ‘std::vector<double> literanger::adjust_pvalues(const > > > > std::vector<double>&)’ at ../src/literanger/utility_math.h:99:48: > > > > /usr/include/c++/13/bits/stl_algobase.h:437:30: warning: ‘void* > > > > __builtin_memmove(void*, const void*, long unsigned int)’ writing > > > > between 9 and 9223372036854775807 bytes into a region of size 8 > > > > overflows the destination [-Wstringop-overflow=] > > > > 437 | __builtin_memmove(__result, __first, sizeof(_Tp) * > > > > _Num); > > > > > > Perhaps you could try creating a minimal reproducible example, > > > standalone C++ program, which still reproduces the warning, and report > > > to GCC (or libstdc++). You may get a response it is a duplicate of the > > > bug Ivan identified, but it still might be useful to have that > > > confirmed. It might also become clearer which problem the compiler seems > > > to see there - so give a hint for a workaround. > > > > > > Even if it gets confirmed as a compiler bug, it might be useful to work > > > it around when possible so that your code builds cleanly, particularly > > > if it doesn't complicate the code much. We are doing the same thing with > > > various warnings in base R - we try to simplify/change the code to help > > > the compiler see it is correct. Also, sometimes one finds on the way > > > that there actually is a (sometimes theoretical but, still) problem. > > > > > > Once you have a minimal reproducible example it might also be easier to > > > experiment with work-arounds. > > > > > > Best > > > Tomas > > > > > > > > > > > ______________________________________________ > > > > R-devel@r-project.org mailing list > > > > https://stat.ethz.ch/mailman/listinfo/r-devel > > > > ______________________________________________ > > R-devel@r-project.org mailing list > > https://stat.ethz.ch/mailman/listinfo/r-devel ______________________________________________ R-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-devel
