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

2017-04-19 Thread Alex Lorenz via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL300667: Add support for editor placeholders to Clang (authored by arphaman). Changed prior to commit: https://reviews.llvm.org/D32081?vs=95596&id=95696#toc Repository: rL LLVM https://reviews.llvm.o

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

2017-04-18 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 95596. arphaman marked 2 inline comments as done. arphaman added a comment. Add comments and remove `-fno-allow-editor-placeholders` from CC1 options. Repository: rL LLVM https://reviews.llvm.org/D32081 Files: include/clang/Basic/DiagnosticLexKinds.td

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

2017-04-18 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added inline comments. Comment at: include/clang/Basic/IdentifierTable.h:358 + /// Return true if this identifier is an editor placeholder. + bool isEditorPlaceholder() const { Nitpick: There should probably be an example in the doc comment. The

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 95349. arphaman added a comment. Check if `IdentifierInfo` is nil in `ActOnIdExpression` to prevent crash. Repository: rL LLVM https://reviews.llvm.org/D32081 Files: include/clang/Basic/DiagnosticLexKinds.td include/clang/Basic/IdentifierTable.h i

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 95348. arphaman marked an inline comment as done. arphaman added a comment. - Don't use the diagnostic engine suppression ranges, but rely on changes to parser/sema for diagnostic suppression. - Rename the compiler flag to '-fallow-editor-placeholders'. Re

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. It's more about handling all of the cases, e.g. the completion results for declaration patterns with placeholders, as those can't be caught with a check at an expression/typename level. I guess then I can catch all of the most common cases now and we can improve the s

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

2017-04-14 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added a comment. > 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? I'd really rather not plumb this into the di

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
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 `isEditorPla

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added a comment. 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 p

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

2017-04-14 Thread Ben Langmuir via Phabricator via cfe-commits
benlangmuir added a comment. 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 p

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman updated this revision to Diff 95297. arphaman added a comment. I've decided to treat each placeholder as an identifier token (like Swift) instead of just lexing the contents. This will prevent spurious errors for declaration name placeholders that have spaces or hyphens inside. Reposi

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

2017-04-14 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman created this revision. This patch is based on the RFC that I've sent out earlier (http://lists.llvm.org/pipermail/cfe-dev/2017-April/053394.html). It teaches Clang's Lexer to recognize the editor placeholders that are produced by some editors like Xcode when expanding code-completion re