Skip to content

Commit

Permalink
Showing 1 changed file with 26 additions and 8 deletions.
Original file line number Diff line number Diff line change
@@ -15,16 +15,11 @@

import java.util.Arrays;

/**
* {@link InternalMethod} objects are copied as properties such as visibility are changed. {@link SharedMethodInfo} stores
* the state that does not change, such as where the method was defined.
*/
public class SharedMethodInfo {

private final SourceSection sourceSection;
private final LexicalScope lexicalScope;
private final Arity arity;
/** The original name of the method. Does not change when aliased. */

This comment has been minimized.

Copy link
@eregon

eregon May 10, 2016

Member

I liked my comment, this one indicates that InternalMethod name can change, but this never does.
The top-level comment might be useful as well to say this is all data derived from parsing a method definition and never changes.

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton May 10, 2016

Author Contributor

I removed it because it's a JavaDoc comment, but attached to a private field.

Maybe I should rename it alias rather than name? That would confer the same information.

This comment has been minimized.

Copy link
@headius

headius May 10, 2016

Member

I don't see any reason not to have JavaDoc for private methods and fields. You can enable generating docs with private items included, and it gets picked up by IDEs.

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton May 10, 2016

Author Contributor

But who would see them? If I'm using SharedMethodInfo then I don't even know this field exists.

This comment has been minimized.

Copy link
@eregon

eregon May 10, 2016

Member

I put them on fields so that when looking at the class implementation it clarifies what fields serves what purpose.
The fact it's a JavaDoc comment means an IDE will show it as I hover on any usage of it.
Having it on the getter would only document a single usage.

alias in InternalMethod? I think that would be unpractical.

This comment has been minimized.

Copy link
@headius

headius May 10, 2016

Member

@chrisseaton Anyone working on SharedMethodInfo would see them. But also...why delete it completely? If the comment had value, it should be restored...and then the difference is in one * that will let IDEs show the doco. This seems like a philosophical debate at this point.

This comment has been minimized.

Copy link
@eregon

eregon May 10, 2016

Member

It's internal documentation, similar to what I did in graal-core. I'll add it back.

This comment has been minimized.

Copy link
@chrisseaton

chrisseaton May 10, 2016

Author Contributor

Ok

private final String name;
private final String indicativeName;
private final boolean isBlock;
@@ -33,14 +28,27 @@ public class SharedMethodInfo {
private final boolean alwaysInline;
private final boolean needsCallerFrame;

public SharedMethodInfo(SourceSection sourceSection, LexicalScope lexicalScope, Arity arity, String name, String indicativeName, boolean isBlock, ArgumentDescriptor[] argumentDescriptors, boolean alwaysClone, boolean alwaysInline, boolean needsCallerFrame) {
public SharedMethodInfo(
SourceSection sourceSection,
LexicalScope lexicalScope,
Arity arity, String name,
String indicativeName,
boolean isBlock,
ArgumentDescriptor[] argumentDescriptors,
boolean alwaysClone,
boolean alwaysInline,
boolean needsCallerFrame) {
if (argumentDescriptors == null) {
argumentDescriptors = new ArgumentDescriptor[]{};
}

this.sourceSection = sourceSection;
this.lexicalScope = lexicalScope;
this.arity = arity;
this.name = name;
this.indicativeName = indicativeName;
this.isBlock = isBlock;
this.argumentDescriptors = argumentDescriptors == null ? new ArgumentDescriptor[] {} : argumentDescriptors;
this.argumentDescriptors = argumentDescriptors;
this.alwaysClone = alwaysClone;
this.alwaysInline = alwaysInline;
this.needsCallerFrame = needsCallerFrame;
@@ -87,7 +95,17 @@ public boolean needsCallerFrame() {
}

public SharedMethodInfo withName(String newName) {
return new SharedMethodInfo(sourceSection, lexicalScope, arity, newName, newName, isBlock, argumentDescriptors, alwaysClone, alwaysInline, needsCallerFrame);
return new SharedMethodInfo(
sourceSection,
lexicalScope,
arity,
newName,
newName,
isBlock,
argumentDescriptors,
alwaysClone,
alwaysInline,
needsCallerFrame);
}

@Override

0 comments on commit 5f631b9

Please sign in to comment.