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). Do you want the note to be like this: ../llvm-test/x.cpp:5:1: note: virtual base class declared here 5 | struct Derived : virtual Base { | ^ 1 error generated. 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
