Skip to content

Reject C++ pointer casts to integer types that are too small [EDGcpfe/28892] - #454

Draft
71iq wants to merge 1 commit into
edgcpp:mainfrom
71iq:fix/edg-28892-pointer-integer-cast
Draft

71iq wants to merge 1 commit into
edgcpp:mainfrom
71iq:fix/edg-28892-pointer-integer-cast

Conversation

@71iq

@71iq 71iq commented Nov 1, 2026 •

Copy link
Copy Markdown

In GNU C++ mode, including Clang mode, a cast of a pointer to an integer type too small to hold it was accepted with a warning. An example is (int)ptr on a 64-bit target. GCC 4.0 and later, and Clang, reject it.

In types.c, the gpp_mode test that allowed this is now gpp_version_is(<40000). GNU C++ before 4.0 accepted such casts with a warning, so code written for those versions keeps working. C and Microsoft mode are unchanged.

regressions/edg-28892 covers the error for object and function pointers, regressions/edg-28892-positive a cast to a wide enough type, and regressions/edg-28892-legacy the warning with --gnu_version=30406.

Nine imported Clang and GCC tests are re-recorded, and each now gets the error its source expects. gnu/cpp/parse/cast1 is an ILP32 test upstream, so it now fails on the 64-bit configurations, as it does with GCC, and passes on the 32-bit ones. gnu/cpp/template/sfinae13 now passes.

Tested with the full test suites for edg_x86_64 and edg_x86_64_cp, compared with a run of main, and with the new and changed tests for edg_x86_32 and edg_x86_32_cp.

The Changes entry uses this PR's number until there is an issue for it.

Reported in discussion #298.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Walkthrough

The conversion check no longer accepts undersized pointer-to-integer casts solely because GNU mode is enabled. Regression tests cover narrow object and function pointer casts, as well as a sufficiently wide destination type.

Changes: Pointer-to-integer cast validation

Layer / File(s) Summary
Conversion rule and regression coverage
src/types.c, tests/tests/regressions/edg-28892.sft.cpp, tests/tests/regressions/edg-28892-positive.sft.cpp, src/pending_changes/edg-28892-pointer-integer-cast.txt
GNU C++ mode no longer permits undersized destinations solely because GNU mode is enabled. C mode and Microsoft extensions retain the size exception. The regression tests check narrow object and function pointer casts and a sufficiently wide destination. The pending-change note records the restriction.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested labels: bug

Suggested reviewers: nv-cmeerw

Merge Risk: Low · up to 8118f

Add the expected recordings before relying on these regression tests to validate the pointer-cast change.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title "Reject C++ pointer casts to integer types that are too small [EDGcpfe/28892]" matches the main change. The raw summary shows that the pending-change note, the types.c update, and the new re…
Full details: Title check

Explanation

The title "Reject C++ pointer casts to integer types that are too small [EDGcpfe/28892]" matches the main change. The raw summary shows that the pending-change note, the types.c update, and the new regression tests all target pointer-to-integer reinterpret_cast conversions in C++ GNU mode when the destination is too small. The PR objectives describe the same change. The bracketed tracker tag is extra detail, but it does not make the title unclear.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between ee3b38e and 8118f59.

Files selected for processing: 4

  • src/pending_changes/edg-28892-pointer-integer-cast.txt
  • src/types.c
  • tests/tests/regressions/edg-28892-positive.sft.cpp
  • tests/tests/regressions/edg-28892.sft.cpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.

71iq added a commit to 71iq/compiler that referenced this pull request Nov 1, 2026
@edgcpp-bot

Copy link
Copy Markdown
Collaborator

AcknowlEDG review results updated for 389fa8f1f379:

CI other.

71iq added a commit to 71iq/compiler that referenced this pull request Nov 1, 2026

@daveedvdv-nvidia daveedvdv-nvidia left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See suggested changes.

Reject C++ pointer casts to undersized integers

In GNU C++ mode, pointer-to-integer reinterpret_cast conversions now
require a destination type large enough to hold the pointer value.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd add a small example here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. The entry now shows reinterpret_cast<unsigned char>(p) on a target with 64-bit pointers, which is now an error with --g++.

Comment thread src/types.c
is_integral(dest_type) &&
(C_mode() || ms_extensions || gpp_mode ||
(C_mode() || ms_extensions ||
dest_of_ptr_cast_big_enough(source_type, dest_type))) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I tracked down why that gpp_mode is there. It's for this test case:

int main()
{
        int *ptr;
        int x;

        x = (int)ptr;
        x = (short)ptr;  // warning in gcc 3.4.6

        return 0;
}

Prior to GCC 4, this was accepted with a warning (https://godbolt.org/z/fG4YcKKr9).
Note that your test case was not accepted even then.
Rather than emulating this 100% accurately, I'd just replace the gpp_mode by gpp_version_is(<40000). That way any pre-GCC-4 code out there that relies on the leniency will continue to work (which should not be a whole lot), and modern code will track GCC 4+.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks for tracking that down. I replaced gpp_mode with gpp_version_is(<40000), so Clang mode and GNU C++ 4.0 and later now give the error. regressions/edg-28892-legacy is your example with --gnu_version=30406: (int)ptr and (short)ptr are accepted with a warning.

@codecov

codecov Bot commented Nov 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@71iq

71iq commented Nov 2, 2026

Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Action performed

Full review finished.

@coderabbitai coderabbitai Bot 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.

  • Fix comments: Fix CodeRabbit comments on this PR

@edgcpp-bot

Copy link
Copy Markdown
Collaborator

AcknowlEDG review results updated for 8118f597e7ba:

CI failure.

…/28892]

In GNU C++ mode (including Clang mode), a cast of a pointer to an integer
type too small to hold it was accepted with a warning.  GCC 4.0 and
later, and Clang, reject it.  Keep the leniency only when emulating GNU
C++ versions before 4.0, which accepted such casts with a warning.

Add regression tests for the error, the accepted wide cast, and the
pre-4.0 GNU behavior, and update the recordings of imported tests that
now get the error their sources expect.  gnu/cpp/parse/cast1 is an ILP32
test upstream; it now fails on 64-bit configurations, as with GCC.
@71iq
71iq force-pushed the fix/edg-28892-pointer-integer-cast branch from 8118f59 to d4836c2 Compare November 8, 2026 09:28
@71iq 71iq changed the title Reject C++ pointer casts to undersized integers Reject C++ pointer casts to integer types that are too small [EDGcpfe/28892] Nov 8, 2026
@edgcpp-bot

Copy link
Copy Markdown
Collaborator

AcknowlEDG review results updated for d4836c2b19ab:

CI success.

This branch has not been deployed

No deployments
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