[PATCH] D38208: Add support for remembering origins to ExternalASTMerger

[Sean Callanan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL314336: Add support for remembering origins to 
ExternalASTMerger (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D38208?vs=116680=116868#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38208

Files:
  cfe/trunk/include/clang/AST/ExternalASTMerger.h
  cfe/trunk/lib/AST/ExternalASTMerger.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/test/Import/extern-c-function/Inputs/F.cpp
  cfe/trunk/test/Import/extern-c-function/test.cpp
  cfe/trunk/test/Import/forward-declared-objc-class/Inputs/S1.m
  cfe/trunk/test/Import/forward-declared-objc-class/Inputs/S2.m
  cfe/trunk/test/Import/forward-declared-objc-class/Inputs/S3.m
  cfe/trunk/test/Import/forward-declared-objc-class/test.m
  cfe/trunk/test/Import/forward-declared-struct/Inputs/S3.c
  cfe/trunk/test/Import/forward-declared-struct/test.c
  cfe/trunk/test/Import/local-struct-use-origins/Inputs/Callee.cpp
  cfe/trunk/test/Import/local-struct-use-origins/test.cpp
  cfe/trunk/test/Import/local-struct/test.cpp
  cfe/trunk/test/Import/objc-definitions-in-expression/Inputs/S.m
  cfe/trunk/test/Import/objc-definitions-in-expression/test.m
  cfe/trunk/test/Import/struct-and-var/Inputs/S1.cpp
  cfe/trunk/test/Import/struct-and-var/Inputs/S2.cpp
  cfe/trunk/test/Import/struct-and-var/test.cpp
  cfe/trunk/test/Import/template/Inputs/T.cpp
  cfe/trunk/test/Import/template/test.cpp
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Index: cfe/trunk/lib/AST/ExternalASTMerger.cpp
===
--- cfe/trunk/lib/AST/ExternalASTMerger.cpp
+++ cfe/trunk/lib/AST/ExternalASTMerger.cpp
@@ -14,6 +14,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/AST/ExternalASTMerger.h"
 
@@ -32,29 +33,18 @@
 
 typedef std::pair Candidate;
 
-class LazyASTImporter : public ASTImporter {
-public:
-  LazyASTImporter(ASTContext , FileManager ,
-  ASTContext , FileManager )
-  : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
-/*MinimalImport=*/true) {}
-  Decl *Imported(Decl *From, Decl *To) override {
-if (auto ToTag = dyn_cast(To)) {
-  ToTag->setHasExternalLexicalStorage();
-  ToTag->setMustBuildLookupTable();
-} else if (auto ToNamespace = dyn_cast(To)) {
-  ToNamespace->setHasExternalVisibleStorage();
-} else if (auto ToContainer = dyn_cast(To)) {
-  ToContainer->setHasExternalLexicalStorage();
-  ToContainer->setMustBuildLookupTable();
-}
-return ASTImporter::Imported(From, To);
-  }
-};
+/// For the given DC, return the DC that is safe to perform lookups on.  This is
+/// the DC we actually want to work with most of the time.
+const DeclContext *CanonicalizeDC(const DeclContext *DC) {
+  if (isa(DC))
+return DC->getRedeclContext();
+  return DC;
+}
 
 Source
 LookupSameContext(Source SourceTU, const DeclContext *DC,
   ASTImporter ) {
+  DC = CanonicalizeDC(DC);
   if (DC->isTranslationUnit()) {
 return SourceTU;
   }
@@ -64,101 +54,328 @@
 // If we couldn't find the parent DC in this TranslationUnit, give up.
 return nullptr;
   }
-  auto ND = cast(DC);
+  auto *ND = cast(DC);
   DeclarationName Name = ND->getDeclName();
   Source SourceName = ReverseImporter.Import(Name);
   DeclContext::lookup_result SearchResult =
   SourceParentDC.get()->lookup(SourceName.get());
   size_t SearchResultSize = SearchResult.size();
-  // Handle multiple candidates once we have a test for it.
-  // This may turn up when we import template specializations correctly.
-  assert(SearchResultSize < 2);
-  if (SearchResultSize == 0) {
-// couldn't find the name, so we have to give up
+  if (SearchResultSize == 0 || SearchResultSize > 1) {
+// There are two cases here.  First, we might not find the name.
+// We might also find multiple copies, in which case we have no
+// guarantee that the one we wanted is the one we pick.  (E.g.,
+// if we have two specializations of the same template it is
+// very hard to determine which is the one you want.)
+//
+// The Origins map fixes this problem by allowing the origin to be
+// explicitly recorded, so we trigger that recording by returning
+// nothing (rather than a possibly-inaccurate guess) here.
 return nullptr;
   } else {
 NamedDecl *SearchResultDecl = SearchResult[0];
-return dyn_cast(SearchResultDecl);
+if (isa(SearchResultDecl) &&
+SearchResultDecl->getKind() == DC->getDeclKind())
+  return cast(SearchResultDecl)->getPrimaryContext();
+return nullptr; // This type of lookup is unsupported
   }
 }
 
-bool IsForwardDeclaration(Decl *D) {
-  if (auto TD = dyn_cast(D)) {
-return !TD->isThisDeclarationADefinition();
-  } else if (auto FD = 

[PATCH] D38208: Add support for remembering origins to ExternalASTMerger

[Sean Callanan via Phabricator via cfe-commits]

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

Updates and small refactors requested by Bruno.


Repository:
  rL LLVM

https://reviews.llvm.org/D38208

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/ExternalASTMerger.cpp
  lib/Sema/SemaType.cpp
  test/Import/extern-c-function/Inputs/F.cpp
  test/Import/extern-c-function/test.cpp
  test/Import/forward-declared-objc-class/Inputs/S1.m
  test/Import/forward-declared-objc-class/Inputs/S2.m
  test/Import/forward-declared-objc-class/Inputs/S3.m
  test/Import/forward-declared-objc-class/test.m
  test/Import/forward-declared-struct/Inputs/S3.c
  test/Import/forward-declared-struct/test.c
  test/Import/local-struct-use-origins/Inputs/Callee.cpp
  test/Import/local-struct-use-origins/test.cpp
  test/Import/local-struct/test.cpp
  test/Import/objc-definitions-in-expression/Inputs/S.m
  test/Import/objc-definitions-in-expression/test.m
  test/Import/struct-and-var/Inputs/S1.cpp
  test/Import/struct-and-var/Inputs/S2.cpp
  test/Import/struct-and-var/test.cpp
  test/Import/template/Inputs/T.cpp
  test/Import/template/test.cpp
  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
@@ -48,8 +48,12 @@
 
 static llvm::cl::opt
 Direct("direct", llvm::cl::Optional,
- llvm::cl::desc("Use the parsed declarations without indirection"));
+   llvm::cl::desc("Use the parsed declarations without indirection"));
 
+static llvm::cl::opt
+UseOrigins("use-origins", llvm::cl::Optional,
+   llvm::cl::desc("Use DeclContext origin information for more accurate lookups"));  
+
 static llvm::cl::list
 ClangArgs("Xcc", llvm::cl::ZeroOrMore,
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -60,13 +64,11 @@
   llvm::cl::desc("The language to parse (default: c++)"),
   llvm::cl::init("c++"));
 
-static llvm::cl::opt
-DumpAST("dump-ast", llvm::cl::init(false),
-llvm::cl::desc("Dump combined AST"));
+static llvm::cl::opt DumpAST("dump-ast", llvm::cl::init(false),
+   llvm::cl::desc("Dump combined AST"));
 
-static llvm::cl::opt
-DumpIR("dump-ir", llvm::cl::init(false),
-llvm::cl::desc("Dump IR from final parse"));
+static llvm::cl::opt DumpIR("dump-ir", llvm::cl::init(false),
+  llvm::cl::desc("Dump IR from final parse"));
 
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
@@ -154,8 +156,7 @@
   }
 };
 
