[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290367: Testbed and skeleton of a new expression parser 
(authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D27180?vs=81991=82360#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/test/Import/clang-flags/Inputs/S.c
  cfe/trunk/test/Import/clang-flags/test.c
  cfe/trunk/test/Import/empty-struct/Inputs/S.c
  cfe/trunk/test/Import/empty-struct/test.c
  cfe/trunk/test/Import/error-in-expression/Inputs/S.c
  cfe/trunk/test/Import/error-in-expression/test.c
  cfe/trunk/test/Import/error-in-import/Inputs/S.c
  cfe/trunk/test/Import/error-in-import/test.c
  cfe/trunk/test/Import/missing-import/test.c
  cfe/trunk/tools/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Index: cfe/trunk/test/CMakeLists.txt
===
--- cfe/trunk/test/CMakeLists.txt
+++ cfe/trunk/test/CMakeLists.txt
@@ -39,6 +39,7 @@
   c-index-test diagtool
   clang-tblgen
   clang-offload-bundler
+  clang-import-test
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)
Index: cfe/trunk/test/Import/empty-struct/test.c
===
--- cfe/trunk/test/Import/empty-struct/test.c
+++ cfe/trunk/test/Import/empty-struct/test.c
@@ -0,0 +1,5 @@
+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
+void expr() {
+  struct S MyS;
+  void *MyPtr = 
+}
Index: cfe/trunk/test/Import/empty-struct/Inputs/S.c
===
--- cfe/trunk/test/Import/empty-struct/Inputs/S.c
+++ cfe/trunk/test/Import/empty-struct/Inputs/S.c
@@ -0,0 +1,2 @@
+struct S {
+};
Index: cfe/trunk/test/Import/clang-flags/Inputs/S.c
===
--- cfe/trunk/test/Import/clang-flags/Inputs/S.c
+++ cfe/trunk/test/Import/clang-flags/Inputs/S.c
@@ -0,0 +1,2 @@
+STRUCT S {
+};
Index: cfe/trunk/test/Import/clang-flags/test.c
===
--- cfe/trunk/test/Import/clang-flags/test.c
+++ cfe/trunk/test/Import/clang-flags/test.c
@@ -0,0 +1,5 @@
+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s -Xcc -DSTRUCT=struct
+void expr() {
+  STRUCT S MyS;
+  void *MyPtr = 
+}
Index: cfe/trunk/test/Import/error-in-expression/Inputs/S.c
===
--- cfe/trunk/test/Import/error-in-expression/Inputs/S.c
+++ cfe/trunk/test/Import/error-in-expression/Inputs/S.c
@@ -0,0 +1,2 @@
+struct S {
+};
Index: cfe/trunk/test/Import/error-in-expression/test.c
===
--- cfe/trunk/test/Import/error-in-expression/test.c
+++ cfe/trunk/test/Import/error-in-expression/test.c
@@ -0,0 +1,6 @@
+// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
+// CHECK: {{.*}}no viable conversion{{.*}}
+void expr() {
+  struct S MyS;
+  void *MyPtr = MyS;
+}
Index: cfe/trunk/test/Import/error-in-import/test.c
===
--- cfe/trunk/test/Import/error-in-import/test.c
+++ cfe/trunk/test/Import/error-in-import/test.c
@@ -0,0 +1,6 @@
+// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
+// CHECK: {{.*}}expected unqualified-id{{.*}}
+void expr() {
+  struct S MyS;
+  void *MyPtr = 
+}
Index: cfe/trunk/test/Import/error-in-import/Inputs/S.c
===
--- cfe/trunk/test/Import/error-in-import/Inputs/S.c
+++ cfe/trunk/test/Import/error-in-import/Inputs/S.c
@@ -0,0 +1,2 @@
+struct S [
+];
Index: cfe/trunk/test/Import/missing-import/test.c
===
--- cfe/trunk/test/Import/missing-import/test.c
+++ cfe/trunk/test/Import/missing-import/test.c
@@ -0,0 +1,6 @@
+// RUN: not clang-import-test -import %S/Inputs/S.c -expression %s 2>&1 | FileCheck %s
+// CHECK: {{.*}}Couldn't open{{.*}}Inputs/S.c{{.*}}
+void expr() {
+  struct S MyS;
+  void *MyPtr = 
+}
Index: cfe/trunk/tools/CMakeLists.txt
===
--- cfe/trunk/tools/CMakeLists.txt
+++ cfe/trunk/tools/CMakeLists.txt
@@ -5,6 +5,7 @@
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
+add_clang_subdirectory(clang-import-test)
 add_clang_subdirectory(clang-offload-bundler)
 
 add_clang_subdirectory(c-index-test)
Index: cfe/trunk/tools/clang-import-test/CMakeLists.txt
===
--- cfe/trunk/tools/clang-import-test/CMakeLists.txt
+++ cfe/trunk/tools/clang-import-test/CMakeLists.txt
@@ -0,0 +1,27 @@
+set(LLVM_LINK_COMPONENTS
+  core
+  support

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Looks good now, thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

Re: [PATCH] D27180: Testbed and skeleton of a new expression parser

[David Blaikie via cfe-commits]

I don't know much about who owns this sort of code, nor Vassil's
work/responsibility in this area - but if he's OK with it/feels able to
sign off on it, that'd be sufficient/fine by me (& others closer to it can
pipe up if he seems like not the right person to approve).

Thanks!
- Dave

On Mon, Dec 19, 2016 at 9:48 AM Sean Callanan  wrote:

> David,
>
> thanks for keeping an eye on this and sorry for the breach of process.
> Would having Vassil approve the changelist (
> https://reviews.llvm.org/D27180) be appropriate?
> Let's say if he has any concerns or can't get to it by tomorrow, we revert
> my patches since they're pretty self-contained.
>
> Sean
>
> On Dec 19, 2016, at 8:55 AM, David Blaikie  wrote:
>
>
>
> On Thu, Dec 15, 2016 at 2:18 PM Sean Callanan via Phabricator via
> cfe-commits  wrote:
>
> spyffe updated this revision to Diff 81661.
> spyffe marked 2 inline comments as done.
> spyffe added a comment.
> Herald added a subscriber: jgosnell.
>
> Applied Vassil and Vedant's comments.  I will commit this soon.
>
>
> Was this change approved/accepted by anyone? "commit if no one has
> objections in " isn't generally how LLVM project changes are
> reviewed/committed.
>
>
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D27180
>
> Files:
>   test/Import/clang-flags/Inputs/S.c
>   test/Import/clang-flags/test.c
>   test/Import/empty-struct/Inputs/S.c
>   test/Import/empty-struct/test.c
>   test/Import/error-in-expression/Inputs/S.c
>   test/Import/error-in-expression/test.c
>   test/Import/error-in-import/Inputs/S.c
>   test/Import/error-in-import/test.c
>   test/Import/missing-import/test.c
>   tools/CMakeLists.txt
>   tools/clang-import-test/CMakeLists.txt
>   tools/clang-import-test/clang-import-test.cpp
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 81991.
spyffe marked 3 inline comments as done.
spyffe added a comment.

Applied Aleksei's comments, and integrated all the CMakeFiles fixes required to 
make the bots happy.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/CMakeLists.txt
  test/Import/clang-flags/Inputs/S.c
  test/Import/clang-flags/test.c
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  test/Import/error-in-expression/Inputs/S.c
  test/Import/error-in-expression/test.c
  test/Import/error-in-import/Inputs/S.c
  test/Import/error-in-import/test.c
  test/Import/missing-import/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,319 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+namespace init_convenience {
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  TestDiagnosticConsumer()
+  : Passthrough(llvm::make_unique()) {}
+
+  virtual void BeginSourceFile(const LangOptions ,
+   const Preprocessor *PP = nullptr) override {
+this->LangOpts = 
+return Passthrough->BeginSourceFile(LangOpts, PP);
+  }
+
+  virtual void EndSourceFile() override {
+this->LangOpts = nullptr;
+Passthrough->EndSourceFile();
+  }
+
+  virtual bool IncludeInDiagnosticCounts() const override {
+return Passthrough->IncludeInDiagnosticCounts();
+  }
+
+private:
+  static void PrintSourceForLocation(const SourceLocation ,
+ SourceManager ) {
+const char *LocData = SM.getCharacterData(Loc, /*Invalid=*/nullptr);
+unsigned LocColumn =
+SM.getSpellingColumnNumber(Loc, /*Invalid=*/nullptr) - 1;
+FileID FID = SM.getFileID(Loc);
+llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, Loc, /*Invalid=*/nullptr);
+
+assert(LocData >= Buffer->getBufferStart() &&
+   LocData < Buffer->getBufferEnd());
+
+const char *LineBegin = LocData - LocColumn;
+
+assert(LineBegin >= Buffer->getBufferStart());
+
+const char *LineEnd = nullptr;
+
+for (LineEnd = LineBegin; *LineEnd != '\n' && *LineEnd != '\r' &&
+  LineEnd < Buffer->getBufferEnd();
+ ++LineEnd)
+  ;
+
+llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
+
+llvm::errs() << LineString << '\n';
+llvm::errs().indent(LocColumn);
+llvm::errs() << '^';
+  }
+
+  virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const Diagnostic ) override {
+if (Info.hasSourceManager() && LangOpts) {
+  SourceManager  = Info.getSourceManager();
+
+  if (Info.getLocation().isValid()) {
+Info.getLocation().print(llvm::errs(), SM);
+llvm::errs() << ": ";
+  }
+
+  SmallString<16> DiagText;
+  Info.FormatDiagnostic(DiagText);
+  llvm::errs() << DiagText << '\n';
+
+  if (Info.getLocation().isValid()) {
+PrintSourceForLocation(Info.getLocation(), SM);
+  }
+
+  for (const CharSourceRange  : Info.getRanges()) {
+bool Invalid = true;
+StringRef Ref = 

Re: [PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via cfe-commits]

Reverted by 290130.  I have new comments from Aleksei in 
https://reviews.llvm.org/D27180 .  I'll apply 
those and update the patch. 

Sean

> On Dec 19, 2016, at 9:48 AM, Sean Callanan  wrote:
> 
> David,
> 
> thanks for keeping an eye on this and sorry for the breach of process.
> Would having Vassil approve the changelist (https://reviews.llvm.org/D27180 
> ) be appropriate?
> Let's say if he has any concerns or can't get to it by tomorrow, we revert my 
> patches since they're pretty self-contained.
> 
> Sean
> 
>> On Dec 19, 2016, at 8:55 AM, David Blaikie > > wrote:
>> 
>> 
>> 
>> On Thu, Dec 15, 2016 at 2:18 PM Sean Callanan via Phabricator via 
>> cfe-commits > 
>> wrote:
>> spyffe updated this revision to Diff 81661.
>> spyffe marked 2 inline comments as done.
>> spyffe added a comment.
>> Herald added a subscriber: jgosnell.
>> 
>> Applied Vassil and Vedant's comments.  I will commit this soon.
>> 
>> Was this change approved/accepted by anyone? "commit if no one has 
>> objections in " isn't generally how LLVM project changes are 
>> reviewed/committed.
>>  
>> 
>> 
>> Repository:
>>   rL LLVM
>> 
>> https://reviews.llvm.org/D27180 
>> 
>> Files:
>>   test/Import/clang-flags/Inputs/S.c
>>   test/Import/clang-flags/test.c
>>   test/Import/empty-struct/Inputs/S.c
>>   test/Import/empty-struct/test.c
>>   test/Import/error-in-expression/Inputs/S.c
>>   test/Import/error-in-expression/test.c
>>   test/Import/error-in-import/Inputs/S.c
>>   test/Import/error-in-import/test.c
>>   test/Import/missing-import/test.c
>>   tools/CMakeLists.txt
>>   tools/clang-import-test/CMakeLists.txt
>>   tools/clang-import-test/clang-import-test.cpp
>> 
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org 
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
>> 

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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Sorry for being late :(




Comment at: cfe/trunk/tools/clang-import-test/clang-import-test.cpp:99
+llvm::errs() << LineString << '\n';
+std::string Space(LocColumn, ' ');
+llvm::errs() << Space.c_str() << '\n';

I still think `indent()` method is more evident here and it doesn't require 
memory allocation.



Comment at: cfe/trunk/tools/clang-import-test/clang-import-test.cpp:125
+if (!Invalid) {
+  llvm::errs() << Ref.str() << '\n';
+}

No `str()` is needed here, `raw_ostream` works well even with non 
null-terminated StringRefs.



Comment at: cfe/trunk/tools/clang-import-test/clang-import-test.cpp:303
+  for (auto I : Imports) {
+  llvm::Expected ImportCI = Parse(I, 
{});
+  if (auto E = ImportCI.takeError()) {

Broken indentation.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

Re: [PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via cfe-commits]

David,

thanks for keeping an eye on this and sorry for the breach of process.
Would having Vassil approve the changelist (https://reviews.llvm.org/D27180 
) be appropriate?
Let's say if he has any concerns or can't get to it by tomorrow, we revert my 
patches since they're pretty self-contained.

Sean

> On Dec 19, 2016, at 8:55 AM, David Blaikie  wrote:
> 
> 
> 
> On Thu, Dec 15, 2016 at 2:18 PM Sean Callanan via Phabricator via cfe-commits 
> > wrote:
> spyffe updated this revision to Diff 81661.
> spyffe marked 2 inline comments as done.
> spyffe added a comment.
> Herald added a subscriber: jgosnell.
> 
> Applied Vassil and Vedant's comments.  I will commit this soon.
> 
> Was this change approved/accepted by anyone? "commit if no one has objections 
> in " isn't generally how LLVM project changes are 
> reviewed/committed.
>  
> 
> 
> Repository:
>   rL LLVM
> 
> https://reviews.llvm.org/D27180 
> 
> Files:
>   test/Import/clang-flags/Inputs/S.c
>   test/Import/clang-flags/test.c
>   test/Import/empty-struct/Inputs/S.c
>   test/Import/empty-struct/test.c
>   test/Import/error-in-expression/Inputs/S.c
>   test/Import/error-in-expression/test.c
>   test/Import/error-in-import/Inputs/S.c
>   test/Import/error-in-import/test.c
>   test/Import/missing-import/test.c
>   tools/CMakeLists.txt
>   tools/clang-import-test/CMakeLists.txt
>   tools/clang-import-test/clang-import-test.cpp
> 
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org 
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits 
> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D27180: Testbed and skeleton of a new expression parser

[Vassil Vassilev via cfe-commits]

On 19/12/16 17:55, David Blaikie wrote:



On Thu, Dec 15, 2016 at 2:18 PM Sean Callanan via Phabricator via 
cfe-commits > wrote:


spyffe updated this revision to Diff 81661.
spyffe marked 2 inline comments as done.
spyffe added a comment.
Herald added a subscriber: jgosnell.

Applied Vassil and Vedant's comments.  I will commit this soon.


Was this change approved/accepted by anyone? "commit if no one has 
objections in " isn't generally how LLVM project changes 
are reviewed/committed.
I could have greenlit this (as I personally pinged Sean on this patch. I 
need some of it to make progress on the libInterpreter part) :(




Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/Import/clang-flags/Inputs/S.c
  test/Import/clang-flags/test.c
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  test/Import/error-in-expression/Inputs/S.c
  test/Import/error-in-expression/test.c
  test/Import/error-in-import/Inputs/S.c
  test/Import/error-in-import/test.c
  test/Import/missing-import/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

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



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

Re: [PATCH] D27180: Testbed and skeleton of a new expression parser

[David Blaikie via cfe-commits]

On Thu, Dec 15, 2016 at 2:18 PM Sean Callanan via Phabricator via
cfe-commits  wrote:

> spyffe updated this revision to Diff 81661.
> spyffe marked 2 inline comments as done.
> spyffe added a comment.
> Herald added a subscriber: jgosnell.
>
> Applied Vassil and Vedant's comments.  I will commit this soon.
>

Was this change approved/accepted by anyone? "commit if no one has
objections in " isn't generally how LLVM project changes are
reviewed/committed.


>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D27180
>
> Files:
>   test/Import/clang-flags/Inputs/S.c
>   test/Import/clang-flags/test.c
>   test/Import/empty-struct/Inputs/S.c
>   test/Import/empty-struct/test.c
>   test/Import/error-in-expression/Inputs/S.c
>   test/Import/error-in-expression/test.c
>   test/Import/error-in-import/Inputs/S.c
>   test/Import/error-in-import/test.c
>   test/Import/missing-import/test.c
>   tools/CMakeLists.txt
>   tools/clang-import-test/CMakeLists.txt
>   tools/clang-import-test/clang-import-test.cpp
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL290004: Testbed and skeleton of a new expression parser 
(authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D27180?vs=81803=81807#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/test/Import/clang-flags/Inputs/S.c
  cfe/trunk/test/Import/clang-flags/test.c
  cfe/trunk/test/Import/empty-struct/Inputs/S.c
  cfe/trunk/test/Import/empty-struct/test.c
  cfe/trunk/test/Import/error-in-expression/Inputs/S.c
  cfe/trunk/test/Import/error-in-expression/test.c
  cfe/trunk/test/Import/error-in-import/Inputs/S.c
  cfe/trunk/test/Import/error-in-import/test.c
  cfe/trunk/test/Import/missing-import/test.c
  cfe/trunk/tools/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/CMakeLists.txt
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Index: cfe/trunk/tools/CMakeLists.txt
===
--- cfe/trunk/tools/CMakeLists.txt
+++ cfe/trunk/tools/CMakeLists.txt
@@ -5,6 +5,7 @@
 add_clang_subdirectory(clang-format)
 add_clang_subdirectory(clang-format-vs)
 add_clang_subdirectory(clang-fuzzer)
+add_clang_subdirectory(clang-import-test)
 add_clang_subdirectory(clang-offload-bundler)
 
 add_clang_subdirectory(c-index-test)
Index: cfe/trunk/tools/clang-import-test/clang-import-test.cpp
===
--- cfe/trunk/tools/clang-import-test/clang-import-test.cpp
+++ cfe/trunk/tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,319 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+namespace init_convenience {
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  TestDiagnosticConsumer()
+  : Passthrough(llvm::make_unique()) {}
+
+  virtual void BeginSourceFile(const LangOptions ,
+   const Preprocessor *PP = nullptr) override {
+this->LangOpts = 
+return Passthrough->BeginSourceFile(LangOpts, PP);
+  }
+
+  virtual void EndSourceFile() override {
+this->LangOpts = nullptr;
+Passthrough->EndSourceFile();
+  }
+
+  virtual bool IncludeInDiagnosticCounts() const override {
+return Passthrough->IncludeInDiagnosticCounts();
+  }
+
+private:
+  static void PrintSourceForLocation(const SourceLocation ,
+ SourceManager ) {
+const char *LocData = SM.getCharacterData(Loc, /*Invalid=*/nullptr);
+unsigned LocColumn =
+SM.getSpellingColumnNumber(Loc, /*Invalid=*/nullptr) - 1;
+FileID FID = SM.getFileID(Loc);
+llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, Loc, /*Invalid=*/nullptr);
+
+assert(LocData >= Buffer->getBufferStart() &&
+   LocData < Buffer->getBufferEnd());
+
+const char *LineBegin = LocData - LocColumn;
+
+assert(LineBegin >= Buffer->getBufferStart());
+
+const char *LineEnd = nullptr;
+
+for (LineEnd = LineBegin; *LineEnd != '\n' && *LineEnd != '\r' &&
+  LineEnd < Buffer->getBufferEnd();
+ ++LineEnd)
+  ;
+
+llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
+
+llvm::errs() << LineString << '\n';
+std::string Space(LocColumn, ' ');
+llvm::errs() << Space.c_str() << '\n';
+  }
+
+  virtual void HandleDiagnostic(DiagnosticsEngine::Level 

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 81803.
spyffe added a comment.

Updated CMakeFiles to fix dependencies.

- Fixed dependencies on `gen_intrinsics`
- Added a testsuite dependency on clang-import-test


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/CMakeLists.txt
  test/Import/clang-flags/Inputs/S.c
  test/Import/clang-flags/test.c
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  test/Import/error-in-expression/Inputs/S.c
  test/Import/error-in-expression/test.c
  test/Import/error-in-import/Inputs/S.c
  test/Import/error-in-import/test.c
  test/Import/missing-import/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,319 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+namespace init_convenience {
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  TestDiagnosticConsumer()
+  : Passthrough(llvm::make_unique()) {}
+
+  virtual void BeginSourceFile(const LangOptions ,
+   const Preprocessor *PP = nullptr) override {
+this->LangOpts = 
+return Passthrough->BeginSourceFile(LangOpts, PP);
+  }
+
+  virtual void EndSourceFile() override {
+this->LangOpts = nullptr;
+Passthrough->EndSourceFile();
+  }
+
+  virtual bool IncludeInDiagnosticCounts() const override {
+return Passthrough->IncludeInDiagnosticCounts();
+  }
+
+private:
+  static void PrintSourceForLocation(const SourceLocation ,
+ SourceManager ) {
+const char *LocData = SM.getCharacterData(Loc, /*Invalid=*/nullptr);
+unsigned LocColumn =
+SM.getSpellingColumnNumber(Loc, /*Invalid=*/nullptr) - 1;
+FileID FID = SM.getFileID(Loc);
+llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, Loc, /*Invalid=*/nullptr);
+
+assert(LocData >= Buffer->getBufferStart() &&
+   LocData < Buffer->getBufferEnd());
+
+const char *LineBegin = LocData - LocColumn;
+
+assert(LineBegin >= Buffer->getBufferStart());
+
+const char *LineEnd = nullptr;
+
+for (LineEnd = LineBegin; *LineEnd != '\n' && *LineEnd != '\r' &&
+  LineEnd < Buffer->getBufferEnd();
+ ++LineEnd)
+  ;
+
+llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
+
+llvm::errs() << LineString << '\n';
+std::string Space(LocColumn, ' ');
+llvm::errs() << Space.c_str() << '\n';
+  }
+
+  virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const Diagnostic ) override {
+if (Info.hasSourceManager() && LangOpts) {
+  SourceManager  = Info.getSourceManager();
+
+  if (Info.getLocation().isValid()) {
+Info.getLocation().print(llvm::errs(), SM);
+llvm::errs() << ": ";
+  }
+
+  SmallString<16> DiagText;
+  Info.FormatDiagnostic(DiagText);
+  llvm::errs() << DiagText << '\n';
+
+  if (Info.getLocation().isValid()) {
+PrintSourceForLocation(Info.getLocation(), SM);
+  }
+
+  for (const CharSourceRange  : Info.getRanges()) {
+bool Invalid = true;
+StringRef 

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 81661.
spyffe marked 2 inline comments as done.
spyffe added a comment.
Herald added a subscriber: jgosnell.

Applied Vassil and Vedant's comments.  I will commit this soon.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/Import/clang-flags/Inputs/S.c
  test/Import/clang-flags/test.c
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  test/Import/error-in-expression/Inputs/S.c
  test/Import/error-in-expression/test.c
  test/Import/error-in-import/Inputs/S.c
  test/Import/error-in-import/test.c
  test/Import/missing-import/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,319 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+namespace init_convenience {
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  TestDiagnosticConsumer()
+  : Passthrough(llvm::make_unique()) {}
+
+  virtual void BeginSourceFile(const LangOptions ,
+   const Preprocessor *PP = nullptr) override {
+this->LangOpts = 
+return Passthrough->BeginSourceFile(LangOpts, PP);
+  }
+
+  virtual void EndSourceFile() override {
+this->LangOpts = nullptr;
+Passthrough->EndSourceFile();
+  }
+
+  virtual bool IncludeInDiagnosticCounts() const override {
+return Passthrough->IncludeInDiagnosticCounts();
+  }
+
+private:
+  static void PrintSourceForLocation(const SourceLocation ,
+ SourceManager ) {
+const char *LocData = SM.getCharacterData(Loc, /*Invalid=*/nullptr);
+unsigned LocColumn =
+SM.getSpellingColumnNumber(Loc, /*Invalid=*/nullptr) - 1;
+FileID FID = SM.getFileID(Loc);
+llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, Loc, /*Invalid=*/nullptr);
+
+assert(LocData >= Buffer->getBufferStart() &&
+   LocData < Buffer->getBufferEnd());
+
+const char *LineBegin = LocData - LocColumn;
+
+assert(LineBegin >= Buffer->getBufferStart());
+
+const char *LineEnd = nullptr;
+
+for (LineEnd = LineBegin; *LineEnd != '\n' && *LineEnd != '\r' &&
+  LineEnd < Buffer->getBufferEnd();
+ ++LineEnd)
+  ;
+
+llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
+
+llvm::errs() << LineString << '\n';
+std::string Space(LocColumn, ' ');
+llvm::errs() << Space.c_str() << '\n';
+  }
+
+  virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const Diagnostic ) override {
+if (Info.hasSourceManager() && LangOpts) {
+  SourceManager  = Info.getSourceManager();
+
+  if (Info.getLocation().isValid()) {
+Info.getLocation().print(llvm::errs(), SM);
+llvm::errs() << ": ";
+  }
+
+  SmallString<16> DiagText;
+  Info.FormatDiagnostic(DiagText);
+  llvm::errs() << DiagText << '\n';
+
+  if (Info.getLocation().isValid()) {
+PrintSourceForLocation(Info.getLocation(), SM);
+  }
+
+  for (const CharSourceRange  : Info.getRanges()) {
+bool Invalid = true;
+StringRef Ref = 

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

Could you move the Build* functions in a utility class/namespace. I need 
something very similar for my ongoing work on the libInterpreter patch and I'd 
like to reuse that part of the patch.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

Thanks for working on this Sean!

Since long time we are planning to propose a patch moving some parts of cling 
(eg. libInterpreter) mainline.

Now I should have a little more time to do this and it'd be great if we can 
share some code between cling and lldb. This testing infrastructure is an ideal 
candidate for this.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Looks good. Apart from some nitpicks, I'm happy with this. This isn't really my 
area so it'd be good to get an explicit lgtm from one of the reviewers.




Comment at: tools/clang-import-test/CMakeLists.txt:7
+  clang-import-test.cpp
+  )
+

@beanz recently went on a crusade to add dependencies to intrinsics_gen to a 
bunch of stuff. Chances are, this tool probably depends on intrinsics_gen, so 
I'd look into that and add the dep if needed.



Comment at: tools/clang-import-test/clang-import-test.cpp:306
+  std::transform(ImportCIs.begin(), ImportCIs.end(), UnownedCIs.begin(),
+ std::mem_fn(::unique_ptr::get));
+  llvm::Expected ExpressionCI =

This transform smells a little strange. I'd rather see 
`ArrayRef>` used everywhere, through a typedef/using-decl 
if necessary.



Comment at: tools/clang-import-test/clang-import-test.cpp:310
+  if (auto E = ExpressionCI.takeError()) {
+llvm::errs() << (llvm::toString(std::move(E)));
+exit(-1);

There are redundant parens around your calls to toString(): I think these 
should be dropped.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 79807.
spyffe marked an inline comment as done.
spyffe added a comment.

Updated to reflect Vedant's comments.  
Also ensured 100% test coverage, by removing handling for errors that will 
never happen and by adding a few new tests.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/Import/clang-flags/Inputs/S.c
  test/Import/clang-flags/test.c
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  test/Import/error-in-expression/Inputs/S.c
  test/Import/error-in-expression/test.c
  test/Import/error-in-import/Inputs/S.c
  test/Import/error-in-import/test.c
  test/Import/missing-import/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,315 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+namespace {
+
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  TestDiagnosticConsumer()
+  : Passthrough(llvm::make_unique()) {}
+
+  virtual void BeginSourceFile(const LangOptions ,
+   const Preprocessor *PP = nullptr) override {
+this->LangOpts = 
+return Passthrough->BeginSourceFile(LangOpts, PP);
+  }
+
+  virtual void EndSourceFile() override {
+this->LangOpts = nullptr;
+Passthrough->EndSourceFile();
+  }
+
+  virtual bool IncludeInDiagnosticCounts() const override {
+return Passthrough->IncludeInDiagnosticCounts();
+  }
+
+private:
+  static void PrintSourceForLocation(const SourceLocation ,
+ SourceManager ) {
+const char *LocData = SM.getCharacterData(Loc, /*Invalid=*/nullptr);
+unsigned LocColumn = SM.getSpellingColumnNumber(Loc, /*Invalid=*/nullptr) - 1;
+FileID FID = SM.getFileID(Loc);
+llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, Loc, /*Invalid=*/nullptr);
+
+assert(LocData >= Buffer->getBufferStart() &&
+   LocData < Buffer->getBufferEnd());
+
+const char *LineBegin = LocData - LocColumn;
+
+assert(LineBegin >= Buffer->getBufferStart());
+
+const char *LineEnd = nullptr;
+
+for (LineEnd = LineBegin; *LineEnd != '\n' && *LineEnd != '\r' &&
+  LineEnd < Buffer->getBufferEnd();
+ ++LineEnd)
+  ;
+
+llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
+
+llvm::errs() << LineString << '\n';
+std::string Space(LocColumn, ' ');
+llvm::errs() << Space.c_str() << '\n';
+  }
+
+  virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const Diagnostic ) override {
+if (Info.hasSourceManager() && LangOpts) {
+  SourceManager  = Info.getSourceManager();
+
+  if (Info.getLocation().isValid()) {
+Info.getLocation().print(llvm::errs(), SM);
+llvm::errs() << ": ";
+  }
+
+  SmallString<16> DiagText;
+  Info.FormatDiagnostic(DiagText);
+  llvm::errs() << DiagText << '\n';
+
+  if (Info.getLocation().isValid()) {
+PrintSourceForLocation(Info.getLocation(), SM);
+  }
+
+  for (const CharSourceRange  : Info.getRanges()) {
+bool Invalid = true;

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe marked 3 inline comments as done.
spyffe added a comment.

Thank you for your review, Vedant!  I will update the patch to reflect your 
comments in a moment.

> I'm concerned about the amount of covered-but-untested code this patch 
> introduces. Since there are no CHECK lines, it's hard for me to verify that 
> this tool is doing the right thing.

What you're observing is that this is very much a skeleton.  What it can do 
right now is let the parser know that a `struct S` exists, but not what its 
contents are.  That's why the test is so simple, too.  As soon as the lexical 
lookup machinery exists, we'll be able to add tests accessing fields etc. and 
make sure everything is copacetic.

> Along with this change, I suggest stripping out a fair amount of code for the 
> initial commit (probably PrintSourceForLocation, and maybe anything related 
> to LogLookups).

That's fair.  As a low-level testing tool I'd like to make sure that I have a 
logging mechanism later on that allows tests to verify that the compiler made 
the right queries during a parse; that said, I can add these functions back in 
when tests require them.




Comment at: tools/clang-import-test/clang-import-test.cpp:302
+bool Parse(const std::string , std::unique_ptr ,
+   llvm::ArrayRef Imports) {
+  CI = BuildCompilerInstance();

vsk wrote:
> I suggest making this `Expected Parse(..., 
> ArrayRef)`. This way, there's no way to 
> mistake CI for an input param, there's no need for an extra step to convert 
> std::unique_ptr to CompilerInstance *, and it's harder to 
> drop an error from Parse without logging/handling it.
Ah yes, this is a pattern I hadn't internalized yet.  Thanks for the reminder.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

Thanks for working on this! I'm happy with the direction of this patch. It 
makes sense that clang would have a tool to help test AST reconstruction from 
'external' sources. As you pointed out, it provides a good avenue for clients 
of the clang AST's to get better test coverage for their custom serialization 
formats. I haven't been paying much attention to the discussion about the 
clangDebuggerSupport library, but it sounds like your work will ultimately 
depend on it. Is that correct?

I've left some lower-level comments inline.

At a higher level, I'm concerned about the amount of covered-but-untested code 
this patch introduces. Since there are no CHECK lines, it's hard for me to 
verify that this tool is doing the right thing. What we really want to test 
right away is that the tool can "dump" the correct definition for `struct S` 
(since this implies that the importing went OK). A good way to go about this 
would be to add a hidden "debug" cl::opt, dump all imported struct decls when 
in -debug mode, and then add some CHECK lines for the expected struct decl.

Along with this change, I suggest stripping out a fair amount of code for the 
initial commit (probably PrintSourceForLocation, and maybe anything related to 
LogLookups).




Comment at: tools/clang-import-test/clang-import-test.cpp:238
+
+  CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.data(),
+ ()[ClangArgv.size()],

spyffe wrote:
> a.sidorin wrote:
> > `ClangArgv.begin(), ClangArgv.end()`
> ```
> .../llvm/tools/clang/tools/clang-import-test/clang-import-test.cpp:236:44: 
> error: no viable conversion from 'iterator' (aka '__wrap_iter **>') to 'const char *const *'
>   CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.begin(), ClangArgv.end(),
>^
> .../llvm/tools/clang/include/clang/Frontend/CompilerInvocation.h:139:49: 
> note: passing argument to parameter 'ArgBegin' here
>  const char* const *ArgBegin,
> ^
> ```
This should resolve it, but it seems less readable: `&*args.cbegin(), 
&*args.cend()`. I'd leave this as-is..



Comment at: tools/clang-import-test/clang-import-test.cpp:164
+  ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
+  ImportCI->getASTContext(), ImportCI->getFileManager(), 
MinimalImport);
+  ReverseImporters[ImportCI] = llvm::make_unique(

I think it's more idiomatic to say `/*MinimalImport=*/true` in clang.



Comment at: tools/clang-import-test/clang-import-test.cpp:282
+llvm::LLVMContext ) {
+  std::string ModuleName("$__module");
+  return std::unique_ptr(CreateLLVMCodeGen(

This might as well be constructed as a StringRef from the get-go, since it's 
eventually converted into one.



Comment at: tools/clang-import-test/clang-import-test.cpp:302
+bool Parse(const std::string , std::unique_ptr ,
+   llvm::ArrayRef Imports) {
+  CI = BuildCompilerInstance();

I suggest making this `Expected Parse(..., 
ArrayRef)`. This way, there's no way to 
mistake CI for an input param, there's no need for an extra step to convert 
std::unique_ptr to CompilerInstance *, and it's harder to 
drop an error from Parse without logging/handling it.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 79605.
spyffe marked 2 inline comments as done.
spyffe added a comment.

Updated to reflect Aleksei's comments.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,342 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("-Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+static llvm::cl::opt LogLookups(
+"log-lookups",
+llvm::cl::desc("Print each lookup performed on behalf of the expression"));
+
+namespace {
+
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+  TestDiagnosticConsumer()
+  : Passthrough(llvm::make_unique()) {}
+
+  virtual void BeginSourceFile(const LangOptions ,
+   const Preprocessor *PP = nullptr) override {
+this->LangOpts = 
+return Passthrough->BeginSourceFile(LangOpts, PP);
+  }
+
+  virtual void EndSourceFile() override {
+this->LangOpts = nullptr;
+Passthrough->EndSourceFile();
+  }
+
+  virtual void finish() override { Passthrough->finish(); }
+
+  virtual bool IncludeInDiagnosticCounts() const override {
+return Passthrough->IncludeInDiagnosticCounts();
+  }
+
+private:
+  static void PrintSourceForLocation(const SourceLocation ,
+ SourceManager ) {
+bool Invalid = true;
+const char *LocData = SM.getCharacterData(Loc, );
+if (Invalid) {
+  return;
+}
+unsigned LocColumn = SM.getSpellingColumnNumber(Loc, ) - 1;
+if (Invalid) {
+  return;
+}
+FileID FID = SM.getFileID(Loc);
+llvm::MemoryBuffer *Buffer = SM.getBuffer(FID, Loc, );
+if (Invalid) {
+  return;
+}
+
+assert(LocData >= Buffer->getBufferStart() &&
+   LocData < Buffer->getBufferEnd());
+
+const char *LineBegin = LocData - LocColumn;
+
+if (LineBegin < Buffer->getBufferStart()) {
+  LineBegin = Buffer->getBufferStart();
+}
+
+const char *LineEnd = nullptr;
+
+for (LineEnd = LineBegin; *LineEnd != '\n' && *LineEnd != '\r' &&
+  LineEnd < Buffer->getBufferEnd();
+ ++LineEnd)
+  ;
+
+llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
+
+llvm::errs() << LineString << '\n';
+std::string Space(LocColumn, ' ');
+llvm::errs() << Space.c_str() << '\n';
+  }
+
+  virtual void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
+const Diagnostic ) override {
+if (Info.hasSourceManager() && LangOpts) {
+  SourceManager  = Info.getSourceManager();
+
+  if (Info.getLocation().isValid()) {
+Info.getLocation().print(llvm::errs(), SM);
+llvm::errs() << ": ";
+  }
+
+  SmallString<16> DiagText;
+  Info.FormatDiagnostic(DiagText);
+  llvm::errs() << DiagText << '\n';
+
+  if (Info.getLocation().isValid()) {
+PrintSourceForLocation(Info.getLocation(), SM);
+  }
+
+  for (const CharSourceRange  : Info.getRanges()) {
+bool Invalid = true;
+StringRef Ref = Lexer::getSourceText(Range, SM, *LangOpts, );
+

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe marked 11 inline comments as done.
spyffe added a comment.

Thank you, Alex!  I've responded in a few places inline below, and will update 
this patch momentarily.




Comment at: tools/clang-import-test/clang-import-test.cpp:106
+
+const char *LineEnd = nullptr;
+

a.sidorin wrote:
> How about something like this:
> ```
> StringRef Remain(LineBegin, Buffer->getBufferEnd() - LineBegin);
> size_t EndPos = Remain.find_first_of("\r\n");
> StringRef Line = (EndPos == StringRef::npos) ? Remain : StringRef(LineBegin, 
> EndPos);
> llvm::errs() << Line << "\n";
> llvm::errs().indent(LocColumn) << "^\n";
> ```
> ?
I'm going to think about this a little more.  Personally I find the loop more 
readable.
That said StringRef instead of std::string seems an obvious win:
```
llvm::StringRef LineString(LineBegin, LineEnd - LineBegin);
``` 



Comment at: tools/clang-import-test/clang-import-test.cpp:152
+class TestExternalASTSource : public ExternalASTSource {
+private:  llvm::ArrayRef ImportCIs;
+  std::map> ForwardImporters;

a.sidorin wrote:
> Please add a newline here.
I just ran clang-format on the whole file.  Sorry for the noise.



Comment at: tools/clang-import-test/clang-import-test.cpp:222
+   SmallVectorImpl ) override {
+
+  }

a.sidorin wrote:
> Extra spaces.
Sigh.  Yes, I had a preliminary implementation of this in mind, but decided to 
split that out into a separate patch.



Comment at: tools/clang-import-test/clang-import-test.cpp:238
+
+  CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.data(),
+ ()[ClangArgv.size()],

a.sidorin wrote:
> `ClangArgv.begin(), ClangArgv.end()`
```
.../llvm/tools/clang/tools/clang-import-test/clang-import-test.cpp:236:44: 
error: no viable conversion from 'iterator' (aka '__wrap_iter') 
to 'const char *const *'
  CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.begin(), ClangArgv.end(),
   ^
.../llvm/tools/clang/include/clang/Frontend/CompilerInvocation.h:139:49: note: 
passing argument to parameter 'ArgBegin' here
 const char* const *ArgBegin,
^
```


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

In https://reviews.llvm.org/D27180#607433, @hfinkel wrote:

> This seems like a great idea. btw, do you know about Cling 
> (https://root.cern.ch/cling)?


Hal, thank you for your interest!  Yes, Cling is what I was referring to when I 
mentioned the ROOT project.  Vassil Vassilev, the developer of Cling, is also 
on the reviewer list.  I had the pleasure of talking to Vassil a few years ago 
at a LLVM Developer Meeting and the idea of creating common infrastructure 
between LLDB and Cling has been rolling around in the back of my head since 
then.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

That's excellent. Thank you for this work, Sean!




Comment at: tools/clang-import-test/CMakeLists.txt:19
+  )
\ No newline at end of file


Please add a newline here.



Comment at: tools/clang-import-test/clang-import-test.cpp:106
+
+const char *LineEnd = nullptr;
+

How about something like this:
```
StringRef Remain(LineBegin, Buffer->getBufferEnd() - LineBegin);
size_t EndPos = Remain.find_first_of("\r\n");
StringRef Line = (EndPos == StringRef::npos) ? Remain : StringRef(LineBegin, 
EndPos);
llvm::errs() << Line << "\n";
llvm::errs().indent(LocColumn) << "^\n";
```
?



Comment at: tools/clang-import-test/clang-import-test.cpp:115
+
+fprintf(stderr, "%s\n", LineString.c_str());
+std::string Space(LocColumn, ' ');

Why do we use `fprintf` instead of `llvm::errs()`?



Comment at: tools/clang-import-test/clang-import-test.cpp:130
+
+  SmallVector DiagText;
+  Info.FormatDiagnostic(DiagText);

SmallString? So, you will not need to push_back.



Comment at: tools/clang-import-test/clang-import-test.cpp:143
+if (!Invalid) {
+  llvm::errs() << Ref.str().c_str() << '\n';
+}

No need in `c_str()` here.



Comment at: tools/clang-import-test/clang-import-test.cpp:152
+class TestExternalASTSource : public ExternalASTSource {
+private:  llvm::ArrayRef ImportCIs;
+  std::map> ForwardImporters;

Please add a newline here.



Comment at: tools/clang-import-test/clang-import-test.cpp:162
+  const bool MinimalImport = true;
+  ForwardImporters.emplace(std::make_pair(
+  ImportCI,

`ForwardImporters[ImportCI] = ...llvm::make_unique<>`?



Comment at: tools/clang-import-test/clang-import-test.cpp:179
+  DeclarationName Name) override {
+std::vector Decls;
+

SmallVector?



Comment at: tools/clang-import-test/clang-import-test.cpp:181
+
+if (llvm::isa(DC)) {
+  for (CompilerInstance *I : ImportCIs) {

In LLVM code, qualifiers for llvm-style casts are not commonly used.



Comment at: tools/clang-import-test/clang-import-test.cpp:222
+   SmallVectorImpl ) override {
+
+  }

Extra spaces.



Comment at: tools/clang-import-test/clang-import-test.cpp:236
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](std::string ) -> const char * { return s.data(); });
+

`[](const std::string )`



Comment at: tools/clang-import-test/clang-import-test.cpp:238
+
+  CompilerInvocation::CreateFromArgs(*Inv, ClangArgv.data(),
+ ()[ClangArgv.size()],

`ClangArgv.begin(), ClangArgv.end()`



Comment at: tools/clang-import-test/clang-import-test.cpp:327
+}
+}
+

// end namespace


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added a comment.

This seems like a great idea. btw, do you know about Cling 
(https://root.cern.ch/cling)?


Repository:
  rL LLVM

https://reviews.llvm.org/D27180



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

[PATCH] D27180: Testbed and skeleton of a new expression parser

[Sean Callanan via Phabricator via cfe-commits]

spyffe created this revision.
spyffe added reviewers: a.sidorin, beanz, loladiro, v.g.vassilev.
spyffe added a subscriber: cfe-commits.
spyffe set the repository for this revision to rL LLVM.
Herald added a subscriber: mgorny.

LLVM's JIT is now the foundation of dynamic-compilation features for many 
languages.  Clang also has low-level support for dynamic compilation 
(`ASTImporter` and `ExternalASTSource`, notably).  How the compiler is set up 
for dynamic parsing is generally left up to individual clients, for example 
LLDB's C/C++/Objective-C expression parser and the ROOT project.

Although this arrangement offers external clients the flexibility to implement 
dynamic features as they see fit, the lack of an in-tree client means that 
subtle bugs can be introduced that cause regressions in the external clients 
but aren't caught by tests (or users) until much later.  LLDB for example 
regularly encounters complicated ODR violation scenarios where it is not 
immediately clear who is at fault.

I propose a simple expression parser be added to Clang.  I aim to have it 
encompass two main features:

- It should be able to look up external declarations from a variety of sources 
(e.g., from previous dynamic compilations, from modules, or from DWARF) and 
have clear conflict resolution rules with easily understood errors.  This 
functionality will be supported by in-tree tests.
- It should work hand in hand with the LLVM JIT to resolve the locations of 
external declarations so that e.g. variables can be redeclared and (for 
high-performance applications like DTrace) external variables can be accessed 
directly from the registers where they reside.

I have attached a tester that parses a sequence of source files and then uses 
them as source data for an expression.  External references are resolved using 
an `ExternalASTSource` that responds to name queries using an `ASTImporter`.  
This is the setup that LLDB uses, and the motivating reason for `MinimalImport` 
in `ASTImporter`.

Over time my intention is to make this more complete and support the many 
scenarios that LLDB can support.  I also want to identify places where LLDB 
does the wrong thing and we can make this functionality more correct, hopefully 
eliminating frustrating and opaque errors.  I also want to spot where Clang 
might get things wrong, and be able to point Clang developers to an 
easy-to-reproduce, in-tree test that doesn't require external projects or 
patches.  Finally, I want to identify how we can make this functionality as 
generic as possible for the current clients besides LLDB, and for potential 
future clients.


Repository:
  rL LLVM

https://reviews.llvm.org/D27180

Files:
  test/Import/empty-struct/Inputs/S.c
  test/Import/empty-struct/test.c
  tools/CMakeLists.txt
  tools/clang-import-test/CMakeLists.txt
  tools/clang-import-test/clang-import-test.cpp

Index: tools/clang-import-test/clang-import-test.cpp
===
--- tools/clang-import-test/clang-import-test.cpp
+++ tools/clang-import-test/clang-import-test.cpp
@@ -0,0 +1,347 @@
+//===-- import-test.cpp - ASTImporter/ExternalASTSource testbed ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "clang/Basic/Builtins.h"
+#include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/TargetInfo.h"
+#include "clang/Basic/TargetOptions.h"
+#include "clang/CodeGen/ModuleBuilder.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Parse/ParseAST.h"
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Host.h"
+#include "llvm/Support/Signals.h"
+
+#include 
+#include 
+
+using namespace clang;
+
+static llvm::cl::opt Expression(
+"expression", llvm::cl::Required,
+llvm::cl::desc("Path to a file containing the expression to parse"));
+
+static llvm::cl::list
+Imports("import", llvm::cl::ZeroOrMore,
+llvm::cl::desc("Path to a file containing declarations to import"));
+
+static llvm::cl::list
+ClangArgs("-Xcc", llvm::cl::ZeroOrMore,
+  llvm::cl::desc("Argument to pass to the CompilerInvocation"),
+  llvm::cl::CommaSeparated);
+
+static llvm::cl::opt LogLookups(
+"log-lookups",
+llvm::cl::desc("Print each lookup performed on behalf of the expression"));
+
+namespace {
+
+class TestDiagnosticConsumer : public DiagnosticConsumer {
+private:
+  std::unique_ptr Passthrough;
+  const LangOptions *LangOpts = nullptr;
+
+public:
+