Skip to content
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

Add transitional syntax for enum structs. #287

Merged
merged 3 commits into from Dec 15, 2018

Conversation

@dvander
Copy link
Member

dvander commented Dec 4, 2018

Add transitional syntax for enum structs.

This amends the Transitional Syntax to allow enum structs, codifying a
permanent replacement and obsoleting the old syntax. The grammar is:

enum struct <name> { <newline>
  [<new-type-expr> <name> [post-dims] <newline>]*
} <newline>

Enum structs declared this way can be used anywhere a new-style type is
declared. Internally, they still decompose into arrays. As such,
anywhere an array is forbidden, enum structs are not allowed.
Similarly, if used in an array, an enum struct type will become a
multi-dimensional array.

Unlike old-style enum structs, they are not accessed with array syntax.
Instead they are accessed through the field operator ("."). For example
an old enum struct might look like:

enum Sample {
  Float:x, Float:y
};
new s[Sample];
s[x] = 10; s[y] = 20;

A new-style example might look like:

enum struct Sample {
  float x;
  float y;
};
Sample s;
s.x = 10; s.y = 20;

Similarly, the field names of a new-style enum struct do not poison the
global scope. In fact they are not accessible, and the enum struct name
itself does not define a global symbol (only a named type).

Enum structs can also have inline methods, similar to methodmaps.
Unlike methodmaps, these methods cannot be native, and do not have
public or private decorators. Like methodmaps, they must use new-style
syntax.

This new syntax obsoletes old-style enum structs. They will no longer be
maintained going forward, and bugs in them will not be fixed. They may
be broken in the future.

Rationale:

Enum structs have been neglected due to having a painfully complex and
strange implementation. Its problems roughly fall into three categories:

  1. Syntax: they are awkwardly layered on top of enums and arrays.
  2. Semantics: they define unscoped globals that expose implementation
    details, and can be used in illegal, nonsensical ways.
  3. Implementation: rather than define a type, they create a shadow
    structure attached to arrays, which then requires complex pattern
    matching to reconstruct.

Developers have been asking for struct syntax, even if it's sugar around
enum structs. In the past we've been reluctant to do this, because
Structs remain a large carrot for moving to a newer compiler. Properly
implemented, they allow many things enum structs do not (including
cross-plugin and native support). Syntactic sugar does not fix the deeper
problems in the runtime, and to fix those, we would need users to accept
backwards compatibility breaks and experimental behavior. Thus, structs
would be the carrot to entice this.

However this thinking was probably mistaken. No "correct" struct
implementation has been forthcoming, and meanwhile, we're stuck with a
poorly implemented and widely used language misfeature. It is better to
provide something maintainable to promote better plugins, and provide
developers with a better environment. This solution is also incomplete:
there are many things enum structs still don't allow, so we have not
lost a carrot for future versions.

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 4, 2018

TODO: finish RTTI and update smx viewer. RTTI must be able to properly rewrite enum struct types on symbols by peeking at idxtag.

@dvander dvander force-pushed the es-sugar branch from feac5b0 to 252b8b9 Dec 4, 2018
@Headline

This comment has been minimized.

Copy link
Member

Headline commented Dec 4, 2018

can we drop the enum from enum struct?

If syntax is identical to how one would expect typical struct usage, why should we treat it differently?

That would also lead to less script breakage when real structs are introduced and enum structs are inevitably deprecated

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 4, 2018

The biggest reason is that I'm not positive it's identical. I haven't thought about it carefully enough, but it seems like there could be differences given that it's still sugar for arrays. Like, pass-by-value vs pass-by-reference.

Another reason is that we already have a "struct" keyword (see core.inc). They have different semantics. The pre-existing keyword allows things like char[] in fields which enum structs do not. Unifying them would be tricky, but not impossible. Basically the pre-existing cases have to fall into declstruct() and the new cases would have to fall into initials(). We could get there with pattern matching, and probably delete the old pstruct code.

The potential for semantic difference is a bigger issue, but I'm nowhere near 100% sold on "enum struct" syntax or future-structs working one way or another.

@Headline

This comment has been minimized.

Copy link
Member

Headline commented Dec 4, 2018

The only reason I suggested that is because I can't see a difference in how this behaves vs real structs (despite the future feature-set that real structs would get).

Since in sourcepawn arrays are always pass-by-reference, that seems inline with how a struct would behave, unless you had a different idea around it. I think it'd be a little misleading to some people, but 90% of the consumers wouldn't be able to identify whether these structs are "real" or not, which in my opinion can be a good thing.

@TotallyMehis

This comment has been minimized.

Copy link

TotallyMehis commented Dec 4, 2018

Internally, they still decompose into arrays

Are we able to get the position of a member and the size of the struct somehow?

Right now you do things like this if you want a global dynamic array with a pseudo struct:

enum
{
    FOO_X,
    FOO_Y,
    FOO_Z_FLOAT,
    
    FOO_SIZE
};

ArrayList globalarray;

