[PATCH] D158540: Improve error message for constexpr constructors of virtual base classes

[Nouman Amir via Phabricator via cfe-commits]

NoumanAmir657 added a comment.

In D158540#4628958 <https://reviews.llvm.org/D158540#4628958>, @aaron.ballman wrote:

> In D158540#4628904 <https://reviews.llvm.org/D158540#4628904>, @NoumanAmir657 wrote:
>
>> In D158540#4628860 <https://reviews.llvm.org/D158540#4628860>, @aaron.ballman wrote:
>>
>>> 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.
>>
>> I changed the error wording in the file DiagnosticSemaKinds.td
>>
>> "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with virtual base classes">;
>
> Okay, *now* I am seeing some changes there .. that's really strange, because my previous viewing only showed the *note* changing. I'm very sorry for the confusing noise; it could be that Phab is a bit flaky (it's considerably slower than it usually is).

Okay, I will change these things.


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

https://reviews.llvm.org/D158540