Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New tidy checks #857

Merged
merged 3 commits into from
Dec 10, 2023
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[slang-tidy] Add checks for port connections
This commit adds two new checks for port connections:

NoDotStarInPortConnection: Checks if the syntax .* has been used to
connect ports

NoImplicitPortNameInPortConnection: Checks if the syntax .port_name has
been used to connect ports
Sustrak committed Dec 10, 2023
commit e15e3d082146c80582a100f6a0ad473ce097c7c5
4 changes: 3 additions & 1 deletion tools/tidy/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -18,7 +18,9 @@ add_library(
src/style/EnforceModuleInstantiationPrefix.cpp
src/style/OnlyANSIPortDecl.cpp
src/synthesis/XilinxDoNotCareValues.cpp
src/synthesis/CastSignedIndex.cpp)
src/synthesis/CastSignedIndex.cpp
src/style/NoDotStarInPortConnection.cpp
src/style/NoImplicitPortNameInPortConnection.cpp)

target_include_directories(slang_tidy_obj_lib PUBLIC include ../../include)
target_link_libraries(slang_tidy_obj_lib PUBLIC slang::slang)
2 changes: 2 additions & 0 deletions tools/tidy/include/TidyDiags.h
Original file line number Diff line number Diff line change
@@ -22,5 +22,7 @@ inline constexpr DiagCode EnforceModuleInstantiationPrefix(DiagSubsystem::Tidy,
inline constexpr DiagCode OnlyANSIPortDecl(DiagSubsystem::Tidy, 8);
inline constexpr DiagCode XilinxDoNotCareValues(DiagSubsystem::Tidy, 9);
inline constexpr DiagCode CastSignedIndex(DiagSubsystem::Tidy, 10);
inline constexpr DiagCode NoDotStarInPortConnection(DiagSubsystem::Tidy, 11);
inline constexpr DiagCode NoImplicitPortNameInPortConnection(DiagSubsystem::Tidy, 12);

} // namespace slang::diag
2 changes: 2 additions & 0 deletions tools/tidy/src/TidyConfig.cpp
Original file line number Diff line number Diff line change
@@ -24,6 +24,8 @@ TidyConfig::TidyConfig() {
styleChecks.emplace("NoOldAlwaysSyntax", CheckStatus::ENABLED);
styleChecks.emplace("EnforceModuleInstantiationPrefix", CheckStatus::ENABLED);
styleChecks.emplace("OnlyANSIPortDecl", CheckStatus::ENABLED);
styleChecks.emplace("NoDotStarInPortConnection", CheckStatus::ENABLED);
styleChecks.emplace("NoImplicitPortNameInPortConnection", CheckStatus::ENABLED);
checkKinds.insert({slang::TidyKind::Style, styleChecks});

auto synthesisChecks = std::unordered_map<std::string, CheckStatus>();
65 changes: 65 additions & 0 deletions tools/tidy/src/style/NoDotStarInPortConnection.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// SPDX-FileCopyrightText: Michael Popoloski
// SPDX-License-Identifier: MIT

#include "ASTHelperVisitors.h"
#include "TidyDiags.h"

#include "slang/syntax/AllSyntax.h"
#include "slang/syntax/SyntaxVisitor.h"

using namespace slang;
using namespace slang::ast;
using namespace slang::syntax;

namespace no_dot_start_in_port_connection {

struct PortConnectionVisitor : public SyntaxVisitor<PortConnectionVisitor> {
void handle(const WildcardPortConnectionSyntax&) { found = true; }

public:
bool found{false};
};

struct MainVisitor : public TidyVisitor, ASTVisitor<MainVisitor, true, true> {
explicit MainVisitor(Diagnostics& diagnostics) : TidyVisitor(diagnostics) {}

void handle(const InstanceBodySymbol& symbol) {
NEEDS_SKIP_SYMBOL(symbol)
PortConnectionVisitor visitor;
symbol.getSyntax()->visit(visitor);
if (visitor.found)
diags.add(diag::NoDotStarInPortConnection, symbol.location);
}
};
} // namespace no_dot_start_in_port_connection

using namespace no_dot_start_in_port_connection;

class NoDotStarInPortConnection : public TidyCheck {
public:
[[maybe_unused]] explicit NoDotStarInPortConnection(TidyKind kind) : TidyCheck(kind) {}

bool check(const RootSymbol& root) override {
MainVisitor visitor(diagnostics);
root.visit(visitor);
if (!diagnostics.empty())
return false;
return true;
}

DiagCode diagCode() const override { return diag::NoDotStarInPortConnection; }

std::string diagString() const override { return "use of .* in port connection list"; }

DiagnosticSeverity diagSeverity() const override { return DiagnosticSeverity::Warning; }

std::string name() const override { return "NoDotStarInPortConnection"; }

std::string description() const override { return shortDescription(); }

std::string shortDescription() const override {
return "Checks if in a module instantiation any port is connected using .* syntax.";
}
};

REGISTER(NoDotStarInPortConnection, NoDotStarInPortConnection, TidyKind::Style)
71 changes: 71 additions & 0 deletions tools/tidy/src/style/NoImplicitPortNameInPortConnection.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
// SPDX-FileCopyrightText: Michael Popoloski
// SPDX-License-Identifier: MIT

#include "ASTHelperVisitors.h"
#include "TidyDiags.h"
#include <iostream>

#include "slang/syntax/AllSyntax.h"
#include "slang/syntax/SyntaxVisitor.h"

using namespace slang;
using namespace slang::ast;
using namespace slang::syntax;

namespace no_implicit_port_name_in_port_connection {

struct PortConnectionVisitor : public SyntaxVisitor<PortConnectionVisitor> {
void handle(const PortConnectionSyntax& syntax) {
if (syntax.toString().find('(') == std::string::npos)
found = true;
}

public:
bool found{false};
};

struct MainVisitor : public TidyVisitor, ASTVisitor<MainVisitor, true, true> {
explicit MainVisitor(Diagnostics& diagnostics) : TidyVisitor(diagnostics) {}

void handle(const InstanceBodySymbol& symbol) {
NEEDS_SKIP_SYMBOL(symbol)
PortConnectionVisitor visitor;
symbol.getSyntax()->visit(visitor);
if (visitor.found)
diags.add(diag::NoImplicitPortNameInPortConnection, symbol.location);
}
};
} // namespace no_implicit_port_name_in_port_connection

using namespace no_implicit_port_name_in_port_connection;

class NoImplicitPortNameInPortConnection : public TidyCheck {
public:
[[maybe_unused]] explicit NoImplicitPortNameInPortConnection(TidyKind kind) : TidyCheck(kind) {}

bool check(const RootSymbol& root) override {
MainVisitor visitor(diagnostics);
root.visit(visitor);
if (!diagnostics.empty())
return false;
return true;
}

DiagCode diagCode() const override { return diag::NoImplicitPortNameInPortConnection; }

std::string diagString() const override {
return "port name not specified. Please use .port_name(net) syntax.";
}

DiagnosticSeverity diagSeverity() const override { return DiagnosticSeverity::Warning; }

std::string name() const override { return "NoImplicitPortNameInPortConnection"; }

std::string description() const override { return shortDescription(); }

std::string shortDescription() const override {
return "Checks if in a module instantiation any port is connected using .port_name syntax.";
}
};

REGISTER(NoImplicitPortNameInPortConnection, NoImplicitPortNameInPortConnection, TidyKind::Style)
4 changes: 3 additions & 1 deletion tools/tidy/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -18,7 +18,9 @@ add_executable(
EnforceModuleInstantiationTest.cpp
OnlyANSIPortDecl.cpp
XilinxDoNotCareValuesTest.cpp
CastSignedIndexTest.cpp)
CastSignedIndexTest.cpp
NoDotStarInPortConnectionTest.cpp
NoImplicitPortNameInPortConnectionTest.cpp)

target_link_libraries(tidy_unittests PRIVATE Catch2::Catch2 slang_tidy_obj_lib)
target_compile_definitions(tidy_unittests PRIVATE UNITTESTS)
53 changes: 53 additions & 0 deletions tools/tidy/tests/NoDotStarInPortConnectionTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// SPDX-FileCopyrightText: Michael Popoloski
// SPDX-License-Identifier: MIT

#include "Test.h"
#include "TidyFactory.h"

TEST_CASE("NoDotStarInPortConnection: Use of dot star in module port connection") {
auto tree = SyntaxTree::fromText(R"(
module test (input clk, input rst);
endmodule

module top ();
logic clk, rst;
test t (.clk(clk), .*);
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);
compilation.getAllDiagnostics();
auto& root = compilation.getRoot();

TidyConfig config;
Registry::setConfig(config);
Registry::setSourceManager(compilation.getSourceManager());
auto visitor = Registry::create("NoDotStarInPortConnection");
bool result = visitor->check(root);
CHECK_FALSE(result);
}

TEST_CASE("NoDotStarInPortConnection: Module port connection port by port") {
auto tree = SyntaxTree::fromText(R"(
module test (input clk, input rst);
endmodule

module top ();
logic clk, rst;
test t (.clk, .rst(rst));
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);
compilation.getAllDiagnostics();
auto& root = compilation.getRoot();

TidyConfig config;
Registry::setConfig(config);
Registry::setSourceManager(compilation.getSourceManager());
auto visitor = Registry::create("NoDotStarInPortConnection");
bool result = visitor->check(root);
CHECK(result);
}
53 changes: 53 additions & 0 deletions tools/tidy/tests/NoImplicitPortNameInPortConnectionTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// SPDX-FileCopyrightText: Michael Popoloski
// SPDX-License-Identifier: MIT

#include "Test.h"
#include "TidyFactory.h"

TEST_CASE("NoImplicitPortNameInPortConnection: Only port name specified in module port connection") {
auto tree = SyntaxTree::fromText(R"(
module test (input clk, input rst);
endmodule

module top ();
logic clk, rst;
test t (.clk, .rst(rst));
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);
compilation.getAllDiagnostics();
auto& root = compilation.getRoot();

TidyConfig config;
Registry::setConfig(config);
Registry::setSourceManager(compilation.getSourceManager());
auto visitor = Registry::create("NoImplicitPortNameInPortConnection");
bool result = visitor->check(root);
CHECK_FALSE(result);
}

TEST_CASE("NoImplicitPortNameInPortConnection: Module port connection port by port") {
auto tree = SyntaxTree::fromText(R"(
module test (input cl()k, input rst);
endmodule

module top ();
logic clk, rst;
test t (.clk(clk), .rst(rst));
endmodule
)");

Compilation compilation;
compilation.addSyntaxTree(tree);
compilation.getAllDiagnostics();
auto& root = compilation.getRoot();

TidyConfig config;
Registry::setConfig(config);
Registry::setSourceManager(compilation.getSourceManager());
auto visitor = Registry::create("NoImplicitPortNameInPortConnection");
bool result = visitor->check(root);
CHECK(result);
}