Skip to content

Commit cdcc375

Browse files
Update design for PR rust-lang#16108: multiple_unsafe_ops_per_block: still lint raw pointer to union field if MSRV < 1.92
1 parent 4d6fa3c commit cdcc375

File tree

4 files changed

+60
-5
lines changed

4 files changed

+60
-5
lines changed

.exp/design-workflow-1-cargo-clippy.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ sequenceDiagram
7979
- Ancestor directories.
8080
- Code attributes.
8181
- CLI flags passed via `CLIPPY_ARGS` (e.g., `-W clippy::perf`).
82-
- **Application**: Used to filter and level lints during registration. Supports groups (correctness, style, perf, etc.) and MSRV specifications.
82+
- **Application**: Used to filter and level lints during registration and to enable lint-specific behavior via access to `Conf` fields like `conf.msrv` for version-dependent logic (e.g., adjusting unsafe operation counts in `multiple_unsafe_ops_per_block` based on MSRV). Supports groups (correctness, style, perf, etc.) and MSRV specifications in `clippy.toml`.
8383
- **Tracking**: Config files and env vars are added to `ParseSess`'s depinfo for incremental builds.
8484

8585
## Lint Execution Details

.exp/design-workflow-2-clippy-driver.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ This workflow ensures deep integration with Rust's compiler pipeline, allowing l
2424
- Early passes (AST/HIR-based, e.g., formatting, doc checks).
2525
- Late passes (typed analysis, e.g., type checks, method calls).
2626
- Supports conditional enabling via \`Conf\`.
27-
- **Configuration Loader (clippy_config)**: \`Conf::read\` parses \`clippy.toml\`, inline attributes (\`#[allow(clippy::lint)]\`), CLI overrides; influences lint levels and params.
27+
- **Configuration Loader (clippy_config)**: \`Conf::read\` parses \`clippy.toml\`, inline attributes (\`#[allow(clippy::lint)]\`), CLI overrides; provides fields like \`msrv\` for lints to access via constructors, influencing levels, params, and version-aware behavior.
2828
- **Lint Utilities (clippy_utils)**: Reusable functions for lints (e.g., type queries, def path matching, expression analysis).
2929
- **Rustc Infrastructure**: \`rustc_driver\`, \`rustc_interface\`, \`rustc_session\` for session management, diagnostics; \`rustc_lint\` for pass registration.
3030
- **Development Helpers**: Tracks files/envs for incremental builds; ICE hook for bug reporting to GitHub.
@@ -102,7 +102,7 @@ sequenceDiagram
102102

103103
- **Performance Considerations**: MIR opt level set to 0 avoids optimizations that could obscure bugs (e.g., constant folding); disables specific passes like \`CheckNull\`. Trade-off: Slower compilation for accuracy.
104104
- **Incremental Compilation**: Tracks runtime-accessed files (Cargo.toml, clippy.toml, driver exe) in \`file_depinfo\` and envs (CLIPPY_CONF_DIR, CLIPPY_ARGS) for Cargo-triggered rebuilds.
105-
- **Configuration Flexibility**: Hierarchical (CLI > file > attributes); supports lint groups, custom params (e.g., max nesting depth).
105+
- **Configuration Flexibility**: Hierarchical (CLI > file > attributes); supports lint groups, custom params (e.g., max nesting depth), and MSRV settings in `clippy.toml` that lints can use via `conf.msrv` for version-aware behavior (e.g., `multiple_unsafe_ops_per_block` adjusts logic based on MSRV).
106106
- **Fallback and Compatibility**: Supports plain rustc delegation for queries or suppressed lints; handles arg files (\`@path\`), sysroot env.
107107
- **Extensibility**: Lints implement traits like \`EarlyLintPass\` or \`LateLintPass\`; new ones added via dev tools, auto-registered in \`lib.rs\`.
108108
- **Diagnostics**: Leverages rustc's system for structured output (spans, suggestions via rustfix); colorized via anstream.

.exp/design-workflow-5-lint-development.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,13 @@ From analysis of source code (e.g., \`clippy_dev/src/new_lint.rs\`, \`clippy_dev
1111
- **clippy_dev**: CLI tools (\`cargo dev\`) including \`new_lint\` for scaffolding and \`update_lints\` for generating registration code from parsed lint declarations.
1212
- **clippy_lints**: Core library housing all lint implementations as structs implementing \`rustc_lint::EarlyLintPass\` or \`LateLintPass\`. Each lint file (\`src/<lint>.rs\` or in submodules like \`methods/\`) contains a \`declare_clippy_lint!\` invocation defining metadata (name, level, category, description, version, location).
1313
- **declare_clippy_lint**: Crate providing the macro for lint declaration (generates \`Lint\` static and \`LintInfo\`) and \`LintListBuilder\` for bulk registration of individual lints and groups (e.g., \`clippy::all\`, \`clippy::correctness\`).
14-
- **clippy_utils**: Shared utilities for common lint tasks (e.g., type queries, def path matching, expression analysis).
14+
- **clippy_utils**: Shared utilities for common lint tasks (e.g., type queries, def path matching, expression analysis, MSRV checking via the `msrvs` module to enable version-specific lint behavior).
1515
- **tests/ui/**: UI test infrastructure where each lint has a subdirectory with input \`.rs\` files and expected \`.stderr\` outputs (and \`.fixed\` for fixes) from Clippy runs to verify diagnostics.
1616
- **clippy-driver** (in root \`src/driver.rs\`): Custom compiler driver that loads \`clippy_lints\`, uses generated \`declared_lints::LINTS\` to register lints via \`LintListBuilder\`, calls \`register_lint_passes\` to add passes, and hooks into rustc's pipeline.
1717
- **lintcheck**: Separate tool (\`lintcheck/src/main.rs\`) for running lints on external crates listed in \`.toml\` files to detect regressions or false positives/negatives.
1818
- **clippy_config**: Handles lint configuration from \`clippy.toml\` and attributes, used in passes.
1919

20-
Other aspects: Lints support configuration options, MSRV restrictions via attributes, categories for grouping, and integration with rustfix for auto-fixes.
20+
Other aspects: Lints support configuration options (via `new(conf)` and lint-specific fields in `Conf`), MSRV-aware behavior via `conf.msrv` and attributes for restrictions, categories for grouping, and integration with rustfix for auto-fixes.
2121

2222
## Scaffolding Sequence Diagram
2323

@@ -40,6 +40,9 @@ sequenceDiagram
4040
UpdateTool->>LintImpl: Parse all declare clippy lint invocations
4141
UpdateTool->>DeclaredLints: Update LINTS array including new lint
4242
UpdateTool->>LibRs: Update mod declarations between comments
43+
alt lint requires Conf access (e.g., for MSRV checks)
44+
Developer->>LibRs: Manually update register_lint_passes instantiation to pass conf to new(conf)
45+
end
4346
UpdateTool->>README: Update lint counts and links
4447
Note over UpdateTool: Handles deprecated and renamed lints too
4548
\`\`\`
@@ -81,6 +84,8 @@ The \`update_lints\` tool parses all \`declare_clippy_lint!\` macros across \`cl
8184
- Refresh docs (README.md, book/, CHANGELOG.md) with lint counts (rounded to 50) and markdown links.
8285
- Manage deprecated/renamed lints in \`deprecated_lints.rs\` and dedicated UI tests.
8386

87+
For lints that require access to the configuration (\`Conf\`), such as to use \`conf.msrv\` for MSRV-dependent logic, the developer must manually update the instantiation of the lint pass in the \`register_lint_passes\` function in \`clippy_lints/src/lib.rs\`. Specifically, modify the closure to capture \`conf\` and pass it to the lint's \`new(conf)\` method, e.g., from \`Box::new(|_| Box::new(LintName))\` to \`Box::new(move |_| Box::new(LintName::new(conf)))\`. This step is not automated by \`update_lints\`.
88+
8489
This ensures new lints are automatically included when compiling \`clippy_lints\`.
8590

8691
### Lint Implementation Details
@@ -90,6 +95,7 @@ This ensures new lints are automatically included when compiling \`clippy_lints\
9095
- **Hooks**: Lints impl methods like \`check_expr\`, \`check_item\` using visitor patterns or queries via \`cx\`.
9196
- **Groups and Levels**: Lints assigned to categories (correctness, style, etc.) auto-grouped by \`LintListBuilder\`; levels (Allow, Warn, Deny).
9297
- **Fixes**: Use \`rustfix::diagnostics::Diagnostic::fix` for auto-fixes via \`--fix\`.
98+
- **Configuration Access**: Lints can implement a \`new(conf: &Conf)\` constructor to access runtime configuration, such as \`conf.msrv\` for adapting logic based on the project's minimum supported Rust version (configured in \`clippy.toml\`). This allows lints to provide accurate diagnostics respecting the MSRV, e.g., not flagging stabilized safe operations in older versions.
9399

94100
### Edge Cases and Extensibility
95101

pr-analysis-16108.md

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
# PR #16108: Workflow Design Impact Analysis
2+
3+
## Affected Workflows
4+
- **cargo-clippy (Workflow 1)**: Changed files include `clippy_config/src/conf.rs`, `clippy_lints/src/lib.rs`, and `clippy_lints/src/multiple_unsafe_ops_per_block.rs`, which are relevant to lint registration, configuration loading, and execution during Cargo subcommand runs. The PR makes the lint MSRV-aware, affecting diagnostic output based on configured MSRV.
5+
- **clippy-driver (Workflow 2)**: Similar to Workflow 1, as the driver uses the same lint registration (`register_lint_passes` with `conf`) and utilities (`msrvs`), impacting direct compilations.
6+
- **lint-development (Workflow 5)**: Primary impact; the PR demonstrates modifying an existing lint to use `Conf` and `msrvs` for version-specific logic, requiring manual update to `lib.rs` registration. Relevant files `clippy_lints/src/`, `clippy_utils/src/`, `tests/ui/`. Introduces process consideration for conf-dependent lints.
7+
- **testing (Workflow 4)**: Updates UI test files (`tests/ui/multiple_unsafe_ops_per_block.*`) to validate the new MSRV-dependent behavior of the lint.
8+
9+
## Workflow 1 Analysis
10+
### Summary of design changes
11+
The PR enhances lint execution by allowing lints like `multiple_unsafe_ops_per_block` to use `conf.msrv` for conditional logic, e.g., not counting safe operations post-Rust 1.92 as unsafe if MSRV >=1.92. This is implemented via updated lint constructor taking `conf`, passed during registration. Benefits: Better accuracy across Rust versions. No new components or steps; leverages existing conf mechanism. Doc updated to highlight example usage in configuration application.
12+
13+
No changes to Mermaid diagram required.
14+
15+
## Workflow 2 Analysis
16+
### Summary of design changes
17+
Analogous to Workflow 1; the driver loads conf and passes to lints, now enabling MSRV-based adaptations in lint passes. Doc updated in configuration loader and flexibility sections to note `conf.msrv` usage and example.
18+
19+
No changes to Mermaid diagrams required.
20+
21+
## Workflow 4 Analysis
22+
### Summary of design changes
23+
The PR updates UI test inputs and expected outputs for `multiple_unsafe_ops_per_block` to reflect new MSRV logic, ensuring comprehensive test coverage for version-dependent cases. No structural changes to testing workflow or designs; routine update aligned with lint evolution.
24+
25+
No Mermaid updates needed (assuming doc has diagrams; validation passed).
26+
27+
## Workflow 5 Analysis
28+
### Summary of design changes
29+
Key changes:
30+
- Added MSRV gate `SAFE_RAW_PTR_TO_UNION_FIELD` in `msrvs.rs` for lints to query version support.
31+
- Lint now holds `Msrv` from `conf.msrv`, uses it in visitor to conditionally classify operations.
32+
- Registration updated to `new(conf)`; lint added to `define_Conf!` list.
33+
- This affects lint development process: when adding conf access (e.g., for MSRV), manual edit to `lib.rs` register_lint_passes needed post-`update_lints`.
34+
- Benefits: Empowers lints with Rust evolution awareness, improving relevance.
35+
- Implications: Additional manual step; future tool enhancement possible.
36+
37+
Updated doc text in components, implementation details, automation section, other aspects.
38+
Updated scaffolding diagram to include optional manual step (addition in green via alt block).
39+
40+
```mermaid
41+
flowchart TD
42+
subgraph Changes from PR 16108
43+
A[Old: UpdateTool -> LibRs: Update mod declarations] --> B[Addition (Green): alt lint requires Conf access <br/> Developer -> LibRs: Manually update registration <br/> to pass conf to new(conf) end]
44+
C[No Removals (Red: none)]
45+
D[Process Change (Yellow): Scaffolding now includes conditional manual intervention after auto-updates for advanced lints using config/MSRV]
46+
end
47+
```
48+
The full updated scaffolding sequence diagram incorporates this addition for completeness.
49+

0 commit comments

Comments
 (0)