[71iq]

In C, the front end accepted an implicit conversion that adds qualifiers below the first level, such as int ** to const int *const *, without a diagnostic. C++ allows this conversion. In C the pointed-to types are not compatible, so it violates the constraints of simple assignment (C23 6.5.17.2), and a diagnostic is required. GCC 14 and later reject it by default.

In types.c, a C conversion that takes this path now gets the caller's default warning when no more specific warning applies. The conversion is still accepted.

regressions/gh-514 checks for the warning in assignment, initialization, argument passing, return, and a three-level conversion, in GNU C23 mode and with --strict_warnings. regressions/gh-514-positive checks that adding const only at the first level, or converting with an explicit cast, gets no diagnostic in strict mode, and regressions/gh-514-cpp that C++ is unchanged.

Tested with the full test suites for edg_x86_64 and edg_x86_64_cp, and with the new and changed tests for edg_x86_32 and edg_x86_32_cp.

Fixes #514

Summary by CodeRabbit

• Bug Fixes
  • C mode now warns when pointer conversions add const qualifiers below the first pointer level, including assignments, initializations, function arguments, and return values.
  • Conversions that add qualifiers only at the first pointer level remain accepted without the new warning.
  • C++ mode continues to accept the corresponding deep-qualification conversions.
• Tests
  • Added regression coverage for these conversion cases in C and C++.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

C mode now warns for implicit pointer conversions that add qualifiers below the first pointer level. Regression tests cover the diagnostic cases, accepted first-level qualification, explicit casts, and corresponding C++ conversions.

Changes: C Pointer Qualification Diagnostics

Layer / File(s)	Summary
C-mode diagnostic behavior src/types.c, src/pending_changes/gh-514-c-mode-cv-qual-conv.txt	C mode assigns a default warning to these conversions when the qualification check does not return a specific warning. The change note describes the conversion as accepted with a diagnostic in C and contrasts this with C++.
C and C++ regression coverage tests/tests/regressions/gh-514.sft.c, tests/tests/regressions/gh-514-positive.sft.c, tests/tests/regressions/gh-514-cpp.sft.cpp, tests/tests/regressions/.gh-514*.rto/*, tests/tests/regressions/.gh-514-cpp.rto/*	C tests cover assignment, initialization, argument passing, return values, and three pointer levels. Positive cases cover first-level qualification and explicit casts. C++ tests cover the corresponding conversion, and expected outputs record the C diagnostics.

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• edgcpp/compiler#455: Changes the pointer-qualification fallback in impl_pointer_conversion, which is also changed here for C mode.

Suggested labels: bug

Suggested reviewers: nv-cmeerw

Merge Risk: Minimal · up to 5c924

C code that implicitly adds qualifiers below the first pointer level now gets a warning, and the conversion is still accepted. No actionable merge-blocking risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: the C front end now diagnoses pointer conversions that add qualifiers below the first pointer level, which matches the types.c change, the GH #514 change note, and the…
Linked Issues check		Issue #514 requires a diagnostic for implicit C pointer conversions that add qualifiers below the first level (for example, int ** to const int *const *) in assignment, initialization, argument passin…
Out of Scope Changes check		The whole-PR changes are limited to src/types.c (a conditional C-mode warning on the below-first-level qualifier conversion path), a pending change note for #514, and regression tests named gh-514, gh…

Full details: Title check

Explanation

The title names the main change: the C front end now diagnoses pointer conversions that add qualifiers below the first pointer level, which matches the types.c change, the GH #514 change note, and the regression tests. The bracketed issue reference is accurate. The PR objectives cite a different PR number (#498) in the body, but the title itself matches the changeset.

Full details: Linked Issues check

Explanation

Issue #514 requires a diagnostic for implicit C pointer conversions that add qualifiers below the first level (for example, int ** to const int *const *) in assignment, initialization, argument passing, and return. The diff to src/types.c adds a branch in the qualifier-addition path. When C_mode() is true and no specific warning was set (warning_suggested == ec_no_error), the branch sets warning_suggested to default_warning_code. The conversion stays accepted, and any specific warning from the check is kept. The same conversion path serves all four contexts, so the diagnostic applies to each. The added tests cover assignment, initialization, argument passing, return, and a three-level conversion (regressions/gh-514, with the expected output in default.1.1.txt and default.2.1.txt). regressions/gh-514-positive covers first-level-only const addition and explicit casts with no diagnostic. regressions/gh-514-cpp covers C++ behavior. The regression expected outputs were not inspected line by line, so the exact diagnostic text is not confirmed. The code change is consistent with the stated requirement.

Full details: Out of Scope Changes check

Explanation

The whole-PR changes are limited to src/types.c (a conditional C-mode warning on the below-first-level qualifier conversion path), a pending change note for #514, and regression tests named gh-514, gh-514-positive, and gh-514-cpp. The source change implements the #514 requirement. The tests and change note support that change. No unrelated functional change was found in the reviewed files.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: edgcpp/compiler
• Branch: main
• File: .coderabbit.yaml
• Commit: 8fa21be
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 0a86d878-3bd1-4391-b329-c9353d492c25

Commits

Reviewing files that changed from the base of the PR and between 8fa21be and 5c9244f.

Files selected for processing: 9

• src/pending_changes/gh-514-c-mode-cv-qual-conv.txt
• src/types.c
• tests/tests/regressions/.gh-514-cpp.rto/default.1.1.txt
• tests/tests/regressions/.gh-514-positive.rto/default.1.1.txt
• tests/tests/regressions/.gh-514.rto/default.1.1.txt
• tests/tests/regressions/.gh-514.rto/default.2.1.txt
• tests/tests/regressions/gh-514-cpp.sft.cpp
• tests/tests/regressions/gh-514-positive.sft.c
• tests/tests/regressions/gh-514.sft.c

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

[edgcpp-bot]

AcknowlEDG review results updated for 3deca25d2018:

• Test - Changes (new)

[edgcpp-bot]

AcknowlEDG review results updated for 3deca25d2018:

• Bench - C (new)
• Bench - C++ (new)
• Test - Changes (unchanged)

CI success.

[daveedvdv-nvidia]

Hello!
I see all your pull requests are in "Draft" state. Is that intentional? Or are you looking for them to be reviewed?

[71iq]

Hello!
I see all your pull requests are in "Draft" state. Is that intentional? Or are you looking for them to be reviewed?

My bad, I forgot to publish haha.

[daveedvdv-nvidia]

I'll try to review these soon.

[daveedvdv-nvidia]

Thanks! This looks right, overall.

A few small things:

1. Comment: Rather than adding a second comment, could you fold this into the existing one just above (which still says "This is an extension in C mode.")? Something like:

          /* Allow conversion between pointers where type qualifiers are
             being added at levels other than the first, e.g.,
             "int **" -> "const int * const *".  These are the const-safe
             cases.  In C mode, this is an extension, and a diagnostic is
             issued since the types pointed to are not "compatible". */

   (Also, front-end comments usually capitalize after a colon, as in "...: The pointed-to types are incompatible." The exception is if the colon
   is followed by a list that cannot be a standalone sentence.)

