Refactor/remove redundant ast filters

[Perdixky]

Summary by CodeRabbit

• Refactor
  • Streamlined declaration visitor infrastructure by removing explicit filters for implicit declarations and implicitly-instantiated template specializations. These previously-excluded declarations now undergo standard processing, providing more comprehensive analysis and consistent behavior. The simplified logic reduces code complexity while maintaining existing special-case handling.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a9d8a81a-5da4-4661-93f2-b58ae2a22b5c

📥 Commits

Reviewing files that changed from the base of the PR and between 0769540 and 4fa9fe6.

📒 Files selected for processing (1)

• src/semantic/filtered_ast_visitor.h

🚧 Files skipped from review as they are similar to previous changes (1)

• src/semantic/filtered_ast_visitor.h

---

📝 Walkthrough

Walkthrough

The FilteredASTVisitor in filtered_ast_visitor.h has been modified to remove explicit filters that previously prevented traversal of implicit declarations and implicit template instantiations, allowing these to proceed through the visitor's normal hooks instead.

Changes

Cohort / File(s)	Summary
Implicit Declaration Traversal Filters src/semantic/filtered_ast_visitor.h	Removed 23 lines of early-return guards that skipped traversal for implicit declarations and implicit template instantiations. Added clarifying comment. Traversal of these declarations now proceeds through standard hooks.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 The filters fall away like morning dew,
Implicit paths now visible and true,
No more gates to block the visitor's way,
Traversal flows more freely every day! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing redundant AST filters from the FilteredASTVisitor, which directly aligns with the refactoring work shown in the code changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/setup-llvm.py`:
- Around line 205-215: The config parsing loop in scripts/setup-llvm.py is too
strict: change the loop that reads cfg.read_text(...) so it normalizes each line
(strip leading/trailing whitespace), accepts both "--sysroot=PATH" and
"--sysroot PATH" forms, strips surrounding quotes from PATH, resolves and
validates the Path, and continue scanning all lines but keep the last valid
sysroot found (instead of returning the first); ensure you still handle OSError
as before and return None if no valid sysroot is found. Use the existing
variables/functions (cfg.read_text, sysroot, Path.resolve(), sysroot.exists())
to locate and update the parsing logic.
- Around line 341-358: The code currently sets needs_install=True when a
provided candidate exists but fails llvm_install_matches_compiler, but leaves
install_path unset so the bundled LLVM is extracted into install_root instead of
the user-specified path; update the branch that checks
llvm_install_matches_compiler(candidate) to explicitly honor the user's
candidate by setting install_path = candidate (and leave needs_install = True so
installation proceeds), and change the log message to clearly state "will
install bundled LLVM to provided path {candidate} (outside active compiler
sysroot)"; reference the symbols args.install_path, candidate,
llvm_install_matches_compiler, install_path, needs_install and install_root to
locate and modify the logic and message.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f72d2ccd-4edb-4fa8-b426-72644940a542

📥 Commits

Reviewing files that changed from the base of the PR and between d42d9d5 and 0769540.

📒 Files selected for processing (3)

• cmake/llvm.cmake
• scripts/setup-llvm.py
• src/semantic/filtered_ast_visitor.h

[Copilot]

Pull request overview

This PR refactors AST traversal to remove redundant implicit-declaration/template-instantiation filtering, and hardens LLVM dependency setup to avoid linking against LLVM installs that are ABI-incompatible with sandboxed toolchains (e.g., pixi/NixOS).

Changes:

• Simplifies FilteredASTVisitor by dropping explicit filtering of implicit decls and implicit template instantiations.
• Enhances setup-llvm.py to detect the active compiler sysroot and reject LLVM installs outside it.
• Updates LLVM CMake integration to also include ${binary_dir}/include so downloaded “private” Clang headers are found.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/semantic/filtered_ast_visitor.h	Removes implicit/implicit-instantiation skip logic from decl traversal.
scripts/setup-llvm.py	Adds compiler sysroot detection and filters out LLVM installs that fall outside the sysroot.
cmake/llvm.cmake	Adds build-tree include directory for fetched private Clang headers.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.