Skip to content

Update error msg for Parsers.scala:1738 and 1739 #1629

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

Merged
merged 1 commit into from
Oct 31, 2016

Conversation

ljdelight
Copy link
Contributor

These error messages are for aux ctors needing non-implicit params. I'm testing the error messages with this code

class Square(val len: Int) {
  // typical cases:
  //  (1) this() parens are forgotten, (2) argument set as implicit
  def this(implicit width: Double) = this(0)
  def this = { this(4) }
}

class MyList(val s: String) {
  def this

@sebastianharko
Copy link

sebastianharko commented Oct 26, 2016

@ljdelight Awesome 👍 - quick tip: you can make the 'error printer' (of dotc or dotr) syntax highlight the example code (the Person class): see previous PRs related to errors :-)

@ljdelight
Copy link
Contributor Author

Neat! I had tried to get syntax highlighting using those inline val explanation = "foobar code is "${def func()}", but it would only work with single-line insertions. I didn't realize the use of other variables and inserting those had a change on behavior, but it works great using similar to

val code = "class Person ..."
val explanation = "text ... ${code}"

I'm not sure how it works, and thanks for the tip

|
|Here's an example class with primary and auxiliary constructors:
|
|$code""".stripMargin
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps the reason as to why somebody would create an auxiliary constructor with an implicit parameter is because they get the error: could not find implicit value for parameter X: Type.

It might be worth mentioning that with auxiliary constructors calling a primary constructor that has an implicit argslist, the implicit value has to be explicitly specified at the callsite.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great feedback, I'll get this done today :D

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'm a bit over my head (learning scala from coursera, on week 7) with creating an example for the other case. I've been looking at math.Ordering and I cannot create a simple example; and my example in this PR actually does against the general consensus of not using basic types in implicit. Do you think I should remove the example and not have one? I'm considering adding more text to the explanation to help with the case of an implicit primary constructor, thoughts @felixmulder?

Copy link
Contributor

@felixmulder felixmulder Oct 31, 2016

Choose a reason for hiding this comment

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

For the sake of progress - perhaps omit the implicit example? This can always be improved later on :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the text 👍

Copy link
Contributor

@felixmulder felixmulder left a comment

Choose a reason for hiding this comment

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

Basically I think this looks good, but perhaps provide an explanation and way to use aux constructors if the user has an implicit argument list in the main constructor.

@felixmulder felixmulder added the area:reporting Error reporting including formatting, implicit suggestions, etc label Oct 28, 2016
These error messages are for aux ctors needing non-implicit params. I'm testing the error messages with this code

```
class Square(val len: Int) {
  // typical cases:
  //  (1) this() parens are forgotten, (2) argument set as implicit
  def this(implicit width: Double) = this(0)
  def this = { this(4) }
}

class MyList(val s: String) {
  def this
```
@felixmulder felixmulder merged commit 01ae7dd into scala:master Oct 31, 2016
@felixmulder
Copy link
Contributor

Thanks @ljdelight! 🎉

@ljdelight ljdelight deleted the errorMessages branch October 31, 2016 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants