Skip to content

Commit

Permalink
Move Wparentheses implementation to AST level so that in the future i…
Browse files Browse the repository at this point in the history
…t can use type information to weed out false positives
  • Loading branch information
MikePopoloski committed Nov 6, 2024
1 parent dd9df36 commit 6d01072
Show file tree
Hide file tree
Showing 9 changed files with 365 additions and 244 deletions.
12 changes: 12 additions & 0 deletions include/slang/ast/Expression.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ SLANG_ENUM(RangeSelectionKind, RANGE)
#undef RANGE
// clang-format on

bool isBitwiseOperator(BinaryOperator op);
bool isComparisonOperator(BinaryOperator op);
bool isShiftOperator(BinaryOperator op);
bool isArithmeticOperator(BinaryOperator op);
bool isRelationalOperator(BinaryOperator op);
std::string_view getOperatorText(BinaryOperator op);
int getOperatorPrecedence(BinaryOperator op);

/// The base class for all expressions in SystemVerilog.
class SLANG_EXPORT Expression {
public:
Expand Down Expand Up @@ -354,6 +362,10 @@ class SLANG_EXPORT Expression {
/// Returns true if any subexpression of this expression is a hierarchical reference.
bool hasHierarchicalReference() const;

/// Returns true if this expression is known to be within a pair of parentheses,
/// and otherwise false.
bool isParenthesized() const;

/// If this expression is an implicit conversion, recursively unwraps to the
/// target operand. Otherwise returns `*this`.
const Expression& unwrapImplicitConversions() const;
Expand Down
8 changes: 6 additions & 2 deletions include/slang/ast/expressions/OperatorExpressions.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,13 @@ class SLANG_EXPORT UnaryExpression : public Expression {
/// The operator.
UnaryOperator op;

/// The source range of the operator token.
SourceRange opRange;

UnaryExpression(UnaryOperator op, const Type& type, Expression& operand,
SourceRange sourceRange) :
Expression(ExpressionKind::UnaryOp, type, sourceRange), op(op), operand_(&operand) {}
SourceRange sourceRange, SourceRange opRange) :
Expression(ExpressionKind::UnaryOp, type, sourceRange), op(op), opRange(opRange),
operand_(&operand) {}

/// @returns the operand
const Expression& operand() const { return *operand_; }
Expand Down
4 changes: 0 additions & 4 deletions include/slang/parsing/Parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,6 @@ class SLANG_EXPORT Parser : ParserBase, syntax::SyntaxFacts {
void checkEmptyBody(const syntax::SyntaxNode& syntax, Token prevToken,
std::string_view syntaxName);

void analyzePrecedence(const syntax::ExpressionSyntax& left,
const syntax::ExpressionSyntax& right, syntax::SyntaxKind opKind,
Token opToken);

// ---- Member variables ----

// The factory used to create new syntax nodes.
Expand Down
15 changes: 0 additions & 15 deletions include/slang/syntax/SyntaxFacts.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,21 +326,6 @@ class SLANG_EXPORT SyntaxFacts {
/// @return true if the given syntax kind is an assignment operator.
static bool isAssignmentOperator(SyntaxKind kind);

/// @return true if the given syntax kind is a bitwise operator.
static bool isBitwiseOperator(SyntaxKind kind);

/// @return true if the given syntax kind is a comparison operator.
static bool isComparisonOperator(SyntaxKind kind);

/// @return true if the given syntax kind is a shift operator.
static bool isShiftOperator(SyntaxKind kind);

/// @return true if the given syntax kind is an arithmetic operator.
static bool isArithmeticOperator(SyntaxKind kind);

/// @return true if the given syntax kind is a relational operator.
static bool isRelationalOperator(SyntaxKind kind);

/// @return a string representing the name of the given data type, if it has a simple name.
static std::string_view getSimpleTypeName(const DataTypeSyntax& syntax);

Expand Down
24 changes: 12 additions & 12 deletions scripts/diagnostics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -311,18 +311,6 @@ warning split-distweight-op SplitDistWeightOp "split dist weight operator is not
note NoteToMatchThis "to match this '{}'"
note NoteLastBlockStarted "last complete block started here"
note NoteLastBlockEnded "and ended here"
warning bitwise-rel-precedence BitwiseRelPrecedence "'{}' has lower precedence than '{}'; '{}' will be evaluated first"
note NotePrecedenceBitwiseFirst "place parentheses around the '{}' expression to evaluate it first"
note NotePrecedenceSilence "place parentheses around the '{}' expression to silence this warning"
warning bitwise-op-parentheses BitwiseOpParentheses "'{}' within '{}'"
warning logical-op-parentheses LogicalOpParentheses "'{}' within '{}'"
warning arith-in-shift ArithInShift "'{}' has lower precedence than '{}'; '{}' will be evaluated first"
warning logical-not-parentheses LogicalNotParentheses "logical not is only applied to the left hand side of this {}"
note NoteLogicalNotFix "add parentheses after the '!' to evaluate the {} first"
note NoteLogicalNotSilence "add parentheses around left hand side expression to silence this warning"
warning conditional-precedence ConditionalPrecedence "operator '?:' has lower precedence than '{}'; '{}' will be evaluated first"
note NoteConditionalPrecedenceFix "place parentheses around the '?:' expression to evaluate it first"
warning consecutive-comparison ConsecutiveComparison "comparisons like 'x < y < z' don't have their mathematical meaning"

subsystem Declarations
error LocalParamNoInitializer "local parameter is missing an initializer"
Expand Down Expand Up @@ -788,6 +776,18 @@ warning arith-op-mismatch ArithOpMismatch "arithmetic between operands of differ
warning bitwise-op-mismatch BitwiseOpMismatch "bitwise operation between operands of different types ({} and {})"
warning comparison-mismatch ComparisonMismatch "comparison between operands of different types ({} and {})"
warning sign-compare SignCompare "comparison of differently signed types ({} and {})"
warning bitwise-rel-precedence BitwiseRelPrecedence "'{}' has lower precedence than '{}'; '{}' will be evaluated first"
note NotePrecedenceBitwiseFirst "place parentheses around the '{}' expression to evaluate it first"
note NotePrecedenceSilence "place parentheses around the '{}' expression to silence this warning"
warning bitwise-op-parentheses BitwiseOpParentheses "'{}' within '{}'"
warning logical-op-parentheses LogicalOpParentheses "'{}' within '{}'"
warning arith-in-shift ArithInShift "'{}' has lower precedence than '{}'; '{}' will be evaluated first"
warning logical-not-parentheses LogicalNotParentheses "logical not is only applied to the left hand side of this {}"
note NoteLogicalNotFix "add parentheses after the '!' to evaluate the {} first"
note NoteLogicalNotSilence "add parentheses around left hand side expression to silence this warning"
warning conditional-precedence ConditionalPrecedence "operator '?:' has lower precedence than '{}'; '{}' will be evaluated first"
note NoteConditionalPrecedenceFix "place parentheses around the '?:' expression to evaluate it first"
warning consecutive-comparison ConsecutiveComparison "comparisons like 'x < y < z' don't have their mathematical meaning"

subsystem Statements
error ReturnNotInSubroutine "return statement is only valid inside task and function blocks"
Expand Down
4 changes: 4 additions & 0 deletions source/ast/Expression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,10 @@ bool Expression::hasHierarchicalReference() const {
return visitor.any;
}

bool Expression::isParenthesized() const {
return syntax && syntax->kind == SyntaxKind::ParenthesizedExpression;
}

const Expression& Expression::unwrapImplicitConversions() const {
auto expr = this;
while (expr->kind == ExpressionKind::Conversion) {
Expand Down
Loading

0 comments on commit 6d01072

Please sign in to comment.