aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM but please land with a release note.
================
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}}
----------------
shafik wrote:
> aaron.ballman wrote:
> > 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.
> It is attached to :
>
> ```
> .x = 1, // reorder-error {{declaration order}}
> ```
>
> We were not issuing it before b/c it was broken, in this case it was just not
> crashing but it was still broken.
>
> We could argue the note should go on the first `.y` although I could see both
> sides since the override is a warning and not an error I think using the
> second is defensible.
>
>
Oh! I had a brain fart and thought this was about *re*initialization of `y`. I
see what's going on now and yeah, this makes sense. I'm not worried about which
`y` the note attaches to.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D154675/new/
https://reviews.llvm.org/D154675
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
