cgen: prefix builtin methods with builtin__#25264
Conversation
|
Connected to Huly®: V_0.6-25002 |
|
A scheme discussed in the past would be to simply prefix builtin names with the module name, the same way every other module in V prefixes the names with the module name. For example, if you call So why not |
|
Yes this would be good. I don't actually have handling for stuff like fn main() {
println := 'a'
print(println)
}The methods that I currently handle are methods on a type such as |
|
Allowing keywords to be used as variable names is a relatively recent change in V. Not everyone agrees with it, but enough wanted it for it to be added. |
|
If we are going for prefixing |
|
|
|
The failing CI job about We can however add a separate API, something like: @[export: 'builtin__ctovstring']
pub fn ctovstring_impl(s &u8) string {
return tos2(s).clone()
}as 1 commit on master, then change I'll take care of it asap. edit: done in 69e80ba . |
|
Did a quick prefix change from Missing stuff so I don't forget later:
|
|
Huh that's weird, it's failing on clang and musl specifically? |
|
musl is just... weird, in general, compared to libc. |
dy-tea
force-pushed
the
master
branch
3 times, most recently
from
b1c6283 to
a359822
Compare
|
Seems to handle everything now? Will go to sleep now and check if the CI worked tomorrow. |
|
(rebased over current master (which solved the unrelated CI failure)) |
I agree |
|
Although it will increase the size of generated C code significantly |
The overhead is 1134 bytes for a small program like examples/hello_world.v (~3.8%). The compiler overhead is also a bit bigger than that for other larger programs, since it also includes the new compiler code, not just the increase from the longer method names. i.e. I expect the overhead for most code to be between 2.5% and 3.8% in terms for generated C code bytesize. |
|
I could not measure a significant performance drop, by running |
|
Running |
dy-tea
changed the title
|
Oh that's good that performance doesn't seem to be impacted. The only thing missing now is some check to prevent people from naming variables with the For example: fn main() {
builtin__string__eq := 'a' == 'b'
println(builtin__string__eq)
}Produces: void main__main(void) {
bool builtin__string__eq = builtin__string__eq(_S("a"), _S("b"));
builtin__println(builtin__string__eq ? _S("true") : _S("false"));
}Which will fail c compilation. Might want a seperate pr for this. Now that I'm thinking about it, this would apply to all modules. |
The new collision is much less likely to happen for arbitrary code, compared to before, and preventing it is very easy to arrange, by banning the |
collisions with them are already unlikely too |
|
Yeah something like: import time
fn main() {
time__now := time.now()
}Will also fail to compile. Will think about a way to approach this. |
|
Reasonably simple... if fn main() {
time__now := 5
}would still work fine, since there is no |
|
This is truly beautiful work. |
|
I think having the prefix match the module name is a good thing - you don't have to guess where it came from, it's right there in the name. |
5 participants
We would like to prevent user-defined variable names from being able to conflict with c functions generated by the compiler.
Most simple thing in most cases is prefixing with _ but we need to specifically check for builtin types in some places as we don't want to add this to user-defined functions which will then reintroduce the issue (maybe we want a separate prefix for user-defined variables as well so they don't conflict with c functions?).
I have some commented out lines in the test at the moment for array and map initialization. Wasn't sure how to handle this.
I handle
int_literalandfloat_literalseparately but there might be a more elegant way to do this?There are some places where I only prefix an underscore and was wondering if it's more efficient to use the string plus operator or string formatting, I currently mostly use string formatting but it can be changed.
This does not handle name collisions with included c functions like
printf. Maybe have a per-module c_reserved array with the included function names? Not sure about the best solution for this.Since this adds quite a few
is_builtin()checks it likely will have some performance impact. It didn't seem too bad but I didn't do any extensive tests.