Coder Social home page Coder Social logo

Comments (31)

udalov avatar udalov commented on August 24, 2024 1

@robstoll

It's a bit late, so dare with me if I overlooked a phrase. I couldn't find how signature usage and body usage differ in propagation. Or lies the only difference in same module exemption?

There are two key differences between body and signature usages, highlighted in the proposal:

  1. body usages of compile-time experimental API need not be propagated
  2. body usages need not be propagated in the same module

Another question to clear up propagation. The proposal does not mention which access levels need propagation. I assume it is public and protected (where protected makes only sense for an open type), right?

Visibility modifiers have no effect on the propagation requirements. The reason is that even a private declaration that uses something experimental might be called indirectly from client code, via some public declaration:

// Library code

@ShinyNewAPI
fun foo() {}

// If we do not require propagation here...
private fun bar() {
    foo()
}

// ... then it's not required here...
fun baz() {
    bar()
}

// Usage

fun usage() {
    // ... and then we get effectively experimental behavior without any explicit opt-in from the user
    baz()
}

from keep.

udalov avatar udalov commented on August 24, 2024 1

FYI the prototype has landed into Kotlin master and 1.2.30. Although it's a 1.3-only API, it's possible to use it with -language-version 1.3 -Xskip-runtime-version-check (the latter argument is needed because of KT-22777).

Note that because Kotlin 1.3 is not yet released, using -language-version 1.3 results in "pre-release" binaries being generated by the compiler, which are not supposed to be published as libraries, because stable versions of the Kotlin compiler will reject compiling against them in the classpath.

from keep.

antanas-arvasevicius avatar antanas-arvasevicius commented on August 24, 2024 1

