Small convenience improvements for Instrumentation API #24
Conversation
This is a concenience method to correct the tags of a source section at a later point, for instance during paring. In my handwritten recursive descent parser, I don't know the complete context during parsing (i could pass it in, but that would make things more complicated), so, I would like to be able to change the tags of a subexpression when I am back in the parent expression where I assemble the AST. Signed-off-by: Stefan Marr <git@stefan-marr.de>
| (identifier != null ? " identifier=" + identifier : "") + " code=" + getCode(); | ||
| } | ||
| if (tags != null && tags.length > 0) { | ||
| result = " tags: " + Arrays.toString(tags); |
There was a problem hiding this comment.
How did that ever work? hm, probably it didn't. yes, you are right. will fix.
Show the tags in the string representation, helpful for debugging. Signed-off-by: Stefan Marr <git@stefan-marr.de>
smarr
force-pushed
the
feature/instrumentation_api
branch
from
6694614 to
3a51395
Compare
|
I think this needs more discussion before merging.
For the last point, this is something I wonder, but doesn't seem to make a lot of sense. Unique source sections, i.e., only one object and one set of tags for a give range. This seems problematic because it might be language dependent or specific to parser/AST structure. Also overlapping source sections are easily constructed, also for identical range. Would be hard to guarantee uniqueness. |
| @@ -292,6 +299,16 @@ public boolean equals(Object obj) { | |||
| } | |||
|
|
|||
| /** | |||
No. SourceSection must be immutable. It is used in HashMaps as key a lot. Since nodes are mutable already its not a big deal if Node#sourceSection is mutable instead.
Unique for a particular Source, or globally unique? I don't think we need that. A proper Equality check is good enough I believe. |
|
@chumer you mentioned using SourceSection in HashMaps, but haven't responded to my question in the instrumentation review about how tags affect equality. The current overrides of |
|
For my use cases, I actually added tags to the hash ( With respect to the 'uniqueness' discussion above, I feel that makes most sense. |
|
Might adding tags to the hash make I continue to see evidence that forcing storage of tags into the |
|
Hm, source section querying? Do you have an example for what that is used? Building the DynamicMetrics tool, I actually found a couple of things that surprised me. For instance, I annotated message sends, and I annotated field reads. In Newspeak (SOMns) there is however no separate syntax for field reads. So, I end up with two different source sections for the same piece of source code. One annotated as message send, and one annotated as field read. In the tool that actually works nicely. I render field reads as blue, and sends as italic. That combines nicely. And, as a reader of this highlighted code, I get a lot of information. However, some might say, I abuse the instrumentation framework by exposing such dynamic information. But I tend to disagree. It works very nicely for me. But back to the question, in such a scenario, what would you be querying for, and which of the source sections would you want? or perhaps both? |
The user sets a breakpoint (e.g. "the statement on line 42 in some source") and expects feedback whether the specified location actually exists. "Don't know yet" is sometimes the right answer. The debugger must (and already does) do extra work to manage "unresolved" breakpoints for files that haven't been loaded. But the feedback is an important usability issue. If the user thinks the location exists, but incorrectly, behavior is mysterious: an instance of the well-known "hidden state" usability failure. |
This came up when I was working with the Irvine Python people: two nodes can (correctly) have the same I'll propose a stronger requirement: a client must be able to distinguish between the case where (when all SourceSections are the same) a single node has two tags and where two separate node each have one of the tags. |
I strongly disagree with what "some might say"! I've been trying to get information out of source code and in front of programmers usefully for many years; that's the whole point of the Instrumentation framework. It is also why I argue constantly for flexibility and openness in the APIs: I named it a "Framework" and I refer to it as a "tool kit". Anybody should be able to create, as easily as possible, developer tools we have not yet imagined. |
|
I will add hashCode, toString and equals implementation to SourceSection for tags. I missed that, thanks. Ok if I close this pull-request? @mlvdv with the EventNodeFactory you can map mulitple "AST location" to one and the same SourceSection. If you care about same SourceSections beeing different in a Map you can use an IdentityHashMap in your tool implementation. |
|
Ok, closing it. |
|
Veto: I am against the name cloneWithTags, I am OK with withTags. |
|
Ok. Will add withTags. |
If I were to use an IdentityHashMap in that fashion, what guarantee would I be depending on about SourceSection identity:
|
…dary to master * commit 'f5dac94d8934f7d0425e3ebf469d31dff82cefb5': Call ThreadLocal<ContextStore>.get() via TruffleBoundary
* Fix writing of unnamed module * Ignore empty package; serialize null-class-loader as 'bootstrap' * Fix checkstyle complaint * Handle null string in JfrSymbolRepository as 0 ID/reference * Don't special-case 'null' module name, serialize it as 0
Mark suite files for 23.1.5 release
No description provided.