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

cppFuncton() emits .Primitive(".Call") instead of just .Call #795

Closed
krlmlr opened this Issue Jan 3, 2018 · 13 comments

Comments

Projects
None yet
4 participants
@krlmlr
Contributor

krlmlr commented Jan 3, 2018

See example below. This is a problem for joint profiling of R and native code, because I'm looking for .Call entries on the call stack, which are absent if the primitive is invoked directly (r-prof/jointprof#8). Would you accept a PR that swtiches to .Call()?

print(Rcpp::cppFunction("int x() { return 1; }"))
#> function () 
#> .Primitive(".Call")(<pointer: 0x7fd9fb532060>)

Created on 2018-01-03 by the reprex package (v0.1.1.9000).

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Jan 3, 2018

Paging @jjallaire -- there was a reason for this but it escapes me now what that was.

@jjallaire

This comment has been minimized.

Member

jjallaire commented Jan 3, 2018

I believe that I borrowed the implementation approach for native routine dispatching from the inline package (which presumably also does this although I haven't checked). I honestly don't understand the difference (if any) so would yield to any more knowledgeable opinion.

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Jan 3, 2018

Well we can try. Seems like a larger change, but hey I have a new package for parallel reverse depends checks so we can try a PR.

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Jan 10, 2018

Is this a 'hot' or needed item for the profiler you are working on?

@krlmlr

This comment has been minimized.

Contributor

krlmlr commented Jan 10, 2018

Yes. It would be great to have it in the next Rcpp release, by when would you need it so that you can include it?

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Jan 10, 2018

Gaa. Replied in the wrong place. Release is pretty much ready and I could ship nowishly.

@krlmlr

This comment has been minimized.

Contributor

krlmlr commented Jan 10, 2018

I have a workaround for this problem in my code, so I don't mind if this has to wait a bit. Feel free to release (but thanks for the reminder).

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Jan 10, 2018

Yeah, next release is better even though I have a feeling that this may end up being totally iinocuous.

@mattfidler

This comment has been minimized.

mattfidler commented Jan 30, 2018

For me, using .Primitive(".Call") was needed to avoid some R CMD Check issues; I just saw this and wanted to point out this issue. I think it is a bit silly and a false positive in this case, but I hade similar problems with dparser

@krlmlr

This comment has been minimized.

Contributor

krlmlr commented Jan 30, 2018

Do you remember which particular issues are worked around this way?

@mattfidler

This comment has been minimized.

mattfidler commented Jan 30, 2018

Sure

Registration problem:
  symbol 'sym' in the local frame:
   .Call(sym, PACKAGE = pkg)
See chapter 'System and foreign language interfaces' in the 'Writing R
Extensions' manual.

If you put Primitive(".Call") this goes away. Eventually I settled on

eval(bquote(function()(.(quote(.Call))(.(sym), PACKAGE=.(pkg)))));

Which directly uses .Call and gets around this check issue.

@eddelbuettel

This comment has been minimized.

Member

eddelbuettel commented Jan 30, 2018

Hm are you talking by chance about R CMD check output? You should never submit cppFunction() generated code (which is what this ticket is about) to R CMD check.

cppFunction() is for creating ad-hoc code at the prompt; such code usually does not go into a package where we use compileAttributes() instead.

There are helpers for the registration issue; compileAttributes() now does this for you.

@mattfidler

This comment has been minimized.

mattfidler commented Jan 30, 2018

Ah. I was confused. Thanks for the clarification. I only thought that was the reason.

eddelbuettel added a commit that referenced this issue Feb 14, 2018

Merge pull request #813 from krlmlr/f-#795-no-primitive-call
Generate .Call(...) instead of .Primitive(.Call)(...) for cppFunction
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment