You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit 0f3e317
Browse filesBrowse the repository at this point in the historyBrowse files
fix sugar expression lifetimes, aliased assignment, and operand lengths (#1513)
* hold nested sugar expressions and views by value
Sugar nodes held every operand by const reference, so an expression kept past the full-expression that built it (e.g. in an auto variable) referred to destroyed temporaries. Nodes now hold nested expressions and views by value, and only objects that own an R object (vectors, matrices) by reference.
Part of #1512.
* evaluate non-elementwise sugar before writing it in place
Assigning a same-length sugar expression to a vector, or any expression to a range, matrix row or matrix column, writes into existing storage while the expression is still being read. When the expression read from that storage at other positions (e.g. `x = rev(x)`, `x[Range(1, n - 1)] = head(x, n - 1)`), the result was silently wrong.
Elementwise nodes now declare themselves via a nested `rcpp_elementwise` type; those are still written in place, and everything else is evaluated into a temporary first.
Part of #1512.
* check that sugar operands have matching lengths
Sugar operations on two or more vectors took their length from one operand and never compared it with the others, so a shorter operand was read past its end and a longer one was silently truncated. Likewise for vectors assigned into a range, matrix row or matrix column. These now signal an error, matching the documented requirement that operands have the same size.
Part of #1512.
* let rows and columns read matching views in place
Evaluating every expression that reads a view into a temporary made updates like `m(_, j) = m(_, j) * 2` several times slower. Whether reading a view in place is safe depends on where the result is written: a column can read other columns of the same matrix (they coincide or are disjoint), a row other rows, and a whole vector any view, but a range can't read another range. Sugar expressions now list their elementwise operands, views declare what kind of view they are, and each write site checks the expression against its own kind of target.
Part of #1512.
* keep sugar length-check errors out of line
With the error path inlined, the compiler could no longer propagate constants into the loops following these checks (e.g. `m(_, j) = m(_, j) * 2.0` lost its constant multiplier and vectorized half as wide).
* document sugar operand lengths, lifetimes, and assignment
* update NEWS
* fix !, &&, and || on single logical results
None of `!all(x)`, `all(x) && any(y)`, `all(x) || any(y)`, `all(x) && true` or `all(x) || true` compiled: the combinators called the non-const get() through const references, the && and || overloads for two results declared their second parameter with the first one's template arguments (so they could never be deduced), and Or_SingleLogicalResult_bool named the And class as its CRTP base.
The combinators now hold their operands by value, like other sugar expressions, and the general && and || follow R's three-valued logic (NA && FALSE is FALSE, NA || TRUE is TRUE).
* update NEWS
* route range-to-range assignment through the aliasing checks
Also mark Range as elementwise, include sugar_operand.h from
is_elementwise.h, and drop stale FIXME comments in ifelse.h.
* read subset elements before writing when both index the same vector
* index lapply operands through VectorBase so vectors yield their stored type
Holding the operand as the derived vector type made lapply() pass a
proxy (e.g. const_string_proxy for character vectors) to the function,
which no longer converted to a const CharacterVector& argument. Seen as
a compile failure in the touch package.
* update ChangeLog
* enable the sugar length checks in development versions only
Twelve CRAN packages rely on a longer sugar operand being cut short, so
an unconditional error would block the release until all of them
update. Derive RCPP_SUGAR_LENGTH_CHECKS from the version in config.h:
on when the fourth component is non-zero (development snapshots, which
is what reverse-dependency checks run against) and off in releases.
Defining the macro before including Rcpp.h overrides the default. The
tests check for the errors only when enabled, and for the truncation
otherwise.
---------
Co-authored-by: Dirk Eddelbuettel <edd@debian.org>
0 commit comments