v icon indicating copy to clipboard operation
v copied to clipboard

"or" handling from "Result" results in unexpected behavior.

Open brys0 opened this issue 2 years ago • 3 comments

Describe the bug

// main.v 

fn main() {
       // ...
	db := client.create_db('my_db') or {
		match err {
			types.AdministratorRequired {
				panic("admin is required")
			}
			types.InvalidDBName {
				panic("Database name not allowed")
			}
			types.DatabaseAlreadyExists {
				types.DB {                         // Should be returned (Line 56)
					name: "my_db"
				}
			}
			else {
				panic(err)
			}
		}
	}
}

Results in compiler error:

main.v:56:11: error: return type mismatch, it should be `void`
   54 |             }
   55 |             types.DatabaseAlreadyExists {
   56 |                 types.DB {
      |                       ~~~~
   57 |                     name: "my_db"
   58 |                 }

Well, this should be fine, lets just create some boilerplate after the match statement that returns a type.DB{} struct, to let the compiler infer the return type of or.

// main.v 

fn main() {
       // ...
	db := client.create_db('my_db') or {
		match err {
			types.AdministratorRequired {
				panic("admin is required")
			}
			types.InvalidDBName {
				panic("Database name not allowed")
			}
			types.DatabaseAlreadyExists {
				types.DB {                         // Should be returned (Line 56)
					name: "my_db"
				}
			}
			else {
				panic(err)
			}
		}
                types.DB {                                        // This shouldn't be seen and is to only let V infer the return type. This is returned and causes everything else to be ignored.
			name: "Should never see this"     
		}
	}
}

Unfortunately, it seems like everything in the block is ignored if something is returned at all in or

Console Output:

types.DB{
    name: 'Should never see this'
}
  • For the purposes of this report the implementation of create_db function is not important, but is provided below for completeness.
// client.v 

// create_db Performs a request to create a database on CouchDB
//
// Possible errors are: `types.InvalidDBName` `types.AdministratorRequired` `types.DatabaseAlreadyExists` `IError`
pub fn (client &Client) create_db(name string) !types.DB {
	response := http.fetch(client.gen_fetch_config("${client.host.str()}/$name", http.Method.put, ''))!
	return match response.status_code {
		201 {
			types.DB{name}
		}
		202 {
			types.DB{name}
		}
		400 {
			types.InvalidDBName{}
		}
		401 {
			types.AdministratorRequired{}
		}
		412 {
			types.DatabaseAlreadyExists{}
		}
		else {
			error(response.body)
		}
	}
}
// types/db.v

// ...

pub struct InvalidDBName {
	Error
}

pub fn (_ &InvalidDBName) msg() string {
	return 'An invalid name was provided'
}

// This error can be safely recovered from, and easily ignored
pub struct DatabaseAlreadyExists {
	Error
}

pub fn (_ &DatabaseAlreadyExists) msg() string {
	return 'A database with this name already exists'
}

Expected Behavior

Everything in the or block should be ran, or have the or block act infer its return type based on the expected result type of the function.

Currently the expected type of return for an or block is void.

main.v:56:11: error: return type mismatch, it should be `void`
   54 |             }
   55 |             types.DatabaseAlreadyExists {
   56 |                 types.DB {
      |                       ~~~~
   57 |                     name: "my_db"
   58 |                 }

Unless a return is given at the root level of or.

Current Behavior

Please see "Describe the bug"

Reproduction Steps

// main.v
module main

struct MySpecialError {
	Error
}

struct MyStruct {
pub:
	name string
}

fn (_ &MySpecialError) msg() string {
	return 'My special error'
}

fn do_a_thing(arg1 string, arg2 bool) !MyStruct {
	// ..
	if arg2 {
		return MySpecialError{}
	}
	return MyStruct{
		name: arg1
	}
}

fn main() {
	data := do_a_thing('my_db', true) or {
		match err {
			MySpecialError {
				MyStruct{
					name: 'my_db'
				}
			}
			else {
				panic(err)
			}
		}
		MyStruct{
			name: "shouldn't get this"
		}
	}
	println(data)
}

Possible Solution

// main.v

fn main() {
       // ...

       // We know the return type of client.create_db is !types.DB, this should automatically be inferred by the `or` block
	db := client.create_db('my_db') or { // Expect return type of types.DB, or [noreturn] function
		match err {
			types.AdministratorRequired {
				panic("admin is required")
			}
			types.InvalidDBName {
				panic("Database name not allowed")
			}
			types.DatabaseAlreadyExists {
				types.DB {                         // This is returned
					name: "my_db"
				}
			}
			else {
				panic(err)
			}
		}
	}
}

Additional Information/Context

No response

V version

V 0.3.3 c7237b1

Environment details (OS name and version, etc.)

OS Name Microsoft Windows 11 Pro Version 10.0.22621 Build 22621

brys0 avatar Apr 02 '23 05:04 brys0

Either I am not understanding everything correctly or you have the wrong idea what your or block is doing. Nothing is being "returned" on Line 56 but instead you are giving a new struct a value and later you are assigning a new value to it still inside that or block. Inside the or block it looks like you are meeting the criteria for it's rules: `or` block must provide a default value of type `MyStruct`, or return/continue/break or call a [noreturn] function like panic(err) or exit(1). So to me it looks like you are providing a default value and then overwriting that value.

Are we missing something here?

SolarWolf-Code avatar Apr 02 '23 05:04 SolarWolf-Code

Sorry I'm on phone so this response might be incorrect.

The or block doesn't require you to declare return for something to be returned. It just simply doesn't work at all. Even with using return it wants type of void. Using return in the root of the or block doesn't work either, but not using return does work.

We can tell stuff is being returned because of the compiler error given at the top. It doesn't expect types.DB it expects return type of "void", this remains consistent whether we use return types.DB{} or not.

brys0 avatar Apr 02 '23 05:04 brys0

Hmm... Testing with the "Reproduction Steps" example if I created a new error such as MySpecialError2 and added that to the match statement it comes down to the order of the errors.

match err {
			MySpecialError2 {
				panic('This is a problem')
			}
			MySpecialError {
				MyStruct{
					name: 'my_db'
				}
			}
			else {
				panic(err)
			}
		}

This returns ```error: return type mismatch, it should be `void````

Although if we instead have MySpecialError comes first we get a panic V panic: This is a problem

So to me this looks the inferred type has to come first in the match statement.

SolarWolf-Code avatar Apr 02 '23 06:04 SolarWolf-Code