RFR: 8173898: StackWalker.walk throws InternalError if called from a constructor invoked through reflection.

[Mandy Chung]

> On Feb 8, 2017, at 3:36 AM, Daniel Fuchs <daniel.fuchs at oracle.com> wrote:
> 
> Hi Mandy,
> 
> Thanks for all the suggestions!
> 
> On 07/02/17 20:03, Mandy Chung wrote:
>>> Please find below a new webrev that incorporates your feedback.
>>> http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.03/
>>> 
>> 
>> I think this sentence is not needed.
>> 216          * <p>A {@code StackWalker} with this {@code SHOW_REFLECT_FRAMES} option
>> 217          * will show all reflection frames.
> 
> I disagree: the text starts by saying:
> "By default reflection frames are hidden. The reflection
> frames include […]"

209          * Shows all reflection frames.

This is the first sentence.  line 216-217 repeats line 209.

> but the sentence at lines 216-217 is the only place where we say what
> the option actually does: it shows all reflection frames.
> I think it's needed.
> Possibly the sentence at lines 216-217 could be moved to become
> the first sentence in the doc comment, but I'd rather not change
> it now that the CCC has approved the doc changes as in webrev.03.

This does not change the spec and this is minor rewording.  No need to update CCC.

What about:

Shows all reflection frames.

<p>By default, reflection frames are hidden.  A {@code StackWalker} with this {@code SHOW_REFLECT_FRAMES} option will show the reflection frames that include ….


> 
> 
>> Nit: what about renaming SimpleObj to ConstructorNewInstance?
>> SimpleObj::found should probably name as `framesInUnnamedOrReflectionPackage`.
> 
> SimpleObj => ConstructorNewInstance
> found => testFramesOrReflectionFrames
>         (frames in the unnamed package are test frames)
> 
> I added some comments as well in the accept() method.

That’s good.

> 
>> Nit: ReflectionFrames.java test - the test is a bit hard maybe SimpleObj can be renamed to TestHelper and `found` be `result`.   Just trying to make the test easier to read and understand.  You may have better idea.
> 
> SimpleObj => StackInspector (it calls walk & getCallerClass
>             to inspect the stack from its constructor)
> found => collectedFrames
> 
> Also added some comments to better explain the purposes of
> StackInspector and StackInspector.Caller.
> 
> Here is the new webrev:
> http://cr.openjdk.java.net/~dfuchs/webrev_8173898/webrev.04/
> 

Thanks for updating the test.

Mandy