-std::unique_ptr
-BuildCompilerInstance() {
+std::unique_ptr BuildCompilerInstance() {
   auto Ins = llvm::make_unique();
   auto DC = llvm::make_unique();
   const bool ShouldOwnClient = true;
@@ -227,29 +228,54 @@
 } // end namespace
 
 namespace {
- 
-void AddExternalSource(
-CompilerInstance ,
-llvm::ArrayRef Imports) {
-  ExternalASTMerger::ImporterEndpoint Target({CI.getASTContext(), CI.getFileManager()});
-  llvm::SmallVector Sources;
-  for (const std::unique_ptr  : Imports) {
-Sources.push_back({CI->getASTContext(), CI->getFileManager()});
+
+/// A container for a CompilerInstance (possibly with an ExternalASTMerger
+/// attached to its ASTContext).
+///
+/// Provides an accessor for the DeclContext origins associated with the
+/// ExternalASTMerger (or an empty list of origins if no ExternalASTMerger is
+/// attached).
+///
+/// This is the main unit of parsed source code maintained by clang-import-test.
+struct CIAndOrigins {
+  using OriginMap = clang::ExternalASTMerger::OriginMap;
+  std::unique_ptr CI;
+
+  ASTContext () { return CI->getASTContext(); }
+  FileManager () { return CI->getFileManager(); }
+  const OriginMap () {
+static const OriginMap EmptyOriginMap;
+if (ExternalASTSource *Source = CI->getASTContext().getExternalSource())
+  return static_cast(Source)->GetOrigins();
+return EmptyOriginMap;
   }
+  DiagnosticConsumer () {
+return CI->getDiagnosticClient();
+  }
+  CompilerInstance () { return *CI; }
+};
+
+void AddExternalSource(CIAndOrigins ,
+   llvm::MutableArrayRef Imports) {
+  ExternalASTMerger::ImporterTarget Target(
+  {CI.getASTContext(), CI.getFileManager()});
+  llvm::SmallVector Sources;
+  for (CIAndOrigins  : Imports)
+Sources.push_back(
+{Import.getASTContext(), Import.getFileManager(), Import.getOriginMap()});
   auto ES = llvm::make_unique(Target, Sources);
   CI.getASTContext().setExternalSource(ES.release());
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
-std::unique_ptr BuildIndirect(std::unique_ptr ) {
-  std::unique_ptr IndirectCI =
-  init_convenience::BuildCompilerInstance();
+CIAndOrigins 

[PATCH] D38208: Add support for remembering origins to ExternalASTMerger

[Sean Callanan via Phabricator via cfe-commits]

spyffe created this revision.
spyffe added a project: clang.

At @bruno 's request, recreating this with cfe-commits as a subscriber.  The 
original is https://reviews.llvm.org/D36589

`ExternalASTMerger` has hitherto relied on being able to look up any Decl 
through its named `DeclContext` chain.  This works for many cases, but causes 
problems for function-local structs, which cannot be looked up in their 
containing `FunctionDecl`.  An example case is

  void f() {
{ struct S { int a; }; }
{ struct S { bool b; }; }
  }

It is not possible to lookup either of the two `S`es individually (or even to 
provide enough information to disambiguate) after parsing is over; and there is 
typically no need to, since they are invisible to the outside world.

However, ExternalASTMerger needs to be able to complete either S on demand.  
This led to an `XFAIL` on test/Import/local-struct, which this patch removes.  
The way the patch works is:

- It defines a new data structure, `ExternalASTMerger::OriginMap`, which 
clients are expected to maintain (default-constructing if the origin does not 
have an `ExternalASTMerger` servicing it)
- As `DeclContext`s are imported, if they cannot be looked up by name they are 
placed in the `OriginMap`.  This allows `ExternalASTMerger` to complete them 
later if necessary.
- As `DeclContext`s are imported from an origin that already has its own 
`OriginMap`, the origins are forwarded – but only for those `DeclContext`s that 
are actually used.  This keeps the amount of stored data minimal.


Repository:
  rL LLVM

https://reviews.llvm.org/D38208

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/ExternalASTMerger.cpp
  lib/Sema/SemaType.cpp
  test/Import/extern-c-function/Inputs/F.cpp
  test/Import/extern-c-function/test.cpp
  test/Import/forward-declared-objc-class/Inputs/S1.m
  test/Import/forward-declared-objc-class/Inputs/S2.m
  test/Import/forward-declared-objc-class/Inputs/S3.m
  test/Import/forward-declared-objc-class/test.m
  test/Import/forward-declared-struct/Inputs/S3.c
  test/Import/forward-declared-struct/test.c
  test/Import/local-struct-use-origins/Inputs/Callee.cpp
  test/Import/local-struct-use-origins/test.cpp
  test/Import/local-struct/test.cpp
  test/Import/objc-definitions-in-expression/Inputs/S.m
  test/Import/objc-definitions-in-expression/test.m
  test/Import/struct-and-var/Inputs/S1.cpp
  test/Import/struct-and-var/Inputs/S2.cpp
  test/Import/struct-and-var/test.cpp
  test/Import/template/Inputs/T.cpp
  test/Import/template/test.cpp
  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
@@ -48,8 +48,12 @@
 
 static llvm::cl::opt
 Direct("direct", llvm::cl::Optional,
- llvm::cl::desc("Use the parsed declarations without indirection"));
+   llvm::cl::desc("Use the parsed declarations without indirection"));
 
+static llvm::cl::opt
+UseOrigins("use-origins", llvm::cl::Optional,
+   llvm::cl::desc("Use DeclContext origin information for more accurate lookups"));  
+
 static llvm::cl::list
 ClangArgs("Xcc", llvm::cl::ZeroOrMore,
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -60,13 +64,11 @@
   llvm::cl::desc("The language to parse (default: c++)"),
   llvm::cl::init("c++"));
 
-static llvm::cl::opt
-DumpAST("dump-ast", llvm::cl::init(false),
-llvm::cl::desc("Dump combined AST"));
+static llvm::cl::opt DumpAST("dump-ast", llvm::cl::init(false),
+   llvm::cl::desc("Dump combined AST"));
 
-static llvm::cl::opt
-DumpIR("dump-ir", llvm::cl::init(false),
-llvm::cl::desc("Dump IR from final parse"));
+static llvm::cl::opt DumpIR("dump-ir", llvm::cl::init(false),
+  llvm::cl::desc("Dump IR from final parse"));
 
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
@@ -154,8 +156,7 @@
   }
 };
 
-std::unique_ptr
-BuildCompilerInstance() {
+std::unique_ptr BuildCompilerInstance() {
   auto Ins = llvm::make_unique();
   auto DC = llvm::make_unique();
   const bool ShouldOwnClient = true;
@@ -227,29 +228,54 @@
 } // end namespace
 
 namespace {
- 
-void AddExternalSource(
-CompilerInstance ,
-llvm::ArrayRef Imports) {
-  ExternalASTMerger::ImporterEndpoint Target({CI.getASTContext(), CI.getFileManager()});
-  llvm::SmallVector Sources;
-  for (const std::unique_ptr  : Imports) {
-Sources.push_back({CI->getASTContext(), CI->getFileManager()});
+
+/// A container for a CompilerInstance (possibly with an ExternalASTMerger
+/// attached to its ASTContext).
+///
+/// Provides an accessor for the DeclContext origins associated with the
+/// ExternalASTMerger (or an empty list of 

[PATCH] D36589: Add support for remembering origins to ExternalASTMerger

[Sean Callanan via Phabricator via cfe-commits]

spyffe abandoned this revision.
spyffe marked 13 inline comments as done.
spyffe added a comment.

At @bruno 's request, abandoning this revision in favor of the updated 
https://reviews.llvm.org/D38208.
That revision has all the changes requested.


https://reviews.llvm.org/D36589



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

[PATCH] D36589: Add support for remembering origins to ExternalASTMerger

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 116405.
spyffe marked 18 inline comments as done.
spyffe added a subscriber: cfe-commits.
spyffe added a comment.

Updated to reflect Bruno's suggestions.

- Commented `ExternalASTMerger.h` extensively.
- Refactored the log into a pluggable `raw_ostream` that defaults to 
`llvm::nulls()`.
- Added comments per Bruno's requests
- Fixed some indentation
- Fixed some variable names and auto [*&] s
- Removed some commented-out code
- Cleaned up some curly braces


https://reviews.llvm.org/D36589

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/ExternalASTMerger.cpp
  lib/Sema/SemaType.cpp
  test/Import/extern-c-function/Inputs/F.cpp
  test/Import/extern-c-function/test.cpp
  test/Import/forward-declared-objc-class/Inputs/S1.m
  test/Import/forward-declared-objc-class/Inputs/S2.m
  test/Import/forward-declared-objc-class/Inputs/S3.m
  test/Import/forward-declared-objc-class/test.m
  test/Import/forward-declared-struct/Inputs/S3.c
  test/Import/forward-declared-struct/test.c
  test/Import/local-struct-use-origins/Inputs/Callee.cpp
  test/Import/local-struct-use-origins/test.cpp
  test/Import/local-struct/test.cpp
  test/Import/objc-definitions-in-expression/Inputs/S.m
  test/Import/objc-definitions-in-expression/test.m
  test/Import/struct-and-var/Inputs/S1.cpp
  test/Import/struct-and-var/Inputs/S2.cpp
  test/Import/struct-and-var/test.cpp
  test/Import/template/Inputs/T.cpp
  test/Import/template/test.cpp
  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
@@ -48,8 +48,12 @@
 
 static llvm::cl::opt
 Direct("direct", llvm::cl::Optional,
- llvm::cl::desc("Use the parsed declarations without indirection"));
+   llvm::cl::desc("Use the parsed declarations without indirection"));
 
+static llvm::cl::opt
+UseOrigins("use-origins", llvm::cl::Optional,
+   llvm::cl::desc("Use DeclContext origin information for more accurate lookups"));  
+
 static llvm::cl::list
 ClangArgs("Xcc", llvm::cl::ZeroOrMore,
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -60,13 +64,11 @@
   llvm::cl::desc("The language to parse (default: c++)"),
   llvm::cl::init("c++"));
 
-static llvm::cl::opt
-DumpAST("dump-ast", llvm::cl::init(false),
-llvm::cl::desc("Dump combined AST"));
+static llvm::cl::opt DumpAST("dump-ast", llvm::cl::init(false),
+   llvm::cl::desc("Dump combined AST"));
 
-static llvm::cl::opt
-DumpIR("dump-ir", llvm::cl::init(false),
-llvm::cl::desc("Dump IR from final parse"));
+static llvm::cl::opt DumpIR("dump-ir", llvm::cl::init(false),
+  llvm::cl::desc("Dump IR from final parse"));
 
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
@@ -154,8 +156,7 @@
   }
 };
 
-std::unique_ptr
-BuildCompilerInstance() {
+std::unique_ptr BuildCompilerInstance() {
   auto Ins = llvm::make_unique();
   auto DC = llvm::make_unique();
   const bool ShouldOwnClient = true;
@@ -227,29 +228,48 @@
 } // end namespace
 
 namespace {
- 
-void AddExternalSource(
-CompilerInstance ,
-llvm::ArrayRef Imports) {
-  ExternalASTMerger::ImporterEndpoint Target({CI.getASTContext(), CI.getFileManager()});
-  llvm::SmallVector Sources;
-  for (const std::unique_ptr  : Imports) {
-Sources.push_back({CI->getASTContext(), CI->getFileManager()});
+
+struct CIAndOrigins {
+  using OriginMap = clang::ExternalASTMerger::OriginMap;
+  std::unique_ptr CI;
+
+  ASTContext () { return CI->getASTContext(); }
+  FileManager () { return CI->getFileManager(); }
+  const OriginMap () {
+static const OriginMap EmptyOriginMap;
+if (ExternalASTSource *Source = CI->getASTContext().getExternalSource())
+  return static_cast(Source)->GetOrigins();
+else
+  return EmptyOriginMap;
   }
+  DiagnosticConsumer () {
+return CI->getDiagnosticClient();
+  }
+  CompilerInstance () { return *CI; }
+};
+
+void AddExternalSource(CIAndOrigins ,
+   llvm::MutableArrayRef Imports) {
+  ExternalASTMerger::ImporterTarget Target(
+  {CI.getASTContext(), CI.getFileManager()});
+  llvm::SmallVector Sources;
+  for (CIAndOrigins  : Imports) {
+Sources.push_back(
+{Import.getASTContext(), Import.getFileManager(), Import.getOriginMap()});
+  }
   auto ES = llvm::make_unique(Target, Sources);
   CI.getASTContext().setExternalSource(ES.release());
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
-std::unique_ptr BuildIndirect(std::unique_ptr ) {
-  std::unique_ptr IndirectCI =
-  init_convenience::BuildCompilerInstance();
+CIAndOrigins 

[PATCH] D36429: [clang-import-test] Option to dump the IR for an expression

[Sean Callanan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL310318: This adds the argument --dump-ir to 
clang-import-test, which allows  (authored by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D36429?vs=110095=110104#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D36429

Files:
  cfe/trunk/test/Import/local-struct/Inputs/Callee.cpp
  cfe/trunk/test/Import/local-struct/test.cpp
  cfe/trunk/test/Import/struct-layout/Inputs/Callee.cpp
  cfe/trunk/test/Import/struct-layout/test.cpp
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp

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
@@ -27,6 +27,7 @@
 #include "clang/Parse/ParseAST.h"
 
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Host.h"
@@ -63,6 +64,10 @@
 DumpAST("dump-ast", llvm::cl::init(false),
 llvm::cl::desc("Dump combined AST"));
 
+static llvm::cl::opt
+DumpIR("dump-ir", llvm::cl::init(false),
+llvm::cl::desc("Dump IR from final parse"));
+
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
 private:
@@ -264,7 +269,7 @@
 llvm::Expected
 Parse(const std::string ,
   llvm::ArrayRef Imports,
-  bool ShouldDumpAST) {
+  bool ShouldDumpAST, bool ShouldDumpIR) {
   std::unique_ptr CI =
   init_convenience::BuildCompilerInstance();
   auto ST = llvm::make_unique();
@@ -279,6 +284,7 @@
 
   auto LLVMCtx = llvm::make_unique();
   ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx));
+  auto  = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
 ASTConsumers.push_back(CreateASTDumper("", true, false, false));
@@ -292,6 +298,8 @@
 return std::move(PE);
   }
   CI->getDiagnosticClient().EndSourceFile();
+  if (ShouldDumpIR)
+CG.GetModule()->print(llvm::outs(), nullptr);
   if (CI->getDiagnosticClient().getNumErrors()) {
 return llvm::make_error(
 "Errors occured while parsing the expression.", std::error_code());
@@ -309,7 +317,7 @@
   std::vector ImportCIs;
   for (auto I : Imports) {
 llvm::Expected ImportCI =
-  Parse(I, {}, false);
+  Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
   llvm::errs() << llvm::toString(std::move(E));
   exit(-1);
@@ -325,7 +333,7 @@
 }
   }
   llvm::Expected ExpressionCI =
-  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
Index: cfe/trunk/test/Import/local-struct/Inputs/Callee.cpp
===
--- cfe/trunk/test/Import/local-struct/Inputs/Callee.cpp
+++ cfe/trunk/test/Import/local-struct/Inputs/Callee.cpp
@@ -0,0 +1,12 @@
+struct Bar {
+  void bar(int _a, bool _b) {
+{
+  struct S { int a; };
+  S s = { _a };
+}
+{
+  struct S { bool b; };
+  S t = { _b };
+}
+  };
+};
Index: cfe/trunk/test/Import/local-struct/test.cpp
===
--- cfe/trunk/test/Import/local-struct/test.cpp
+++ cfe/trunk/test/Import/local-struct/test.cpp
@@ -0,0 +1,8 @@
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// XFAIL: *
+// CHECK: %struct.S = type { i
+// CHECK: %struct.S.0 = type { i1 }
+
+void foo() {
+  return Bar().bar(3, true);
+}
Index: cfe/trunk/test/Import/struct-layout/Inputs/Callee.cpp
===
--- cfe/trunk/test/Import/struct-layout/Inputs/Callee.cpp
+++ cfe/trunk/test/Import/struct-layout/Inputs/Callee.cpp
@@ -0,0 +1,9 @@
+struct S {
+  int a;
+};
+
+struct Bar {
+  void bar(int _a) {
+S s = { _a };
+  };
+};
Index: cfe/trunk/test/Import/struct-layout/test.cpp
===
--- cfe/trunk/test/Import/struct-layout/test.cpp
+++ cfe/trunk/test/Import/struct-layout/test.cpp
@@ -0,0 +1,6 @@
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// CHECK: %struct.S = type { i
+
+void foo() {
+  return Bar().bar(3);
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36429: [clang-import-test] Option to dump the IR for an expression

[Sean Callanan via Phabricator via cfe-commits]

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

Eliminate sensitive dependence on `sizeof(int)`.  `bool` should still be 
rendered as `i1` though.


https://reviews.llvm.org/D36429

Files:
  test/Import/local-struct/Inputs/Callee.cpp
  test/Import/local-struct/test.cpp
  test/Import/struct-layout/Inputs/Callee.cpp
  test/Import/struct-layout/test.cpp
  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
@@ -27,6 +27,7 @@
 #include "clang/Parse/ParseAST.h"
 
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Host.h"
@@ -63,6 +64,10 @@
 DumpAST("dump-ast", llvm::cl::init(false),
 llvm::cl::desc("Dump combined AST"));
 
+static llvm::cl::opt
+DumpIR("dump-ir", llvm::cl::init(false),
+llvm::cl::desc("Dump IR from final parse"));
+
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
 private:
@@ -264,7 +269,7 @@
 llvm::Expected
 Parse(const std::string ,
   llvm::ArrayRef Imports,
-  bool ShouldDumpAST) {
+  bool ShouldDumpAST, bool ShouldDumpIR) {
   std::unique_ptr CI =
   init_convenience::BuildCompilerInstance();
   auto ST = llvm::make_unique();
@@ -279,6 +284,7 @@
 
   auto LLVMCtx = llvm::make_unique();
   ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx));
+  auto  = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
 ASTConsumers.push_back(CreateASTDumper("", true, false, false));
@@ -292,6 +298,9 @@
 return std::move(PE);
   }
   CI->getDiagnosticClient().EndSourceFile();
+  if (ShouldDumpIR) {
+CG.GetModule()->print(llvm::outs(), nullptr);
+  }
   if (CI->getDiagnosticClient().getNumErrors()) {
 return llvm::make_error(
 "Errors occured while parsing the expression.", std::error_code());
@@ -309,7 +318,7 @@
   std::vector ImportCIs;
   for (auto I : Imports) {
 llvm::Expected ImportCI =
-  Parse(I, {}, false);
+  Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
   llvm::errs() << llvm::toString(std::move(E));
   exit(-1);
@@ -325,7 +334,7 @@
 }
   }
   llvm::Expected ExpressionCI =
-  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
Index: test/Import/struct-layout/test.cpp
===
--- test/Import/struct-layout/test.cpp
+++ test/Import/struct-layout/test.cpp
@@ -1,7 +1,6 @@
-// RUN: clang-import-test -dump-ast -import %S/Inputs/Hierarchy.cpp -expression %s | FileCheck %s
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// CHECK: %struct.S = type { i
 
-// CHECK: Overrides:{{.*}}Base::foo
-
 void foo() {
-  Derived d;
+  return Bar().bar(3);
 }
Index: test/Import/struct-layout/Inputs/Callee.cpp
===
--- test/Import/struct-layout/Inputs/Callee.cpp
+++ test/Import/struct-layout/Inputs/Callee.cpp
@@ -1,9 +1,9 @@
-class Base {
-public:
-  virtual void foo() {}
+struct S {
+  int a;
 };
 
-class Derived : public Base {
-public:
-  void foo() override {}
+struct Bar {
+  void bar(int _a) {
+S s = { _a };
+  };
 };
Index: test/Import/local-struct/test.cpp
===
--- test/Import/local-struct/test.cpp
+++ test/Import/local-struct/test.cpp
@@ -1,7 +1,8 @@
-// RUN: clang-import-test -dump-ast -import %S/Inputs/Hierarchy.cpp -expression %s | FileCheck %s
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// XFAIL: *
+// CHECK: %struct.S = type { i
+// CHECK: %struct.S.0 = type { i1 }
 
-// CHECK: Overrides:{{.*}}Base::foo
-
 void foo() {
-  Derived d;
+  return Bar().bar(3, true);
 }
Index: test/Import/local-struct/Inputs/Callee.cpp
===
--- test/Import/local-struct/Inputs/Callee.cpp
+++ test/Import/local-struct/Inputs/Callee.cpp
@@ -1,9 +1,12 @@
-class Base {
-public:
-  virtual void foo() {}
+struct Bar {
+  void bar(int _a, bool _b) {
+{
+  struct S { int a; };
+  S s = { _a };
+}
+{
+  struct S { bool b; };
+  S t = { _b };
+}
+  };
 };
-
-class Derived : public Base {
-public:
-  void foo() override {}
-};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D36429: [clang-import-test] Option to dump the IR for an expression

[Sean Callanan via Phabricator via cfe-commits]

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

Added a passing test for a global struct, so we have something that'll fail if 
the IR dumping breaks.


https://reviews.llvm.org/D36429

Files:
  test/Import/local-struct/Inputs/Callee.cpp
  test/Import/local-struct/test.cpp
  test/Import/struct-layout/Inputs/Callee.cpp
  test/Import/struct-layout/test.cpp
  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
@@ -27,6 +27,7 @@
 #include "clang/Parse/ParseAST.h"
 
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Host.h"
@@ -63,6 +64,10 @@
 DumpAST("dump-ast", llvm::cl::init(false),
 llvm::cl::desc("Dump combined AST"));
 
+static llvm::cl::opt
+DumpIR("dump-ir", llvm::cl::init(false),
+llvm::cl::desc("Dump IR from final parse"));
+
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
 private:
@@ -264,7 +269,7 @@
 llvm::Expected
 Parse(const std::string ,
   llvm::ArrayRef Imports,
-  bool ShouldDumpAST) {
+  bool ShouldDumpAST, bool ShouldDumpIR) {
   std::unique_ptr CI =
   init_convenience::BuildCompilerInstance();
   auto ST = llvm::make_unique();
@@ -279,6 +284,7 @@
 
   auto LLVMCtx = llvm::make_unique();
   ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx));
+  auto  = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
 ASTConsumers.push_back(CreateASTDumper("", true, false, false));
@@ -292,6 +298,9 @@
 return std::move(PE);
   }
   CI->getDiagnosticClient().EndSourceFile();
+  if (ShouldDumpIR) {
+CG.GetModule()->print(llvm::outs(), nullptr);
+  }
   if (CI->getDiagnosticClient().getNumErrors()) {
 return llvm::make_error(
 "Errors occured while parsing the expression.", std::error_code());
@@ -309,7 +318,7 @@
   std::vector ImportCIs;
   for (auto I : Imports) {
 llvm::Expected ImportCI =
-  Parse(I, {}, false);
+  Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
   llvm::errs() << llvm::toString(std::move(E));
   exit(-1);
@@ -325,7 +334,7 @@
 }
   }
   llvm::Expected ExpressionCI =
-  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
Index: test/Import/struct-layout/test.cpp
===
--- test/Import/struct-layout/test.cpp
+++ test/Import/struct-layout/test.cpp
@@ -1,7 +1,6 @@
-// RUN: clang-import-test -dump-ast -import %S/Inputs/Hierarchy.cpp -expression %s | FileCheck %s
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// CHECK: %struct.S = type { i32 }
 
-// CHECK: Overrides:{{.*}}Base::foo
-
 void foo() {
-  Derived d;
+  return Bar().bar(3);
 }
Index: test/Import/struct-layout/Inputs/Callee.cpp
===
--- test/Import/struct-layout/Inputs/Callee.cpp
+++ test/Import/struct-layout/Inputs/Callee.cpp
@@ -1,9 +1,9 @@
-class Base {
-public:
-  virtual void foo() {}
+struct S {
+  int a;
 };
 
-class Derived : public Base {
-public:
-  void foo() override {}
+struct Bar {
+  void bar(int _a) {
+S s = { _a };
+  };
 };
Index: test/Import/local-struct/test.cpp
===
--- test/Import/local-struct/test.cpp
+++ test/Import/local-struct/test.cpp
@@ -1,7 +1,8 @@
-// RUN: clang-import-test -dump-ast -import %S/Inputs/Hierarchy.cpp -expression %s | FileCheck %s
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// XFAIL: *
+// CHECK: %struct.S = type { i32 }
+// CHECK: %struct.S.0 = type { i8 }
 
-// CHECK: Overrides:{{.*}}Base::foo
-
 void foo() {
-  Derived d;
+  return Bar().bar(3, true);
 }
Index: test/Import/local-struct/Inputs/Callee.cpp
===
--- test/Import/local-struct/Inputs/Callee.cpp
+++ test/Import/local-struct/Inputs/Callee.cpp
@@ -1,9 +1,12 @@
-class Base {
-public:
-  virtual void foo() {}
+struct Bar {
+  void bar(int _a, bool _b) {
+{
+  struct S { int a; };
+  S s = { _a };
+}
+{
+  struct S { bool b; };
+  S t = { _b };
+}
+  };
 };
-
-class Derived : public Base {
-public:
-  void foo() override {}
-};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D36429: [clang-import-test] Option to dump the IR for an expression

[Sean Callanan via Phabricator via cfe-commits]

spyffe created this revision.

This adds the argument `--dump-ir` to `clang-import-test`, which allows viewing 
of the final IR.   This is useful for confirming that structure layout was 
correct.

I've added an XFAILed test that exercises this, checking that a struct defined 
inside a function body has the right fields.  Currently it does not because 
`LookupSameContext()` (ExternalASTMerger.cpp) can't find the struct.  This is 
an issue I intend to resolve separately.


Repository:
  rL LLVM

https://reviews.llvm.org/D36429

Files:
  test/Import/local-struct/Inputs/Callee.cpp
  test/Import/local-struct/test.cpp
  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
@@ -27,6 +27,7 @@
 #include "clang/Parse/ParseAST.h"
 
 #include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Host.h"
@@ -63,6 +64,10 @@
 DumpAST("dump-ast", llvm::cl::init(false),
 llvm::cl::desc("Dump combined AST"));
 
+static llvm::cl::opt
+DumpIR("dump-ir", llvm::cl::init(false),
+llvm::cl::desc("Dump IR from final parse"));
+
 namespace init_convenience {
 class TestDiagnosticConsumer : public DiagnosticConsumer {
 private:
@@ -264,7 +269,7 @@
 llvm::Expected
 Parse(const std::string ,
   llvm::ArrayRef Imports,
-  bool ShouldDumpAST) {
+  bool ShouldDumpAST, bool ShouldDumpIR) {
   std::unique_ptr CI =
   init_convenience::BuildCompilerInstance();
   auto ST = llvm::make_unique();
@@ -279,6 +284,7 @@
 
   auto LLVMCtx = llvm::make_unique();
   ASTConsumers.push_back(init_convenience::BuildCodeGen(*CI, *LLVMCtx));
+  auto  = *static_cast(ASTConsumers.back().get());
 
   if (ShouldDumpAST)
 ASTConsumers.push_back(CreateASTDumper("", true, false, false));
@@ -292,6 +298,9 @@
 return std::move(PE);
   }
   CI->getDiagnosticClient().EndSourceFile();
+  if (ShouldDumpIR) {
+CG.GetModule()->print(llvm::outs(), nullptr);
+  }
   if (CI->getDiagnosticClient().getNumErrors()) {
 return llvm::make_error(
 "Errors occured while parsing the expression.", std::error_code());
@@ -309,7 +318,7 @@
   std::vector ImportCIs;
   for (auto I : Imports) {
 llvm::Expected ImportCI =
-  Parse(I, {}, false);
+  Parse(I, {}, false, false);
 if (auto E = ImportCI.takeError()) {
   llvm::errs() << llvm::toString(std::move(E));
   exit(-1);
@@ -325,7 +334,7 @@
 }
   }
   llvm::Expected ExpressionCI =
-  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs, DumpAST, DumpIR);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
Index: test/Import/local-struct/test.cpp
===
--- test/Import/local-struct/test.cpp
+++ test/Import/local-struct/test.cpp
@@ -1,7 +1,8 @@
-// RUN: clang-import-test -dump-ast -import %S/Inputs/Hierarchy.cpp -expression %s | FileCheck %s
+// RUN: clang-import-test -dump-ir -import %S/Inputs/Callee.cpp -expression %s | FileCheck %s
+// XFAIL: *
+// CHECK: %struct.S = type { i32 }
+// CHECK: %struct.S.0 = type { i8 }
 
-// CHECK: Overrides:{{.*}}Base::foo
-
 void foo() {
-  Derived d;
+  return Bar().bar(3, true);
 }
Index: test/Import/local-struct/Inputs/Callee.cpp
===
--- test/Import/local-struct/Inputs/Callee.cpp
+++ test/Import/local-struct/Inputs/Callee.cpp
@@ -1,9 +1,12 @@
-class Base {
-public:
-  virtual void foo() {}
+struct Bar {
+  void bar(int _a, bool _b) {
+{
+  struct S { int a; };
+  S s = { _a };
+}
+{
+  struct S { bool b; };
+  S t = { _b };
+}
+  };
 };
-
-class Derived : public Base {
-public:
-  void foo() override {}
-};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D6550: ASTImporter: Fix missing SourceLoc imports

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

Hmm, the transforming in place of `SelLocs` reads a little weirdly to me, but 
other than that the code seems fine.
Is your concern that you don't know how to write an Objective-C test that would 
cover this?  It looks to me like an Objective-C interface with a method:

  @interface MyClass { }
  -(int)addInt:(int)a toInt:(int)b moduloInt:(int)c;
  @end

might be enough for an ASTMerge test.  If you want to make `clang-import-test` 
use this code, then we might need to add one or two things into that tester to 
handle Objective-C method lookup.


https://reviews.llvm.org/D6550



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

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

[Sean Callanan via Phabricator via cfe-commits]

spyffe marked 2 inline comments as done.
spyffe added inline comments.



Comment at: tools/clang-import-test/clang-import-test.cpp:263
+AddExternalSource(*CI, Imports);
+  }
 

bruno wrote:
> No need for the curly braces here
Okay.


https://reviews.llvm.org/D32981



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

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

[Sean Callanan via Phabricator via cfe-commits]

spyffe marked an inline comment as done.
spyffe added inline comments.



Comment at: lib/AST/ASTImporter.cpp:1757
   D2->setAnonymousStructOrUnion(true);
+if (PrevDecl) {
+  D2->setPreviousDecl(PrevDecl);

a.sidorin wrote:
> By the way, should we do the same for some other kinds of Decls (like 
> FunctionDecl)? If so, could you write a NOTE comment?
I added this as a `FIXME`, in the style of the other notes about incomplete 
implementation in this file.


https://reviews.llvm.org/D32981



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

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

[Sean Callanan via Phabricator via cfe-commits]

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

• Added a FIXME per Aleksei Sidorin's suggestion.


https://reviews.llvm.org/D32981

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/ASTImporter.cpp
  lib/AST/ASTStructuralEquivalence.cpp
  lib/AST/ExternalASTMerger.cpp
  test/Import/conflicting-struct/Inputs/S1.cpp
  test/Import/conflicting-struct/Inputs/S2.cpp
  test/Import/conflicting-struct/test.cpp
  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
@@ -42,6 +42,10 @@
 Imports("import", llvm::cl::ZeroOrMore,
 llvm::cl::desc("Path to a file containing declarations to import"));
 
+static llvm::cl::opt
+Direct("direct", llvm::cl::Optional,
+ llvm::cl::desc("Use the parsed declarations without indirection"));
+
 static llvm::cl::list
 ClangArgs("Xcc", llvm::cl::ZeroOrMore,
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -172,6 +176,14 @@
   return Ins;
 }
 
+std::unique_ptr
+BuildCompilerInstance(ArrayRef ClangArgs) {
+  std::vector ClangArgv(ClangArgs.size());
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string ) -> const char * { return s.data(); });
+  return init_convenience::BuildCompilerInstance(ClangArgv);
+}
+
 std::unique_ptr
 BuildASTContext(CompilerInstance , SelectorTable , Builtin::Context ) {
   auto AST = llvm::make_unique(
@@ -205,6 +217,21 @@
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
+std::unique_ptr BuildIndirect(std::unique_ptr ) {
+  std::vector ClangArgv(ClangArgs.size());
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string ) -> const char * { return s.data(); });
+  std::unique_ptr IndirectCI =
+  init_convenience::BuildCompilerInstance(ClangArgv);
+  auto ST = llvm::make_unique();
+  auto BC = llvm::make_unique();
+  std::unique_ptr AST =
+  init_convenience::BuildASTContext(*IndirectCI, *ST, *BC);
+  IndirectCI->setASTContext(AST.release());
+  AddExternalSource(*IndirectCI, CI);
+  return IndirectCI;
+}
+
 llvm::Error ParseSource(const std::string , CompilerInstance ,
 CodeGenerator ) {
   SourceManager  = CI.getSourceManager();
@@ -231,7 +258,9 @@
   std::unique_ptr AST =
   init_convenience::BuildASTContext(*CI, *ST, *BC);
   CI->setASTContext(AST.release());
-  AddExternalSource(*CI, Imports);
+  if (Imports.size()) {
+AddExternalSource(*CI, Imports);
+  }
 
   auto LLVMCtx = llvm::make_unique();
   std::unique_ptr CG =
@@ -268,12 +297,26 @@
   ImportCIs.push_back(std::move(*ImportCI));
 }
   }
+  std::vector IndirectCIs;
+  if (!Direct) {
+for (auto  : ImportCIs) {
+  llvm::Expected IndirectCI =
+  BuildIndirect(ImportCI);
+  if (auto E = IndirectCI.takeError()) {
+llvm::errs() << llvm::toString(std::move(E));
+exit(-1);
+  } else {
+IndirectCIs.push_back(std::move(*IndirectCI));
+  }
+}
+  }
   llvm::Expected ExpressionCI =
-  Parse(Expression, ImportCIs);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
   } else {
 return 0;
   }
 }
+
Index: test/Import/conflicting-struct/test.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/test.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s
+void expr() {
+  S MyS;
+  T MyT;
+  MyS.a = 3;
+  MyT.u.b = 2;
+}
Index: test/Import/conflicting-struct/Inputs/S2.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/Inputs/S2.cpp
@@ -0,0 +1,7 @@
+class U {
+  int b;
+};
+
+class T {
+  U u;
+};
Index: test/Import/conflicting-struct/Inputs/S1.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/Inputs/S1.cpp
@@ -0,0 +1,6 @@
+class T;
+
+class S {
+  T *t;
+  int a;
+};
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -178,3 +178,9 @@
 }
   });
 }
+
+void ExternalASTMerger::CompleteType(TagDecl *Tag) {
+  SmallVector Result;
+  FindExternalLexicalDecls(Tag, [](Decl::Kind) { return true; }, Result);
+  Tag->setHasExternalLexicalStorage(false);
+}
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- lib/AST/ASTStructuralEquivalence.cpp

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

[Sean Callanan via Phabricator via cfe-commits]

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

Eliminated a //missing newline at end of file// error


Repository:
  rL LLVM

https://reviews.llvm.org/D32981

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/ASTImporter.cpp
  lib/AST/ASTStructuralEquivalence.cpp
  lib/AST/ExternalASTMerger.cpp
  test/Import/conflicting-struct/Inputs/S1.cpp
  test/Import/conflicting-struct/Inputs/S2.cpp
  test/Import/conflicting-struct/test.cpp
  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
@@ -42,6 +42,10 @@
 Imports("import", llvm::cl::ZeroOrMore,
 llvm::cl::desc("Path to a file containing declarations to import"));
 
+static llvm::cl::opt
+Direct("direct", llvm::cl::Optional,
+ llvm::cl::desc("Use the parsed declarations without indirection"));
+
 static llvm::cl::list
 ClangArgs("Xcc", llvm::cl::ZeroOrMore,
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -172,6 +176,14 @@
   return Ins;
 }
 
+std::unique_ptr
+BuildCompilerInstance(ArrayRef ClangArgs) {
+  std::vector ClangArgv(ClangArgs.size());
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string ) -> const char * { return s.data(); });
+  return init_convenience::BuildCompilerInstance(ClangArgv);
+}
+
 std::unique_ptr
 BuildASTContext(CompilerInstance , SelectorTable , Builtin::Context ) {
   auto AST = llvm::make_unique(
@@ -205,6 +217,21 @@
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
+std::unique_ptr BuildIndirect(std::unique_ptr ) {
+  std::vector ClangArgv(ClangArgs.size());
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string ) -> const char * { return s.data(); });
+  std::unique_ptr IndirectCI =
+  init_convenience::BuildCompilerInstance(ClangArgv);
+  auto ST = llvm::make_unique();
+  auto BC = llvm::make_unique();
+  std::unique_ptr AST =
+  init_convenience::BuildASTContext(*IndirectCI, *ST, *BC);
+  IndirectCI->setASTContext(AST.release());
+  AddExternalSource(*IndirectCI, CI);
+  return IndirectCI;
+}
+
 llvm::Error ParseSource(const std::string , CompilerInstance ,
 CodeGenerator ) {
   SourceManager  = CI.getSourceManager();
@@ -231,7 +258,9 @@
   std::unique_ptr AST =
   init_convenience::BuildASTContext(*CI, *ST, *BC);
   CI->setASTContext(AST.release());
-  AddExternalSource(*CI, Imports);
+  if (Imports.size()) {
+AddExternalSource(*CI, Imports);
+  }
 
   auto LLVMCtx = llvm::make_unique();
   std::unique_ptr CG =
@@ -268,12 +297,26 @@
   ImportCIs.push_back(std::move(*ImportCI));
 }
   }
+  std::vector IndirectCIs;
+  if (!Direct) {
+for (auto  : ImportCIs) {
+  llvm::Expected IndirectCI =
+  BuildIndirect(ImportCI);
+  if (auto E = IndirectCI.takeError()) {
+llvm::errs() << llvm::toString(std::move(E));
+exit(-1);
+  } else {
+IndirectCIs.push_back(std::move(*IndirectCI));
+  }
+}
+  }
   llvm::Expected ExpressionCI =
-  Parse(Expression, ImportCIs);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
   } else {
 return 0;
   }
 }
+
Index: test/Import/conflicting-struct/test.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/test.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s
+void expr() {
+  S MyS;
+  T MyT;
+  MyS.a = 3;
+  MyT.u.b = 2;
+}
Index: test/Import/conflicting-struct/Inputs/S2.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/Inputs/S2.cpp
@@ -0,0 +1,7 @@
+class U {
+  int b;
+};
+
+class T {
+  U u;
+};
Index: test/Import/conflicting-struct/Inputs/S1.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/Inputs/S1.cpp
@@ -0,0 +1,6 @@
+class T;
+
+class S {
+  T *t;
+  int a;
+};
Index: lib/AST/ExternalASTMerger.cpp
===
--- lib/AST/ExternalASTMerger.cpp
+++ lib/AST/ExternalASTMerger.cpp
@@ -178,3 +178,9 @@
 }
   });
 }
+
+void ExternalASTMerger::CompleteType(TagDecl *Tag) {
+  SmallVector Result;
+  FindExternalLexicalDecls(Tag, [](Decl::Kind) { return true; }, Result);
+  Tag->setHasExternalLexicalStorage(false);
+}
Index: lib/AST/ASTStructuralEquivalence.cpp
===
--- 

[PATCH] D32981: [ASTImporter] Improve handling of incomplete types

[Sean Callanan via Phabricator via cfe-commits]

spyffe created this revision.

`ASTImporter` has some bugs when it's importing types that themselves come from 
an `ExternalASTSource`.  This is exposed particularly in the behavior when 
comparing complete TagDecls with forward declarations.  This patch does several 
things:

- Adds a test case making sure that conflicting forward-declarations are 
resolved correctly;
- Extends the `clang-import-test` harness to test two-level importing, so that 
we make sure we complete types when necessary; and
- Fixes a few bugs I found this way.  Failure to complete types was one; 
however, I also discovered that complete RecordDecls aren't properly added to 
the `redecls` chain for existing forward declarations.


Repository:
  rL LLVM

https://reviews.llvm.org/D32981

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/ASTImporter.cpp
  lib/AST/ASTStructuralEquivalence.cpp
  lib/AST/ExternalASTMerger.cpp
  test/Import/conflicting-struct/Inputs/S1.cpp
  test/Import/conflicting-struct/Inputs/S2.cpp
  test/Import/conflicting-struct/test.cpp
  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
