Improvements to Casting (#1483)

* Improve handling of cast detection when have a more complex cast than just a single identifier.

* Improve comments around heuristic for casting

* Added nested enum test.

* Improve comments

* Define function like - output change.

* Use lookup for types in determining if cast or not.

* Add _isCast function

* Add heuristic test to nested-enum.slang that works if the type test fails.

* Change hueristic based on review.
Allow (..)( to always be an expression, because if it's a type it will be turned into a cast later.

* Fix output of define-function-like.slang - which changes again with improved casting support.

* Improve testing for type in cast - if we find a decl and it's not a type, then we know it's not a cast.

3 files changed, 306 insertions, 18 deletions

diff --git a/source/slang/slang-parser.cpp b/source/slang/slang-parser.cpp
index ed20e0d2e..460f781fe 100644
--- a/source/slang/slang-parser.cpp
+++ b/source/slang/slang-parser.cpp
@@ -3380,6 +3380,61 @@ namespace Slang
         return stmt;
     }
 
+    static bool _isType(Decl* decl)
+    {
+        return decl && (as<AggTypeDecl>(decl) || as<SimpleTypeDecl>(decl));
+    }
+
+    // TODO(JS):
+    // This only handles StaticMemberExpr, and VarExpr lookup scenarios!
+    static Decl* _tryResolveDecl(Parser* parser, Expr* expr)
+    {
+        if (auto staticMemberExpr = as<StaticMemberExpr>(expr))
+        {
+            Decl* baseTypeDecl = _tryResolveDecl(parser, staticMemberExpr->baseExpression);
+            if (!baseTypeDecl)
+            {
+                return nullptr;
+            }
+            if (AggTypeDecl* aggTypeDecl = as<AggTypeDecl>(baseTypeDecl))
+            {
+                // TODO(JS):
+                // Is it valid to always have empty substitution set here?
+                DeclRef<ContainerDecl> declRef(aggTypeDecl, SubstitutionSet());
+
+                auto lookupResult = lookUpDirectAndTransparentMembers(
+                    parser->astBuilder,
+                    nullptr, // no semantics visitor available yet
+                    staticMemberExpr->name,
+                    declRef);
+
+                if (!lookupResult.isValid() || lookupResult.isOverloaded())
+                    return nullptr;
+
+                return lookupResult.item.declRef.getDecl();
+            }
+
+            // Didn't find it
+            return nullptr;
+        }
+
+        if (auto varExpr = as<VarExpr>(expr))
+        {
+            // Do the lookup in the current scope
+            auto lookupResult = lookUp(
+                parser->astBuilder,
+                nullptr, // no semantics visitor available yet
+                varExpr->name,
+                parser->currentScope);
+            if (!lookupResult.isValid() || lookupResult.isOverloaded())
+                return nullptr;
+
+            return lookupResult.item.declRef.getDecl();
+        }
+
+        return nullptr;
+    }
+
     static bool isTypeName(Parser* parser, Name* name)
     {
         auto lookupResult = lookUp(
@@ -3390,19 +3445,7 @@ namespace Slang
         if(!lookupResult.isValid() || lookupResult.isOverloaded())
             return false;
 
-        auto decl = lookupResult.item.declRef.getDecl();
-        if( auto typeDecl = as<AggTypeDecl>(decl) )
-        {
-            return true;
-        }
-        else if( auto typeVarDecl = as<SimpleTypeDecl>(decl) )
-        {
-            return true;
-        }
-        else
-        {
-            return false;
-        }
+        return _isType(lookupResult.item.declRef.getDecl());
     }
 
     static bool peekTypeName(Parser* parser)
@@ -4310,6 +4353,137 @@ namespace Slang
         return value;
     }
 
+    static bool _isCast(Parser* parser, Expr* expr)
+    {
+
+        // We can't just look at expr and look up if it's a type, because we allow
+        // out-of-order declarations. So to a first approximation we'll try and
+        // determine if it is a cast via a heuristic based on what comes next
+
+        TokenType tokenType = peekTokenType(parser);
+
+        // Expression
+        // ==========
+        //
+        // Misc: ; ) [ ] , . = ? (ternary) { } ++ -- -> 
+        // Binary ops: * / | & ^ % << >> 
+        // Logical ops: || &&  
+        // Comparisons: != == < > <= =>
+        //
+        // Any assign op
+        // 
+        // If we don't have pointers then
+        // & : (Thing::Another) &v 
+        // * : (Thing::Another)*ptr is a cast.
+        //
+        // Cast
+        // ====
+        //
+        // Misc: (
+        // Identifier, Literal
+        // Unary ops: !, ~
+        // 
+        // Ambiguous
+        // =========
+        //
+        // - : Can be unary and therefore a cast or a binary subtract, and therefore an expression
+        // + : Can be unary and therefore could be a cast, or a binary add and therefore an expression
+        //
+        // Arbitrary
+        // =========
+        //
+        // End of file, End of directive, Invalid, :, ::
+
+        switch (tokenType)
+        {
+            case TokenType::FloatingPointLiteral:
+            case TokenType::CharLiteral:
+            case TokenType::IntegerLiteral:
+            case TokenType::Identifier:
+            case TokenType::OpNot:
+            case TokenType::OpBitNot:
+            {
+                // If followed by one of these, must be a cast
+                return true;
+            }
+            case TokenType::LParent:
+            {
+                // If we are followed by ( it might not be a cast - it could be a call invocation.
+                // BUT we can always *assume* it is a call, because such a 'call' will be correctly
+                // handled as a cast if necessary later.
+                return false;
+            }
+            case TokenType::OpAdd:
+            case TokenType::OpSub:
+            {
+                // + - are ambiguous, it could be a binary + or - so -> expression, or unary -> cast
+                //
+                // (Some::Stuff) + 3
+                // (Some::Stuff) - 3
+                // Strictly I can only tell if this is an expression or a cast if I know Some::Stuff is a type or not
+                // but we can't know here in general because we allow out-of-order declarations.
+
+                // If we can determine it's a type, then it must be a cast, and we are done.
+                // 
+                // NOTE! This test can only determine if it's a type *iff* it has already been defined. A future out
+                // of order declaration, will not be correctly found here.
+                //
+                // This means the semantics change depending on the order of definition (!)
+                Decl* decl = _tryResolveDecl(parser, expr);
+                // If we can find the decl-> we can resolve unambiguously
+                if (decl)
+                {
+                    return _isType(decl);
+                }
+
+                // Now we use a heuristic.
+                //
+                // Whitespace before, whitespace after->binary
+                // No whitespace before, no whitespace after->binary
+                // Otherwise->unary
+                //
+                // Unary -> cast, binary -> expression.
+                // 
+                // Ie:
+                // (Some::Stuff) +3  - must be a cast
+                // (Some::Stuff)+ 3  - must be a cast (?) This is a bit odd.
+                // (Some::Stuff) + 3 - must be an expression.
+                // (Some::Stuff)+3 - must be an expression.
+
+                // TODO(JS): This covers the (SomeScope::Identifier) case
+                //
+                // But perhaps there other ways of referring to types, that this now misses? With associated types/generics perhaps.
+                // 
+                // For now we'll assume it's not a cast if it's not a StaticMemberExpr
+                // The reason for the restriction (which perhaps can be broadened), is we don't
+                // want the interpretation of something in parentheses to be determined by something as common as + or - whitespace.
+
+                if (auto staticMemberExpr = dynamicCast<StaticMemberExpr>(expr))
+                {
+                    // Apply the heuristic:
+                    TokenReader::ParsingCursor cursor = parser->tokenReader.getCursor();
+                    // Skip the + or -
+                    const Token opToken = advanceToken(parser);
+                    // Peek the next token to see if it was preceded by white space
+                    const Token nextToken = peekToken(parser);
+
+                    // Rewind
+                    parser->tokenReader.setCursor(cursor);
+
+                    const bool isBinary = (nextToken.flags & TokenFlag::AfterWhitespace) == (opToken.flags & TokenFlag::AfterWhitespace);
+
+                    // If it's binary it's not a cast
+                    return !isBinary;
+                }
+                break;
+            }
+            default: break;
+        }
+
+        // We'll assume it's not a cast
+        return false;
+    }
+
     static Expr* parseAtomicExpr(Parser* parser)
     {
         switch( peekTokenType(parser) )
@@ -4320,7 +4494,7 @@ namespace Slang
             return nullptr;
 
         // Either:
-        // - parenthized expression `(exp)`
+        // - parenthesized expression `(exp)`
         // - cast `(type) exp`
         //
         // Proper disambiguation requires mixing up parsing
@@ -4344,13 +4518,39 @@ namespace Slang
                 }
                 else
                 {
+                    // The above branch catches the case where we have a cast like (Thing), but with
+                    // the scoping operator it will not handle (SomeScope::Thing). In that case this
+                    // branch will be taken. This is okay in so far as SomeScope::Thing will parse
+                    // as an expression.
+                    
                     Expr* base = parser->ParseExpression();
+
                     parser->ReadToken(TokenType::RParent);
 
-                    ParenExpr* parenExpr = parser->astBuilder->create<ParenExpr>();
-                    parenExpr->loc = openParen.loc;
-                    parenExpr->base = base;
-                    return parenExpr;
+                    // We now try and determine by what base is, if this is actually a cast or an expression in parentheses
+                    if (_isCast(parser, base))
+                    {
+                        // Parse as a cast
+
+                        TypeCastExpr* tcexpr = parser->astBuilder->create<ExplicitCastExpr>();
+                        tcexpr->loc = openParen.loc;
+
+                        tcexpr->functionExpr = base;
+
+                        auto arg = parsePrefixExpr(parser);
+                        tcexpr->arguments.add(arg);
+
+                        return tcexpr;
+                    }
+                    else
+                    {
+                        // Pass as an expression in parentheses
+
+                        ParenExpr* parenExpr = parser->astBuilder->create<ParenExpr>();
+                        parenExpr->loc = openParen.loc;
+                        parenExpr->base = base;
+                        return parenExpr;
+                    }
                 }
             }
 
