[PATCH] Diagnose ref-qualifiers occuring after virt-specifier-seq and generate fixit hints

[Richard Smith]

On Mon, Jan 26, 2015 at 7:38 AM, Ehsan Akhgari <ehsan.akhgari at gmail.com>
wrote:

> ================
> Comment at: lib/Parse/ParseDeclCXX.cpp:2100-2139
> @@ +2099,42 @@
> +                              unsigned* QualifierLoc) {
> +      FixItHint Insertion;
> +      if (DS.getTypeQualifiers() & TypeQual) {
> +        if (!(Function.TypeQuals & TypeQual)) {
> +          std::string Name(FixItName);
> +          Name += " ";
> +          Insertion = FixItHint::CreateInsertion(VS.getFirstLocation(),
> Name.c_str());
> +          Function.TypeQuals |= TypeQual;
> +          *QualifierLoc = SpecLoc.getRawEncoding();
> +        }
> +        Diag(SpecLoc, diag::err_declspec_after_virtspec)
> +          << FixItName
> +          << VirtSpecifiers::getSpecifierName(VS.getLastSpecifier())
> +          << FixItHint::CreateRemoval(SpecLoc)
> +          << Insertion;
> +      }
> +    };
> +    DeclSpecCheck(DeclSpec::TQ_const, "const", DS.getConstSpecLoc(),
> +                  &Function.ConstQualifierLoc);
> +    DeclSpecCheck(DeclSpec::TQ_volatile, "volatile",
> DS.getVolatileSpecLoc(),
> +                  &Function.VolatileQualifierLoc);
> +    DeclSpecCheck(DeclSpec::TQ_restrict, "restrict",
> DS.getRestrictSpecLoc(),
> +                  &Function.RestrictQualifierLoc);
> +  }
> +
> +  // Parse ref-qualifiers.
> +  bool RefQualifierIsLValueRef = true;
> +  SourceLocation RefQualifierLoc;
> +  if (ParseRefQualifier(RefQualifierIsLValueRef, RefQualifierLoc)) {
> +    const char *Name = (RefQualifierIsLValueRef ? "& " : "&& ");
> +    FixItHint Insertion =
> FixItHint::CreateInsertion(VS.getFirstLocation(), Name);
> +    Function.RefQualifierIsLValueRef = RefQualifierIsLValueRef;
> +    Function.RefQualifierLoc = RefQualifierLoc.getRawEncoding();
> +
> +    Diag(RefQualifierLoc, diag::err_declspec_after_virtspec)
> +      << (RefQualifierIsLValueRef ? "&" : "&&")
> +      << VirtSpecifiers::getSpecifierName(VS.getLastSpecifier())
> +      << FixItHint::CreateRemoval(RefQualifierLoc)
> +      << Insertion;
> +    D.SetRangeEnd(RefQualifierLoc);
> +  }
> +}
> ----------------
> rsmith wrote:
> > Maybe only produce a single diagnostic no matter how many specifiers
> were provided? You can then also use `FixItHint::CreateInsertionFromRange`
> to produce a fixit that copies the complete range of specifiers from after
> the virt-specifier to before (this will preserve things like macro names
> and whitespace between the tokens).
> Can you please tell me how that would work?  My fixit hints are careful so
> that if you have something like: `void foo() override final &;` you would
> get `void foo() final & override;` whereas if you have `void foo() final
> override final &;` you would get `void foo() final & override;` (IOW you
> won't get two final keywords.)
>

Hmm, yes, this is tricky. It would be great if we remembered fixit hints
across diagnostics; then you could first emit removal fixits for the
redundant qualifiers and later emit an insertion-from-range hint (which
would pick up whatever remains of that range after the redundant qualifiers
are removed). This same issue will affect *any* insertion-from-range fixit
hint, though, so I think you should go ahead with CreateInsertionFromRange
and we can fix the issue with that interacting with other fixits separately.


> I don't know how to do the same with a range based insertion fixit hint...
>
> http://reviews.llvm.org/D7012
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150210/974307a2/attachment.html>