fix!: unify comptime expression/statement with target type

[asterite]

Description

Problem

Resolves #8369
Resolves #10624

Summary

This is just an experimental PR but I want to see if existing code breaks.

The idea here is that if a comptime statement or expression is in a position where there's an explicit type (for example a let statement with an explicit type, or a return type, or a comptime block given to a function argument) then we unify that comptime statement/expression with that target type.

Before this PR this didn't compile:

fn main() {
    let x: u8 = comptime { 1 };
}

The reason was that "1" was typed as Field because the "u8" information was inaccessible to the block. Now we unify that "1" with "u8" so it ends up being "u8".

This PR is a breaking change because previously this was valid:

fn main() {
  let _: str<5> = comptime { f"Hello" };
}

The reason it worked is that format strings were lowered to string literals. Well, that still happens, but now it fails to compile because a fmtstr doesn't unify with a str. We can fix this in two ways. The first is not using a type annotation, in which case the type will be inferred, and because a fmtstr lowers into a str, it works:

fn main() {
  let _ = comptime { f"Hello" };
}

The second way to fix this is using as_quoted_str:

fn main() {
  let _: str<5> = comptime { f"Hello".as_quoted_str!() };
}

The docs for "as_quoted_str" say:

Returns the format string contents (with the leading and trailing double quotes) as a Quoted string literal (not a format string literal).

So here we are effectively transforming that format string into a string literal, whose type is str<5>, and now everything compiles.

Given that inferring types of let is more common than explicitly specifying their types, in the vast majority of cases this should work out of the box (external repos checked here all work, so it's likely not common to return format strings from a comptime context).

The only odd thing now is that the comptime block is type-checked in one way or another depending on an explicit type. But maybe that's fine: in the example above, and before this PR, to fix the above we could change "1" to "1_u8" so we have to specify the type somewhere if we want a different type.

Additional Context

1. This PR uncovered a bug in the aes128_encrypt implementation in the comptime interpreter: it was returning a slice, but it should return an array. It worked because lowering a slice to an array seems to work (not sure why the error happened just now and not before, though).
2. If we go with this, we should probably update the lowering docs to explain how comptime blocks are type-checked.
3. I feel Cannot print global fmtstr #6990 is related to this issue/PR (though it's not solved here) as globals are comptime values that are lowered, so the code in Cannot print global fmtstr #6990 should give an error because the final type of the global is str, not fmtstr (to fix it one would need to use as_quoted_str!())

User Documentation

Check one:

• No user documentation needed.
• Changes in docs/ included in this PR.
• [For Experimental Features] Changes in docs/ to be submitted in a separate PR.

PR Checklist

• I have tested the changes locally.
• I have formatted the changes with Prettier and/or cargo fmt on default settings.

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 91a4f9c	Previous: a9401fe	Ratio
test_report_zkpassport_noir_rsa_	2 s	1 s	2

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 8bbc72e	Previous: a550737	Ratio
rollup-block-root-single-tx	0.003 s	0.002 s	1.50

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 5b5f93e	Previous: 6a42a54	Ratio
sha512-100-bytes	2.001 s	1.616 s	1.24

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[jfecher]

Looks good and makes sense to me.

One thing I am concerned with (preventing me from approving) is lowering format strings into regular strings. If they keep their old format string type but are lowered as a normal string literal, does this mean our runtime can get confused if it later tries to format these strings by looking at their types? They would no longer have an environment to format.

[asterite]

One thing I am concerned with (preventing me from approving) is lowering format strings into regular strings. If they keep their old format string type but are lowered as a normal string literal, does this mean our runtime can get confused if it later tries to format these strings by looking at their types? They would no longer have an environment to format.

Good point. I wonder if it could still be lowered to a format string... 🤔

Let's say we have this:

fn main() {
    let f = comptime {
        let x = 1;
        f"x is {x}"
    };
}

Right now f"x is {x}" is immediately evaluated to "x is 1". But maybe at comptime we could still keep the fragments, except that now fragments are either strings or values, so in this case we'd have [String("x is "), Value(1)]. Then when we lower it, in into_expression, we create this expression:

{
  let fragment1: Field = 1;
  f"x is {fragment1}"
}

That is, we create a block expression with one temporary value for each Value fragment.

Then, I think, all comptime values lower to their type.

You'd have to call as_quoted_str!() to get a str, but maybe that's fine, or at least less unexpected than the current situation.

[asterite]

@jfecher I tried the above in #11002

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'ACVM Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 8bbc72e	Previous: a550737	Ratio
perfectly_parallel_batch_inversion_opcodes	2792949 ns/iter (± 13919)	2260554 ns/iter (± 2648)	1.24

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench

[asterite]

This is ready for review again

[asterite]

@jfecher Actually, I noticed one "inconsistency" remains: CtString is still lowered to str. For example this compiles and works fine:

use std::append::Append;

fn main() {
    let s: str<_> = comptime {
        let ct_string = CtString::new().append("Hello");
        ct_string
    };
    println(s);
}

Should we keep that, or should we lower CtString to something of type CtString (it's doable). Maybe with CtString this is less of an issue because CtString only works in comptime, so the type can't escape the comptime block and, for example, you can't use it in a global.

[jfecher]

Sorry for missing this.

Can you create an issue for lowering CtString into CtString - and ensuring there's still a way users can lower it into a regular string if they want?

[github-actions]

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Brillig Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: 91a4f9c	Previous: a9401fe	Ratio
rollup-block-root-single-tx	0.004 s	0.003 s	1.33

This comment was automatically generated by workflow using github-action-benchmark.

CC: @TomAFrench