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

Ensure correct overload for diagnose() is called even in Transform context #4830

Merged
merged 1 commit into from
Aug 6, 2024

Conversation

asl
Copy link
Contributor

@asl asl commented Jul 24, 2024

This ensures we do not see duplicate diagnostics from there.

Some small cleanup while there.

@asl asl added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jul 24, 2024
@asl asl requested a review from vlstill July 24, 2024 06:04
@asl
Copy link
Contributor Author

asl commented Jul 24, 2024

Found this while working on #4829, full explanation is in #4829 (comment)

Long story short: when calling error() from inside Transformer we passed non-const pointer to Node and due to this wrong (SourceInfo-less) overload was selected.

@asl asl requested a review from fruffy July 24, 2024 06:06
lib/source_file.h Show resolved Hide resolved
@ChrisDodd
Copy link
Contributor

It seems like from the stdout changes here, we're losing some non-duplicate error messages?

@asl
Copy link
Contributor Author

asl commented Jul 30, 2024

It seems like from the stdout changes here, we're losing some non-duplicate error messages?

Not quite. We just visit the same (invalid) code twice despite error already being reported. As a result, multiple errors for the same source line were emitted.

For example:

type-in-expr.p4(5): [--Werror=type-error] error: 4 < H: structured annotation must be compile-time constant values
@foo[bar=4<H]
         ^^^
type-in-expr.p4(5): [--Werror=type-error] error: H: Type cannot be used here, expecting an expression.
@foo[bar=4<H]
           ^
type-in-expr.p4(6): [--Werror=type-error] error: Error while analyzing control p
control p<H>(in H hdrs, out bool flag)
        ^

So, here we are emitting two errors for the same line 5. And the diagnostics on line 6 is just an additional code inside learn() that checks error counts before and after and emit additional error if they are not same. In the majority of cases the latter is superflous.

After the patch the intended functionality is restored, only single error per source line is reported.

…ntext.

This ensures we do not see duplicate diagnostics from there.

Signed-off-by: Anton Korobeynikov <[email protected]>
@asl asl enabled auto-merge August 6, 2024 06:33
@asl asl added this pull request to the merge queue Aug 6, 2024
Merged via the queue into p4lang:main with commit 95e590e Aug 6, 2024
18 checks passed
@asl asl deleted the fix-diagnose branch August 6, 2024 07:55
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.

3 participants