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

Blog post about given prioritization changes #1675

Merged
merged 13 commits into from
Aug 20, 2024
Merged

Conversation

bracevac
Copy link
Contributor

@bracevac bracevac commented Aug 1, 2024

No description provided.

* Clarify compiler behavior and compiler options
* Add discussion on changes to context bounds
// Scala 3.5: prints "library-defined"
```

Scala 3.5 will automatically issue

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to turn off these warnings with a flag or is the only way through @noWarm or -source:3.6?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ask because this breaks builds with fatal warnings.

Copy link
Contributor Author

@bracevac bracevac Aug 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is currently no flag to suppress those specific warnings and you'd have to use @nowarn or -source:3.6. As you point out. I've added that to the blog post.

@bracevac
Copy link
Contributor Author

bracevac commented Aug 5, 2024

Making this a draft for now, as we decided to delay the rollout of the new prioritization scheme.

@lrytz
Copy link
Member

lrytz commented Aug 6, 2024

It's good that the motivating example is something relevant in practice. However, starting the blog with Functor/Monad/Traverse could be overwhelming to Scala programmers who aren't using pure FP style / libraries. When they come across the new warning they would hopefully end up at the blog post. It might be good to put the simpler example (LibComponent) first.

@bracevac bracevac marked this pull request as ready for review August 16, 2024 22:19
Copy link
Contributor

@WojciechMazur WojciechMazur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general looks good to me. I've found some typos and have idea how we can slightly change structure of the text to make it a bit more positive upfront

_posts/2024-08-19-given-priority-change-3.7.md Outdated Show resolved Hide resolved
_posts/2024-08-19-given-priority-change-3.7.md Outdated Show resolved Hide resolved
_most general_ subtype instead (`libComponent`).


### Motivation: Better Handling of Inheritance Triangles & Typeclasses
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might switch the order of Motivation and New Prioritization of Givens.
By doing so we're starting with the overall language improvement (positive vibes), then pointing out the possible drawbacks (negativity), followed by migration guides (neutral, slightly positive).
So the overall structure might look like:

# New Prioritization of Givens in Scala 3.7 (intro)

Scala 3.7 will introduce changes to how `givens` are resolved, which can
affect program behaviour when multiple givens are present. 
The goal of this change is to make a givens resolution more predictable, however, it can lead to problems when migrating to Scala 3.7 or later. 
In this article we'd like to explain the motivation, possible issues and migration guides to prepare developers for the upcoming changes.
...

### Motivation 
Why change the priority to the `given` .... 

### Source incompatibility of the new givens prioritization 
Unfortunately, this change might affect source compatibility of some Scala codebases. Let's consider a library that provides a default
... <here example from the 1st paragraph>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

_most general_ subtype instead (`libComponent`).


### Motivation: Better Handling of Inheritance Triangles & Typeclasses
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most of Scala documentation we're using Type Class in opposition to Typeclass that seems to be used by Haskell community

Suggested change
### Motivation: Better Handling of Inheritance Triangles & Typeclasses
### Motivation: Better Handling of Inheritance Triangles & Type classes

There's more of typeclass usages in the text

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type class is also the correct terminology here as in typeclassopedia:

The original spelling of “type class” is with two words, as evidenced by, for example, the Haskell 2010 Language Report, early papers on type classes like Type classes in Haskell and Type classes: an exploration of the design space, and Hudak et al.’s history of Haskell. However, as often happens with two-word phrases that see a lot of use, it has started to show up as one word (“typeclass”) or, rarely, hyphenated (“type-class”).

Copy link
Member

@SethTisue SethTisue Aug 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw (and at the risk of igniting a full-on holy war 😁), in Scala contexts I think "typeclass" is preferable because our typeclasses aren't classes. Haskell doesn't have classes so there is no confusion in that context but in ours there is

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my boss uses ‘typeclass’, who am I to split words? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel too strongly about it, though I tend to agree with @SethTisue here. But if there's a majority that wants to have it changed to "Type Classes", I'll bend the knee.

Scala 3.7 will finalize the transition, making the new given
prioritization the standard behavior.

#### Suppressing Warnings
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can consider adding a paragraph here that suppressing warnings is only a temporary solution. Eg.

Consider that suppressing warnings should become only a temporary workaround. 
They can be used to resolve rare false-positive warnings produced by the compiler. 
However, they're only delaying the time when changes in either your codebase or libraries used by your project are required. Suppressing warnings can lead to unexpected problems when upgrading to the future versions of the Scala compiler.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Open with the motivation first, then
with the negative part.
(`userComponent`). However, in Scala 3.7, it selects the value with the
_most general_ subtype instead (`libComponent`).

Here's the revised version with improved transitions and adjustments for the reordering:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The revised version looks great, let's use it. When it's done I think we're ready to publish it.

@WojciechMazur
Copy link
Contributor

@SethTisue Seems like this one is ready to go. I don't have write access to proceed with merge. Can you please take care of it?

@adpi2 adpi2 merged commit decdf22 into scala:main Aug 20, 2024
1 check passed
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

Successfully merging this pull request may close these issues.

8 participants