[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 10 08:24:46 PDT 2023
aaron.ballman added a comment.
In D154675#4484194 <https://reviews.llvm.org/D154675#4484194>, @cor3ntin wrote:
> Thanks, this looks good! (the libc++ test seems completely unrelated, it happens before compilation starts).
> Can you add re release note though? Thanks!
+1 to both of these sentences. :-)
================
Comment at: clang/lib/Sema/SemaInit.cpp:2847
- unsigned OldIndex = NumBases + PrevField->getFieldIndex();
+ unsigned OldIndex = StructuredIndex - 1;
if (StructuredList && OldIndex <= StructuredList->getNumInits()) {
----------------
Should we be asserting that `StructuredIndex` is not 0? The logic in this function makes it tough to see, but the fact that we're in a block checking `IsFirstDesignator` makes me think this is dangerous.
================
Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:66
.y = 1, // override-note {{previous}}
- .y = 1, // override-error {{overrides prior initialization}}
+ .y = 1, // override-error {{overrides prior initialization}} // reorder-note {{previous initialization for field 'y' is here}}
.x = 1, // reorder-error {{declaration order}} override-error {{overrides prior initialization}} override-note {{previous}}
----------------
A few questions: 1) what is this new note attached to? The only `reorder-*` diagnostic I see in the test is for the initialization of `x`. 2) is the note actually on the correct previous use? I would have expected this to be on the assignment to `y` one line above.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154675/new/
https://reviews.llvm.org/D154675
More information about the cfe-commits
mailing list