Truffle API feedback

[Jaroslav Tulach]

Hello Tim,
#5 and #6 are fixed. Please check the latest Javadoc: http://lafo.ssw.uni-linz.ac.at/javadoc/truffle/latest/

Please kemp reminding me about the rest.
-jt

Odesláno z iPadu

19. 6. 2015 v 18:59, Tim Boudreau <niftiness at gmail.com>:

> 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