This is an archive of the discontinued LLVM Phabricator instance.

[clang] Warning for incomplete array initializer lists
Needs ReviewPublic

Authored by Sinitax on Jun 3 2023, 2:45 PM.

Details

Reviewers
None
Group Reviewers
Restricted Project
Summary

This patch implements a warning for incomplete array initializer lists.

Without warnings a 'gap' in an array can go unnoticed when the size increases. Consider what happens with the array const char *err_msg[ERR_TYPE_COUNT] = { ... } when a new ERR_.. enum value is added.

Related: https://github.com/llvm/llvm-project/issues/22692

Diff Detail

Event Timeline

Sinitax created this revision.Jun 3 2023, 2:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 3 2023, 2:45 PM
Sinitax requested review of this revision.Jun 3 2023, 2:45 PM
Sinitax retitled this revision from Warning for uninitialized elements in fixed-size arrays to [clang] Warning for uninitialized elements in fixed-size arrays.Jun 3 2023, 2:50 PM
Sinitax updated this revision to Diff 528162.Jun 3 2023, 4:28 PM

Remove new warning from -Wuninitialized group for backwards-compatibility.

Sinitax updated this revision to Diff 528256.Jun 4 2023, 6:22 PM

Add new warning to its own group.

Sinitax updated this revision to Diff 528387.Jun 5 2023, 5:51 AM

Apply git-clang-format.

shafik added a reviewer: Restricted Project.Jun 5 2023, 9:54 PM
shafik added a subscriber: shafik.

Adding reviewer for more visibility

philnik added a subscriber: philnik.Jun 6 2023, 8:52 AM
philnik added inline comments.
clang/include/clang/Basic/DiagnosticGroups.td
732 ↗(On Diff #528387)

Why do you make this a new warning group instead of just adding it to -Wuninitialized?

clang/lib/Sema/SemaInit.cpp
983 ↗(On Diff #528387)

You'll want to add tests for this in clang/test/Sema

Needs release notes and tests. Patch needs full-context added.

clang/include/clang/Basic/DiagnosticGroups.td
732 ↗(On Diff #528387)

I think the new group makes sense, but it should be a part of Uninitialized as well (see 733).

clang/lib/Sema/SemaInit.cpp
991 ↗(On Diff #528387)

should probably just do range-for here, use the .inits here instead. Perhaps even better is a llvm::find of nullptr.

Sinitax updated this revision to Diff 529658.Jun 8 2023, 10:15 AM

Rename warning to -Wincomplete-array-initializer-list for clarity, and add test case.

Sinitax updated this revision to Diff 529671.Jun 8 2023, 10:32 AM

Rebased changes onto commit tagged 'llvmorg-17-init', and added release notes.

Why do you make this a new warning group instead of just adding it to -Wuninitialized?

I think a separate group is best, since the warning emits false positives: an intentional empty-initialization (such as char buf[BUF_SIZ] = { 0 }) is indistinguishable from unintentionally incomplete initialization (such as int pair[2] = { ENUM_A /*, ENUM_B */ }).

For this reason users would want to selectively enable it..

#pragma clang diagnostic push
#pragma clang diagnostic warning "-Wincomplete-initializer-list"

const char *errmsg[ERR_COUNT] = {
    [ERR_A] = "error a",
};

#pragma clang diagnostic pop

.. but I'm not really satisfied with this solution. Feedback on how to differentiate between the aforementioned cases, if possible, is appreciated.

Sinitax updated this revision to Diff 530501.Jun 12 2023, 7:26 AM

Rebase patch against branch main.

Sinitax updated this revision to Diff 530726.Jun 12 2023, 5:17 PM

Fix test.

Sinitax retitled this revision from [clang] Warning for uninitialized elements in fixed-size arrays to [clang] Warning for incomplete array initializer lists.Jun 15 2023, 8:20 AM
Sinitax edited the summary of this revision. (Show Details)
Sinitax marked 4 inline comments as done.Jun 22 2023, 4:57 AM

Can you test in c++ mode?

I'd like to see a test for something like

struct {
	int a, b, c, d  = 1; // d has an initializer
} bar2 = {
	.a = 1,
	.b = 1,
	.c = 1
};

Otherwise looks good