Skip to content

Fixes #6626. Pass sym.name to sanitizeName instead of sym.name.asSimpleName #7290

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
Sep 26, 2019

Conversation

ashwinbhaskar
Copy link
Contributor

@ashwinbhaskar ashwinbhaskar commented Sep 23, 2019

asSimpleName method on DerivedName is implemented to throw UnspportedOperationException. The class GenericSignatures's method classSig calls asSimpleName on instances of Name without checking on the actual type (DerivedName extends TermName which extends Name).
~~A check has been added to refrain calling asSimpleName if the type is DerivedName. ~~
As @smarter suggested, refrained from calling asSimpleName for all the cases because mangledString takes care of giving the name of the class in byte code and it is already being called by sanitizeName.

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 ("Add" instead of "Added")
  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! ☀️

@ashwinbhaskar ashwinbhaskar force-pushed the compiler_crash_fix branch 3 times, most recently from acad425 to 4bcadd2 Compare September 23, 2019 06:23
@ashwinbhaskar
Copy link
Contributor Author

Line 170: should be "name.asSimpleName" instead of "sym.name.asSimpleName".
PS. Coding style: remove space after "Unit" (line 167) and space before ":" (4 times).

done

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

See comment.

@ashwinbhaskar ashwinbhaskar changed the title Fixes #6626. Refrain from calling asSimpleName method on DerivedName Fixes #6626. Pass sym.name to sanitizeName instead of sym.name.asSimpleName Sep 23, 2019
@ashwinbhaskar ashwinbhaskar changed the title Fixes #6626. Pass sym.name to sanitizeName instead of sym.name.asSimpleName Fixes #6626. Pass sym.name to sanitizeName instead of sym.name.asSimpleName Sep 23, 2019
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

This PR is missing a test.

@ashwinbhaskar
Copy link
Contributor Author

ashwinbhaskar commented Sep 24, 2019

This PR is missing a test.

@smarter added test

@ashwinbhaskar
Copy link
Contributor Author

@smarter can this be merged now?

@smarter
Copy link
Member

smarter commented Sep 25, 2019

If you look at other tests in generic-java-signatures like https://github.com/lampepfl/dotty/blob/master/tests/generic-java-signatures/andTypes.scala you'll see that they have a main method which prints the generic type signature of members, the output of this main method must match the corresponding check file https://github.com/lampepfl/dotty/blob/master/tests/generic-java-signatures/andTypes.check. Your test should be enhanced to contain a similar main method (you can just copy paste the one from andTypes.scala) and a check file to make sure that we output the correct signature for a

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Thank you!

@smarter smarter merged commit b63c3e5 into scala:master Sep 26, 2019
@ashwinbhaskar ashwinbhaskar deleted the compiler_crash_fix branch October 21, 2019 04:23
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