Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate second parameter of SourceSection.createUnavailable() #200

Closed
pniederw opened this issue Sep 3, 2016 · 7 comments
Closed

Deprecate second parameter of SourceSection.createUnavailable() #200

pniederw opened this issue Sep 3, 2016 · 7 comments
Labels

Comments

@pniederw
Copy link

pniederw commented Sep 3, 2016

O.17 has deprecated SourceSection.getIdentifier(). Presumably this means that the second parameter to SourceSection.createUnavailable() should also be deprecated, as all it does it to set identifier.

@chumer
Copy link
Member

chumer commented Sep 4, 2016

Thanks for this suggestion. Its already on the way. We are , very likely, going to deprecate SourceSection#createUnavailalble and replace it with Source#createUnavailableSection() including a SourceSection#isUnavailable().

We are also going to shrink SourceSection to just charIndex with length. lines and columns indices are going to be computed lazily. All of this we are doing to minimize the footprint of source sections.

BTW.: You can also compute source sections lazily in Node#getSourceSection() by storing the indices in the Node directly and the Source object in the RootNode.

@smarr
Copy link
Contributor

smarr commented Sep 4, 2016

@chumer: If I remember correctly, the instrumentation API checks for source sections to be available as a precondition for checking tags. Is that going to be removed as part of the change you mentioned?

@chumer
Copy link
Member

chumer commented Sep 4, 2016

No not due to this change. Source sections are only queried if instrumentations are actually installed (does not yet apply for root nodes, but that will change). We optimize for the fast case where we only execute but not instrument the application.

The source section restriction might change, as soon as we add support for dynamic changes in tags. Its on the TODO list, but not yet appeared on top.

@smarr
Copy link
Contributor

smarr commented Sep 4, 2016

Sorry for getting off-topic for this issue here:

@chumer ok, but this means instrumentation usage force lazy source section creation? That's how I'd read https://github.com/graalvm/truffle/blob/master/truffle/com.oracle.truffle.api.instrumentation/src/com/oracle/truffle/api/instrumentation/InstrumentationHandler.java#L908

@chumer
Copy link
Member

chumer commented Sep 4, 2016

Yes in this case we would need to request the source section atm. We could remove the call to #getSourceSection if we lift the source section restriction, which i am not yet sure if its going to happen.

@mlvdv
Copy link
Contributor

mlvdv commented Sep 6, 2016

Source sections are only queried if instrumentations are actually installed

Won't there be a need for source information when a language implementation prints a stack trace?

Also: some languages offer features that depend on instrumentation, where the line between language and tool support is blurry.

dougxc pushed a commit that referenced this issue Sep 15, 2016
* commit 'ce6ae985b3a504f9c2fafab05adda87076545e6d':
  OM: remove extra spaces in JavaDoc
@chumer
Copy link
Member

chumer commented Jan 9, 2018

Michael is right that the source information might also be materialized when the stack trace is printed. It still can be deferred until when it is actually printed.

The discussed method SourceSection#createUnavailable was already removed from the API.

@chumer chumer closed this as completed Jan 9, 2018
@chumer chumer added the truffle label Jan 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants