Initializer lists

[marsupial]

Description

Allows C++11 style initializer list syntax.
Uses type information available at compile time to fill in proper types for variables, function arguments, and return values.
Makes ASTcompound_initializer a subclass of ASTtype_constructor so it can use it's codegen and typecheck methods.

Tests

testsuite/initlist
testsuite/oslc-err-initlist-args
testsuite/oslc-err-initlist-return
testsuite/array-aassign

Checklist:

• I have read the contribution guidelines.
• I have previously submitted a Contributor License Agreement.
• I have updated the documentation, if applicable.
• I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
• My code follows the prevailing code style of this project.

[lgritz]

Hi, sorry if I merged things in the wrong order, but it appears that this initializer list patch conflicts with some of the changes you made (that I've already merged) about how to score and resolve function polymorphism. There's a conflict in ASTfunction_call::typecheck, at least.

Can you take a look, rebase this on current master, fix any conflicts, and update the PR? I like the idea of this, but want to make sure I'm testing properly including how it interacts with that other set of changes.

[marsupial]

No particular order to these..It's taking a moment to merge in the stashed resolution,
but #843 addresses a problem that should have been included in #837.
Sorry 'bout that.

[marsupial]

Gonna resolve this and rebase to validate on Windows.

[marsupial]

So a new class of ambiguity now arrises.

struct A{ float a, b; };
struct B{ float b, c; };

float ambig(A a);
float ambig(B b);

ambig({0,1});  // ambig(A) or ambig(B); ?

I would think this is too ambiguous and erroring might be wise?

---

struct C { float r,g,b; };

float noise(C c);

noise({0,1,2});  // noise(color), noise(vector), or noise(C); ?

Consider ambiguity via init-list an error or put user structs at the end of the rules for selection, so noise(color) is chosen?

[lgritz]

Eew, too confusing, looks like an invitation for bugs. Let's issue an error for this kind of ambiguity.

[marsupial]

Are you thinking erring for all three ambiguities presented?

I.e. should noise({0,1,2}); be allowed but warn & use precedence unless a function with a user struct is in competition (which then errs)?

Is there a test case/snippet for the changes to ASTtype_constructor::typecheck you pushed in?

[lgritz]

I'm tempted to consider it an error (we can't "break" anything, since we didn't support {} constructors before at all). Then if people tell us that it's too strict, we can think about rank+warn like we do for function arguments.

Re: type_constructor... are you asking for an explanation of what I changed, or an OSL test case that shows the case that it applies to?

[marsupial]

Shame noise({0,1,2}); does seem rather convenient, esp. considering a color4 variant: noise({{4,3,2}, 1}); vs noise(color4(color(4,3,2), 1))

I think I get what changed, but would like an explicit example beyond (vector)foo()) vs vector(foo()) or know if a test case I'm not seeing already covers that.

Just to verify mucking around in ASTtype_constructor::typecheck hasn't regressed.

[lgritz]

I'll add a testsuite case that verifies the type constructor fix.

Noise is an interesting case. It is tempting to want noise({x,y,z}) to work, but it's only because we know that noise(point) and noise(vector) are identical implementations. So it works for noise, but in the general case where you had func(structA) and func(structB) that were different, but A and B could each have the same initializer list form, it seems like it would be ripe for errors.

I'm not entirely convinced that noise{{3,2,1},0) is really a lot better than noise(point(3,2,1),0), or at least not enough to be worth grappling with a general solution to ambiguous cases. But if people really really didn't like having to write vector, you could always just define

float noise(x,y,z) { return noise(vector(x,y,z)); }

(and all its varieties)

Dunno if that's solving any real problems for people, or if it's just rearranging deck chairs.

[marsupial]

I'm not entirely convinced that noise{{3,2,1},0) is really a lot better than noise(point(3,2,1),0)

Agree, but in more complicated cases (like color4 or worse) avoiding nested constructor calls is nice.

---

Part of resolving this has shown two important cases overlooked in #844 that are currently present in master.

struct A { float a; };
struct B { float b; };

A ambig() { printf("A\n"); A a = { 1 }; return a; }
B ambig() { printf("B\n"); B b = { 2 }; return b; }
void ambig() {}

void subfunc(A a) { printf("AA\n"); }
void subfunc(B b) { printf("BB\n"); }

shader test()
{
    // Both are still undefined behavior, based on which function defined last.
    ambig();
    subfunc(ambig());
}

To me it seems return candidates precedence should be either:
float, int, color, vector, point, normal, matrix, string, struct, void
struct, float, int, color, vector, point, normal, matrix, string, void

When ambiguity is being resolved via return type, always error if more than one overload returns a user struct.

[lgritz]

I prefer your first precedence list -- structs should be lower priority than built-in types, I believe.

And I agree that it should be an error for multiple possible overloads are user structs. I don't see any way to definitively rank user structs except by declaration order, which is what we're striving to avoid.

[marsupial]

Do a separate PR for this or just a single commit in this one that can be cherry-picked?

Hopefully the last un-defined case is closures, do they have a higher or lower priority than the type they wrap?

FWIW, the only reason I saw of structs being highest is so explicit casting an overloaded builtin isn't necessary.

struct A {...};
A noise(point p);
color mix(color a, color b, A v);

// Believe a cast is necessary to ever invoke the mix above.
// But the intent is far clearer that way anyway.
mix(a, b, (A)noise(P));

[lgritz]

Separate PR -- either way is ok. Since the initializer notation is new, and it'll take at least several days after merging before even the people using master regularly are using it in earnest, I think it's ok to get the 90% solution in first, and then have a follow-up PR in the next few days that handles the tricky edge cases.

closures -- gee, I have no idea. Make your best guess. I don't have a preference at this time. I suspect that there will be very few practical cases where this turns up. You can't typecast a non-closure to a closure, so it won't be an issue of function argument ambiguity. It can only be an issue if there are polymorphic functions that can return either closures or non-closures for the same argument list. Seems like an extreme edge case, and if we see that the strategy we choose is tripping people up, we can change it. (Remember, 'master' is subject to change in the rules until it becomes a release branch).

[marsupial]

Updated & includes #858 for my sanity later.

[lgritz]

I think LGTM and passes all my tests. There are some stylistic fixups I'm tempted to make, but I think I'll just do that myself as a separate PR.