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

lib: Add P4::warningCount() and P4::infoCount() #5098

Merged
merged 7 commits into from
Jan 15, 2025

Conversation

vlstill
Copy link
Contributor

@vlstill vlstill commented Jan 14, 2025

There is errorCount and diagnosticCount but no warningCount and no infoCount. This PR adds them.

@vlstill vlstill added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jan 14, 2025
@vlstill vlstill self-assigned this Jan 14, 2025
@vlstill vlstill force-pushed the vstill/warn-info-count branch from 996d3cd to 9e8d633 Compare January 14, 2025 20:14
@vlstill vlstill requested a review from qobilidop January 14, 2025 20:15
@vlstill vlstill requested review from asl and fruffy January 14, 2025 20:15
lib/error.h Outdated Show resolved Hide resolved
@@ -69,51 +69,58 @@ TEST_F(Diagnostics, P4_16_Disable) {
@diagnostic("uninitialized_out_param", "disable")
)"));
EXPECT_TRUE(test);
EXPECT_EQ(0u, ::P4::diagnosticCount());
EXPECT_EQ(0u, P4::diagnosticCount());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably do not even need the P4:: qualifier here. Not sure what our general policy is but these functions are all available in the P4 namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. I guess I am used to working on a code where P4 is just one of the namespaces :-).

vlstill and others added 2 commits January 14, 2025 21:25
Co-authored-by: Fabian Ruffy <[email protected]>
Signed-off-by: Vladimír Štill <[email protected]>
Signed-off-by: Vladimír Štill <[email protected]>
Copy link
Contributor

@asl asl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly looks reasonable

Copy link
Member

@qobilidop qobilidop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall. Just have some nits.

lib/error.h Outdated Show resolved Hide resolved
test/gtest/diagnostics.cpp Outdated Show resolved Hide resolved
test/gtest/diagnostics.cpp Show resolved Hide resolved
Signed-off-by: Vladimír Štill <[email protected]>
Signed-off-by: Vladimír Štill <[email protected]>
Signed-off-by: Vladimír Štill <[email protected]>
@vlstill vlstill enabled auto-merge January 15, 2025 07:10
@vlstill vlstill added this pull request to the merge queue Jan 15, 2025
Merged via the queue into p4lang:main with commit 1bae920 Jan 15, 2025
19 checks passed
@vlstill vlstill deleted the vstill/warn-info-count branch January 15, 2025 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants