Skip to content

Commit

Permalink
https://github.com/manifold-systems/manifold/issues/641
Browse files Browse the repository at this point in the history 
- fix default value compilation on records
  • Loading branch information
@rsmckinney
rsmckinney committed Jan 11, 2025
1 parent efb24f1 commit 6bb7ed8
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 158 deletions.
3 changes: 3 additions & 0 deletions manifold-deps-parent/manifold-params-test/src/test/java/manifold/ext/params/ScratchTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ public void testConstructor()
CtorTest<String> str = new CtorTest<>(foo:"foo");
assertEquals( "foo", str.getFoo() );

str = new CtorTest<>("foo");
assertEquals( "foo", str.getFoo() );

str = new CtorTest<>(());
assertEquals( "hi", str.getFoo() );

Expand Down
16 changes: 7 additions & 9 deletions manifold-deps-parent/manifold-params/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ public String valueOf(char[] data,

valueOf(array) // use default values for offet and count
valueOf(array, 2) // use default value for count
valueOf(data: array, count: 20) // use default value for offset
valueOf((array, count: 20)) // use a tuple expression to mix positional and named arguments
valueOf(array, count:20) // use default value for offset
```

Optional parameters and named arguments are fully integrated in both **IntelliJ IDEA** and **Android Studio**. Use the IDE's
Expand Down Expand Up @@ -103,9 +102,9 @@ configure("MyConfig",
autoSave: false);
```
## Binary compatibility
## Binary compatible

Adding new optional parameters to existing methods maintains binary compatibility.
Adding new optional parameters to existing methods is binary compatible with code compiled before adding the new parameters.

Version 1.
```java
Expand All @@ -117,14 +116,13 @@ public void size(int width, int height = width) {...}
```
Code compiled with v1 still runs with v2, without having to recompile with v2.

Additionally, code not compiled with manifold-params can still benefit from using optional parameters methods compiled
with manifold-params.

Code compiled w/o manifold-params can call method overloads reflecting the optional parameters.
Additionally, code compiled without `manifold-params` can still benefit from code compiled with optional parameters. In
this case default parameter values can be used with calls to method overloads that are generated for this purpose.
```java
size(myWidth);
size(myWidth, myHeight);
```
```


# IDE Support

Expand Down
168 changes: 19 additions & 149 deletions manifold-deps-parent/manifold-params/src/main/java/manifold/ext/params/ParamsProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,13 @@
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;

import static manifold.ext.params.ParamsIssueMsg.*;

public class ParamsProcessor implements ICompilerComponent, TaskListener
{
private static final long RECORD = 1L << 61; // from Flags in newer JDKs

private BasicJavacTask _javacTask;
private Context _context;
private TaskEvent _taskEvent;
Expand Down Expand Up @@ -194,12 +195,6 @@ public void finished( TaskEvent e )

}

@Override
public boolean isSuppressed( JCDiagnostic.DiagnosticPosition pos, String issueKey, Object[] args )
{
return ICompilerComponent.super.isSuppressed( pos, issueKey, args );
}

// Add data classes reflecting parameters and corresponding method overloads
//
private class Enter_Start extends TreeTranslator
Expand Down Expand Up @@ -288,6 +283,7 @@ private void handleRecord( JCClassDecl tree )
if( isRecordParam( def ) )
{
params = params.append( (JCVariableDecl)copier.copy( def ) );
((JCVariableDecl)def).init = null; // records compile parameters directly as fields, gotta remove the init
}
}

Expand All @@ -301,7 +297,6 @@ private void handleRecord( JCClassDecl tree )

private boolean isRecordParam( JCTree def )
{
final long RECORD = 1L << 61; // from Flags in newer JDKs
return def instanceof JCVariableDecl && (((JCVariableDecl)def).getModifiers().flags & RECORD) != 0;
}

Expand All @@ -314,12 +309,17 @@ public void visitMethodDef( JCMethodDecl methodDecl )

private void processParams( JCMethodDecl methodDecl )
{
boolean processed = false;
for( JCVariableDecl param : methodDecl.params )
{
if( param.init != null )
{
handleOptionalParams( methodDecl );
return;
if( !processed )
{
handleOptionalParams( methodDecl );
processed = true;
}
param.init = null; // remove init just in case javac decides not to like it at some point
}
}
}
Expand All @@ -333,22 +333,21 @@ private void handleOptionalParams( JCMethodDecl optParamsMeth )

ArrayList<JCTree> defs = _newDefs.peek().snd;

// Generate an inner class to encapsulate default values and as a means to assign default values to unassigned params.
// The tuple expr used to call the method is transformed into a new-expression on the generated inner class.
JCClassDecl paramsClass = makeParamsClass( optParamsMeth, make, copier );
defs.add( paramsClass );

// Generate a method having the above generated inner class as its single parameter, this method delegates calls to
// the original optional params method.
JCMethodDecl paramsMethod = makeParamsMethod( optParamsMeth, paramsClass, make, copier );
defs.add( paramsMethod );

if( shouldAddTelescopeMethods( optParamsMeth ) )
{
ArrayList<JCMethodDecl> telescopeMethods = makeTelescopeMethods( optParamsMeth, make, copier );
defs.addAll( telescopeMethods );
}
}

private boolean shouldAddTelescopeMethods( JCMethodDecl optParamsMeth )
{
return true;
// Generate telescoping methods, for binary compatibility and for handling call sites having all positional arguments.
// A call site having all positional arguments is parsed normally, reflecting all the args separately. Whereas a call
// site having one or more named arguments parses all the arguments as a single tuple expression.
ArrayList<JCMethodDecl> telescopeMethods = makeTelescopeMethods( optParamsMeth, make, copier );
defs.addAll( telescopeMethods );
}

private ArrayList<JCMethodDecl> makeTelescopeMethods( JCMethodDecl optParamsMeth, TreeMaker make, TreeCopier<?> copier )
Expand Down Expand Up @@ -464,135 +463,6 @@ private List<JCExpression> makeTupleArg( TreeMaker make, Name name )
return List.of( make.Apply( List.nil(), make.Ident( getNames().fromString( "$manifold_label" ) ), List.of( make.Ident( name ) ) ),
make.Ident( name ) );
}
// private JCMethodDecl makeTelescopeMethod( JCMethodDecl targetMethod, TreeMaker make, TreeCopier<?> copier, int optParamsInSig )
// {
// JCMethodDecl copy = copier.copy( targetMethod );
//
// ArrayList<JCVariableDecl> reqParams = new ArrayList<>();
// ArrayList<JCVariableDecl> optParams = new ArrayList<>();
// for( JCVariableDecl p: copy.params )
// {
// if( p.init == null )
// {
// reqParams.add( p );
// }
// else
// {
// optParams.add( p );
// }
// }
//
// List<Name> targetParams = List.from( copy.params.stream().map( e -> e.name ).collect( Collectors.toList() ) );
//
// // telescope interface methods have default impls that forward to original
// long flags = targetMethod.getModifiers().flags;
// if( (flags & Flags.STATIC) == 0 &&
// (classDecl().mods.flags & Flags.INTERFACE) != 0 )
// {
// flags |= Flags.DEFAULT;
// }
// copy.mods.flags = flags;
// // mark with @manifold_params for IJ use
// JCAnnotation paramsAnno = make.Annotation( memberAccess( make, manifold_params.class.getTypeName() ), List.nil() );
// copy.mods.annotations = copy.mods.annotations.append( paramsAnno );
//
// // Params
// ArrayList<JCVariableDecl> params = new ArrayList<>();
// ArrayList<JCExpression> args = new ArrayList<JCExpression>()
// {{
// addAll( targetParams.stream().map( e -> (JCExpression)null ).collect( Collectors.toList() ) );
// }};
// ArrayList<JCStatement> locals = new ArrayList<>();
// for( JCVariableDecl reqParam: reqParams )
// {
// params.add( make.VarDef( make.Modifiers( Flags.PARAMETER ), reqParam.name, reqParam.vartype, null ) );
// int index = targetParams.indexOf( reqParam.name );
// args.set( index, make.Ident( reqParam.name ) );
// }
// for( int i = 0; i < optParamsInSig; i++ )
// {
// JCVariableDecl optParam = optParams.get( i );
// int index = targetParams.indexOf( optParam.name );
// params.add( index, optParam );
// args.set( index, make.Ident( optParam.name ) );
// }
// for( int i = optParamsInSig; i < optParams.size(); i++ )
// {
// JCVariableDecl local = optParams.get( i );
// local.mods.flags &= ~Flags.PARAMETER;
// int index = targetParams.indexOf( local.name );
// args.set( index, make.Ident( local.name ) );
// locals.add( local );
// }
// if( args.stream().anyMatch( Objects::isNull ) )
// {
// throw new IllegalStateException( "null " );
// }
// copy.params = List.from( params );
//
// // body
// JCMethodInvocation forwardCall;
// if( targetMethod.name.equals( getNames().init ) )
// {
// forwardCall = make.Apply( List.nil(), make.Ident( getNames()._this ), List.from( args ) );
// }
// else
// {
// forwardCall = make.Apply( List.nil(), make.Ident( targetMethod.name ), List.from( args ) );
// }
// JCStatement stmt;
// if( targetMethod.restype == null ||
// (targetMethod.restype instanceof JCPrimitiveTypeTree &&
// ((JCPrimitiveTypeTree)targetMethod.restype).typetag == TypeTag.VOID) )
// {
// stmt = make.Exec( forwardCall );
// }
// else
// {
// stmt = make.Return( forwardCall );
// }
// copy.body = make.Block( 0L, List.from( locals ).append( stmt ) );
// return copy;
// }

// // make a structural interface reflecting the parameters of the method
// // String foo(String name, int age = 100) {...}
// // @Structural interface $foo {
// // @val String name;
// // @val int age = 100;
// // }
// private JCClassDecl makeStructuralInterface( JCMethodDecl targetMethod, TreeMaker make, TreeCopier<?> copier )
// {
// // name & modifiers
// JCTree.JCModifiers modifiers = make.Modifiers( Flags.INTERFACE, List.of( makeAnnotation( Structural.class ) ) );
// modifiers.pos = make.pos;
// Names names = getNames();
// Name name = names.fromString( "$" + targetMethod.getName() +
// targetMethod.params.stream().map( e -> "_" + e.name ).reduce( "", (a,b) -> a+b ) );
//
// // Type params
// List<JCTypeParameter> typeParams = List.nil();
// typeParams = typeParams.appendList( copier.copy( List.from( targetMethod.getTypeParameters() ) ) );
// if( (targetMethod.getModifiers().flags & Flags.STATIC) == 0 )
// {
// typeParams = typeParams.appendList( copier.copy( List.from( classDecl().getTypeParameters().stream()
// .map( e -> make.TypeParameter( e.name, List.nil() ) ).collect( Collectors.toList() ) ) ) );
// }
//
// // Properties
// List<JCVariableDecl> methParams = targetMethod.getParameters();
// List<JCTree> properties = List.nil();
// for( JCVariableDecl methParam : methParams )
// {
// JCExpression propType = (JCExpression)copier.copy( methParam.getType() );
// JCVariableDecl property = make.VarDef( make.Modifiers( 0L, List.of( makeAnnotation( val.class ) ) ), methParam.name, propType, methParam.init );
// property.pos = make.pos;
// properties = properties.append( property );
// }
//
// return make.ClassDef( modifiers, name, typeParams, null, List.nil(), properties );
// }
//

// make a static inner class reflecting the parameters of the method
//
Expand Down

0 comments on commit 6bb7ed8

Please sign in to comment.