Hi, is there a way to use multiple experimental APIs on the same class/method?
Currently I'm trying to set couple @UseExperimental annotations but IDE says:
"This annotation is not repeatable"
e.g.:

    @UseExperimental(ExperimentalCoroutinesApi::class)
    @UseExperimental(KtorExperimentalAPI::class) // <- error here: not repeatable annotation
    private suspend fun connectToSocket() { ... 

Using Kotlin 1.3.10

from keep.

LouisCAD avatar LouisCAD commented on August 24, 2024

Looks good! I'm myself developing a few experimental libraries, including one already released for BluetoothGatt on Android relying on kotlinx.coroutines, and this would be really useful.

from keep.

cy6erGn0m avatar cy6erGn0m commented on August 24, 2024

There is "Same module exemption" but what if my library consist of multiple modules? Is there way to avoid propagation requirement except suppressing an error?

from keep.

robstoll avatar robstoll commented on August 24, 2024

It's a bit late, so dare with me if I overlooked a phrase. I couldn't find how signature usage and body usage differ in propagation. Or lies the only difference in same module exemption?

from keep.

robstoll avatar robstoll commented on August 24, 2024

Another question to clear up propagation. The proposal does not mention which access levels need propagation. I assume it is public and protected (where protected makes only sense for an open type), right?

from keep.

udalov avatar udalov commented on August 24, 2024

@cy6erGn0m

There is "Same module exemption" but what if my library consist of multiple modules? Is there way to avoid propagation requirement except suppressing an error?

Generally, if you're using an experimental API, you must propagate it. Otherwise, your clients will not be aware of the fact that something may break, and may break unexpectedly. From this perspective, it's irrelevant whether two modules (where a module is a single self-contained compilation unit) comprise a library in any sense, or not.

from keep.

cy6erGn0m avatar cy6erGn0m commented on August 24, 2024

@udalov

From this perspective, it's irrelevant whether two modules (where a module is a single self-contained compilation unit) comprise a library in any sense, or not.

But there is a difference because all the usages are inside of a single project (but different modules) so it a user depends on the same version of my modules then there is no risk so I'd like to force exemption to not poison everything. Consider:

kotlinx-corotunes:

  • core
  • io

So I'd like to use experimental core's functions in io's implementation but avoid poisoning all my io's stable API functions

from keep.

udalov avatar udalov commented on August 24, 2024

@cy6erGn0m We cannot guarantee that the user depends on the same version of these two modules though. If the user depends on kotlinx-coroutines-io version 4.2, and gets kotlinx-coroutines-core version 4.3 via a transitive dependency, any usage of a function from kotlinx-coroutines-io may break because the underlying experimental implementation was changed binary incompatibly. Whereas the user doesn't expect any breakage because

  1. no opt-in was given to any experimental features
  2. all non-experimental API in 4.3 should be binary compatible with 4.2 according to the compatibility guide

from keep.

udalov avatar udalov commented on August 24, 2024

I've updated the proposal after an internal discussion:

  • we've decided to get rid fo the confusing behavior in the "same module exemption" rule: what is taken into account now is the declaring module of the annotation marker. If the call site is in that module, body usages do not require propagation. If it's in another module, body usages always require propagation. (The reason is that otherwise it was easy to come up with an example with three modules where if the middle module is not recompiled, one could observe runtime errors in the leaf module upon changes to the experimental API, even if no consent to use any experimental API was given in the leaf module.)
  • we've changed Scope.{COMPILE_TIME/RUNTIME} to Impact.{COMPILATION/LINKAGE/RUNTIME} to be able to say more clearly what exactly can the changes to the experimental API break.

from keep.

udalov avatar udalov commented on August 24, 2024

A few minor updates:

  • same module exemption is now also supported for non-effectively-public inline functions, since they can only be used in the same module where the experimental API is introduced and thus are guaranteed to be updated accordingly after any breaking change to the experimental API
  • UseExperimental's argument has been renamed from annotationClass to markerClass to avoid clash with an existing extension property annotationClass that returns a class of the annotation

from keep.

udalov avatar udalov commented on August 24, 2024

We've had one more look at this proposal internally and decided to greatly simplify it, or otherwise it was getting out of hand pretty fast. The major change is that we now do not intend to make experimental declarations "poisonous" and verify it in the compiler as much as possible. Because of this simplification, the concept of Impact (also known as Scope previously) goes away, along with signature/body usages, same module exemption, etc.

We've also discussed a bit how we are going to use Experimental in the standard library in relation to SinceKotlin, which I summarized in the new section.

The prototype of the new simplified approach is currently being worked on.

from keep.

udalov avatar udalov commented on August 24, 2024

The support for Experimental as described in this proposal has been implemented and is available since 1.2.50 (with -language-version 1.3) and since 1.3.

from keep.

ilya-g avatar ilya-g commented on August 24, 2024

Usually declaring an experimental annotation requires to spell a long list of targets where it makes sense:

@Target(
    CLASS,
    ANNOTATION_CLASS,
    PROPERTY,
    FIELD,
    LOCAL_VARIABLE,
    VALUE_PARAMETER,
    CONSTRUCTOR,
    FUNCTION,
    PROPERTY_GETTER,
    PROPERTY_SETTER,
    TYPEALIAS
)

It's easy to forget this incantation and get an experimental marker annotation applicable where it should not have been.

What if we imply these targets by default for the annotations marked with @Experimental, if @Target is not specified?

from keep.

udalov avatar udalov commented on August 24, 2024

@ilya-g Sounds like a good idea to me, although a bit too implicit for Kotlin. Please report an issue.

from keep.

pdvrieze avatar pdvrieze commented on August 24, 2024

The feature as currently available in 1.3 is quite interesting and clearly solves a problem. However, I feel it is still quite limited in scope. In particular feature effectively introduces custom visibility scopes - see for example how kotlinx.serialization uses it to limit the reflection API - it is not actually experimental, it doesn't work properly on non-JVM targets). Thinking about this I had an idea how this can be extended.

Why not allow for proper user-defined scopes. You would have the ability to define a custom scope as annotation, specify visibility (and possibly warning/error level). Then the annotation can be used on various identifiers to determine a rich scope. The effective visibility would be the intersection between the declared regular visibility modifier and the annotation - if the code is protected the annotation does not widen it, but an inheriting class cannot access it either without the annotation either - if the annotation is internal this makes a symbol annotated as @MyScope protected effectively inaccessible to external modules. This would look like:

@Target(ANNOTATION_CLASS)
@Retention(BINARY)
annotation class CustomScope(val visibility: Visibility = Visibility.PUBLIC, val level: Level = Level.ERROR) {
    enum class Level { WARNING, ERROR }
    enum class Visibility { PUBLIC, INTERNAL, PROTECTED, PRIVATE }
}

@Target(CLASS, PROPERTY, LOCAL_VARIABLE, VALUE_PARAMETER, CONSTRUCTOR, FUNCTION,
        PROPERTY_GETTER, PROPERTY_SETTER, EXPRESSION, FILE, TYPEALIAS)
@Retention(SOURCE)
annotation class UseScope(
    vararg val markerClass: KClass<out Annotation>
)

The semantics of the annotation and the @UseScope annotation are similar to @Experimental, but the application is broadened to explicitly be about scopes rather than working for public APIs but actually being clearer as to what the feature means. It would solve a number of additional use cases beyond allowing for experimental code:

  • It would allow for a library to have different APIs for different use cases: for example one for plugins, one for mere users.
  • It would allow for internal or otherwise restricted visibilities that could also replace the most common uses for package level visibility

Limitations:

  • These custom scopes are use-site opt-ins. Within a module it is not possible to stop code from either having the annotation (becoming part of the interface) or declaring usage - Of course this can be statically analysed and restricted. It should be fairly straightforward to limit propagating use of the visibility beyond the module (if this is desirable).
  • It does not directly express the semantics of experimental
  • It needs a bit more compiler support to work correctly for non-public visibility.

What do you think about this idea?

from keep.

LouisCAD avatar LouisCAD commented on August 24, 2024

from keep.

antanas-arvasevicius avatar antanas-arvasevicius commented on August 24, 2024

Lol, sorry. I've missed that its argument is "vararg". IntelliJ not suggesting this.

from keep.

antanas-arvasevicius avatar antanas-arvasevicius commented on August 24, 2024

Hi, I was thinking about that UseExperimental annotation and I think the better approach would be omit "vararg" and make it repeatable, because:
a) Different experimental APIs will have different lifecycles so they do not become stable all at once;
After API will become stable to remove UseExperimental(API::class) will be simpler than variant with "varargs" and all source code could be replaced with simple search+replace.

