[PATCH] D32081: Add support for editor placeholders to Clang's lexer

Alex Lorenz via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 14 13:34:55 PDT 2017


arphaman added a comment.

In https://reviews.llvm.org/D32081#727511, @arphaman wrote:

> In https://reviews.llvm.org/D32081#727450, @benlangmuir wrote:
>
> > Rather than stick ranges into the diagnostic engine, I think it would be cleaner to have the identifier have a method like `isEditorPlaceholder()` that can be used to avoid situations like this in a principled way, or to customize behaviour for placeholders in the parser, etc.  That's how we are handling it in Swift.  Using an API on the placeholder is also better for handling errors that could be caused by the placeholder but not have it as the primary location.
> >
> > What do you think?
>
>
> I thought about this as well. I'm not sure which way is better though. The current way is simple in the sense that we automatically suppress all diagnostics in the placeholder, so we don't have to modify the parser at all. Clang generates placeholders in a bunch of different places so I reckon there'll have to be a lot of modifications to suppress all problematic cases. Although I suppose that if we teach the parser about the most common places where placeholders could occur (e.g. expressions), that would probably suppress the majority of diagnostics that we care about. I'll try out the parser changes right now.


It seems that suppressing placeholders in expressions and type names would cover the a lot of the common problematic cases. What do you think about a hybrid approach: Don't suppress diagnostics in the placeholder range when we handle the placeholders in parser/sema, but do suppress the range when the placeholder isn't explicitly handled?


Repository:
  rL LLVM

https://reviews.llvm.org/D32081





More information about the cfe-commits mailing list