[cfe-commits] r158178 - in /cfe/trunk: lib/Sema/SemaDecl.cpp test/FixIt/fixit.cpp

[Douglas Gregor]

On Jun 7, 2012, at 6:20 PM, Kaelyn Uhrain wrote:

> On Thu, Jun 7, 2012 at 5:15 PM, Douglas Gregor <dgregor at apple.com> wrote:
> 
> On Jun 7, 2012, at 4:57 PM, Kaelyn Uhrain wrote:
> 
> > Author: rikka
> > Date: Thu Jun  7 18:57:12 2012
> > New Revision: 158178
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=158178&view=rev
> > Log:
> > Teach the FixIt in DiagnoseInvalidRedeclaration how to replace the written
> > nested name specifiers in addition to the function's identifier when the
> > correction has a different nested name specifier.
> >
> > Modified:
> >    cfe/trunk/lib/Sema/SemaDecl.cpp
> >    cfe/trunk/test/FixIt/fixit.cpp
> >
> > Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=158178&r1=158177&r2=158178&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> > +++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Jun  7 18:57:12 2012
> > @@ -4742,15 +4742,19 @@
> >     }
> >   }
> >
> > -  if (Correction)
> > -    SemaRef.Diag(NewFD->getLocation(), DiagMsg)
> > +  if (Correction) {
> > +    SourceRange FixItLoc(NewFD->getLocation());
> > +    CXXScopeSpec &SS = ExtraArgs.D.getCXXScopeSpec();
> > +    if (Correction.getCorrectionSpecifier() && SS.isValid())
> > +      FixItLoc.setBegin(SS.getBeginLoc());
> > +    SemaRef.Diag(NewFD->getLocStart(), DiagMsg)
> >         << Name << NewDC << Correction.getQuoted(SemaRef.getLangOpts())
> >         << FixItHint::CreateReplacement(
> > -            NewFD->getLocation(),
> > -            Correction.getAsString(SemaRef.getLangOpts()));
> > -  else
> > +            FixItLoc, Correction.getAsString(SemaRef.getLangOpts()));
> 
> Don't you also need to update D.getCXXScopeSpec() to the correction specifier?
> 
> If I remember correctly from way back when I first added the namespace-aware typo correction support, changing that CXXScopeSpec broke things in unexpected ways.... though it may also have been that I did not (and still don't) know how update it with the freshly synthesized NestedNameSpecifier* that lacks location information--or even that things broke because I hadn't created a corresponding NestedNameSpecifierLoc with valid location info. Though from what I can tell e.g. with the test case I added to make sure the substitution didn't leave the old specifier in the code, updating D.getCXXScopeSpecifier() isn't necessary.

Okay, thanks for clarifying.

	- Doug

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120607/f2d0debd/attachment.html>