tidy/
pal.rs

1//! Tidy check to enforce rules about platform-specific code in std.
2//!
3//! This is intended to maintain existing standards of code
4//! organization in hopes that the standard library will continue to
5//! be refactored to isolate platform-specific bits, making porting
6//! easier; where "standard library" roughly means "all the
7//! dependencies of the std and test crates".
8//!
9//! This generally means placing restrictions on where `cfg(unix)`,
10//! `cfg(windows)`, `cfg(target_os)` and `cfg(target_env)` may appear,
11//! the basic objective being to isolate platform-specific code to the
12//! platform-specific `std::sys` modules, and to the allocation,
13//! unwinding, and libc crates.
14//!
15//! Following are the basic rules, though there are currently
16//! exceptions:
17//!
18//! - core may not have platform-specific code.
19//! - libpanic_abort may have platform-specific code.
20//! - libpanic_unwind may have platform-specific code.
21//! - libunwind may have platform-specific code.
22//! - other crates in the std facade may not.
23//! - std may have platform-specific code in the following places:
24//!   - `sys/`
25//!   - `os/`
26//!
27//! `std/sys_common` should _not_ contain platform-specific code.
28//! Finally, because std contains tests with platform-specific
29//! `ignore` attributes, once the parser encounters `mod tests`,
30//! platform-specific cfgs are allowed. Not sure yet how to deal with
31//! this in the long term.
32
33use std::path::Path;
34
35use crate::walk::{filter_dirs, walk};
36
37// Paths that may contain platform-specific code.
38const EXCEPTION_PATHS: &[&str] = &[
39    "library/compiler-builtins",
40    "library/std_detect",
41    "library/windows_targets",
42    "library/panic_abort",
43    "library/panic_unwind",
44    "library/unwind",
45    "library/rtstartup", // Not sure what to do about this. magic stuff for mingw
46    "library/test",      // Probably should defer to unstable `std::sys` APIs.
47    // The `VaList` implementation must have platform specific code.
48    // The Windows implementation of a `va_list` is always a character
49    // pointer regardless of the target architecture. As a result,
50    // we must use `#[cfg(windows)]` to conditionally compile the
51    // correct `VaList` structure for windows.
52    "library/core/src/ffi/va_list.rs",
53    // core::ffi contains platform-specific type and linkage configuration
54    "library/core/src/ffi/mod.rs",
55    "library/core/src/ffi/primitives.rs",
56    "library/std/src/sys", // Platform-specific code for std lives here.
57    "library/std/src/os",  // Platform-specific public interfaces
58    // Temporary `std` exceptions
59    // FIXME: platform-specific code should be moved to `sys`
60    "library/std/src/io/copy.rs",
61    "library/std/src/io/stdio.rs",
62    "library/std/src/lib.rs", // for miniz_oxide leaking docs, which itself workaround
63    "library/std/src/path.rs",
64    "library/std/src/sys_common", // Should only contain abstractions over platforms
65    "library/std/src/net/test.rs", // Utility helpers for tests
66    "library/std/src/io/error.rs", // Repr unpacked needed for UEFI
67];
68
69pub fn check(path: &Path, bad: &mut bool) {
70    // Sanity check that the complex parsing here works.
71    let mut saw_target_arch = false;
72    let mut saw_cfg_bang = false;
73    walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
74        let file = entry.path();
75        let filestr = file.to_string_lossy().replace("\\", "/");
76        if !filestr.ends_with(".rs") {
77            return;
78        }
79
80        let is_exception_path = EXCEPTION_PATHS.iter().any(|s| filestr.contains(&**s));
81        if is_exception_path {
82            return;
83        }
84
85        // exclude tests and benchmarks as some platforms do not support all tests
86        if filestr.contains("tests") || filestr.contains("benches") {
87            return;
88        }
89
90        check_cfgs(contents, file, bad, &mut saw_target_arch, &mut saw_cfg_bang);
91    });
92
93    assert!(saw_target_arch);
94    assert!(saw_cfg_bang);
95}
96
97fn check_cfgs(
98    contents: &str,
99    file: &Path,
100    bad: &mut bool,
101    saw_target_arch: &mut bool,
102    saw_cfg_bang: &mut bool,
103) {
104    // Pull out all `cfg(...)` and `cfg!(...)` strings.
105    let cfgs = parse_cfgs(contents);
106
107    let mut line_numbers: Option<Vec<usize>> = None;
108    let mut err = |idx: usize, cfg: &str| {
109        if line_numbers.is_none() {
110            line_numbers = Some(contents.match_indices('\n').map(|(i, _)| i).collect());
111        }
112        let line_numbers = line_numbers.as_ref().expect("");
113        let line = match line_numbers.binary_search(&idx) {
114            Ok(_) => unreachable!(),
115            Err(i) => i + 1,
116        };
117        tidy_error!(bad, "{}:{}: platform-specific cfg: {}", file.display(), line, cfg);
118    };
119
120    for (idx, cfg) in cfgs {
121        // Sanity check that the parsing here works.
122        if !*saw_target_arch && cfg.contains("target_arch") {
123            *saw_target_arch = true
124        }
125        if !*saw_cfg_bang && cfg.contains("cfg!") {
126            *saw_cfg_bang = true
127        }
128
129        let contains_platform_specific_cfg = cfg.contains("target_os")
130            || cfg.contains("target_env")
131            || cfg.contains("target_abi")
132            || cfg.contains("target_vendor")
133            || cfg.contains("target_family")
134            || cfg.contains("unix")
135            || cfg.contains("windows");
136
137        if !contains_platform_specific_cfg {
138            continue;
139        }
140
141        let preceded_by_doc_comment = {
142            let pre_contents = &contents[..idx];
143            let pre_newline = pre_contents.rfind('\n');
144            let pre_doc_comment = pre_contents.rfind("///");
145            match (pre_newline, pre_doc_comment) {
146                (Some(n), Some(c)) => n < c,
147                (None, Some(_)) => true,
148                (_, None) => false,
149            }
150        };
151
152        if preceded_by_doc_comment {
153            continue;
154        }
155
156        // exclude tests as some platforms do not support all tests
157        if cfg.contains("test") {
158            continue;
159        }
160
161        err(idx, cfg);
162    }
163}
164
165fn parse_cfgs(contents: &str) -> Vec<(usize, &str)> {
166    let candidate_cfgs = contents.match_indices("cfg");
167    let candidate_cfg_idxs = candidate_cfgs.map(|(i, _)| i);
168    // This is puling out the indexes of all "cfg" strings
169    // that appear to be tokens followed by a parenthesis.
170    let cfgs = candidate_cfg_idxs.filter(|i| {
171        let pre_idx = i.saturating_sub(1);
172        let succeeds_non_ident = !contents
173            .as_bytes()
174            .get(pre_idx)
175            .cloned()
176            .map(char::from)
177            .map(char::is_alphanumeric)
178            .unwrap_or(false);
179        let contents_after = &contents[*i..];
180        let first_paren = contents_after.find('(');
181        let paren_idx = first_paren.map(|ip| i + ip);
182        let preceeds_whitespace_and_paren = paren_idx
183            .map(|ip| {
184                let maybe_space = &contents[*i + "cfg".len()..ip];
185                maybe_space.chars().all(|c| char::is_whitespace(c) || c == '!')
186            })
187            .unwrap_or(false);
188
189        succeeds_non_ident && preceeds_whitespace_and_paren
190    });
191
192    cfgs.flat_map(|i| {
193        let mut depth = 0;
194        let contents_from = &contents[i..];
195        for (j, byte) in contents_from.bytes().enumerate() {
196            match byte {
197                b'(' => {
198                    depth += 1;
199                }
200                b')' => {
201                    depth -= 1;
202                    if depth == 0 {
203                        return Some((i, &contents_from[..=j]));
204                    }
205                }
206                _ => {}
207            }
208        }
209
210        // if the parentheses are unbalanced just ignore this cfg -- it'll be caught when attempting
211        // to run the compiler, and there's no real reason to lint it separately here
212        None
213    })
214    .collect()
215}