8264216: [lworld] unknown.Class.default gives misleading compilation error #372

jespersm · 2021-03-26T07:49:37Z

Make the compilation error for somepkgname.UnknownClass.default as informative as UnknownClass.default.

Progress

Change must not contain extraneous whitespace

Issue

JDK-8264216: [lworld] unknown.Class.default gives misleading compilation error

Reviewers

Srikanth Adayapalam (@sadayapalam - Committer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/valhalla pull/372/head:pull/372
$ git checkout pull/372

Update a local copy of the PR:
$ git checkout pull/372
$ git pull https://git.openjdk.java.net/valhalla pull/372/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 372

View PR using the GUI difftool:
$ git pr show -t 372

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/valhalla/pull/372.diff

bridgekeeper · 2021-03-26T07:50:12Z

👋 Welcome back jespersm! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2021-03-26T07:56:55Z

@jespersm This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8264216: [lworld] unknown.Class.default gives misleading compilation error

Reviewed-by: sadayapalam

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 2 new commits pushed to the lworld branch:

83d849f: 8263441: [lworld] TestUnloadedInlineTypeField fails due to unexpected compilation level
226abe2: 8262831: [lworld] C2 compilation fails with assert "should be addp but is Phi"

Please see this link for an up-to-date comparison between the source branch of this pull request and the lworld branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@sadayapalam) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

mlbridge · 2021-03-26T08:00:50Z

Webrevs

01: Full - Incremental (94e65ef6)
00: Full (4a9c011)

jespersm · 2021-03-26T09:33:11Z

/integrate

openjdk · 2021-03-26T09:33:55Z

@jespersm
Your change (at version 4a9c011) is now ready to be sponsored by a Committer.

sadayapalam · 2021-03-29T04:19:37Z

Thanks Jesper, I offer to be the reviewer and sponsor for this. Will study this later today and share any comments.

sadayapalam

Please fix the issues mentioned. Also confirm whether you have run all the tests.

sadayapalam · 2021-03-31T09:39:10Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java

@@ -38,6 +38,7 @@
 import com.sun.source.tree.TreeVisitor;
 import com.sun.source.util.SimpleTreeVisitor;
 import com.sun.tools.javac.code.*;
+import com.sun.tools.javac.code.Kinds.KindName;


This import looks unused ??

Ooopsie, I'll fix that.

sadayapalam · 2021-03-31T09:39:10Z

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java

+                    log.error(tree.pos, Errors.CantResolveLocation(Kinds.KindName.CLASS, site.tsym.getQualifiedName(), null, null,
+                            Fragments.Location(Kinds.typeKindName(env.enclClass.type), env.enclClass.type, null)));
+                    yield syms.errSymbol;
+                }


Looks ok, inventing a new error message altogether (such as default cannot be applied to a package) looks too heavy a hammer.

test/langtools/tools/javac/valhalla/lworld-values/UnknownTypeDefault.out

sadayapalam

Changes look good. Thanks Jesper!

sadayapalam · 2021-04-01T07:50:33Z

/sponsor

openjdk · 2021-04-01T07:51:40Z

@sadayapalam The PR has been updated since the change author (@jespersm) issued the integrate command - the author must perform this command again.

jespersm · 2021-04-01T08:14:00Z

/integrate

openjdk · 2021-04-01T08:15:04Z

@jespersm
Your change (at version 94e65ef) is now ready to be sponsored by a Committer.

sadayapalam · 2021-04-01T08:26:33Z

/sponsor

openjdk · 2021-04-01T08:28:05Z

@sadayapalam @jespersm Since your change was applied there have been 2 commits pushed to the lworld branch:

83d849f: 8263441: [lworld] TestUnloadedInlineTypeField fails due to unexpected compilation level
226abe2: 8262831: [lworld] C2 compilation fails with assert "should be addp but is Phi"

Your commit was automatically rebased without conflicts.

Pushed as commit 28a4ec0.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

8264216: unknown.Class.default gives misleading compilation error

fcc25e8

jespersm changed the title ~~8264216: unknown.Class.default gives misleading compilation error~~ 8264216: [lworld] unknown.Class.default gives misleading compilation error Mar 26, 2021

Tabs -> spaces

4a9c011

openjdk bot added ready rfr labels Mar 26, 2021

openjdk bot added the sponsor label Mar 26, 2021

sadayapalam suggested changes Mar 31, 2021

View changes

Address review comments

94e65ef

openjdk bot removed the sponsor label Mar 31, 2021

sadayapalam approved these changes Apr 1, 2021

View changes

openjdk bot added the sponsor label Apr 1, 2021

openjdk bot closed this Apr 1, 2021

openjdk bot added integrated and removed sponsor ready rfr labels Apr 1, 2021

8264216: [lworld] unknown.Class.default gives misleading compilation error #372

8264216: [lworld] unknown.Class.default gives misleading compilation error #372

jespersm commented Mar 26, 2021 •

edited by openjdk[bot] bot

bridgekeeper bot commented Mar 26, 2021

openjdk bot commented Mar 26, 2021 •

edited

mlbridge bot commented Mar 26, 2021 •

edited

jespersm commented Mar 26, 2021

openjdk bot commented Mar 26, 2021

sadayapalam commented Mar 29, 2021

sadayapalam left a comment

sadayapalam Mar 31, 2021

jespersm Mar 31, 2021

sadayapalam Mar 31, 2021

sadayapalam left a comment

sadayapalam commented Apr 1, 2021

openjdk bot commented Apr 1, 2021

jespersm commented Apr 1, 2021

openjdk bot commented Apr 1, 2021

sadayapalam commented Apr 1, 2021

openjdk bot commented Apr 1, 2021

8264216: [lworld] unknown.Class.default gives misleading compilation error #372

8264216: [lworld] unknown.Class.default gives misleading compilation error #372

Conversation

jespersm commented Mar 26, 2021 • edited by openjdk[bot] bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Mar 26, 2021

openjdk bot commented Mar 26, 2021 • edited

mlbridge bot commented Mar 26, 2021 • edited

Webrevs

jespersm commented Mar 26, 2021

openjdk bot commented Mar 26, 2021

sadayapalam commented Mar 29, 2021

sadayapalam left a comment

sadayapalam Mar 31, 2021

Choose a reason for hiding this comment

jespersm Mar 31, 2021

Choose a reason for hiding this comment

sadayapalam Mar 31, 2021

Choose a reason for hiding this comment

sadayapalam left a comment

sadayapalam commented Apr 1, 2021

openjdk bot commented Apr 1, 2021

jespersm commented Apr 1, 2021

openjdk bot commented Apr 1, 2021

sadayapalam commented Apr 1, 2021

openjdk bot commented Apr 1, 2021

jespersm commented Mar 26, 2021 •

edited by openjdk[bot] bot

openjdk bot commented Mar 26, 2021 •

edited

mlbridge bot commented Mar 26, 2021 •

edited