[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

In D75665#2084889 , @thakis wrote:

> This breaks check-clang on mac: http://45.33.8.238/mac/15258/step_7.txt
>
> Please take a look, and revert for now if it takes a while to fix.


Thanks for reporting this, i think the output is valuable for debugging. I have 
restricted the test case to linux for now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks check-clang on mac: http://45.33.8.238/mac/15258/step_7.txt

Please take a look, and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG020815fafd15: [analyzer] On-demand parsing capability for 
CTU (authored by gamesh411).

Changed prior to commit:
  https://reviews.llvm.org/D75665?vs=269677=269749#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,98 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsWrongFromatError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsWrongFromatError =
+Err.getCode() == index_error_code::invocation_list_wrong_format;
+  });
+  EXPECT_TRUE(IsWrongFromatError);
+}
+
+TEST(CrossTranslationUnit, AmbiguousInvocationListIsDetected) {
+  // The same source file occurs twice (for two different architecture) in
+  // this test case. The disambiguation is the responsibility of the user.
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- -c
+- -m32
+- -o
+- main32.o
+- /tmp/main.cpp
+  /tmp/main.cpp:
+- clang++
+- -c
+- -m64
+- -o
+- main64.o
+- /tmp/main.cpp
+  )";
+
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsAmbiguousError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsAmbiguousError =
+Err.getCode() == index_error_code::invocation_list_ambiguous;
+  });
+  EXPECT_TRUE(IsAmbiguousError);
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 269677.
gamesh411 added a comment.

add ambiguity checking during invocation list parsing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,98 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsWrongFromatError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsWrongFromatError =
+Err.getCode() == index_error_code::invocation_list_wrong_format;
+  });
+  EXPECT_TRUE(IsWrongFromatError);
+}
+
+TEST(CrossTranslationUnit, AmbiguousInvocationListIsDetected) {
+  // The same source file occurs twice (for two different architecture) in
+  // this test case. The disambiguation is the responsibility of the user.
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- -c
+- -m32
+- -o
+- main32.o
+- /tmp/main.cpp
+  /tmp/main.cpp:
+- clang++
+- -c
+- -m64
+- -o
+- main64.o
+- /tmp/main.cpp
+  )";
+
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsAmbiguousError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsAmbiguousError =
+Err.getCode() == index_error_code::invocation_list_ambiguous;
+  });
+  EXPECT_TRUE(IsAmbiguousError);
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- /tmp/main.cpp
+  /tmp/other.cpp:
+- g++
+- -c
+- -o
+- /tmp/other.o
+- /tmp/other.cpp
+  )";
+  llvm::Expected 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 269644.
gamesh411 added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,98 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsWrongFromatError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsWrongFromatError =
+Err.getCode() == index_error_code::invocation_list_wrong_format;
+  });
+  EXPECT_TRUE(IsWrongFromatError);
+}
+
+TEST(CrossTranslationUnit, AmbiguousInvocationListIsDetected) {
+  // The same source file occurs twice (for two different architecture) in
+  // this test case. The disambiguation is the responsibility of the user.
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- -c
+- -m32
+- -o
+- main32.o
+- /tmp/main.cpp
+  /tmp/main.cpp:
+- clang++
+- -c
+- -m64
+- -o
+- main64.o
+- /tmp/main.cpp
+  )";
+
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsAmbiguousError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsAmbiguousError =
+Err.getCode() == index_error_code::invocation_list_ambiguous;
+  });
+  EXPECT_TRUE(IsAmbiguousError);
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- /tmp/main.cpp
+  /tmp/other.cpp:
+- g++
+- -c
+- -o
+- /tmp/other.o
+- /tmp/other.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 269639.
gamesh411 added a comment.

add ambiguous invocation list test case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,96 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsWrongFromatError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsWrongFromatError =
+Err.getCode() == index_error_code::invocation_list_wrong_format;
+  });
+  EXPECT_TRUE(IsWrongFromatError);
+}
+
+TEST(CrossTranslationUnit, AmbiguousInvocationListIsDetected) {
+  // The same source file occurs twice (for two different architecture) in
+  // this test case. The disambiguation is responsibility of the user.
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- -m32
+- -o
+- main32.o
+- /tmp/main.cpp
+  /tmp/main.cpp:
+- clang++
+- -m64
+- -o
+- main64.o
+- /tmp/main.cpp
+  )";
+
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  bool IsAmbiguousError = false;
+  llvm::handleAllErrors(Result.takeError(), [&](IndexError ) {
+IsAmbiguousError =
+Err.getCode() == index_error_code::invocation_list_ambiguous;
+  });
+  EXPECT_TRUE(IsAmbiguousError);
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- /tmp/main.cpp
+  /tmp/other.cpp:
+- g++
+- -c
+- -o
+- /tmp/other.o
+- /tmp/other.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 269615.
gamesh411 added a comment.

use consumeError in test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,63 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+  consumeError(Result.takeError());
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- /tmp/main.cpp
+  /tmp/other.cpp:
+- g++
+- -c
+- -o
+- /tmp/other.o
+- /tmp/other.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 2);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getKey(), "/tmp/main.cpp");
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/other.o");
+  EXPECT_EQ(It->getValue()[2], "/tmp/main.cpp");
+
+  It = Result->find("/tmp/other.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "g++");
+  EXPECT_EQ(It->getValue()[1], "-c");
+  EXPECT_EQ(It->getValue()[2], "-o");
+  EXPECT_EQ(It->getValue()[3], "/tmp/other.o");
+  EXPECT_EQ(It->getValue()[4], "/tmp/other.cpp");
 }
 
 } // end namespace cross_tu
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:119-120
+  return "Invocation list file contains multiple references to the same "
+ "source"
+ " file.";
+case index_error_code::invocation_list_file_not_found:

These two lines could be formatted together.



Comment at: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp:233
+  EXPECT_EQ(It->getValue()[4], "/tmp/other.cpp");
 }
 

Just for good measure, I would like a test case on the "invocation list is 
ambiguous" error. (So in case we in the future figure out how to disambiguate, 
we can remove that test case!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 269527.
gamesh411 added a comment.

Fix test case, and reorder warning


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,62 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result.takeError()));
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- /tmp/main.cpp
+  /tmp/other.cpp:
+- g++
+- -c
+- -o
+- /tmp/other.o
+- /tmp/other.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 2);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getKey(), "/tmp/main.cpp");
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/other.o");
+  EXPECT_EQ(It->getValue()[2], "/tmp/main.cpp");
+
+  It = Result->find("/tmp/other.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "g++");
+  EXPECT_EQ(It->getValue()[1], "-c");
+  EXPECT_EQ(It->getValue()[2], "-o");
+  EXPECT_EQ(It->getValue()[3], "/tmp/other.o");
+  EXPECT_EQ(It->getValue()[4], "/tmp/other.cpp");
 }
 
 } // end namespace cross_tu
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 269439.
gamesh411 added a comment.

Extend index file format
Update documentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,24 +174,62 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
+TEST(CrossTranslationUnit, EmptyInvocationListIsNotValid) {
+  auto Input = "";
 
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_FALSE(static_cast(Result));
+}
+
+TEST(CrossTranslationUnit, SingleInvocationCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/main.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 1);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/main.cpp");
+}
+
+TEST(CrossTranslationUnit, MultipleInvocationsCanBeParsed) {
+  auto Input = R"(
+  /tmp/main.cpp:
+- clang++
+- /tmp/other.o
+- /tmp/main.cpp
+  /tmp/other.cpp:
+- g++
+- -c
+- -o
+- /tmp/other.o
+- /tmp/other.cpp
+  )";
+  llvm::Expected Result = parseInvocationList(Input);
+  EXPECT_TRUE(static_cast(Result));
+
+  EXPECT_EQ(Result->size(), 2);
+
+  auto It = Result->find("/tmp/main.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getKey(), "/tmp/main.cpp");
+  EXPECT_EQ(It->getValue()[0], "clang++");
+  EXPECT_EQ(It->getValue()[1], "/tmp/other.o");
+  EXPECT_EQ(It->getValue()[2], "/tmp/main.cpp");
+
+  It = Result->find("/tmp/other.cpp");
+  EXPECT_TRUE(It != Result->end());
+  EXPECT_EQ(It->getValue()[0], "g++");
+  EXPECT_EQ(It->getValue()[1], "-c");
+  EXPECT_EQ(It->getValue()[2], "-o");
+  EXPECT_EQ(It->getValue()[3], "/tmp/other.o");
+  EXPECT_EQ(It->getValue()[4], "/tmp/other.cpp");
 }
 
 } // end namespace cross_tu
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 267143.
gamesh411 marked 6 inline comments as done.
gamesh411 added a comment.