b) Usage of experimental APIs is not predictable, at first I can use ObsoleteCoroutinesAPI and few days later use some KtorInternalAPI. So to add new UseExperimental is more convenient instead of adding more arguments in existing UseExperimental. Also git merge requests will look better.

c) Now I have an option to add some experimental APIs usage on a whole class and on methods so I'm using multiple experimental APIs and not using "varargs". So if I want to use one more experimental API in a method I'll need pass multiple arguments for UseExperimental. Some UseExperimental now have one argument (for class) and some mulitple arguments (for method).

Wouldn't be better to remove "vararg" support from UseExperimental and only make it repeatable?
It will look more consistent, less cognitive thinking, easier to implement autosuggestions in IDE, easier to find and remove all UseExperimental() annotations then API becomes stable.

from keep.

pdvrieze avatar pdvrieze commented on August 24, 2024

@antanas-arvasevicius One problem with your suggestion is that the JVM 1.6 target does not support repeated annotations. The retention of the annotation should however be at least class (so that the compiler can verify API levels on usage). The current API seems to be the most elegant solution.

from keep.

antanas-arvasevicius avatar antanas-arvasevicius commented on August 24, 2024

@pdvrieze Android Studio 3+ should support repeatable annotations. See: https://developer.android.com/studio/write/java8-support

from keep.

udalov avatar udalov commented on August 24, 2024

@pdvrieze Sorry for the late answer. I'm afraid I didn't completely understand how CustomScope is supposed to be used in your proposal. Could you please provide an example usage of a library code and the client code using it, demonstrating how exactly will it allow the compiler to warn the user about unwanted usages?

from keep.

pdvrieze avatar pdvrieze commented on August 24, 2024

@udalov I'll try to provide some examples.
First based upon kotlinx.serialization runtime/common/src/main/kotlin/kotlinx/serialization/SerialImplicits.kt

Instead of:

@Experimental
annotation class ImplicitReflectionSerializer

we would generalize this to

@CustomScope(level=Level.WARNING)
annotation class ImplicitReflectionSerializer

However the idea is more extensive. One can define any kind of scope:

@CustomScope(Visibility.PUBLIC) annotation class CustomizationAPI
@CustomScope(Visibility.INTERNAL) annotation class TestOnly
@CustomScope(Visibility.INTERNAL) annotation class FriendOfExampleClass
@CustomScope(Visibility.PROTECTED) annotation class ProtectedScope
@CustomScope(Visibility.PRIVATE) annotation class PrivateScope
@CustomScope(Visibility.PRIVATE) annotation class MyLockedVar

open class ExampleClass {
  @MyLockedVar
  val myLockedVarLock = ReentrantLock()
  
  @MyLockedVar
  var myLockedVar: SomeObject = someInitializer