@@ -42,6 +42,10 @@
 Imports("import", llvm::cl::ZeroOrMore,
 llvm::cl::desc("Path to a file containing declarations to import"));
 
+static llvm::cl::opt
+Direct("direct", llvm::cl::Optional,
+ llvm::cl::desc("Use the parsed declarations without indirection"));
+
 static llvm::cl::list
 ClangArgs("Xcc", llvm::cl::ZeroOrMore,
   llvm::cl::desc("Argument to pass to the CompilerInvocation"),
@@ -172,6 +176,14 @@
   return Ins;
 }
 
+std::unique_ptr
+BuildCompilerInstance(ArrayRef ClangArgs) {
+  std::vector ClangArgv(ClangArgs.size());
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string ) -> const char * { return s.data(); });
+  return init_convenience::BuildCompilerInstance(ClangArgv);
+}
+
 std::unique_ptr
 BuildASTContext(CompilerInstance , SelectorTable , Builtin::Context ) {
   auto AST = llvm::make_unique(
@@ -205,6 +217,21 @@
   CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
+std::unique_ptr BuildIndirect(std::unique_ptr ) {
+  std::vector ClangArgv(ClangArgs.size());
+  std::transform(ClangArgs.begin(), ClangArgs.end(), ClangArgv.begin(),
+ [](const std::string ) -> const char * { return s.data(); });
+  std::unique_ptr IndirectCI =
+  init_convenience::BuildCompilerInstance(ClangArgv);
+  auto ST = llvm::make_unique();
+  auto BC = llvm::make_unique();
+  std::unique_ptr AST =
+  init_convenience::BuildASTContext(*IndirectCI, *ST, *BC);
+  IndirectCI->setASTContext(AST.release());
+  AddExternalSource(*IndirectCI, CI);
+  return IndirectCI;
+}
+
 llvm::Error ParseSource(const std::string , CompilerInstance ,
 CodeGenerator ) {
   SourceManager  = CI.getSourceManager();
@@ -231,7 +258,9 @@
   std::unique_ptr AST =
   init_convenience::BuildASTContext(*CI, *ST, *BC);
   CI->setASTContext(AST.release());
-  AddExternalSource(*CI, Imports);
+  if (Imports.size()) {
+AddExternalSource(*CI, Imports);
+  }
 
   auto LLVMCtx = llvm::make_unique();
   std::unique_ptr CG =
@@ -268,12 +297,26 @@
   ImportCIs.push_back(std::move(*ImportCI));
 }
   }
+  std::vector IndirectCIs;
+  if (!Direct) {
+for (auto  : ImportCIs) {
+  llvm::Expected IndirectCI =
+  BuildIndirect(ImportCI);
+  if (auto E = IndirectCI.takeError()) {
+llvm::errs() << llvm::toString(std::move(E));
+exit(-1);
+  } else {
+IndirectCIs.push_back(std::move(*IndirectCI));
+  }
+}
+  }
   llvm::Expected ExpressionCI =
-  Parse(Expression, ImportCIs);
+  Parse(Expression, Direct ? ImportCIs : IndirectCIs);
   if (auto E = ExpressionCI.takeError()) {
 llvm::errs() << llvm::toString(std::move(E));
 exit(-1);
   } else {
 return 0;
   }
 }
