[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
github-actions[bot] wrote: @vvd170501 Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with a build, you may recieve a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail [here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr). If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of [LLVM development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/AaronBallman closed https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From f73060f7f09a747c90fa559641abd8c72f4ee66f Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH 1/8] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e8b4139d7893ce..0791a0002319de 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -517,7 +517,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 91105d4231f06a..8cf58299602ec7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6167,6 +6167,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 0fd458837163e5..3227f16dd0c1ce 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff 5dc9e87c8cae7842edcaa4dd01308873109208da fa70ace81c2311636ce369259aded2b8f40ad0d6 -- clang/lib/Sema/SemaInit.cpp clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp `` View the diff from clang-format here. ``diff diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index e44752bb03..bb8748f443 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2358,7 +2358,8 @@ void InitListChecker::CheckStructUnionTypes( // Emit warnings for missing struct field initializers. // This check is disabled for designated initializers in C. // This matches gcc behaviour. - bool IsCDesignatedInitializer = HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus; + bool IsCDesignatedInitializer = + HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus; if (!VerifyOnly && InitializedSomething && !RD->isUnion() && !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && !IsCDesignatedInitializer) { `` https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From f73060f7f09a747c90fa559641abd8c72f4ee66f Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH 1/7] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e8b4139d7893ce..0791a0002319de 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -517,7 +517,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 91105d4231f06a..8cf58299602ec7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6167,6 +6167,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 0fd458837163e5..3227f16dd0c1ce 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { erichkeane wrote: That one definitely helps... I was hoping they grouped better. But I guess if that is what we can get. Basically my issue is how difficult it was to deduce what I was looking at with all hte conditions. Perhaps Aaron has a suggestion. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 edited https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { vvd170501 wrote: > I am asking for the list of '!'s to be shorter/grouped into a couple of > 'bools' with descriptive names Like this? Other checks seem unrelated to each other. ```diff + bool IsCDesignatedInitializer = HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus; if (!VerifyOnly && InitializedSomething && !RD->isUnion() && !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && - !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { + !IsCDesignatedInitializer) { ``` https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { erichkeane wrote: I'm not asking to add a level/'if'. I'm asking for declaring a set of bools above this 'if' that better reflect what is being tested. With HasDesignatedInit/CPlusPlus checks only being vaguely related, and the 'sort' of the checks not really being clear, I am asking for the list of '!'s to be shorter/grouped into a couple of 'bools' with descriptive names. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 edited https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { vvd170501 wrote: @erichkeane, I did something like this before 83864610d6b6470a0f229c516821d8344d994562, but this adds another indentation level. It's possible to calculate `DisableCheck` before the `if` statement, but it'll probably be less optimal than the current solution - if `VerifyOnly` is true, `DisableCheck` is not needed. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { vvd170501 wrote: Unexpanded condition corresponds to the comment I left a few lines above: > This check is disabled for designated initializers in C. For me personally, it looks a bit less readable in expanded form ("check unused fields if there are no designated initializers or if language is C++") https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From f73060f7f09a747c90fa559641abd8c72f4ee66f Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH 1/6] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e8b4139d7893ce..0791a0002319de 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -517,7 +517,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 91105d4231f06a..8cf58299602ec7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6167,6 +6167,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 0fd458837163e5..3227f16dd0c1ce 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From f73060f7f09a747c90fa559641abd8c72f4ee66f Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH 1/5] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e8b4139d7893ce..0791a0002319de 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -517,7 +517,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 91105d4231f06a..8cf58299602ec7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6167,6 +6167,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 0fd458837163e5..3227f16dd0c1ce 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { erichkeane wrote: Minor preference for extracting the condition out into well-named bool variables, this is getting to be a little bit of a mess. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,8 +2356,11 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { + // This check is disabled for designated initializers in C. + // This matches gcc behaviour. + if (!VerifyOnly && InitializedSomething && !RD->isUnion() && + !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) && + !(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus)) { AaronBallman wrote: ```suggestion (!HasDesignatedInit || SemaRef.getLangOpts().CPlusPlus)) { ``` https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/AaronBallman approved this pull request. Changes mostly LGTM aside from some minor nits. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -148,6 +148,10 @@ Non-comprehensive list of changes in this release New Compiler Flags -- +- ``-Wmissing-designated-field-initializers``, grouped under ``-Wmissing-designated-field-initializers``. + This diagnostic can be disabled to make ``-Wmissing-designated-field-initializers`` behave AaronBallman wrote: ```suggestion This diagnostic can be disabled to make ``-Wmissing-field-initializers`` behave ``` https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -120,6 +120,10 @@ Non-comprehensive list of changes in this release New Compiler Flags -- +- ``-Wmissing-designated-field-initializers``, grouped under ``-Wmissing-designated-field-initializers``. AaronBallman wrote: ```suggestion - ``-Wmissing-designated-field-initializers``, grouped under ``-Wmissing-field-initializers``. ``` https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -120,6 +120,10 @@ Non-comprehensive list of changes in this release New Compiler Flags -- +- ``-Wmissing-designated-field-initializers``, grouped under ``-Wmissing-designated-field-initializers``. AaronBallman wrote: I think it would be good to have if we are certain the changes are correct and safe (we're running out of time for getting things into 18.x). https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
vvd170501 wrote: Ping https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From f73060f7f09a747c90fa559641abd8c72f4ee66f Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH 1/4] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index e8b4139d7893ce..0791a0002319de 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -517,7 +517,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 91105d4231f06a..8cf58299602ec7 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6167,6 +6167,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index 0fd458837163e5..3227f16dd0c1ce 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -120,6 +120,10 @@ Non-comprehensive list of changes in this release New Compiler Flags -- +- ``-Wmissing-designated-field-initializers``, grouped under ``-Wmissing-designated-field-initializers``. shafik wrote: CC @AaronBallman https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -120,6 +120,10 @@ Non-comprehensive list of changes in this release New Compiler Flags -- +- ``-Wmissing-designated-field-initializers``, grouped under ``-Wmissing-designated-field-initializers``. vvd170501 wrote: Would it be ok to also add this patch to the 18.x release branch? It would be nice to have a way to keep old behavior of the m-f-i check when upgrading from Clang 16/17 to 18. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From 9701011a9c88397cf3bb2c089e7295f62c7e13fe Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH 1/4] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 975eca0ad9b642..bda533f77cc56a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -516,7 +516,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 83b89d1449f420..bbb46a9d6f3c6a 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6167,6 +6167,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index b6de06464cd6f3..08aa50ebad331f 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) || +(HasDesignatedInit && !SemaRef.getLangOpts().CPlusPlus); AaronBallman wrote: This can be hoisted into the preceding `if` statement so we drop a level of indentation below. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
@@ -6165,6 +6165,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; AaronBallman wrote: ```suggestion // The same warning, but another group is needed to disable it separately. def warn_missing_designated_field_initializers : Warning< warn_missing_field_initializers.Summary>, InGroup, DefaultIgnore; ``` This way we don't have to worry about the two diagnostics getting different wording. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/AaronBallman edited https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/AaronBallman commented: Thank you for this improvement! Please add a release note to `clang/docs/ReleaseNotes.rst` so users know about the new diagnostic group. https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 edited https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Vadim D. (vvd170501) Changes Fixes #68933. #56628 changed the behavior of `-Wmissing-field-initializers`, which introduces many new warnings in C++ code that uses partial designated initializers. If such code is being built with `-Wextra -Werror`, this change will break the build. This PR adds a new flag that allows to disable these new warnings, as was suggested by @AaronBallman in the original issue: https://github.com/llvm/llvm-project/issues/56628#issuecomment-1761510850 --- Full diff: https://github.com/llvm/llvm-project/pull/81364.diff 4 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+9-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+4) - (modified) clang/lib/Sema/SemaInit.cpp (+26-24) - (modified) clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp (+6-5) ``diff diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 975eca0ad9b642..bda533f77cc56a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -516,7 +516,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b4dc4feee8e63a..69e197e26b9b45 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6165,6 +6165,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index b6de06464cd6f3..08aa50ebad331f 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly &&
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 ready_for_review https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 edited https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 updated https://github.com/llvm/llvm-project/pull/81364 >From b7e3ffb25e1a50ac29e50ac8536ea93244f219a1 Mon Sep 17 00:00:00 2001 From: vvd170501 <36827317+vvd170...@users.noreply.github.com> Date: Sat, 10 Feb 2024 19:19:52 +0300 Subject: [PATCH] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 975eca0ad9b642..bda533f77cc56a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -516,7 +516,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b4dc4feee8e63a..69e197e26b9b45 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6165,6 +6165,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index b6de06464cd6f3..08aa50ebad331f 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
github-actions[bot] wrote: Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using `@` followed by their GitHub username. If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the [LLVM GitHub User Guide](https://llvm.org/docs/GitHub.html). You can also ask questions in a comment on this PR, on the [LLVM Discord](https://discord.com/invite/xS7Z362) or on the [forums](https://discourse.llvm.org/). https://github.com/llvm/llvm-project/pull/81364 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang] Add -Wmissing-designated-field-initializers (PR #81364)
https://github.com/vvd170501 created https://github.com/llvm/llvm-project/pull/81364 Fixes #68933. >From aa48713c9bf23c24bbc0c22dd67f332ca970d2b8 Mon Sep 17 00:00:00 2001 From: Vadim Dudkin Date: Sat, 10 Feb 2024 02:29:48 +0300 Subject: [PATCH] Add -Wmissing-designated-field-initializers, decide whether to skip m-f-i check only when needed --- clang/include/clang/Basic/DiagnosticGroups.td | 10 +++- .../clang/Basic/DiagnosticSemaKinds.td| 4 ++ clang/lib/Sema/SemaInit.cpp | 50 ++- .../SemaCXX/cxx2a-initializer-aggregates.cpp | 11 ++-- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 975eca0ad9b642..bda533f77cc56a 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -516,7 +516,15 @@ def MethodSignatures : DiagGroup<"method-signatures">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; -def MissingFieldInitializers : DiagGroup<"missing-field-initializers">; +def MissingDesignatedFieldInitializers : DiagGroup<"missing-designated-field-initializers">{ + code Documentation = [{ +Warn about designated initializers with some fields missing (only in C++). + }]; +} +// Default -Wmissing-field-initializers matches gcc behavior, +// but missing-designated-field-initializers can be turned off to match old clang behavior. +def MissingFieldInitializers : DiagGroup<"missing-field-initializers", + [MissingDesignatedFieldInitializers]>; def ModuleLock : DiagGroup<"module-lock">; def ModuleBuild : DiagGroup<"module-build">; def ModuleImport : DiagGroup<"module-import">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index b4dc4feee8e63a..69e197e26b9b45 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -6165,6 +6165,10 @@ def ext_initializer_string_for_char_array_too_long : ExtWarn< def warn_missing_field_initializers : Warning< "missing field %0 initializer">, InGroup, DefaultIgnore; +// The same warning, but another group is needed to disable it separately. +def warn_missing_designated_field_initializers : Warning< + "missing field %0 initializer">, + InGroup, DefaultIgnore; def warn_braces_around_init : Warning< "braces around %select{scalar |}0initializer">, InGroup>; diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp index b6de06464cd6f3..08aa50ebad331f 100644 --- a/clang/lib/Sema/SemaInit.cpp +++ b/clang/lib/Sema/SemaInit.cpp @@ -2227,8 +2227,6 @@ void InitListChecker::CheckStructUnionTypes( size_t NumRecordDecls = llvm::count_if(RD->decls(), [&](const Decl *D) { return isa(D) || isa(D); }); - bool CheckForMissingFields = -!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()); bool HasDesignatedInit = false; llvm::SmallPtrSet InitializedFields; @@ -2269,11 +2267,6 @@ void InitListChecker::CheckStructUnionTypes( } InitializedSomething = true; - - // Disable check for missing fields when designators are used. - // This matches gcc behaviour. - if (!SemaRef.getLangOpts().CPlusPlus) -CheckForMissingFields = false; continue; } @@ -2285,7 +2278,7 @@ void InitListChecker::CheckStructUnionTypes( // These are okay for randomized structures. [C99 6.7.8p19] // // Also, if there is only one element in the structure, we allow something -// like this, because it's really not randomized in the tranditional sense. +// like this, because it's really not randomized in the traditional sense. // // struct foo h = {bar}; auto IsZeroInitializer = [&](const Expr *I) { @@ -2363,23 +2356,32 @@ void InitListChecker::CheckStructUnionTypes( } // Emit warnings for missing struct field initializers. - if (!VerifyOnly && InitializedSomething && CheckForMissingFields && - !RD->isUnion()) { -// It is possible we have one or more unnamed bitfields remaining. -// Find first (if any) named field and emit warning. -for (RecordDecl::field_iterator it = HasDesignatedInit ? RD->field_begin() - : Field, -end = RD->field_end(); - it != end; ++it) { - if (HasDesignatedInit && InitializedFields.count(*it)) -continue; + if (!VerifyOnly && InitializedSomething && !RD->isUnion()) { +// Disable missing fields check for: +// - Zero initializers +// - Designated initializers (only in C). This matches gcc behaviour. +bool DisableCheck = +IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts()) || +