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.

1 files changed, 217 insertions, 0 deletions

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<String, String> reservedWords;
+
+
     RefPtr<ProgramSyntaxNode>   loweredProgram;
 
     Dictionary<Decl*, RefPtr<Decl>> 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.