+
Index: test/Import/conflicting-struct/test.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/test.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-import-test --import %S/Inputs/S1.cpp --import %S/Inputs/S2.cpp -expression %s
+void expr() {
+  S MyS;
+  T MyT;
+  MyS.a = 3;
+  MyT.u.b = 2;
+}
Index: test/Import/conflicting-struct/Inputs/S2.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/Inputs/S2.cpp
@@ -0,0 +1,7 @@
+class U {
+  int b;
+};
+
+class T {
+  U u;
+};
Index: test/Import/conflicting-struct/Inputs/S1.cpp
===
--- /dev/null
+++ test/Import/conflicting-struct/Inputs/S1.cpp
@@ -0,0 +1,6 @@

[PATCH] D32724: [Modules] Include the enabled sanitizers in the module hash

[Sean Callanan via Phabricator via cfe-commits]

spyffe added inline comments.



Comment at: lib/Basic/LangOptions.cpp:32
 
-  // FIXME: This should not be reset; modules can be different with different
-  // sanitizer options (this affects __has_feature(address_sanitizer) etc).
-  Sanitize.clear();
+  // These options do not affect AST generation.
   SanitizerBlacklistFiles.clear();

vsk wrote:
> spyffe wrote:
> > I'd replace this with
> > ```
> > Sanitize.clear(SanitizerKind::CFI | SanitizerKind::Integer | 
> > SanitizerKind::Nullability | SanitizerKind::Undefined);
> > ```
> > We know those options don't affect modules, as demonstrated by you clearing 
> > them anyway in CompilerInvocation...
> I don't think this would work, and don't quite see a way to make it work. The 
> problem is that when importing a module into a CU, the CU's hash is required 
> to match the to-be-imported module's hash [1]. If we clear some sanitizer 
> options in resetNonModularOptions(), then the "matching hashes" check would 
> break, because you can't reset the non-modular options in a CU that you're 
> importing a module into. You'd end up disabling the sanitizers for the CU 
> you're building.
> 
> [1] CompilerInstance.cpp
> ```
> 1095   assert(ImportingInstance.getInvocation().getModuleHash() ==
>   
>   
>   
>   
> 1096  Invocation->getModuleHash() && "Module hash mismatch!"); 
> ```
Okay, I probably just didn't understand the role of `resetNonModularOptions()`.


https://reviews.llvm.org/D32724



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

[PATCH] D32724: [Modules] Include the enabled sanitizers in the module hash

[Sean Callanan via Phabricator via cfe-commits]

spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.

A few minor nits, but the operation of ignoring certain sanitizer flags seems 
to be happening in the wrong place.




Comment at: lib/Basic/LangOptions.cpp:32
 
-  // FIXME: This should not be reset; modules can be different with different
-  // sanitizer options (this affects __has_feature(address_sanitizer) etc).
-  Sanitize.clear();
+  // These options do not affect AST generation.
   SanitizerBlacklistFiles.clear();

I'd replace this with
```
Sanitize.clear(SanitizerKind::CFI | SanitizerKind::Integer | 
SanitizerKind::Nullability | SanitizerKind::Undefined);
```
We know those options don't affect modules, as demonstrated by you clearing 
them anyway in CompilerInvocation...



Comment at: lib/Frontend/CompilerInvocation.cpp:2699
+  SanitizerSet SanHash = LangOpts->Sanitize;
+  SanHash.clear(SanitizerKind::CFI | SanitizerKind::Integer |
+SanitizerKind::Nullability | SanitizerKind::Undefined);

If `clearNonModularOptions()` does the right thing, you may not need to clear 
these flags here



Comment at: test/Modules/check-for-sanitizer-feature.cpp:25
+//
+// First, built without any sanitization.
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.2 \

Typo: //build//



Comment at: test/Modules/check-for-sanitizer-feature.cpp:42
+#if HAS_ASAN != 1
+#error Does not have the address_sanitizer feature, but should.
+#endif

This error isn't very illuminating: I'd say
> Module doesn't have the address_sanitizer feature, but main program does.
Same below.




https://reviews.llvm.org/D32724



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

[PATCH] D31777: [ASTImporter] Move structural equivalence context to its own file. NFCI

[Sean Callanan via Phabricator via cfe-commits]

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

Looks good to me.  This is a good step toward simplifying the ASTImporter and 
breaking out its parts to be useful on their own.
Obviously make sure the tests pass :)


https://reviews.llvm.org/D31777



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

[PATCH] D30435: [clang-import-test] Lookup inside entities

