subject:"\[PATCH\] D34256\: \[PR33394\] Avoid lexing editor placeholders when running the preprocessor only"

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

2017-06-16 Thread Alex Lorenz via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL305576: [PR33394] Avoid lexing editor placeholders when 
Clang is used only (authored by arphaman).

Changed prior to commit:
  https://reviews.llvm.org/D34256?vs=102764=102862#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D34256

Files:
  cfe/trunk/include/clang/Lex/PreprocessorOptions.h
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Lex/Lexer.cpp
  cfe/trunk/test/Frontend/pp-only-no-editor-placeholders.c

Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -2379,9 +2379,51 @@
   Opts.AllowEditorPlaceholders = Args.hasArg(OPT_fallow_editor_placeholders);
 }
 
+static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
+  switch (Action) {
+  case frontend::ASTDeclList:
+  case frontend::ASTDump:
+  case frontend::ASTPrint:
+  case frontend::ASTView:
+  case frontend::EmitAssembly:
+  case frontend::EmitBC:
+  case frontend::EmitHTML:
+  case frontend::EmitLLVM:
+  case frontend::EmitLLVMOnly:
+  case frontend::EmitCodeGenOnly:
+  case frontend::EmitObj:
+  case frontend::FixIt:
+  case frontend::GenerateModule:
+  case frontend::GenerateModuleInterface:
+  case frontend::GeneratePCH:
+  case frontend::GeneratePTH:
+  case frontend::ParseSyntaxOnly:
+  case frontend::ModuleFileInfo:
+  case frontend::VerifyPCH:
+  case frontend::PluginAction:
+  case frontend::PrintDeclContext:
+  case frontend::RewriteObjC:
+  case frontend::RewriteTest:
+  case frontend::RunAnalysis:
+  case frontend::MigrateSource:
+return false;
+
+  case frontend::DumpRawTokens:
+  case frontend::DumpTokens:
+  case frontend::InitOnly:
+  case frontend::PrintPreamble:
+  case frontend::PrintPreprocessedInput:
+  case frontend::RewriteMacros:
+  case frontend::RunPreprocessorOnly:
+return true;
+  }
+  llvm_unreachable("invalid frontend action");
+}
+
 static void ParsePreprocessorArgs(PreprocessorOptions , ArgList ,
   FileManager ,
-  DiagnosticsEngine ) {
+  DiagnosticsEngine ,
+  frontend::ActionKind Action) {
   using namespace options;
   Opts.ImplicitPCHInclude = Args.getLastArgValue(OPT_include_pch);
   Opts.ImplicitPTHInclude = Args.getLastArgValue(OPT_include_pth);
@@ -2454,52 +2496,23 @@
 else
   Opts.ObjCXXARCStandardLibrary = (ObjCXXARCStandardLibraryKind)Library;
   }