Fix documentation and commit message


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,110 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Inputs
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/Inputs/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/Inputs/ctu-other.cpp
+//
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// compile_commands.json is only needed for the extdef_mapping, not for the analysis itself.
+// RUN: echo '[{"directory":"%t/Inputs","command":"clang++ ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/Inputs","command":"clang++ ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+//
+// RUN: echo '{"%t/Inputs/ctu-chain.cpp": ["g++", "%t/Inputs/ctu-chain.cpp"], 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

whisperity resigned from this revision.
whisperity added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:26
+#include "llvm/ADT/Triple.h"
+#include "llvm/Option/ArgList.h"
 #include "llvm/Support/ErrorHandling.h"

Duped include in L34.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked 2 inline comments as done.
gamesh411 added a comment.

The remaining documentation and test changes are also underway.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227
+/// Identifier.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

martong wrote:
> xazax.hun wrote:
> > martong wrote:
> > > xazax.hun wrote:
> > > > I am not sure if this is good design.
> > > > Here, if the meaning of the `Identifier` depends on the subclass, the 
> > > > caller of this method always needs to be aware of the dynamic type of 
> > > > the object. What is the point of a common base class if we always need 
> > > > to know the dynamic type?
> > > > 
> > > > Looking at the code this does not look bad. But it might be a code 
> > > > smell.
> > > The way how we process the `extDefMapping` file is identical in both 
> > > cases. That's an index, keyed with the `USR`s of functions and then we 
> > > get back a value. And the way how we use that value is different. In the 
> > > PCH case that holds the path for the `.ast` file, in the ODM case that is 
> > > the name of the source file which we must find in the compile db. So, I 
> > > think the process of getting the AST for a USR requires the polymorphic 
> > > behavior from the loaders.
> > > 
> > > We discussed other alternatives with Endre. We were thinking that maybe 
> > > the `extDefMapping` file should be identical in both cases. But then we 
> > > would need to add the `.ast` postfixes for the entries in the PCH case. 
> > > And we cannot just do that, because we may not know if what is the 
> > > correct postfix. The user may have generated `.pch` files instead. Also, 
> > > we don't want to compel any Clang user to use CodeChecker (CC will always 
> > > create `.ast` files). CTU should be running fine by manually executing 
> > > the independent steps.
> > Let me rephrase my concerns a bit. Do we really need a polymorphic 
> > `ASTLoader` to be present for the whole analysis? Wouldn't it make more 
> > sense to always do the same thing, i.e. if we are given a pch file load it, 
> > if we are given a source file, parse it? This way we would not be 
> > restricted to on-demand or two pass ctu analysis, but we could do any 
> > combination of the two.
> > 
> Well yeah, we could do that, it is a good idea, thanks! We will consider this 
> in the future. I like in this idea that the command line options to Clang 
> would be simplified. But then we must be transparent and show/log the user 
> which method we are using.
I will implement this separation by suffixes in the next change.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:285
   public:
-ASTUnitStorage(const CompilerInstance );
+ASTUnitStorage(CompilerInstance );
 /// Loads an ASTUnit for a function.

xazax.hun wrote:
> Why is this no longer const?
Most usages of the CI require a non-const ref (this is due to lacking API 
support for getting analyzer options), and I found it more convenient to just 
take a non-const ref, instead of const_casting it away later. Not sure myself, 
but I think I will roll with this now (and I should definitely check the API of 
CompilerInstance).



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:234
+  public:
+explicit ASTFileLoader(CompilerInstance , StringRef CTUDir);
+

whisperity wrote:
> Is the `explicit` needed here?
This constructor started out with having a single param only. I agree, that 
`explicit` is no longer necessary.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:260
+/// that produce the AST used for analysis.
+StringRef OnDemandParsingDatabase;
+

martong wrote:
> Should we rename this member to PathToInvocationList... ?
Renamed to `InvocationListFilePath`.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:122-123
   return "Load threshold reached";
+case index_error_code::ambiguous_compilation_database:
+  return "Compilation database contains multiple references to the same "
+ "source file.";

whisperity wrote:
> still using terms "compilation database", is this intended?
renamed



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:362
 
-  return ASTUnit::LoadFromASTFile(
-  std::string(ASTFilePath), CI.getPCHContainerOperations()->getRawReader(),
-  ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts());
+  auto LoadFromFile = [this](StringRef Path) {
+IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();

balazske wrote:
> Is here a lambda necessary? I think it must not be over-used. In this case we 
> can construct the file path as the first step and then use it to load the 
> file, without using lambda call.
Refactored to use path concatenation.

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 266473.
gamesh411 marked 29 inline comments as done.
gamesh411 added a comment.

Update functional changes, documentation update incoming


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,105 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Inputs
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/Inputs/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/Inputs/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/Inputs","command":"clang++ ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/Inputs","command":"clang++ ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: echo '{"%t/Inputs/ctu-chain.cpp": ["g++", "%t/Inputs/ctu-chain.cpp"], "%t/Inputs/ctu-other.cpp": ["g++", "%t/Inputs/ctu-other.cpp"]}' | sed -e 's/\\//g' > 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Endre, just checked the latest update. Overall looks good to me, but found 
some nits.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:260
+/// that produce the AST used for analysis.
+StringRef OnDemandParsingDatabase;
+

Should we rename this member to PathToInvocationList... ?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:581
+  SmallVector CommandLineArgs(InvocationCommand.size());
+  std::transform(InvocationCommand.begin(), InvocationCommand.end(),
+ CommandLineArgs.begin(),

Could we avoid this transfer if InvocationList was storing `const char *` 
values instead of std::strings? Why can't we store `char*`s in InvocationList?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:593
+CrossTranslationUnitContext::ASTOnDemandLoader::lazyInitCompileCommands() {
+  /// Lazily initialize the invocation list filed used for on-demand parsing.
+  if (InvocationList)

typo: filed -> field



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:6
+// Path substitutions on Windows platform could contain backslashes. These are 
escaped in the json file.
+// RUN: echo '[{"directory":"%t","command":"gcc -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: echo '"%t/ctu-other.c": ["gcc", "-std=c89", "-Wno-visibility", 
"ctu-other.c"]' | sed -e 's/\\//g' > %t/invocations.yaml

Perhaps we could document here that the compile_commands.json is needed ONLY 
for %clang_extdef_map.



Comment at: clang/test/Analysis/ctu-on-demand-parsing.cpp:9
+// RUN: echo '[{"directory":"%t/Inputs","command":"clang++ 
ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/Inputs","command":"clang++
 ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: echo '{"%t/Inputs/ctu-chain.cpp": ["g++", "%t/Inputs/ctu-chain.cpp"], 
"%t/Inputs/ctu-other.cpp": ["g++", "%t/Inputs/ctu-other.cpp"]}' | sed -e 
's/\\//g' > %t/invocations.yaml
+// RUN: cd "%t" && %clang_extdef_map Inputs/ctu-chain.cpp Inputs/ctu-other.cpp 
> externalDefMap.txt

I know this is just a nit, but this is very hard to read. Do you think we could 
break this up (and other long lines) into multiple lines  but within the same 
RUN directive?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

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

(Maybe this will make Phab not show "✅ Accepted"...)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Please check the summary of the patch, it seems to contain old information as 
well.




Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:210-212
+Preferably the same compilation database should be used when generating the 
external definitions, and
+during analysis.  The analysis invocation must be provided with the directory 
which contains the mapping
+files, and the compilation database which is used to determine compiler flags.

This is obsolete information.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:234
+  public:
+explicit ASTFileLoader(CompilerInstance , StringRef CTUDir);
+

Is the `explicit` needed here?



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:388-389
+"parsing there is no need for pre-dumping ASTs. External "
+"definition mapping is still needed, and a valid compilation "
+"database with compile commands for the external TUs is also "
+"necessary. Disabled by default.",

"valid compilation database" is this line still what we wish to say? A YAML is 
used, so most likely we need a "valid invocation list" or something.



Comment at: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def:393-398
+ANALYZER_OPTION(
+StringRef, CTUInvocationList, "ctu-invocation-list",
+"The path to the YAML format file containing a mapping from source file "
+"paths to command-line invocations represented as a list of arguments. "
+"This invocation is used produce the source-file's AST.",
+"invocations.yaml")

Maybe it would be worthwhile to add a dummy example like `main.cpp: g++ 
hello-world.cpp` which has the proper format into this help.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:23-24
 #include "clang/Index/USRGeneration.h"
-#include "llvm/ADT/Triple.h"
+#include "clang/Tooling/JSONCompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"

The idea here was to not depend on libtooling but these lines are still here!



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:34
 #include 
+#include 
 #include 

<> -> "" and order



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:122-123
   return "Load threshold reached";
+case index_error_code::ambiguous_compilation_database:
+  return "Compilation database contains multiple references to the same "
+ "source file.";

still using terms "compilation database", is this intended?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:583
+ CommandLineArgs.begin(),
+ [](auto &) { return CmdPart.c_str(); });
+