[Sean Callanan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL299976: [clang-import-test] Lookup inside contexts (authored 
by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D30435?vs=93929=94875#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D30435

Files:
  cfe/trunk/include/clang/AST/ExternalASTMerger.h
  cfe/trunk/lib/AST/CMakeLists.txt
  cfe/trunk/lib/AST/ExternalASTMerger.cpp
  cfe/trunk/test/Import/forward-declared-struct/Inputs/S1.c
  cfe/trunk/test/Import/forward-declared-struct/Inputs/S2.c
  cfe/trunk/test/Import/forward-declared-struct/test.c
  cfe/trunk/test/Import/member-in-struct/Inputs/S.c
  cfe/trunk/test/Import/member-in-struct/test.c
  cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S1.c
  cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S2.c
  cfe/trunk/test/Import/multiple-forward-declarations/test.c
  cfe/trunk/test/Import/overloaded-function/Inputs/F1.c
  cfe/trunk/test/Import/overloaded-function/Inputs/F2.c
  cfe/trunk/test/Import/overloaded-function/test.c
  cfe/trunk/test/Import/struct-in-namespace/Inputs/N1.cpp
  cfe/trunk/test/Import/struct-in-namespace/Inputs/N2.cpp
  cfe/trunk/test/Import/struct-in-namespace/Inputs/N3.cpp
  cfe/trunk/test/Import/struct-in-namespace/test.cpp
  cfe/trunk/test/Import/template-specialization/Inputs/T.cpp
  cfe/trunk/test/Import/template-specialization/test.cpp
  cfe/trunk/tools/clang-import-test/clang-import-test.cpp

Index: cfe/trunk/test/Import/multiple-forward-declarations/test.c
===
--- cfe/trunk/test/Import/multiple-forward-declarations/test.c
+++ cfe/trunk/test/Import/multiple-forward-declarations/test.c
@@ -0,0 +1,4 @@
+// RUN: clang-import-test -import %S/Inputs/S1.c --import %S/Inputs/S2.c -expression %s
+void expr() {
+  struct S *MySPtr;
+}
Index: cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S2.c
===
--- cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S2.c
+++ cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S2.c
@@ -0,0 +1 @@
+struct S;
Index: cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S1.c
===
--- cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S1.c
+++ cfe/trunk/test/Import/multiple-forward-declarations/Inputs/S1.c
@@ -0,0 +1 @@
+struct S;
Index: cfe/trunk/test/Import/struct-in-namespace/Inputs/N3.cpp
===
--- cfe/trunk/test/Import/struct-in-namespace/Inputs/N3.cpp
+++ cfe/trunk/test/Import/struct-in-namespace/Inputs/N3.cpp
@@ -0,0 +1,5 @@
+namespace M {
+  struct V {
+int d;
+  };
+}
Index: cfe/trunk/test/Import/struct-in-namespace/Inputs/N2.cpp
===
--- cfe/trunk/test/Import/struct-in-namespace/Inputs/N2.cpp
+++ cfe/trunk/test/Import/struct-in-namespace/Inputs/N2.cpp
@@ -0,0 +1,5 @@
+namespace N {
+  struct U {
+int c;
+  };
+}
Index: cfe/trunk/test/Import/struct-in-namespace/Inputs/N1.cpp
===
--- cfe/trunk/test/Import/struct-in-namespace/Inputs/N1.cpp
+++ cfe/trunk/test/Import/struct-in-namespace/Inputs/N1.cpp
@@ -0,0 +1,11 @@
+namespace N {
+  struct S {
+int a;
+  };
+}
+
+namespace N {
+  struct T {
+int b;
+  };
+}
Index: cfe/trunk/test/Import/struct-in-namespace/test.cpp
===
--- cfe/trunk/test/Import/struct-in-namespace/test.cpp
+++ cfe/trunk/test/Import/struct-in-namespace/test.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-import-test -import %S/Inputs/N1.cpp -import %S/Inputs/N2.cpp -import %S/Inputs/N3.cpp -expression %s
+void expr() {
+  N::S s;
+  N::T t;
+  N::U u;
+  int d = s.a + t.b + u.c;
+}
Index: cfe/trunk/test/Import/member-in-struct/Inputs/S.c
===
--- cfe/trunk/test/Import/member-in-struct/Inputs/S.c
+++ cfe/trunk/test/Import/member-in-struct/Inputs/S.c
@@ -0,0 +1,3 @@
+struct S {
+  int a;
+};
Index: cfe/trunk/test/Import/member-in-struct/test.c
===
--- cfe/trunk/test/Import/member-in-struct/test.c
+++ cfe/trunk/test/Import/member-in-struct/test.c
@@ -0,0 +1,5 @@
+// RUN: clang-import-test -import %S/Inputs/S.c -expression %s
+void expr() {
+  struct S MyS;
+  MyS.a = 3;
+}
Index: cfe/trunk/test/Import/template-specialization/Inputs/T.cpp
===
--- cfe/trunk/test/Import/template-specialization/Inputs/T.cpp
+++ cfe/trunk/test/Import/template-specialization/Inputs/T.cpp
@@ -0,0 +1,14 @@
+template  struct A {
+};
+
+template <> struct A {
+  struct B {
+int f;
+  };
+};
+
+template <> struct A {
+  struct B {
+int g;
+  };
+};
Index: 

[PATCH] D30435: [clang-import-test] Lookup inside entities

[Sean Callanan via Phabricator via cfe-commits]

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

Added the `ExternalASTMerger` implementation/interface, which I hadn't `svn 
add`ed before generating the last diff.


Repository:
  rL LLVM

https://reviews.llvm.org/D30435

Files:
  include/clang/AST/ExternalASTMerger.h
  lib/AST/CMakeLists.txt
  lib/AST/ExternalASTMerger.cpp
  test/Import/forward-declared-struct/Inputs/S1.c
  test/Import/forward-declared-struct/Inputs/S2.c
  test/Import/forward-declared-struct/test.c
  test/Import/member-in-struct/Inputs/S.c
  test/Import/member-in-struct/test.c
  test/Import/multiple-forward-declarations/Inputs/S1.c
  test/Import/multiple-forward-declarations/Inputs/S2.c
  test/Import/multiple-forward-declarations/test.c
  test/Import/overloaded-function/Inputs/F1.c
  test/Import/overloaded-function/Inputs/F2.c
  test/Import/overloaded-function/test.c
  test/Import/struct-in-namespace/Inputs/N1.cpp
  test/Import/struct-in-namespace/Inputs/N2.cpp
  test/Import/struct-in-namespace/Inputs/N3.cpp
  test/Import/struct-in-namespace/test.cpp
  test/Import/template-specialization/Inputs/T.cpp
  test/Import/template-specialization/test.cpp
  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
@@ -9,6 +9,8 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/ExternalASTMerger.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
@@ -189,61 +191,18 @@
 } // end namespace
 
 namespace {
-class TestExternalASTSource : public ExternalASTSource {
-private:
-  llvm::ArrayRef ImportCIs;
-  std::map> ForwardImporters;
-  std::map> ReverseImporters;
-
-public:
-  TestExternalASTSource(
-  CompilerInstance ,
-  llvm::ArrayRef ImportCIs)
-  : ImportCIs(ImportCIs) {
-for (const std::unique_ptr  : ImportCIs) {
-  ForwardImporters[ImportCI.get()] = llvm::make_unique(
-  ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
-  ImportCI->getASTContext(), ImportCI->getFileManager(),
-  /*MinimalImport=*/true);
-  ReverseImporters[ImportCI.get()] = llvm::make_unique(
-  ImportCI->getASTContext(), ImportCI->getFileManager(),
-  ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
-  /*MinimalImport=*/true);
-}
-  }
-
-  bool FindExternalVisibleDeclsByName(const DeclContext *DC,
-  DeclarationName Name) override {
-llvm::SmallVector Decls;
-
-if (isa(DC)) {
-  for (const std::unique_ptr  : ImportCIs) {
-DeclarationName FromName = ReverseImporters[I.get()]->Import(Name);
-DeclContextLookupResult Result =
-I->getASTContext().getTranslationUnitDecl()->lookup(FromName);
-for (NamedDecl *FromD : Result) {
-  NamedDecl *D =
-  llvm::cast(ForwardImporters[I.get()]->Import(FromD));
-  Decls.push_back(D);
-}
-  }
-}
-if (Decls.empty()) {
-  return false;
-} else {
-  SetExternalVisibleDeclsForName(DC, Name, Decls);
-  return true;
-}
-  }
-};
-
+ 
 void AddExternalSource(
 CompilerInstance ,
 llvm::ArrayRef Imports) {
-  ASTContext  = CI.getASTContext();
-  auto ES = llvm::make_unique(CI, Imports);
-  AST.setExternalSource(ES.release());
-  AST.getTranslationUnitDecl()->setHasExternalVisibleStorage();
+  ExternalASTMerger::ImporterEndpoint Target({CI.getASTContext(), CI.getFileManager()});
+  llvm::SmallVector Sources;
+  for (const std::unique_ptr  : Imports) {
+Sources.push_back({CI->getASTContext(), CI->getFileManager()});
+  }
+  auto ES = llvm::make_unique(Target, Sources);
+  CI.getASTContext().setExternalSource(ES.release());
+  CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
 llvm::Error ParseSource(const std::string , CompilerInstance ,
@@ -292,6 +251,7 @@
 return std::move(CI);
   }
 }
+
 } // end namespace
 
 int main(int argc, const char **argv) {
Index: test/Import/template-specialization/test.cpp
===
--- test/Import/template-specialization/test.cpp
+++ test/Import/template-specialization/test.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-import-test -import %S/Inputs/T.cpp -expression %s
+// XFAIL: *
+void expr() {
+  A::B b1;
+  A::B b2;
+  b1.f + b2.g;
+}
Index: test/Import/template-specialization/Inputs/T.cpp
===
--- test/Import/template-specialization/Inputs/T.cpp
+++ test/Import/template-specialization/Inputs/T.cpp
@@ -0,0 +1,14 @@
+template  struct A {
+};
+
+template <> 

[PATCH] D30435: [clang-import-test] Lookup inside entities

[Sean Callanan via Phabricator via cfe-commits]

spyffe updated this revision to Diff 93927.
spyffe added a comment.
Herald added a subscriber: mgorny.

• Broke all the `ASTImporter` and `ExternalASTSource` subclassing logic into 
its own source file, with its API defined in `AST/ExternalASTMerger.h`.
• Cleaned up the API for LLDB's consumption: now rather than a 
`CompilerInstance` a client just has to provide an `ASTContext` and a 
`FileManager` (just like when setting up an `ASTImporter`).  This allows e.g. 
DWARF to be a valid source.


Repository:
  rL LLVM

https://reviews.llvm.org/D30435

Files:
  lib/AST/CMakeLists.txt
  test/Import/forward-declared-struct/Inputs/S1.c
  test/Import/forward-declared-struct/Inputs/S2.c
  test/Import/forward-declared-struct/test.c
  test/Import/member-in-struct/Inputs/S.c
  test/Import/member-in-struct/test.c
  test/Import/multiple-forward-declarations/Inputs/S1.c
  test/Import/multiple-forward-declarations/Inputs/S2.c
  test/Import/multiple-forward-declarations/test.c
  test/Import/overloaded-function/Inputs/F1.c
  test/Import/overloaded-function/Inputs/F2.c
  test/Import/overloaded-function/test.c
  test/Import/struct-in-namespace/Inputs/N1.cpp
  test/Import/struct-in-namespace/Inputs/N2.cpp
  test/Import/struct-in-namespace/Inputs/N3.cpp
  test/Import/struct-in-namespace/test.cpp
  test/Import/template-specialization/Inputs/T.cpp
  test/Import/template-specialization/test.cpp
  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
@@ -9,6 +9,8 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/ExternalASTMerger.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
@@ -189,61 +191,18 @@
 } // end namespace
 
 namespace {
-class TestExternalASTSource : public ExternalASTSource {
-private:
-  llvm::ArrayRef ImportCIs;
-  std::map> ForwardImporters;
-  std::map> ReverseImporters;
-
-public:
-  TestExternalASTSource(
-  CompilerInstance ,
-  llvm::ArrayRef ImportCIs)
-  : ImportCIs(ImportCIs) {
-for (const std::unique_ptr  : ImportCIs) {
-  ForwardImporters[ImportCI.get()] = llvm::make_unique(
-  ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
-  ImportCI->getASTContext(), ImportCI->getFileManager(),
-  /*MinimalImport=*/true);
-  ReverseImporters[ImportCI.get()] = llvm::make_unique(
-  ImportCI->getASTContext(), ImportCI->getFileManager(),
-  ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
-  /*MinimalImport=*/true);
-}
-  }
-
-  bool FindExternalVisibleDeclsByName(const DeclContext *DC,
-  DeclarationName Name) override {
-llvm::SmallVector Decls;
-
-if (isa(DC)) {
-  for (const std::unique_ptr  : ImportCIs) {
-DeclarationName FromName = ReverseImporters[I.get()]->Import(Name);
-DeclContextLookupResult Result =
-I->getASTContext().getTranslationUnitDecl()->lookup(FromName);
-for (NamedDecl *FromD : Result) {
-  NamedDecl *D =
-  llvm::cast(ForwardImporters[I.get()]->Import(FromD));
-  Decls.push_back(D);
-}
-  }
-}
-if (Decls.empty()) {
-  return false;
-} else {
-  SetExternalVisibleDeclsForName(DC, Name, Decls);
-  return true;
-}
-  }
-};
-
+ 
 void AddExternalSource(
 CompilerInstance ,
 llvm::ArrayRef Imports) {
-  ASTContext  = CI.getASTContext();
-  auto ES = llvm::make_unique(CI, Imports);
-  AST.setExternalSource(ES.release());
-  AST.getTranslationUnitDecl()->setHasExternalVisibleStorage();
+  ExternalASTMerger::ImporterEndpoint Target({CI.getASTContext(), CI.getFileManager()});
+  llvm::SmallVector Sources;
+  for (const std::unique_ptr  : Imports) {
+Sources.push_back({CI->getASTContext(), CI->getFileManager()});
+  }
+  auto ES = llvm::make_unique(Target, Sources);
+  CI.getASTContext().setExternalSource(ES.release());
+  CI.getASTContext().getTranslationUnitDecl()->setHasExternalVisibleStorage();
 }
 
 llvm::Error ParseSource(const std::string , CompilerInstance ,
@@ -292,6 +251,7 @@
 return std::move(CI);
   }
 }
+
 } // end namespace
 
 int main(int argc, const char **argv) {
Index: test/Import/template-specialization/test.cpp
===
--- test/Import/template-specialization/test.cpp
+++ test/Import/template-specialization/test.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-import-test -import %S/Inputs/T.cpp -expression %s
+// XFAIL: *
+void expr() {
+  A::B b1;
+  A::B b2;
+  b1.f + b2.g;
+}
Index: 

[PATCH] D30435: [clang-import-test] Lookup inside entities

[Sean Callanan via Phabricator via cfe-commits]

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

• Removed namespace qualification of `cast` across the board.
• Standardized on `SmallVector`
• Factored out the filtering of forward declarations
• Fixed a loop as requested.
• Made a Origin<> wrapper for arbitrary AST objects that makes it clear where 
they live, so importation and searching gets a little clearer.


Repository:
  rL LLVM

https://reviews.llvm.org/D30435

Files:
  test/Import/forward-declared-struct/Inputs/S1.c
  test/Import/forward-declared-struct/Inputs/S2.c
  test/Import/forward-declared-struct/test.c
  test/Import/member-in-struct/Inputs/S.c
  test/Import/member-in-struct/test.c
  test/Import/multiple-forward-declarations/Inputs/S1.c
  test/Import/multiple-forward-declarations/Inputs/S2.c
  test/Import/multiple-forward-declarations/test.c
  test/Import/overloaded-function/Inputs/F1.c
  test/Import/overloaded-function/Inputs/F2.c
  test/Import/overloaded-function/test.c
  test/Import/struct-in-namespace/Inputs/N1.cpp
  test/Import/struct-in-namespace/Inputs/N2.cpp
  test/Import/struct-in-namespace/Inputs/N3.cpp
  test/Import/struct-in-namespace/test.cpp
  test/Import/template-specialization/Inputs/T.cpp
  test/Import/template-specialization/test.cpp
  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
@@ -9,6 +9,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
@@ -189,19 +190,90 @@
 } // end namespace
 
 namespace {
+
+template  struct Source {
+  T t;
+  Source(T &) : t(std::move(t)) {}
+  operator T() { return t; }
+  template  U () { return t; }
+  template  const U () const { return t; }
+  template  operator Source() { return Source(t); }
+};
+
+typedef std::pair Candidate;
+
+class TestASTImporter : public ASTImporter {
+public:
+  TestASTImporter(ASTContext , FileManager ,
+  ASTContext , FileManager ,
+  bool MinimalImport)
+  : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
+MinimalImport) {}
+  Decl *Imported(Decl *From, Decl *To) override {
+if (auto ToTag = dyn_cast(To)) {
+  ToTag->setHasExternalLexicalStorage();
+} else if (auto ToNamespace = dyn_cast(To)) {
+  ToNamespace->setHasExternalVisibleStorage();
+}
+return ASTImporter::Imported(From, To);
+  }
+};
+
+Source
+LookupSameContext(Source SourceTU, const DeclContext *DC,
+  ASTImporter ) {
+  if (DC->isTranslationUnit()) {
+return SourceTU;
+  }
+  Source SourceParentDC =
+  LookupSameContext(SourceTU, DC->getParent(), ReverseImporter);
+  if (!SourceParentDC) {
+// If we couldn't find the parent DC in this TranslationUnit, give up.
+return nullptr;
+  }
+  auto ND = cast(DC);
+  DeclarationName Name = ND->getDeclName();
+  Source SourceName = ReverseImporter.Import(Name);
+  DeclContext::lookup_result SearchResult =
+  SourceParentDC.get()->lookup(SourceName.get());
+  size_t SearchResultSize = SearchResult.size();
+  // Handle multiple candidates once we have a test for it.
+  // This may turn up when we import template specializations correctly.
+  assert(SearchResultSize < 2);
+  if (SearchResultSize == 0) {
+// couldn't find the name, so we have to give up
+return nullptr;
+  } else {
+NamedDecl *SearchResultDecl = SearchResult[0];
+return dyn_cast(SearchResultDecl);
+  }
+}
+
+bool IsForwardDeclaration(Decl *D) {
+  assert(!isa(D)); // TODO handle this case
+  if (auto TD = dyn_cast(D)) {
+return !TD->isThisDeclarationADefinition();
+  } else if (auto FD = dyn_cast(D)) {
+return !FD->isThisDeclarationADefinition();
+  } else {
+return false;
+  }
+}
+
 class TestExternalASTSource : public ExternalASTSource {
 private:
   llvm::ArrayRef ImportCIs;
-  std::map> ForwardImporters;
+  std::map>
+  ForwardImporters;
   std::map> ReverseImporters;
 
 public:
   TestExternalASTSource(
   CompilerInstance ,
   llvm::ArrayRef ImportCIs)
   : ImportCIs(ImportCIs) {
 for (const std::unique_ptr  : ImportCIs) {
-  ForwardImporters[ImportCI.get()] = llvm::make_unique(
+  ForwardImporters[ImportCI.get()] = llvm::make_unique(
   ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
   ImportCI->getASTContext(), ImportCI->getFileManager(),
   /*MinimalImport=*/true);
@@ -212,29 +284,93 @@
 }
   }
 
+  void
+  ForEachMatchingDC(const DeclContext *DC,
+std::function

[PATCH] D30435: [clang-import-test] Lookup inside entities

[Sean Callanan via Phabricator via cfe-commits]

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

• Applied Adrian's suggestion to move comments before the line they apply to.
• Got to 100% coverage by changing a test and making an assert for something I 
couldn't make happen in a test.

I did not take Adrian's suggestion to remove braces around a single-statement 
`if` because (as far as I can tell) the LLVM coding standard is silent on the 
topic and I prefer to have the braces there.


Repository:
  rL LLVM

https://reviews.llvm.org/D30435

Files:
  test/Import/forward-declared-struct/Inputs/S1.c
  test/Import/forward-declared-struct/Inputs/S2.c
  test/Import/forward-declared-struct/test.c
  test/Import/member-in-struct/Inputs/S.c
  test/Import/member-in-struct/test.c
  test/Import/multiple-forward-declarations/Inputs/S1.c
  test/Import/multiple-forward-declarations/Inputs/S2.c
  test/Import/multiple-forward-declarations/test.c
  test/Import/overloaded-function/Inputs/F1.c
  test/Import/overloaded-function/Inputs/F2.c
  test/Import/overloaded-function/test.c
  test/Import/struct-in-namespace/Inputs/N1.cpp
  test/Import/struct-in-namespace/Inputs/N2.cpp
  test/Import/struct-in-namespace/Inputs/N3.cpp
  test/Import/struct-in-namespace/test.cpp
  test/Import/template-specialization/Inputs/T.cpp
  test/Import/template-specialization/test.cpp
  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
@@ -9,6 +9,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
@@ -189,19 +190,77 @@
 } // end namespace
 
 namespace {
+class TestASTImporter : public ASTImporter {
+public:
+  TestASTImporter(ASTContext , FileManager ,
+  ASTContext , FileManager ,
+  bool MinimalImport)
+  : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
+MinimalImport) {}
+  Decl *Imported(Decl *From, Decl *To) override {
+if (auto ToTag = llvm::dyn_cast(To)) {
+  ToTag->setHasExternalLexicalStorage();
+} else if (auto ToNamespace = llvm::dyn_cast(To)) {
+  ToNamespace->setHasExternalVisibleStorage();
+}
+return ASTImporter::Imported(From, To);
+  }
+};
+
+const DeclContext *LookupSameContext(const TranslationUnitDecl *SearchTU,
+ const DeclContext *SourceDC,
+ ASTImporter ) {
+  if (SourceDC->isTranslationUnit()) {
+return SearchTU;
+  }
+  const DeclContext *SearchParentDC =
+  LookupSameContext(SearchTU, SourceDC->getParent(), SourceToSearchImporter);
+  if (!SearchParentDC) {
+// If we couldn't find the parent DC in this TranslationUnit, give up.
+return nullptr;
+  }
+  auto SourceNamedDecl = cast(SourceDC);
+  DeclarationName SourceName = SourceNamedDecl->getDeclName();
+  DeclarationName SearchName = SourceToSearchImporter.Import(SourceName);
+  DeclContext::lookup_result SearchResult = SearchParentDC->lookup(SearchName);
+  size_t SearchResultSize = SearchResult.size();
+  // Handle multiple candidates once we have a test for it.
+  // This may turn up when we import template specializations correctly.
+  assert(SearchResultSize < 2); 
+  if (SearchResultSize == 0) {
+// couldn't find the name, so we have to give up
+return nullptr;
+  } else {
+NamedDecl *SearchResultDecl = SearchResult[0];
+return dyn_cast(SearchResultDecl);
+  }
+}
+
+bool IsForwardDeclaration(Decl *D) {
+  assert(!isa(D)); // TODO handle this case
+  if (auto TD = dyn_cast(D)) {
+return !TD->isThisDeclarationADefinition();
+  } else if (auto FD = dyn_cast(D)) {
+return !FD->isThisDeclarationADefinition();
+  } else {
+return false;
+  }
+}
+
 class TestExternalASTSource : public ExternalASTSource {
 private:
   llvm::ArrayRef ImportCIs;
-  std::map> ForwardImporters;
+  std::map>
+  ForwardImporters;
   std::map> ReverseImporters;
 
 public:
   TestExternalASTSource(
   CompilerInstance ,
   llvm::ArrayRef ImportCIs)
   : ImportCIs(ImportCIs) {
 for (const std::unique_ptr  : ImportCIs) {
-  ForwardImporters[ImportCI.get()] = llvm::make_unique(
+  ForwardImporters[ImportCI.get()] = llvm::make_unique(
   ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
   ImportCI->getASTContext(), ImportCI->getFileManager(),
   /*MinimalImport=*/true);
@@ -212,29 +271,88 @@
 }
   }
 
+  void ForEachMatchingDC(const DeclContext *DC,
+ std::function

[PATCH] D30435: [clang-import-test] Lookup inside entities

[Sean Callanan via Phabricator via cfe-commits]

spyffe created this revision.
Herald added a subscriber: jgosnell.

clang-import-test has until now been only able to report top-level entities.  
This is clearly insufficient; we should be able to look inside structs and 
namespaces also.
This patch adds new test cases for a variety of lookups inside existing 
entities, and adds the functionality necessary to make most of these testcases 
work.
One testcase is known to fail because of ASTImporter limitations when importing 
templates; this will be addressed separately.


Repository:
  rL LLVM

https://reviews.llvm.org/D30435

Files:
  test/Import/forward-declared-struct/Inputs/S1.c
  test/Import/forward-declared-struct/Inputs/S2.c
  test/Import/forward-declared-struct/test.c
  test/Import/member-in-struct/Inputs/S.c
  test/Import/member-in-struct/test.c
  test/Import/multiple-forward-declarations/Inputs/S1.c
  test/Import/multiple-forward-declarations/Inputs/S2.c
  test/Import/multiple-forward-declarations/test.c
  test/Import/overloaded-function/Inputs/F1.c
  test/Import/overloaded-function/Inputs/F2.c
  test/Import/overloaded-function/test.c
  test/Import/struct-in-namespace/Inputs/N1.cpp
  test/Import/struct-in-namespace/Inputs/N2.cpp
  test/Import/struct-in-namespace/test.cpp
  test/Import/template-specialization/Inputs/T.cpp
  test/Import/template-specialization/test.cpp
  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
@@ -9,6 +9,7 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTImporter.h"
+#include "clang/AST/DeclObjC.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/SourceLocation.h"
@@ -189,19 +190,80 @@
 } // end namespace
 
 namespace {
+class TestASTImporter : public ASTImporter {
+public:
+  TestASTImporter(ASTContext , FileManager ,
+  ASTContext , FileManager ,
+  bool MinimalImport)
+  : ASTImporter(ToContext, ToFileManager, FromContext, FromFileManager,
+MinimalImport) {}
+  Decl *Imported(Decl *From, Decl *To) override {
+if (auto ToTag = llvm::dyn_cast(To)) {
+  ToTag->setHasExternalLexicalStorage();
+} else if (auto ToNamespace = llvm::dyn_cast(To)) {
+  ToNamespace->setHasExternalVisibleStorage();
+}
+return ASTImporter::Imported(From, To);
+  }
+};
+
+const DeclContext *LookupSameContext(const TranslationUnitDecl *SearchTU,
+ const DeclContext *SourceDC,
+ ASTImporter ) {
+  if (SourceDC->isTranslationUnit()) {
+return SearchTU;
+  }
+  const DeclContext *SearchParentDC =
+  LookupSameContext(SearchTU, SourceDC->getParent(), SourceToSearchImporter);
+  if (!SearchParentDC) {
+return nullptr;
+  }
+  if (auto SourceNamedDecl = dyn_cast(SourceDC)) {
+DeclarationName SourceName = SourceNamedDecl->getDeclName();
+DeclarationName SearchName = SourceToSearchImporter.Import(SourceName);
+DeclContext::lookup_result SearchResult =
+SearchParentDC->lookup(SearchName);
+size_t SearchResultSize = SearchResult.size();
+if (SearchResultSize == 0) {
+  return nullptr; // couldn't find the name, so we have to give up
+} else if (SearchResultSize > 1) {
+  return nullptr; // unhandled case; here we need to perform overload
+  // resolution
+} else {
+  NamedDecl *SearchResultDecl = SearchResult[0];
+  return dyn_cast(SearchResultDecl);
+}
+  } else {
+return nullptr; // unhandled case; here we need to handle e.g.
+// NamespaceDecls
+  }
+}
+
+bool IsForwardDeclaration(Decl *D) {
+  assert(!isa(D)); // TODO handle this case
+  if (auto TD = dyn_cast(D)) {
+return !TD->isThisDeclarationADefinition();
+  } else if (auto FD = dyn_cast(D)) {
+return !FD->isThisDeclarationADefinition();
+  } else {
+return false;
+  }
+}
+
 class TestExternalASTSource : public ExternalASTSource {
 private:
   llvm::ArrayRef ImportCIs;
-  std::map> ForwardImporters;
+  std::map>
+  ForwardImporters;
   std::map> ReverseImporters;
 
 public:
   TestExternalASTSource(
   CompilerInstance ,
   llvm::ArrayRef ImportCIs)
   : ImportCIs(ImportCIs) {
 for (const std::unique_ptr  : ImportCIs) {
-  ForwardImporters[ImportCI.get()] = llvm::make_unique(
+  ForwardImporters[ImportCI.get()] = llvm::make_unique(
   ExpressionCI.getASTContext(), ExpressionCI.getFileManager(),
   ImportCI->getASTContext(), ImportCI->getFileManager(),
   /*MinimalImport=*/true);
@@ -212,29 +274,88 @@
 }
   }
 
+  void ForEachMatchingDC(const DeclContext *DC,
+ std::function

[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

[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 = 

[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] D27683: Prepare PrettyStackTrace for LLDB adoption

[Sean Callanan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL289689: Prepare PrettyStackTrace for LLDB adoption (authored 
by spyffe).

Changed prior to commit:
  https://reviews.llvm.org/D27683?vs=81304=81426#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D27683

Files:
  llvm/trunk/include/llvm/Support/PrettyStackTrace.h
  llvm/trunk/lib/Support/PrettyStackTrace.cpp


Index: llvm/trunk/include/llvm/Support/PrettyStackTrace.h
===
--- llvm/trunk/include/llvm/Support/PrettyStackTrace.h
+++ llvm/trunk/include/llvm/Support/PrettyStackTrace.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_SUPPORT_PRETTYSTACKTRACE_H
 #define LLVM_SUPPORT_PRETTYSTACKTRACE_H
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 
 namespace llvm {
@@ -55,6 +56,16 @@
 void print(raw_ostream ) const override;
   };
 
+  /// PrettyStackTraceFormat - This object prints a string (which may use
+  /// printf-style formatting but should not contain newlines) to the stream
+  /// as the stack trace when a crash occurs.
+  class PrettyStackTraceFormat : public PrettyStackTraceEntry {
+llvm::SmallVector Str;
+  public:
+PrettyStackTraceFormat(const char *Format, ...);
+void print(raw_ostream ) const override;
+  };
+
   /// PrettyStackTraceProgram - This object prints a specified program 
arguments
   /// to the stream as the stack trace when a crash occurs.
   class PrettyStackTraceProgram : public PrettyStackTraceEntry {
Index: llvm/trunk/lib/Support/PrettyStackTrace.cpp
===
--- llvm/trunk/lib/Support/PrettyStackTrace.cpp
+++ llvm/trunk/lib/Support/PrettyStackTrace.cpp
@@ -88,12 +88,12 @@
 __attribute__((section("__DATA," CRASHREPORTER_ANNOTATIONS_SECTION))) 
 = { CRASHREPORTER_ANNOTATIONS_VERSION, 0, 0, 0, 0, 0, 0 };
 }
-#elif defined (__APPLE__) && HAVE_CRASHREPORTER_INFO
-static const char *__crashreporter_info__ = 0;
+#elif defined(__APPLE__) && HAVE_CRASHREPORTER_INFO
+extern "C" const char *__crashreporter_info__
+__attribute__((visibility("hidden"))) = 0;
 asm(".desc ___crashreporter_info__, 0x10");
 #endif
 
-
 /// CrashHandler - This callback is run if a fatal signal is delivered to the
 /// process, it prints the pretty stack trace.
 static void CrashHandler(void *) {
@@ -141,10 +141,26 @@
 #endif
 }
 
-void PrettyStackTraceString::print(raw_ostream ) const {
-  OS << Str << "\n";
+void PrettyStackTraceString::print(raw_ostream ) const { OS << Str << "\n"; 
}
+
+PrettyStackTraceFormat::PrettyStackTraceFormat(const char *Format, ...) {
+  va_list AP;
+  va_start(AP, Format);
+  const int SizeOrError = vsnprintf(nullptr, 0, Format, AP);
+  va_end(AP);
+  if (SizeOrError < 0) {
+return;
+  }
+
+  const int Size = SizeOrError + 1; // '\0'
+  Str.resize(Size);
+  va_start(AP, Format);
+  vsnprintf(Str.data(), Size, Format, AP);
+  va_end(AP);
 }
 
+void PrettyStackTraceFormat::print(raw_ostream ) const { OS << Str << "\n"; 
}
+
 void PrettyStackTraceProgram::print(raw_ostream ) const {
   OS << "Program arguments: ";
   // Print the argument list.


Index: llvm/trunk/include/llvm/Support/PrettyStackTrace.h
===
--- llvm/trunk/include/llvm/Support/PrettyStackTrace.h
+++ llvm/trunk/include/llvm/Support/PrettyStackTrace.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_SUPPORT_PRETTYSTACKTRACE_H
 #define LLVM_SUPPORT_PRETTYSTACKTRACE_H
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 
 namespace llvm {
@@ -55,6 +56,16 @@
 void print(raw_ostream ) const override;
   };
 
+  /// PrettyStackTraceFormat - This object prints a string (which may use
+  /// printf-style formatting but should not contain newlines) to the stream
+  /// as the stack trace when a crash occurs.
+  class PrettyStackTraceFormat : public PrettyStackTraceEntry {
+llvm::SmallVector Str;
+  public:
+PrettyStackTraceFormat(const char *Format, ...);
+void print(raw_ostream ) const override;
+  };
+
   /// PrettyStackTraceProgram - This object prints a specified program arguments
   /// to the stream as the stack trace when a crash occurs.
   class PrettyStackTraceProgram : public PrettyStackTraceEntry {
Index: llvm/trunk/lib/Support/PrettyStackTrace.cpp
===
--- llvm/trunk/lib/Support/PrettyStackTrace.cpp
+++ llvm/trunk/lib/Support/PrettyStackTrace.cpp
@@ -88,12 +88,12 @@
 __attribute__((section("__DATA," CRASHREPORTER_ANNOTATIONS_SECTION))) 
 = { CRASHREPORTER_ANNOTATIONS_VERSION, 0, 0, 0, 0, 0, 0 };
 }
-#elif defined (__APPLE__) && HAVE_CRASHREPORTER_INFO
-static const char *__crashreporter_info__ = 0;
+#elif defined(__APPLE__) && HAVE_CRASHREPORTER_INFO
+extern "C" const char *__crashreporter_info__
+__attribute__((visibility("hidden"))) = 0;
 asm(".desc 

[PATCH] D27683: Prepare PrettyStackTrace for LLDB adoption

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

The LLDB side of this is https://reviews.llvm.org/D27735


Repository:
  rL LLVM

https://reviews.llvm.org/D27683



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

[PATCH] D27683: Prepare PrettyStackTrace for LLDB adoption

[Sean Callanan via Phabricator via cfe-commits]

spyffe retitled this revision from "Fix the linkage for __crashtracer_info__" 
to "Prepare PrettyStackTrace for LLDB adoption".
spyffe updated the summary for this revision.
spyffe updated this revision to Diff 81304.

Repository:
  rL LLVM

https://reviews.llvm.org/D27683

Files:
  include/llvm/Support/PrettyStackTrace.h
  lib/Support/PrettyStackTrace.cpp


Index: lib/Support/PrettyStackTrace.cpp
===
--- lib/Support/PrettyStackTrace.cpp
+++ lib/Support/PrettyStackTrace.cpp
@@ -89,7 +89,7 @@
 = { CRASHREPORTER_ANNOTATIONS_VERSION, 0, 0, 0, 0, 0, 0 };
 }
 #elif defined (__APPLE__) && HAVE_CRASHREPORTER_INFO
-static const char *__crashreporter_info__ = 0;
+extern "C" const char *__crashreporter_info__ 
__attribute__((visibility("hidden"))) = 0;
 asm(".desc ___crashreporter_info__, 0x10");
 #endif
 
@@ -145,6 +145,28 @@
   OS << Str << "\n";
 }
 
+PrettyStackTraceFormat::PrettyStackTraceFormat(const char *format, ...) {
+  va_list ap;
+
+  va_start(ap, format);
+  const int size_or_error = vsnprintf(nullptr, 0, format, ap);
+  va_end(ap);
+
+  if (size_or_error < 0) {
+return;
+  }
+
+  const int size = size_or_error + 1; // '\0'
+
+  Str.resize(size);
+
+  va_start(ap, format);
+  vsnprintf(Str.data(), size, format, ap);
+  va_end(ap);
+}
+
+void PrettyStackTraceFormat::print(raw_ostream ) const { OS << Str << "\n"; 
}
+
 void PrettyStackTraceProgram::print(raw_ostream ) const {
   OS << "Program arguments: ";
   // Print the argument list.
Index: include/llvm/Support/PrettyStackTrace.h
===
--- include/llvm/Support/PrettyStackTrace.h
+++ include/llvm/Support/PrettyStackTrace.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_SUPPORT_PRETTYSTACKTRACE_H
 #define LLVM_SUPPORT_PRETTYSTACKTRACE_H
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 
 namespace llvm {
@@ -55,6 +56,16 @@
 void print(raw_ostream ) const override;
   };
 
+  /// PrettyStackTraceFormat - This object prints a string (which may use
+  /// printf-style formatting but should not contain newlines) to the stream
+  /// as the stack trace when a crash occurs.
+  class PrettyStackTraceFormat : public PrettyStackTraceEntry {
+llvm::SmallVector Str;
+  public:
+PrettyStackTraceFormat(const char *format, ...);
+void print(raw_ostream ) const override;
+  };
+
   /// PrettyStackTraceProgram - This object prints a specified program 
arguments
   /// to the stream as the stack trace when a crash occurs.
   class PrettyStackTraceProgram : public PrettyStackTraceEntry {


Index: lib/Support/PrettyStackTrace.cpp
===
--- lib/Support/PrettyStackTrace.cpp
+++ lib/Support/PrettyStackTrace.cpp
@@ -89,7 +89,7 @@
 = { CRASHREPORTER_ANNOTATIONS_VERSION, 0, 0, 0, 0, 0, 0 };
 }
 #elif defined (__APPLE__) && HAVE_CRASHREPORTER_INFO
-static const char *__crashreporter_info__ = 0;
+extern "C" const char *__crashreporter_info__ __attribute__((visibility("hidden"))) = 0;
 asm(".desc ___crashreporter_info__, 0x10");
 #endif
 
@@ -145,6 +145,28 @@
   OS << Str << "\n";
 }
 
+PrettyStackTraceFormat::PrettyStackTraceFormat(const char *format, ...) {
+  va_list ap;
+
+  va_start(ap, format);
+  const int size_or_error = vsnprintf(nullptr, 0, format, ap);
+  va_end(ap);
+
+  if (size_or_error < 0) {
+return;
+  }
+
+  const int size = size_or_error + 1; // '\0'
+
+  Str.resize(size);
+
+  va_start(ap, format);
+  vsnprintf(Str.data(), size, format, ap);
+  va_end(ap);
+}
+
+void PrettyStackTraceFormat::print(raw_ostream ) const { OS << Str << "\n"; }
+
 void PrettyStackTraceProgram::print(raw_ostream ) const {
   OS << "Program arguments: ";
   // Print the argument list.
Index: include/llvm/Support/PrettyStackTrace.h
===
--- include/llvm/Support/PrettyStackTrace.h
+++ include/llvm/Support/PrettyStackTrace.h
@@ -16,6 +16,7 @@
 #ifndef LLVM_SUPPORT_PRETTYSTACKTRACE_H
 #define LLVM_SUPPORT_PRETTYSTACKTRACE_H
 
+#include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Compiler.h"
 
 namespace llvm {
@@ -55,6 +56,16 @@
 void print(raw_ostream ) const override;
   };
 
+  /// PrettyStackTraceFormat - This object prints a string (which may use
+  /// printf-style formatting but should not contain newlines) to the stream
+  /// as the stack trace when a crash occurs.
+  class PrettyStackTraceFormat : public PrettyStackTraceEntry {
+llvm::SmallVector Str;
+  public:
+PrettyStackTraceFormat(const char *format, ...);
+void print(raw_ostream ) const override;
+  };
+
   /// PrettyStackTraceProgram - This object prints a specified program arguments
   /// to the stream as the stack trace when a crash occurs.
   class PrettyStackTraceProgram : public PrettyStackTraceEntry {
___
cfe-commits mailing list

[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

[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] D27033: [ASTImporter] Support importing UnresolvedLookupExpr nodes

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

Marked the place I was talking about in D27181 




Comment at: lib/AST/ASTImporter.cpp:6489
+  DeclarationNameInfo NameInfo(E->getName(), E->getNameLoc());
+  ImportDeclarationNameLoc(E->getNameInfo(), NameInfo);
+

Commented in [[ https://reviews.llvm.org/D27181 | D27181 ]] that usage of 
`ImportDeclarationNameLoc` look different in different places.


https://reviews.llvm.org/D27033



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

[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

Looks good, but I have a concern about the underlying branch's apparently 
inconsistent initialization of `DeclarationNameInfo`.  Aleksei, could you 
clarify how that code works?  Should we have a helper function so we don't have 
to carefully repeat this pattern everywhere?




Comment at: lib/AST/ASTImporter.cpp:4305
+  DeclarationNameInfo NameInfo(Name, 
Importer.Import(D->getNameInfo().getLoc()));
+  ImportDeclarationNameLoc(D->getNameInfo(), NameInfo);
+

I've seen this pattern before, in [[ https://reviews.llvm.org/D27033 | D20733 
]], at ASTImporter.cpp:6488:
```
  DeclarationNameInfo NameInfo(E->getName(), E->getNameLoc());
  ImportDeclarationNameLoc(E->getNameInfo(), NameInfo);
```
That code didn't do the `Import` during the initialization of the 
`DeclarationNameInfo`.  Is either of these incorrect?


https://reviews.llvm.org/D27181



___
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:
+  

[PATCH] D26753: ASTImporter: improve support for C++ templates

[Sean Callanan via Phabricator via cfe-commits]

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

Yeah, that test looks great!  Thanks!




Comment at: lib/AST/ASTImporter.cpp:496
+return false;
+  if (DN1->isIdentifier())
+return IsStructurallyEquivalent(DN1->getIdentifier(),

spyffe wrote:
> We should probably also check whether `DN1->isIdentifier() == 
> DN2->isIdentifier()`.
Looking at my comment with fresh post Thanksgiving eyes, that would be totally 
wrong.  The `IsStructurallyEquivalent` is fine.



Comment at: 
test/ASTMerge/class-template-partial-spec/Inputs/class-template-partial-spec2.cpp:67
+template
+struct Child1: public Two::Three::Parent {
+  char member;

ooh, nice!


https://reviews.llvm.org/D26753



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

[PATCH] D27033: [ASTImporter] Support importing UnresolvedLookupExpr nodes

[Sean Callanan via Phabricator via cfe-commits]

spyffe added a comment.

I only have a stylistic nit to add to Aleksei's comments.




Comment at: lib/AST/ASTImporter.cpp:6492
+  UnresolvedSet<8> ToDecls;
+  for (UnresolvedLookupExpr::decls_iterator S = E->decls_begin(),
+F = E->decls_end();

a.sidorin wrote:
> `auto` will look nice here.
Alternatively,
```
for (Decl *D : E->decls())
```


https://reviews.llvm.org/D27033



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

[PATCH] D26904: [astimporter] Support importing CXXDependentScopeMemberExpr and FunctionTemplateDecl

[Sean Callanan via Phabricator via cfe-commits]

spyffe requested changes to this revision.
spyffe added a comment.
This revision now requires changes to proceed.

There are several missing imports here, as well as a few minor nits.
If the unit test cases aren't catching these, I'm a little concerned.  We 
should be catching this.
Also we should definitely test that bodies of function templates (in 
particular, bodies that use the template arguments) get imported properly.




Comment at: lib/AST/ASTImporter.cpp:2327
+void ASTNodeImporter::ImportAttributes(Decl *From, Decl *To) {
+  for (Decl::attr_iterator I = From->attr_begin(), E = From->attr_end(); I != 
E;
+   ++I) {

Would
```
for (Attr *A : From->atrs()) {
```
work in this case?



Comment at: lib/AST/ASTImporter.cpp:3564
+
+  FunctionTemplateDecl *ToFunc = FunctionTemplateDecl::Create(
+  Importer.getToContext(), DC, Loc, Name, Params, TemplatedFD);

You didn't import `TemplatedFD` before installing it in `ToFunc`.  This code is 
broken, and we should make sure the unit tests know to catch these cases.



Comment at: lib/AST/ASTImporter.cpp:6482
+  return CXXDependentScopeMemberExpr::Create(Importer.getToContext(),
+ Base, BaseType,
+ E->isArrow(),

You're installing `Base` and `BaseType` without importing them, as well as all 
the `Loc`s.


https://reviews.llvm.org/D26904



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