[
https://issues.apache.org/jira/browse/GROOVY-12741?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18140810#comment-18140810
]
ASF GitHub Bot commented on GROOVY-12741:
-----------------------------------------
codeconsole opened a new pull request, #3276:
URL: https://github.com/apache/groovy/pull/3276
Fixes [GROOVY-12741](https://issues.apache.org/jira/browse/GROOVY-12741).
The contracts transform runs in semantic analysis, before the trait
transform, and treated a trait as a class: it generated the condition class
from the trait method's parameters and marked the trait `@Contracted`. The
trait transform then moved each method with a body into the helper class as a
static method with a leading `$self` parameter, and the helper was woven in
instruction selection with one argument more than the condition class accepts.
The effects: `@Requires` on a trait method with a parameter crashed class
generation (`ArrayIndexOutOfBoundsException` in
`InvocationWriter.loadArguments`, or `StaticInvocationWriter` under
`@CompileStatic`); `@Requires` without parameters and `@Ensures` compiled, then
every call failed with `No such property: $GCONTRACTS_ENABLED for class:
Bank$Trait$Helper`; a trait `@Invariant` was inherited by the implementing
class and failed while the trait's fields were still being initialised.
`@ThrowsIf` and `@Decreases` on a trait method with a body are woven into the
helper class after the trait transform has rewritten the body, so a condition
that refers to trait state has no receiver there.
Supporting contracts on trait methods needs a design (where the check runs,
what `this` is, how a trait invariant interacts with trait field
initialisation, private and static trait methods). This change makes the module
report a compile error instead of crashing or failing at run time.
`ClosureExpressionEvaluationASTTransformation` rejects, for every trait in the
unit, a class-level contract annotation (`@Invariant is not supported on a
trait (Bank).`) and a contract, `@ThrowsIf` or `@Decreases` on a method with a
body (`@Requires is not supported on a trait method with a body (Bank.deposit);
only an abstract trait method may carry a contract.`). A contract on an
abstract trait method keeps working, as does one on an interface method a trait
implements.
The crash that `@ThrowsIf` and `@Decreases` on a trait method hit first, in
core `TraitComposer`, is
[GROOVY-12743](https://issues.apache.org/jira/browse/GROOVY-12743) with its own
pull request; this change rejects them before that point is reached, so the two
are independent.
Tests: a new `TraitContractValidationTests` (six tests: precondition with
and without parameters, postcondition, trait invariant, statically compiled
trait, and the abstract trait method that keeps working), two tests in
`ThrowsIfTests` and one in `MethodDecreasesTests`. The `groovy-contracts`
module passes with the change (405 tests).
> groovy-contracts: a contract annotation on a trait or a trait method with a
> body crashes the compiler or fails at run time
> --------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12741
> URL: https://issues.apache.org/jira/browse/GROOVY-12741
> Project: Groovy
> Issue Type: Bug
> Components: groovy-contracts
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> h3. Problem
> A {{@Requires}} on a trait method with a parameter crashes class generation:
> {code:groovy}
> import groovy.contracts.*
> trait Bank {
> BigDecimal balance = 0
> @Requires({ amount > 0 })
> void deposit(BigDecimal amount) { balance += amount }
> }
> class Account implements Bank {}
> new Account().deposit(1); println 'ok'
> {code}
> {noformat}
> General error during instruction selection: Index 1 out of bounds for length 1
> java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for length 1
> at
> org.codehaus.groovy.classgen.asm.InvocationWriter.loadArguments(InvocationWriter.java:379)
> at
> org.codehaus.groovy.classgen.asm.InvocationWriter.writeDirectMethodCall(InvocationWriter.java:292)
> at
> org.codehaus.groovy.classgen.asm.InvocationWriter.makeDirectCall(InvocationWriter.java:436)
> at
> org.codehaus.groovy.classgen.asm.InvocationWriter.makeCall(InvocationWriter.java:191)
> at
> org.codehaus.groovy.classgen.asm.InvocationWriter.writeInvokeMethod(InvocationWriter.java:622)
> at
> org.codehaus.groovy.classgen.AsmClassGenerator.visitMethodCallExpression(AsmClassGenerator.java:1244)
> {noformat}
> The same happens with {{@CompileStatic}} on the trait (in
> {{StaticInvocationWriter.loadArguments}}). The other contract annotations do
> not crash but do not work either:
> * {{@Requires}} on a trait method without parameters, and {{@Ensures}} on any
> trait method: the class compiles, then every call fails with
> {{MissingPropertyException: No such property: $GCONTRACTS_ENABLED for class:
> Bank$Trait$Helper}}.
> * {{@Invariant}} on a trait: inherited by the implementing class like an
> interface invariant, it fails at construction, while the trait's fields are
> still being initialised (the condition sees {{balance}} as null).
> A contract on an abstract trait method works (it applies to the implementing
> class like an interface contract), and so does one on an interface method a
> trait implements.
> Reproduced on Groovy 6.0.0 and on master (df1fe66ed1).
> h3. Cause
> The contracts transform runs in SEMANTIC_ANALYSIS before the trait transform
> and treats the trait as a class: it generates the condition class with the
> trait method's parameters and marks the trait {{@Contracted}}. The trait
> transform then moves each method with a body into the helper class as a
> static method with a leading {{$self}} parameter, keeping the annotations,
> and copies the class annotations to the helper. In INSTRUCTION_SELECTION the
> module weaves the helper (it carries {{@Contracted}};
> {{CandidateChecks.isContractsCandidate}} is not consulted), building the
> {{doCall}} argument list from the helper method's parameters, one more than
> the condition class accepts; without parameters the extra argument is dropped
> and the woven {{$GCONTRACTS_ENABLED}} guard has no field to refer to in the
> helper.
> h3. Fix
> Supporting contracts on trait methods needs a design (where the check runs,
> what {{this}} is, how a trait invariant interacts with trait field
> initialisation, private and static trait methods). For now the module reports
> a compile error instead of crashing:
> {noformat}
> [groovy-contracts] @Requires is not supported on a trait method with a body
> (Bank.deposit); only an abstract trait method may carry a contract.
> [groovy-contracts] @Invariant is not supported on a trait (Bank).
> {noformat}
> {{@ThrowsIf}} and {{@Decreases}} on a trait method with a body are reported
> the same way (without the abstract-method hint, since on an abstract method
> they stay informational). Their transforms run after the trait transform has
> moved the method into the helper class and rewritten the body to reach trait
> state through {{$self}}; the guard or measure woven there is not rewritten,
> so a condition that refers to trait state fails at run time with a missing
> property on the helper class, and only a condition that refers to parameters
> alone happened to work (once the separate core crash in {{TraitComposer}} for
> annotations with a closure member, GROOVY-12743, is fixed).
> {noformat}
> [groovy-contracts] @ThrowsIf is not supported on a trait method with a body
> (Bank.withdraw).
> [groovy-contracts] @Decreases is not supported on a trait method with a body
> (Counting.sumUp).
> {noformat}
> A contract on an abstract trait method keeps working. Pull request to follow.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
