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

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

paulk-asert commented on code in PR #2645:
URL: https://github.com/apache/groovy/pull/2645#discussion_r3672245049


##########
src/main/java/org/codehaus/groovy/control/CompilerConfiguration.java:
##########
@@ -424,6 +434,20 @@ public void setLogClassgenStackTraceMaxDepth(int 
logClassgenStackTraceMaxDepth)
      */
     private Set<String> scriptExtensions = new LinkedHashSet<>();
 
+    /**
+     * Consumer vetoes for {@link groovy.transform.stc.ClassTag @ClassTag} 
preemption under static
+     * compilation (GROOVY-12115). Preemption is declared by the API author
+     * ({@code @ClassTag(preempt=true)}) and contained to the declaring class; 
this set lets the
+     * consuming build veto it anyway. Entries use the same selector forms as
+     * {@code groovy.extension.disable}: a bare method name ({@code 
withDefault}) vetoes all
+     * overloads of that name, a signature form ({@code 
withDefault(Map,Class,Closure)}, simple or
+     * fully-qualified type names) vetoes the exact overload, and {@code *} 
vetoes all preemption.
+     * Additive injection (supplying an otherwise-mandatory token, such as for 
{@code asChecked})
+     * is never gated by this set. Seeded from the {@code 
groovy.classtag.preemption.disable}
+     * system property; empty (the default) honours declared intent. Never 
null.
+     */
+    private Set<String> classTagPreemptionDisabled = 
splitRespectingParens(getSystemPropertySafe("groovy.classtag.preemption.disable"));

Review Comment:
   Actually, given that it is rare, perhaps the global opt-out is enough. The 
thinking behind the list is that a Grails app using multiple plugins might want 
to upgrade some of those plugins and so be able to opt out just some of the 
changes plugins might make in newer versions. But if you can change some of the 
code, you can opt-out individual ones by swapping out of CompileStatic or 
whatever.