diff --git a/tests/language-feature/enums/nested-enum.slang b/tests/language-feature/enums/nested-enum.slang
new file mode 100644
index 000000000..942a9e72d
--- /dev/null
+++ b/tests/language-feature/enums/nested-enum.slang
@@ -0,0 +1,84 @@
+// nested-num.slang
+
+// Test enums defined nested in a struct work as expected. 
+
+//TEST(compute):COMPARE_COMPUTE:
+
+struct Outer
+{
+    enum Channel
+    {
+        Red,
+        Green,
+        Blue,
+        Alpha,
+    }
+    
+    static int doSomething(int v) { return v + 1; }
+    typedef int SomeType;
+    
+    static int someValue = 10;
+    
+    static int getHeuristicResult()
+    {
+        // This tests that the cast heuristic works here. 
+        // when the compiler cannot determine if it's a type or not at this point, 
+        // because these declarations have not been seen.
+            
+        // Has whitespace (between + and the next thing) -> must be an expression
+        int value = (Inner::anotherValue) + 1;
+        // No whitespace, so assumed to be a cast
+        Inner::Enum anotherValue = (Inner::Enum) +1;
+        
+        return Inner::doSomethingElse(value + (int)anotherValue);
+    }
+    
+    struct Inner
+    {
+        enum Enum
+        {
+            A,
+            B,
+        };
+        
+        static int anotherValue = 10;
+        static int doSomethingElse(int a) { return a - 1; }
+    }
+}
+
+int test(int val)
+{
+    Outer::Channel channel = (Outer::Channel)val; // Outer::Channel(val);
+    Outer::Channel otherChannel = channel; // (Outer::Channel)val;
+
+    typedef Outer::Channel Channel;
+
+    int result = 0;
+    if(channel == Channel.Red)          result += 1;
+    if(channel != Channel.Green)        result += 16;
+    if(otherChannel == Channel.Blue)    result += 16*16;
+    if(otherChannel != Channel.Alpha)   result += 16*16*16;
+
+    return result;
+}
+
+//TEST_INPUT:ubuffer(data=[0 0 0 0], stride=4):out,name=outputBuffer
+RWStructuredBuffer<int> outputBuffer;
+
+[numthreads(4, 1, 1)]
+void computeMain(uint3 dispatchThreadID : SV_DispatchThreadID)
+{
+    int value = (Outer::someValue) + 1 + Outer::getHeuristicResult();
+    Outer::Channel anotherValue = (Outer::Channel) +1;
+    
+    // These work because the type can be determined because they are already declared at this point
+    // Check can see this is a function call
+    value = (Outer::doSomething)(value);
+    // Check can see this is a cast
+    value += (Outer::SomeType)(value);
+    
+    uint tid = dispatchThreadID.x;
+    int inVal = tid;
+    int outVal = test(inVal) + value * 2 + int(anotherValue) * 4;
+    outputBuffer[tid] = outVal;
+}
diff --git a/tests/language-feature/enums/nested-enum.slang.expected.txt b/tests/language-feature/enums/nested-enum.slang.expected.txt
new file mode 100644
index 000000000..9a26c2421
--- /dev/null
+++ b/tests/language-feature/enums/nested-enum.slang.expected.txt
@@ -0,0 +1,4 @@
+1071
+1060
+1170
+70