Skip to content

Commit

Permalink
v1800-2023: ref static type checking rules
Browse files Browse the repository at this point in the history
  • Loading branch information
MikePopoloski committed Mar 30, 2024
1 parent 564e180 commit 3c5e5e0
Show file tree
Hide file tree
Showing 15 changed files with 175 additions and 73 deletions.
8 changes: 5 additions & 3 deletions include/slang/ast/Expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ class EvalContext;
class InstanceSymbolBase;
class Type;
class ValueSymbol;
enum class VariableFlags : uint16_t;

// clang-format off
#define EXPRESSION(x) \
Expand Down Expand Up @@ -175,14 +176,15 @@ class SLANG_EXPORT Expression {
bitmask<ASTFlags> extraFlags = ASTFlags::None);

/// Binds a connection to a ref argument from the given syntax nodes.
static const Expression& bindRefArg(const Type& lhs, bool isConstRef,
static const Expression& bindRefArg(const Type& lhs, bitmask<VariableFlags> argFlags,
const ExpressionSyntax& rhs, SourceLocation location,
const ASTContext& context);

/// Binds an argument or port connection with the given direction.
static const Expression& bindArgument(const Type& argType, ArgumentDirection direction,
const ExpressionSyntax& syntax, const ASTContext& context,
bool isConstRef = false);
bitmask<VariableFlags> argFlags,
const ExpressionSyntax& syntax,
const ASTContext& context);

/// Checks that the given expression is valid for the specified connection direction.
/// @returns true if the connection is valid and false otherwise.
Expand Down
1 change: 1 addition & 0 deletions scripts/diagnostics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ error ExpectedNetDelay "invalid net delay; expected min:typ:max or single expres
error InvalidRefArg "invalid expression for pass by reference; only variables, class properties, and members of unpacked structs and arrays are allowed"
error RefTypeMismatch "argument of type {} connects to 'ref' port of inequivalent type {}"
error ConstVarToRef "cannot bind const variable to 'ref' argument"
error AutoVarToRefStatic "cannot pass automatic variables or elements of dynamically sized arrays to 'ref static' argument"
error MissingInvocationParens "parentheses are required when invoking function '{}'"
error ChainedMethodParens "parentheses are required when chaining method calls"
error NameListWithScopeRandomize "lookup restriction list not allowed for std::randomize call"
Expand Down
52 changes: 37 additions & 15 deletions source/ast/Expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,35 +262,49 @@ const Expression& Expression::bindRValue(const Type& lhs, const ExpressionSyntax
return convertAssignment(ctx, lhs, expr, location);
}

