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

v.pref: error for v file.v --unknown-option #21391

Merged
merged 8 commits into from May 18, 2024

Conversation

JoeyShapiro
Copy link
Contributor

Describe the Problem

Compiling a single code file with flags does not give a warning, and continues the compile.
This will work on real or fake flags
This does not look nice and could cause unknown usage.

$ ./v examples/hello_world.v -autofree
$ ./v examples/hello_world.v -garbage
$ echo $?
0

Show the Change

This change reports an error and exits as such. It will report the same error if the argument is valid or not, but it is clear what the problem is.

$ ./v examples/hello_world.v -autofree
Build flags must be before the source file.
$ ./v examples/hello_world.v -garbage
Build flags must be before the source file.
$ echo $?
1

@spytheman
Copy link
Member

I agree that not reporting an error for ./v examples/hello_world.v -garbage is a problem.
Note however that recognized options do work after the source file currently, and will not after your change, which is a breaking change:

#0 09:47:17 ᛋ master /v/oo❱./v examples/hello_world.v -gc none
#0 09:49:15 ᛋ master /v/oo❱ls -l examples/hello_world
-rwxrwxr-x 1 delian delian 502792 May  1 09:49 examples/hello_world
#0 09:49:20 ᛋ master /v/oo❱./v examples/hello_world.v
#0 09:49:23 ᛋ master /v/oo❱ls -l examples/hello_world
-rwxrwxr-x 1 delian delian 699344 May  1 09:49 examples/hello_world

@spytheman
Copy link
Member

Testing on your branch, shows that it behaves differently than your demonstration:

#0 09:54:22 ᛋ master /v/oo❱
#0 09:54:22 ᛋ master /v/oo❱git switch -
Switched to branch 'v-file-flags'
#0 09:54:23 ᛋ v-file-flags /v/oo❱./v self
V self compiling ...
V built successfully as executable "v".
#0 09:54:27 ᛋ v-file-flags /v/oo❱./v examples/hello_world.v -autofree
#0 09:54:30 ᛋ v-file-flags /v/oo❱./v examples/hello_world.v -garbage
Build flags must be before the source file.
#1 09:54:34 ᛋ v-file-flags /v/oo❱echo $?
1
#0 09:54:38 ᛋ v-file-flags /v/oo❱

... which is nice.

@JoeyShapiro
Copy link
Contributor Author

wow a response, and yeah, it is acting different, my bad. ok give me a bit to work on this, I might have to retitle this issue.

@JalonSolov
Copy link
Contributor

Personally, I would like for it to warn about options after the filename. This is one way v build is different from v run. In v run, all options after the filename are passed to the target program after it is built. In v build, they're just treated as options to the build.

It can be a bit confusing to new users as to why it is ok to have v file.v -gc none but v run file.v -gc none doesn't work the same.

@JoeyShapiro
Copy link
Contributor Author

yes I agree. that is why I want this worked out. I realized what I was doing. doing a double dash --skip-unused is not the same as -skip-unused. This is why in my testing it appeared like they were not working.
So let me reformat my statement.

I want to make some statements, and we can discuss, before I start making any changes
If an option is valid, on either side, it works as intended. however, an invalid option on the right side will not be caught.
I think this examples rounds out all the problems

$ ./v -o hello-skip.c -skip-unused examples/hello_world.v
$ ./v -o hello-maybe-skip.c examples/hello_world.v -skip-unused
$ ./v -o hello-wont-skip.c examples/hello_world.v --skip-unused
$ ./v -o nothing.c --skip-unused examples/hello_world.v
Unknown argument `--skip-unused`

$ diff -q hello-skip.c hello-maybe-skip.c
$ diff -q hello-skip.c hello-wont-skip.c 
Files hello-skip.c and hello-wont-skip.c differ

@spytheman
Copy link
Member

(rebased over master and resolved conflicts)

@spytheman spytheman changed the title pref: Give Warnings When Compiling Sources with Flags v.pref: error for v file.v --unknown-option May 18, 2024
@spytheman
Copy link
Member

(rebased again, since master had an unrelated error, that is now fixed)

@spytheman spytheman merged commit c8bf38c into vlang:master May 18, 2024
63 checks passed
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

3 participants