Values declared inside conditional blocks and match arms leak to module export list

[ehllie]

When writing top level, non function definitions, any definition created inside the expression on the right hand side of the definition will leak into that module's export list, but also will not be available inside that module.

Let's first create a file A.an

A.an

foo =
  if true then
    bar = 4
    bar + 5
  else 0

bar = "Hello, I'm a string!"

baz = "${bar}\nI'm a string too!"

Output of ante -tc A.an

bar : (Int a)
baz : String
foo : (Int a)

The flags -tc tell the compiler to print out the types of exports in a given module. We can see the bar defined inside the then branch of the if block is leaking to the export list, rather than the bar we define later.
When we import the variables, the behaviour is a little confusing as well:

B.an

import A

cond_val =
  if true
  then bar
  else 0

type Foo a =
  v: Int a

some_foo = Some (Foo 4)

some_branch_fn () =
  match some_foo
  | Some (Foo v) ->
      does_not_leak = v * 24
      does_not_leak - 4
  | None -> 0

some_branch =
  match some_foo
  | Some (Foo matched_v) ->
      does_leak = matched_v * 24
      does_leak - 4
  | None -> 0

print <| cond_val
print <| baz

Output of ante -tc B.an

Foo : (forall a b. ((Int a) -> (Foo a) can b))
cond_val : I32
does_leak : (Int a)
matched_v : (Int a)
some_branch : (Int a)
some_branch_fn : (forall c b. (Unit -> (Int c) can b))
  given Mul (Int c), Sub (Int c)
some_foo : (Maybe (Foo a))

Output of running the program

4
Hello, I'm a string!
I'm a string too!

It's not just conditional expression branches that leak definitions, but match patterns too.
It's also possible to cause a panic with this issue, like so:

Extra lines in A.an

oops =
  match (None: Maybe I32)
  | Some never ->
    never_assigned = 4 + never
    never + never_assigned
  | None -> 0

Extra lines in B.an

print <| never_assigned

Result of running B.an

thread 'main' panicked at src/hir/monomorphisation.rs:851:17:
internal error: entered unreachable code: MatchPatterns should already be defined.
 Encountered while compiling never 580: I32
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

This behaviour is at the very least confusing. At least one of those things should be done to address it:

• If it's intended for arbitrary constant expressions to be allowed on the export list of modules, and all extra values defined when defining them to be exported as well, an error should be emitted when trying to redefine them in the same module.
• Create implicit scopes on the right hand side of top level definitions.
• Only allow top level definitions to be functions, and implicit execution of the main () function
• Require explicit definition exports Design import and publicity semantics #111

I am personally not a fan of the first option, since that still leaves plenty of issues open, namely the panic from this issue. I think doing a mix of the other options. I am also not a fan of having a module execute top to bottom, because I don't like having a module import potentially execute code. It makes more sense and is more clear to me to only have imported code execute when I use some function that it exports. It is nice to be able to define constant values, but I feel like they should have a limited amount of effects they can execute, and also not be leaking definitions from their right hand side expression. That would warrant a separate issue however.

Doing the 2nd thing should require the least bikeshedding, and also solve the immediate problem.

[jfecher]

Hmm, this is definitely an unintentional bug. Neither bar nor does_leak should be exported into global scope at all.

This also looks like a recurrence of a previous bug IIRC but I can't seem to find the issue on GitHub. The code for If and Match in name resolution both correctly pushes a new scope for each of their branches, and is skipped entirely for the declaration phase of that pass which collects globals.

https://github.com/jfecher/ante/blob/master/src/nameresolution/mod.rs#L1058-L1060

I am a fan of having a module execute top to bottom, because I don't like having a module import potentially execute code.

Just to clarify, do you mean to say you are not a fan of having the module execute top to bottom?
I don't like importing modules running code either. My original plan to resolve this was to have a dependency graph to still allow globals to be defined out of order while also allowing top level code to be run once at the start of the program. Any (non-function) dependency cycles would be an error.

[ehllie]

Just to clarify, do you mean to say you are not a fan of having the module execute top to bottom?

Yes, that's what I meant, just lost the not heh