aaron.ballman added a comment.

In D158540#4628629 <https://reviews.llvm.org/D158540#4628629>, @NoumanAmir657 
wrote:

> In D158540#4628310 <https://reviews.llvm.org/D158540#4628310>, @aaron.ballman 
> wrote:
>
>> In D158540#4628265 <https://reviews.llvm.org/D158540#4628265>, 
>> @NoumanAmir657 wrote:
>>
>>> No, I don't have code examples that showcase the importance of the note. As 
>>> you said the class context would be obvious whenever we run into this error.
>>> The test files also don't show where the note would be helpful.
>>
>> The crux of https://github.com/llvm/llvm-project/issues/64843 is about the 
>> error diagnostic, and that logic hasn't been changed in this patch -- are 
>> there other changes that are missing from the patch? The text of the tests 
>> shows that the error diagnostic behavior should have changed as well, but 
>> I'm not seeing the functional changes to make that happen.
>
> Can you elaborate further? I did not understand what you mean by functional 
> changes. According to my knowledge, I don't think anything else is missing 
> from the patch.

The only changes I see in the review are the addition of 
`note_incorrect_defaulted_constexpr` and emitting that note. However, I see 
test cases changing from:

  constexpr W() __constant = default; // expected-error {{defaulted definition 
of default constructor is not constexpr}}

to

  constexpr W() __constant = default; // expected-error {{default constructor 
cannot be 'constexpr' in a class with virtual base classes}} expected-note 
{{see reference to function 'W' in 'W' class}}

where the error wording is now different, but I don't see any code changes to 
the compiler for the error wording.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158540/new/

https://reviews.llvm.org/D158540

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to