Skip to content

Add test for java.lang.String#intern()#135

Merged
chitoku-k merged 11 commits into
masterfrom
string/intern
May 10, 2019
Merged

Add test for java.lang.String#intern()#135
chitoku-k merged 11 commits into
masterfrom
string/intern

Conversation

@chitoku-k
Copy link
Copy Markdown
Member

The implementation is currently under the progress and this PR aims to add it along with the test.

See also: #120

@m3m0r7
Copy link
Copy Markdown
Member

m3m0r7 commented May 7, 2019

I'll change features as following.

  • Allow to overwrite the constant pool.

Is it correct?

@m3m0r7
Copy link
Copy Markdown
Member

m3m0r7 commented May 7, 2019

@chitoku-k

@chitoku-k
Copy link
Copy Markdown
Member Author

@memory-agape Probably that should make sense. For clarification,

  1. Allow String#intern() to overwrite the constant pool.
  2. Any literals that appear after the write operation will refer to (1).

And, please have a look at #120 as well.

@m3m0r7
Copy link
Copy Markdown
Member

m3m0r7 commented May 7, 2019

@chitoku-k
Thanks 👍
I'll try to implement.

@m3m0r7
Copy link
Copy Markdown
Member

m3m0r7 commented May 9, 2019

@chitoku-k
You can add new tests.

@chitoku-k
Copy link
Copy Markdown
Member Author

chitoku-k commented May 9, 2019

OK, I found a ridiculous edge case that isn't covered.

Whereas the following prints the expected result,

String te = "te";
String st = "st";
String test = te + st;

System.out.println(System.identityHashCode(test)); // => 356573597

test.intern();
System.out.println(System.identityHashCode("test")); // => 356573597

the interning does not happen in JVM when a literal has already appeared in that scope:

String te = "te";
String st = "st";
String test = te + st;

System.out.println(System.identityHashCode(test))  // => 356573597

// This line is added to make a literal appear before interned
String dummy = "test";

test.intern();
System.out.println(System.identityHashCode("test")); // => JVM: 1735600054, PHPJava: 356573597

Adding the failing test...

@m3m0r7
Copy link
Copy Markdown
Member

m3m0r7 commented May 9, 2019

Very sad (T_T)

Add FreezableInterface to manage the _Utf8
@chitoku-k
Copy link
Copy Markdown
Member Author

I think this PR finally got ready for reviewing!

@chitoku-k chitoku-k marked this pull request as ready for review May 10, 2019 03:50
@chitoku-k chitoku-k requested a review from m3m0r7 May 10, 2019 03:50
@m3m0r7
Copy link
Copy Markdown
Member

m3m0r7 commented May 10, 2019

@chitoku-k
Looks good to me!
Please merge this PR after fixing conflicted files.

@chitoku-k chitoku-k merged commit 3e5102b into master May 10, 2019
@chitoku-k chitoku-k deleted the string/intern branch May 10, 2019 08:12
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.

2 participants