aeternity / aesophia

Stand alone compiler for the Sophia smart contract language
https://docs.aeternity.com/aesophia
ISC License
51 stars 19 forks source link

Incorrect warning: nonexistent __x variable is shadowed #476

Open brainiacfive opened 1 year ago

brainiacfive commented 1 year ago

I get a compiler warning

The definition of `__x` shadows an older definition at (builtin location).

I am not using __x in my source. Tried to eliminate the String include from it, only Option left, but it throws the same warning; so it would not be include conflict. No other includes.

Looks like a compiler issue to me. Will provided a minimal source example later. Maybe you have an idea already from this stub notice.

brainiacfive commented 1 year ago
(builtin location)

is verbatim the warning, not a placeholder I inserted.

marc0olo commented 1 year ago

@brainiacfive this is happening in a (currently) private contract, right?

if you can't provide a minimal example right now you could maybe invite @ghallak and/or @radrow to the repo to check with the contract where the compiler currently produces this warning

hanssv commented 1 year ago

I think the compiler uses the variable __x when it desugars some of the Map-syntax - could be something 'interesting' going on there.

hanssv commented 1 year ago
contract C =
  record r1 = { f : bool, g : bool }
  record state = { m : map(int, r1) }

  let default1 = { f = false, g = false }

  entrypoint init() = {m = {}}

  stateful entrypoint f(the1 : int) =
    put(state{ m[the1 = default1].f = true })

is a small example capturing the same problem - the compiled code is correct in this case, but that might be luck (cc @radrow and @ghallak )

brainiacfive commented 1 year ago

Much appreciated. I saw more warnings using __x meanwhile; in folds.

Do you know which variable in your example is the first use of __x that would get shadowed by which other variable that is the second use? I'd be checking for patterns of that in the contract source to make sure it is harmless.

radrow commented 1 year ago

It's a bug caused by running code analysis on desugared code. Nothing to worry about. We'll take a look and fix it soon.

radrow commented 1 year ago

If you are worried though, or if it may impact something serious, you can use aesophia_cli with pp_asm to check if the function is compiled correctly. The assembler should be fairly readable.

brainiacfive commented 1 year ago

Thank you, will do!

brainiacfive commented 1 year ago

The asm code is fascinating but not something to crack on the fly. Can you @radrow @hanssv share what sugar is causing the use of __x so I can change the code to avoid it? I assume a nesting of scopes is a prerequisite for the effect to happen. So I should be able to eliminate the select places this could be happening. I'd rather have this evened out, thanks!

brainiacfive commented 1 year ago

Tried with replacing

put(state{ used[account = unused].abstraction = true })

with

let u = used[account = unused]{ abstraction  = true }
put(state{  used[account] = u })

On @hanssv recommendation, but so far no luck.

hanssv commented 1 year ago

You need to change all five of them, and indeed it does workaround the problem.

hanssv commented 1 year ago

I think the concrete issue has been worked around - as for the compiler, I think using __x everywhere might be a bit too simplistic 🙈 - and it should be fixed eventually.