Skip to content

Commit

Permalink
Refactor dereference operations into tagged unions (OpenDreamProject#…
Browse files Browse the repository at this point in the history
…1603)

* refactor deref

* add IsFuzzy prop and eliminate CompileErrorExceptions

* s

* stop complaining

* forgot to throw here

* more code improvements in ParseDereference

* remove notimplementedexceptions here

* review comments + added some docs
  • Loading branch information
distributivgesetz authored Jan 9, 2024
1 parent d83ae6d commit cf334d6
Show file tree
Hide file tree
Showing 10 changed files with 368 additions and 445 deletions.
64 changes: 36 additions & 28 deletions DMCompiler/Compiler/DM/DMAST.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2693,34 +2693,42 @@ public override void Visit(DMASTVisitor visitor) {


public sealed class DMASTDereference : DMASTExpression {
public enum OperationKind {
Invalid,

Field, // x.y
FieldSafe, // x?.y
FieldSearch, // x:y
FieldSafeSearch, // x?:y

Index, // x[y]
IndexSafe, // x?[y]

Call, // x.y()
CallSafe, // x?.y()
CallSearch, // x:y()
CallSafeSearch, // x?:y()
}

public struct Operation {
public OperationKind Kind;

// Field*, Call*
public DMASTIdentifier Identifier;

// Index*
public DMASTExpression Index;

// Call*
public DMASTCallParameter[] Parameters;
public abstract class Operation {
/// <summary>
/// The location of the operation.
/// </summary>
public required Location Location;
/// <summary>
/// Whether we should short circuit if the expression we are accessing is null.
/// </summary>
public required bool Safe; // x?.y, x?.y() etc
}

public abstract class NamedOperation : Operation {
/// <summary>
/// Name of the identifier.
/// </summary>
public required string Identifier;
/// <summary>
/// Whether we should check if the variable exists or not.
/// </summary>
public required bool NoSearch; // x:y, x:y()
}

public sealed class FieldOperation : NamedOperation;

public sealed class IndexOperation : Operation {
/// <summary>
/// The index expression that we use to index this expression (constant or otherwise).
/// </summary>
public required DMASTExpression Index; // x[y], x?[y]
}

public sealed class CallOperation : NamedOperation {
/// <summary>
/// The parameters that we call this proc with.
/// </summary>
public required DMASTCallParameter[] Parameters; // x.y(),
}

public DMASTExpression Expression;
Expand Down
112 changes: 50 additions & 62 deletions DMCompiler/Compiler/DM/DMParser.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System;
using System.Collections.Generic;
using System.Linq;
using DMCompiler.Compiler.DMPreprocessor;
using OpenDreamShared.Compiler;
using OpenDreamShared.Dream;
using System;
using System.Collections.Generic;
using System.Linq;
using String = System.String;

namespace DMCompiler.Compiler.DM {
Expand Down Expand Up @@ -2220,15 +2220,13 @@ private void BracketWhitespace() {
Token token = Current();

// Check for a valid deref operation token
{
if (!Check(DereferenceTypes)) {
Whitespace();
if (!Check(DereferenceTypes)) {
Whitespace();

token = Current();
token = Current();

if (!Check(WhitespacedDereferenceTypes)) {
break;
}
if (!Check(WhitespacedDereferenceTypes)) {
break;
}
}

Expand Down Expand Up @@ -2259,46 +2257,49 @@ private void BracketWhitespace() {
break;
}

DMASTDereference.Operation operation = new() {
Kind = DMASTDereference.OperationKind.Invalid,
};
DMASTDereference.Operation operation;

switch (token.Type) {
case TokenType.DM_Period:
case TokenType.DM_QuestionPeriod:
case TokenType.DM_Colon:
case TokenType.DM_QuestionColon: {
DMASTIdentifier identifier = Identifier();
var identifier = Identifier();

operation.Kind = token.Type switch {
TokenType.DM_Period => DMASTDereference.OperationKind.Field,
TokenType.DM_QuestionPeriod => DMASTDereference.OperationKind.FieldSafe,
TokenType.DM_Colon => DMASTDereference.OperationKind.FieldSearch,
TokenType.DM_QuestionColon => DMASTDereference.OperationKind.FieldSafeSearch,
_ => throw new InvalidOperationException(),
};

operation.Identifier = identifier;
if (identifier == null) {
DMCompiler.Emit(WarningCode.BadToken, token.Location, "Identifier expected");
return new DMASTConstantNull(token.Location);
}

operation = new DMASTDereference.FieldOperation {
Location = identifier.Location,
Safe = token.Type is TokenType.DM_QuestionPeriod or TokenType.DM_QuestionColon,
Identifier = identifier.Identifier,
NoSearch = token.Type is TokenType.DM_Colon or TokenType.DM_QuestionColon
};
break;
}

case TokenType.DM_LeftBracket:
case TokenType.DM_QuestionLeftBracket: {
ternaryBHasPriority = true;

Whitespace();
DMASTExpression index = Expression();
ConsumeRightBracket();
ternaryBHasPriority = true;

operation.Kind = token.Type switch {
TokenType.DM_LeftBracket => DMASTDereference.OperationKind.Index,
TokenType.DM_QuestionLeftBracket => DMASTDereference.OperationKind.IndexSafe,
_ => throw new InvalidOperationException(),
};
Whitespace();
var index = Expression();
ConsumeRightBracket();

operation.Index = index;
if (index == null) {
DMCompiler.Emit(WarningCode.BadToken, token.Location, "Expression expected");
return new DMASTConstantNull(token.Location);
}

operation = new DMASTDereference.IndexOperation {
Index = index,
Location = index.Location,
Safe = token.Type is TokenType.DM_QuestionLeftBracket
};
break;
}

default:
throw new InvalidOperationException("unhandled dereference token");
Expand All @@ -2308,39 +2309,26 @@ private void BracketWhitespace() {
if (allowCalls) {
Whitespace();

DMASTCallParameter[] parameters = ProcCall();
var parameters = ProcCall();

if (parameters != null) {
ternaryBHasPriority = true;

switch (operation.Kind) {
case DMASTDereference.OperationKind.Field:
operation.Kind = DMASTDereference.OperationKind.Call;
operation.Parameters = parameters;
break;

case DMASTDereference.OperationKind.FieldSafe:
operation.Kind = DMASTDereference.OperationKind.CallSafe;
operation.Parameters = parameters;
break;

case DMASTDereference.OperationKind.FieldSearch:
operation.Kind = DMASTDereference.OperationKind.CallSearch;
operation.Parameters = parameters;
break;

case DMASTDereference.OperationKind.FieldSafeSearch:
operation.Kind = DMASTDereference.OperationKind.CallSafeSearch;
operation.Parameters = parameters;
switch (operation) {
case DMASTDereference.FieldOperation fieldOperation:
operation = new DMASTDereference.CallOperation {
Parameters = parameters,
Location = fieldOperation.Location,
Safe = fieldOperation.Safe,
Identifier = fieldOperation.Identifier,
NoSearch = fieldOperation.NoSearch
};
break;

case DMASTDereference.OperationKind.Index:
case DMASTDereference.OperationKind.IndexSafe:
Error("attempt to call an invalid l-value");
return null;
case DMASTDereference.IndexOperation:
DMCompiler.Emit(WarningCode.BadToken, token.Location, "Attempt to call an invalid l-value");
return new DMASTConstantNull(token.Location);

case DMASTDereference.OperationKind.Call:
case DMASTDereference.OperationKind.CallSafe:
default:
throw new InvalidOperationException("unhandled dereference operation kind");
}
Expand All @@ -2350,7 +2338,7 @@ private void BracketWhitespace() {
operations.Add(operation);
}

if (operations.Any()) {
if (operations.Count != 0) {
Whitespace();
return new DMASTDereference(expression.Location, expression, operations.ToArray());
}
Expand All @@ -2360,7 +2348,7 @@ private void BracketWhitespace() {
return expression;
}

private DMASTExpression ParseProcCall(DMASTExpression expression) {
private DMASTExpression? ParseProcCall(DMASTExpression? expression) {
if (expression is not (DMASTCallable or DMASTIdentifier or DMASTGlobalIdentifier)) return expression;

Whitespace();
Expand Down
6 changes: 6 additions & 0 deletions DMCompiler/DM/DMExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ public virtual string GetNameof(DMObject dmObject, DMProc proc) {
throw new CompileAbortException(Location, "nameof: requires a var, proc reference, or type path");
}

/// <summary>
/// Determines whether the expression returns an ambiguous path.
/// </summary>
/// <remarks>Dereferencing these expressions will always skip validation via the "expr:y" operation.</remarks>
public virtual bool PathIsFuzzy => false;

public virtual DreamPath? Path => null;

public virtual DreamPath? NestedPath => Path;
Expand Down
2 changes: 2 additions & 0 deletions DMCompiler/DM/Expressions/Binary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ public override void EmitPushValue(DMObject dmObject, DMProc proc) {

// x & y
sealed class BinaryAnd : BinaryOp {
public override bool PathIsFuzzy => true;

public BinaryAnd(Location location, DMExpression lhs, DMExpression rhs)
: base(location, lhs, rhs) { }

Expand Down
12 changes: 12 additions & 0 deletions DMCompiler/DM/Expressions/Builtins.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ sealed class New : DMExpression {
private readonly DMExpression _expr;
private readonly ArgumentList _arguments;

public override bool PathIsFuzzy => Path == null;

public New(Location location, DMExpression expr, ArgumentList arguments) : base(location) {
_expr = expr;
_arguments = arguments;
Expand Down Expand Up @@ -345,6 +347,8 @@ public override void EmitPushValue(DMObject dmObject, DMProc proc) {
internal sealed class IsNull : DMExpression {
private readonly DMExpression _value;

public override bool PathIsFuzzy => true;

public IsNull(Location location, DMExpression value) : base(location) {
_value = value;
}
Expand All @@ -359,6 +363,8 @@ public override void EmitPushValue(DMObject dmObject, DMProc proc) {
internal sealed class Length : DMExpression {
private readonly DMExpression _value;

public override bool PathIsFuzzy => true;

public Length(Location location, DMExpression value) : base(location) {
_value = value;
}
Expand All @@ -374,6 +380,8 @@ internal sealed class GetStep : DMExpression {
private readonly DMExpression _ref;
private readonly DMExpression _dir;

public override bool PathIsFuzzy => true;

public GetStep(Location location, DMExpression refValue, DMExpression dir) : base(location) {
_ref = refValue;
_dir = dir;
Expand All @@ -391,6 +399,8 @@ internal sealed class GetDir : DMExpression {
private readonly DMExpression _loc1;
private readonly DMExpression _loc2;

public override bool PathIsFuzzy => true;

public GetDir(Location location, DMExpression loc1, DMExpression loc2) : base(location) {
_loc1 = loc1;
_loc2 = loc2;
Expand All @@ -408,6 +418,8 @@ sealed class List : DMExpression {
private readonly (DMExpression? Key, DMExpression Value)[] _values;
private readonly bool _isAssociative;

public override bool PathIsFuzzy => true;

public List(Location location, (DMExpression? Key, DMExpression Value)[] values) : base(location) {
_values = values;

Expand Down
Loading

0 comments on commit cf334d6

Please sign in to comment.