balazske wrote:
> Is here a special reason to use `auto &&` type (and not `auto &` or 
> `std::string &`)? Probably this code is faulty: The `CmdPart` is moved out 
> and then destructed, before the content of it (where the `c_str` points to) 
> is used?
@balazske This is a generic lambda where `auto&&` is the universal reference. 
It is equivalent to writing `template  f(T&& param)`. It obeys the 
usual point of instantiation and reference cascading rules, if a `const 
std::string&` is given as `T`, `const std::string& &&` will become a single `&`.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:609
+  llvm::SourceMgr SM;
+  llvm::yaml::Stream InvocationFiles(*ContentBuffer, SM);
+

Are the headers from which these types come from included?



Comment at: clang/test/Analysis/Inputs/ctu-other.c:34-38
+// TODO: Support the GNU extension asm keyword as well.
+// Example using the GNU extension: asm("mov $42, %0" : "=r"(res));
 int inlineAsm() {
   int res;
+  __asm__("mov $42, %0"

Possibly unrelated change?



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:1-15
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: cp "%s" "%t/ctu-on-demand-parsing.c"
+// RUN: cp "%S/Inputs/ctu-other.c" "%t/ctu-other.c"
+// Path substitutions on Windows platform could contain backslashes. These are 
escaped in the json file.
+// RUN: echo '[{"directory":"%t","command":"gcc -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: echo '"%t/ctu-other.c": ["gcc", "-std=c89", "-Wno-visibility", 
"ctu-other.c"]' | sed -e 's/\\//g' > %t/invocations.yaml

Does the LIT syntax allow for better organising these commands? This is now a 
bit hard to read


Something like this would help:
```
// RUN: rm -rf %t && mkdir -p %t
// RUN: cp "%s" "%t/ctu-on-demand-parsing.c" && cp "%S/Inputs/ctu-other.c" 
"%t/ctu-other.c"
// [empty]
// [comment]
// RUN: [json 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:362
 
-  return ASTUnit::LoadFromASTFile(
-  std::string(ASTFilePath), CI.getPCHContainerOperations()->getRawReader(),
-  ASTUnit::LoadEverything, Diags, CI.getFileSystemOpts());
+  auto LoadFromFile = [this](StringRef Path) {
+IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();

Is here a lambda necessary? I think it must not be over-used. In this case we 
can construct the file path as the first step and then use it to load the file, 
without using lambda call.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:583
+ CommandLineArgs.begin(),
+ [](auto &) { return CmdPart.c_str(); });
+

Is here a special reason to use `auto &&` type (and not `auto &` or 
`std::string &`)? Probably this code is faulty: The `CmdPart` is moved out and 
then destructed, before the content of it (where the `c_str` points to) is used?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 263294.
gamesh411 added a comment.

Implement a solution without a dependency on clangTooling


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,105 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/Inputs
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/Inputs/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/Inputs/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/Inputs","command":"clang++ ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/Inputs","command":"clang++ ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: echo '{"%t/Inputs/ctu-chain.cpp": ["g++", "%t/Inputs/ctu-chain.cpp"], "%t/Inputs/ctu-other.cpp": ["g++", "%t/Inputs/ctu-other.cpp"]}' | sed -e 's/\\//g' > %t/invocations.yaml
+// RUN: cd "%t" && %clang_extdef_map 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 260554.
gamesh411 added a comment.

Remove arch target from another invocation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: cd "%t" && 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

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



Comment at: clang/lib/CrossTU/CMakeLists.txt:13
   clangIndex
+  clangTooling
+  clangSerialization

thakis wrote:
> We've been very careful to make clang (the compiler binary) only depend on 
> clangToolingCore and not on the much bigger clangTooling library. Since clang 
> depends on the CrossTU library, this breaks that. Can you reorganize things 
> so that we don't need a dependency from clang-the-compiler-binary on 
> clangTooling?
I have gone, and investigated where the main functionality of on-demand AST 
loading comes from. (Only after wishfully thinking changing the dependency from 
`clangTooling` to `clangToolingCore` would work, which of course didnt't )

It seems that this functionality used the major components of `clangTooling`, 
namely `JSONCompilationDatabase::loadFromFile`, `ClangTool ctor/dtor`, 
`ClangTool::buildAST`.
In order to circumvent the dependency and the reimplementation of handling 
compilation databases, a call to an external tool could be used, or the 
clangTooling library would need to be refactored in a way which is not a very 
tasteful refactoring. IMHO. I would like to know which of the aforementioned 
way is deemed to be best.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 260310.
gamesh411 added a comment.

[NFC] Fix arcanist double commit revisioning


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: cd "%t" && 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 260308.
gamesh411 added a comment.

Remove platform constraint from test file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp


Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- clang/test/Analysis/ctu-on-demand-parsing.cpp
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -7,7 +7,7 @@
 // Path substitutions on Windows platform could contain backslashes. These are 
escaped in the json file.
 // RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c 
ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++
 -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
 // RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > 
externalDefMap.txt
-// RUN: cd "%t" && %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN: cd "%t" && %clang_analyze_cc1 \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
 // RUN:   -analyzer-config ctu-dir=ctudir \
Index: clang/test/Analysis/ctu-on-demand-parsing.c
===
--- clang/test/Analysis/ctu-on-demand-parsing.c
+++ clang/test/Analysis/ctu-on-demand-parsing.c
@@ -5,7 +5,7 @@
 // Path substitutions on Windows platform could contain backslashes. These are 
escaped in the json file.
 // RUN: echo '[{"directory":"%t","command":"gcc -c -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
 // RUN: cd "%t" && %clang_extdef_map ctu-other.c > externalDefMap.txt
-// RUN: cd "%t" && %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only 
-std=c89 -analyze \
+// RUN: cd "%t" && %clang_cc1 -fsyntax-only -std=c89 -analyze \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
 // RUN:   -analyzer-config ctu-dir=. \


Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- clang/test/Analysis/ctu-on-demand-parsing.cpp
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -7,7 +7,7 @@
 // Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
 // RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
 // RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
-// RUN: cd "%t" && %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN: cd "%t" && %clang_analyze_cc1 \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
 // RUN:   -analyzer-config ctu-dir=ctudir \
Index: clang/test/Analysis/ctu-on-demand-parsing.c
===
--- clang/test/Analysis/ctu-on-demand-parsing.c
+++ clang/test/Analysis/ctu-on-demand-parsing.c
@@ -5,7 +5,7 @@
 // Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
 // RUN: echo '[{"directory":"%t","command":"gcc -c -std=c89 -Wno-visibility ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > %t/compile_commands.json
 // RUN: cd "%t" && %clang_extdef_map ctu-other.c > externalDefMap.txt
-// RUN: cd "%t" && %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze \
+// RUN: cd "%t" && %clang_cc1 -fsyntax-only -std=c89 -analyze \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
 // RUN:   -analyzer-config ctu-dir=. \
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

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

Non-linux buildbots were utterly broken :( . Trying to fix them...

Thanks for reverting this for me :)
Also I will investigate the possibility to not depend on Tooling.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Reverted in 
https://github.com/llvm/llvm-project/commit/96717125e852d1c6ddf41c22dd2d556f4f5aa34d.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

Sorry, this change broke a couple bots that are in the official CI, at least 
http://lab.llvm.org:8011/builders/llvm-clang-win-x-aarch64/builds/7566 and 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/66618.
 I'm reverting this change -- feel free to reland after fixing the issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Looks like this also breaks tests on Windows and Mac, eg 
http://45.33.8.238/win/13902/step_7.txt

It probably makes sense to revert while you investigate?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/CrossTU/CMakeLists.txt:13
   clangIndex
+  clangTooling
+  clangSerialization

We've been very careful to make clang (the compiler binary) only depend on 
clangToolingCore and not on the much bigger clangTooling library. Since clang 
depends on the CrossTU library, this breaks that. Can you reorganize things so 
that we don't need a dependency from clang-the-compiler-binary on clangTooling?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 closed this revision.
gamesh411 added a comment.

I am landing this. Thanks for the reviews @martong @balazske @xazax.hun 
@whisperity !


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 260238.
gamesh411 added a comment.

Fix index error enum declaration


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: cd "%t" && %clang_analyze_cc1 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 260229.
gamesh411 added a comment.

Reword enum value


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

@whisperity Thanks for the review :3




Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:111-114
+  -Xclang -analyzer-config -Xclang 
experimental-enable-naive-ctu-analysis=true \
+  -Xclang -analyzer-config -Xclang ctu-dir=. \
+  -Xclang -analyzer-config -Xclang ctu-on-demand-parsing=false \
+  -Xclang -analyzer-output=plist-multi-file \

whisperity wrote:
> Are these flags documented somewhere?
As with ctu-dir, these flags are only documented in analyzer options. However, 
it would indeed be beneficial to list CTU related flags here in the user 
documentation. I will create a new revision for that specifically.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:331-335
+  [INFO 2019-07-16 17:21] - To view results in the terminal use the 
"CodeChecker parse" command.
+  [INFO 2019-07-16 17:21] - To store results use the "CodeChecker store" 
command.
+  [INFO 2019-07-16 17:21] - See --help and the user guide for further options 
about parsing and storing the reports.
+  [INFO 2019-07-16 17:21] - =
+  [INFO 2019-07-16 17:21] - Analysis length: 0.659618854523 sec.

whisperity wrote:
> These lines could be removed, I think, they don't add anything to the 
> presentation.
Good point, i remove it from here and from the above example as well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 260078.
gamesh411 marked 15 inline comments as done.
gamesh411 added a comment.

Fix issues in documentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:391
+Currently On-demand analysis is not supported with `scan-build-py`.
\ No newline at end of file


What's this line? Is this added by Phab, or is this a weird merge conflict / 
parsing error of the patch?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227
+/// Identifier.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

xazax.hun wrote:
> martong wrote:
> > xazax.hun wrote:
> > > I am not sure if this is good design.
> > > Here, if the meaning of the `Identifier` depends on the subclass, the 
> > > caller of this method always needs to be aware of the dynamic type of the 
> > > object. What is the point of a common base class if we always need to 
> > > know the dynamic type?
> > > 
> > > Looking at the code this does not look bad. But it might be a code smell.
> > The way how we process the `extDefMapping` file is identical in both cases. 
> > That's an index, keyed with the `USR`s of functions and then we get back a 
> > value. And the way how we use that value is different. In the PCH case that 
> > holds the path for the `.ast` file, in the ODM case that is the name of the 
> > source file which we must find in the compile db. So, I think the process 
> > of getting the AST for a USR requires the polymorphic behavior from the 
> > loaders.
> > 
> > We discussed other alternatives with Endre. We were thinking that maybe the 
> > `extDefMapping` file should be identical in both cases. But then we would 
> > need to add the `.ast` postfixes for the entries in the PCH case. And we 
> > cannot just do that, because we may not know if what is the correct 
> > postfix. The user may have generated `.pch` files instead. Also, we don't 
> > want to compel any Clang user to use CodeChecker (CC will always create 
> > `.ast` files). CTU should be running fine by manually executing the 
> > independent steps.
> Let me rephrase my concerns a bit. Do we really need a polymorphic 
> `ASTLoader` to be present for the whole analysis? Wouldn't it make more sense 
> to always do the same thing, i.e. if we are given a pch file load it, if we 
> are given a source file, parse it? This way we would not be restricted to 
> on-demand or two pass ctu analysis, but we could do any combination of the 
> two.
> 
Well yeah, we could do that, it is a good idea, thanks! We will consider this 
in the future. I like in this idea that the command line options to Clang would 
be simplified. But then we must be transparent and show/log the user which 
method we are using.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:21
+
+PCH based analysis
+__

I think it's PCH-based with a -.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:25
+These can be generated by the Clang Frontend itself, and must be arranged in a 
specific way in the filesystem.
+The index, which maps function USR names to PCH dumps containing them must 
also be generated by the
+`clang-extdef-mapping` clang tool. The external mapping creation implicitly 
uses a compilation command database to

symbols' USR names



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:26
+The index, which maps function USR names to PCH dumps containing them must 
also be generated by the
+`clang-extdef-mapping` clang tool. The external mapping creation implicitly 
uses a compilation command database to
+determine the compilation flags used.

martong wrote:
> Maybe just simply write `This tool uses a compilation command database ...`
> (Same below with the on-demand version.)
~~clang tool~~



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:26
+The index, which maps function USR names to PCH dumps containing them must 
also be generated by the
+`clang-extdef-mapping` clang tool. The external mapping creation implicitly 
uses a compilation command database to
+determine the compilation flags used.

whisperity wrote:
> martong wrote:
> > Maybe just simply write `This tool uses a compilation command database ...`
> > (Same below with the on-demand version.)
> ~~clang tool~~
And perhaps add some cross-references to what a compilation database is, etc. 
These things are also documented within Clang's doc tree.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:31
+
+[PCH] Manual CTU Analysis
+-

Instead of a prefix, create an overview of PCH-based analysis, and add this and 
the next one as a 3rd-level heading?



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:111-114
+  -Xclang -analyzer-config -Xclang 
experimental-enable-naive-ctu-analysis=true \
+  -Xclang -analyzer-config -Xclang ctu-dir=. \
+  -Xclang -analyzer-config -Xclang ctu-on-demand-parsing=false \
+  -Xclang -analyzer-output=plist-multi-file \

Are these flags documented somewhere?



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:227-228
+compilation database in order to determine the exact compiler invocation used 
for each TU.
+The index, which maps function USR names to source files containing them must 
also be generated by the
+`clang-extdef-mapping` clang tool. The mapping of external definitions 
implicitly uses a compilation command database to
+determine the compilation flags used. Preferably the same compilation database 
should be used when generating the

Same here for comments from above.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:331-335
+  [INFO 2019-07-16 17:21] - To view results in the terminal use the 
"CodeChecker parse" command.
+  [INFO 2019-07-16 17:21] - To store results use the "CodeChecker store" 
command.
+  [INFO 2019-07-16 17:21] - See --help and the user guide for further options 
about parsing and storing the reports.
+  [INFO 2019-07-16 17:21] - =
+  [INFO 2019-07-16 17:21] - Analysis length: 0.659618854523 sec.

These lines could be removed, I think, they don't add anything to the 
presentation.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:336-337
+  [INFO 2019-07-16 17:21] - Analysis length: 0.659618854523 sec.
+  $ ls
+  compile_commands.json  foo.cpp main.cpp  reports
+  $ tree reports

Due to the lack of colours, perhaps you could use `ls -F` which suffixes each 
directory with a `/`: `reports/`.



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:387
+-
+We actively develop CTU with CodeChecker as a "runner" script, `scan-build-py` 
is not actively developed for CTU.
+`scan-build-py` has various errors and issues, expect it to work with the very 
basic projects only.

as ~~a runner script~~ the driver for this feature



Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:388
+We actively develop CTU with CodeChecker as a "runner" script, `scan-build-py` 
is not actively developed for CTU.
+`scan-build-py` has various errors and issues, expect it to work with the very 
basic projects only.
+

expect it to work only ...

(to emphasise the limitation)



Comment at: 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun accepted this revision.
xazax.hun added a comment.

Ok, looks good to me.

The minor nit regarding the naming is easy to fix before commit. The design 
question I had is not a blocker, my suggested alternative can be implemented 
later (if desired) in a backward-compatible way from the user's point of view.




Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227
+/// Identifier.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

martong wrote:
> xazax.hun wrote:
> > I am not sure if this is good design.
> > Here, if the meaning of the `Identifier` depends on the subclass, the 
> > caller of this method always needs to be aware of the dynamic type of the 
> > object. What is the point of a common base class if we always need to know 
> > the dynamic type?
> > 
> > Looking at the code this does not look bad. But it might be a code smell.
> The way how we process the `extDefMapping` file is identical in both cases. 
> That's an index, keyed with the `USR`s of functions and then we get back a 
> value. And the way how we use that value is different. In the PCH case that 
> holds the path for the `.ast` file, in the ODM case that is the name of the 
> source file which we must find in the compile db. So, I think the process of 
> getting the AST for a USR requires the polymorphic behavior from the loaders.
> 
> We discussed other alternatives with Endre. We were thinking that maybe the 
> `extDefMapping` file should be identical in both cases. But then we would 
> need to add the `.ast` postfixes for the entries in the PCH case. And we 
> cannot just do that, because we may not know if what is the correct postfix. 
> The user may have generated `.pch` files instead. Also, we don't want to 
> compel any Clang user to use CodeChecker (CC will always create `.ast` 
> files). CTU should be running fine by manually executing the independent 
> steps.
Let me rephrase my concerns a bit. Do we really need a polymorphic `ASTLoader` 
to be present for the whole analysis? Wouldn't it make more sense to always do 
the same thing, i.e. if we are given a pch file load it, if we are given a 
source file, parse it? This way we would not be restricted to on-demand or two 
pass ctu analysis, but we could do any combination of the two.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:56
+  load_threshold_reached,
+  ambiguous_compile_commands_database
 };

xazax.hun wrote:
> The two enum values refer to compilation database and compile command 
> database. I'd prefer to use the same wording in both values.
+1



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227
+/// Identifier.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

xazax.hun wrote:
> I am not sure if this is good design.
> Here, if the meaning of the `Identifier` depends on the subclass, the caller 
> of this method always needs to be aware of the dynamic type of the object. 
> What is the point of a common base class if we always need to know the 
> dynamic type?
> 
> Looking at the code this does not look bad. But it might be a code smell.
The way how we process the `extDefMapping` file is identical in both cases. 
That's an index, keyed with the `USR`s of functions and then we get back a 
value. And the way how we use that value is different. In the PCH case that 
holds the path for the `.ast` file, in the ODM case that is the name of the 
source file which we must find in the compile db. So, I think the process of 
getting the AST for a USR requires the polymorphic behavior from the loaders.

We discussed other alternatives with Endre. We were thinking that maybe the 
`extDefMapping` file should be identical in both cases. But then we would need 
to add the `.ast` postfixes for the entries in the PCH case. And we cannot just 
do that, because we may not know if what is the correct postfix. The user may 
have generated `.pch` files instead. Also, we don't want to compel any Clang 
user to use CodeChecker (CC will always create `.ast` files). CTU should be 
running fine by manually executing the independent steps.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:120
+case index_error_code::ambiguous_compile_commands_database:
+  return "Compile commands database contains multiple references to the "
+ "same source file.";

xazax.hun wrote:
> Unfortunately, this is a very common case for a large number of projects that 
> supports multiple targets (e.g. 32 and 64 bits). Is there a plan to mitigate 
> this problem?
I don't think we could do that. We need to merge ASTs of those TUs that are 
linkable. Otherwise the merge will be unsuccessful because of certain 
mismatches in Decls (structural eq will fail).
Even in CodeChecker we are planning to issue a hard error in these ambiguous 
cases, even with the PCH method too. (The error would be suppressible by the 
user of course, but if that is suppressed then we take no responsibility.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:56
+  load_threshold_reached,
+  ambiguous_compile_commands_database
 };

The two enum values refer to compilation database and compile command database. 
I'd prefer to use the same wording in both values.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:227
+/// Identifier.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

I am not sure if this is good design.
Here, if the meaning of the `Identifier` depends on the subclass, the caller of 
this method always needs to be aware of the dynamic type of the object. What is 
the point of a common base class if we always need to know the dynamic type?

Looking at the code this does not look bad. But it might be a code smell.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:285
   public:
-ASTUnitStorage(const CompilerInstance );
+ASTUnitStorage(CompilerInstance );
 /// Loads an ASTUnit for a function.

Why is this no longer const?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:120
+case index_error_code::ambiguous_compile_commands_database:
+  return "Compile commands database contains multiple references to the "
+ "same source file.";

Unfortunately, this is a very common case for a large number of projects that 
supports multiple targets (e.g. 32 and 64 bits). Is there a plan to mitigate 
this problem?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

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

Thanks Endre for the docs! I checked the CI job also, seemed okay, so, I think 
we are ready! Nice work! Let's do the commit!




Comment at: clang/docs/analyzer/user-docs/CrossTranslationUnit.rst:26
+The index, which maps function USR names to PCH dumps containing them must 
also be generated by the
+`clang-extdef-mapping` clang tool. The external mapping creation implicitly 
uses a compilation command database to
+determine the compilation flags used.

Maybe just simply write `This tool uses a compilation command database ...`
(Same below with the on-demand version.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 258674.
gamesh411 added a comment.

Update user documentation


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/docs/analyzer/user-docs/CrossTranslationUnit.rst
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 258092.
gamesh411 added a comment.

Add ambiguous compilation database test case


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing-ambigous-compilation-database.c
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: cd "%t" && %clang_analyze_cc1 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 257985.
gamesh411 marked an inline comment as done.
gamesh411 added a comment.

Rebase and update the number of analyzer options


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: cd "%t" && %clang_analyze_cc1 -triple x86_64-pc-linux-gnu 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked 5 inline comments as done.
gamesh411 added inline comments.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:457
+return llvm::make_error(
+index_error_code::ambiguous_compile_commands_database);
+

martong wrote:
> Could we have a lit test for this case? I.e having a compile_commands.json 
> with two ambiguous entries for the same file and then we should expect the 
> compiler diag?
> (Note, I am asking this because it is not immediate for me from the code that 
> this will ever fire.)
Good idea, I will create a standalone test case for this!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 257896.
gamesh411 marked an inline comment as done.
gamesh411 added a comment.

Implement review suggestions


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,102 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %s %t/ctu-on-demand-parsing.cpp
+// RUN: cp %S/ctu-hdr.h %t/ctu-hdr.h
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: cp %S/Inputs/ctu-other.cpp %t/ctudir/ctu-other.cpp
+// Path substitutions on Windows platform could contain backslashes. These are escaped in the json file.
+// RUN: echo '[{"directory":"%t/ctudir","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%t/ctudir","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/compile_commands.json
+// RUN: cd "%t/ctudir" && %clang_extdef_map ctu-chain.cpp ctu-other.cpp > externalDefMap.txt
+// RUN: cd "%t" && %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked 7 inline comments as done.
gamesh411 added inline comments.



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:5
+// RUN: cp "%S/Inputs/ctu-other.c" "%t/ctu-other.c"
+// RUN: echo '[{"directory":"%t","command":"gcc -c -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: cd "%t" && %clang_extdef_map ctu-other.c > externalDefMap.txt

martong wrote:
> Why do we need the `-Wno-visibility` flag?
Needed because the in-expression definition of a struct (which we cannot 
import) also warrants a compiler warning, and this comes up 2 times even (once 
when generating the extdef-mapping, second time when we analyze the file). Its 
easier to handle this warning in the compilation database, as that removes both 
occurrences of the warning.



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:6
+// RUN: echo '[{"directory":"%t","command":"gcc -c -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: cd "%t" && %clang_extdef_map ctu-other.c > externalDefMap.txt
+// RUN: cd "%t" && %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only 
-std=c89 -analyze \

martong wrote:
> If we execute `clang_extdef_map` then we don't need 
> `Inputs/ctu-other.c[pp].externalDefMap.on-the-fly.txt` files. Do we?
Cannot remove as of yet, because they have .ast suffixed entries. In order to 
remove them the whole logic of what should be inside the index 
(externalDefMap.txt) for on-demand and non-on-demand cases should be discussed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 marked 23 inline comments as done.
gamesh411 added a comment.

Answered most review comments. Thanks for the reviewers @balazske, @martong so 
far.
The question of absolute path policy is still up for debate.




Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:365
 
+  llvm::SmallString<256> AbsPath = CTUDir;
+  llvm::sys::path::append(AbsPath, Identifier);

martong wrote:
> Could we check somehow if `CTUDir` is indeed an absolute path? If not then 
> probably we should bail out with an error. Though I am not sure if there is 
> an easy and portable way in llvm:: to check for beeing an absolute path.
Actually this does not need to be absolute. The naming of the variable should 
be changed to PrefixedPath or similar. I will do that for now.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:365
 
+  llvm::SmallString<256> AbsPath = CTUDir;
+  llvm::sys::path::append(AbsPath, Identifier);

gamesh411 wrote:
> martong wrote:
> > Could we check somehow if `CTUDir` is indeed an absolute path? If not then 
> > probably we should bail out with an error. Though I am not sure if there is 
> > an easy and portable way in llvm:: to check for beeing an absolute path.
> Actually this does not need to be absolute. The naming of the variable should 
> be changed to PrefixedPath or similar. I will do that for now.
It not necessarily an absolute path, only prefixed by CTUDir. There is no clear 
policy as to where to use strictly absolute paths and where to accept relative 
paths. IMHO this should be considered, and I am most certainly open for a 
discussion. In the meantime, I will just rename the variable to PrefixedPath.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+

martong wrote:
> martong wrote:
> > martong wrote:
> > > Seems like `Tool` has it's lifetime ended when `load()` finishes. But we 
> > > have long living `ASTUnits` whose lifetime are longer then the `Tool` 
> > > which created them. Is this not a problem?
> > `CI.getPCHContainerOperations()` is probably not needed, because we are not 
> > going to exercise the ASTReader, so this could be a nullptr (default param 
> > value).
> If `CI.getPCHContainerOperations()` is not needed then we can remove the `CI` 
> member of `ASTOnDemandLoader`.
As for the automatic lifetime of Tool, I am inclined to say this is not the 
case. I have run my clang-build with sanitizers (undefined and memory), and no 
bad access or UB was reported. If someone has more domain knowledge about 
tooling and the lifetime issues that could arise, I would be thrilled to be 
enlightened.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+

gamesh411 wrote:
> martong wrote:
> > martong wrote:
> > > martong wrote:
> > > > Seems like `Tool` has it's lifetime ended when `load()` finishes. But 
> > > > we have long living `ASTUnits` whose lifetime are longer then the 
> > > > `Tool` which created them. Is this not a problem?
> > > `CI.getPCHContainerOperations()` is probably not needed, because we are 
> > > not going to exercise the ASTReader, so this could be a nullptr (default 
> > > param value).
> > If `CI.getPCHContainerOperations()` is not needed then we can remove the 
> > `CI` member of `ASTOnDemandLoader`.
> As for the automatic lifetime of Tool, I am inclined to say this is not the 
> case. I have run my clang-build with sanitizers (undefined and memory), and 
> no bad access or UB was reported. If someone has more domain knowledge about 
> tooling and the lifetime issues that could arise, I would be thrilled to be 
> enlightened.
As for the need to pass PCHContainerOperaitons:
I think you are right, and I will check if this modifies the behaviour of the 
import in any way. I am removing this for now.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:443
+/// be the absolute path of the file.
+*SourceFilePath = Identifier.str();
+

martong wrote:
> Perhaps we should make sure that `Identifier` is indeed an absolute path and 
> if not then we should emit an error. If a user do not use CodeChecker to 
> generate the ExternalDefMapping it may contain relative paths. See e.g.: 
> https://clang.llvm.org/docs/analyzer/user-docs/CrossTranslationUnit.html#manual-ctu-analysis
The policy of absolute and relative paths should be discussed. See my previous 
answer concerning AbsPath variable above. BTW `llvm::sys::path::is_absolute` is 
a solution for asserting this as I see it.



Comment at: clang/test/Analysis/ctu-main.c:53
   clang_analyzer_eval(res == 6); // expected-warning{{TRUE}}
+  // Call something with 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The tests are getting better, thanks Endre!




Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:5
+// RUN: cp "%S/Inputs/ctu-other.c" "%t/ctu-other.c"
+// RUN: echo '[{"directory":"%t","command":"gcc -c -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: cd "%t" && %clang_extdef_map ctu-other.c > externalDefMap.txt

Why do we need the `-Wno-visibility` flag?



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:6
+// RUN: echo '[{"directory":"%t","command":"gcc -c -std=c89 -Wno-visibility 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/compile_commands.json
+// RUN: cd "%t" && %clang_extdef_map ctu-other.c > externalDefMap.txt
+// RUN: cd "%t" && %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only 
-std=c89 -analyze \

If we execute `clang_extdef_map` then we don't need 
`Inputs/ctu-other.c[pp].externalDefMap.on-the-fly.txt` files. Do we?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 257622.
gamesh411 added a comment.

Reorganize test code to overcome testing infrastructure limitations


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,100 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: echo '[{"directory":"%S/Inputs","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%S/Inputs","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/ctudir/compile_commands.json
+// RUN: %clang_extdef_map %S/Inputs/ctu-chain.cpp %S/Inputs/ctu-other.cpp > %t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -triple x86_64-pc-linux-gnu \
+// RUN:   -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
+// RUN:   -analyzer-config ctu-dir="%t/ctudir" \
+// RUN:   -analyzer-config ctu-on-demand-parsing=true \
+// RUN:   

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D75665#1971440 , @gamesh411 wrote:

> In D75665#1907914 , @martong wrote:
>
> > The warning below suggests that we parse the ctu-other.c file (and 
> > presumably every file) as a C++ file, even if it should be parsed as a C 
> > file. Perhaps the invocation of the parser is missing some setting? Also, 
> > could this be the reason why on-the-fly and pch driven ctu gives different 
> > statistics?
> >
> >   warning: treating 'c' input as 'c++' when in C++ mode, this behavior is 
> > deprecated [-Wdeprecated]
> >   
> > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c:47:26:
> >  error: 'DataType' cannot be defined in a parameter type
> >   int structInProto(struct DataType {int a;int b; } * d) {
> >^
> >   1 error generated.
> >   Error while processing 
> > /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c.
> >  
> >
>
>
> I have investigated the issue, and the compiler flags were looked up using 
> the heuristic implemented in tooling.
>  This heuristic looks for the suitable compilation database in an upward 
> ascending fashion inside the directory tree startin from the input source 
> file.
>  By copying both the source file and the compilation database to the test 
> directory this heuristic does the right thing now. (up until now the found 
> compile_commands.json was the one used for llvm-project itself, and picked up 
> a c++ specific compilation).
>
> This issue is solved, however the ASTImporter still cannot import the inline 
> definition of the struct, and emits an error.
>  Right now I am debugging the master branch that uses the AST-dumps wheter I 
> also encounter this error inside the ASTImporter (just to see if I am 
> operating on sane assumptions).


I've been looking into this and the ASTImporter indeed does not (cannot) import 
the definition of `structInProto()`. But that's just fine, that is why we have 
a branching for the call expression (TRUE/FALSE) below in ctu-main.c . All 
other functions whose definition were imported gives only the TRUE branch.

  clang_analyzer_eval(structInProto() == 0); // expected-warning{{TRUE}} 
expected-warning{{FALSE}}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.



In D75665#1907914 , @martong wrote:

> The warning below suggests that we parse the ctu-other.c file (and presumably 
> every file) as a C++ file, even if it should be parsed as a C file. Perhaps 
> the invocation of the parser is missing some setting? Also, could this be the 
> reason why on-the-fly and pch driven ctu gives different statistics?
>
>   warning: treating 'c' input as 'c++' when in C++ mode, this behavior is 
> deprecated [-Wdeprecated]
>   
> /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c:47:26:
>  error: 'DataType' cannot be defined in a parameter type
>   int structInProto(struct DataType {int a;int b; } * d) {
>^
>   1 error generated.
>   Error while processing 
> /mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c.
>  
>


I have investigated the issue, and the compiler flags were looked up using the 
heuristic implemented in tooling.
This heuristic looks for the suitable compilation database in an upward 
ascending fashion inside the directory tree startin from the input source file.
By copying both the source file and the compilation database to the test 
directory this heuristic does the right thing now. (up until now the found 
compile_commands.json was the one used for llvm-project itself, and picked up a 
c++ specific compilation).

This issue is solved, however the ASTImporter still cannot import the inline 
definition of the struct, and emits an error.
Right now I am debugging the master branch that uses the AST-dumps wheter I 
also encounter this error inside the ASTImporter (just to see if I am operating 
on sane assumptions).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

In D75665#1907938 , @martong wrote:

> > As the CTUDir string prefix is only needed in case of the old approach, I 
> > have refactored the external definition mapping storage to NOT include that.
>
> Both `ctu-main.c` and `ctu-on-demand-parsing.c` use the `-analyzer-config 
> ctu-dir=%t/ctudir2` setting. So it is not clear for me why we don't need the 
> prefix. I believe we still need that directory setting otherwise where could 
> we find the externalDefMap file?


You are right, the ctudir2 prefix is not needed as the %t substitution is 
test-file-level unique.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 256212.
gamesh411 added a comment.
Herald added a subscriber: ASDenysPetrov.

Refactored the test
Copying the compile_commands.json in case of test for C files
results in the extdef mapping tool finding the correct compiler
flags for the C to be imported.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.on-the-fly.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.on-the-fly.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,100 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: echo '[{"directory":"%S/Inputs","command":"clang++ -c ctu-chain.cpp","file":"ctu-chain.cpp"},{"directory":"%S/Inputs","command":"clang++ -c ctu-other.cpp","file":"ctu-other.cpp"}]' | sed -e 's/\\//g' > %t/ctudir/compile_commands.json
+// RUN: %clang_extdef_map %S/Inputs/ctu-chain.cpp %S/Inputs/ctu-other.cpp > %t/ctudir/externalDefMap.txt
+// RUN: %clang_analyze_cc1 -triple 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:365
 
+  llvm::SmallString<256> AbsPath = CTUDir;
+  llvm::sys::path::append(AbsPath, Identifier);

Could we check somehow if `CTUDir` is indeed an absolute path? If not then 
probably we should bail out with an error. Though I am not sure if there is an 
easy and portable way in llvm:: to check for beeing an absolute path.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:375
+/// Load the AST from a source-file, which is supposed to be located inside the
+/// compilation database \p OnDemandParsingCommands. The compilation database
+/// can contain the path of the file under the key "file" as an absolute path,

There is no variable named as `OnDemandParsingCommands`, perhaps you made a 
rename but forgot to rename in the comments?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:384
+/// the invocation inside ClangTool is always made with an absolute path. \p
+/// ASTSourcePath is assumed to be the lookup-name of the file, which comes 
from
+/// the Index. The Index is built by the \p clang-extdef-mapping tool, which is

There is no variable named as `ASTSourcePath`, perhaps you made a rename but 
forgot to rename in the comments?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+

martong wrote:
> martong wrote:
> > Seems like `Tool` has it's lifetime ended when `load()` finishes. But we 
> > have long living `ASTUnits` whose lifetime are longer then the `Tool` which 
> > created them. Is this not a problem?
> `CI.getPCHContainerOperations()` is probably not needed, because we are not 
> going to exercise the ASTReader, so this could be a nullptr (default param 
> value).
If `CI.getPCHContainerOperations()` is not needed then we can remove the `CI` 
member of `ASTOnDemandLoader`.



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:443
+/// be the absolute path of the file.
+*SourceFilePath = Identifier.str();
+

Perhaps we should make sure that `Identifier` is indeed an absolute path and if 
not then we should emit an error. If a user do not use CodeChecker to generate 
the ExternalDefMapping it may contain relative paths. See e.g.: 
https://clang.llvm.org/docs/analyzer/user-docs/CrossTranslationUnit.html#manual-ctu-analysis



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:457
+return llvm::make_error(
+index_error_code::ambiguous_compile_commands_database);
+

Could we have a lit test for this case? I.e having a compile_commands.json with 
two ambiguous entries for the same file and then we should expect the compiler 
diag?
(Note, I am asking this because it is not immediate for me from the code that 
this will ever fire.)



Comment at: clang/test/Analysis/ctu-main.c:53
   clang_analyzer_eval(res == 6); // expected-warning{{TRUE}}
+  // Call something with uninitialized from the same function in which the 
implicit was called.
+  // This is necessary to reproduce a special bug in NoStoreFuncVisitor.

Is this hunk related?



Comment at: clang/test/Analysis/ctu-on-demand-parsing.c:3
 // RUN: mkdir -p %t/ctudir2
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu \
-// RUN:   -emit-pch -o %t/ctudir2/ctu-other.c.ast %S/Inputs/ctu-other.c
-// RUN: cp %S/Inputs/ctu-other.c.externalDefMap.txt 
%t/ctudir2/externalDefMap.txt
-// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -std=c89 -analyze 
\
+// RUN: echo '[{"directory":"%S/Inputs","command":"clang -x c -std=c89 -c 
ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
%t/ctudir2/compile_commands.json
+// RUN: %clang_extdef_map %S/Inputs/ctu-other.c > %t/ctudir2/externalDefMap.txt

Why do we need `sed` here? I don't see any `\` in the compile commands json. Is 
it because of Windows builds? Could you please explain in a comment maybe in 
the previous line.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:222
+
+  struct ASTLoader {
+/// Load the ASTUnit by an identifier. Subclasses should determine what 
this

`class` would look better here? (The descendants are `class` too.)



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:225
+/// would be.
+virtual LoadResultTy load(StringRef Identifier) = 0;
+virtual ~ASTLoader() = default;

Probably it is good to mention in the documentation that the function is used 
with a string read from a file and the type of the file determines the meaning 
of the `Identifier` here (the calling code does have no direct knowledge about 
it).



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:460
+  /// Ideally there is exactly one entry in the compilation database that
+  /// matchse the source file.
+  if (ASTs.size() != 1)

matches



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:604
+   index_error_code::failed_to_get_external_ast);
+}
+

Members of `ASTOnDemandLoader` would be better in a single group in the source 
file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:578
+
+  // Optional OnDemandParsingDatabase;
+  // if (Opts.CTUOnDemandParsing)

left here some debugging


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:23
 #include "llvm/ADT/StringMap.h"
+#include "llvm/IR/DiagnosticInfo.h"
+#include "llvm/IR/OperandTraits.h"

Perhaps this include is needed only in the .cpp file?



Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:97
 llvm::Expected>
-parseCrossTUIndex(StringRef IndexPath, StringRef CrossTUDir);
+parseCrossTUIndex(StringRef IndexPath);
 

Ah, so we use an absolute path for `IndexPath` from now on? If yes, could you 
please add that to the comments above?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:121
+  return "Compile commands database contains multiple references to the "
+ "same sorce file.";
 }

typo: sorce -> source



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+

Seems like `Tool` has it's lifetime ended when `load()` finishes. But we have 
long living `ASTUnits` whose lifetime are longer then the `Tool` which created 
them. Is this not a problem?



Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:413
+  Files.push_back(std::string(Identifier));
+  ClangTool Tool(*CompileCommands, Files, CI.getPCHContainerOperations());
+

martong wrote:
> Seems like `Tool` has it's lifetime ended when `load()` finishes. But we have 
> long living `ASTUnits` whose lifetime are longer then the `Tool` which 
> created them. Is this not a problem?
`CI.getPCHContainerOperations()` is probably not needed, because we are not 
going to exercise the ASTReader, so this could be a nullptr (default param 
value).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> As the CTUDir string prefix is only needed in case of the old approach, I 
> have refactored the external definition mapping storage to NOT include that.

Both `ctu-main.c` and `ctu-on-demand-parsing.c` use the `-analyzer-config 
ctu-dir=%t/ctudir2` setting. So it is not clear for me why we don't need the 
prefix. I believe we still need that directory setting otherwise where could we 
find the externalDefMap file?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

The warning below suggests that we parse the ctu-other.c file (and presumably 
every file) as a C++ file, even if it should be parsed as a C file. Perhaps the 
invocation of the parser is missing some setting? Also, could this be the 
reason why on-the-fly and pch driven ctu gives different statistics?

  warning: treating 'c' input as 'c++' when in C++ mode, this behavior is 
deprecated [-Wdeprecated]
  
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c:47:26:
 error: 'DataType' cannot be defined in a parameter type
  int structInProto(struct DataType {int a;int b; } * d) {
   ^
  1 error generated.
  Error while processing 
/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/clang/test/Analysis/Inputs/ctu-other.c.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

This patch adds an alternative way of loading ASTs to provide the CTU 
definitions needed during analysis.
The additional approach is to use Tooling with a user-provided 
compile_commands.json,
and look up the needed file from the output of the ext-def-mapping tool (which 
is now used for both approaches).

As the CTUDir string prefix is only needed in case of the old approach, I have 
refactored the external definition mapping storage to NOT include that.

Experience with diagnostics generated by the on-demand approach shows that 
location information inside plists contain file paths as they were in the 
compile_commands.json.
I would say, a uniform handling of paths is desirable, so an ArgAdjuster is 
added to the Clang Tool which implements the path normalization step.

The following test still fails. As far as I can tell, this is due to the C 
language being laxer. The concrete failure:

   TEST 'Clang :: Analysis/ctu-on-demand-parsing.c' FAILED 

  Script:
  --
  : 'RUN: at line 1';   rm -rf 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp
 && mkdir 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp
  : 'RUN: at line 2';   mkdir -p 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2
  : 'RUN: at line 3';   echo 
'[{"directory":"/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs","command":"clang
 -x c -std=c89 -c ctu-other.c","file":"ctu-other.c"}]' | sed -e 's/\\//g' > 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2/compile_commands.json
  : 'RUN: at line 4';   
/home/gamesh411/projects/clang-rwd/bin/clang-extdef-mapping 
/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs/ctu-other.c > 
/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2/externalDefMap.txt
  : 'RUN: at line 5';   /home/gamesh411/projects/clang-rwd/bin/clang -cc1 
-internal-isystem /home/gamesh411/projects/clang-rwd/lib/clang/11.0.0/include 
-nostdsysteminc -triple x86_64-pc-linux-gnu -fsyntax-only -x c -std=c89 
-analyze-analyzer-checker=core,debug.ExprInspection-analyzer-config 
experimental-enable-naive-ctu-analysis=true-analyzer-config 
ctu-dir=/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2
-analyzer-config ctu-on-demand-parsing=true-analyzer-config 
ctu-on-demand-parsing-database="/home/gamesh411/projects/clang-rwd/tools/clang/test/Analysis/Output/ctu-on-demand-parsing.c.tmp/ctudir2/compile_commands.json"
-verify 
/home/gamesh411/projects/llvm-project/clang/test/Analysis/ctu-on-demand-parsing.c
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  warning: treating 'c' input as 'c++' when in C++ mode, this behavior is 
deprecated [-Wdeprecated]
  warning: unknown warning option '-Wno-maybe-uninitialized'; did you mean 
'-Wno-uninitialized'? [-Wunknown-warning-option]
  
/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs/ctu-other.c:47:26:
 error: 'DataType' cannot be defined in a parameter type
  int structInProto(struct DataType {int a;int b; } * d) {
   ^
  1 warning and 1 error generated.
  Error while processing 
/home/gamesh411/projects/llvm-project/clang/test/Analysis/Inputs/ctu-other.c.

Would it be ok to remove those tests? Currently, I see no solution for solving 
this test case, any insight is welcome.




Comment at: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp:176
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;

CTUDir is no longer prepended to the mapping by default, as on-demand parsing 
does not use the CTUDir prefix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75665/new/

https://reviews.llvm.org/D75665



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

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 created this revision.
gamesh411 added reviewers: martong, balazske.
Herald added subscribers: cfe-commits, steakhal, Charusso, dkrupp, donat.nagy, 
Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, 
xazax.hun, whisperity, mgorny.
Herald added a reviewer: Szelethus.
Herald added a project: clang.

Add an option to enable on-demand parsing of needed ASTs during CTU analysis.
Two options are introduced. CTUOnDemandParsing enables the feature, and
CTUOnDemandParsingDatabase specifies the path to a compilation database, which
has all the necessary information to generate the ASTs. In case an empty
string is given, on-the-fly parsing is disabled.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75665

Files:
  clang/include/clang/CrossTU/CrossTranslationUnit.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/lib/CrossTU/CMakeLists.txt
  clang/lib/CrossTU/CrossTranslationUnit.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/StaticAnalyzer/Core/CallEvent.cpp
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.on-the-fly.txt
  clang/test/Analysis/Inputs/ctu-other.c.externalDefMap.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.on-the-fly.txt
  clang/test/Analysis/Inputs/ctu-other.cpp.externalDefMap.txt
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ctu-different-triples.cpp
  clang/test/Analysis/ctu-main.c
  clang/test/Analysis/ctu-main.cpp
  clang/test/Analysis/ctu-on-demand-parsing.c
  clang/test/Analysis/ctu-on-demand-parsing.cpp
  clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp

Index: clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
===
--- clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
+++ clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
@@ -7,10 +7,11 @@
 //===--===//
 
 #include "clang/CrossTU/CrossTranslationUnit.h"
-#include "clang/Frontend/CompilerInstance.h"
 #include "clang/AST/ASTConsumer.h"
+#include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -162,7 +163,7 @@
   IndexFile.os().flush();
   EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
   llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "");
+  parseCrossTUIndex(IndexFileName);
   EXPECT_TRUE((bool)IndexOrErr);
   llvm::StringMap ParsedIndex = IndexOrErr.get();
   for (const auto  : Index) {
@@ -173,25 +174,5 @@
 EXPECT_TRUE(Index.count(E.getKey()));
 }
 
-TEST(CrossTranslationUnit, CTUDirIsHandledCorrectly) {
-  llvm::StringMap Index;
-  Index["a"] = "/b/c/d";
-  std::string IndexText = createCrossTUIndexString(Index);
-
-  int IndexFD;
-  llvm::SmallString<256> IndexFileName;
-  ASSERT_FALSE(llvm::sys::fs::createTemporaryFile("index", "txt", IndexFD,
-  IndexFileName));
-  llvm::ToolOutputFile IndexFile(IndexFileName, IndexFD);
-  IndexFile.os() << IndexText;
-  IndexFile.os().flush();
-  EXPECT_TRUE(llvm::sys::fs::exists(IndexFileName));
-  llvm::Expected> IndexOrErr =
-  parseCrossTUIndex(IndexFileName, "/ctudir");
-  EXPECT_TRUE((bool)IndexOrErr);
-  llvm::StringMap ParsedIndex = IndexOrErr.get();
-  EXPECT_EQ(ParsedIndex["a"], "/ctudir/b/c/d");
-}
-
 } // end namespace cross_tu
 } // end namespace clang
Index: clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
===
--- clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
+++ clang/test/Analysis/ctu-unknown-parts-in-triples.cpp
@@ -5,7 +5,7 @@
 // RUN: mkdir -p %t/ctudir
 // RUN: %clang_cc1 -std=c++14 -triple x86_64-pc-linux-gnu \
 // RUN:   -emit-pch -o %t/ctudir/ctu-other.cpp.ast %S/Inputs/ctu-other.cpp
-// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.txt %t/ctudir/externalDefMap.txt
+// RUN: cp %S/Inputs/ctu-other.cpp.externalDefMap.ast-dump.txt %t/ctudir/externalDefMap.txt
 // RUN: %clang_analyze_cc1 -std=c++14 -triple x86_64-unknown-linux-gnu \
 // RUN:   -analyzer-checker=core,debug.ExprInspection \
 // RUN:   -analyzer-config experimental-enable-naive-ctu-analysis=true \
Index: clang/test/Analysis/ctu-on-demand-parsing.cpp
===
--- /dev/null
+++ clang/test/Analysis/ctu-on-demand-parsing.cpp
@@ -0,0 +1,100 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: mkdir -p %t/ctudir
+// RUN: cp %S/Inputs/ctu-chain.cpp %t/ctudir/ctu-chain.cpp
+// RUN: echo '[{"directory":"%S/Inputs","command":"clang++ -c