2. Test headers: Please add a //remark: line to each of the three tests (it's what shows up in the test results and expectation logs), e.g.:

   //remark:C: impl. ptr conv. w/ qualifiers below first level
   

3. Test coverage: c-deep-pointer-qualification only covers assignment. Since initialization, argument passing, and return each produce a different diagnostic, it would be good to include them too Also, add a three-level case.

4. C++ test: Its recording contains warning: variable "q" was set but never used (the front end issues that in C++ even with (void)q, independently of this change). Not a problem, but if you were to use q differently (e.g., pass it to a function), the recording wouldn't be as noisy.

5. Issue: I created C: no diagnostic for implicit pointer conversions that add qualifiers below the first level #514 for this (from discussion EDG silently accepts deep pointer qualification conversions in C #472). Please use its number in the commit message ([GH #514] instead of []), the Changes entry tag, and the pending Changes file name (e.g., gh-514-c-mode-cv-qual-conv.txt), and consider naming the tests gh-514, gh-514-positive, and gh-514-cpp.

FYI, #455 changes the else if right after the lines you added, so whichever of the two lands second will need a (trivial) rebase.

[71iq]

Thanks for the review. All five points are addressed in the force-pushed commit, which is also rebased on current main:

1. The new comment is folded into the existing one, using your wording.
2. Each test has a //remark: line.
3. gh-514 now covers assignment, initialization, argument passing and return, each with its own diagnostic, plus a three-level conversion.
4. The C++ test passes q to a function, so its recording is empty.
5. The commit, the Changes entry (gh-514-c-mode-cv-qual-conv.txt) and the tests (gh-514, gh-514-positive, gh-514-cpp) use C: no diagnostic for implicit pointer conversions that add qualifiers below the first level #514.

I'll rebase whichever of this and #455 lands second.

[edgcpp-bot]

AcknowlEDG review results updated for 5c9244ff43ee:

• Bench - C (updated)
• Bench - C++ (updated)
• Test - Changes (updated)

CI success.