static bool canConnectToRefArg(const Expression& expr, bool isConstRef,
bool allowConstClassHandle = false) {
static bool canConnectToRefArg(const ASTContext& context, const Expression& expr,
bitmask<VariableFlags> argFlags, bool allowConstClassHandle = false,
bool disallowDynamicArrays = false) {
auto sym = expr.getSymbolReference(/* allowPacked */ false);
if (!sym || !VariableSymbol::isKind(sym->kind))
return false;

auto& var = sym->as<VariableSymbol>();
if (!isConstRef && var.flags.has(VariableFlags::Const) &&
if (!argFlags.has(VariableFlags::Const) && var.flags.has(VariableFlags::Const) &&
(!allowConstClassHandle || !var.getType().isClass())) {
return false;
}

const bool isRefStatic = argFlags.has(VariableFlags::RefStatic);
if (isRefStatic) {
if (var.lifetime == VariableLifetime::Automatic &&
!var.flags.has(VariableFlags::RefStatic)) {
return false;
}

if (disallowDynamicArrays && var.getType().isDynamicallySizedArray())
return false;
}

// Need to recursively check the left hand side of element selects and member accesses
// to be sure this is actually an lvalue and not, for example, the result of a
// function call or something.
switch (expr.kind) {
case ExpressionKind::ElementSelect:
return canConnectToRefArg(expr.as<ElementSelectExpression>().value(), isConstRef,
false);
return canConnectToRefArg(context, expr.as<ElementSelectExpression>().value(), argFlags,
false, isRefStatic);
case ExpressionKind::RangeSelect:
return canConnectToRefArg(expr.as<RangeSelectExpression>().value(), isConstRef, false);
return canConnectToRefArg(context, expr.as<RangeSelectExpression>().value(), argFlags,
false, isRefStatic);
case ExpressionKind::MemberAccess:
return canConnectToRefArg(expr.as<MemberAccessExpression>().value(), isConstRef, true);
return canConnectToRefArg(context, expr.as<MemberAccessExpression>().value(), argFlags,
true);
default:
return true;
}
}

const Expression& Expression::bindRefArg(const Type& lhs, bool isConstRef,
const Expression& Expression::bindRefArg(const Type& lhs, bitmask<VariableFlags> argFlags,
const ExpressionSyntax& rhs, SourceLocation location,
const ASTContext& context) {
Compilation& comp = context.getCompilation();
Expand All @@ -301,12 +315,19 @@ const Expression& Expression::bindRefArg(const Type& lhs, bool isConstRef,
if (lhs.isError())
return badExpr(comp, &expr);

if (!canConnectToRefArg(expr, isConstRef)) {
// If we can't bind to ref but we can bind to 'const ref', issue a more
// specific error about constness.
const bool isConstRef = argFlags.has(VariableFlags::Const);
if (!canConnectToRefArg(context, expr, argFlags)) {
DiagCode code = diag::InvalidRefArg;
if (!isConstRef && canConnectToRefArg(expr, true))
if (!isConstRef && canConnectToRefArg(context, expr, argFlags | VariableFlags::Const)) {
// If we can't bind to ref but we can bind to 'const ref', issue a more
// specific error about constness.
code = diag::ConstVarToRef;
}
else if (argFlags.has(VariableFlags::RefStatic) &&
canConnectToRefArg(context, expr, argFlags & ~VariableFlags::RefStatic)) {
// Same idea, but for ref static restrictions.
code = diag::AutoVarToRefStatic;
}

context.addDiag(code, location) << expr.sourceRange;
return badExpr(comp, &expr);
Expand All @@ -332,8 +353,9 @@ const Expression& Expression::bindRefArg(const Type& lhs, bool isConstRef,
}

const Expression& Expression::bindArgument(const Type& argType, ArgumentDirection direction,
bitmask<VariableFlags> argFlags,
const ExpressionSyntax& syntax,
const ASTContext& context, bool isConstRef) {
const ASTContext& context) {
auto loc = syntax.getFirstToken().location();
switch (direction) {
case ArgumentDirection::In:
Expand All @@ -342,7 +364,7 @@ const Expression& Expression::bindArgument(const Type& argType, ArgumentDirectio
case ArgumentDirection::InOut:
return bindLValue(syntax, argType, loc, context, direction == ArgumentDirection::InOut);
case ArgumentDirection::Ref:
return bindRefArg(argType, isConstRef, syntax, loc, context);
return bindRefArg(argType, argFlags, syntax, loc, context);
}
SLANG_UNREACHABLE;
}
Expand All @@ -359,7 +381,7 @@ bool Expression::checkConnectionDirection(const Expression& expr, ArgumentDirect
case ArgumentDirection::InOut:
return expr.requireLValue(context, loc, flags | AssignFlags::InOutPort);
case ArgumentDirection::Ref:
if (!canConnectToRefArg(expr, /* isConstRef */ false)) {
if (!canConnectToRefArg(context, expr, VariableFlags::None)) {
context.addDiag(diag::InvalidRefArg, loc) << expr.sourceRange;
return false;
}
Expand Down
4 changes: 3 additions & 1 deletion source/ast/SystemSubroutine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "slang/ast/Compilation.h"
#include "slang/ast/EvalContext.h"
#include "slang/ast/Expression.h"
#include "slang/ast/symbols/VariableSymbols.h"
#include "slang/ast/types/Type.h"
#include "slang/diagnostics/ExpressionsDiags.h"
#include "slang/diagnostics/SysFuncsDiags.h"
Expand Down Expand Up @@ -110,7 +111,8 @@ const Expression& SimpleSystemSubroutine::bindArgument(size_t argIndex, const AS
if (argIndex >= argTypes.size())
return SystemSubroutine::bindArgument(argIndex, context, syntax, args);

return Expression::bindArgument(*argTypes[argIndex], ArgumentDirection::In, syntax, context);
return Expression::bindArgument(*argTypes[argIndex], ArgumentDirection::In, {}, syntax,
context);
}

const Type& SimpleSystemSubroutine::checkArguments(const ASTContext& context, const Args& args,
Expand Down
29 changes: 19 additions & 10 deletions source/ast/builtins/ArrayMethods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,10 @@ class AssocArrayDeleteMethod : public SystemSubroutine {
// Argument type comes from the index type of the previous argument.
if (argIndex == 1) {
auto indexType = args[0]->type->getAssociativeIndexType();
if (indexType)
return Expression::bindArgument(*indexType, ArgumentDirection::In, syntax, context);
if (indexType) {
return Expression::bindArgument(*indexType, ArgumentDirection::In, {}, syntax,
context);
}
}

return SystemSubroutine::bindArgument(argIndex, context, syntax, args);
Expand Down Expand Up @@ -633,8 +635,10 @@ class AssocArrayExistsMethod : public SystemSubroutine {
// Argument type comes from the index type of the previous argument.
if (argIndex == 1) {
auto indexType = args[0]->type->getAssociativeIndexType();
if (indexType)
return Expression::bindArgument(*indexType, ArgumentDirection::In, syntax, context);
if (indexType) {
return Expression::bindArgument(*indexType, ArgumentDirection::In, {}, syntax,
context);
}
}

return SystemSubroutine::bindArgument(argIndex, context, syntax, args);
Expand Down Expand Up @@ -681,9 +685,10 @@ class AssocArrayTraversalMethod : public SystemSubroutine {
// Argument type comes from the index type of the previous argument.
if (argIndex == 1) {
auto indexType = args[0]->type->getAssociativeIndexType();
if (indexType)
return Expression::bindArgument(*indexType, ArgumentDirection::Ref, syntax,
if (indexType) {
return Expression::bindArgument(*indexType, ArgumentDirection::Ref, {}, syntax,
context);
}
}

return SystemSubroutine::bindArgument(argIndex, context, syntax, args);
Expand Down Expand Up @@ -770,8 +775,10 @@ class QueuePushMethod : public SystemSubroutine {
// Argument type comes from the element type of the queue.
if (argIndex == 1) {
auto elemType = args[0]->type->getArrayElementType();
if (elemType)
return Expression::bindArgument(*elemType, ArgumentDirection::In, syntax, context);
if (elemType) {
return Expression::bindArgument(*elemType, ArgumentDirection::In, {}, syntax,
context);
}
}

return SystemSubroutine::bindArgument(argIndex, context, syntax, args);
Expand Down Expand Up @@ -822,8 +829,10 @@ class QueueInsertMethod : public SystemSubroutine {
// Argument type comes from the element type of the queue.
if (argIndex == 2) {
auto elemType = args[0]->type->getArrayElementType();
if (elemType)
return Expression::bindArgument(*elemType, ArgumentDirection::In, syntax, context);
if (elemType) {
return Expression::bindArgument(*elemType, ArgumentDirection::In, {}, syntax,
context);
}
}

return SystemSubroutine::bindArgument(argIndex, context, syntax, args);
Expand Down
3 changes: 2 additions & 1 deletion source/ast/builtins/CoverageFuncs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "slang/ast/SystemSubroutine.h"
#include "slang/ast/expressions/MiscExpressions.h"
#include "slang/ast/symbols/InstanceSymbols.h"
#include "slang/ast/symbols/VariableSymbols.h"
#include "slang/ast/types/Type.h"
#include "slang/diagnostics/SysFuncsDiags.h"
#include "slang/syntax/AllSyntax.h"
Expand Down Expand Up @@ -40,7 +41,7 @@ class CoverageNameOrHierFunc : public SystemSubroutine {
LookupFlags::AllowRoot);
}

return Expression::bindArgument(*argTypes[argIndex], ArgumentDirection::In, syntax,
return Expression::bindArgument(*argTypes[argIndex], ArgumentDirection::In, {}, syntax,
context);
}

Expand Down
3 changes: 2 additions & 1 deletion source/ast/builtins/EnumMethods.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//------------------------------------------------------------------------------
#include "slang/ast/Compilation.h"
#include "slang/ast/SystemSubroutine.h"
#include "slang/ast/symbols/VariableSymbols.h"
#include "slang/ast/types/AllTypes.h"

namespace slang::ast::builtins {
Expand Down Expand Up @@ -71,7 +72,7 @@ class EnumNextPrevMethod : public SystemSubroutine {
return SystemSubroutine::bindArgument(argIndex, context, syntax, args);

return Expression::bindArgument(context.getCompilation().getUnsignedIntType(),
ArgumentDirection::In, syntax, context);
ArgumentDirection::In, {}, syntax, context);
}

const Type& checkArguments(const ASTContext& context, const Args& args, SourceRange range,
Expand Down
4 changes: 2 additions & 2 deletions source/ast/expressions/AssignmentExpressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1550,7 +1550,7 @@ Expression& SimpleAssignmentPatternExpression::forStruct(
uint32_t index = 0;
SmallVector<const Expression*> elems;
for (auto item : syntax.items) {
auto& expr = Expression::bindArgument(*types[index++], direction, *item, context);
auto& expr = Expression::bindArgument(*types[index++], direction, {}, *item, context);
elems.push_back(&expr);
bad |= expr.bad();
}
Expand All @@ -1573,7 +1573,7 @@ static std::span<const Expression* const> bindExpressionList(

SmallVector<const Expression*> elems;
for (auto item : items) {
auto& expr = Expression::bindArgument(elementType, direction, *item, context);
auto& expr = Expression::bindArgument(elementType, direction, {}, *item, context);
elems.push_back(&expr);
bad |= expr.bad();
}
Expand Down
8 changes: 4 additions & 4 deletions source/ast/expressions/CallExpression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ bool CallExpression::bindArgs(const ArgumentListSyntax* argSyntax,
addDefaultDriver(*expr, *formal);
}
else if (auto exSyn = context.requireSimpleExpr(arg->as<PropertyExprSyntax>())) {
expr = &Expression::bindArgument(formal->getType(), formal->direction, *exSyn,
context, formal->flags.has(VariableFlags::Const));
expr = &Expression::bindArgument(formal->getType(), formal->direction,
formal->flags, *exSyn, context);
}

// Make sure there isn't also a named value for this argument.
Expand Down Expand Up @@ -280,8 +280,8 @@ bool CallExpression::bindArgs(const ArgumentListSyntax* argSyntax,
}
}
else if (auto exSyn = context.requireSimpleExpr(*arg)) {
expr = &Expression::bindArgument(formal->getType(), formal->direction, *exSyn,
context, formal->flags.has(VariableFlags::Const));
expr = &Expression::bindArgument(formal->getType(), formal->direction,
formal->flags, *exSyn, context);
}
}
else {
Expand Down
Loading

0 comments on commit 3c5e5e0

Please sign in to comment.