Remove redundant declarations - #34
Open
cepetr wants to merge 3 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR removes redundant declarations to allow enabling -Wredundant-decls, primarily by shifting VFS-related declarations/wrappers out of py/builtin.h and aligning module declarations with py/objmodule.h.
Changes:
- Removes redundant module and VFS-related declarations from
py/builtin.h, relying onpy/objmodule.hfor module externs. - Refactors VFS import stat and built-in
open()plumbing by adding wrappers in.cfiles (avoiding includingvfs.hfrombuiltin.h). - Adjusts
mp_hal_delay_usdefinition in the Unix port header to avoid redundant declaration warnings.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| py/parse.c | Includes py/objmodule.h to pick up module externs previously provided via py/builtin.h. |
| py/modbuiltins.c | Adds VFS header include and a VFS-to-builtin open() adapter function/object when MICROPY_VFS is enabled. |
| py/builtinimport.c | Adds VFS header include and a VFS-backed mp_import_stat implementation when MICROPY_VFS is enabled. |
| py/builtin.h | Removes redundant VFS/module declarations to eliminate -Wredundant-decls triggers. |
| ports/unix/mphalport.h | Defines mp_hal_delay_us as a self-referential macro before the inline definition to suppress redundant prototypes from py/mphal.h. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
cepetr
force-pushed
the
cepetr/rm-redundant-decls
branch
2 times, most recently
from
August 26, 2026 12:38
f18a8d1 to
b159455
Compare
cepetr
force-pushed
the
cepetr/rm-redundant-decls
branch
2 times, most recently
from
August 26, 2026 13:32
36b3f8e to
022c794
Compare
mmilata
approved these changes
Aug 26, 2026
mmilata
left a comment
Member
There was a problem hiding this comment.
LGTM but please fix author of the last commit
Author: nobody <nobody@satoshilabs.com>
cepetr
force-pushed
the
cepetr/rm-redundant-decls
branch
from
August 26, 2026 19:03
022c794 to
6d41cf0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
These changes are required to enable
-Wredundant-declsin the firmware repository (trezor/trezor-firmware#7698).builtin.h, which are also declared inpy/objmodule.h(imported fromgenhdr/)builtin.h. Sincevfs.hcannot be included inbuiltin.hdue to a circular dependency, a slightly larger refactoring was neededmp_hal_delay_usis defined to remove the redundant declaration warning. It is now consistent with other declarations inmphalport.hThis fix doesn't resolve all redundant declarations in the MicroPython code, just those that affect us.