Skip to content

Conversation

bmwangmh
Copy link

@bmwangmh bmwangmh commented Jul 11, 2025

Related Issues

Type of Pull Request

  • Bug fix
  • New feature
    • I have already discussed this feature with the maintainers.
  • Refactor
  • Performance optimization
  • Documentation
  • Other (please describe):

Does this PR change existing behavior?

  • Yes (please describe the changes below)
    • All diagnostics will be flitered then published. Simply remove all duplicated diagnostics.
  • No

Does this PR introduce new dependencies?

  • No
  • Yes (please check binary size changes)

Checklist:

  • Tests added/updated for bug fixes or new features
  • Compatible with Windows/Linux/macOS

Copy link

semanticdiff-com bot commented Jul 11, 2025

Review changes with  SemanticDiff

Changed Files
File Status
  crates/moonbuild/src/entry.rs  40% smaller

Copy link

Missing import for HashSet

Category
Correctness
Code Snippet
Lines 271, 295: let mut seen = HashSet::new();
Recommendation
Add use std::collections::HashSet; at the top of the file or ensure HashSet is imported in the module's use statements
Reasoning
The code uses HashSet::new() but there's no visible import for HashSet in the diff, which will cause compilation errors

Duplicated deduplication logic across two code paths

Category
Maintainability
Code Snippet
Lines 271-279 and 295-300: Both code blocks implement identical HashSet-based deduplication
Recommendation
Extract the deduplication logic into a helper function or method that can be reused by both code paths
Reasoning
Having identical logic in two places violates DRY principle and makes maintenance harder - future changes to deduplication behavior would need to be made in multiple places

Potential memory overhead for large diagnostic sets

Category
Performance
Code Snippet
Lines 271, 295: let mut seen = HashSet::new();
Recommendation
Consider adding capacity hints if the expected number of diagnostics is known, or document memory usage expectations for large builds
Reasoning
HashSet will grow dynamically, potentially causing multiple reallocations. For large diagnostic sets, this could impact memory usage and performance

@bmwangmh bmwangmh force-pushed the fix-wbtest-duplicate-diag branch 2 times, most recently from 0595dee to 2820133 Compare July 11, 2025 05:10
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.

1 participant