Skip to content

Commit

Permalink
Merge pull request #4612 from Quviq/PR-remove-unfloating
Browse files Browse the repository at this point in the history
Compiler Coverage: don't do tick unfloating
  • Loading branch information
michaelpj committed May 13, 2022
2 parents 751e750 + 3952682 commit 7f4f497
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 140 deletions.
@@ -0,0 +1 @@
(fun (con bytestring) (fun (con bytestring) (fun (con bytestring) (con bool))))
30 changes: 3 additions & 27 deletions plutus-tx-plugin/src/PlutusTx/Compiler/Expr.hs
Expand Up @@ -607,27 +607,6 @@ entryExitTracing lamName displayName e ty =
guarantee that the annotation `a` is logged before we execute `body`.
-}

{- Note [Tick-unfloating]
GHC likes to float ticks on case scrutinees. This screws with boolean
coverage (because the result of the case expression is not necessarily
boolean typed) so we un-float these ticks. Specifically, GHC will convert an
expression like:
```
if <tick with source location of condition> condition
then trueCase
else falseCase
```
into something equivalent to:
```
<tick with source location of condition>
if condition
then trueCase
else falseCase
```
which means that we don't get boolean coverage for `condition` as there is no tick around it (so we
don't know where it is).
-}

{- Note [Boolean coverage]
During testing it is useful (sometimes even critical) to know which boolean
expressions have evaluated to true and false respectively. To track this we
Expand Down Expand Up @@ -838,9 +817,6 @@ compileExpr e = withContextM 2 (sdToTxt $ "Compiling expr:" GHC.<+> GHC.ppr e) $
let binds = pure $ PIR.TermBind () PIR.NonStrict v scrutinee'
pure $ PIR.Let () PIR.NonRec binds mainCase

-- See Note [Tick-unfloating]
GHC.Tick tick (GHC.Case scrutinee b t alts) -> compileExpr (GHC.Case (GHC.Tick tick scrutinee) b t alts)

-- we can use source notes to get a better context for the inner expression
-- these are put in when you compile with -g
-- See Note [What source locations to cover]
Expand All @@ -865,9 +841,9 @@ compileExpr e = withContextM 2 (sdToTxt $ "Compiling expr:" GHC.<+> GHC.ppr e) $
included as a coverage annotation. This has both advantages and disadvantages.
On the one hand "trying as hard as we can" gives us as much coverage information as
possible. On the other hand GHC can sometimes do tricky things like tick floating
(see Note [Tick-unfloating]) that will degrade the quality of the coverage information
we get. However, we have yet to find any evidence that GHC treats different ticks
differently with regards to tick floating.
that will degrade the quality of the coverage information we get. However, we have
yet to find any evidence that GHC treats different ticks differently with regards
to tick floating.
-}

{- Note [Partial type signature for getSourceSpan]
Expand Down
2 changes: 1 addition & 1 deletion plutus-tx-plugin/test/Plugin/Coverage/Spec.hs
Expand Up @@ -53,7 +53,7 @@ coverage = testNested "Coverage"
[ pure $ testGroup "Application heads and line coverage"
[ mkTests "noBool" noBool Set.empty [28]
, mkTests "boolTrueFalse" boolTrueFalse (Set.singleton "&&") [31]
, mkTests "boolOtherFunction" boolOtherFunction (Set.fromList ["&&", "==", "otherFun"]) [34, 38, 39, 40]
, mkTests "boolOtherFunction" boolOtherFunction (Set.fromList ["&&", "=="]) [34, 38, 39, 40]
, mkTests "boolOtherFunctionSimplifiesAway" boolOtherFunctionSimplifiesAway (Set.fromList ["&&", "=="]) [46]
, mkTests "boolQualifiedDisappears" boolQualifiedDisappears Set.empty [49]
]
Expand Down
212 changes: 100 additions & 112 deletions plutus-tx-plugin/test/Plugin/Coverage/coverageCode.plc.golden
Expand Up @@ -276,148 +276,136 @@
{
[
[
{ (builtin trace) (all dead (type) [ Maybe Bool ]) }
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 38, _covLocEndLine = 40, _covLocStartCol = 1, _covLocEndCol = 33})"
)
]
(abs
dead
(type)
{
[
{ Maybe_match (con integer) }
{
[
[
{
(builtin trace)
(all dead (type) [ Maybe (con integer) ])
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 38, _covLocEndLine = 40, _covLocStartCol = 1, _covLocEndCol = 33})"
)
]
(abs dead (type) x)
]
(all dead (type) dead)
}
]
(all dead (type) [ Maybe Bool ])
}
(lam
y
(con integer)
(abs
dead
(type)
{
[
[
[
{
[ { Maybe_match (con integer) } x ]
(all dead (type) [ Maybe Bool ])
}
(lam
y
(con integer)
(abs
dead
(type)
{
[
Bool_match
[
[
[
traceBool
(con
string
"CoverBool (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 12, _covLocEndCol = 22}) True"
)
]
(con
string
"CoverBool (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 12, _covLocEndCol = 22}) False"
)
]
{
(builtin trace)
(all dead (type) [ Maybe Bool ])
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 12, _covLocEndCol = 22})"
)
]
(abs
dead
(type)
{
[
[
{
(builtin trace) (all dead (type) Bool)
[
Bool_match
[
otherFun
{
[
[
{
(builtin trace)
(all
dead
(type)
(con integer)
)
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 21, _covLocEndCol = 22})"
)
]
(abs dead (type) y)
]
(all dead (type) dead)
}
]
]
(all dead (type) [ Maybe Bool ])
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 12, _covLocEndCol = 22})"
)
]
(abs
dead
(type)
[
otherFun
(abs
dead
(type)
{
[
[
{
(builtin trace)
(all
dead (type) (con integer)
dead (type) [ Maybe Bool ]
)
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 21, _covLocEndCol = 22})"
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 26, _covLocEndCol = 36})"
)
]
(abs dead (type) y)
(abs
dead
(type)
[
{ Just Bool }
{
[
[
{
(builtin trace)
(all dead (type) Bool)
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 31, _covLocEndCol = 36})"
)
]
(abs dead (type) False)
]
(all dead (type) dead)
}
]
)
]
(all dead (type) dead)
}
]
)
)
]
(abs dead (type) [ fail (con unit ()) ])
]
(all dead (type) dead)
}
]
)
]
(all dead (type) [ Maybe Bool ])
(all dead (type) dead)
}
(abs
dead
(type)
{
[
[
{
(builtin trace)
(all dead (type) [ Maybe Bool ])
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 26, _covLocEndCol = 36})"
)
]
(abs
dead
(type)
[
{ Just Bool }
{
[
[
{
(builtin trace)
(all dead (type) Bool)
}
(con
string
"CoverLocation (CovLoc {_covLocFile = \"test/Plugin/Coverage/Spec.hs\", _covLocStartLine = 39, _covLocEndLine = 39, _covLocStartCol = 31, _covLocEndCol = 36})"
)
]
(abs dead (type) False)
]
(all dead (type) dead)
}
]
)
]
(all dead (type) dead)
}
)
]
(abs dead (type) [ fail (con unit ()) ])
]
(all dead (type) dead)
}
)
)
]
(abs dead (type) [ fail (con unit ()) ])
)
)
]
(abs dead (type) [ fail (con unit ()) ])
]
(all dead (type) dead)
}
)
]
(all dead (type) dead)
}
Expand Down

0 comments on commit 7f4f497

Please sign in to comment.