+
+  // Always avoid lexing editor placeholders when we're just running the
+  // preprocessor as we never want to emit the
+  // "editor placeholder in source file" error in PP only mode.
+  if (isStrictlyPreprocessorAction(Action))
+Opts.LexEditorPlaceholders = false;
 }
 
 static void ParsePreprocessorOutputArgs(PreprocessorOutputOptions ,
 ArgList ,
 frontend::ActionKind Action) {
   using namespace options;
 
-  switch (Action) {
-  case frontend::ASTDeclList:
-  case frontend::ASTDump:
-  case frontend::ASTPrint:
-  case frontend::ASTView:
-  case frontend::EmitAssembly:
-  case frontend::EmitBC:
-  case frontend::EmitHTML:
-  case frontend::EmitLLVM:
-  case frontend::EmitLLVMOnly:
-  case frontend::EmitCodeGenOnly:
-  case frontend::EmitObj:
-  case frontend::FixIt:
-  case frontend::GenerateModule:
-  case frontend::GenerateModuleInterface:
-  case frontend::GeneratePCH:
-  case frontend::GeneratePTH:
-  case frontend::ParseSyntaxOnly:
-  case frontend::ModuleFileInfo:
-  case frontend::VerifyPCH:
-  case frontend::PluginAction:
-  case frontend::PrintDeclContext:
-  case frontend::RewriteObjC:
-  case frontend::RewriteTest:
-  case frontend::RunAnalysis:
-  case frontend::MigrateSource:
-Opts.ShowCPP = 0;
-break;
-
-  case frontend::DumpRawTokens:
-  case frontend::DumpTokens:
-  case frontend::InitOnly:
-  case frontend::PrintPreamble:
-  case frontend::PrintPreprocessedInput:
-  case frontend::RewriteMacros:
-  case frontend::RunPreprocessorOnly:
+  if (isStrictlyPreprocessorAction(Action))
 Opts.ShowCPP = !Args.hasArg(OPT_dM);
-break;
-  }
+  else
+Opts.ShowCPP = 0;
 
   Opts.ShowComments = Args.hasArg(OPT_C);
   Opts.ShowLineMarkers = !Args.hasArg(OPT_P);
@@ -2626,7 +2639,8 @@
   // ParsePreprocessorArgs and remove the FileManager
   // parameters from the function and the "FileManager.h" #include.
   FileManager FileMgr(Res.getFileSystemOpts());
-  ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags);
+  ParsePreprocessorArgs(Res.getPreprocessorOpts(), Args, FileMgr, Diags,
+Res.getFrontendOpts().ProgramAction);
   ParsePreprocessorOutputArgs(Res.getPreprocessorOutputOpts(), Args,
   Res.getFrontendOpts().ProgramAction);
 
Index:

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

2017-06-16 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir accepted this revision.
benlangmuir added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D34256



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

2017-06-15 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman updated this revision to Diff 102764.
arphaman added a comment.

Fair enough. I removed the special checks for `<#>` and `<##>`.


Repository:
  rL LLVM

https://reviews.llvm.org/D34256

Files:
  include/clang/Lex/PreprocessorOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/Lexer.cpp
  test/Frontend/pp-only-no-editor-placeholders.c

Index: test/Frontend/pp-only-no-editor-placeholders.c
===
--- /dev/null
+++ test/Frontend/pp-only-no-editor-placeholders.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -E -verify -o - %s | FileCheck %s
+// expected-no-diagnostics
+
+<#placeholder#>; // CHECK: <#placeholder#>;
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -19,6 +19,7 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Compiler.h"
@@ -2750,7 +2751,7 @@
 
 bool Lexer::lexEditorPlaceholder(Token , const char *CurPtr) {
   assert(CurPtr[-1] == '<' && CurPtr[0] == '#' && "Not a placeholder!");
-  if (!PP || LexingRawMode)
+  if (!PP || !PP->getPreprocessorOpts().LexEditorPlaceholders || LexingRawMode)
 return false;
   const char *End = findPlaceholderEnd(CurPtr + 1, BufferEnd);
   if (!End)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2379,9 +2379,51 @@
   Opts.AllowEditorPlaceholders = Args.hasArg(OPT_fallow_editor_placeholders);
 }
 
+static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
+  switch (Action) {
+  case frontend::ASTDeclList:
+  case frontend::ASTDump:
+  case frontend::ASTPrint:
+  case frontend::ASTView:
+  case frontend::EmitAssembly:
+  case frontend::EmitBC:
+  case frontend::EmitHTML:
+  case frontend::EmitLLVM:
+  case frontend::EmitLLVMOnly:
+  case frontend::EmitCodeGenOnly:
+  case frontend::EmitObj:
+  case frontend::FixIt:
+  case frontend::GenerateModule:
+  case frontend::GenerateModuleInterface:
+  case frontend::GeneratePCH:
+  case frontend::GeneratePTH:
+  case frontend::ParseSyntaxOnly:
+  case frontend::ModuleFileInfo:
+  case frontend::VerifyPCH:
+  case frontend::PluginAction:
+  case frontend::PrintDeclContext:
+  case frontend::RewriteObjC:
+  case frontend::RewriteTest:
+  case frontend::RunAnalysis:
+  case frontend::MigrateSource:
+return false;
+
+  case frontend::DumpRawTokens:
+  case frontend::DumpTokens:
+  case frontend::InitOnly:
+  case frontend::PrintPreamble:
+  case frontend::PrintPreprocessedInput:
+  case frontend::RewriteMacros:
+  case frontend::RunPreprocessorOnly:
+return true;
+  }
+  llvm_unreachable("invalid frontend action");
+}
+
 static void ParsePreprocessorArgs(PreprocessorOptions , ArgList ,
   FileManager ,
-  DiagnosticsEngine ) {
+  DiagnosticsEngine ,
+  frontend::ActionKind Action) {
   using namespace options;
   Opts.ImplicitPCHInclude = Args.getLastArgValue(OPT_include_pch);
   Opts.ImplicitPTHInclude = Args.getLastArgValue(OPT_include_pth);
@@ -2454,52 +2496,23 @@
 else
   Opts.ObjCXXARCStandardLibrary = (ObjCXXARCStandardLibraryKind)Library;
   }
+
+  // Always avoid lexing editor placeholders when we're just running the
+  // preprocessor as we never want to emit the
+  // "editor placeholder in source file" error in PP only mode.
+  if (isStrictlyPreprocessorAction(Action))
+Opts.LexEditorPlaceholders = false;
 }
 
 static void ParsePreprocessorOutputArgs(PreprocessorOutputOptions ,
 ArgList ,
 frontend::ActionKind Action) {
   using namespace options;
 
-  switch (Action) {
-  case frontend::ASTDeclList:
-  case frontend::ASTDump:
-  case frontend::ASTPrint:
-  case frontend::ASTView:
-  case frontend::EmitAssembly:
-  case frontend::EmitBC:
-  case frontend::EmitHTML:
-  case frontend::EmitLLVM:
-  case frontend::EmitLLVMOnly:
-  case frontend::EmitCodeGenOnly:
-  case frontend::EmitObj:
-  case frontend::FixIt:
-  case frontend::GenerateModule:
-  case frontend::GenerateModuleInterface:
-  case frontend::GeneratePCH:
-  case frontend::GeneratePTH:
-  case frontend::ParseSyntaxOnly:
-  case frontend::ModuleFileInfo:
-  case frontend::VerifyPCH:
-  case frontend::PluginAction:
-  case frontend::PrintDeclContext:
-  case frontend::RewriteObjC:
-  case frontend::RewriteTest:
-  case frontend::RunAnalysis:
-  case frontend::MigrateSource:
-Opts.ShowCPP = 0;
-break;
-
-  case frontend::DumpRawTokens:
-  case frontend::DumpTokens:
-  case frontend::InitOnly:
-  case

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

2017-06-15 Thread Ben Langmuir via Phabricator via cfe-commits

benlangmuir added a comment.

I agree with not detecting these during PP-only, but there's nothing wrong with 
`<#>`.  It's either not a placeholder, or it's part of a placeholder like 
`<#>#>`, which is a placeholder containing the text ">".  Similarly, `<##` 
could be the start of an empty placeholder `<##>`.


Repository:
  rL LLVM

https://reviews.llvm.org/D34256



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

2017-06-15 Thread Alex Lorenz via Phabricator via cfe-commits

arphaman created this revision.

r300667 added support for editor placeholder to Clang. That commit didn’t take 
into account that users who use Clang for preprocessing only (-E) will get the 
“editor placeholder in source file” error when preprocessing their source 
(PR33394).

This commit ensures that Clang doesn't lex editor placeholders when running a 
preprocessor only action. It also ensures that tokens like `<#>` and `<##>` 
won't form valid placeholders.

rdar://32718000


Repository:
  rL LLVM

https://reviews.llvm.org/D34256

Files:
  include/clang/Lex/PreprocessorOptions.h
  lib/Frontend/CompilerInvocation.cpp
  lib/Lex/Lexer.cpp
  test/Frontend/pp-only-no-editor-placeholders.c
  test/Parser/editor-placeholder-recovery.cpp

Index: test/Parser/editor-placeholder-recovery.cpp
===
--- test/Parser/editor-placeholder-recovery.cpp
+++ test/Parser/editor-placeholder-recovery.cpp
@@ -69,3 +69,11 @@
   // expected-error@-2 {{editor placeholder in source file}}
 #endif
 }
+
+// These two are not valid placeholders:
+void notPlaceholders1() {
+<#> // expected-error {{expected expression}} // expected-error {{expected expression}}
+}
+void notPlaceholders2() {
+<##> // expected-error {{expected expression}} // expected-error {{expected expression}}
+}
Index: test/Frontend/pp-only-no-editor-placeholders.c
===
--- /dev/null
+++ test/Frontend/pp-only-no-editor-placeholders.c
@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -E -verify -o - %s | FileCheck %s
+// expected-no-diagnostics
+
+<#placeholder#>; // CHECK: <#placeholder#>;
Index: lib/Lex/Lexer.cpp
===
--- lib/Lex/Lexer.cpp
+++ lib/Lex/Lexer.cpp
@@ -19,6 +19,7 @@
 #include "clang/Lex/LexDiagnostic.h"
 #include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Lex/PreprocessorOptions.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/Support/Compiler.h"
@@ -2750,9 +2751,12 @@
 
 bool Lexer::lexEditorPlaceholder(Token , const char *CurPtr) {
   assert(CurPtr[-1] == '<' && CurPtr[0] == '#' && "Not a placeholder!");
-  if (!PP || LexingRawMode)
+  if (!PP || !PP->getPreprocessorOpts().LexEditorPlaceholders || LexingRawMode)
 return false;
-  const char *End = findPlaceholderEnd(CurPtr + 1, BufferEnd);
+  const char *NextPtr = CurPtr + 1;
+  if (NextPtr < BufferEnd && (NextPtr[0] == '>' || NextPtr[0] == '#'))
+return false;
+  const char *End = findPlaceholderEnd(NextPtr, BufferEnd);
   if (!End)
 return false;
   const char *Start = CurPtr - 1;
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -2379,9 +2379,51 @@
   Opts.AllowEditorPlaceholders = Args.hasArg(OPT_fallow_editor_placeholders);
 }
 
+static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
+  switch (Action) {
+  case frontend::ASTDeclList:
+  case frontend::ASTDump:
+  case frontend::ASTPrint:
+  case frontend::ASTView:
+  case frontend::EmitAssembly:
+  case frontend::EmitBC:
+  case frontend::EmitHTML:
+  case frontend::EmitLLVM:
+  case frontend::EmitLLVMOnly:
+  case frontend::EmitCodeGenOnly:
+  case frontend::EmitObj:
+  case frontend::FixIt:
+  case frontend::GenerateModule:
+  case frontend::GenerateModuleInterface:
+  case frontend::GeneratePCH:
+  case frontend::GeneratePTH:
+  case frontend::ParseSyntaxOnly:
+  case frontend::ModuleFileInfo:
+  case frontend::VerifyPCH:
+  case frontend::PluginAction:
+  case frontend::PrintDeclContext:
+  case frontend::RewriteObjC:
+  case frontend::RewriteTest:
+  case frontend::RunAnalysis:
+  case frontend::MigrateSource:
+return false;
+
+  case frontend::DumpRawTokens:
+  case frontend::DumpTokens:
+  case frontend::InitOnly:
+  case frontend::PrintPreamble:
+  case frontend::PrintPreprocessedInput:
+  case frontend::RewriteMacros:
+  case frontend::RunPreprocessorOnly:
+return true;
+  }
+  llvm_unreachable("invalid frontend action");
+}
+
 static void ParsePreprocessorArgs(PreprocessorOptions , ArgList ,
   FileManager ,
-  DiagnosticsEngine ) {
+  DiagnosticsEngine ,
+  frontend::ActionKind Action) {
   using namespace options;
   Opts.ImplicitPCHInclude = Args.getLastArgValue(OPT_include_pch);
   Opts.ImplicitPTHInclude = Args.getLastArgValue(OPT_include_pth);
@@ -2454,52 +2496,23 @@
 else
   Opts.ObjCXXARCStandardLibrary = (ObjCXXARCStandardLibraryKind)Library;
   }
+
+  // Always avoid lexing editor placeholders when we're just running the
+  // preprocessor as we never want to emit the
+  // "editor placeholder in source file" error in PP only mode.
+  if

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

[PATCH] D34256: [PR33394] Avoid lexing editor placeholders when running the preprocessor only

5 matches

Site Navigation

Mail list logo

Footer information