[cfe-commits] PATCH: Make DiagnoseInvalidRedeclaration state when a member decl does not match due to "const" mismatch (issue 5167048)

[David Blaikie]

On Mon, Oct 3, 2011 at 9:43 AM, Kaelyn Uhrain <rikka at google.com> wrote:

> On Fri, Sep 30, 2011 at 4:39 PM, Eli Friedman <eli.friedman at gmail.com>wrote:
>
>> On Fri, Sep 30, 2011 at 4:12 PM,  <rikka at google.com> wrote:
>> > Reviewers: chandlerc,
>> >
>> >
>> >
>> > Please review this at http://codereview.appspot.com/5167048/
>> >
>> > Affected files:
>> >  M include/clang/Basic/DiagnosticSemaKinds.td
>> >  M lib/Sema/SemaDecl.cpp
>> >  M test/SemaCXX/function-redecl.cpp
>> >  M test/SemaCXX/nested-name-spec.cpp
>>
>> I don't really like the wording "member declaration has const
>> keyword"; there is no guarantee that the note points anywhere near the
>> keyword "const", and you've lost the "nearly matches" part of the
>> original diagnostic.  Maybe something more like "member declaration
>> does not match because it is const qualified"?
>>
>
> Yeah, I was having trouble coming up with wording that would work both when
> the declaration being diagnosed doesn't have a "const" but one of the
> matched decls does, and when the matched decl doesn't but the decl being
> diagnosed does. In particular I was trying to avoid having those two cases
> being two separate Diag statements under two branches of an if statement.
>

Any particular reason you were trying to avoid that? I expect it'd probably
make for better diagnostic text (as Eli was suggesting - though admittedly
we haven't quite arrived at the ideal text yet) but I expect it'd also be
nice to add fixits too & those would differ between the two cases (if we
chose a consistent approach - eg: always assume the original declaration is
correct so issue a fixit to fix the definition (by adding or removing
const)).

- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111003/0830a525/attachment.html>