Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Added explicit (const_)string_proxy/SEXP comparisons to resolve ambiguity#372

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
eddelbuettel merged 1 commit intoRcppCore:masterfromdcdillon:master
Sep 9, 2015

Conversation

@dcdillon
Copy link
Contributor

This fixes compile failures in readr, readxl, and rpg

@eddelbuettel
Copy link
Member

... and tidyr. Thanks for fixing it and submitting a PR while my rev.dep. check is still running!

@eddelbuettel
Copy link
Member

Ok, I checked and this fixed readr, readxl, rpg and tidyr. We still have package icd9 failing with:

** libsccache gcc -I/usr/share/R/include -DNDEBUG   -I"/usr/local/lib/R/site-library/Rcpp/include"  -fopenmp -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -O3 -Wall -pipe -pedantic -std=gnu99 -c cutil.c -o cutil.occache g++ -I/usr/share/R/include -DNDEBUG   -I"/usr/local/lib/R/site-library/Rcpp/include"  -I. -fopenmp -std=c++11 -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -O3 -Wall -pipe -Wno-unused -pedantic -c is.cpp -o is.occache g++ -I/usr/share/R/include -DNDEBUG   -I"/usr/local/lib/R/site-library/Rcpp/include"  -I. -fopenmp -std=c++11 -fpic  -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -D_FORTIFY_SOURCE=2 -g  -O3 -Wall -pipe -Wno-unused -pedantic -c manip.cpp -o manip.omanip.cpp: In function ‘Rcpp::String icd9AddLeadingZeroesMajorSingle(Rcpp::String)’:manip.cpp:25:12: error: ambiguous overload for ‘operator==’ (operand types are ‘Rcpp::String’ and ‘SEXP’)  if (major == NA_STRING) {            ^manip.cpp:25:12: note: candidates are:In file included from /usr/local/lib/R/site-library/Rcpp/include/Rcpp/Vector.h:67:0,                 from /usr/local/lib/R/site-library/Rcpp/include/Rcpp.h:38,                 from ./convert.h:22,                 from manip.cpp:20:/usr/local/lib/R/site-library/Rcpp/include/Rcpp/String.h:414:14: note: bool Rcpp::String::operator==(const Rcpp::String&) const         bool operator==( const Rcpp::String& other) const {              ^/usr/local/lib/R/site-library/Rcpp/include/Rcpp/String.h:429:14: note: bool Rcpp::String::operator==(const const_StringProxy&) const         bool operator==( const const_StringProxy& other) const {              ^/usr/lib/R/etc/Makeconf:143: recipe for target 'manip.o' failedmake: *** [manip.o] Error 1make: *** Waiting for unfinished jobs....

@thirdwing
Copy link
Member

I think add those lines below intoString.h should fixicd9.

bool operator==( SEXP other) const {                                    return get_sexp() == other;                                     }                                                                   bool operator!=( SEXP other) const {                                    return get_sexp() != other;                                      }

@eddelbuettel
Copy link
Member

Nice work,@thirdwing !

When I looked asvector/string_proxy.h I got into trouble with the forward reference. Going toString.h works as you say (and we do indeed needget_sexp() instead ofget().

@dcdillon Do you want to add this to the PR or shall I just add it after merging?

edd@max:/tmp/rcpp-tmp/Rcpp$ diff -u ~/git/rcpp/inst/include/Rcpp/String.h inst/include/Rcpp/String.h                                                                                                                 --- /home/edd/git/rcpp/inst/include/Rcpp/String.h2015-09-0809:59:23.570918337 -0500+++ inst/include/Rcpp/String.h2015-09-0905:54:04.478869310 -0500@@ -438,6 +438,14 @@return strcmp( get_cstring(), other.get_cstring() ) >0;         }+booloperator==( SEXP other )const {+returnget_sexp() == other;+        }++booloperator!=( SEXP other )const {+returnget_sexp() != other;+        }+private:/** the CHARSXP this String encapsulates*/edd@max:/tmp/rcpp-tmp/Rcpp$

Ah shucks, I'll just do it and then start a new test run ...

eddelbuettel added a commit that referenced this pull requestSep 9, 2015
Added explicit (const_)string_proxy/SEXP comparisons to resolve ambiguity
@eddelbuetteleddelbuettel merged commit0d483c7 intoRcppCore:masterSep 9, 2015
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dcdillon@eddelbuettel@thirdwing

[8]ページ先頭

©2009-2025 Movatter.jp