From f69bc6cdb10aab2d1b202668cb7ecbcc0ddf33f2 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Fri, 7 Jul 2017 14:30:26 -0700 Subject: Fully parse function bodies, even in "rewriter" mode This is in anticipation of needing to have more complete knowledge to be able to handle user code that `import`s library functionality. The big picture of this change is just to remove the `UnparsedStmt` class that was used to hold the bodies of user functions as opaque token streams, and thus to let the full parser and compiler loose on that code. That is the easy part, of course, and the hard part is all the fixes that this requires in the rest of the compielr to make this even remotely work. Subsequent commit address a lot of other issues, so this particular commit mostly represents work-in-progress. One detail is that this change puts a conditional around nearly every diagnostic message in `check.cpp` to suppress thing when in rewriter mode. I have yet to check how that works out if there are errors in anything we actually need to understand for the purposes of generating reflection data. --- source/slang/emit.cpp | 166 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 110 insertions(+), 56 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index df74d19e4..830ff6d97 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -238,7 +238,9 @@ struct EmitVisitor if (isReservedWord(name)) { - name = name + "_"; + // TODO(tfoley): Need to put this back in, as part of lowering. + +// name = name + "_"; } advanceToSourceLocation(loc); @@ -970,6 +972,14 @@ struct EmitVisitor { e = derefExpr->base; } + + if (auto declRefExpr = e.As()) + { + auto decl = declRefExpr->declRef.getDecl(); + if (decl && decl->HasModifier()) + return true; + } + // Is the expression referencing a constant buffer? if (auto cbufferType = e->Type->As()) { @@ -1308,6 +1318,13 @@ struct EmitVisitor if(needClose) Emit(")"); } + void visitParenExpr(ParenExpr* expr, ExprEmitArg const& arg) + { + Emit("("); + EmitExpr(expr->base); + Emit(")"); + } + void visitAssignExpr(AssignExpr* assignExpr, ExprEmitArg const& arg) { auto outerPrec = arg.outerPrec; @@ -1318,6 +1335,64 @@ struct EmitVisitor if(needClose) Emit(")"); } + void emitUncheckedCallExpr( + RefPtr callExpr, + String const& funcName, + ExprEmitArg const& arg) + { + auto outerPrec = arg.outerPrec; + auto funcExpr = callExpr->FunctionExpr; + + // This can occur when we are dealing with unchecked input syntax, + // because we are in "rewriter" mode. In this case we should go + // ahead and emit things in the form that they were written. + if( auto infixExpr = callExpr.As() ) + { + EmitBinExpr( + outerPrec, + kPrecedence_Comma, + funcName.Buffer(), + callExpr); + } + else if( auto prefixExpr = callExpr.As() ) + { + EmitUnaryExpr( + outerPrec, + kPrecedence_Prefix, + funcName.Buffer(), + "", + callExpr); + } + else if(auto postfixExpr = callExpr.As()) + { + EmitUnaryExpr( + outerPrec, + kPrecedence_Postfix, + "", + funcName.Buffer(), + callExpr); + } + else + { + bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); + + auto funcExpr = callExpr->FunctionExpr; + EmitExpr(funcExpr); + + Emit("("); + UInt argCount = callExpr->Arguments.Count(); + for (UInt aa = 0; aa < argCount; ++aa) + { + if (aa != 0) Emit(", "); + EmitExpr(callExpr->Arguments[aa]); + } + Emit(")"); + + if (needClose) Emit(")"); + } + } + + void visitInvokeExpressionSyntaxNode( RefPtr callExpr, ExprEmitArg const& arg) @@ -1331,39 +1406,7 @@ struct EmitVisitor auto funcDecl = funcDeclRef.getDecl(); if(!funcDecl) { - // This can occur when we are dealing with unchecked input syntax, - // because we are in "rewriter" mode. In this case we should go - // ahead and emit things in the form that they were written. - if( auto infixExpr = callExpr.As() ) - { - EmitBinExpr( - outerPrec, - kPrecedence_Comma, - funcDeclRefExpr->name.Buffer(), - callExpr); - } - else if( auto prefixExpr = callExpr.As() ) - { - EmitUnaryExpr( - outerPrec, - kPrecedence_Prefix, - funcDeclRefExpr->name.Buffer(), - "", - callExpr); - } - else if(auto postfixExpr = callExpr.As()) - { - EmitUnaryExpr( - outerPrec, - kPrecedence_Postfix, - "", - funcDeclRefExpr->name.Buffer(), - callExpr); - } - else - { - emitSimpleCallExpr(callExpr, outerPrec); - } + emitUncheckedCallExpr(callExpr, funcDeclRef.GetName(), arg); return; } else if (auto intrinsicOpModifier = funcDecl->FindModifier()) @@ -1408,6 +1451,7 @@ struct EmitVisitor case IntrinsicOp::Sequence: EmitBinExpr(outerPrec, kPrecedence_Comma, ",", callExpr); return; #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryExpr(outerPrec, kPrecedence_Prefix, #OP, "", callExpr); return + CASE(Pos, +); CASE(Neg, -); CASE(Not, !); CASE(BitNot, ~); @@ -1603,6 +1647,11 @@ struct EmitVisitor } } } + else if (auto overloadedExpr = funcExpr.As()) + { + emitUncheckedCallExpr(callExpr, overloadedExpr->lookupResult2.getName(), arg); + return; + } // Fall through to default handling... emitSimpleCallExpr(callExpr, outerPrec); @@ -1633,7 +1682,16 @@ struct EmitVisitor Emit("."); } - emitName(memberExpr->declRef.GetName()); + if (!memberExpr->declRef) + { + // This case arises when checking didn't find anything, but we were + // in "rewrite" mode so we blazed ahead anyway. + emitName(memberExpr->name); + } + else + { + emit(memberExpr->declRef.GetName()); + } if(needClose) Emit(")"); } @@ -1655,14 +1713,17 @@ struct EmitVisitor if(needClose) Emit(")"); } - void visitIndexExpressionSyntaxNode(IndexExpressionSyntaxNode* indexExpr, ExprEmitArg const& arg) + void visitIndexExpressionSyntaxNode(IndexExpressionSyntaxNode* subscriptExpr, ExprEmitArg const& arg) { auto outerPrec = arg.outerPrec; bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); - EmitExprWithPrecedence(indexExpr->BaseExpression, kPrecedence_Postfix); + EmitExprWithPrecedence(subscriptExpr->BaseExpression, kPrecedence_Postfix); Emit("["); - EmitExpr(indexExpr->IndexExpression); + if (auto indexExpr = subscriptExpr->IndexExpression) + { + EmitExpr(indexExpr); + } Emit("]"); if(needClose) Emit(")"); @@ -1696,7 +1757,7 @@ struct EmitVisitor } else { - emitName(varExpr->name); + emit(varExpr->name); } if(needClose) Emit(")"); @@ -1777,6 +1838,16 @@ struct EmitVisitor void visitTypeCastExpressionSyntaxNode(TypeCastExpressionSyntaxNode* castExpr, ExprEmitArg const& arg) { + if (context->isRewrite) + { + if (dynamic_cast(castExpr)) + { + // This was an implicit cast, so don't try to output it + EmitExprWithPrecedence(castExpr->Expression, arg.outerPrec); + return; + } + } + bool needClose = false; switch(context->shared->target) { @@ -1858,18 +1929,6 @@ struct EmitVisitor } } - - void EmitUnparsedStmt(RefPtr stmt) - { - // TODO: actually emit the tokens that made up the statement... - Emit("{\n"); - for( auto& token : stmt->tokens ) - { - emitTokenWithLocation(token); - } - Emit("}\n"); - } - void EmitStmt(RefPtr stmt) { // Try to ensure that debugging can find the right location @@ -1888,11 +1947,6 @@ struct EmitVisitor } return; } - else if( auto unparsedStmt = stmt.As() ) - { - EmitUnparsedStmt(unparsedStmt); - return; - } else if (auto exprStmt = stmt.As()) { EmitExpr(exprStmt->Expression); -- cgit v1.2.3 From 836a9985ac8540c2c9b27fd438ce757dc6884b9b Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Sat, 8 Jul 2017 16:44:48 -0700 Subject: Try to emit expressions with minimal parentheses When emitting code it is easy to be overly defensive and emit tons of extra parentheses. In some cases these are just annoying, and make the output more cluttered than it needs to be. In other cases, though, being over-aggressive here can actually break things when a downstream compiler has more stringent requirements (e.g., doesn't allow a general expression for the function in a call expression, but only a particular set of atomic/postfix expressions). This wasn't as bad when we weren't parsing function bodies in user code, but now that we *are* it becomes important to not emit bad parentheses and screw up their code. At the same time, though, we don't want to fail to output them and silently break code. A nice property today is that we preserve parentheses in the input code, so hopefully we don't ever break operator precedence. The code already had an approach to avoid *some* parens, by tracking an "outer precedence" and only emitting parens for an expression if they were needed relative to this outer precedence. That approach doesn't handle associativity, and so it doesn't work for things like chains of postfix operators. The new approach basically tracks *two* outer precedence levels: one on the left, and one on the right. When recursing into a sub-expression of an op (e.g., the `A` in `A + B`) on of the precdence levels for the recursive call will come from the outer environment, and the other from the operation itself (e.g., `A` has `(X, +)` as its left/right precdence, where `X` is whatever was to the left of `A + B`, while `B` gets `(+,Y)`). One more piece of the puzzle is that an operator like `+` actually exposes *two* precedence levels: one for the left-hand side and one for the right-hand, so that if both `A` and `B` are themselves uses of `+`, `A` won't get parens, but `B` will. Finally, when we have an un-checked application of an operator (which our AST stores as something like a function-call node), we do a little lookup step to find the corresponding operator and its precedence (while for things that actually got resolved we *know* the precedence. --- source/slang/emit.cpp | 431 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 315 insertions(+), 116 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 830ff6d97..a1d95ca7e 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -84,6 +84,194 @@ static String getStringOrIdentifierTokenValue( } +enum EPrecedence +{ +#define LEFT(NAME) \ + kEPrecedence_##NAME##_Left, \ + kEPrecedence_##NAME##_Right + +#define RIGHT(NAME) \ + kEPrecedence_##NAME##_Right, \ + kEPrecedence_##NAME##_Left + +#define NONASSOC(NAME) \ + kEPrecedence_##NAME##_Left, \ + kEPrecedence_##NAME##_Right = kEPrecedence_##NAME##_Left + + NONASSOC(None), + LEFT(Comma), + + NONASSOC(General), + + RIGHT(Assign), + + RIGHT(Conditional), + + LEFT(Or), + LEFT(And), + LEFT(BitOr), + LEFT(BitXor), + LEFT(BitAnd), + + LEFT(Equality), + LEFT(Relational), + LEFT(Shift), + LEFT(Additive), + LEFT(Multiplicative), + RIGHT(Prefix), + LEFT(Postfix), + NONASSOC(Atomic), + +#if 0 + + kEPrecedence_None, + kEPrecedence_Comma, + + kEPrecedence_Assign, + kEPrecedence_AddAssign = kEPrecedence_Assign, + kEPrecedence_SubAssign = kEPrecedence_Assign, + kEPrecedence_MulAssign = kEPrecedence_Assign, + kEPrecedence_DivAssign = kEPrecedence_Assign, + kEPrecedence_ModAssign = kEPrecedence_Assign, + kEPrecedence_LshAssign = kEPrecedence_Assign, + kEPrecedence_RshAssign = kEPrecedence_Assign, + kEPrecedence_OrAssign = kEPrecedence_Assign, + kEPrecedence_AndAssign = kEPrecedence_Assign, + kEPrecedence_XorAssign = kEPrecedence_Assign, + + kEPrecedence_General = kEPrecedence_Assign, + + kEPrecedence_Conditional, // "ternary" + kEPrecedence_Or, + kEPrecedence_And, + kEPrecedence_BitOr, + kEPrecedence_BitXor, + kEPrecedence_BitAnd, + + kEPrecedence_Eql, + kEPrecedence_Neq = kEPrecedence_Eql, + + kEPrecedence_Less, + kEPrecedence_Greater = kEPrecedence_Less, + kEPrecedence_Leq = kEPrecedence_Less, + kEPrecedence_Geq = kEPrecedence_Less, + + kEPrecedence_Lsh, + kEPrecedence_Rsh = kEPrecedence_Lsh, + + kEPrecedence_Add, + kEPrecedence_Sub = kEPrecedence_Add, + + kEPrecedence_Mul, + kEPrecedence_Div = kEPrecedence_Mul, + kEPrecedence_Mod = kEPrecedence_Mul, + + kEPrecedence_Prefix, + kEPrecedence_Postfix, + kEPrecedence_Atomic = kEPrecedence_Postfix + +#endif + +}; + +// Info on an op for emit purposes +struct EOpInfo +{ + char const* op; + EPrecedence leftPrecedence; + EPrecedence rightPrecedence; +}; + +#define OP(NAME, TEXT, PREC) \ +static const EOpInfo kEOp_##NAME = { TEXT, kEPrecedence_##PREC##_Left, kEPrecedence_##PREC##_Right, } + +OP(None, "", None); + +OP(Comma, ",", Comma); + +OP(General, "", General); + +OP(Assign, "=", Assign); +OP(AddAssign, "+=", Assign); +OP(SubAssign, "-=", Assign); +OP(MulAssign, "*=", Assign); +OP(DivAssign, "/=", Assign); +OP(ModAssign, "%=", Assign); +OP(LshAssign, "<<=", Assign); +OP(RshAssign, ">>=", Assign); +OP(OrAssign, "|=", Assign); +OP(AndAssign, "&=", Assign); +OP(XorAssign, "^=", Assign); + +OP(Conditional, "?:", Conditional); + +OP(Or, "||", Or); +OP(And, "&&", And); +OP(BitOr, "|", BitOr); +OP(BitXor, "^", BitXor); +OP(BitAnd, "&", BitAnd); + +OP(Eql, "==", Equality); +OP(Neq, "!=", Equality); + +OP(Less, "<", Relational); +OP(Greater, ">", Relational); +OP(Leq, "<=", Relational); +OP(Geq, ">=", Relational); + +OP(Lsh, "<<", Shift); +OP(Rsh, ">>", Shift); + +OP(Add, "+", Additive); +OP(Sub, "-", Additive); + +OP(Mul, "*", Multiplicative); +OP(Div, "/", Multiplicative); +OP(Mod, "%", Multiplicative); + +OP(Prefix, "", Prefix); +OP(Postfix, "", Postfix); +OP(Atomic, "", Atomic); + +#undef OP + +// Table to allow data-driven lookup of an op based on its +// name (to assist when outputting unchecked operator calls) +static EOpInfo const* const kInfixOpInfos[] = +{ + &kEOp_Comma, + &kEOp_Assign, + &kEOp_AddAssign, + &kEOp_SubAssign, + &kEOp_MulAssign, + &kEOp_DivAssign, + &kEOp_ModAssign, + &kEOp_LshAssign, + &kEOp_RshAssign, + &kEOp_OrAssign, + &kEOp_AndAssign, + &kEOp_XorAssign, + &kEOp_Or, + &kEOp_And, + &kEOp_BitOr, + &kEOp_BitXor, + &kEOp_BitAnd, + &kEOp_Eql, + &kEOp_Neq, + &kEOp_Less, + &kEOp_Greater, + &kEOp_Leq, + &kEOp_Geq, + &kEOp_Lsh, + &kEOp_Rsh, + &kEOp_Add, + &kEOp_Sub, + &kEOp_Mul, + &kEOp_Div, + &kEOp_Mod, +}; + + // // represents a declarator for use in emitting types @@ -113,7 +301,7 @@ struct TypeEmitArg struct ExprEmitArg { - int outerPrec; + EOpInfo outerPrec; }; struct DeclEmitArg @@ -989,73 +1177,30 @@ struct EmitVisitor return false; } - enum - { - kPrecedence_None, - kPrecedence_Comma, - - kPrecedence_Assign, - kPrecedence_AddAssign = kPrecedence_Assign, - kPrecedence_SubAssign = kPrecedence_Assign, - kPrecedence_MulAssign = kPrecedence_Assign, - kPrecedence_DivAssign = kPrecedence_Assign, - kPrecedence_ModAssign = kPrecedence_Assign, - kPrecedence_LshAssign = kPrecedence_Assign, - kPrecedence_RshAssign = kPrecedence_Assign, - kPrecedence_OrAssign = kPrecedence_Assign, - kPrecedence_AndAssign = kPrecedence_Assign, - kPrecedence_XorAssign = kPrecedence_Assign, - - kPrecedence_General = kPrecedence_Assign, - - kPrecedence_Conditional, // "ternary" - kPrecedence_Or, - kPrecedence_And, - kPrecedence_BitOr, - kPrecedence_BitXor, - kPrecedence_BitAnd, - - kPrecedence_Eql, - kPrecedence_Neq = kPrecedence_Eql, - - kPrecedence_Less, - kPrecedence_Greater = kPrecedence_Less, - kPrecedence_Leq = kPrecedence_Less, - kPrecedence_Geq = kPrecedence_Less, - - kPrecedence_Lsh, - kPrecedence_Rsh = kPrecedence_Lsh, - - kPrecedence_Add, - kPrecedence_Sub = kPrecedence_Add, - - kPrecedence_Mul, - kPrecedence_Div = kPrecedence_Mul, - kPrecedence_Mod = kPrecedence_Mul, - - kPrecedence_Prefix, - kPrecedence_Postfix, - kPrecedence_Atomic = kPrecedence_Postfix - }; - +#if 0 void EmitPostfixExpr(RefPtr expr) { - EmitExprWithPrecedence(expr, kPrecedence_Postfix); + EmitExprWithPrecedence(expr, kEOp_Postfix); } +#endif void EmitExpr(RefPtr expr) { - EmitExprWithPrecedence(expr, kPrecedence_General); + EmitExprWithPrecedence(expr, kEOp_General); } - bool MaybeEmitParens(int outerPrec, int prec) + bool MaybeEmitParens(EOpInfo& outerPrec, EOpInfo prec) { - if (prec <= outerPrec) + bool needParens = (prec.leftPrecedence <= outerPrec.leftPrecedence) + || (prec.rightPrecedence <= outerPrec.rightPrecedence); + + if (needParens) { Emit("("); - return true; + + outerPrec = kEOp_None; } - return false; + return needParens; } // When we are going to emit an expression in an l-value context, @@ -1081,8 +1226,8 @@ struct EmitVisitor } void emitInfixExprImpl( - int outerPrec, - int prec, + EOpInfo outerPrec, + EOpInfo prec, char const* op, RefPtr binExpr, bool isAssign) @@ -1095,30 +1240,30 @@ struct EmitVisitor left = prepareLValueExpr(left); } - EmitExprWithPrecedence(left, prec); + EmitExprWithPrecedence(left, leftSide(outerPrec, prec)); Emit(" "); Emit(op); Emit(" "); - EmitExprWithPrecedence(binExpr->Arguments[1], prec); + EmitExprWithPrecedence(binExpr->Arguments[1], rightSide(prec, outerPrec)); if (needsClose) { Emit(")"); } } - void EmitBinExpr(int outerPrec, int prec, char const* op, RefPtr binExpr) + void EmitBinExpr(EOpInfo outerPrec, EOpInfo prec, char const* op, RefPtr binExpr) { emitInfixExprImpl(outerPrec, prec, op, binExpr, false); } - void EmitBinAssignExpr(int outerPrec, int prec, char const* op, RefPtr binExpr) + void EmitBinAssignExpr(EOpInfo outerPrec, EOpInfo prec, char const* op, RefPtr binExpr) { emitInfixExprImpl(outerPrec, prec, op, binExpr, true); } void emitUnaryExprImpl( - int outerPrec, - int prec, + EOpInfo outerPrec, + EOpInfo prec, char const* preOp, char const* postOp, RefPtr expr, @@ -1133,7 +1278,16 @@ struct EmitVisitor arg = prepareLValueExpr(arg); } - EmitExprWithPrecedence(arg, prec); + if (preOp) + { + EmitExprWithPrecedence(arg, rightSide(prec, outerPrec)); + } + else + { + assert(postOp); + EmitExprWithPrecedence(arg, leftSide(outerPrec, prec)); + } + Emit(postOp); if (needsClose) { @@ -1142,8 +1296,8 @@ struct EmitVisitor } void EmitUnaryExpr( - int outerPrec, - int prec, + EOpInfo outerPrec, + EOpInfo prec, char const* preOp, char const* postOp, RefPtr expr) @@ -1152,8 +1306,8 @@ struct EmitVisitor } void EmitUnaryAssignExpr( - int outerPrec, - int prec, + EOpInfo outerPrec, + EOpInfo prec, char const* preOp, char const* postOp, RefPtr expr) @@ -1216,9 +1370,10 @@ struct EmitVisitor // just an expression of the form `f(a0, a1, ...)` void emitSimpleCallExpr( RefPtr callExpr, - int outerPrec) + EOpInfo outerPrec) { - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); + auto prec = kEOp_Postfix; + bool needClose = MaybeEmitParens(outerPrec, prec); auto funcExpr = callExpr->FunctionExpr; if (auto funcDeclRefExpr = funcExpr.As()) @@ -1232,13 +1387,13 @@ struct EmitVisitor else { // default case: just emit the decl ref - EmitExpr(funcExpr); + EmitExprWithPrecedence(funcExpr, leftSide(outerPrec, prec)); } } else { // default case: just emit the expression - EmitPostfixExpr(funcExpr); + EmitExprWithPrecedence(funcExpr, leftSide(outerPrec, prec)); } Emit("("); @@ -1283,7 +1438,23 @@ struct EmitVisitor emit("\""); } - void EmitExprWithPrecedence(RefPtr expr, int outerPrec) + EOpInfo leftSide(EOpInfo const& outerPrec, EOpInfo const& prec) + { + EOpInfo result; + result.leftPrecedence = outerPrec.leftPrecedence; + result.rightPrecedence = prec.leftPrecedence; + return result; + } + + EOpInfo rightSide(EOpInfo const& prec, EOpInfo const& outerPrec) + { + EOpInfo result; + result.leftPrecedence = prec.rightPrecedence; + result.rightPrecedence = outerPrec.rightPrecedence; + return result; + } + + void EmitExprWithPrecedence(RefPtr expr, EOpInfo outerPrec) { ExprEmitArg arg; arg.outerPrec = outerPrec; @@ -1291,6 +1462,14 @@ struct EmitVisitor ExprVisitorWithArg::dispatch(expr, arg); } + void EmitExprWithPrecedence(RefPtr expr, EPrecedence leftPrec, EPrecedence rightPrec) + { + EOpInfo outerPrec; + outerPrec.leftPrecedence = leftPrec; + outerPrec.rightPrecedence = rightPrec; + } + + #define UNEXPECTED(NAME) \ void visit##NAME(NAME*, ExprEmitArg const&) \ { Emit(#NAME); } @@ -1299,39 +1478,43 @@ struct EmitVisitor #undef UNEXPECTED - void visitSharedTypeExpr(SharedTypeExpr* expr, ExprEmitArg const& arg) + void visitSharedTypeExpr(SharedTypeExpr* expr, ExprEmitArg const&) { emitTypeExp(expr->base); } void visitSelectExpressionSyntaxNode(SelectExpressionSyntaxNode* selectExpr, ExprEmitArg const& arg) { + auto prec = kEOp_Conditional; auto outerPrec = arg.outerPrec; - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Conditional); + bool needClose = MaybeEmitParens(outerPrec, kEOp_Conditional); + + // TODO(tfoley): Need to ver the precedence here... - EmitExprWithPrecedence(selectExpr->Arguments[0], kPrecedence_Conditional); + EmitExprWithPrecedence(selectExpr->Arguments[0], leftSide(outerPrec, prec)); Emit(" ? "); - EmitExprWithPrecedence(selectExpr->Arguments[1], kPrecedence_Conditional); + EmitExprWithPrecedence(selectExpr->Arguments[1], prec); Emit(" : "); - EmitExprWithPrecedence(selectExpr->Arguments[2], kPrecedence_Conditional); + EmitExprWithPrecedence(selectExpr->Arguments[2], rightSide(prec, outerPrec)); if(needClose) Emit(")"); } - void visitParenExpr(ParenExpr* expr, ExprEmitArg const& arg) + void visitParenExpr(ParenExpr* expr, ExprEmitArg const&) { Emit("("); - EmitExpr(expr->base); + EmitExprWithPrecedence(expr->base, kEOp_None); Emit(")"); } void visitAssignExpr(AssignExpr* assignExpr, ExprEmitArg const& arg) { + auto prec = kEOp_Assign; auto outerPrec = arg.outerPrec; - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Assign); - EmitExprWithPrecedence(assignExpr->left, kPrecedence_Assign); + bool needClose = MaybeEmitParens(outerPrec, prec); + EmitExprWithPrecedence(assignExpr->left, leftSide(outerPrec, prec)); Emit(" = "); - EmitExprWithPrecedence(assignExpr->right, kPrecedence_Assign); + EmitExprWithPrecedence(assignExpr->right, rightSide(prec, outerPrec)); if(needClose) Emit(")"); } @@ -1348,9 +1531,19 @@ struct EmitVisitor // ahead and emit things in the form that they were written. if( auto infixExpr = callExpr.As() ) { + auto prec = kEOp_Comma; + for (auto opInfo : kInfixOpInfos) + { + if (funcName == opInfo->op) + { + prec = *opInfo; + break; + } + } + EmitBinExpr( outerPrec, - kPrecedence_Comma, + prec, funcName.Buffer(), callExpr); } @@ -1358,7 +1551,7 @@ struct EmitVisitor { EmitUnaryExpr( outerPrec, - kPrecedence_Prefix, + kEOp_Prefix, funcName.Buffer(), "", callExpr); @@ -1367,16 +1560,15 @@ struct EmitVisitor { EmitUnaryExpr( outerPrec, - kPrecedence_Postfix, + kEOp_Postfix, "", funcName.Buffer(), callExpr); } else { - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); + bool needClose = MaybeEmitParens(outerPrec, kEOp_Postfix); - auto funcExpr = callExpr->FunctionExpr; EmitExpr(funcExpr); Emit("("); @@ -1413,7 +1605,7 @@ struct EmitVisitor { switch (intrinsicOpModifier->op) { - #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitBinExpr(outerPrec, kPrecedence_##NAME, #OP, callExpr); return + #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitBinExpr(outerPrec, kEOp_##NAME, #OP, callExpr); return CASE(Mul, *); CASE(Div, / ); CASE(Mod, %); @@ -1434,7 +1626,7 @@ struct EmitVisitor CASE(Or, || ); #undef CASE - #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitBinAssignExpr(outerPrec, kPrecedence_##NAME, #OP, callExpr); return + #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitBinAssignExpr(outerPrec, kEOp_##NAME, #OP, callExpr); return CASE(Assign, =); CASE(AddAssign, +=); CASE(SubAssign, -=); @@ -1448,21 +1640,21 @@ struct EmitVisitor CASE(XorAssign, ^=); #undef CASE - case IntrinsicOp::Sequence: EmitBinExpr(outerPrec, kPrecedence_Comma, ",", callExpr); return; + case IntrinsicOp::Sequence: EmitBinExpr(outerPrec, kEOp_Comma, ",", callExpr); return; - #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryExpr(outerPrec, kPrecedence_Prefix, #OP, "", callExpr); return + #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryExpr(outerPrec, kEOp_Prefix, #OP, "", callExpr); return CASE(Pos, +); CASE(Neg, -); CASE(Not, !); CASE(BitNot, ~); #undef CASE - #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryAssignExpr(outerPrec, kPrecedence_Prefix, #OP, "", callExpr); return + #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryAssignExpr(outerPrec, kEOp_Prefix, #OP, "", callExpr); return CASE(PreInc, ++); CASE(PreDec, --); #undef CASE - #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryAssignExpr(outerPrec, kPrecedence_Postfix, "", #OP, callExpr); return + #define CASE(NAME, OP) case IntrinsicOp::NAME: EmitUnaryAssignExpr(outerPrec, kEOp_Postfix, "", #OP, callExpr); return CASE(PostInc, ++); CASE(PostDec, --); #undef CASE @@ -1661,8 +1853,9 @@ struct EmitVisitor void visitMemberExpressionSyntaxNode(MemberExpressionSyntaxNode* memberExpr, ExprEmitArg const& arg) { + auto prec = kEOp_Postfix; auto outerPrec = arg.outerPrec; - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); + bool needClose = MaybeEmitParens(outerPrec, prec); // TODO(tfoley): figure out a good way to reference // declarations that might be generic and/or might @@ -1678,7 +1871,7 @@ struct EmitVisitor } else { - EmitExprWithPrecedence(memberExpr->BaseExpression, kPrecedence_Postfix); + EmitExprWithPrecedence(memberExpr->BaseExpression, leftSide(outerPrec, prec)); Emit("."); } @@ -1698,10 +1891,11 @@ struct EmitVisitor void visitSwizzleExpr(SwizzleExpr* swizExpr, ExprEmitArg const& arg) { + auto prec = kEOp_Postfix; auto outerPrec = arg.outerPrec; - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); + bool needClose = MaybeEmitParens(outerPrec, prec); - EmitExprWithPrecedence(swizExpr->base, kPrecedence_Postfix); + EmitExprWithPrecedence(swizExpr->base, leftSide(outerPrec, prec)); Emit("."); static const char* kComponentNames[] = { "x", "y", "z", "w" }; int elementCount = swizExpr->elementCount; @@ -1715,10 +1909,11 @@ struct EmitVisitor void visitIndexExpressionSyntaxNode(IndexExpressionSyntaxNode* subscriptExpr, ExprEmitArg const& arg) { + auto prec = kEOp_Postfix; auto outerPrec = arg.outerPrec; - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Postfix); + bool needClose = MaybeEmitParens(outerPrec, prec); - EmitExprWithPrecedence(subscriptExpr->BaseExpression, kPrecedence_Postfix); + EmitExprWithPrecedence(subscriptExpr->BaseExpression, leftSide(outerPrec, prec)); Emit("["); if (auto indexExpr = subscriptExpr->IndexExpression) { @@ -1729,14 +1924,16 @@ struct EmitVisitor if(needClose) Emit(")"); } - void visitOverloadedExpr(OverloadedExpr* expr, ExprEmitArg const& arg) + void visitOverloadedExpr(OverloadedExpr* expr, ExprEmitArg const&) { emitName(expr->lookupResult2.getName()); } void visitVarExpressionSyntaxNode(VarExpressionSyntaxNode* varExpr, ExprEmitArg const& arg) { - bool needClose = MaybeEmitParens(arg.outerPrec, kPrecedence_Atomic); + auto prec = kEOp_Atomic; + auto outerPrec = arg.outerPrec; + bool needClose = MaybeEmitParens(outerPrec, kEOp_Atomic); // TODO: This won't be valid if we had to generate a qualified // reference for some reason. @@ -1765,16 +1962,14 @@ struct EmitVisitor void visitDerefExpr(DerefExpr* derefExpr, ExprEmitArg const& arg) { - auto outerPrec = arg.outerPrec; - // TODO(tfoley): dereference shouldn't always be implicit - EmitExprWithPrecedence(derefExpr->base, outerPrec); + ExprVisitorWithArg::dispatch(derefExpr->base, arg); } void visitConstantExpressionSyntaxNode(ConstantExpressionSyntaxNode* litExpr, ExprEmitArg const& arg) { auto outerPrec = arg.outerPrec; - bool needClose = MaybeEmitParens(outerPrec, kPrecedence_Atomic); + bool needClose = MaybeEmitParens(outerPrec, kEOp_Atomic); char const* suffix = ""; auto type = litExpr->Type.type; @@ -1843,7 +2038,7 @@ struct EmitVisitor if (dynamic_cast(castExpr)) { // This was an implicit cast, so don't try to output it - EmitExprWithPrecedence(castExpr->Expression, arg.outerPrec); + ExprVisitorWithArg::dispatch(castExpr->Expression, arg); return; } } @@ -1862,19 +2057,23 @@ struct EmitVisitor default: // HLSL (and C/C++) prefer cast syntax // (In fact, HLSL doesn't allow constructor syntax for some conversions it allows as a cast) - needClose = MaybeEmitParens(arg.outerPrec, kPrecedence_Prefix); - - Emit("("); - EmitType(castExpr->Type); - Emit(")("); - EmitExpr(castExpr->Expression); - Emit(")"); + { + auto prec = kEOp_Prefix; + auto outerPrec = arg.outerPrec; + needClose = MaybeEmitParens(outerPrec, prec); + + Emit("("); + EmitType(castExpr->Type); + Emit(")("); + EmitExpr(castExpr->Expression); + Emit(")"); + } break; } if(needClose) Emit(")"); } - void visitInitializerListExpr(InitializerListExpr* expr, ExprEmitArg const& arg) + void visitInitializerListExpr(InitializerListExpr* expr, ExprEmitArg const&) { Emit("{ "); for(auto& arg : expr->args) -- cgit v1.2.3 From 6233f9b35f1901ca33c53ce37f9b1517e91e1d79 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Sat, 8 Jul 2017 17:04:31 -0700 Subject: Revise how hidden implicit casts are recognized. The old approach used an `isRewriter` flag in the emit logic, but I kind of need that flag to go away. Instead, I now how the semantic checking pass detect whether an implicitly-generated type cast is in rewriter code, and if so it uses the new `HiddenImplicitCastExpr` AST node. The emit logic then looks for that specific node and eliminates it. --- source/slang/check.cpp | 14 +++++++++++++- source/slang/emit.cpp | 17 +++++++---------- source/slang/expr-defs.h | 11 +++++++++++ source/slang/parser.cpp | 2 +- 4 files changed, 32 insertions(+), 12 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/check.cpp b/source/slang/check.cpp index f21f9480c..556f141c0 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -987,7 +987,19 @@ namespace Slang RefPtr toType, RefPtr fromExpr) { - auto castExpr = new ImplicitCastExpr(); + // In "rewrite" mode, we will generate a different syntax node + // to indicate that this type-cast was implicitly generated + // by the compiler, and shouldn't appear in the output code. + RefPtr castExpr; + if (isRewriteMode()) + { + castExpr = new HiddenImplicitCastExpr(); + } + else + { + castExpr = new ImplicitCastExpr(); + } + castExpr->Position = fromExpr->Position; castExpr->TargetType.type = toType; castExpr->Type = QualType(toType); diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index a1d95ca7e..a8c0083c1 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -2031,18 +2031,15 @@ struct EmitVisitor if(needClose) Emit(")"); } - void visitTypeCastExpressionSyntaxNode(TypeCastExpressionSyntaxNode* castExpr, ExprEmitArg const& arg) + void visitHiddenImplicitCastExpr(HiddenImplicitCastExpr* castExpr, ExprEmitArg const& arg) { - if (context->isRewrite) - { - if (dynamic_cast(castExpr)) - { - // This was an implicit cast, so don't try to output it - ExprVisitorWithArg::dispatch(castExpr->Expression, arg); - return; - } - } + // This was an implicit cast inserted in code parsed in "rewriter" mode, + // so we don't want to output it and change what the user's code looked like. + ExprVisitorWithArg::dispatch(castExpr->Expression, arg); + } + void visitTypeCastExpressionSyntaxNode(TypeCastExpressionSyntaxNode* castExpr, ExprEmitArg const& arg) + { bool needClose = false; switch(context->shared->target) { diff --git a/source/slang/expr-defs.h b/source/slang/expr-defs.h index 5ca6629b9..0dac324b9 100644 --- a/source/slang/expr-defs.h +++ b/source/slang/expr-defs.h @@ -91,14 +91,25 @@ SYNTAX_CLASS(DerefExpr, ExpressionSyntaxNode) SYNTAX_FIELD(RefPtr, base) END_SYNTAX_CLASS() +// Any operation that performs type-casting SYNTAX_CLASS(TypeCastExpressionSyntaxNode, ExpressionSyntaxNode) SYNTAX_FIELD(TypeExp, TargetType) SYNTAX_FIELD(RefPtr, Expression) END_SYNTAX_CLASS() +// An explicit type-cast that appear in the user's code with `(Type) expr` syntax +SYNTAX_CLASS(ExplicitCastExpr, TypeCastExpressionSyntaxNode) +END_SYNTAX_CLASS() + +// An implicit type-cast inserted during semantic checking SYNTAX_CLASS(ImplicitCastExpr, TypeCastExpressionSyntaxNode) END_SYNTAX_CLASS() +// An implicit type-cast that should also be hidden on output, +// because we don't want to mess with the user's code +SYNTAX_CLASS(HiddenImplicitCastExpr, ImplicitCastExpr) +END_SYNTAX_CLASS() + SIMPLE_SYNTAX_CLASS(SelectExpressionSyntaxNode, OperatorExpressionSyntaxNode) SIMPLE_SYNTAX_CLASS(GenericAppExpr, AppExprBase) diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp index 2056bf809..0efc8cd77 100644 --- a/source/slang/parser.cpp +++ b/source/slang/parser.cpp @@ -3092,7 +3092,7 @@ namespace Slang if (peekTypeName(parser) && parser->LookAheadToken(TokenType::RParent, 1)) { - RefPtr tcexpr = new TypeCastExpressionSyntaxNode(); + RefPtr tcexpr = new ExplicitCastExpr(); parser->FillPosition(tcexpr.Ptr()); tcexpr->TargetType = parser->ParseTypeExp(); parser->ReadToken(TokenType::RParent); -- cgit v1.2.3 From a40b6679931f672a911070fcc7eeb41c52e8b8fd Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Sat, 8 Jul 2017 17:21:37 -0700 Subject: Differentiate HLSL `for` loops in AST HLSL has the bad scoping behavior for `for` loops, and we need to respect that. But, we need to have correct scoping for GLSL, and we'd like it for Slang. We also need to ensure that `for` loops written in a "correct" language get the correct behavior when emitted as HLSL. There was already code to handle this in the emit pass, but it was unfortunately using an `isRewrite` flag to try to tell if the HLSL behavior was wanted. This doesn't work when the code being emitted might come from a mix of languages. This change adds a distinct `UnscopedForStmt` syntax node type, and uses that when parsing HLSL input (bot not for other languages). We make sure to preserve this node type through lowering, and then specialize our emit logic on this case. With this, there are no more remaining uses of `isRewrite` in the emit logic, which is good because it didn't mean what I needed it to mean any more (since we now emit only a single module, that was merged during lowering). --- source/slang/emit.cpp | 17 ++--------------- source/slang/lower.cpp | 16 +++++++++++++--- source/slang/parser.cpp | 33 ++++++++++++++++++++++++++++----- source/slang/stmt-defs.h | 5 +++++ 4 files changed, 48 insertions(+), 23 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index a8c0083c1..24b0dd717 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -57,10 +57,6 @@ struct EmitContext { // The shared context that is in effect SharedEmitContext* shared; - - // Are we in "rewrite" mode, where we are trying to reproduce the input - // code as closely as posible? - bool isRewrite; }; // @@ -2196,12 +2192,10 @@ struct EmitVisitor // The one wrinkle is that HLSL implements the // bad approach to scoping a `for` loop variable, // so we need to avoid those outer `{...}` when - // we are generating HLSL via "rewrite" (that is, - // without our semantic checks). + // we are emitting code that was written in HLSL. // bool brokenScoping = false; - if (context->shared->target == CodeGenTarget::HLSL - && context->isRewrite) + if (forStmt.As()) { brokenScoping = true; } @@ -3406,10 +3400,6 @@ struct EmitVisitor } }; -bool isRewriteRequest( - SourceLanguage sourceLanguage, - CodeGenTarget target); - String emitEntryPoint( EntryPointRequest* entryPoint, ProgramLayout* programLayout, @@ -3465,9 +3455,6 @@ String emitEntryPoint( EmitContext context; context.shared = &sharedContext; - context.isRewrite = isRewriteRequest( - translationUnit->sourceLanguage, - target); EmitVisitor visitor(&context); diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp index c154c96c8..8a4b7a4b1 100644 --- a/source/slang/lower.cpp +++ b/source/slang/lower.cpp @@ -723,10 +723,10 @@ struct LoweringVisitor addStmt(loweredStmt); } - - void visitForStatementSyntaxNode(ForStatementSyntaxNode* stmt) + void lowerForStmtCommon( + RefPtr loweredStmt, + ForStatementSyntaxNode* stmt) { - RefPtr loweredStmt = new ForStatementSyntaxNode(); lowerScopeStmtFields(loweredStmt, stmt); LoweringVisitor subVisitor = pushScope(loweredStmt, stmt); @@ -739,6 +739,16 @@ struct LoweringVisitor addStmt(loweredStmt); } + void visitForStatementSyntaxNode(ForStatementSyntaxNode* stmt) + { + lowerForStmtCommon(new ForStatementSyntaxNode(), stmt); + } + + void visitUnscopedForStmt(UnscopedForStmt* stmt) + { + lowerForStmtCommon(new UnscopedForStmt(), stmt); + } + void visitWhileStatementSyntaxNode(WhileStatementSyntaxNode* stmt) { RefPtr loweredStmt = new WhileStatementSyntaxNode(); diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp index 0efc8cd77..199275a2d 100644 --- a/source/slang/parser.cpp +++ b/source/slang/parser.cpp @@ -2673,12 +2673,32 @@ namespace Slang RefPtr Parser::ParseForStatement() { RefPtr scopeDecl = new ScopeDecl(); - RefPtr stmt = new ForStatementSyntaxNode(); + + // HLSL implements the bad approach to scoping a `for` loop + // variable, and we want to respect that, but *only* when + // parsing HLSL code. + // + + bool brokenScoping = translationUnit->sourceLanguage == SourceLanguage::HLSL; + + // We will create a distinct syntax node class for the unscoped + // case, just so that we can correctly handle it in downstream + // logic. + // + RefPtr stmt; + if (brokenScoping) + { + stmt = new UnscopedForStmt(); + } + else + { + stmt = new ForStatementSyntaxNode(); + } + stmt->scopeDecl = scopeDecl; - // Note(tfoley): HLSL implements `for` with incorrect scoping. - // We need an option to turn on this behavior in a kind of "legacy" mode -// PushScope(scopeDecl.Ptr()); + if(!brokenScoping) + PushScope(scopeDecl.Ptr()); FillPosition(stmt.Ptr()); ReadToken("for"); ReadToken(TokenType::LParent); @@ -2704,7 +2724,10 @@ namespace Slang stmt->SideEffectExpression = ParseExpression(); ReadToken(TokenType::RParent); stmt->Statement = ParseStatement(); -// PopScope(); + + if (!brokenScoping) + PopScope(); + return stmt; } diff --git a/source/slang/stmt-defs.h b/source/slang/stmt-defs.h index 165ffea83..15826abc4 100644 --- a/source/slang/stmt-defs.h +++ b/source/slang/stmt-defs.h @@ -66,6 +66,7 @@ SIMPLE_SYNTAX_CLASS(DefaultStmt, CaseStmtBase) ABSTRACT_SYNTAX_CLASS(LoopStmt, BreakableStmt) END_SYNTAX_CLASS() +// A `for` statement SYNTAX_CLASS(ForStatementSyntaxNode, LoopStmt) SYNTAX_FIELD(RefPtr, InitialStatement) SYNTAX_FIELD(RefPtr, SideEffectExpression) @@ -73,6 +74,10 @@ SYNTAX_CLASS(ForStatementSyntaxNode, LoopStmt) SYNTAX_FIELD(RefPtr, Statement) END_SYNTAX_CLASS() +// A `for` statement in a language that doesn't restrict the scope +// of the loop variable to the body. +SYNTAX_CLASS(UnscopedForStmt, ForStatementSyntaxNode); +END_SYNTAX_CLASS() SYNTAX_CLASS(WhileStatementSyntaxNode, LoopStmt) SYNTAX_FIELD(RefPtr, Predicate) -- cgit v1.2.3 From 780a0bcd3724cad77cb65f931f111273776c9ca4 Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Sat, 8 Jul 2017 17:47:31 -0700 Subject: Add back `UnparsedStmt` If the user doesn't use any `import` declarations, there is no reason to parse their code at all, so having the option of falling back to `UnparsedStmt` can potentially save us some headaches down the road. The new rule now is that if you have the "no checking" flag on, *and* the parser hasn't yet seen any `import` declarations, then it still used `UnparsedStmt` to avoid touching function bodies. Otherwise, I go ahead and parse function bodies, and assume I can rewrite any code I can semantically understand. --- source/slang/check.cpp | 5 +++++ source/slang/emit.cpp | 16 ++++++++++++++ source/slang/lower.cpp | 10 +++++++++ source/slang/parser.cpp | 56 ++++++++++++++++++++++++++++++++++++++++++++++++ source/slang/stmt-defs.h | 7 ++++++ 5 files changed, 94 insertions(+) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/check.cpp b/source/slang/check.cpp index 556f141c0..2b9abef90 100644 --- a/source/slang/check.cpp +++ b/source/slang/check.cpp @@ -1790,6 +1790,11 @@ namespace Slang checkStmt(stmt->NegativeStatement); } + void visitUnparsedStmt(UnparsedStmt*) + { + // Nothing to do + } + void visitEmptyStatementSyntaxNode(EmptyStatementSyntaxNode*) { // Nothing to do diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 24b0dd717..20b9856c5 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -2121,6 +2121,17 @@ struct EmitVisitor } } + void EmitUnparsedStmt(RefPtr stmt) + { + // TODO: actually emit the tokens that made up the statement... + Emit("{\n"); + for( auto& token : stmt->tokens ) + { + emitTokenWithLocation(token); + } + Emit("}\n"); + } + void EmitStmt(RefPtr stmt) { // Try to ensure that debugging can find the right location @@ -2139,6 +2150,11 @@ struct EmitVisitor } return; } + else if( auto unparsedStmt = stmt.As() ) + { + EmitUnparsedStmt(unparsedStmt); + return; + } else if (auto exprStmt = stmt.As()) { EmitExpr(exprStmt->Expression); diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp index 8a4b7a4b1..0d54faf0b 100644 --- a/source/slang/lower.cpp +++ b/source/slang/lower.cpp @@ -688,6 +688,16 @@ struct LoweringVisitor addStmt(loweredStmt); } + void visitUnparsedStmt(UnparsedStmt* stmt) + { + RefPtr loweredStmt = new UnparsedStmt(); + lowerStmtFields(loweredStmt, stmt); + + loweredStmt->tokens = stmt->tokens; + + addStmt(loweredStmt); + } + void visitCaseStmt(CaseStmt* stmt) { RefPtr loweredStmt = new CaseStmt(); diff --git a/source/slang/parser.cpp b/source/slang/parser.cpp index 199275a2d..759fc0c20 100644 --- a/source/slang/parser.cpp +++ b/source/slang/parser.cpp @@ -44,6 +44,11 @@ namespace Slang String fileName; int genericDepth = 0; + // Have we seen any `import` declarations? If so, we need + // to parse function bodies completely, even if we are in + // "rewrite" mode. + bool haveSeenAnyImportDecls = false; + // Is the parser in a "recovering" state? // During recovery we don't emit additional errors, until we find // a token that we expected, when we exit recovery. @@ -802,6 +807,8 @@ namespace Slang static RefPtr parseImportDecl( Parser* parser) { + parser->haveSeenAnyImportDecls = true; + parser->ReadToken("__import"); auto decl = new ImportDecl(); @@ -842,6 +849,8 @@ namespace Slang static RefPtr parsePoundImportDecl( Parser* parser) { + parser->haveSeenAnyImportDecls = true; + Token importToken = parser->ReadToken(TokenType::PoundImport); auto decl = new ImportDecl(); @@ -2599,6 +2608,53 @@ namespace Slang RefPtr Parser::ParseBlockStatement() { + // If we are being asked not to check things *and* we haven't + // seen any `import` declarations yet, then we can safely assume + // that function bodies should be left as-is. + if( (translationUnit->compileFlags & SLANG_COMPILE_FLAG_NO_CHECKING) + && !haveSeenAnyImportDecls ) + { + // We have been asked to parse the input, but not attempt to understand it. + + // TODO: record start/end locations... + + List tokens; + + ReadToken(TokenType::LBrace); + + int depth = 1; + for( ;;) + { + switch( tokenReader.PeekTokenType() ) + { + case TokenType::EndOfFile: + goto done; + + case TokenType::RBrace: + depth--; + if(depth == 0) + goto done; + break; + + case TokenType::LBrace: + depth++; + break; + + default: + break; + } + + auto token = tokenReader.AdvanceToken(); + tokens.Add(token); + } + done: + ReadToken(TokenType::RBrace); + + RefPtr unparsedStmt = new UnparsedStmt(); + unparsedStmt->tokens = tokens; + return unparsedStmt; + } + RefPtr scopeDecl = new ScopeDecl(); RefPtr blockStatement = new BlockStmt(); blockStatement->scopeDecl = scopeDecl; diff --git a/source/slang/stmt-defs.h b/source/slang/stmt-defs.h index 15826abc4..9dea40fbf 100644 --- a/source/slang/stmt-defs.h +++ b/source/slang/stmt-defs.h @@ -16,6 +16,13 @@ SYNTAX_CLASS(BlockStmt, ScopeStmt) SYNTAX_FIELD(RefPtr, body); END_SYNTAX_CLASS() +// A statement that we aren't going to parse or check, because +// we want to let a downstream compiler handle any issues +SYNTAX_CLASS(UnparsedStmt, StatementSyntaxNode) + // The tokens that were contained between `{` and `}` + FIELD(List, tokens) +END_SYNTAX_CLASS() + SIMPLE_SYNTAX_CLASS(EmptyStatementSyntaxNode, StatementSyntaxNode) SIMPLE_SYNTAX_CLASS(DiscardStatementSyntaxNode, StatementSyntaxNode) -- cgit v1.2.3 From 5c864cbd58ea7cca6bec7b1f023422e27f0bcb1f Mon Sep 17 00:00:00 2001 From: Tim Foley Date: Sat, 8 Jul 2017 18:05:02 -0700 Subject: Move renaming logic to lowering pass. Code in Slang that is cross-compiled *might* introduce declarations that collide with language keywords that are reserved in the target. This was previously being dealth with during final code emission, but the challenge there is that we want to allow user code that is being "rewritten" to use whatever identifiers it wants (they know better than us what is an error), and only apply renaming to our own code. The approach here is to apply renaming during lowering - we validate each declaration to make sure its name is valid. Any expressions/types that refer to those declarations will automatically get emitted with the new name (while unchecked expressions will continue to be emitted with their existing name). This isn't quite perfect, since we could in theory still rename a declaration in user code. A more robust version down the line would try to determine if a declaration was nested inside code for the "rewriter." Also note that this does *not* deal with any issues of name conflicts that might arise between modules. That would require a more complete and robust renaming pass, which seems tricky for me to pull off. --- source/slang/emit.cpp | 195 -------------------------------------------- source/slang/lower.cpp | 217 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 217 insertions(+), 195 deletions(-) (limited to 'source/slang/emit.cpp') diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp index 20b9856c5..c135090c1 100644 --- a/source/slang/emit.cpp +++ b/source/slang/emit.cpp @@ -21,9 +21,6 @@ struct SharedEmitContext // The target language we want to generate code for CodeGenTarget target; - // A set of words reserved by the target - Dictionary reservedWords; - // The string of code we've built so far StringBuilder sb; @@ -385,48 +382,12 @@ struct EmitVisitor Emit(text.begin(), text.end()); } - bool isReservedWord(String const& name) - { - return context->shared->reservedWords.TryGetValue(name) != nullptr; - } - void emitName( String const& inName, CodePosition const& loc) { String name = inName; - // By default, we would like to emit a name in the generated - // code exactly as it appeared in the soriginal program. - // When that isn't possible, we'd like to emit a name as - // close to the original as possible (to ensure that existing - // debugging tools still work reasonably well). - // - // One reason why a name might not be allowed as-is is that - // it could collide with a reserved word in the target language. - // Another reason is that it might not follow a naming convention - // imposed by the target (e.g., in GLSL names starting with - // `gl_` or containing `__` are reserved). - // - // Given a name that should not be allowed, we want to - // change it to a name that *is* allowed. e.g., by adding - // `_` to the end of a reserved word. - // - // The next problem this creates is that the modified name - // could not collide with an existing use of the same - // (valid) name. - // - // For now we are going to solve this problem in a simple - // and ad hoc fashion, but longer term we'll want to do - // something sytematic. - - if (isReservedWord(name)) - { - // TODO(tfoley): Need to put this back in, as part of lowering. - -// name = name + "_"; - } - advanceToSourceLocation(loc); emit(name); } @@ -3261,159 +3222,6 @@ struct EmitVisitor throw "unimplemented"; } } - - void registerReservedWord( - String const& name) - { - context->shared->reservedWords.Add(name, name); - } - - void registerReservedWords() - { - #define WORD(NAME) registerReservedWord(#NAME) - - switch (context->shared->target) - { - case CodeGenTarget::GLSL: - WORD(attribute); - WORD(const); - WORD(uniform); - WORD(varying); - WORD(buffer); - - WORD(shared); - WORD(coherent); - WORD(volatile); - WORD(restrict); - WORD(readonly); - WORD(writeonly); - WORD(atomic_unit); - WORD(layout); - WORD(centroid); - WORD(flat); - WORD(smooth); - WORD(noperspective); - WORD(patch); - WORD(sample); - WORD(break); - WORD(continue); - WORD(do); - WORD(for); - WORD(while); - WORD(switch); - WORD(case); - WORD(default); - WORD(if); - WORD(else); - WORD(subroutine); - WORD(in); - WORD(out); - WORD(inout); - WORD(float); - WORD(double); - WORD(int); - WORD(void); - WORD(bool); - WORD(true); - WORD(false); - WORD(invariant); - WORD(precise); - WORD(discard); - WORD(return); - - WORD(lowp); - WORD(mediump); - WORD(highp); - WORD(precision); - WORD(struct); - WORD(uint); - - WORD(common); - WORD(partition); - WORD(active); - WORD(asm); - WORD(class); - WORD(union); - WORD(enum); - WORD(typedef); - WORD(template); - WORD(this); - WORD(resource); - - WORD(goto); - WORD(inline); - WORD(noinline); - WORD(public); - WORD(static); - WORD(extern); - WORD(external); - WORD(interface); - WORD(long); - WORD(short); - WORD(half); - WORD(fixed); - WORD(unsigned); - WORD(superp); - WORD(input); - WORD(output); - WORD(filter); - WORD(sizeof); - WORD(cast); - WORD(namespace); - WORD(using); - - #define CASE(NAME) \ - WORD(NAME ## 2); WORD(NAME ## 3); WORD(NAME ## 4) - - CASE(mat); - CASE(dmat); - CASE(mat2x); - CASE(mat3x); - CASE(mat4x); - CASE(dmat2x); - CASE(dmat3x); - CASE(dmat4x); - CASE(vec); - CASE(ivec); - CASE(bvec); - CASE(dvec); - CASE(uvec); - CASE(hvec); - CASE(fvec); - - #undef CASE - - #define CASE(NAME) \ - WORD(NAME ## 1D); \ - WORD(NAME ## 2D); \ - WORD(NAME ## 3D); \ - WORD(NAME ## Cube); \ - WORD(NAME ## 1DArray); \ - WORD(NAME ## 2DArray); \ - WORD(NAME ## 3DArray); \ - WORD(NAME ## CubeArray);\ - WORD(NAME ## 2DMS); \ - WORD(NAME ## 2DMSArray) \ - /* end */ - - #define CASE2(NAME) \ - CASE(NAME); \ - CASE(i ## NAME); \ - CASE(u ## NAME) \ - /* end */ - - CASE2(sampler); - CASE2(image); - CASE2(texture); - - #undef CASE2 - #undef CASE - break; - - default: - break; - } - } }; String emitEntryPoint( @@ -3474,9 +3282,6 @@ String emitEntryPoint( EmitVisitor visitor(&context); - // TODO: this should only need to take the shared context - visitor.registerReservedWords(); - auto translationUnitSyntax = translationUnit->SyntaxNode.Ptr(); diff --git a/source/slang/lower.cpp b/source/slang/lower.cpp index 0d54faf0b..0bb047791 100644 --- a/source/slang/lower.cpp +++ b/source/slang/lower.cpp @@ -194,8 +194,17 @@ public: struct SharedLoweringContext { ProgramLayout* programLayout; + + // The target we are going to generate code for. + // + // We may need to specialize how constructs get lowered based + // on the capabilities of the target language. CodeGenTarget target; + // A set of words reserved by the target + Dictionary reservedWords; + + RefPtr loweredProgram; Dictionary> loweredDecls; @@ -238,6 +247,165 @@ struct LoweringVisitor CodeGenTarget getTarget() { return shared->target; } + bool isReservedWord(String const& name) + { + return shared->reservedWords.TryGetValue(name) != nullptr; + } + + void registerReservedWord( + String const& name) + { + shared->reservedWords.Add(name, name); + } + + void registerReservedWords() + { + #define WORD(NAME) registerReservedWord(#NAME) + + switch (shared->target) + { + case CodeGenTarget::GLSL: + WORD(attribute); + WORD(const); + WORD(uniform); + WORD(varying); + WORD(buffer); + + WORD(shared); + WORD(coherent); + WORD(volatile); + WORD(restrict); + WORD(readonly); + WORD(writeonly); + WORD(atomic_unit); + WORD(layout); + WORD(centroid); + WORD(flat); + WORD(smooth); + WORD(noperspective); + WORD(patch); + WORD(sample); + WORD(break); + WORD(continue); + WORD(do); + WORD(for); + WORD(while); + WORD(switch); + WORD(case); + WORD(default); + WORD(if); + WORD(else); + WORD(subroutine); + WORD(in); + WORD(out); + WORD(inout); + WORD(float); + WORD(double); + WORD(int); + WORD(void); + WORD(bool); + WORD(true); + WORD(false); + WORD(invariant); + WORD(precise); + WORD(discard); + WORD(return); + + WORD(lowp); + WORD(mediump); + WORD(highp); + WORD(precision); + WORD(struct); + WORD(uint); + + WORD(common); + WORD(partition); + WORD(active); + WORD(asm); + WORD(class); + WORD(union); + WORD(enum); + WORD(typedef); + WORD(template); + WORD(this); + WORD(resource); + + WORD(goto); + WORD(inline); + WORD(noinline); + WORD(public); + WORD(static); + WORD(extern); + WORD(external); + WORD(interface); + WORD(long); + WORD(short); + WORD(half); + WORD(fixed); + WORD(unsigned); + WORD(superp); + WORD(input); + WORD(output); + WORD(filter); + WORD(sizeof); + WORD(cast); + WORD(namespace); + WORD(using); + + #define CASE(NAME) \ + WORD(NAME ## 2); WORD(NAME ## 3); WORD(NAME ## 4) + + CASE(mat); + CASE(dmat); + CASE(mat2x); + CASE(mat3x); + CASE(mat4x); + CASE(dmat2x); + CASE(dmat3x); + CASE(dmat4x); + CASE(vec); + CASE(ivec); + CASE(bvec); + CASE(dvec); + CASE(uvec); + CASE(hvec); + CASE(fvec); + + #undef CASE + + #define CASE(NAME) \ + WORD(NAME ## 1D); \ + WORD(NAME ## 2D); \ + WORD(NAME ## 3D); \ + WORD(NAME ## Cube); \ + WORD(NAME ## 1DArray); \ + WORD(NAME ## 2DArray); \ + WORD(NAME ## 3DArray); \ + WORD(NAME ## CubeArray);\ + WORD(NAME ## 2DMS); \ + WORD(NAME ## 2DMSArray) \ + /* end */ + + #define CASE2(NAME) \ + CASE(NAME); \ + CASE(i ## NAME); \ + CASE(u ## NAME) \ + /* end */ + + CASE2(sampler); + CASE2(image); + CASE2(texture); + + #undef CASE2 + #undef CASE + break; + + default: + break; + } + } + + // // Values // @@ -975,6 +1143,43 @@ struct LoweringVisitor shared->mapLoweredDeclToOriginal.Add(loweredDecl, decl); } + // If the name of the declarations collides with a reserved word + // for the code generation target, then rename it to avoid the conflict + // + // Note that this does *not* implement any kind of comprehensive renaming + // to, e.g., avoid conflicts between user-defined and library functions. + void ensureDeclHasAValidName(Decl* decl) + { + // By default, we would like to emit a name in the generated + // code exactly as it appeared in the original program. + // When that isn't possible, we'd like to emit a name as + // close to the original as possible (to ensure that existing + // debugging tools still work reasonably well). + // + // One reason why a name might not be allowed as-is is that + // it could collide with a reserved word in the target language. + // Another reason is that it might not follow a naming convention + // imposed by the target (e.g., in GLSL names starting with + // `gl_` or containing `__` are reserved). + // + // Given a name that should not be allowed, we want to + // change it to a name that *is* allowed. e.g., by adding + // `_` to the end of a reserved word. + // + // The next problem this creates is that the modified name + // could not collide with an existing use of the same + // (valid) name. + // + // For now we are going to solve this problem in a simple + // and ad hoc fashion, but longer term we'll want to do + // something sytematic. + + if (isReservedWord(decl->getName())) + { + decl->Name.Content.append("_"); + } + } + void lowerDeclCommon( Decl* loweredDecl, Decl* decl) @@ -984,6 +1189,9 @@ struct LoweringVisitor loweredDecl->Position = decl->Position; loweredDecl->Name = decl->getNameToken(); + // Deal with renaming - we shouldn't allow decls with names that are reserved words + ensureDeclHasAValidName(loweredDecl); + // Lower modifiers as needed // HACK: just doing a shallow copy of modifiers, which will @@ -1325,6 +1533,8 @@ struct LoweringVisitor globalVarDecl->Name.Content = info.name; globalVarDecl->Type.type = type; + ensureDeclHasAValidName(globalVarDecl); + addMember(shared->loweredProgram, globalVarDecl); // Add the layout information @@ -1526,6 +1736,8 @@ struct LoweringVisitor localVarDecl->Name.Content = paramDecl->getName(); localVarDecl->Type = lowerType(paramDecl->Type); + ensureDeclHasAValidName(localVarDecl); + subVisitor.addDecl(localVarDecl); EntryPointParamPair paramPair; @@ -1560,6 +1772,8 @@ struct LoweringVisitor resultVarDecl->Name.Content = "_main_result"; resultVarDecl->Type = TypeExp(loweredEntryPointFunc->ReturnType); + ensureDeclHasAValidName(resultVarDecl); + subVisitor.addDecl(resultVarDecl); } @@ -1804,6 +2018,9 @@ LoweredEntryPoint lowerEntryPoint( visitor.shared = &sharedContext; visitor.parentDecl = loweredProgram; + // TODO: this should only need to take the shared context + visitor.registerReservedWords(); + // We need to register the lowered program as the lowered version // of the existing translation unit declaration. -- cgit v1.2.3