[ 
https://issues.apache.org/jira/browse/GROOVY-12355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18111770#comment-18111770
 ] 

ASF GitHub Bot commented on GROOVY-12355:
-----------------------------------------

paulk-asert opened a new pull request, #2878:
URL: https://github.com/apache/groovy/pull/2878

   …me's number categories
   
   The runtime divides with floating-point math when either operand is a Float 
or Double, wrapper or primitive, and with BigDecimal math for integral, 
BigInteger and BigDecimal operands. The type checker only recognised primitive 
float and double, so Double / Integer was inferred as BigDecimal, and an 
operand typed as Number, whose runtime category is unknown, was inferred as 
BigDecimal too. Statically compiled code then cast the Double the runtime 
produced and failed.
   
   Unwrap before testing the floating category, and infer BigDecimal only when 
both operands belong to a category that divides that way; a Number operand 
yields Number. The group operations had the same blind spot from the other 
side: they took the category of whichever operand they recognised, so Number * 
int was int. An operand of unknown category now yields Number unless a 
floating-point partner decides the result, as it does at runtime.
   
   An arithmetic result that could not be typed before may now be a Number 
where it was an int or BigDecimal, so code that assigned it to a narrower type 
needs an explicit conversion, as the GROOVY-5539 test now shows. StringUtil.bar 
with Double arguments works without change.




> STC infers BigDecimal for division of Number or wrapper Double/Float operands
> -----------------------------------------------------------------------------
>
>                 Key: GROOVY-12355
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12355
>             Project: Groovy
>          Issue Type: Bug
>    Affects Versions: 6.0.0-RC-1, 5.1.2
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>
> Under {{@CompileStatic}} (and {{@TypeChecked}}), the static type checker 
> infers {{BigDecimal}} as the result of a division whenever neither operand is 
> a *primitive* {{float}} or {{double}}. This is wrong in two cases:
> * wrapper {{Double}}/{{Float}} operands, e.g. {{Double / Integer}}, which 
> dynamic Groovy evaluates to a {{Double}}
> * operands whose static type is {{Number}} (or another non-leaf numeric 
> type), where the runtime category is unknown
> In {{StaticTypeCheckingVisitor.getMathResultType}}, the {{DIVIDE}} branch 
> uses {{isFloatingCategory}}, which only recognises primitives, and otherwise 
> falls back to {{BigDecimal_TYPE}}. Group operations ({{+}}, {{-}}, {{*}}) 
> already fall back to {{Number_TYPE}} via {{getGroupOperationResultType}}; 
> division should do the same, i.e. infer the LUB {{Number}} when the operand 
> category can't be determined, and {{Double}} when either operand is a wrapper 
> floating type. This matches the runtime signature {{NumberMath.divide(Number, 
> Number): Number}}.
> The wrong inference is masked in a simple assignment, because the assignment 
> path runs {{castToType}} which converts a {{Double}} to a {{BigDecimal}}. So 
> {{def q = a / b}} silently returns a different type to dynamic Groovy. In a 
> chained expression the compiler emits a {{CHECKCAST}} to {{BigDecimal}} and 
> the code throws {{ClassCastException}}.
> h3. Reproducer
> {code:groovy}
> import groovy.transform.CompileStatic
> @CompileStatic
> class P {
>     static Object nn(Number a, Number b) { a / b }
>     static Object nnChain(Number a, Number b) { ((a / b) * 8).intValue() }
>     static Object di(Double a, Integer b) { a / b }
>     static Object diChain(Double a, Integer b) { ((a / b) * 8).intValue() }
> }
> class D { // dynamic, for comparison
>     static Object nn(Number a, Number b) { a / b }
>     static Object nnChain(Number a, Number b) { ((a / b) * 8).intValue() }
> }
> assert D.nn(1.5d, 2) instanceof Double        // 0.75
> assert D.nnChain(1.5d, 2) == 6
> assert P.nn(1.5d, 2) instanceof BigDecimal    // differs from dynamic
> assert P.di(1.5d, 2) instanceof BigDecimal    // differs from dynamic
> P.nnChain(1.5d, 2)  // ClassCastException: Double cannot be cast to BigDecimal
> P.diChain(1.5d, 2)  // ClassCastException: Double cannot be cast to BigDecimal
> {code}
> Results, called with {{(1.5d, 2)}}:
> ||expression||dynamic||@CompileStatic||
> |{{Number / Number}}|{{Double}} 0.75|{{BigDecimal}} 0.75|
> |{{Double / Integer}}|{{Double}} 0.75|{{BigDecimal}} 0.75|
> |{{((Number / Number) * 8).intValue()}}|6|{{ClassCastException}}|
> |{{((Double / Integer) * 8).intValue()}}|6|{{ClassCastException}}|
> |{{double / int}}|{{Double}}|{{double}}|
> Behaviour is identical on 4.0.33, 5.1.1 and 6.0.0-beta-3, so this is 
> long-standing rather than a regression.
> h3. Real-world impact
> {{org.codehaus.groovy.util.StringUtil.bar(Number x, Number min, Number max, 
> int width)}} (added in 5.0.0) is compiled statically and contains exactly the 
> chained shape above:
> {code:groovy}
> int barWidth = ((x - min) / interval * fracWidth).intValue()
> {code}
> so {{bar(0.45d, 0, 2)}} (or any {{Double}}/{{Float}} argument) throws 
> {{ClassCastException}}, while {{bar(0.45, 0, 2)}} with a {{BigDecimal}} 
> literal works. Fixing the inference makes {{bar}} work without changes, since 
> every intermediate becomes {{Number}}.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to