public void OnPluginStart()
{
    globalarray = new ArrayList( FOO_SIZE );
    
    
    any f[FOO_SIZE];
    f[FOO_X] = 1;
    f[FOO_Y] = 2;
    f[FOO_Z_FLOAT] = view_as<int>( 3.0 );
    
    globalarray.PushArray( f );
    
    
    float z = view_as<float>( globalarray.Get( 0, FOO_Z_FLOAT ) );
    PrintToServer( "z: %.1f", z );
}

Something like this perhaps:

enum struct Foo
{
    int x;
    int y;
    float z;
};

ArrayList globalarray;

public void OnPluginStart()
{
    globalarray = new ArrayList( sizeof( Foo ) ); // ???
    
    
    Foo f;
    f.x = 1;
    f.y = 2;
    f.z = 3.0;
    
    globalarray.PushArray( view_as<any>( f ) ); // ???
    
    
    float z = view_as<float>( globalarray.Get( 0, Foo::z ) ); // ???
    PrintToServer( "z: %.1f", z );
}
@assyrianic

This comment has been minimized.

Copy link

assyrianic commented Dec 4, 2018

I have created a tuple type that acts exactly like a C/C++ struct, including packing and padding features. would it be possible to back the enum struct with the tuple as a handle?

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 4, 2018

Since in sourcepawn arrays are always pass-by-reference, that seems inline with how a struct would behave, unless you had a different idea around it.

In almost every other language, arrays are pass-by-ref and structs are pass-by-value. I don't know if that's more or less confusing or not. But probably it'd be confusing if structs and enum-structs behaved differently.

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 4, 2018

Internally, they still decompose into arrays

Are we able to get the position of a member and the size of the struct somehow?

I'll give some thought about how to integrate this with native data structures. Exposing sizeof() and offsets seems like a mistake to me. If we wind up dropping the "enum" from this syntax as Headline suggested, we absolutely don't want that. The fact that it is an array internally must be completely hidden. Otherwise, adding proper struct features in the future will get much more difficult, since we'll have to make sure array-like hacks continue to work.

If we keep the "enum struct" syntax, then it might be okay, since it's clearly a separate feature.

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 4, 2018

I have created a tuple type that acts exactly like a C/C++ struct, including packing and padding features. would it be possible to back the enum struct with the tuple as a handle?

This is intended to be a high-level feature, so packing and padding are a bit overkill. It depends what the tuples themselves are backed by though. Actual, honest-to-SourceMod handles would require that they get closed without explicit calls to CloseHandle. I'm not sure I want to die on that hill right now. It's possible you meant some other kind of handle though.

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 4, 2018

Thinking about this more: this PR was intended to paper over bad syntax, not bring a fully-featured implementation. So that makes me think:

  • Calling them "enum structs" (or something other than pure "structs") is better.
  • We should keep the behavior of sizeof(variable) (which this patch does) and sizeof(EnumStructType) (which is a TODO).
  • We should expose something like sizeof(EnumStructType.field) and something else to get the offsets - maybe just EnumStructType.field.

... Rather than going for gold and figuring out how to integrate true structs with containers like ArrayList.

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 5, 2018

The latest patch adds support for @TotallyMehis's use case, using the syntax in the test case. I picked the resolution operator over the field operator, since it is really accessing meta-information and not a named static property.

@assyrianic

This comment has been minimized.

Copy link

assyrianic commented Dec 5, 2018

@dvander to be exact, my tuple implementation is a static, dynamically allocated array that uses a vector to store the fields and size of each member but this kind of tuple was implemented with C's limitations in design so it's likely very useless here unless you find a way to add type-safety to it.

Getting back on topic though, I think the enum struct is a good hold over in terms of having a way to cleanly organize data without having to resort to a weird combination of semantics and arrays.

Realistically, IIRC, if methodmaps can be backed with arrays in some way rather than with enums, then you've pretty much added practical classes.

@VoiDeD

This comment has been minimized.

Copy link
Contributor

VoiDeD commented Dec 6, 2018

Just a note per IRC to add tests for sizeof(x.y) and sizeof(X::y).

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 6, 2018

Thanks, that should work now.

Copy link
Member

Headline left a comment

I know we're not using c++17 atm, but just a thought

wait that's c++11, can we?

compiler/parser.cpp Outdated Show resolved Hide resolved
@dvander dvander force-pushed the es-sugar branch from 0680f1a to d9dac33 Dec 8, 2018
@assyrianic

This comment has been minimized.

Copy link

assyrianic commented Dec 9, 2018

quick suggestion.
perhaps it's better to name it as record or fieldmap rather than enum struct?

enum struct Foo {
    int x;
    int y;
    float z;
};

to

record Foo {
    int x;
    int y;
    float z;
};

or possibly

fieldmap Foo {
    int x;
    int y;
    float z;
};

My reasoning is that newer coders will look at enum struct and struct and misunderstand their semantics. Not to mention that naming it record makes more sense than enum struct. fieldmap also works since we have methodmap.

