Skip to content

Commit 8e03ca8

Browse files
Add a linter for calls to omp_set_num_threads (#6345)
Part of #6272 Probably, the step to pre-process the file to strip comments will be repeated by other linters, perhaps we should _just_ work from the preprocessed file for the C linters? Leaving the simple version for now that's more parallel to the existing code.
1 parent 3b9dcd2 commit 8e03ca8

File tree

3 files changed

+17
-6
lines changed

3 files changed

+17
-6
lines changed
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
2+
# Ensure no calls to omp_set_num_threads() [to avoid affecting other packages and base R]
3+
# Only comments referring to it should be in openmp-utils.c
4+
omp_set_num_threads_linter = function(c_file) {
5+
# strip comments, we only care if the function appears in actual code.
6+
processed_lines = system2("gcc", c("-fpreprocessed", "-E", c_file), stdout=TRUE, stderr=FALSE)
7+
idx = grep("omp_set_num_threads", processed_lines, fixed = TRUE)
8+
if (!length(idx)) return()
9+
stop(sprintf(
10+
"In %s, found omp_set_num_threads() usage, which could affect other packages and base R:\n%s",
11+
c_file, paste0(" ", format(idx), ":", processed_lines[idx], collapse = "\n")
12+
))
13+
}

.dev/CRAN_Release.cmd

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,6 @@ grep -RI --exclude-dir=".git" --exclude="*.md" --exclude="*~" --color='auto' -P
5353
# Unicode is now ok. This unicode in tests.Rraw is passing on CRAN.
5454
grep -RI --exclude-dir=".git" --exclude="*.md" --exclude="*~" --color='auto' -n "[\]u[0-9]" ./
5555

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

6057
# Ensure no calls to omp_set_nested() as i) it's hard to fully honor OMP_THREAD_LIMIT as required by CRAN, and
6158
# ii) a simpler non-nested approach is always preferable if possible, as has been the case so far

.github/workflows/code-quality.yaml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ jobs:
4040
- uses: r-lib/actions/setup-r@v2
4141
- name: Lint
4242
run: |
43-
for (f in list.files('.ci/linters/c', full.names=TRUE)) source(f)
44-
for (f in list.files('src', pattern='[.]c$', full.names=TRUE)) {
45-
alloc_linter(f)
43+
linter_env = new.env()
44+
for (f in list.files('.ci/linters/c', full.names=TRUE)) sys.source(f, linter_env)
45+
for (f in list.files('src', pattern='[.][ch]$', full.names=TRUE)) {
46+
for (linter in ls(linter_env)) linter_env[[linter]](f)
4647
# TODO(#6272): Incorporate more checks from CRAN_Release
4748
}
4849
shell: Rscript {0}

0 commit comments

Comments
 (0)