Skip to content

Javaparser #183

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

Closed
wants to merge 15 commits into from
Closed

Javaparser #183

wants to merge 15 commits into from

Conversation

olhotak
Copy link
Contributor

@olhotak olhotak commented Oct 10, 2014

Java parser for dotty.

As we discussed, three tests are still failing:

  • t2409:
    Requires import of companion object before first constructor.
  • t1751 and t294:
    Tree transformer does not transform repeated method arguments within an annotation instantiation.

Review by @odersky @DarkDimius

val vparams = formalParams()
if (!isVoid) rtpt = optArrayBrackets(rtpt)
optThrows()
val bodyOk = !inInterface || (mods is Flags.DefaultMethod)
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you didn't pick up scala/scala@17a1abb.

It would be really helpful to record the SHA of scala/scala from which you ported this, so that in the future merging is possible. It would also be great to discuss if this is a straight port, if if you had to make any interesting changes to fit into dotc.

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 ported from scala/scala@9753f23, which was before scala/scala@17a1abb.

I have updated the first commit message with the SHA and some discussion of the changes from scalac. Only very minor changes were needed.

@DarkDimius
Copy link
Contributor

for t1751 and t294 types in tree are correct(and thus were correctly tranformed).
But types created while retyping in Ychech are incorrect.

@olhotak olhotak force-pushed the javaparser branch 2 times, most recently from 7866919 to 8e28653 Compare October 10, 2014 18:52
olhotak referenced this pull request in dotty-staging/dotty Oct 10, 2014
@olhotak
Copy link
Contributor Author

olhotak commented Oct 27, 2014

I tried cherry-picking 1b79def from #184, but it did not fix t1751 and t294.

I investigated the behaviour of elimrepeated in the debugger. It appears that it does actually eliminate repeated parameters correctly, even though they are inside an annotation tree.

However, after the method type with eliminated parameters has been created, it gets thrown out by a call to cur.current in Denotations.scala. Could this be an issue with the lifetime of the transformed method info? I think I will need help in person to debug this.

Ported from scalac 2.11.x branch SHA 9753f23f9362b25a9f481b11dd8d51187187882a

This is mostly a direct port, with few significant dotty-specific
changes needed. The two more significant changes are:

In dotty, the first constructor of a class is pulled out separately from
the other stats in the Template.

The keyword detection code (buildKeywordArray) was moved into Tokens so
that it can more cleanly be shared by the Scala and Java scanners.
A Java constructor needs to see the import of the companion object of the class. It is not necessary to move to an outer context because a Java constructor does not have an implementation. scalac also does it this way: see Namers.Namer.createNamer.isConstrParam.
The dummy constructor is needed so that the real constructors see the import of the companion object.

The constructor has a parameter of type Unit so that no Java code can call it.
transformSym explicitly checks that a field is JavaDefined and does not create a symbol for it.

Creation of a setter body looks for the symbol and fails because it does not find it.

We do not need setter bodies for Java fields because we are not generating bytecode for them.
@olhotak
Copy link
Contributor Author

olhotak commented Oct 30, 2014

I have rebased to dotty master and fixed most of the resulting issues.

I have fixed the problem with Java constructors not seeing the import of the companion object.

The two elimrepeated tests are still failing.

Newly failing is t1782. It contains a NamedArg in an annotation declaration. FirstTransform does not transform it because it is in an annotation, then fails an assertion because the NamedArg still exists.

@DarkDimius DarkDimius closed this Dec 3, 2014
WojciechMazur pushed a commit to WojciechMazur/dotty that referenced this pull request Mar 19, 2025
Backport "Undo patch of double-block apply" to 3.3 LTS
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