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

checker: add fntype casting validations #23872

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

felipensp
Copy link
Member

Fix #23852

Copy link

Connected to Huly®: V_0.6-22276

@spytheman
Copy link
Member

none should be allowed for option values:

type Foo = fn (bar string)

fn test_main() {
    mut func := ?Foo(none)
    dump(func)
}

@spytheman
Copy link
Member

On master that passes, but it fails here currently (which is the CI's problem):
image

(as an aside, the error is not shown on the CI, because of the -silent flag that was passed to reduce the scrolling for the normally OK lines in the test)

@felipensp
Copy link
Member Author

On master that passes, but it fails here currently (which is the CI's problem): image

(as an aside, the error is not shown on the CI, because of the -silent flag that was passed to reduce the scrolling for the normally OK lines in the test)

Right, I'll add the option test too. 👍🏻

@medvednikov
Copy link
Member

Thanks!

@medvednikov
Copy link
Member

casting number to function value should be done inside unsafe{} blocks

This should not be allowed at all. We recently forced requiring nil instead of 0 for the same reason.

@medvednikov
Copy link
Member

We can do a warning for now (use nil instead of 0)

@spytheman
Copy link
Member

This should not be allowed at all. We recently forced requiring nil instead of 0 for the same reason.

No, it should be allowed in unsafe {}.

@medvednikov
Copy link
Member

Why?

@spytheman
Copy link
Member

Because there are cases (for example with -freestanding, you can write code that can then be booted on bare metal), when you write code, where you do know the actual address of a function (because it is an external requirement), and do want to cast it to a function pointer.

Restricting what can be done in unsafe{} blocks in general kind of defeats the whole purpose of the unsafe expressions.

@medvednikov
Copy link
Member

medvednikov commented Mar 8, 2025

Then just use Func(voidptr(1234))

It's more clear this way too. Func(1234) looks like a function call.

V positions itself as a safe language. You can't cast numbers to functions in a safe language :)

@spytheman
Copy link
Member

Then just use Func(voidptr(1234))

It's more clear this way too. Func(1234) looks like a function call.

V positions itself as a safe language. You can't cast numbers to functions in a safe language :)

We are talking then about:
unsafe { Func(voidptr(1234)) }

I think that is very verbose for no gain at all.

@spytheman
Copy link
Member

spytheman commented Mar 8, 2025

V positions itself as a safe language.

V itself does not do anything on its own. It is a language, not a person.

You, however position it as a language, that I will not want to use for certain tasks, because of the verbosity and edge cases.

Unsafe blocks are in V, to allow people to write unsafe code in them, not to make them jump through hoops to avoid weird quirks in the compiler.

@felipensp
Copy link
Member Author

I've added V UI PR fix vlang/ui#599

@felipensp felipensp marked this pull request as ready for review March 8, 2025 14:41
Copy link
Member

@spytheman spytheman left a comment

Choose a reason for hiding this comment

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

Excellent work @felipensp, thank you.

However, I will not merge it, since @medvednikov expressed another opinion, and he is ultimately the arbiter of what should happen with his language.

I do hope, that he will reconsider, but that is of course up to him.

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.

V should require unsafe for type castings of zero
3 participants