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

Use tinycthread on all platforms #79

Merged
merged 2 commits into from
Jan 4, 2019
Merged

Use tinycthread on all platforms #79

merged 2 commits into from
Jan 4, 2019

Conversation

wch
Copy link
Member

@wch wch commented Jan 3, 2019

Fixes #77. This PR makes it so later stops trying to use C11-style threads.

It changes the function names in tinycthread so that they don't conflict with (and inadverantly link to) functions with the same name from C11-style threads.

Installation can be tested by building the Fedora 29-Anaconda Docker image as described in #77, then running the following:

docker run --rm -ti fr /bin/bash
yum install -y unzip
wget https://github.com/r-lib/later/archive/remove-tinycthread.zip
unzip remove-tinycthread.zip
R CMD build --no-build-vignettes later-remove-tinycthread
R CMD INSTALL later_0.7.5.9001.tar.gz

For some reason, R CMD check doesn't work, but I think it's because there's some missing system dependencies.

[root@f07d3a24fdad /]# R CMD check later_0.7.5.9001.tar.gz 
Error in system(paste(which, shQuote(names[i])), intern = TRUE, ignore.stderr = TRUE) : 
  error in running command
Execution halted

wch added 2 commits January 4, 2019 13:59
This also changes the function names in tinycthread so that they don't
conflict with (and inadverantly link to) functions with the same name
from C11-style threads.h.
@wch wch force-pushed the remove-tinycthread branch from 48cc5fb to 612536f Compare January 4, 2019 19:59
@wch wch merged commit a3b0137 into master Jan 4, 2019
@wch wch deleted the remove-tinycthread branch January 4, 2019 20:21
* system's functions that have the same name, instead of the local functions.
*/

#define thrd_t THREADS_H_ERROR
Copy link

Choose a reason for hiding this comment

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

This name (thrd_t) still exists even though it's listed here as bad. Pretty sure that means that THREADS_H_ERROR becomes the name of the type, which does mean the wrong type name is avoided, but I'm not sure that's what was intended.

@QuLogic
Copy link

QuLogic commented Feb 12, 2019

On a related note, names ending in _t are reserved for future type names and should not be used. Technically, tinycthread was in violation of this already...

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