daml icon indicating copy to clipboard operation
daml copied to clipboard

Improve error message for fields called `this`, `self` or `arg` in templates or template choices

Open akrmn opened this issue 2 years ago • 1 comments

Currently, declaring fields with these names results in poor error messages, for example,

template Foo
  with
    p : Party
  where
    signatory p

    choice FooC : ()
      with
        arg : Decimal
      controller p
      do
        return ()

fails with

  Foo.daml:41:18: error:
  • Conflicting definitions for ‘arg’
  Bound at: Foo.daml:41:18
  <no location info>
  • In a lambda abstraction

  Foo.daml:42:7: error:
  • Conflicting definitions for ‘arg’
  Bound at: Foo.daml:(42,7)-(43,17)
  <no location info>
  • In a lambda abstraction

Ideally, this would be caught by the daml-preprocessor, giving users an actionable error message.

Background

This was originally reported as https://github.com/digital-asset/daml/issues/3017, but the idea there was to allow such fields by tweaking the desugaring. That approach introduces a lot of complexity for little gain, so instead we just want to improve the error messages.

akrmn avatar May 06 '22 13:05 akrmn

Might be easier to forbid arg etc as a variable name in this context and propagate this as an error message.

basvangijzel-DA avatar Sep 07 '22 09:09 basvangijzel-DA

We'd need to specifically check choice/argument fields, and any body that can take this, self or arg as an argument. This sounds like it could get messy, I think a daml suggested warning would be better

samuel-williams-da avatar Mar 03 '23 09:03 samuel-williams-da

Alternatively, we can simply search template/choice arguments to look for these, and not generate the argument label in desugaring if it exists. Maybe alongside a warning for using this name

samuel-williams-da avatar Mar 03 '23 11:03 samuel-williams-da