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

c_array --> c_ptr error improvements #25343

Merged
merged 14 commits into from
Jun 28, 2024

Conversation

vasslitvinov
Copy link
Member

@vasslitvinov vasslitvinov commented Jun 23, 2024

This PR improves the error message when passing a const c_array to a formal of type c_ptr. It also fixes the error-ness and the diagnostics when a c_array is cast to c_ptr[Const] and improves error messages for arrays, lists, and c_array that are related to their element types.

This PR resolves #24797 and more. It implements Option 2 in #24797 (comment), meaning it preserves the behavior prior to this PR. The contribution of this PR in this regard is an added clarification that explains to the user why the error reported in #24797 occurred.

Specifically, error: const actual is passed to 'ref' formal 'x' of operator : is now followed by while coercing an actual to formal 'a' .

Dev history as of 32e05b3 contain an implementation of Option 1 in the above comment. Option 1 is preferred in the long run, see #22809 and #24797 (comment). Note that #22809 talks about ref vs. const ref formals whereas #24797 is concerned with passed-by-value c_ptr vs. c_ptrConst formals.

The reason this PR implements Option 2 is stability concerns. Switching to Option 1 in the future will not break any existing working programs. Whereas switching from O-1 to O-2, in case we end up liking O-2 in the long run, will invalidate some programs.

This PR also fixes problems with resolving among the overloads of operator : from c_array to c_ptr[Const] by refactoring these overloads. For example:

  • The cast c_array(int) --> c_ptrConst(void) resulted in an "ambiguous call" error. It is allowed now.

  • The cast c_array(int) --> c_ptr(real) was allowed. It is an error now.

  • The cast c_array(int) --> c_ptrConst(real) resulted in an "ambiguous call" error. Now it is an error "cast of c_array to c_ptr with a different element type".

While there

Enable : to be the function name in the --explain-call option, given that other operator names are allowed. For example, --explain-call "<:23" was already legal, this PR makes the following legal as well: --explain-call "::23". I had tried to use this several times in the past and been set back by the inability to do so.

Tidy up / regularize the checks for creating an array, c_array, or list with the element type of void or nothing. Prior to this PR the creation of a c_array with void/nothing element type resulted in a compiler crash or an unclear error message.

Have the errors upon the creation of a list with void/nothing element type point directly into user code, not list implementation code. This allowed me to remove a couple of prediffs. We should apply this change to maps and sets as well, to improve their error messages.

Allow certain error messages to show the name of the formal also in standard modules, where previously they would show the name of the formal only in user modules. Rationale: the names of the formals in standard modules are part of those modules' interfaces and should not be hidden.

Minor verbage and quotation improvements in error messages and in the --help message for --explain-call and --explain-instantiation.

Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
@vasslitvinov
Copy link
Member Author

Switching this PR to "draft" until I implement Option 2. This is primarily due to 2.0-stability considerations. Option 1 permits some programs that Option 2 does not. So if we end up choosing Option 2, to be consistent with the status quo in #22809 or for other considerations, switching to it from Option 1 would be a breaking change.

Option 1 is implemented in these commits:
4e65ae6 Do not match 'const' c_array arg to c_ptr formal
c583f47 DO match 'const' c_array arg to c_ptrConst formal
02db559 Fix test/extern/fer./c-array/c-array-coercions
ccfd913 Add 'const' to the explanation message
which this commit reverts.

Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
Signed-off-by: Vassily Litvinov <vasslitvinov@users.noreply.github.com>
@vasslitvinov vasslitvinov marked this pull request as ready for review June 28, 2024 01:13
Copy link
Contributor

@dlongnecke-cray dlongnecke-cray left a comment

Choose a reason for hiding this comment

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

Lots of cleanup. LGTM!

@vasslitvinov vasslitvinov merged commit d1c213d into chapel-lang:main Jun 28, 2024
7 checks passed
@vasslitvinov vasslitvinov deleted the better-const-errors branch June 28, 2024 03:06
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.

[Bug]: Need better error message for const passed to c_ptr
2 participants