Skip to content

Remove redundant declarations - #34

Open
cepetr wants to merge 3 commits into
trezor-v1.28.0from
cepetr/rm-redundant-decls
Open

Remove redundant declarations#34
cepetr wants to merge 3 commits into
trezor-v1.28.0from
cepetr/rm-redundant-decls

Conversation

@cepetr

@cepetr cepetr commented Aug 26, 2026

Copy link
Copy Markdown

These changes are required to enable -Wredundant-decls in the firmware repository (trezor/trezor-firmware#7698).

  • Removes redundant external declarations of modules from builtin.h, which are also declared in py/objmodule.h (imported from genhdr/)
  • Removes redundant declarations of VFS functions from builtin.h. Since vfs.h cannot be included in builtin.h due to a circular dependency, a slightly larger refactoring was needed
  • Modifies how mp_hal_delay_us is defined to remove the redundant declaration warning. It is now consistent with other declarations in mphalport.h

This fix doesn't resolve all redundant declarations in the MicroPython code, just those that affect us.

@cepetr cepetr self-assigned this Aug 26, 2026
@cepetr
cepetr requested review from mmilata and a lite review from Copilot August 26, 2026 11:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 on py/objmodule.h for module externs.
  • Refactors VFS import stat and built-in open() plumbing by adding wrappers in .c files (avoiding including vfs.h from builtin.h).
  • Adjusts mp_hal_delay_us definition 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.

Comment thread py/builtinimport.c
@cepetr
cepetr removed the request for review from mmilata August 26, 2026 12:00
@cepetr
cepetr force-pushed the cepetr/rm-redundant-decls branch 2 times, most recently from f18a8d1 to b159455 Compare August 26, 2026 12:38

@mmilata mmilata left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but please fix author of the last commit

Author: nobody <nobody@satoshilabs.com>

@cepetr
cepetr force-pushed the cepetr/rm-redundant-decls branch from 022c794 to 6d41cf0 Compare August 26, 2026 19:03
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.

3 participants