Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opt-in link-time lint warnings, e.g. when ld can be ldh, or jp can be jr #1017

Open
3 tasks
Rangi42 opened this issue Aug 4, 2022 · 4 comments
Open
3 tasks
Labels
enhancement Typically new features; lesser priority than bugs rgbasm This affects RGBASM rgblink This affects RGBLINK

Comments

@Rangi42
Copy link
Contributor

Rangi42 commented Aug 4, 2022

These would be useful as opt-in link-time warnings:

  • For every ld, check for and suggest using ldh instead if possible.

    (This would be an improved substitute for the removed feature of rgbasm -L, which optimized known-constant ld to ldh.)

  • For every jp, check for and suggest using jr instead if possible.

  • For every control flow instruction (call, jp, jr, rst) targeting a label in ROMX, do a lint equivalent of assert BANK(@) == 0 || BANK(<target>) == 0 || BANK(@) == BANK(<target>).

    (rst always targets ROM0, but if we add support for swappable ROM0, possibly via HDFs ([Feature request/proposal] Hardware layout description files (custom mapper support) #524), then this would be multi-banked.)

    Do this only if PC is in a ROMX section, which also excludes LOAD. (In theory, BANK(@) != 0 for ROMX, but -t maps ROMX to ROM0. The destination is then guaranteed to be in ROM0 as well, but that may not remain the case if we add some similar feature to HDFs, so a cheap stringent check for BANK(@) == 0 is good for future-proofing.)

    (Possibly make this a multi-level lint, where by default cross-SECTION jumps are not checked?)

@Rangi42 Rangi42 added enhancement Typically new features; lesser priority than bugs rgblink This affects RGBLINK labels Aug 4, 2022
@aaaaaa123456789
Copy link
Member

I'd say that all warnings should be enabled by -W flags. Other than that, good idea.

@Rangi42 Rangi42 added this to the 0.6.0 milestone Aug 27, 2022
@Rangi42 Rangi42 added the rgbasm This affects RGBASM label Aug 27, 2022
@Rangi42 Rangi42 modified the milestones: 0.6.0, v0.7.0 Aug 27, 2022
@Rangi42
Copy link
Contributor Author

Rangi42 commented Aug 27, 2022

Hm, so rgbasm and rgblink would both need flags for this.

@Rangi42
Copy link
Contributor Author

Rangi42 commented Dec 11, 2022

As pointed out in #1114, it would be useful to allow each warning for all occurrences of a pattern, or just the first occurrence, or not warn for that pattern. ("A pattern" might be "ld which can be ldh", or "jp which can be jr", or "ld X, Y after ld Y, X", or "call+ret which can be jp", or so on.)

Inline comments or pragmas to suppress warnings at particular sites would also be useful (e.g. maybe this ld [$ff00], a is in a LOAD section and will be modifiable to non-HRAM addresses).

@im-mi
Copy link

im-mi commented Dec 28, 2022

Checking if a jump target is close enough to use jr when the target label exists in a different section is bound to cause all sorts of unwanted warnings as sections get shifted around by the linker. Perhaps that feature should be limited to cases where the jump target is in the same section (or section fragment, for fragmented sections).

On a similar note, a warning could also be produced when performing a suspicious call or jump. For example: if code in a non-fixed ROM bank is calling a subroutine in a different non-fixed ROM bank, then it's probably a bug.

Of course many of these features can be implemented using macros, but having them built-in would be pretty convenient.

@Rangi42 Rangi42 removed this from the v0.9.0 milestone Aug 6, 2024
@Rangi42 Rangi42 mentioned this issue Sep 18, 2024
3 tasks
@Rangi42 Rangi42 changed the title Opt-in warnings when ld can be ldh, or jp can be jr Opt-in link-time lint warnings, e.g. when ld can be ldh, or jp can be jr Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Typically new features; lesser priority than bugs rgbasm This affects RGBASM rgblink This affects RGBLINK
Projects
None yet
Development

No branches or pull requests

4 participants