Skip to content

Conversation

@duchainer
Copy link

@duchainer duchainer commented Oct 16, 2025

WHY?

First Odin users that try to use append often forget to pass a pointer, and pass-by-value.

This is a first attempt at helping with that, but a better version would be a generic way to check when any proc values should be pass-by-pointer instead of pass-by-value, as discussed at https://forum.odin-lang.org/t/least-parsable-most-common-error-message/1241/3

image

Better takes on it

  1. If we keep the gb_string_is_equal, we should have the append_proc_name declaration defined once or at compile time, instead of on all check_call_arguments_proc_group with non-empty arguments.
  2. The generic approach for all missed pass-by-pointer arguments:
    a. Check, efficiently, if any argument match a pointer type of the possible procs
    b. If so, have the pass-by-pointer message for that/those argument(s).

Why not have done those things already

I need more practice with C++, not yet familiar enough to make it good and generic yet.
So I might come back to it later.

@duchainer duchainer force-pushed the pointer-tips-for-append-error-message branch 2 times, most recently from c813594 to b562069 Compare October 16, 2025 16:55
WHY?

First Odin users that try to use append often forget to pass a pointer,
and pass-by-value.

This is a first attempt at helping with that, but a better version would
be a generic way to check when any proc values should be pass-by-pointer instead of
pass-by-value, as discussed at https://forum.odin-lang.org/t/least-parsable-most-common-error-message/1241/3
@duchainer duchainer force-pushed the pointer-tips-for-append-error-message branch from b562069 to 1b85af9 Compare October 16, 2025 17:04
@Creativty
Copy link
Contributor

I think a better solution would be to whenever a procedure takes a pointer, and you pass the underlying type of the pointer, the suggestion comes up. Not just hard-coding it for append.
Although I can see this being a pain in the ass to handle due to procedure groups.

@duchainer
Copy link
Author

I think a better solution would be to whenever a procedure takes a pointer, and you pass the underlying type of the pointer, the suggestion comes up. Not just hard-coding it for append. Although I can see this being a pain in the ass to handle due to procedure groups.

Yeah I agree that this is a naive way and that the generic approach would be way better.

I also have an open PR for the more generic non-proc-group polymorphic proc case : #5818

I'll see to also have the "Suggestion:" for non-polymorphic procs for now too.

And maybe I'll find a not-too-bad generic way for the proc-group, but that looks like something that someone with more C++ would have an easier time. We'll see if I can get some time later for that. :)

Thanks for the comment/review 💯

duchainer added a commit to duchainer/Odin that referenced this pull request Oct 16, 2025
Like other PRS:
 - odin-lang#5817
 - odin-lang#5818

But for the non-proc-group, non-polymorphic proc-calls.
@gingerBill
Copy link
Member

This is a bodge and I will not accept it. Please don't hardcode things like this.

@gingerBill gingerBill closed this Oct 23, 2025
@duchainer
Copy link
Author

duchainer commented Nov 1, 2025

This is a bodge and I will not accept it. Please don't hardcode things like this.

Gotcha, sorry that this PR is not good. I'll do better.

Would #5818 make sense, for non-proc-group, but polymorphic procedures?
Or would it still be a bodge because we only check for the the first argument being a pointer, and we should cover cases of any parameters count?

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