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.

1 files changed, 218 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;
+                    }
                 }
             }