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

Slightly more robust pull of .Machine entries #6154

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MichaelChirico
Copy link
Member

Related to #6153, getting longdouble.digits== is not the most helpful, I think there's a couple ways we could get that. I assume based on the base R code that we're seeing NULL there, but this will help clarify the actual value / rely less on inference.

@MichaelChirico
Copy link
Member Author

format() will at least let us distinguish integer() from NULL.

Copy link

github-actions bot commented May 28, 2024

Comparison Plot

Generated via commit 2eeb415

Download link for the artifact containing the test results: ↓ atime-results.zip

Time taken to finish the standard R installation steps: 11 minutes and 44 seconds

Time taken to run atime::atime_pkg on the tests: 3 minutes and 18 seconds

@ben-schwen
Copy link
Member

Maybe also add capabilities()["long.double"]?

@MichaelChirico
Copy link
Member Author

I guess that's overkill, here's how that element of capabilities() is defined:

https://github.com/r-devel/r-svn/blob/e0f5c5e1c60bca2b7b2044eb0b4770e287cd770e/src/main/platform.c#L2627-L2628

Here's the LDOUBLE macro:

https://github.com/r-devel/r-svn/blob/1f537a2469956b505ad10135eeee282fe33a8475/src/include/Defn.h#L2407-L2411

In turn I think HAVE_LONG_DOUBLE gets set by ./configure.

Here's how .Machine$sizeof.longdouble is defined:

https://github.com/r-devel/r-svn/blob/1f537a2469956b505ad10135eeee282fe33a8475/src/main/machine.c#L139-L144

Here's the SIZEOF_LONG_DOUBLE macro:

https://github.com/r-devel/r-svn/blob/1f537a2469956b505ad10135eeee282fe33a8475/src/gnuwin32/fixed/h/config.h#L1182-L1191

So there might be some daylight between the two, WDYT?

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.

None yet

2 participants