> Reify a receiver's type argument as a Class parameter under @CompileStatic 
> (@ClassTag)
> --------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12115
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12115
>             Project: Groovy
>          Issue Type: New Feature
>          Components: Static compilation, Static Type Checker
>            Reporter: Eric Milles
>            Assignee: Paul King
>            Priority: Major
>              Labels: GEP
>
> h1. Summary
> Introduce a parameter annotation (working name {{{}@ClassTag{}}}) that lets 
> the static compiler *auto-supply a reified {{Class<K>}} argument* from a 
> method receiver's generic type, so callers can omit type tokens that today 
> exist only to work around erasure.
> Split off from GROOVY-11807. That issue ships the *runtime* half — explicit 
> {{{}Class<K>{}}}-token overloads that enforce types at run time. This issue 
> is the *compile-time ergonomics* half: making the token optional under static 
> compilation.
> h2. Background and origin
> On the JVM, a method that needs the runtime type of a generic type parameter 
> cannot
> recover it (erasure), so the API must take an explicit {{Class<K>}} token. 
> Groovy
> already does this in several places, e.g. {{asChecked}} (since 5.0.0) and the 
> new
> key-checked {{withDefault}} overloads from GROOVY-11807:
> {code:java}
> // DefaultGroovyMethods
> public static <T>   Collection<T> asChecked(Collection<T> self, Class<T> 
> type)            // -> Collections.checkedCollection
> public static <K,V> Map<K,V>      asChecked(Map<K,V> self, Class<K> keyType, 
> Class<V> valueType) // -> Collections.checkedMap
> // GROOVY-11807 (merged for 6.0.0), Class params carry a /* @ClassTag 
> GROOVY-12115 */ marker
> public static <K,V> Map<K,V> withDefault(Map<K,V> self, Class<K> keyType, 
> Class<V> valueType, Closure<V> init)
> public static <K,V> Map<K,V> withDefault(Map<K,V> self, Class<K> keyType, 
> Closure<V> init)
> {code}
> In every case the {{Class}} argument is *redundant with a type the compiler 
> already knows* (the receiver's type argument). {{@ClassTag}} removes the 
> redundancy under static compilation.
> h2. The problem
>  * Verbose: {{list.asChecked(String)}} / {{map.asChecked(Number, Foo)}} 
> restate the
> element/key/value types the static type already declares.
>  * Not transparent: the safety-improving variant of {{withDefault}} can't be 
> selected
> unless the caller manually threads the {{{}Class{}}}.
>  * The tokens cannot be inferred at run time — only the compiler, with the 
> static type
> in hand, can supply them.
> h2. Proposal: {{@ClassTag}}
> A parameter annotation marking a {{Class<X>}} parameter as 
> {*}compiler-supplied{*}. Under
> {{@CompileStatic}} / {{@TypeChecked}} the type checker may select an overload 
> whose only
> extra parameter is a {{@ClassTag Class<X>}} and synthesise {{X.class}} from 
> the
> receiver's matching type argument, rewriting the call. Dynamic Groovy ignores 
> the
> annotation and binds the original overload.
> {code:java}
> @Documented
> @Incubating
> @Retention(RetentionPolicy.RUNTIME) // read off (possibly reflection-backed) 
> extension-method
>                                     // parameters by the STC, so CLASS/SOURCE 
> do not suffice
> @Target(ElementType.PARAMETER)
> public @interface ClassTag {
>     // Optional override naming the type variable to reify, for when the 
> parameter
>     // type cannot carry it. Usage e.g.:  @ClassTag("K")
>     String value() default "";
> }
> {code}
> h3. Annotation shape and binding
>  * *Param-level, not method+index.* An earlier sketch put 
> {{@OverrideCall(1)}} on the
> method with an index. Moving the marker onto the parameter makes the 
> *position*
> implicit (where it sits) and lets the *parameter type* {{Class<K>}} encode 
> *which*
> generic to reify — strictly more self-describing than a bare index (which 
> never said
> K-vs-V).
>  * *Default binding* reads the type variable from {{{}Class<X>{}}}. The type 
> checker sees
> {{<X>}} pre-erasure (and the Signature attribute retains it), so this is 
> reliable.
> * *Optional override* {{@ClassTag("K")}}, for edge cases the parameter type 
> can't
> express: a {{Class<?>}}/raw token, or a method whose own type variables don't 
> line up with the
> receiver's. The STC *rejects a name that does not resolve to a type variable 
> in scope*, so typos
> fail at compile time — *but only for methods compiled from source in the 
> current unit*. An extension
> method from an already-compiled library (e.g. a Java DGM method) is never 
> visited as source, so a
> typo in its override silently disables injection rather than being reported. 
> Library {{@ClassTag}}
> parameters therefore use the no-override form in practice, where the name is 
> read from
> {{Class<X>}} and cannot be mistyped.
> h3. Multiple tokens and fill order
> Some consumers need more than one token, e.g. asChecked(Map, @ClassTag 
> Class<K>,
> @ClassTag Class<V>). Rule: each {{@ClassTag}} parameter is synthesised *at its
> declared position*; the caller's supplied arguments map to the remaining 
> (non-tag)
> parameters left-to-right. For {{asChecked()}} that means "fill slots 1 and 2 
> with
> {{{}K.class{}}}, {{V.class}} in declaration order." (This generalises the 
> single-hole case
> the {{@OverrideCall(1)}} index handled.)
> When more than one overload could absorb tokens, the checker prefers the one 
> that
> reifies the *most* tokens (strongest checking); if equally-specific 
> candidates disagree
> on the tokens, the call is too ambiguous and *no injection happens* (the call 
> binds as
> written).
> h3. Overload selection / preemption
> For a call that *already matches* a no-token overload (e.g. 
> {{withDefault\{...}}}), the
> {{@ClassTag}} overload can *preempt* it for a transparent upgrade. Preemption 
> means
> unchanged source changes behaviour under static typing, so it is *gated*:
>  * *Config allowlist.* Preemption applies only to method names in
> {{CompilerConfiguration.classTagPreemptionTargets}} (default holds only 
> {{withDefault}}).
> Additive injection (supplying an otherwise-mandatory token, e.g. for 
> {{asChecked}}) is
> never gated.
>  * *Object-erasure guard.* When every token would erase to {{Object}} (e.g. an
> untyped/{{def}} receiver, or a bare {{[:]}} literal) there is nothing to 
> check, so the
> lenient overload is kept — no silent upgrade to a checked view that can never 
> reject
> anything.
>  * *Applicability decides, with rollback.* A candidate tag overload is 
> matched by arity
> (non-tag parameter count = supplied argument count) and reifiability; whether 
> its
> remaining parameters accept the supplied argument types is decided by 
> retrying normal
> overload selection with the synthesised tokens inserted. If the retry finds no
> applicable method the insertion is rolled back — injection can never make a 
> call fail
> that would otherwise succeed, nor change the error reported for a call as 
> written.
> Policy for API authors stands: use preemption *only when the new behaviour is 
> an
> unambiguous bug-fix* (the old behaviour has no legitimate users). Otherwise 
> *mint a new
> name* (e.g. {{withCheckedDefault}}) so the opt-in is by name and nothing 
> existing
> changes meaning. {{@ClassTag}} still earns its keep there by letting static 
> callers
> omit the {{Class}}.
> h2. Examples (before / after, under @CompileStatic)
> The receiver must carry statically-known type arguments — a typed variable, 
> parameter,
> field or return value. A bare literal receiver ({{[]}} / {{[:]}}) is
> {{ArrayList<Object>}} / {{LinkedHashMap<Object,Object>}}-typed, so nothing 
> can be
> reified and no injection happens ({{asChecked()}} then fails to resolve; 
> {{withDefault}}
> stays lenient).
> ||API||Today||With @ClassTag||
> |asChecked (list)|{{list.asChecked(String)}}|{{list.asChecked()}}|
> |asChecked (map)|{{map.asChecked(Number, Foo)}}|{{map.asChecked()}}|
> |withDefault 
> (map)|{{map.withDefault(Number)\{...}}}|{{map.withDefault\{...}}} 
> (transparently checked)|
> {code:groovy}
> // asChecked: the Class was mandatory -> @ClassTag makes it omissible 
> (opt-in, additive)
> Map<Number,Foo> map = [:]
> Map<Number,Foo> m = map.asChecked()               // compiler injects 
> Number.class, Foo.class
> // withDefault: a no-Class form already exists -> @ClassTag overload preempts 
> it (allowlisted)
> Map<Number,String> base = [:]
> Map<Number,String> md = base.withDefault{ 'n/a' } // same source as today
> ((Map) md).put('x', 'y')                          // now ClassCastException 
> (key checked)
> {code}
> h2. Boundaries and edges
>  * *Static-only.* The injection is an STC rule. In dynamic Groovy:
>  ** {{withDefault}} keeps binding the *unchecked* overload (no K to inject) — 
> same
> source, different behaviour by compilation mode.
>  ** {{asChecked()}} (no Class) won't resolve at all — there is no zero-arg 
> method and
> no injection -> {{{}MissingMethodException{}}}. The shorter spelling is 
> {*}static-only syntax{*}.
>  * *K must be statically known.* {{Map<Number,?>}} works; {{def}} / raw 
> {{Map}} /
> wildcard-only don't — no token is synthesised, so {{withDefault}} stays 
> lenient and
> {{asChecked()}} won't compile.
>  * *Fail-soft (as implemented).* When a token can't be reified the compiler 
> silently
> degrades: no injection, and the call binds as written. Additive use then 
> fails loud
> naturally (the token-less call doesn't resolve); preemption keeps the lenient 
> binding.
> Whether the preemption case should warn rather than stay silent remains a 
> discussion point (see the questions section).
>  * *Escape hatch.* The explicit {{Class}} form always works — in every mode, 
> and when
> the type is unknown.
>  * *Fidelity is erased.* {{Class<K>}} reifies only the *erased* class:
> {{List<String>}} and {{List<Integer>}} are indistinguishable (see TypeTag 
> section).
> * *Override validation is source-only.* {{@ClassTag("name")}} typo-checking 
> runs when the declaring
> method is type-checked from source; compiled extension methods are not 
> re-validated by the consumer.
> h2. Retrofit candidates (codebase audit)
> ||Tier||API||Why it fits / note||
> |1|{{asChecked(...)}} — 9 overloads, since 5.0.0|Textbook case: explicit 
> {{Class}} is purely an erasure-workaround over 
> {{{}Collections.checkedXxx{}}}. Also the multi-token (K,V) stress test.|
> |1|{{withDefault}} / {{withCheckedDefault}} (GROOVY-11807)|Type-laundering 
> insert via the untyped {{{}Map.get(Object){}}}; reified K lets it check. 
> Unifies with checked maps: {{withDefault = checkedMap + default}} once K is 
> reified.|
> |2|{{ListWithDefault}} ({{{}(T) initClosure.call(...){}}})|Default cast to 
> element type; weaker — the closure return is already statically checkable.|
> |3 (new method)|{{collection.toArray()}} -> {{T[]}}|Reified array creation 
> (the Kotlin {{reified}} / Scala {{ClassTag}} array case). New API, not a 
> retrofit of {{{}asType{}}}.|
> |none|{{{}asType{}}}/{{{}as{}}}, {{{}newInstance(Class){}}}, 
> {{{}find{}}}/{{{}grep{}}}-by-Class, {{castToType}}|The {{Class}} is the 
> user's {*}intent{*}, not an erasure-workaround — auto-supplying would be 
> wrong.|
> Net: the realistic footprint is small and clustered around *checked-view and
> default-growing collection APIs*, not a sprawling refactor.
> h2. Reification fidelity: ClassTag vs a possible TypeTag tier (weaker)
> {{@ClassTag}} supplies an *erased* {{{}Class<K>{}}}. A richer future tier 
> could inject a
> *full-generic* token (a super-type token {{{}TypeToken<K>{}}}), analogous to 
> Scala's
> {{TypeTag}} / {{scala.quoted.Type}} versus its {{{}ClassTag{}}}. Kotlin's
> {{inline fun <reified T>}} is the JVM precedent for the cheap tier.
> *This extension now looks weak given the boundaries above:*
>  * The concrete candidates ({{{}asChecked{}}} -> 
> {{{}Collections.checkedMap{}}}, {{{}withDefault{}}})
> only ever do {{isInstance}} checks, which are erased anyway — the erased 
> class suffices.
>  * Restricting the source to the *receiver's type argument* and to 
> *static-only*
> injection further limits where full-generic fidelity would pay off.
> Recommendation: keep {{ClassTag}} (erased) as the design; note 
> {{{}TypeTag{}}}/{{{}TypeToken{}}}
> as a *possible later tier* only if a real full-generic consumer appears. The 
> annotation
> *name* could encode the fidelity ({{{}@ClassTag{}}} now, reserving 
> {{@TypeTag}} for the full
> tier), or favour familiarity ({{{}@Reified{}}}, after Kotlin) — see the 
> questions section.
> h2. Comparison with Scala / Kotlin / Java
> ||Mechanism||How the type is obtained||Fidelity||Propagation||
> |Groovy {{@ClassTag}} |compiler synthesises {{Class}} from the receiver's 
> type arg|erased class|fail-soft |
> |Scala {{ClassTag}}|implicit/{{{}using{}}} evidence materialised from the 
> static type|erased class|fail-loud (context bound {{{}[T: ClassTag]{}}})|
> |Scala {{TypeTag}} / {{quoted.Type}}|implicit/{{{}using{}}} evidence|full 
> generic|fail-loud|
> |Kotlin {{reified}}|{{inline}} + reify the type parameter|actual type 
> (inlined)|enforced by {{inline}}|
> |Java baseline|explicit {{Class<T>}} token ({{{}EnumMap{}}}, 
> {{{}Collections.checkedMap{}}}, {{{}getAnnotation(Class<T>){}}})|erased 
> class|manual|
> |Guava/Gson {{TypeToken<T>}}|super-type token|full generic|manual|
> In short: {{@ClassTag}} is Scala's {{ClassTag}} idea (compiler-supplied 
> runtime type
> evidence) delivered via an annotation + STC call-rewrite instead of implicit
> parameters; it auto-supplies the *Java {{Class<T>}} token* idiom rather than 
> inventing
> a new runtime representation.
> h2. Proposed scope / deliverables
> # The {{@ClassTag}} annotation (parameter target; {{RUNTIME}} retention, since
> extension-method parameters are read off reflection-backed class nodes; 
> optional
> {{value}} override).
> # STC support: overload selection that prefers a {{@ClassTag}} overload and 
> synthesises
> the {{X.class}} argument(s) from the receiver's type arguments; validation of
> {{{}value{}}}; well-defined multi-token fill order.
> # First consumers: retrofit {{asChecked}} and the GROOVY-11807 {{withDefault}}
> overloads (validating the K,V multi-token case).
> # Document the decided stances (see the questions section)
> *Out of scope:* the full-generic {{{}TypeTag{}}}/{{{}TypeToken{}}} tier; any 
> change to dynamic
> dispatch (dynamic callers keep passing the {{Class}} explicitly).
> h2. Questions (and proposed stance)
> Each item records the stance the proposed implementation currently takes; all 
> remain subject
> to revision pending community review.
>  * *Preemption control surface and default allowlist.* Preemption means 
> unchanged source
> can change behaviour under static typing — should it be governed by a 
> hardcoded rule,
> new-name-only, or configuration? *Proposed stance:* a config-level allowlist,
> {{CompilerConfiguration.classTagPreemptionTargets}} — an opt-in set of method 
> names whose
> shipped default holds only {{withDefault}} (the unambiguous bug-fix case). 
> Additive
> injection (supplying an otherwise-mandatory token, e.g. for {{asChecked}}) is 
> never gated.
> Clearing the set disables preemption entirely; adding a name opts a user API 
> in. Trade-off
> acknowledged: semantics become configuration-dependent (a build or IDE 
> constructing a
> different {{CompilerConfiguration}} resolves the call differently), which is 
> why the
> shipped default is minimal and the feature is {{@Incubating}}.
>  * *Diagnostic when a token cannot be reified.* Stay silent, warn, or error 
> (Scala
> context-bound style)? *Proposed stance:* stay silent (fail-soft). Additive 
> use already
> fails loud naturally (no token is synthesised, so the call does not resolve). 
> For
> preemption, a non-reifiable receiver (raw/wildcard, or every token erasing to 
> {{Object}})
> keeps the lenient binding — the code behaves exactly as it did before the 
> feature existed,
> so silence is the honest signal. A warning tier could be added later if 
> experience shows
> the silent keep confuses users; a hard error would defeat the 
> transparent-upgrade goal.
>  * *Annotation name.* {{@ClassTag}} vs {{@Reified}} vs {{@TypeTag}}? 
> *Proposed stance:*
> {{@ClassTag}} — honest that only the *erased* class is supplied (Scala's 
> precedent for
> exactly this fidelity), and it reserves {{@TypeTag}} for a possible later 
> full-generic
> tier. {{@Reified}} (after Kotlin) would overpromise: Kotlin's {{reified}} 
> delivers the
> actual type via inlining, which this is not.
>  * *Retention and API status.* Is a public {{RUNTIME}} annotation the 
> intended surface,
> versus an internal marker? *Proposed stance:* yes — public, {{RUNTIME}}, 
> {{@Incubating}}.
> {{RUNTIME}} (not {{CLASS}} as first sketched) is forced by mechanics: 
> extension-method
> parameters are read off a reflection-backed {{ClassNode}} that only exposes
> runtime-visible annotations. Public because third-party APIs (not just DGM) 
> are meant to
> be able to opt in; {{@Incubating}} preserves the freedom to adjust the 
> surface.
>  * *Override-validation reach.* {{@ClassTag("name")}} typo-checking is 
> source-only; the
> same annotation on an already-compiled (e.g. Java DGM) extension method is not
> re-validated by the consumer. Acceptable, or do we want a check at the 
> library's own build
> time? *Proposed stance:* acceptable. Library authors are steered to the 
> no-override form
> (the type-variable name is read from {{Class<X>}} and cannot be mistyped); 
> the override
> exists for raw/wildcard tokens and is validated whenever the declaring method 
> is compiled
> from source. A library-build-time check could be added later without changing 
> the surface.
> h2. Related issues
>  * GROOVY-11807 — runtime key-checked {{withDefault}} (the quick-win this 
> builds on);
> merged for 6.0.0. Its new {{Class}} parameters carried {{/* @ClassTag 
> GROOVY-12115 */}}
> markers as the seam for this work; the PR for this issue replaces them with 
> the real
> annotation.



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

Reply via email to