Better errors for literal ranges in PlutusTx (PLT-8174) #5619

kwxm · 2023-11-09T11:42:31Z

[See also issues #5589 and #3395].

Suppose you have this code in a Haskell program:

   let !_ = $$(compile [|| [1..99] :: [Integer] ||])

The plugin currently gives you a 186-line error message like this:

GHC Core to PLC plugin: Error: Reference to a name which is not a local, a builtin, or an external INLINABLE function: Variable GHC.Num.Integer.integerGt#
                               No unfolding
Context: Compiling expr: GHC.Num.Integer.integerGt#
Context: Compiling expr: GHC.Num.Integer.integerGt# x1
Context: Compiling expr: GHC.Num.Integer.integerGt# x1 y1
Context: Compiling expr: case GHC.Num.Integer.integerGt# x1 y1 of {
                           __DEFAULT -> c x1 (go3 (GHC.Num.Integer.integerAdd x1 1));
                           1# -> n
...
<lots of nested context information>
...
Context: Compiling definition of: GHC.Enum.$fEnumInteger_$cenumFromTo
Context: Compiling expr: GHC.Enum.$fEnumInteger_$cenumFromTo
Context: Compiling expr: GHC.Enum.$fEnumInteger_$cenumFromTo 1
Context: Compiling expr: GHC.Enum.$fEnumInteger_$cenumFromTo 1 99
Context: Compiling expr at "main:Main:(26,15)-(26,52)"

This happens because the range is desugared to a call to enumFromTo in the Enum class, but things are complicated by the fact that in the case of Integer there is extensive inlining and rewriting.

This PR improves things slightly by spotting uses of enumFromTo and related functions and suggesting that the user should use PlutusTx.enumFromTo instead. I've also added PlutusTx.enumFromThenTo for ranges like [1, 5..101] with non-unit steps.

With this change the above error message is replaced by

GHC Core to PLC plugin: Error: Unsupported feature: Literal ranges are not supported: please use PlutusTx.Enum.enumFromTo or PlutusTx.Enum.enumFromThenTo
Context: Compiling expr: GHC.Enum.$fEnumInteger_$cenumFromTo
Context: Compiling expr: GHC.Enum.$fEnumInteger_$cenumFromTo 1
Context: Compiling expr: GHC.Enum.$fEnumInteger_$cenumFromTo 1 99
Context: Compiling expr at "main:Main:(26,15)-(26,52)"

This is an improvement, but is still not ideal: if the range is nested inside multiple functions then you'll still get lots of nested contexts with the final line (which is the only one containing a source location) pointing to the splice, which may be a long way from where the range occurs. I think we should merge this for now and think about ways to improve the situation.

Possible improvements

I thought about getting the plugin to replace occurrences of enumFromTo with PlutusTx.enumFromTo. This is tricky for a number of reasons:

Because of the inlining/rewriting for Integer some other function may be called and it's difficult to work out what's going on.
We'd need a GHC Name for PlutusTx.enumFromTo: those are quite complicated (they include unique IDs, for example) and I'm not sure how to get hold of one.
Maybe it's just not a good idea to silently replace a standard Haskell function with our own version.

I looked at trying to improve the context information as well, but it looked as if that'd take quite a lot of work.

The plugin provides a PlutusTx.Plugin:context-level option which looks as if it should let you change the amount of context information, but I couldn't get it to work. I think it may be getting overridden by calls to traceCompilation like this.
Is there any way to get accurate location information for stuff that's reachable transitively from the splice? You can kind of see what might be going on, but there can be huge amounts of GHC Core to work through. See issues Compilation errors report do not point to the location #5589 and Use of ranges in Plutus #3395 for complaints about this.

Delete accidentally-added file

…tput-hk/plutus into kwxm/plugin/better-range-errors

kwxm · 2023-11-09T11:44:02Z

plutus-tx-plugin/test/TH/Spec.hs

I didn't change anything in this file but appear to have reformatted it accidentally. I don't think it'll do any harm.

kwxm · 2023-11-09T11:45:10Z

plutus-tx/src/PlutusTx/Enum.hs

-  enumFromTo x y
-    | x > y = []
-    | otherwise = x : enumFromTo (succ x) y
+  enumFromTo x lim


I changed y to lim so that it looks more like what's in GHC.Enum. That changed a lot of golden files too.

kwxm · 2023-11-09T11:45:46Z

plutus-tx-plugin/test/Budget/9.2/show.pir.golden

@@ -200,12 +200,12 @@ let
 in
 letrec
  !`$fEnumBool_$cenumFromTo` : integer -> integer -> List integer
-    = \(x : integer) (y : integer) ->
+    = \(x : integer) (lim : integer) ->


See later for an explanation of why all of these have changed.

kwxm · 2023-11-09T11:54:48Z

plutus-tx-plugin/plutus-tx-plugin.cabal

@@ -115,7 +115,7 @@ executable gen-plugin-opts-doc

  default-language: Haskell2010

-test-suite plutus-tx-tests
+test-suite plutus-tx-plugin-tests


I changed this because plutus-tx contains plutus-tx-test and I was getting very confused.

kwxm · 2023-11-09T13:01:20Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+    | Just m <- GHC.nameModule_maybe n
+    , GHC.moduleNameString (GHC.moduleName m) == "GHC.Enum" =
+        ("$fEnum" `isPrefixOf` methodName &&
+         (  "_$cenumFromTo"     `isSuffixOf` methodName  -- [1..100]


Is there any better way of getting names of particular instances of methods like enumFromTo? This was the best I could do and I'm not sure how robust it will be. I suppose that if the names that GHC produces change then the tests will start to fail.

Oh, this is too bad. I hadn't realized that GHC might specialize enumFromTo.

The answer to your question is, unfortunately, no. There's no way to prevent GHC from specializing a method. It is only possible to disable user written rules, not builtin rules.

I'm not sure if we should match on the specialized name. I don't know if we can guarantee that that isn't an arbitrarily messed up version 🤔

We pass some information about GHC names through to the compiler in NameInfo in the CompileContext. Then you can use getThing to get the TypedThing and getName on that to get the Name. See how the stuff in Builitns works.

We pass some information about GHC names through to the compiler in NameInfo in the CompileContext. Then you can use getThing to get the TypedThing and getName on that to get the Name. See how the stuff in Builitns works.

Maybe we can come back to this in a later PR. I've run out of JIRA points on this.

I'm not sure if we should match on the specialized name. I don't know if we can guarantee that that isn't an arbitrarily messed up version 🤔

That's what I was asking about in my original comment. Can we actually get at the original name, especially after the inlining/rewriting? Is that what NameInfo is about?

No, NameInfo is just for getting an actual GHC Name instead of string matching. I'm not sure it will match up if it has in fact been specialized. I'm not sure what we do in other cases, perhaps we just fail to spot stuff 🤔

I guess it's not too bad since we're failing instead of silently compiling differently.

I guess since this is inherently heuristic rather than matching a precise name there's no point doing the NameInfo stuff and this is fine.

kwxm · 2023-11-09T16:55:23Z

The checkScriptContextEquality test in plutus-benchmark is failing because its CPU budget has increased slightly. I'm finding it difficult to imagine why this might be happening.

Edit: the budget increase seems to be due to the addition of enumFromThenTo to Plutus.tx.Enum. Maybe I should remove that again since it probably won't be used very often.

Edit: I've updated the test results, but we could still remove enumFromThenTo.

kwxm · 2023-11-10T12:03:34Z

plutus-ledger-api/test-plugin/Spec/Value.hs

@@ -2,7 +2,6 @@
 {-# LANGUAGE OverloadedStrings #-}
 {-# LANGUAGE TemplateHaskell   #-}

-{-# OPTIONS_GHC -fplugin PlutusTx.Plugin #-}


Nothing to do with this PR: I'm just taking the opportunity to remove it while I'm here.

Nope, I'm going to revert that. It may be causing problems with at least one GHC version.

zliu41

Looks good.

zliu41 · 2023-11-17T03:40:34Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+    | Just m <- GHC.nameModule_maybe n
+    , GHC.moduleNameString (GHC.moduleName m) == "GHC.Enum" =
+        ("$fEnum" `isPrefixOf` methodName &&
+         (  "_$cenumFromTo"     `isSuffixOf` methodName  -- [1..100]


Oh, this is too bad. I hadn't realized that GHC might specialize enumFromTo.

The answer to your question is, unfortunately, no. There's no way to prevent GHC from specializing a method. It is only possible to disable user written rules, not builtin rules.

zliu41 · 2023-11-17T03:47:49Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+      -- by looking for occurrences of GHC.Enum.enumFromTo and similar functions; the same error
+      -- occurs if these functions are used explicitly.
+      | isProbablyBoundedRange n ->
+          throwPlain $ UnsupportedError "Literal ranges are not supported: please use PlutusTx.Enum.enumFromTo or PlutusTx.Enum.enumFromThenTo"


We don't know for sure that the user used list range syntax. You can instead say something like: "Unsupported feature: GHC.Enum.Enum. You probably used list range syntax, which is not currently supported. You can use PlutusTx.Enum.Enum instead".

We don't know for sure that the user used list range syntax.

Yes, I did worry about that but decided to go for the shorter version to avoid a giant string in the source code. I'll change it.

zliu41 · 2023-11-17T03:49:56Z

plutus-tx-plugin/test/Plugin/Errors/Spec.hs

+
+-- Tests for literal ranges (and the corresponding methods in GHC.Enum). These
+-- should all fail with informative error messages.
+rangeEnumFromTo :: CompiledCode [Integer]


Nice tests.

michaelpj · 2023-11-17T10:50:12Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+    | Just m <- GHC.nameModule_maybe n
+    , GHC.moduleNameString (GHC.moduleName m) == "GHC.Enum" =
+        ("$fEnum" `isPrefixOf` methodName &&
+         (  "_$cenumFromTo"     `isSuffixOf` methodName  -- [1..100]


I'm not sure if we should match on the specialized name. I don't know if we can guarantee that that isn't an arbitrarily messed up version 🤔

michaelpj · 2023-11-17T10:52:02Z

plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs

+    | Just m <- GHC.nameModule_maybe n
+    , GHC.moduleNameString (GHC.moduleName m) == "GHC.Enum" =
+        ("$fEnum" `isPrefixOf` methodName &&
+         (  "_$cenumFromTo"     `isSuffixOf` methodName  -- [1..100]


We pass some information about GHC names through to the compiler in NameInfo in the CompileContext. Then you can use getThing to get the TypedThing and getName on that to get the Name. See how the stuff in Builitns works.

michaelpj · 2023-11-17T10:53:04Z

plutus-tx/src/PlutusTx/Enum.hs

+  -- | Construct a list from the given range (corresponds to [a,b..c]).  This
+  -- has the same semantics as the Haskell version,so if a==b and c>=b then you
+  -- get an infinite list, which you probably don't want in Plutus Core.
+  enumFromThenTo :: a -> a -> a -> [a]


I'm not sure if we should add this. It's not that useful and it makes the dictionaries bigger.

I'm not sure if we should add this. It's not that useful and it makes the dictionaries bigger.

Yeah, I wondered about that. I thought I might as well add it while I was looking at this, but it didn't initially occur to me that it would make the dictionary bigger.

kwxm · 2023-11-23T10:49:17Z

I think I'd like to merge this to get it out of the way. There are a number of things I'd like to revisit at a later date though: see the original PR comments.

bezirg · 2023-12-04T16:20:48Z

@kwxm @michaelpj Have you looked into using RebindableSyntax instead?

From https://downloads.haskell.org/ghc/latest/docs/users_guide/exts/rebindable_syntax.html :

So the [RebindableSyntax](https://downloads.haskell.org/ghc/latest/docs/users_guide/exts/rebindable_syntax.html#extension-RebindableSyntax) extension causes the following pieces of built-in syntax to refer to whatever is in scope, not the Prelude versions:

List notation, such as [x,y] or [m..n] can also be treated via rebindable syntax if you use -XOverloadedLists; see [Overloaded lists](https://downloads.haskell.org/ghc/latest/docs/users_guide/exts/overloaded_lists.html#overloaded-lists).

It should make the list notation work for PlutusTx, at least on paper.

kwxm added 25 commits November 2, 2023 08:23

WIP

cc2703e

Add PlutusTx.enumFromThenTo and tests

f46e80b

Remove range failure tests

983f6cd

Generalise range syntax check to non-Integer types

9daed91

Extend enumFromThenTo tests

62cc8fb

Extend enumFromThenTo tests

a4e3011

Extend enumFromThenTo tests

1af0087

Restore missing test

acaa303

Restore missing comment

094efa3

Cover rewrite rules for enum ranges

8b88ac9

Separate checks for finite and infinite ranges

b3f877d

Separate checks for finite and infinite ranges

ed1bb4d

Improve tests involving possibly infinite lists

50673bc

Golden tests for literal range errors

00f8844

Amend comment

8c17243

Really delete unwanted files

f6d1f1a

Update comment

33421ed

Delete plutus-tx-plugin/test/tmp/Main.hs

1188177

Delete accidentally-added file

Didn't want to commit those

a04f3d8

Merge branch 'kwxm/plugin/better-range-errors' of github.com:input-ou…

1d87725

…tput-hk/plutus into kwxm/plugin/better-range-errors

plutus-tx-tests -> plutus-tx-plugin-tests

3122346

Tidying up

7c2130b

Efficiency improvements

699ebff

Improve comment

beb3b52

Add changelog entries

2d41078

kwxm added the Plinth label Nov 9, 2023

kwxm requested review from michaelpj and zliu41 November 9, 2023 11:42

kwxm commented Nov 9, 2023

View reviewed changes

Merge branch 'master' into kwxm/plugin/better-range-errors

db2a12d

kwxm commented Nov 9, 2023

View reviewed changes

Remove redundant pragma

69d45d3

kwxm commented Nov 10, 2023

View reviewed changes

kwxm added 4 commits November 14, 2023 06:35

Merge branch 'master' into kwxm/plugin/better-range-errors

88c015d

Update budget test results

a86d5c8

Restore -fplugin pragma in plutus-ledger-api test

fa4ef71

Merge branch 'master' into kwxm/plugin/better-range-errors

e133bbe

zliu41 approved these changes Nov 17, 2023

View reviewed changes

kwxm added 2 commits November 17, 2023 10:23

Merge branch 'master' into kwxm/plugin/better-range-errors

fd46147

Improve error messages

01fb753

michaelpj reviewed Nov 17, 2023

View reviewed changes

kwxm added 4 commits November 17, 2023 10:56

Improve error messages

ef3e5b8

Update error messages in golden files

465ddad

Update error messages in golden files

33d3787

Merge branch 'master' into kwxm/plugin/better-range-errors

2cc18fc

kwxm enabled auto-merge (squash) November 23, 2023 10:49

kwxm merged commit b923816 into master Nov 23, 2023
6 checks passed

kwxm deleted the kwxm/plugin/better-range-errors branch November 23, 2023 13:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better errors for literal ranges in PlutusTx (PLT-8174) #5619

Better errors for literal ranges in PlutusTx (PLT-8174) #5619

kwxm commented Nov 9, 2023 •

edited

Loading

kwxm Nov 9, 2023

kwxm Nov 9, 2023

kwxm Nov 9, 2023

kwxm Nov 9, 2023

kwxm Nov 9, 2023

zliu41 Nov 17, 2023

michaelpj Nov 17, 2023

michaelpj Nov 17, 2023

kwxm Nov 17, 2023

kwxm Nov 17, 2023 •

edited

Loading

michaelpj Nov 17, 2023

michaelpj Nov 17, 2023

michaelpj Nov 17, 2023

kwxm commented Nov 9, 2023 •

edited

Loading

kwxm Nov 10, 2023

kwxm Nov 14, 2023

zliu41 left a comment

zliu41 Nov 17, 2023

zliu41 Nov 17, 2023

kwxm Nov 17, 2023

zliu41 Nov 17, 2023

michaelpj Nov 17, 2023

michaelpj Nov 17, 2023

michaelpj Nov 17, 2023

kwxm Nov 17, 2023

kwxm commented Nov 23, 2023

bezirg commented Dec 4, 2023 •

edited

Loading

Better errors for literal ranges in PlutusTx (PLT-8174) #5619

Better errors for literal ranges in PlutusTx (PLT-8174) #5619

Conversation

kwxm commented Nov 9, 2023 • edited Loading

Possible improvements

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwxm Nov 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwxm commented Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zliu41 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kwxm commented Nov 23, 2023

bezirg commented Dec 4, 2023 • edited Loading

kwxm commented Nov 9, 2023 •

edited

Loading

kwxm Nov 17, 2023 •

edited

Loading

kwxm commented Nov 9, 2023 •

edited

Loading

bezirg commented Dec 4, 2023 •

edited

Loading