Truffle API feedback

[Tim Boudreau]

Hi, folks,

I've been poking around randomly in the Truffle sources, and made a few
notes on places where the API could be cleaner (which I think Jarda Tulach
is actively working on):

1a.  CompilerOptions.setOption(String name, Object value);

would be easier to use if it returned CompilerOptions instead of void, so
method chaining can be used.  In general, any time something like this
returns void, you're throwing away an opportunity to make calling code less
verbose.

1b. A cleaner and safer pattern would be:

public abstract class CompilerOption<T> {
   protected abstract void validate(T value) throws SomeException;
//because you can
}

and

<T> CompilerOptions.setOption(CompilerOption<T>, T value);

and let people define static fields with CompilerOption instances.

That would be typesafe and would eliminate typos in string names.  And
would probably allow language authors to factor their option processing
more cleanly than a big switch statement.

--------

2. CompilerDirectives.injectBranchProbability(double probability, boolean
condition) and the constants such as LIKELY_PROBABILITY

You're better off with a Probability class that hides the floating point.
There's no particular reason this sort of computation needs to be floating
point, and if it does, you're still better off with
Probability.isMoreLikelyThan(otherProbability) or
Probability.times(otherProbability).

More flexibility later, you can put bounds on legal values (what would a
probability of Float.MIN_VALUE mean?  Make that impossible), and it's
cleaner.

--------

3.  ReplaceObserver.nodeReplaced - observer/listener methods are clearer if
named on$EVENT, e.g. onNodeReplaced - so if implemented as a mix-in, it is
still obvious that this method responds to an event.

Same for SourceListener, and probably other things named *Listener /
*Observer.

--------

4.  Should TruffleOptions really have world-writable static fields?

--------

5.  BytesDecoder.decode() - should probably return CharSequence, not
String.  With CharSequence you keep the option of avoiding a memory copy,
with an implementation of CharSequence over the raw array bytes.

Once it's String, you can never go back.  Same for Source.getCode().

--------

6.  Source.setFileCaching() - this does not look like it should be API.
Maybe some attribute of whatever is using the Graal API on initialization
(perhaps user code -> language -> truffle), but not here.

-- 
http://timboudreau.com