Fix use of 'this' handle in non-static class property initializers, a…

…lso fix lookup of class randomize vs std randomize methods

bindings/python/SymbolBindings.cpp

@@ -34,6 +34,7 @@ void registerSymbols(py::module_& m) {
         .value("NoSelectors", LookupFlags::NoSelectors)
         .value("AllowRoot", LookupFlags::AllowRoot)
         .value("IfacePortConn", LookupFlags::IfacePortConn)
+        .value("StaticInitializer", LookupFlags::StaticInitializer)
         .value("ForceHierarchical", LookupFlags::ForceHierarchical);
 
     py::class_<LookupLocation>(m, "LookupLocation")

include/slang/ast/Lookup.h

@@ -67,10 +67,13 @@ enum class SLANG_EXPORT LookupFlags {
     /// Lookup is resolving an interface port connection expression.
     IfacePortConn = 1 << 9,
 
+    /// Lookup is within a static initializer expression.
+    StaticInitializer = 1 << 10,
+
     /// Treat this lookup as hierarchical even if it's a simple name.
     ForceHierarchical = AllowDeclaredAfter | NoUndeclaredErrorIfUninstantiated
 };
-SLANG_BITMASK(LookupFlags, IfacePortConn)
+SLANG_BITMASK(LookupFlags, StaticInitializer)
 
 /// This type denotes the ordering of symbols within a particular scope, for the purposes of
 /// determining whether a found symbol is visible compared to the given location.

include/slang/ast/Symbol.h

@@ -249,19 +249,28 @@ class SLANG_EXPORT Symbol {
     /// Returns the next sibling symbol in the parent scope, if one exists.
     const Symbol* getNextSibling() const { return nextInScope; }
 
+    /// Sets the parent scope of this symbol.
+    ///
+    /// Typically this is not called directly; add the symbol to the scope
+    /// via the Scope::addMember method.
+    void setParent(const Scope& scope) { parentScope = &scope; }
+
+    /// Sets the parent scope of this symbol.
+    ///
+    /// Typically this is not called directly; add the symbol to the scope
+    /// via the Scope::addMember method.
+    void setParent(const Scope& scope, SymbolIndex index) {
+        setParent(scope);
+        indexInScope = index;
+    }
+
     template<typename TVisitor, typename... Args>
     decltype(auto) visit(TVisitor&& visitor, Args&&... args) const;
 
 protected:
     Symbol(SymbolKind kind, std::string_view name, SourceLocation location) :
         kind(kind), name(name), location(location) {}
 
-    void setParent(const Scope& scope) { parentScope = &scope; }
-    void setParent(const Scope& scope, SymbolIndex index) {
-        setParent(scope);
-        indexInScope = index;
-    }
-
 private:
     friend class Scope;
 

include/slang/ast/symbols/ClassSymbols.h

@@ -46,6 +46,12 @@ class SLANG_EXPORT ClassType : public Type, public Scope {
     /// a pointer to that generic class definition.
     const GenericClassDefSymbol* genericClass = nullptr;
 
+    /// A variable that points to the instance of this class itself, which is
+    /// used by non-static class property initializers that refers to the
+    /// special "this" handle. Subroutines and constraint blocks have their
+    /// own "thisVar" members.
+    const VariableSymbol* thisVar = nullptr;
+
     /// Set to true if the class is an abstract class (declared with the
     /// "virtual" keyword).
     bool isAbstract = false;

source/ast/Expression.cpp

@@ -947,6 +947,9 @@ Expression& Expression::bindName(Compilation& compilation, const NameSyntax& syn
         flags |= LookupFlags::AllowDeclaredAfter;
     }
 
+    if (context.flags.has(ASTFlags::StaticInitializer))
+        flags |= LookupFlags::StaticInitializer;
+
     // Special case scenarios: temporary variables, class-scoped randomize calls,
     // and expanding sequences and properties.
     if (context.firstTempVar || context.randomizeDetails || context.assertionInstance) {

source/ast/Lookup.cpp

@@ -914,7 +914,8 @@ void unwrapResult(const Scope& scope, std::optional<SourceRange> range, LookupRe
     }
 }
 
-const Symbol* findThisHandle(const Scope& scope, SourceRange range, LookupResult& result) {
+const Symbol* findThisHandle(const Scope& scope, bitmask<LookupFlags> flags, SourceRange range,
+                             LookupResult& result) {
     // Find the parent method, if we can.
     const Symbol* parent = &scope.asSymbol();
     while (parent->kind == SymbolKind::StatementBlock ||
@@ -929,24 +930,32 @@ const Symbol* findThisHandle(const Scope& scope, SourceRange range, LookupResult
         if (sub.thisVar)
             return sub.thisVar;
     }
-
-    if (parent->kind == SymbolKind::ConstraintBlock) {
+    else if (parent->kind == SymbolKind::ConstraintBlock) {
         auto thisVar = parent->as<ConstraintBlockSymbol>().thisVar;
         if (thisVar)
             return thisVar;
     }
+    else if (parent->kind == SymbolKind::ClassType && !flags.has(LookupFlags::StaticInitializer)) {
+        return parent->as<ClassType>().thisVar;
+    }
 
     result.addDiag(scope, diag::InvalidThisHandle, range);
     return nullptr;
 }
 
-const Symbol* findSuperHandle(const Scope& scope, SourceRange range, LookupResult& result) {
-    auto [parent, _] = Lookup::getContainingClass(scope);
+const Symbol* findSuperHandle(const Scope& scope, bitmask<LookupFlags> flags, SourceRange range,
+                              LookupResult& result) {
+    auto [parent, inStatic] = Lookup::getContainingClass(scope);
     if (!parent) {
         result.addDiag(scope, diag::SuperOutsideClass, range);
         return nullptr;
     }
 
+    if (inStatic || flags.has(LookupFlags::StaticInitializer)) {
+        result.addDiag(scope, diag::NonStaticClassProperty, range) << "super"sv;
+        return nullptr;
+    }
+
     auto base = parent->getBaseClass();
     if (!base && !parent->name.empty()) {
         result.addDiag(scope, diag::SuperNoBase, range) << parent->name;
@@ -998,7 +1007,7 @@ void Lookup::name(const NameSyntax& syntax, const ASTContext& context, bitmask<L
                 result.errorIfSelectors(context);
             return;
         case SyntaxKind::ThisHandle:
-            result.found = findThisHandle(scope, syntax.sourceRange(), result);
+            result.found = findThisHandle(scope, flags, syntax.sourceRange(), result);
             return;
         case SyntaxKind::SystemName: {
             // If this is a system name, look up directly in the compilation.
@@ -1540,19 +1549,19 @@ bool Lookup::withinClassRandomize(const ASTContext& context, const NameSyntax& s
         case SyntaxKind::ThisHandle:
             result.found = details.thisVar;
             if (!result.found)
-                result.found = findThisHandle(*context.scope, name.range, result);
+                result.found = findThisHandle(*context.scope, flags, name.range, result);
 
             if (result.found && nameParts.back().kind == SyntaxKind::SuperHandle) {
                 // Handle "this.super.whatever" the same as if the user had just
                 // written "super.whatever".
                 name = nameParts.back().name;
                 nameParts.pop_back();
-                result.found = findSuperHandle(findSuperScope(), name.range, result);
+                result.found = findSuperHandle(findSuperScope(), flags, name.range, result);
                 colonParts = 1;
             }
             break;
         case SyntaxKind::SuperHandle:
-            result.found = findSuperHandle(findSuperScope(), name.range, result);
+            result.found = findSuperHandle(findSuperScope(), flags, name.range, result);
             colonParts = 1; // pretend we used colon access to resolve class scoped name
             break;
         default:
@@ -1665,7 +1674,10 @@ void Lookup::unqualifiedImpl(const Scope& scope, std::string_view name, LookupLo
                         return;
                     }
 
-                    locationGood = sub.getReturnType().isVoid();
+                    // Also allow built-ins to always be found; they don't really
+                    // have a location since they're intrinsically defined.
+                    locationGood = sub.getReturnType().isVoid() ||
+                                   sub.flags.has(MethodFlags::BuiltIn);
                     break;
                 }
                 case SymbolKind::MethodPrototype: {
@@ -1676,7 +1688,8 @@ void Lookup::unqualifiedImpl(const Scope& scope, std::string_view name, LookupLo
                         return;
                     }
 
-                    locationGood = sub.getReturnType().isVoid();
+                    locationGood = sub.getReturnType().isVoid() ||
+                                   sub.flags.has(MethodFlags::BuiltIn);
                     break;
                 }
                 case SymbolKind::Sequence:
@@ -1959,18 +1972,18 @@ void Lookup::qualified(const ScopedNameSyntax& syntax, const ASTContext& context
             lookupDownward(nameParts, first, context, flags, result);
             return;
         case SyntaxKind::ThisHandle:
-            result.found = findThisHandle(scope, first.range, result);
+            result.found = findThisHandle(scope, flags, first.range, result);
             if (result.found && nameParts.back().kind == SyntaxKind::SuperHandle) {
                 // Handle "this.super.whatever" the same as if the user had just
                 // written "super.whatever".
                 first = nameParts.back().name;
                 nameParts.pop_back();
-                result.found = findSuperHandle(scope, first.range, result);
+                result.found = findSuperHandle(scope, flags, first.range, result);
                 colonParts = 1;
             }
             break;
         case SyntaxKind::SuperHandle:
-            result.found = findSuperHandle(scope, first.range, result);
+            result.found = findSuperHandle(scope, flags, first.range, result);
             colonParts = 1; // pretend we used colon access to resolve class scoped name
             break;
         case SyntaxKind::LocalScope:

source/ast/symbols/ClassSymbols.cpp

@@ -239,6 +239,14 @@ void ClassType::populate(const Scope& scope, const ClassDeclarationSyntax& synta
     if (constraint_mode)
         constraint_mode->addArg("on_ff", comp.getBitType());
 
+    // Give this class a "thisVar" that can be used by non-static class
+    // property initializers to refer to their own instance.
+    auto tv = comp.emplace<VariableSymbol>("this", location, VariableLifetime::Automatic);
+    tv->setType(*this);
+    tv->flags |= VariableFlags::Const | VariableFlags::CompilerGenerated;
+    tv->setParent(*this);
+    thisVar = tv;
+
     // This needs to happen last, otherwise setting "needs elaboration" before
     // trying to access the name map can cause infinite recursion.
     if (syntax.extendsClause || syntax.implementsClause)

tests/unittests/ast/ClassTests.cpp

@@ -2795,3 +2795,65 @@ endmodule
     CHECK(diags[5].code == diag::ClassPrivateMembersBitstream);
     CHECK(diags[6].code == diag::ClassPrivateMembersBitstream);
 }
+
+TEST_CASE("Using this keyword in class property intializer") {
+    auto tree = SyntaxTree::fromText(R"(
+typedef class B;
+class A;
+    B b;
+    function new(B b);
+        this.b = b;
+    endfunction
+endclass
+
+class B;
+    A a1 = new(this);
+    static A a2 = new(this);
+endclass
+
+class C extends B;
+    A a1 = super.a1;
+    static A a2 = super.a1;
+    static A a3 = super.a2;
+endclass
+
+module test;
+    B b = new();
+endmodule
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 3);
+    CHECK(diags[0].code == diag::InvalidThisHandle);
+    CHECK(diags[1].code == diag::NonStaticClassProperty);
+    CHECK(diags[2].code == diag::NonStaticClassProperty);
+}
+
+TEST_CASE("Super handle and class randomize used from static contexts") {
+    auto tree = SyntaxTree::fromText(R"(
+class A;
+    static int blah;
+endclass
+
+class B extends A;
+    static int i = randomize;
+
+    static function void foo;
+        int i = super.blah;
+        int j = randomize;
+    endfunction
+endclass
+)");
+
+    Compilation compilation;
+    compilation.addSyntaxTree(tree);
+
+    auto& diags = compilation.getAllDiagnostics();
+    REQUIRE(diags.size() == 3);
+    CHECK(diags[0].code == diag::NonStaticClassMethod);
+    CHECK(diags[1].code == diag::NonStaticClassProperty);
+    CHECK(diags[2].code == diag::NonStaticClassMethod);
+}