[Compiler] remove locations arround atomic expressions

[remyhaemmerle-da]

Those are useless for the run time.
Remove them reduce the dar size by 8% (average calculated using all the dar files generated in the repo)

[samuel-williams-da]

We can remove it, but if we ever want to improve call stacks in the engine, or even add a debugger, we'll need this back

[remyhaemmerle-da]

but if we ever want to improve call stacks in the engine, or even add a debugger, we'll need this back

For the call stack those are useless, because those expressions do not increase the stack (it is actually what I mean by useless for the runtime).

For the debugger, I am not sure they will be so usefull neither -- we could discuss about that.
On the other hand I think it is worth dropping them for a 8% gain. We could put it back once we have a debugger and we see they are actually usefull.

[samuel-williams-da]

Would they not point to the exact value that throws an error? i.e. the "error" builtin?

[samuel-williams-da]

They are essentially the bottom of the callstack, no?

[remyhaemmerle-da]

Would they not point to the exact value that throws an error? i.e. the "error" builtin?

No. You will get the location around the Application.

e.g.
If you have ELoc(ETyApp t (ELoc (EApp (ELoc (EBuiltin Error) l1) msg) l2) l3), the stack trace will use the location l2.
If you have ELoc(ETyApp t (EApp (ELoc (EBuiltin Error) l1) msg) l3), the stack trace will use the location l3.
If you have ETyApp t (EApp (ELoc (EBuiltin Error) l1) msg), and not other location anywhere else the stack trace will use "unknown"

[samuel-williams-da]

Right okay, and theres no cases where whatever is wrapping the atomic has a location that isn't good enough. Happy to merge

[roger-bosman-da]

Should we not have a unit test that asserts this property, and therefore signals regressions?

[remyhaemmerle-da]

Should we not have a unit test that asserts this property, and therefore signals regressions?

Fair enough.