Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile error in 0.7.3 #3106

Closed
Jafet opened this issue Sep 15, 2017 · 12 comments
Closed

Compile error in 0.7.3 #3106

Jafet opened this issue Sep 15, 2017 · 12 comments
Labels

Comments

@Jafet
Copy link

@Jafet Jafet commented Sep 15, 2017

The 0.7.3 CRAN package fails to compile. The error is the same as reported in #3078:

> install.packages("dplyr")
[...]
join_exports.cpp: In function ‘Rcpp::DataFrame semi_join_impl(Rcpp::DataFrame, Rcpp::DataFrame, Rcpp::CharacterVector, Rcpp::CharacterVector, bool)’:
join_exports.cpp:174:21: error: ‘Rcpp::DataFrame {aka class Rcpp::DataFrame_Impl<Rcpp::PreserveStorage>}’ has no member named ‘nrow’; did you mean ‘nrows’?
   indices.reserve(x.nrow());
                     ^~~~
                     nrows
/usr/lib/R/etc/Makeconf:141: recipe for target 'join_exports.o' failed

This is just a typo, isn't it? Changing to x.nrows() makes the install succeed. Line: join_exports.cpp:174

This happened when installing dplyr over Debian's R packages. Environment:

% lsb_release -d
Description:	Debian GNU/Linux testing (buster)

% R --version
R version 3.3.3 (2017-03-06) -- "Another Canoe"
...

% g++ --version
g++ (Debian 7.2.0-4) 7.2.0
...
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Sep 15, 2017

Thanks. What Rcpp version do you have? This should disappear when you upgrade, but we might just change it too.

@Jafet
Copy link
Author

@Jafet Jafet commented Sep 15, 2017

It is Debian's r-cran-rcpp 0.12.9-1. I see that DataFrame::nrow wasn't available until 0.12.10.

@krlmlr krlmlr added the bug label Sep 15, 2017
@krlmlr
Copy link
Member

@krlmlr krlmlr commented Sep 15, 2017

🤦‍♂️ 🤦‍♂️

I'll try to push a fix soon.

krlmlr added a commit to krlmlr/dplyr that referenced this issue Sep 15, 2017
@Jafet
Copy link
Author

@Jafet Jafet commented Sep 16, 2017

@Jafet Jafet closed this Sep 16, 2017
@calvinwy
Copy link

@calvinwy calvinwy commented Sep 25, 2017

I am having the same error, seems like it haven't been fixed yet? At least not from the CRAN repository?

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Sep 25, 2017

The GitHub version should work, the CRAN submission is underway.

@podalv
Copy link

@podalv podalv commented Sep 28, 2017

GitHub version has still the same problem. Does nobody test stuff before release?

@lionel-
Copy link
Member

@lionel- lionel- commented Sep 28, 2017

install_github("tidyverse/dplyr#3098")

@calvinwy
Copy link

@calvinwy calvinwy commented Sep 28, 2017

@podalv , I download it from GitHub and fixed the problem manually, and then install it locally.

@podalv
Copy link

@podalv podalv commented Sep 28, 2017

Yeah, had to do the same, I was just being a bit frustrated that 3 days ago this issue was marked as fixed in GitHub. Oh well :)

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Sep 28, 2017

@lionel-: Thanks, there seems to be a downside to not merging release candidates.

@calvinwy
Copy link

@calvinwy calvinwy commented Sep 28, 2017

@krlmlr : No worries, as long as everything works. Thanks for your effort fixing the problem!

krlmlr added a commit that referenced this issue Sep 29, 2017
* use explicit constructor call instead of assignment

* NEWS

* Fix UB during vector slice construction

* resolve UB by using copy instead of reference

* use pointer in object constructor to avoid passing temporary

* NEWS and bump

* release desc

* avoid using method from Rcpp 0.12.10, closes #3106

* revdep results

* final CRAN comments

* add NEWS

* rebuild with dev tibble

* don't use shields.io

* build on three flavors of R
@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants