8000 Address SI-9675 Add warning about non-sensible equals in anonymous functions by teldosas · Pull Request #5662 · scala/scala · GitHub
[go: up one dir, main page]
Skip to content

Address SI-9675 Add warning about non-sensible equals in anonymous functions #5662

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 16, 2017
Merged

Address SI-9675 Add warning about non-sensible equals in anonymous functions #5662

merged 1 commit into from
Feb 16, 2017

Conversation

teldosas
Copy link
Contributor

The problem was that in an attempt to address an issue with #4473, warnings about non-sensible equals were removed for synthetic entities. But an anonymous function is also a synthetic entity, so warnings were removed for these too by mistake.

@scala-jenkins scala-jenkins added this to the 2.12.2 milestone Jan 26, 2017
@teldosas
Copy link
Contributor Author

I signed the CLA

@adriaanm adriaanm added the welcome hello new contributor! label Jan 26, 2017
@teldosas
Copy link
Contributor Author

review @adriaanm
(Is this how I'm supposed to ask for review?)

@SethTisue SethTisue requested a review from adriaanm January 29, 2017 16:42
@adriaanm
Copy link
Contributor
adriaanm commented Jan 31, 2017 •
edited
Loading

Good catch! Looks like a good example of where we could sharpen the distinction between artifact and synthetic.

adriaanm
adriaanm suggested changes Jan 31, 2017
View reviewed changes
Copy link
Contributor
@adriaanm adriaanm left a comment

Choose a reason for hiding this comment

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

The fix looks good, but the test could be rewritten as a neg/ test instead of a run/ one, right? You'll need to create a neg/t9675.flags file that contains -Xfatal-warnings, but that should be it.

@teldosas
Copy link
Contributor Author

@adriaanm But that does not guarantee that we get a warning for every single function. It would be enough to get a warning for a single function for it to succeed.

@adriaanm
Copy link
Contributor
adriaanm commented Jan 31, 2017 via email

@teldosas
Copy link
Contributor Author
teldosas commented Jan 31, 2017 •
8000 edited
Loading

Ok I made the changes. Now it is more clear to me what "run" means. Thanks @adriaanm

@adriaanm
Copy link
Contributor
adriaanm commented Jan 31, 2017 via email

@adriaanm
Copy link
Contributor

How is this test related to this PR?

[[syntax trees at end of                    parser]] // client_2.scala
package <empty> {
  object Test extends App {
    def <init>() = {
      super.<init>();
      ()
    };
    val wit = Random.foo("1");
    var t = 1;
    var one: wit.T = 1
  }
}

[[syntax trees at end of                     namer]] // client_2.scala: tree is unchanged since parser
!!  409 - run/random                                [compilation failed]

@teldosas
Copy link
Contributor Author

@adriaanm it doesn't it's about another bug that I was working on, I pushed it by mistake. I will remove it

@teldosas
Copy link
Contributor Author
teldosas commented Feb 1, 2017

@adriaanm I removed it. It should be fine now

This was referenced Feb 9, 2017
Closed
Merged
@adriaanm adriaanm merged commit e62e9e2 into scala:2.12.x Feb 16, 2017
@adriaanm
Copy link
Contributor

Thanks, @teldosas! For your next fix, please use a commit title more like "SI-9675 warn about non-sensible equals in anonymous functions" (start with ticket number, use active voice)

@som-snytt
Copy link
Contributor

There was no adequate emoji to say, "oh, artifact vs synthetic, I was just wondering about that."

@som-snytt som-snytt self-assigned this Feb 16, 2017
@SethTisue SethTisue mentioned this pull request Feb 18, 2017
Merged
@teldosas teldosas deleted the SI-9675 branch February 18, 2017 14:59
@scabug scabug mentioned this pull request Apr 7, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriaanm adriaanm adriaanm approved these changes

Assignees

@som-snytt som-snytt

Labels
welcome hello new contributor!
Projects
None yet
Milestone
2.12.2
Development

Successfully merging this pull request may close these issues.
4 participants
@teldosas @adriaanm @som-snytt @scala-jenkins
0