Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many deprecated methods should be removed in 0.5.0 #2765

Closed
LeeTibbert opened this issue Aug 2, 2022 · 11 comments
Closed

Many deprecated methods should be removed in 0.5.0 #2765

LeeTibbert opened this issue Aug 2, 2022 · 11 comments
Milestone

Comments

@LeeTibbert
Copy link
Contributor

LeeTibbert commented Aug 2, 2022

There are probably a large (?) number of places where methods have been deprecated in 0.4.x.

2022-11-19 The alloc/stackalloc deprecations are removed in PR #3003

The 0.5.0 release offers the opportunity to remove those methods. This reduces the amount
of dead code in SN and its attendant costs.

In particular, whilst trying to track down solutions to my favorite 0.5.0 annoyances, I came
across code in the aforementioned UnsafePackageCompat.scala (beware there are two
of them, both probably need the same fix).

 /** Heap allocate and zero-initialize n values using current implicit         
   *  allocator. This method takes argument of type `CSSize` for easier interop\
,                                                                               
   *  but it' always converted into `CSize`                                     
   */
  @deprecated(
    "alloc with signed type is deprecated, convert size to unsigned value",
    since = "0.4.0"
  )
  inline def alloc[T](inline n: CSSize)(using Tag[T], Zone): Ptr[T] =
    alloc[T](n.toUInt)

and

/** Stack allocate n values of given type.                                    
   *                                                                            
   *  Note: unlike alloc, the memory is not zero-initialized. This method takes 
   *  argument of type `CSSize` for easier interop, but it's always converted   
   *  into `CSize`                                                              
   */
  @deprecated(
    "alloc with signed type is deprecated, convert size to unsigned value",
    since = "0.4.0"
  )

Note well that the comment about memory not being cleared is patently wrong
and has been for a while.

If given the go ahead, I can make a PR for this case in two files.

Please advise.

What is really needed is a sweep through all the code looking for @deprecated, hopefully
with a SN version when it was deprecated. I can understand that code deprecated in 0.4.4, 0.4.5, 0.4.next,
might not be a candidate for removal. Certainly it is worth cleaning out items deprecated in earlier versions.
Yes, Java still has entry points deprecated in Java 1.1 & 1.2 and wears them as a badge of honor.

@ekrich
Copy link
Member

ekrich commented Aug 2, 2022

Since we are essentially using Early Semantic versioning, we should be able to remove or change behaviors between 0.4.x and 0.5.0. I think these can be removed for 0.5.0 since everyone will need to rebuild. Looking for other deprecation is a good idea too.

@LeeTibbert
Copy link
Contributor Author

Looking for low hanging fruit whilst I have some intensive tests running.

Not comprehensive, just reporting as I am finding.

nativelib/src/main/scala/scala/scalanative/runtime/Platform.scala has
some '0.4.1'

nativelib has some GC deprecations. A brave soul could examine those.
I am afraid of the Curse of the Mummy.

Javalib has a few deprecations, but then Java likes deprecations.

Posixlib appears to have one "0.4.5" deprecation, according
to the letter it could be removed. Seems better to let it
be until 0.6.0. Other views solicited.

@LeeTibbert
Copy link
Contributor Author

LeeTibbert commented Aug 2, 2022

tools/src/main/scala/scala/scalanative/build/Config.scala:  @deprecated("Not nee
ded: discovery is internal", "0.4.0")
tools/src/main/scala/scala/scalanative/build/Config.scala:  @deprecated("Not nee
ded: discovery is internal", "0.4.0")

2022-11-12: This deprecation seems to be removed. Sometimes the simple passage of time
brings movement in the right direction.

                 Since this is internal code, I see no need to document it in the public facing change.log
                 0.5.0.md.  If anyone thinks differently, please let me know.

@LeeTibbert
Copy link
Contributor Author

LeeTibbert commented Aug 2, 2022

sbt-scala-native/src/main/scala/scala/scalanative/sbtplugin/ScalaNativePlugin.sc
ala:  @deprecated("use autoImport instead", "0.3.7")

Now fixed, see duplicate below. I must really not like this one if I keep
reporting it.

@ekrich
Copy link
Member

ekrich commented Aug 2, 2022

I think the most important thing if we remove deprecated code is that we somehow make sure we capture them with workarounds for the release notes. Maybe we should add release notes or add to them as we go.

@LeeTibbert
Copy link
Contributor Author

I did a quick run through a number of other directories, skipping scalalib. Not pristine but
pleasantly & surprisingly clean.

@LeeTibbert
Copy link
Contributor Author

I think the most important thing if we remove deprecated code is that we somehow make sure we capture them with workarounds for the release notes. Maybe we should add release notes or add to them as we go.

Yes, a section describing removed routines would be essential. The deprecation messages themselves
usually offer workarounds, that could be copied over to the docs.

I, for one, do not want to be creating new workarounds for things which have been deprecated for many versions.
(Could you forecast that I was going to say something like that?)
I almost never want to disappoint customers, but there is always the corner case, held well in reserve.

Much as I would like to pick these of one by one, since this sieving appears to have come up
with only a few, I am thinking one PR, sometime when CI is particularly slow.

@ekrich
Copy link
Member

ekrich commented Aug 2, 2022

You can copy 0.4.0.md since that was the last breaking version to 0.5.0.md and start filling that out with details of the removals. That would give us a head start.

Edit: It is not really fair to make only one person do this for a major release so this will help and start a new good process.

@LeeTibbert
Copy link
Contributor Author

LeeTibbert commented Aug 11, 2022

2022-11-19 WIP PR #2783 contained a fix for this but never merged. So this deprecation still exists.

Another one for the hit parade:
ScalaNativePlugin.scala

  @deprecated("use autoImport instead", "0.3.7")
  val AutoImport = autoImport

I began the procees of removing long standing deprecations in
PR #2783.

I followed the Eric's sage suggestion to create a rough 0.5.0-SNAPSHOT.md
My intent is to make entries in that file, and here, as I remove no longer
relevant deprecations. I hope that others will do otherwise as they
encounter candidates, or at least log them here.

Fixing these is sort of like eating popcorn.

@LeeTibbert LeeTibbert changed the title Two deprecated methods in UnsafePackageCompat.scala should be removed in 0.5.0 Many deprecated methods should be removed in 0.5.0 Aug 11, 2022
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this issue Aug 11, 2022
@ekrich ekrich added this to the 0.5.0 milestone Aug 12, 2022
@LeeTibbert
Copy link
Contributor Author

Trying to keep accurate score.

PR #2988 removed two deprecated declarations in libc.signal. PR #2991 updates the changelog.
I also added the "Suggested replacement" from this discussion. Kinder than just "you're hosed!".

LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this issue Nov 19, 2022
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this issue Nov 19, 2022
@LeeTibbert LeeTibbert reopened this Nov 23, 2022
@LeeTibbert
Copy link
Contributor Author

I am still pecking away at removing deprecated items. I think I have 5+ files to go.

Someday this Issue can Rest In Peace.

LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this issue Dec 3, 2022
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this issue Dec 6, 2022
WojciechMazur pushed a commit that referenced this issue Dec 13, 2022
* Partial fix #2765: Another tranche of stale deprecation removals

* Merge this doc into PR #3003 mainline changes
LeeTibbert added a commit to LeeTibbert/scala-native-fork that referenced this issue Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants