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

Suppress readability-redundant-member-init #4538

Merged

Conversation

danakj
Copy link
Contributor

@danakj danakj commented Nov 15, 2024

This initializes the DriverResult::per_file_success field explicitly with = {} in order to encode that DriverResult can be constucted via aggregate initialization while omitting the per_file_success field. This prevents -Wmissing-designated-field-initializers from firing in newer clang versions when constructing DriverResult like:

return {.success = false};

Newer clang-tidy warns that the = {} is redundant however it is not, as its marking which fields need to be explicitly initialized. So we suppress it.

.clang-tidy Outdated
@@ -46,6 +46,7 @@ Checks:
-readability-function-cognitive-complexity, -readability-else-after-return,
-readability-identifier-length, -readability-implicit-bool-conversion,
-readability-magic-numbers, -readability-make-member-function-const,
-readability-redundant-member-init,
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the other PR, to document why we're doing this, can you add a note above like:

  # - readability-redundant-member-init hinders us from setting initializers to
  #   silence -Wmissing-designated-field-initializers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, done and I rebased on top of #4540, so it should land first.

@danakj danakj force-pushed the readability-redundant-member-init branch from 61fa456 to 0890154 Compare November 15, 2024 20:03
auto-merge was automatically disabled November 15, 2024 20:09

Head branch was pushed to by a user without write access

@danakj danakj force-pushed the readability-redundant-member-init branch from f451a8f to ba58f26 Compare November 15, 2024 20:09
This initializes the DriverResult::per_file_success field explicitly
with `= {}` in order to encode that DriverResult can be constucted via
aggregate initialization while omitting the per_file_success field.
This prevents -Wmissing-designated-field-initializers from firing in
newer clang versions when constructing DriverResult like:
```
return {.success = false};
```

Newer clang-tidy warns that the `= {}` is redundant however it is not,
as its marking which fields need to be explicitly initialized. So we
suppress it.
auto-merge was automatically disabled November 15, 2024 21:55

Head branch was pushed to by a user without write access

@danakj danakj force-pushed the readability-redundant-member-init branch from ba58f26 to 92f2e84 Compare November 15, 2024 21:55
@danakj
Copy link
Contributor Author

danakj commented Nov 15, 2024

Rebased, PTAL

@danakj
Copy link
Contributor Author

danakj commented Nov 18, 2024

bump, needs re-merge approval after rebase

@jonmeow jonmeow added this pull request to the merge queue Nov 18, 2024
Merged via the queue into carbon-language:trunk with commit cb94609 Nov 18, 2024
8 checks passed
@danakj danakj deleted the readability-redundant-member-init branch November 18, 2024 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants