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

Make procedure mismatch more informative with pragma/call convention mismatches #18384

Merged
merged 14 commits into from
Jul 6, 2021

Conversation

beef331
Copy link
Collaborator

@beef331 beef331 commented Jun 28, 2021

The following is a few showcases of the output changes

Examples:

proc a(a: int, c: float){.cdecl.} = echo "Hmm"
var b: proc(a: int, c: float){.noSideEffect.} = a
b(10, 3.14)

Used to output:

Error: type mismatch: got 'proc (a: int, c: float){.cdecl, gcsafe, locks: 0.}' for 'a' but expected 'proc (a: int, c: float){.closure, noSideEffect.}'

Now outputs:

Error: type mismatch: got 'proc (a: int, c: float){.cdecl, gcsafe, locks: 0.}' for 'a' but expected 'proc (a: int, c: float){.closure, noSideEffect.}'
Pragma mismatch got '{..}', but expected '{.noSideEffect.}'.
proc a(a: int, c: float){.cdecl.} = echo "Hmm"
var b: proc(a: int, c: float){.nimcall, noSideEffect.} = a
b(10, 3.14)

Used to output:

Error: type mismatch: got 'proc (a: int, c: float){.cdecl, gcsafe, locks: 0.}' for 'a' but expected 'proc (a: int, c: float){.nimcall, noSideEffect.}'

Now Outputs:

Error: type mismatch: got 'proc (a: int, c: float){.cdecl, gcsafe, locks: 0.}' for 'a' but expected 'proc (a: int, c: float){.nimcall, noSideEffect.}'
Calling convention mismatch got '{.cdecl.}' but expected '{.nimcall.}'.
Pragma mismatch got '{..}', but expected '{.noSideEffect.}'.

compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
@ringabout
Copy link
Member

Please add a test.

@beef331
Copy link
Collaborator Author

beef331 commented Jun 29, 2021

Tests will be added, Timothee wanted the PR up so there could be some discussion to what this should contain, so after there is a agreement on what this should show/do and I implement it we'll get tests.

compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
@beef331 beef331 changed the title Added more concise calling convention/pragma mismatch messages Make procedure mismatch more informative with pragma/call convention mismatches Jul 2, 2021
compiler/sigmatch.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
Copy link
Member

@timotheecour timotheecour left a comment

Choose a reason for hiding this comment

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

LGTM, the remaining comments are not critical and can be addressed in followup PR; great addition!

compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
compiler/types.nim Outdated Show resolved Hide resolved
@timotheecour
Copy link
Member

CI failure unrelated => SciNim/Datamancer#10

@Araq Araq merged commit 252eea8 into nim-lang:devel Jul 6, 2021
PMunch pushed a commit to PMunch/Nim that referenced this pull request Mar 28, 2022
…mismatches (nim-lang#18384)

* Added more concise calling convention/pragma mismatch messages
* Now only adds callConvMsg/lock message  when sensible
* Fixed message formatting
* Added tests, and fixed some bugs
* Tests joined, and always indenting
* More tests and more bug fixes
* Fixed first test in tprocmismatch
* Using var param for writting mismatches
* Better logic for handling proc type rel and conv/pragma mismatch
* Refactored getProcConvMismatch
* Fixed callConv message formatting
* Fixed test for proper message
* Cleanup to address issues
* getProcConvMismatch now returns tuple, and reformatted code
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.

None yet

5 participants