Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

rule that all structures with more than 8 fields are always passed as reference in methods should be removed #17077

Closed
2 tasks
i582 opened this issue Jan 22, 2023 · 1 comment · May be fixed by #17159
Closed
2 tasks
Assignees
Labels
Bug This tag is applied to issues which reports bugs. Feature Request This issue is made to request a feature. Unit: Compiler Bugs/feature requests, that are related to the V compiler in general. Weird/Undocumented Behaviour This issue is related to a bug, that was surprising and wasted user's time.

Comments

@i582
Copy link
Contributor

i582 commented Jan 22, 2023

Describe the feature

This rule sounds as strange as possible, and what is most terrible, it is IMPLICIT and NOT DESCRIBED in the documentation.

I don't see the point in this rule because the developer can decide YOURSELF when the structure should be passed as reference or not.
Moreover, adding a new field to the structure completely changes the receiver's pass logic, which can be dangerous.

The compiler SHOULD NOT show off one's "intelligence" and add IMPLICITIES.

Example:

https://play.vlang.io/?query=06bda73b49 (8 fields)

7ffd53de0f90
7ffd53de0f30

All ok, different pointers because the receiver doesn't have a pointer type

https://play.vlang.io/p/88dc100004 (9 fields)

7ffe6b15c1ac
7ffe6b15c1ac

Not ok, same pointers.

Compiler code:

if !rec.is_mut && !rec.typ.is_ptr() && info.fields.len > 8 {

Use Case

Proposed Solution

Remove this rule.

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

Version used

V 0.3.2 89aa695.865c0ea

Environment details (OS name and version, etc.)

OS: macos, macOS, 13.0.1, 22A400
Processor: 10 cpus, 64bit, little endian, Apple M1 Pro
CC version: Apple clang version 14.0.0 (clang-1400.0.29.202)

getwd: /Users/petrmakhnev/vlang-foundation/specification
vmodules: /Users/petrmakhnev/.vmodules
vroot: /Users/petrmakhnev/v
vexe: /Users/petrmakhnev/v/v
vexe mtime: 2023-01-22 21:48:01
is vroot writable: true
is vmodules writable: true
V full version: V 0.3.2 89aa695.865c0ea

Git version: git version 2.37.1 (Apple Git-137.1)
Git vroot status: weekly.2022.50-266-g865c0ea8-dirty
.git/config present: true
thirdparty/tcc status: thirdparty-macos-arm64 173c526e
@i582 i582 added the Feature Request This issue is made to request a feature. label Jan 22, 2023
@i582 i582 changed the title rule that all structures with more than 8 fields are always allocated on the heap should be removed rule that all structures with more than 8 fields are always passed as reference in methods should be removed Jan 22, 2023
@spytheman
Copy link
Member

I agree 💯 - that is an anti feature, or even a bug imho.

@spytheman spytheman self-assigned this Jan 23, 2023
@spytheman spytheman added Bug This tag is applied to issues which reports bugs. Weird/Undocumented Behaviour This issue is related to a bug, that was surprising and wasted user's time. Unit: Compiler Bugs/feature requests, that are related to the V compiler in general. labels Jan 23, 2023
@vlang vlang locked and limited conversation to collaborators Jun 27, 2023
@medvednikov medvednikov converted this issue into discussion #18635 Jun 27, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
Bug This tag is applied to issues which reports bugs. Feature Request This issue is made to request a feature. Unit: Compiler Bugs/feature requests, that are related to the V compiler in general. Weird/Undocumented Behaviour This issue is related to a bug, that was surprising and wasted user's time.
Projects
None yet
2 participants