[cfe-commits] [patch] Fixed for function specifiers

Douglas Gregor dgregor at apple.com
Thu May 14 10:41:34 PDT 2009


On May 13, 2009, at 1:56 PM, Anders Johnsen wrote:

> Hi Eli,
>
> The isExplicitOkay changes look generally okay, except that I'm not
> sure about changing the text of the diagnostic: it's stating something
> that isn't actually true in C++0x.
>
> Yeah, fixed.

I think it could still be slightly better with, e.g.,

def err_explicit_non_function : Error<
   "'explicit' can only appear on constructors%select{| and conversion  
functions}0">;

Then, when you emit the diagnostic, use, e.g.,

	Diag(Loc, diag::err_explicit_non_function) << getLangOptions 
().CPlusPlus0x;

> I don't think the change to the signature of
> DiagnoseFunctionSpecifiers is a good idea: I never actually got around
> to implementing it, but it makes it impossible to fix the FIXME at the
> top of the function.  It may seem like code duplication, but I think
> I'd prefer to use separate diagnostics for the
> ParsedFreeStandingDeclSpec case.
>
> Changed back, and tried to fix the FIXME.  (should be removed if  
> solution is approved)


I definitely prefer it this way, where it points at the identifier. It  
makes much more sense for, e.g.,

	inline int f(int), g;

I suggest that we introduce fix-it hints that remove the erroneous  
specifier, e.g., for inline on non-functions, use:

	Diag(D.getIdentifierLoc(), diag::err_inline_non_function)
		<< CodeModificationHint::CreateRemoval(SourceRange(D.getDeclSpec 
().getInlineSpecLoc()))
		<< SourceRange(D.getDeclSpec().getInlineSpecLoc());

With those tweaks, this patch looks good. Thanks!

   - Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20090514/86bed5e3/attachment.html>


More information about the cfe-commits mailing list