Skip to content

Conversation

@vitorsvieira
Copy link
Contributor

Regarding #1589, this PR addresses the following error:

  • "Bad symbolic reference. A signature..." - SymDenotations.scala:1964

Copy link
Member

@dottybot dottybot left a comment

Choose a reason for hiding this comment

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

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Commit Messages

We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).

Please stick to these guidelines for commit messages:

  1. Separate subject from body with a blank line
  2. When fixing an issue, start your commit message with Fix #<ISSUE-NBR>:
  3. Limit the subject line to 72 characters
  4. Capitalize the subject line
  5. Do not end the subject line with a period
  6. Use the imperative mood in the subject line ("Added" instead of "Add")
  7. Wrap the body at 80 characters
  8. Use the body to explain what and why vs. how

adapted from https://chris.beams.io/posts/git-commit

Have an awesome day! ☀️

|the classpath might be incompatible with the version used when compiling $src."""
ctx.error(errMsg)
if (ctx.debug) throw new Error()
val errMsg = BadSymbolicReference(location, name, denot.owner, src)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be a def

name: String,
denotationOwner: Symbol,
src: String
)(implicit ctx: Context) extends Message(BadSymbolicReferenceID) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would change BadSymbolicReference so that it only has denot: SymDenotation. You can derive location, name, owner and src from denot.

import config.Config
import config.Printers.{incremental, noPrinter}
import reporting.diagnostic.Message
import reporting.diagnostic.messages._
Copy link
Contributor

Choose a reason for hiding this comment

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

Only import what you need

private val (location, src) =
if (file != null) (s" in $file", file.toString)
else ("", "the signature")
private val name = ctx.fresh.setSetting(ctx.settings.YdebugNames, true).nameString(denot.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

We usually don't create fields to only store temporary values. If you need to precompute something for msg , you can use a block:

val msg = {
  val tmp1 = ...
  val tmp2 = ...
  doSomethingWith(tmp1, tmp2)
}

If a temporary computation is shared like name, we usually make it a def. We might want to create a field if we don't want to perform a computation multiple times. In this case I guess it make sense to keep name a field since ctx.fresh... may be expensive.

val msg =
hl"""Bad symbolic reference. A signature$location
|refers to $name in ${denotationOwner.showKind} ${denotationOwner.showFullName} which is not available.
|refers to $name in ${denot.owner.showKind} ${denot.owner.showFullName} which is not available.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here you can also store the result of owner in a temporary val within the block

@allanrenucci allanrenucci self-assigned this Dec 11, 2017
@allanrenucci allanrenucci merged commit 33ebbb8 into scala:master Dec 11, 2017
@vitorsvieira vitorsvieira deleted the wip/1593-bad-symbolic-reference branch December 11, 2017 17:37
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.

3 participants