Skip to content

workaround mangle collision between decltype(Rf_error) and decltype(Rf_warning) #85

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

Merged
merged 5 commits into from
Aug 17, 2020

Conversation

bkietz
Copy link
Collaborator

@bkietz bkietz commented Aug 7, 2020

I also cleaned up the signatures of detail:: functions, hopefully it's more readable

@bkietz bkietz requested a review from romainfrancois August 7, 2020 14:20
@romainfrancois
Copy link
Collaborator

I'm still getting the same problem on my current branch of arrow:

==> R CMD INSTALL --preclean --no-multiarch --with-keep.source r

* installing to library ‘/Users/romainfrancois/.R/library/4.0’
*** Generating code with data-raw/codegen.R
* installing *source* package ‘arrow’ ...
** using staged installation
Warning messages:
1: package ‘decor’ was built under R version 4.0.2 
2: package ‘vctrs’ was built under R version 4.0.2 
*** > 378 functions decorated with [[arrow|s3::export]]
*** > generated file `src/arrowExports.cpp`
*** > generated file `R/arrowExports.R`
*** Arrow C++ libraries found via pkg-config
PKG_CFLAGS=-DNDEBUG -I/usr/local/Cellar/apache-arrow/HEAD-81d3f26/include -DARROW_R_WITH_ARROW
PKG_LIBS=-L/usr/local/Cellar/apache-arrow/HEAD-81d3f26/lib -larrow_dataset -lparquet -larrow
clang++ -mmacosx-version-min=10.13 -std=gnu++11 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -DNDEBUG -I/usr/local/Cellar/apache-arrow/HEAD-81d3f26/include -DARROW_R_WITH_ARROW -I'/Users/romainfrancois/.R/library/4.0/cpp11/include' -I/usr/local/include   -fPIC  -Wall -O3 -Wall -Wimplicit-int-float-conversion -c array.cpp -o array.o
** libs
In file included from array.cpp:18:
In file included from ././arrow_types.h:22:
In file included from ././arrow_cpp11.h:37:
In file included from /Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11.hpp:5:
In file included from /Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11/as.hpp:9:
/Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11/protect.hpp:240:58: error: definition with same mangled name '_ZNK5cpp117protect8functionIFvPKczEEclIJRS3_EEEDTclclL_ZNSt3__17declvalIPS4_EEDTclsr3std3__1E9__declvalIT_ELi0EEEvEEspclsr3stdE7declvalIOT_EEEEDpSE_' as another definition
    decltype(std::declval<F*>()(std::declval<A&&>()...)) operator()(A&&... a) const {
                                                         ^
/Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11/protect.hpp:240:58: note: previous definition is here
1 error generated.
make: *** [array.o] Error 1
ERROR: compilation failed for package ‘arrow’
* removing ‘/Users/romainfrancois/.R/library/4.0/arrow’
* restoring previous ‘/Users/romainfrancois/.R/library/4.0/arrow’

Exited with status 1.

Copy link
Collaborator

@romainfrancois romainfrancois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor comments, but not really fixing the issue

@jimhester
Copy link
Member

If the code prior to #71 works on clang and the current code works on gcc one approach would be to ifdef them?

don't really enjoy playing compiler whack-a-mole though :(

@bkietz
Copy link
Collaborator Author

bkietz commented Aug 7, 2020

@jimhester if we can add a test with a minimal reproducer for the compilation failure I'd personally be comfortable adding the #ifdef and trying to golf it out in a follow up. @romainfrancois is working on one

@romainfrancois
Copy link
Collaborator

cpp11::cpp_source(code = '
#include <cpp11.hpp>

[[cpp11::register]]
std::string fun() {
  cpp11::stop("b");
  cpp11::warning("c");
}
', quiet = FALSE)
#> clang++ -mmacosx-version-min=10.13 -std=gnu++11 -I"/Library/Frameworks/R.framework/Resources/include" -DNDEBUG -I/Users/romainfrancois/.R/library/4.0/cpp11/include  -I/usr/local/include   -fPIC  -Wall -O3 -Wall -Wimplicit-int-float-conversion -c /private/var/folders/4b/hn4fq98s6810s4ccv2f9hm2h0000gn/T/RtmpnWGTon/file16a78559d98cd/src/code_0.cpp -o /private/var/folders/4b/hn4fq98s6810s4ccv2f9hm2h0000gn/T/RtmpnWGTon/file16a78559d98cd/src/code_0.o
#> In file included from /private/var/folders/4b/hn4fq98s6810s4ccv2f9hm2h0000gn/T/RtmpnWGTon/file16a78559d98cd/src/code_0.cpp:2:
#> In file included from /Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11.hpp:5:
#> In file included from /Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11/as.hpp:9:
#> /Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11/protect.hpp:240:58: error: definition with same mangled name '_ZNK5cpp117protect8functionIFvPKczEEclIJRS3_EEEDTclclL_ZNSt3__17declvalIPS4_EEDTclsr3std3__1E9__declvalIT_ELi0EEEvEEspclsr3stdE7declvalIOT_EEEEDpSE_' as another definition
#>     decltype(std::declval<F*>()(std::declval<A&&>()...)) operator()(A&&... a) const {
#>                                                          ^
#> /Users/romainfrancois/.R/library/4.0/cpp11/include/cpp11/protect.hpp:240:58: note: previous definition is here
#> 1 error generated.
#> make: *** [/private/var/folders/4b/hn4fq98s6810s4ccv2f9hm2h0000gn/T/RtmpnWGTon/file16a78559d98cd/src/code_0.o] Error 1
#> Error in dyn.load(shared_lib, local = TRUE, now = TRUE): unable to load shared object '/var/folders/4b/hn4fq98s6810s4ccv2f9hm2h0000gn/T//RtmpnWGTon/file16a78559d98cd/src/code_0.so':
#>   dlopen(/var/folders/4b/hn4fq98s6810s4ccv2f9hm2h0000gn/T//RtmpnWGTon/file16a78559d98cd/src/code_0.so, 6): image not found

Created on 2020-08-10 by the reprex package (v0.3.0.9001)

@jimhester
Copy link
Member

So is the ifdef solution the best option to go forward with this?

@bkietz
Copy link
Collaborator Author

bkietz commented Aug 13, 2020

I'm looking at Romain's reproducer. I'll either find a fix today or revert to #ifdefs

@bkietz bkietz force-pushed the linker-error-workaround branch from d6e5528 to 25014d8 Compare August 13, 2020 18:03
@bkietz
Copy link
Collaborator Author

bkietz commented Aug 13, 2020

@romainfrancois PTAL

@bkietz bkietz changed the title avoid anonymous lambdas in protect::function::operator() workaround mangle collision between decltype(Rf_error) and decltype(Rf_warning) Aug 13, 2020
Copy link
Collaborator

@romainfrancois romainfrancois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does fix the issue with the reprex and I can compile arrow#cpp11.

Thanks @bkietz

In terms of style, I'm just wondering if this should rather be safe_noreturn[Rf_error] but I guess this is ok as not supposed to be used by code outside of cpp11.

romainfrancois added a commit to romainfrancois/arrow that referenced this pull request Aug 17, 2020
romainfrancois added a commit to romainfrancois/arrow that referenced this pull request Aug 17, 2020
@bkietz bkietz merged commit fdc21c8 into r-lib:master Aug 17, 2020
@bkietz
Copy link
Collaborator Author

bkietz commented Aug 17, 2020

@romainfrancois it is internal, so we can rewrite as safe_noreturn anytime

romainfrancois added a commit to romainfrancois/arrow that referenced this pull request Aug 18, 2020
romainfrancois added a commit to romainfrancois/arrow that referenced this pull request Aug 20, 2020
nealrichardson pushed a commit to romainfrancois/arrow that referenced this pull request Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants