Emit `#line` directives more aggressively

We now output a line directive for (nearly) every declaration, statement, and modifier, so that hopefully there will be fewer cases where a downstream error doesn't point to the correct line.

This exposes a lot of issues where we can/should clean up the simplicity of the code we emit (e.g., not output redundant parens; tracking source locations for types better).
These kinds of issues will need to be addressed in follow-on changes.
A few big ones:

- Because GLSL doesn't allow for file names in `#line` directives, we really need to expose some data that can clean up error messages (or can be used by an application to do the same) so that they know which file is which.

- We really need a command line option (and an equivalent API flag) to turn off emission of `#line` directies, so that the user can get moderately clean code as output.

1 files changed, 127 insertions, 17 deletions

diff --git a/source/slang/emit.cpp b/source/slang/emit.cpp
index 21ae498b6..21704f6ff 100644
--- a/source/slang/emit.cpp
+++ b/source/slang/emit.cpp
@@ -19,6 +19,8 @@ struct EmitContext
 
     // Current source position for tracking purposes...
     CodePosition loc;
+    CodePosition nextSourceLocation;
+    bool needToUpdateSourceLocation;
 
     // The target language we want to generate code for
     CodeGenTarget target;
@@ -39,12 +41,26 @@ struct EmitContext
 static void EmitDecl(EmitContext* context, RefPtr<Decl> decl);
 static void EmitDecl(EmitContext* context, RefPtr<DeclBase> declBase);
 static void EmitDeclUsingLayout(EmitContext* context, RefPtr<Decl> decl, RefPtr<VarLayout> layout);
+
+static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, Token const& nameToken);
 static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, String const& name);
 static void EmitType(EmitContext* context, RefPtr<ExpressionType> type);
+
+static void EmitType(EmitContext* context, TypeExp const& typeExp, Token const& nameToken);
+static void EmitType(EmitContext* context, TypeExp const& typeExp, String const& name);
+static void EmitType(EmitContext* context, TypeExp const& typeExp);
+
 static void EmitExpr(EmitContext* context, RefPtr<ExpressionSyntaxNode> expr);
 static void EmitStmt(EmitContext* context, RefPtr<StatementSyntaxNode> stmt);
 static void EmitDeclRef(EmitContext* context, DeclRef<Decl> declRef);
 
+static void advanceToSourceLocation(
+    EmitContext*        context,
+    CodePosition const& sourceLocation);
+
+static void flushSourceLocationChange(
+    EmitContext*        context);
+
 // Low-level emit logic
 
 static void emitRawTextSpan(EmitContext* context, char const* textBegin, char const* textEnd)
@@ -62,6 +78,10 @@ static void emitRawText(EmitContext* context, char const* text)
 
 static void emitTextSpan(EmitContext* context, char const* textBegin, char const* textEnd)
 {
+    // If the source location has changed in a way that required update,
+    // do it now!
+    flushSourceLocationChange(context);
+
     // Emit the raw text
     emitRawTextSpan(context, textBegin, textEnd);
 
@@ -116,7 +136,10 @@ static bool isReservedWord(EmitContext* context, String const& name)
     return context->reservedWords.TryGetValue(name) != nullptr;
 }
 
-static void emitName(EmitContext* context, String const& inName)
+static void emitName(
+    EmitContext*        context,
+    String const&       inName,
+    CodePosition const& loc)
 {
     String name = inName;
 
@@ -149,9 +172,20 @@ static void emitName(EmitContext* context, String const& inName)
         name = name + "_";
     }
 
+    advanceToSourceLocation(context, loc);
     emit(context, name);
 }
 
+static void emitName(EmitContext* context, Token const& nameToken)
+{
+    emitName(context, nameToken.Content, nameToken.Position);
+}
+
+static void emitName(EmitContext* context, String const& name)
+{
+    emitName(context, name, CodePosition());
+}
+
 static void Emit(EmitContext* context, UInt value)
 {
     char buffer[32];
@@ -796,6 +830,10 @@ static void EmitExprWithPrecedence(EmitContext* context, RefPtr<ExpressionSyntax
     {
         needClose = MaybeEmitParens(context, outerPrec, kPrecedence_Atomic);
 
+        // TODO: This won't be valid if we had to generate a qualified
+        // reference for some reason.
+        advanceToSourceLocation(context, varExpr->Position);
+
         // Because of the "rewriter" use case, it is possible that we will
         // be trying to emit an expression that hasn't been wired up to
         // any associated declaration. In that case, we will just emit
@@ -917,6 +955,7 @@ struct EDeclarator
 
     // Used for `Flavor::Name`
     String name;
+    CodePosition loc;
 
     // Used for `Flavor::Array`
     IntVal* elementCount;
@@ -931,7 +970,7 @@ static void EmitDeclarator(EmitContext* context, EDeclarator* declarator)
     switch (declarator->flavor)
     {
     case EDeclarator::Flavor::Name:
-        emitName(context, declarator->name);
+        emitName(context, declarator->name, declarator->loc);
         break;
 
     case EDeclarator::Flavor::Array:
@@ -1040,7 +1079,7 @@ static void emitHLSLTextureType(
     }
     Emit(context, "<");
     EmitType(context, texType->elementType);
-    Emit(context, ">");
+    Emit(context, " >");
 }
 
 static void emitGLSLTextureOrTextureSamplerType(
@@ -1196,7 +1235,6 @@ static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, EDeclara
             break;
         }
 
-        Emit(context, " ");
         EmitDeclarator(context, declarator);
         return;
     }
@@ -1234,28 +1272,24 @@ static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, EDeclara
             break;
         }
 
-        Emit(context, " ");
         EmitDeclarator(context, declarator);
         return;
     }
     else if (auto texType = type->As<TextureType>())
     {
         emitTextureType(context, texType);
-        Emit(context, " ");
         EmitDeclarator(context, declarator);
         return;
     }
     else if (auto textureSamplerType = type->As<TextureSamplerType>())
     {
         emitTextureSamplerType(context, textureSamplerType);
-        Emit(context, " ");
         EmitDeclarator(context, declarator);
         return;
     }
     else if (auto imageType = type->As<GLSLImageType>())
     {
         emitImageType(context, imageType);
-        Emit(context, " ");
         EmitDeclarator(context, declarator);
         return;
     }
@@ -1314,19 +1348,56 @@ static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, EDeclara
     throw "unimplemented";
 }
 
-static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, String const& name)
+static void EmitType(
+    EmitContext*            context,
+    RefPtr<ExpressionType>  type,
+    CodePosition const&     typeLoc,
+    String const&           name,
+    CodePosition const&     nameLoc)
 {
+    advanceToSourceLocation(context, typeLoc);
+
     EDeclarator nameDeclarator;
     nameDeclarator.flavor = EDeclarator::Flavor::Name;
     nameDeclarator.name = name;
+    nameDeclarator.loc = nameLoc;
     EmitType(context, type, &nameDeclarator);
 }
 
+
+static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, Token const& nameToken)
+{
+    EmitType(context, type, CodePosition(), nameToken.Content, nameToken.Position);
+}
+
+
+static void EmitType(EmitContext* context, RefPtr<ExpressionType> type, String const& name)
+{
+    EmitType(context, type, CodePosition(), name, CodePosition());
+}
+
 static void EmitType(EmitContext* context, RefPtr<ExpressionType> type)
 {
     EmitType(context, type, nullptr);
 }
 
+static void EmitType(EmitContext* context, TypeExp const& typeExp, Token const& nameToken)
+{
+    EmitType(context, typeExp.type, typeExp.exp->Position, nameToken.Content, nameToken.Position);
+}
+
+static void EmitType(EmitContext* context, TypeExp const& typeExp, String const& name)
+{
+    EmitType(context, typeExp.type, typeExp.exp->Position, name, CodePosition());
+}
+
+static void EmitType(EmitContext* context, TypeExp const& typeExp)
+{
+    advanceToSourceLocation(context, typeExp.exp->Position);
+    EmitType(context, typeExp.type, nullptr);
+}
+
+
 // Statements
 
 // Emit a statement as a `{}`-enclosed block statement, but avoid adding redundant
@@ -1454,10 +1525,14 @@ static void emitLineDirectiveAndUpdateSourceLocation(
     context->loc.Col = 1;
 }
 
-static void advanceToSourceLocation(
+static void emitLineDirectiveIfNeeded(
     EmitContext*        context,
     CodePosition const& sourceLocation)
 {
+    // Ignore invalid source locations
+    if(sourceLocation.Line <= 0)
+        return;
+
     // If we are currently emitting code at a source location with
     // a differnet file or line, *or* if the source location is
     // somehow later on the line than what we want to emit,
@@ -1506,6 +1581,32 @@ static void advanceToSourceLocation(
     }
 }
 
+static void advanceToSourceLocation(
+    EmitContext*        context,
+    CodePosition const& sourceLocation)
+{
+    // Skip invalid locations
+    if(sourceLocation.Line <= 0)
+        return;
+
+    context->needToUpdateSourceLocation = true;
+    context->nextSourceLocation = sourceLocation;
+}
+
+static void flushSourceLocationChange(
+    EmitContext*        context)
+{
+    if(!context->needToUpdateSourceLocation)
+        return;
+
+    // Note: the order matters here, because trying to update
+    // the source location may involve outputting text that
+    // advances the location, and outputting text is what
+    // triggers this flush operation.
+    context->needToUpdateSourceLocation = false;
+    emitLineDirectiveIfNeeded(context, context->nextSourceLocation);
+}
+
 static void emitTokenWithLocation(EmitContext* context, Token const& token)
 {
     if( token.Position.FileName.Length() != 0 )
@@ -1542,6 +1643,9 @@ static void EmitUnparsedStmt(EmitContext* context, RefPtr<UnparsedStmt> stmt)
 
 static void EmitStmt(EmitContext* context, RefPtr<StatementSyntaxNode> stmt)
 {
+    // Try to ensure that debugging can find the right location
+    advanceToSourceLocation(context, stmt->Position);
+
     if (auto blockStmt = stmt.As<BlockStatementSyntaxNode>())
     {
         EmitBlockStmt(context, blockStmt);
@@ -1703,7 +1807,7 @@ static void EmitDeclRef(EmitContext* context, DeclRef<Decl> declRef)
             if (aa != 0) Emit(context, ",");
             EmitVal(context, subst->args[aa]);
         }
-        Emit(context, ">");
+        Emit(context, " >");
     }
 
 }
@@ -1741,6 +1845,8 @@ static void EmitModifiers(EmitContext* context, RefPtr<Decl> decl)
 
     for (auto mod = decl->modifiers.first; mod; mod = mod->next)
     {
+        advanceToSourceLocation(context, mod->Position);
+
         if (0) {}
 
         #define CASE(TYPE, KEYWORD) \
@@ -1931,7 +2037,7 @@ static void EmitStructDecl(EmitContext* context, RefPtr<StructSyntaxNode> decl)
         return;
 
     Emit(context, "struct ");
-    emitName(context, decl->Name.Content);
+    emitName(context, decl->Name);
     Emit(context, "\n{\n");
 
     // TODO(tfoley): Need to hoist members functions, etc. out to global scope
@@ -1945,7 +2051,7 @@ static void EmitVarDeclCommon(EmitContext* context, DeclRef<VarDeclBase> declRef
 {
     EmitModifiers(context, declRef.getDecl());
 
-    EmitType(context, GetType(declRef), declRef.GetName());
+    EmitType(context, GetType(declRef), declRef.getDecl()->getNameToken());
 
     EmitSemantics(context, declRef.getDecl());
 
@@ -2095,7 +2201,7 @@ static void emitHLSLParameterBlockDecl(
     if( auto reflectionNameModifier = varDecl->FindModifier<ParameterBlockReflectionName>() )
     {
         Emit(context, " ");
-        emitName(context, reflectionNameModifier->nameToken.Content);
+        emitName(context, reflectionNameModifier->nameToken);
     }
 
     EmitSemantics(context, varDecl, kESemanticMask_None);
@@ -2265,7 +2371,7 @@ static void emitGLSLParameterBlockDecl(
     if( auto reflectionNameModifier = varDecl->FindModifier<ParameterBlockReflectionName>() )
     {
         Emit(context, " ");
-        emitName(context, reflectionNameModifier->nameToken.Content);
+        emitName(context, reflectionNameModifier->nameToken);
     }
 
     Emit(context, "\n{\n");
@@ -2291,7 +2397,7 @@ static void emitGLSLParameterBlockDecl(
     if( varDecl->Name.Type != TokenType::Unknown )
     {
         Emit(context, " ");
-        emitName(context, varDecl->Name.Content);
+        emitName(context, varDecl->Name);
     }
 
     Emit(context, ";\n");
@@ -2360,7 +2466,7 @@ static void EmitFuncDecl(EmitContext* context, RefPtr<FunctionSyntaxNode> decl)
     // isn't allowed by declarator syntax and/or language rules, we could
     // hypothetically wrap things in a `typedef` and work around it.
 
-    EmitType(context, decl->ReturnType, decl->Name.Content);
+    EmitType(context, decl->ReturnType, decl->Name);
 
     Emit(context, "(");
     bool first = true;
@@ -2517,6 +2623,10 @@ static void EmitDeclImpl(EmitContext* context, RefPtr<Decl> decl, RefPtr<VarLayo
     if (decl->HasModifier<FromStdLibModifier>())
         return;
 
+
+    // Try to ensure that debugging can find the right location
+    advanceToSourceLocation(context, decl->Position);
+
     if (auto typeDefDecl = decl.As<TypeDefDecl>())
     {
         EmitTypeDefDecl(context, typeDefDecl);