This looks good to me, although I'd prefer if previous_mismatch were in camelCase. Otherwise, please commit. Thanks!
Sent from my iPhone On Jun 8, 2011, at 2:13 PM, Richard Trieu <[email protected]> wrote: > On Tue, Jun 7, 2011 at 2:18 PM, Richard Trieu <[email protected]> wrote: > > > On Wed, Jun 1, 2011 at 4:33 PM, Douglas Gregor <[email protected]> wrote: > > On May 29, 2011, at 1:51 PM, Richard Trieu wrote: > >> Made changes to how 'struct'/'class' mismatches are handled. >> - Removed fix-it hints from template instaniations since changes to the >> templates are rarely helpful. >> - Changed the caret in template instaniations from the class/struct name to >> the >> class/struct keyword, matching the other warnings. >> - Do not offer fix-it hints when multiple declarations disagree. Warnings >> are >> still given. >> - Once a definition is found, offer a fix-it hint to all previous >> declarations >> with wrong tag. >> - Declarations that disagree with a previous definition will get a fix-it >> hint >> to change the declaration. > > I think this is great. I have one question: > > + if (isTemplate) { > + Diag(NewTagLoc, diag::warn_struct_class_tag_mismatch) > + << (NewTag == TTK_Class) > + << isTemplate << &Name > + << FixItHint::CreateReplacement(SourceRange(NewTagLoc), > + OldTag == TTK_Class ? > + "class" : "struct"); > + Diag(Previous->getLocation(), diag::note_previous_use); > + return true; > + } > > Why do we bail out early for templates, rather than treating them the same > way as non-templates? > > - Doug > > Not really. I thought there was some issue with it earlier, but can't > remember what it is now. I'll test it out and see if there's any problems. > > Made the following changes: > Templates and non-templates are now treated the same. > Added FileCheck to the test case. > Fixed issue so that definitions now check all previous declarations for > mismatches. > <struct-class-mismatch2.patch>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
