On Thu, 10 Sep 2020 03:18:23 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> This is a rather large but generally simple cleanup.
>>
>> We use a lot of raw C-style casts to "(JavaThread*)" typically after
>> checking "thread->is_Java_thread()". To simplify
>> this pattern, and make the code look somewhat cleaner we introduce a
>> convenience function Thread::as_Java_thread(),
>> which asserts is_Java_thread() and returns "this" cast to "JavaThread*". A
>> const version, as_const_Java_thread(), was
>> also added to allow use in cases where we start with "const Thread *".
>> Summary of changes:
>> - changed raw casts to use as_Java_thread()
>> - removed redundant checks of is_Java_thread() as it is now done in
>> as_Java_thread()
>> - Removed checks that are checking the type system e.g.
>> void foo(JavaThread* t) {
>> assert(t->is_Java_thread(), "must be")
>> the only way the assert can fire is if someone deliberately bypasses the
>> type system, and if we are going to worry
>> about that then we would need asserts like this on every method that expects
>> a JavaThread! The right place for the
>> assertion is at the head of any such call chain.
>> - Removed asserts that are already guaranteed in the caller.
>> - Use JavaThread::current() where appropriate.
>> - Use pre-existing "thread" variable where available rather than casting
>> THREAD
>>
>> Change locations found by grepping for variations of the cast syntax
>> "(JavaThread*)" - it is possible some may have
>> been missed.
>> Testing: tiers 1-3
>>
>> Thanks,
>> David
>
> David Holmes has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Fixed serious bug in dependencies.cpp. The v1 change was wrong and got
> restored but
> not commited, then a later rollback to v1 lost the fix.
>
> Minor issues Dan spotted now fixed.
Marked as reviewed by kbarrett (Reviewer).
-------------
PR: https://git.openjdk.java.net/jdk/pull/37