  @UseScope(MyLockedVar::class)
  inline fun <R> useLockedVar(action: (SomeObject)->R):R = myLockedVarLock.withLock {
    action(myLockedVar)
  }

  @PrivateScope
  var brittleVar: Any?=null // This would actually be private due to the annotation
  // perhaps brittleVar needs to have a lock before it can be updated, or any other operation where even 
  // class scope is not appropriate

  @UseScope(PrivateScope::class)
  fun doSomethingWithBrittleVar() {
    brittleVar = "Something" // some actually sensitive operation that can break
  }

  @ProtectedScope
  internal fun doSomethingOnlyInternal() = todo() // This is not visible outside the module - or child classes

  fun accidentallyUseBrittleVar() {
    println(brittleVar) // Will not compile due to scope
  }

  @FriendOfExampleClass // Actually has internal visibility
  fun friendOnlyFun() =todo("do something "internal")

  @TestOnly
  fun resetHookOnlyNeededForUnitTests() = todo()

  @CustomizationAPI // API only useful for customization, not regular users
  fun setCustomObjectFactory(factory: CustomObjectFactory) = todo()
}

// In same module
@UsesScope(FriendOfExampleClass::class)
fun friendFun(c: ExampleClass) // do something that calls c.friendOnlyFun

// In a different module (or the same one)
@UseScope(CustomizationAPI::class)
fun ExampleClass.reconfigure() {
  setCustomObjectFactory(myCustomObjectFactory)
}

@TestOnly
class TestExampleClass {
var c: ExampleClass

  @BeforeEach
  fun resetClass() {
    c.resetHookOnlyNeededForUnitTests()
  }
}

What are the benefits:

  • Libraries can distinguish between different kinds of public API's for different purposes - be it experimental or some other reason to separate it. For example customization APIs can be easy to use incorrectly and can clutter up the documentation for mere users.
  • It is a superset of @experimentalapi
  • It solves the problem that package protection is used to solve in Java, in particular it allows for a TestOnly scope.
  • In large classes it can even restrict access to private members to a select few (especially audited) functions. One could use this for example to ensure that a certain variable is always accessed through its lock (for example useLockedVar)
  • It prevents misuse of @experimentalapi (such as the kotlinx.serialization example shows - the code is not experimental, it is merely non-portable)
  • Using annotations there is no keyword proliferation.

Considerations:

