Skip to content

Improved documentation of some object model classes #5

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

Merged
merged 4 commits into from
Jan 20, 2016

Conversation

smarr
Copy link
Contributor

@smarr smarr commented Jan 20, 2016

This PR adds some comments to Layout.createShape(..), Location, and Property.

Not sure whether the term 'language-specific' is ideal, but I wanted to document which elements are free for implementers to use.

Ping @jtulach and @woess.

@chumer
Copy link
Member

chumer commented Jan 20, 2016

Glossed over it. Looks good.


public abstract Shape createShape(ObjectType operations, Object sharedData, int id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@woess Can you create something else than a root shape manually? Maybe we should name the method also createRootShape.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently don't have any explicit concept of a root shape in names. But yes, you can only create root shapes this way. I'll consider renaming it.

@chumer chumer assigned chumer and woess and unassigned chumer Jan 20, 2016
@woess
Copy link
Member

woess commented Jan 20, 2016

Looks good.

woess added a commit that referenced this pull request Jan 20, 2016
Improved documentation of some object model classes
@woess woess merged commit 81e8946 into oracle:master Jan 20, 2016
@jtulach
Copy link
Contributor

jtulach commented Jan 20, 2016

Why don't we document method return values in Javadoc? My IDE complains about its being non-present...

@smarr
Copy link
Contributor Author

smarr commented Jan 20, 2016

It looked redundant to me. Should it be documented?

@smarr smarr deleted the pr/object-mode-comments branch January 20, 2016 20:22
@jtulach
Copy link
Contributor

jtulach commented Jan 21, 2016

Documenting return type of a method is "redundant"? Redundant to what? I am looking for example at createShape method which is missing return tag, and I don't think adding:

@return new instance of shape that can be used to ....

would hurt.

@smarr
Copy link
Contributor Author

smarr commented Jan 21, 2016

Ok, just for the sake of argument, and since I am not really used to writing JavaDoc, here my reasoning and my questions:

When looking at the createShape(...) methods, I was wondering what those arguments mean.
And, it is quite complex to figure out what they are eventually used for. I had to step through 7-10 or so methods to see the final constructor. And then, I had to see all the uses of these fields, which were not documented either. So, my main objective was to safe other people that kind of work and document those basic parameters.

At no point in the process I had a question about the returned object, because well, createShape(...) kind of says it all. Of course, it does not tell me anything about the invariants that each shape fulfills, which might also differ between the methods (I don't think they do).

So, ok, I thought, I'd add this comment:

/**
 * Create a root shape.
 *
 * @param objectType that describes the object instance with this shape.
 */

Eclipse's template also gave me the @return. But since I just typed Create a root shape., which essentially duplicates the method name, in my eyes it was redundant to what I could write for @return.

Thinking about it now, I guess, I did feel it isn't very DRY, and I didn't want to have three lines that say essentially the same.

So, now the question for me is whether we want the @return for consistency everywhere? And, what one should usually document in the @return. Are there any typical properties/invariants that are commonly put there?

@woess
Copy link
Member

woess commented Jan 21, 2016

I've seen too many @return descriptions that just repeat what the method description already said (usually getters and the like). How that improves javadoc is beyond me.
My rule of thumb is, it should either add additional information about the return value or summarize what the method returns after a long description of what the method does. Otherwise it's redundant.

@jtulach
Copy link
Contributor

jtulach commented Jan 22, 2016

In case you want to avoid duplication, I would prefer less verbose prose section and more descriptive @return value - that is the structured information about the method, not the prose section.

One property that should be put into @return section is whether it can return null or not. Different projects have different rules for that - I don't know what Truffle policy is - probably there is none yet.

dougxc pushed a commit that referenced this pull request Apr 6, 2016
…ruffle:DirectAccessors to master

Simple and direct Accessor nested interfaces

* commit 'fe123211a38c3e5be9eba37fbd87e63cdecb08a1':
  Let each package provide their own Accessor.Interface that other package can lookup (via Accessor) and use.
chrisseaton added a commit to Shopify/graal that referenced this pull request Dec 27, 2019
Introduce the thermometer tool
XiaohongGong pushed a commit to XiaohongGong/graal that referenced this pull request Feb 24, 2020
This patch adds the following match rules to generate bitfield
move instruction on AArch64:
* (RightShift (LeftShift value a) b)         -> SBFX/SBFIZ
* (UnsignedRightShift (LeftShift value a) b) -> UBFX/UBFIZ
* (LeftShift (SignExtend value) a)           -> SBFIZ
Eg:
  lsl    w0, w1, oracle#8
  asr    w0, w0, oracle#15
is optimized to:
  sbfx   w0, w1, oracle#7, oracle#17

It also adds the rules to integrate the ZeroExtend with unsigned
bitfield move operation.
Eg:
  ubfiz  w0, w1, oracle#5, oracle#12
  and    x0, x0, #0xffffffff
is optimized to:
  ubfiz  x0, x1, oracle#5, oracle#12

Change-Id: I2b635d4895db0d5f4c30630176f336e9a226ccaf
johanvos pushed a commit to johanvos/graal that referenced this pull request Jun 22, 2021
jerboaa pushed a commit to jerboaa/graal that referenced this pull request Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants