Skip to content

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented May 3, 2023

New DIAGNOSE_ERROR and DIAGNOSE_CRITICAL macros are added. These accept an ID which should indicate a SwiftDiagnosticsSource definition in codeql_diagnostics, together with the usual format + arguments accepted by other LOG_* macros. This peculiar API was somewhat conditioned by binlog's API: its macros take a category that is then stringified before usage (as opposed to accepting a string directly). In particular in order to use the category as a diagnostic source discriminant we cannot rely on any kind of evaluation giving us the category to use.

When the log is flushed, these special logs will result in an error JSON diagnostic entry in the database.

Also this moves all the logging infrastructure in a common log directory within swift, as this will need to be used by the xcode-autobuilder as well.

redsun82 added 3 commits May 3, 2023 14:32
New `DIAGNOSE_ERROR` and `DIAGNOSE_CRITICAL` macros are added.

These accept an ID which should indicate a diagnostic source via
a function definition in `codeql::diagnostics`, together with the usual
format + arguments accepted by other `LOG_*` macros.

When the log is flushed, these special logs will result in an error JSON
diagnostic entry in the database.
@redsun82 redsun82 requested a review from sashabu May 3, 2023 12:39
@github-actions github-actions bot added the Swift label May 3, 2023
@redsun82 redsun82 marked this pull request as ready for review May 3, 2023 13:46
@redsun82 redsun82 requested review from a team as code owners May 3, 2023 13:46
namespace diagnostics {
inline void internal_error() {
SwiftDiagnosticsSource::create("internal_error", "Internal error", {},
"Contact us about this issue");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this needs rewording and proper links, but in any case it's not used yet

@redsun82
Copy link
Contributor Author

redsun82 commented May 3, 2023

I've tested it on a dummy DIAGNOSE_ERROR call, also with codeql database export-diagnostics, but if you feel a single example should be added to make more sense of the PR I can add the first proper diagnostic to the xcode-autobuilder tomorrow

Copy link
Contributor

@sashabu sashabu left a comment

Choose a reason for hiding this comment

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

High-level LGTM, but left a few implementation comments.

@redsun82
Copy link
Contributor Author

redsun82 commented May 4, 2023

@sashabu I've somewhat simplified how diagnostics source are defined, a more direct constexpr SwiftDiagnosticsSource is enough, as opposed to the function that needed to call SwiftDiagnosticsSource::create. I've also merged the assertion PR right now, so I'll go on and replace the new asserts here with that.

@redsun82 redsun82 requested a review from sashabu May 4, 2023 13:15
@redsun82 redsun82 merged commit b511c5f into main May 4, 2023
@redsun82 redsun82 deleted the redsun82/swift-json branch May 4, 2023 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants