Skip to content

Commit

Permalink
Add a linter for calls to omp_set_num_threads
Browse files Browse the repository at this point in the history
  • Loading branch information
MichaelChirico committed Aug 3, 2024
1 parent 2dac309 commit 40d4fc5
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 6 deletions.
11 changes: 11 additions & 0 deletions .ci/linters/c/omp_set_num_threads_linter.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Ensure no calls to omp_set_num_threads() [to avoid affecting other packages and base R]
# Only comments referring to it should be in openmp-utils.c
omp_set_num_threads_linter = function(c_file) {
processed_lines = system2("gcc", c("-fpreprocessed", "-E", "-xc", "-"), stdin=c_file, stdout=TRUE, stderr=FALSE)
idx = grep("omp_set_num_threads", processed_lines, fixed = TRUE)
if (!length(idx)) return()
stop(sprintf(
"In %s, found 'omp_set_num_threads() usage, which could affect other packages and base R:\n%s",
c_file, paste0(" ", format(idx), ":", processed_lines[idx], collapse = "\n")
))
}
3 changes: 0 additions & 3 deletions .dev/CRAN_Release.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ grep -RI --exclude-dir=".git" --exclude="*.md" --exclude="*~" --color='auto' -P
# Unicode is now ok. This unicode in tests.Rraw is passing on CRAN.
grep -RI --exclude-dir=".git" --exclude="*.md" --exclude="*~" --color='auto' -n "[\]u[0-9]" ./

# Ensure no calls to omp_set_num_threads() [to avoid affecting other packages and base R]
# Only comments referring to it should be in openmp-utils.c
grep omp_set_num_threads ./src/*

# Ensure no calls to omp_set_nested() as i) it's hard to fully honor OMP_THREAD_LIMIT as required by CRAN, and
# ii) a simpler non-nested approach is always preferable if possible, as has been the case so far
Expand Down
7 changes: 4 additions & 3 deletions .github/workflows/code-quality.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ jobs:
- uses: r-lib/actions/setup-r@v2
- name: Lint
run: |
for (f in list.files('.ci/linters/c', full.names=TRUE)) source(f)
for (f in list.files('src', pattern='[.]c$', full.names=TRUE)) {
alloc_linter(f)
linter_env = new.env()
for (f in list.files('.ci/linters/c', full.names=TRUE)) sys.source(f, linter_env)
for (f in list.files('src', pattern='[.][ch]$', full.names=TRUE)) {
for (linter in ls(linter_env)) linter_env[[linter]](f)
# TODO(#6272): Incorporate more checks from CRAN_Release
}
shell: Rscript {0}

0 comments on commit 40d4fc5

Please sign in to comment.