@dvander dvander force-pushed the es-sugar branch from edc7d08 to db8d30b Dec 9, 2018
@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 9, 2018

This is syntactic sugar for a feature everyone already refers to as "enum structs". We're not really adding structs, or something analogous to structs, because enum structs compose differently and can be casted to/from arrays. I would rather call it what it is.

(A better name might be "array structs" or "named arrays", but I'm working with the keywords available.)

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 9, 2018

I'm considering this PR feature-complete. I will probably have some follow-up commits to clean up parser support or fix bugs, but will otherwise leave it open for a few days for additional comments.

@assyrianic

This comment has been minimized.

Copy link

assyrianic commented Dec 9, 2018

Question. From the SM wiki
7. Methodmaps can only be defined over scalars - that is, the "this" parameter can never be an array. This means they cannot be used for enum-structs.

I'm going to assume that this limitation still persists with enum struct currently but is there really no way to allow this to be an array so that enum structs can be applicable to a methodmap?

From one of the test code, I see that the methodmap is defined and then used by the enum struct when the usual way would be to define the enum struct and then the methodmap wraps around it.

@dvander dvander force-pushed the es-sugar branch from 2e28476 to f91a137 Dec 9, 2018
@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 9, 2018

I'm tentatively planning to enable methods on enum structs, but probably not in this PR.

@dvander

This comment has been minimized.

Copy link
Member Author

dvander commented Dec 9, 2018

One change I haven't decided on yet: whether to disallow enum struct types in natives. If two plugins use different versions of the same named enum struct, the offsets may be different. True structs would be able to deal with this, but right now, we can't, and it may lead to very obscure bugs if users don't version cross-plugin APIs properly.

It would be trivial to get around such a restriction by using "any", as ArrayList and friends already do.

@dvander dvander force-pushed the es-sugar branch 3 times, most recently from 8391f3d to d18cab0 Dec 12, 2018
This amends the Transitional Syntax to allow enum structs, codifying a
permanent replacement and obsoleting the old syntax. The grammar is:

    enum struct <name> { <newline>
      [<new-type-expr> <name> [post-dims] <newline>]*
    } <newline>

Enum structs declared this way can be used anywhere a new-style type is
declared. Internally, they still decompose into arrays. As such,
anywhere an array is forbidden, enum structs are not allowed.
Similarly, if used in an array, an enum struct type will become a
multi-dimensional array.

Unlike old-style enum structs, they are not accessed with array syntax.
Instead they are accessed through the field operator ("."). For example
an old enum struct might look like:

    enum Sample {
      Float:x, Float:y
    };
    new s[Sample];
    s[x] = 10; s[y] = 20;

A new-style example might look like:

    enum struct Sample {
      float x;
      float y;
    };
    Sample s;
    s.x = 10; s.y = 20;

Similarly, the field names of a new-style enum struct do not poison the
global scope. In fact they are not accessible, and the enum struct name
itself does not define a global symbol (only a named type).

Additionally, a long-standing bug has been fixed where strings within
enum structs could not be indexed properly. Also note that enum structs
are forbidden in natives, and must be casted to any[] to be communicated
across plugin boundaries. If doing this cross-plugin, care must be taken
that the ordering of the enum struct fields do not change.

This new syntax obsoletes old-style enum structs. They will no longer be
maintained going forward, and bugs in them will not be fixed. They may
be broken in the future.

Rationale:

Enum structs have been neglected due to having a painfully complex and
strange implementation. Its problems roughly fall into three categories:
 1. Syntax: they are awkwardly layered on top of enums and arrays.
 2. Semantics: they define unscoped globals that expose implementation
    details, and can be used in illegal, nonsensical ways.
 3. Implementation: rather than define a type, they create a shadow
    structure attached to arrays, which then requires complex pattern
    matching to reconstruct.

Developers have been asking for struct syntax, even if it's sugar around
enum structs. In the past we've been reluctant to do this, because
Structs remain a large carrot for moving to a newer compiler. Properly
implemented, they allow many things enum structs do not (including
cross-plugin and native support). Syntactic sugar does not fix the deeper
problems in the runtime, and to fix those, we would need users to accept
backwards compatibility breaks and experimental behavior. Thus, structs
would be the carrot to entice this.

However this thinking was probably mistaken. No "correct" struct
implementation has been forthcoming, and meanwhile, we're stuck with a
poorly implemented and widely used language misfeature. It is better to
provide something maintainable to promote better plugins, and provide
developers with a better environment. This solution is also incomplete:
there are many things enum structs still don't allow, so we have not
lost a carrot for future versions.
@dvander dvander force-pushed the es-sugar branch from d18cab0 to b4d45fa Dec 14, 2018
Unlike methodmaps, enum struct methods cannot be native.
@dvander dvander force-pushed the es-sugar branch from b4d45fa to ce719b6 Dec 14, 2018
@dvander dvander merged commit dad353e into master Dec 15, 2018
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@dvander dvander deleted the es-sugar branch Dec 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.