  • Some possibilities may be more worthwhile than others but there is value in being a generic solution. Is a custom private scope valuable in practice? It might be with more complex classes, but not including private is inconsistent and it is not detrimental.
  • For warning-level scopes the annotation would not actually replace the pre-established scope.
  • @UseScope(...) is a bit verbose, I'm not sure what to do about that.
  • There is no implied usage semantics to the scope, while there are clear semantics on what the compiler will do, it does not imply to the user that it is for experimental API's only. It only requires opt-in.

from keep.

udalov avatar udalov commented on August 24, 2024

@pdvrieze Thank you for the elaborate example and the explanation!

Do I understand it correctly that the compiler uses CustomScope's visibility value to infer the actual visibility of the annotated declaration? E.g. is ExampleClass.brittleVar actually private in the bytecode? I'm not sure then if this would be any better than using the ExperimentalAPI simply as it exists today (maybe under a different name -- see below), and requiring the user to specify the visibility manually via public/internal/protected/private modifiers, which is more natural since modifiers are a very basic language construct, unlike annotations.

It seems to me that if the user manually adds private to brittleVar/myLockedVar, internal to friendOnlyFun/resetHookOnlyNeededForUnitTests etc., the remaining code will look exactly the same as today, only differing in the names: Experimental -> CustomScope, UseExperimental -> UseScope. Am I missing something? The only exception would be doSomethingOnlyInternal which has to be marked both protected and internal which is impossible in Kotlin, but since there's no such visibility in the language, this wouldn't work as simply in your proposal either, because this should be supported in the compiler first anyway. In other words, the visibility aspect in your proposal seems pretty detached from the scope aspect to me, and I don't see any reason to mix two independent language features when it's not required.

Regarding this point:

It prevents misuse of @experimentalapi (such as the kotlinx.serialization example shows - the code is not experimental, it is merely non-portable)

I agree that the annotated API is not experimental and thus marking it as Experimental seems unfair. However, since the tool that the feature provides fits for the purpose the API's author wants to achieve, from my perspective this means that the feature is not actually misused and the problem is rather only in the name. If the Experimental annotation was named differently, for example Scoped (inspired by your proposal) or Restricted or something like that, but behaved exactly the same, we wouldn't call it a misuse, right? There's already an issue highlighting this problem: https://youtrack.jetbrains.com/issue/KT-26216

from keep.

pdvrieze avatar pdvrieze commented on August 24, 2024

@udalov Indeed brittleVar would be actually private in bytecode. In the case of private it may not be that worthwhile, but for visibilities such as internal it forces all users to be at most internal (but they can still be private) in bytecode. Private is there for completeness. Most of the difference indeed is in the name.

I can see the point about not mixing it, but in some cases you may want to limit a scope to a certain application. Using the annotation scope does not work as it relates to the declaration of the scope, not the use of it.

My point is mainly to rename, but also to broaden it up a bit. Although that technically breaks the annotations for semantics aspect. Btw. protected internal would be mangled as internal, but protected in the bytecode. The issue is quite close to my idea indeed. I'm not sure about misuse as wording is important, but it doesn't misuse semantics.

from keep.

udalov avatar udalov commented on August 24, 2024

@pdvrieze Please share your naming suggestions in the comments to KT-26216, we'll come back to them when discussing whether and how to rename Experimental.

Regarding the visibility-altering behavior of an annotation -- I'm pretty sure this would lead to lots of issues in the compiler and tooling, the main reason of which is that you'd need to resolve and type-check all annotations before you can deduce the actual visibility of a declaration. In fact, the compiler even might need visibility of a declaration before resolving annotations right now in some cases, I can't be sure because such basic information was always available lexically, before any resolution happens. I think it would also complicate the ability of people reading the source code: in case the visibility is not evident from the scope annotation's name, it's pretty hard to understand whether the declaration you're looking at is public or private. To be sure, you'll need to check the source of all annotations on that declaration in the worst case. As such, I don't think this would be a good addition to this feature.

However, if you have any other suggestions on how to broaden the feature without introducing this sort of problems, we'll be glad to discuss them -- please share in KT-26216 as well. Thanks!

from keep.

pdvrieze avatar pdvrieze commented on August 24, 2024

@udalov I've added a short comment tot he bug.

On the visibility altering behaviour, perhaps I've not been sufficiently clear on how it works. Effectively it works as an upper bound on the visibility. It should work as follows: if a visibility modifier has been specified at the use site (not the custom scope) it will either be used or fail to compile (violates the bound). If no visibility modifier is specified (default public) then there are two options: either derive the visibility from the annotation, or check the visibility of the annotation and throw an error in case the visibility of the scope is not public. In all cases the actual visibility will be recorded in the visibility attributes of the method exactly the same way it is done already.

The more complex case is that of protected internal. This does not exist on Java bytecode level, but might be able to benefit from the same mechanism applied to regular internal. This could/should still be recorded in the extended Kotlin signature.

Key is that all visibility is resolved/determined at compilation of the scope use site and recorded in the usual way - so everything is visible/final in the method/class/.. declaration and does not depend on scope resolution. It might be that explicit visibility is better than default (except for public as is now) from a readability perspective and I don't mind dropping the default system - I still think it can helpful to be able to limit a scope to a visibility on its application (of course the annotation itself can have limited visibility too).

Scope usage thus at first does not need to resolve the scope annotation at all, it merely needs to compare the locally available scope/api with the one(s) declared on the symbol (potentially) being used. Only the currently already available error/warning option needs resolving/loading the annotation.

from keep.

udalov avatar udalov commented on August 24, 2024

We've discussed this proposal internally once again and decided to change it in the following way:

  • Rename Experimental -> RequiresOptIn, UseExperimental -> OptIn and the whole feature to "opt-in requirements" to enable broader usages.
  • Add custom message to RequiresOptIn to override the default "this API is experimental" compiler diagnostic message.
  • Remove the compiler argument -Xexperimental (but keep -Xuse-experimental, now known as -Xopt-in).

from keep.

pdvrieze avatar pdvrieze commented on August 24, 2024

The rename seems like a good idea, it also matches other uses of the capability.

from keep.

IRus avatar IRus commented on August 24, 2024

Therefore, we will require each user of RequiresOptIn to provide at least one -Xopt-in compiler argument
Unless one of these arguments is provided, the compiler will report a warning

Is require mean that compiler should report error instead?

Issue about this behavior KT-37507

from keep.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.