[Lldb-commits] [PATCH] D60871: [CodeComplete] Remove obsolete isOutputBinary().

[Sam McCall via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB358696: [CodeComplete] Remove obsolete isOutputBinary(). 
(authored by sammccall, committed by ).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60871?vs=195740&id=195782#toc

Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D60871

Files:
  source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -703,7 +703,7 @@
   ///
   CodeComplete(CompletionRequest &request, clang::LangOptions ops,
std::string expr, unsigned position)
-  : CodeCompleteConsumer(CodeCompleteOptions(), false),
+  : CodeCompleteConsumer(CodeCompleteOptions()),
 m_info(std::make_shared()), 
m_expr(expr),
 m_position(position), m_request(request), m_desc_policy(ops) {
 


Index: source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -703,7 +703,7 @@
   ///
   CodeComplete(CompletionRequest &request, clang::LangOptions ops,
std::string expr, unsigned position)
-  : CodeCompleteConsumer(CodeCompleteOptions(), false),
+  : CodeCompleteConsumer(CodeCompleteOptions()),
 m_info(std::make_shared()), m_expr(expr),
 m_position(position), m_request(request), m_desc_policy(ops) {
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 396324.
sammccall added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Fix lldb build


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/ctor-signature.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -995,7 +995,8 @@
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 // At the moment we don't filter out any overloaded candidates.
   }
 
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -656,14 +656,15 @@
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates,
-   SourceLocation OpenParLoc) override {
+   SourceLocation OpenParLoc,
+   bool Braced) override {
   StoredResults.reserve(StoredResults.size() + NumCandidates);
   for (unsigned I = 0; I != NumCandidates; ++I) {
-CodeCompletionString *StoredCompletion
-  = Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
+CodeCompletionString *StoredCompletion =
+Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
 getCodeCompletionTUInfo(),
-includeBriefComments());
-
+includeBriefComments(), Braced);
+
 CXCompletionResult R;
 R.CursorKind = CXCursor_OverloadCandidate;
 R.CompletionString = StoredCompletion;
Index: clang/test/CodeCompletion/ctor-signature.cpp
===
--- clang/test/CodeCompletion/ctor-signature.cpp
+++ clang/test/CodeCompletion/ctor-signature.cpp
@@ -15,3 +15,40 @@
   // CHECK-CC2: OVERLOAD: Foo(<#const Foo &#>)
   // CHECK-CC2: OVERLOAD: Foo(<#Foo &&#>
 }
+
+namespace std {
+template  struct initializer_list {};
+} // namespace std
+
+struct Bar {
+  // CHECK-BRACED: OVERLOAD: Bar{<#int#>}
+  Bar(int);
+  // CHECK-BRACED: OVERLOAD: Bar{<#double#>, double}
+  Bar(double, double);
+  // FIXME: no support for init-list constructors yet.
+  // CHECK-BRACED-NOT: OVERLOAD: {{.*}}char
+  Bar(std::initializer_list C);
+  // CHECK-BRACED: OVERLOAD: Bar{<#const Bar &#>}
+  // CHECK-BRACED: OVERLOAD: Bar{<#T *Pointer#>}
+  template  Bar(T *Pointer);
+};
+
+auto b1 = Bar{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:36:15 %s | FileCheck -check-prefix=CHECK-BRACED %s
+Bar b2{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s | FileCheck -check-prefix=CHECK-BRACED %s
+static int consumeBar(Bar) { return 0; }
+int b3 = consumeBar({});
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:41:22 %s | FileCheck -check-prefix=CHECK-BRACED %s
+
+struct Aggregate {
+  // FIXME: no support for aggregates yet.
+  // CHECK-AGGREGATE-NOT: OVERLOAD: Aggregate{<#const Aggregate &#>}
+  // CHECK-AGGREGATE-NOT: OVERLOAD: {{.*}}first
+  int first;
+  int second;
+};
+
+Aggregate a{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:52:13 %s | FileCheck -check-prefix=CHECK-AGGREGATE %s
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3761,7 +3761,8 @@
 CodeCompletionString *
 CodeCompleteConsu

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 396325.
sammccall edited the summary of this revision.
sammccall added a comment.
Herald added subscribers: JDevlieghere, ilya-biryukov.

Revert some unintended changes, clean up tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/ctor-signature.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -995,7 +995,8 @@
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 // At the moment we don't filter out any overloaded candidates.
   }
 
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -656,14 +656,15 @@
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates,
-   SourceLocation OpenParLoc) override {
+   SourceLocation OpenParLoc,
+   bool Braced) override {
   StoredResults.reserve(StoredResults.size() + NumCandidates);
   for (unsigned I = 0; I != NumCandidates; ++I) {
-CodeCompletionString *StoredCompletion
-  = Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
+CodeCompletionString *StoredCompletion =
+Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
 getCodeCompletionTUInfo(),
-includeBriefComments());
-
+includeBriefComments(), Braced);
+
 CXCompletionResult R;
 R.CursorKind = CXCursor_OverloadCandidate;
 R.CompletionString = StoredCompletion;
Index: clang/test/CodeCompletion/ctor-signature.cpp
===
--- clang/test/CodeCompletion/ctor-signature.cpp
+++ clang/test/CodeCompletion/ctor-signature.cpp
@@ -15,3 +15,40 @@
   // CHECK-CC2: OVERLOAD: Foo(<#const Foo &#>)
   // CHECK-CC2: OVERLOAD: Foo(<#Foo &&#>
 }
+
+namespace std {
+template  struct initializer_list {};
+} // namespace std
+
+struct Bar {
+  // CHECK-BRACED: OVERLOAD: Bar{<#int#>}
+  Bar(int);
+  // CHECK-BRACED: OVERLOAD: Bar{<#double#>, double}
+  Bar(double, double);
+  // FIXME: no support for init-list constructors yet.
+  // CHECK-BRACED-NOT: OVERLOAD: {{.*}}char
+  Bar(std::initializer_list C);
+  // CHECK-BRACED: OVERLOAD: Bar{<#const Bar &#>}
+  // CHECK-BRACED: OVERLOAD: Bar{<#T *Pointer#>}
+  template  Bar(T *Pointer);
+};
+
+auto b1 = Bar{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:36:15 %s | FileCheck -check-prefix=CHECK-BRACED %s
+Bar b2{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s | FileCheck -check-prefix=CHECK-BRACED %s
+static int consumeBar(Bar) { return 0; }
+int b3 = consumeBar({});
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:41:22 %s | FileCheck -check-prefix=CHECK-BRACED %s
+
+struct Aggregate {
+  // FIXME: no support for aggregates yet.
+  // CHECK-AGGREGATE-NOT: OVERLOAD: Aggregate{<#const Aggregate &#>}
+  // CHECK-AGGREGATE-NOT: OVERLOAD: {{.*}}first
+  int first;
+  int second;
+};
+
+Aggregate a{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:52:13 %s | FileCheck -check-prefix=CHECK-AGGREGATE %s
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 396362.
sammccall added a comment.

Fix clangd tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/ctor-signature.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -995,7 +995,8 @@
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 // At the moment we don't filter out any overloaded candidates.
   }
 
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -656,14 +656,15 @@
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates,
-   SourceLocation OpenParLoc) override {
+   SourceLocation OpenParLoc,
+   bool Braced) override {
   StoredResults.reserve(StoredResults.size() + NumCandidates);
   for (unsigned I = 0; I != NumCandidates; ++I) {
-CodeCompletionString *StoredCompletion
-  = Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
+CodeCompletionString *StoredCompletion =
+Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
 getCodeCompletionTUInfo(),
-includeBriefComments());
-
+includeBriefComments(), Braced);
+
 CXCompletionResult R;
 R.CursorKind = CXCursor_OverloadCandidate;
 R.CompletionString = StoredCompletion;
Index: clang/test/CodeCompletion/ctor-signature.cpp
===
--- clang/test/CodeCompletion/ctor-signature.cpp
+++ clang/test/CodeCompletion/ctor-signature.cpp
@@ -15,3 +15,40 @@
   // CHECK-CC2: OVERLOAD: Foo(<#const Foo &#>)
   // CHECK-CC2: OVERLOAD: Foo(<#Foo &&#>
 }
+
+namespace std {
+template  struct initializer_list {};
+} // namespace std
+
+struct Bar {
+  // CHECK-BRACED: OVERLOAD: Bar{<#int#>}
+  Bar(int);
+  // CHECK-BRACED: OVERLOAD: Bar{<#double#>, double}
+  Bar(double, double);
+  // FIXME: no support for init-list constructors yet.
+  // CHECK-BRACED-NOT: OVERLOAD: {{.*}}char
+  Bar(std::initializer_list C);
+  // CHECK-BRACED: OVERLOAD: Bar{<#const Bar &#>}
+  // CHECK-BRACED: OVERLOAD: Bar{<#T *Pointer#>}
+  template  Bar(T *Pointer);
+};
+
+auto b1 = Bar{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:36:15 %s | FileCheck -check-prefix=CHECK-BRACED %s
+Bar b2{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s | FileCheck -check-prefix=CHECK-BRACED %s
+static int consumeBar(Bar) { return 0; }
+int b3 = consumeBar({});
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:41:22 %s | FileCheck -check-prefix=CHECK-BRACED %s
+
+struct Aggregate {
+  // FIXME: no support for aggregates yet.
+  // CHECK-AGGREGATE-NOT: OVERLOAD: Aggregate{<#const Aggregate &#>}
+  // CHECK-AGGREGATE-NOT: OVERLOAD: {{.*}}first
+  int first;
+  int second;
+};
+
+Aggregate a{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:52:13 %s | FileCheck -check-prefix=CHECK-AGGREGATE %s
+
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3761,7 +3761,8 @@
 CodeCompletionString *
 CodeCompleteConsumer::Overload

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via Phabricator via lldb-commits]

sammccall added inline comments.



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp:995
   /// \param NumCandidates the number of overload candidates
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,

kadircet wrote:
> why not just drop the method? it is not pure, and no-op in base too.
That's a reasonable question, but it's not my code so IDK if it's a style thing.
(I was tempted to remove it but it seems gratuitous for this kind of change)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

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

[Lldb-commits] [PATCH] D116317: [CodeCompletion] Signature help for braced constructor calls

[Sam McCall via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG92417eaf3329: [CodeCompletion] Signature help for braced 
constructor calls (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D116317?vs=396362&id=397111#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116317

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseInit.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/CodeCompleteConsumer.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/test/CodeCompletion/ctor-signature.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
@@ -995,7 +995,8 @@
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
  unsigned NumCandidates,
- SourceLocation OpenParLoc) override {
+ SourceLocation OpenParLoc,
+ bool Braced) override {
 // At the moment we don't filter out any overloaded candidates.
   }
 
Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -656,14 +656,15 @@
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
unsigned NumCandidates,
-   SourceLocation OpenParLoc) override {
+   SourceLocation OpenParLoc,
+   bool Braced) override {
   StoredResults.reserve(StoredResults.size() + NumCandidates);
   for (unsigned I = 0; I != NumCandidates; ++I) {
-CodeCompletionString *StoredCompletion
-  = Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
+CodeCompletionString *StoredCompletion =
+Candidates[I].CreateSignatureString(CurrentArg, S, getAllocator(),
 getCodeCompletionTUInfo(),
-includeBriefComments());
-
+includeBriefComments(), Braced);
+
 CXCompletionResult R;
 R.CursorKind = CXCursor_OverloadCandidate;
 R.CompletionString = StoredCompletion;
Index: clang/test/CodeCompletion/ctor-signature.cpp
===
--- clang/test/CodeCompletion/ctor-signature.cpp
+++ clang/test/CodeCompletion/ctor-signature.cpp
@@ -15,3 +15,40 @@
   // CHECK-CC2: OVERLOAD: Foo(<#const Foo &#>)
   // CHECK-CC2: OVERLOAD: Foo(<#Foo &&#>
 }
+
+namespace std {
+template  struct initializer_list {};
+} // namespace std
+
+struct Bar {
+  // CHECK-BRACED: OVERLOAD: Bar{<#int#>}
+  Bar(int);
+  // CHECK-BRACED: OVERLOAD: Bar{<#double#>, double}
+  Bar(double, double);
+  // FIXME: no support for init-list constructors yet.
+  // CHECK-BRACED-NOT: OVERLOAD: {{.*}}char
+  Bar(std::initializer_list C);
+  // CHECK-BRACED: OVERLOAD: Bar{<#const Bar &#>}
+  // CHECK-BRACED: OVERLOAD: Bar{<#T *Pointer#>}
+  template  Bar(T *Pointer);
+};
+
+auto b1 = Bar{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:36:15 %s | FileCheck -check-prefix=CHECK-BRACED %s
+Bar b2{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:38:8 %s | FileCheck -check-prefix=CHECK-BRACED %s
+static int consumeBar(Bar) { return 0; }
+int b3 = consumeBar({});
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:41:22 %s | FileCheck -check-prefix=CHECK-BRACED %s
+
+struct Aggregate {
+  // FIXME: no support for aggregates yet.
+  // CHECK-AGGREGATE-NOT: OVERLOAD: Aggregate{<#const Aggregate &#>}
+  // CHECK-AGGREGATE-NOT: OVERLOAD: {{.*}}first
+  int first;
+  int second;
+};
+
+Aggregate a{};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:52:13 %s | FileCheck -check-prefix=CHECK-AGGREGATE %s
+
Index: clang/lib/Sema/SemaCo

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 394648.
sammccall added a comment.

Fix QualTypeNames bug that showed up in external testing.
(Now covered by a unittest added separately)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
  clang/test/SemaCXX/warn-enum-compare.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/Tooling/StencilTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -559,7 +559,7 @@
 
 TEST_F(StencilTest, DescribeUnqualifiedType) {
   std::string Snippet = "using N::C; C c; c;";
-  std::string Expected = "N::C";
+  std::string Expected = "C";
   auto StmtMatch =
   matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
   ASSERT_TRUE(StmtMatch);
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/SemaCXX/warn-enum-compare.cpp
===
--- clang/test/SemaCXX/warn-enum-compare.cpp
+++ clang/test/SemaCXX/warn-enum-compare.cpp
@@ -78,15 +78,15 @@
 
   while (B1 == B2); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
   while (name1::B2 == name2::B3); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
+  while (z == name2::B2); // expected-warning  {{comparison of different enumeration types ('Baz' and 'name2::Baz')}}
 
   while (

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 394553.
sammccall marked an inline comment as done.
sammccall added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

update lldb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/test/CXX/stmt.stmt/stmt.iter/stmt.ranged/p1.cpp
  clang/test/SemaCXX/warn-enum-compare.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/Tooling/StencilTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/unittests/Tooling/StencilTest.cpp
===
--- clang/unittests/Tooling/StencilTest.cpp
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -559,7 +559,7 @@
 
 TEST_F(StencilTest, DescribeUnqualifiedType) {
   std::string Snippet = "using N::C; C c; c;";
-  std::string Expected = "N::C";
+  std::string Expected = "C";
   auto StmtMatch =
   matchStmt(Snippet, declRefExpr(hasType(qualType().bind("type";
   ASSERT_TRUE(StmtMatch);
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/SemaCXX/warn-enum-compare.cpp
===
--- clang/test/SemaCXX/warn-enum-compare.cpp
+++ clang/test/SemaCXX/warn-enum-compare.cpp
@@ -78,15 +78,15 @@
 
   while (B1 == B2); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
   while (name1::B2 == name2::B3); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
-  while (z == name2::B2); // expected-warning  {{comparison of different enumeration types ('name1::Baz' and 'name2::Baz')}}
+  while (z == name2::B2); // expected-warning  {{comparison of different enumeration types ('Baz' and 'name2::Baz')}}
 
   while (B1

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

Having done some out-of-tree testing, it seems this silently breaks enough 
ASTMatchers that it'll be hard to get it to stick.

(We don't strictly need to block on out-of-tree failures, but I'm pretty sure a 
lot of stuff in-tree is broken too, and the out-of-tree users just have better 
test coverage).

It would be great if things like 
`expr(hasType(hasDeclaration(cxxRecordDecl(...`, 
`loc(qualType(hasDeclaration(cxxRecordDecl(...` still matched the cases 
they used to. Even with this change at front of mind, it's really surprising 
and inconvenient to have to explicitly unwrap this level of sugar.
ElaboratedType gets implicitly unwrapped by hasDeclaration, I suspect UsingType 
is more usefully treated like this than like TypedefType.
Then some other special-purpose matcher would be used to traverse the 
UsingType->UsingShadowDecl edge instead of hasDeclaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

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

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 394806.
sammccall added a comment.

Fix serialization (found with msan)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/tools/libclang/CIndex.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/AST/ast-dump-using.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-using.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck -strict-whitespace %s
+
+namespace a {
+struct S;
+}
+namespace b {
+using a::S;
+// CHECK:  UsingDecl {{.*}} a::S
+// CHECK-NEXT: UsingShadowDecl {{.*}} implicit CXXRecord {{.*}} 'S'
+// CHECK-NEXT: `-RecordType {{.*}} 'a::S'
+typedef S f; // to dump the introduced type
+// CHECK:  TypedefDecl
+// CHECK-NEXT: `-UsingType {{.*}} 'a::S' sugar
+// CHECK-NEXT:   |-UsingShadow {{.*}} 'S'
+// CHECK-NEXT:   `-RecordType {{.*}} 'a::S'
+}
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -396,6 +396,10 @@
   Record.AddSourceLocation(TL.getNameLoc());
 }
 
+void TypeLocWriter::VisitUsingTypeLoc(UsingTypeLoc TL) {
+  Record.AddSourceLocation(TL.getNameLoc());
+}
+
 void TypeLocWriter::VisitTypedefTypeLoc(TypedefTypeLoc TL) {
   Record.AddSourceLocation(TL.getNameLoc());
 }
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTRead

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 394780.
sammccall added a comment.

Make hasDeclaration look through UsingType, and throughUsingDecl work on
UsingType too. This is more ergonomic, more similar to existing behavior
that matchers rely on, and similar in spirit to DeclRefExpr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/tools/libclang/CIndex.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/AST/ast-dump-using.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-using.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck -strict-whitespace %s
+
+namespace a {
+struct S;
+}
+namespace b {
+using a::S;
+// CHECK:  UsingDecl {{.*}} a::S
+// CHECK-NEXT: UsingShadowDecl {{.*}} implicit CXXRecord {{.*}} 'S'
+// CHECK-NEXT: `-RecordType {{.*}} 'a::S'
+typedef S f; // to dump the introduced type
+// CHECK:  TypedefDecl
+// CHECK-NEXT: `-UsingType {{.*}} 'a::S' sugar
+// CHECK-NEXT:   |-UsingShadow {{.*}} 'S'
+// CHECK-NEXT:   `-RecordType {{.*}} 'a::S'
+}
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -396,6 +396,8 @@
   Record.AddSourceLocation(TL.getNameLoc());
 }
 
+void TypeLocWriter::VisitUsingTypeLoc(UsingTypeLoc TL) {}
+
 void TypeLocWriter::VisitTypedefTypeLoc(TypedefTypeLoc TL) {
   Record.AddSourceLocation(TL.getNameLoc());
 }
Index: clang/lib/Serialization/ASTReader.cpp
===

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 394666.
sammccall added a comment.

TypePrinter prints UsingTypes with their underlying qualifiers by default.
Fix qualifier bug in QualTypeNames
Add a bit more detail to AST dump (UsingShadow)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/tools/libclang/CIndex.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/AST/ast-dump-using.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-using.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck -strict-whitespace %s
+
+namespace a {
+struct S;
+}
+namespace b {
+using a::S;
+// CHECK:  UsingDecl {{.*}} a::S
+// CHECK-NEXT: UsingShadowDecl {{.*}} implicit CXXRecord {{.*}} 'S'
+// CHECK-NEXT: `-RecordType {{.*}} 'a::S'
+typedef S f; // to dump the introduced type
+// CHECK:  TypedefDecl
+// CHECK-NEXT: `-UsingType {{.*}} 'a::S' sugar
+// CHECK-NEXT:   |-UsingShadow {{.*}} 'S'
+// CHECK-NEXT:   `-RecordType {{.*}} 'a::S'
+}
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -396,6 +396,8 @@
   Record.AddSourceLocation(TL.getNameLoc());
 }
 
+void TypeLocWriter::VisitUsingTypeLoc(UsingTypeLoc TL) {}
+
 void TypeLocWriter::VisitTypedefTypeLoc(TypedefTypeLoc TL) {
   Record.AddSourceLocation(TL.getNameLoc());
 }
Index: clang/lib/Serialization/ASTReader.cpp
===
--- 

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGe1600db19d63: [AST] Add UsingType: a sugar type for types 
found via UsingDecl (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -562,6 +562,15 @@
   functionDecl(hasDescendant(typedefDecl(has(atomicType());
 }
 
+TEST_P(ImportType, ImportUsingType) {
+  MatchVerifier Verifier;
+  testImport("struct C {};"
+ "void declToImport() { using ::C; new C{}; }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ functionDecl(hasDescendant(
+ cxxNewExpr(hasType(pointerType(pointee(usingType(;
+}
+
 TEST_P(ImportDecl, ImportFunctionTemplateDecl) {
   MatchVerifier Verifier;
   testImport("template  void declToImport() { };", Lang_CXX03, "",
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/AST/ast-dump-using.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-using.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck -strict-whitespace %s
+
+namespace a {
+struct S;
+}
+namespace b {
+using a::S;
+// CHECK:  UsingDecl {{.*}} a::S
+// CHECK-NEXT

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 395434.
sammccall added a comment.
Herald added a reviewer: shafik.

Fix ASTImporter, add test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

Files:
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/misc/UnusedUsingDeclsCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/PropertiesBase.td
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/include/clang/Basic/TypeNodes.td
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -2581,6 +2581,7 @@
 case clang::Type::Typedef:
 case clang::Type::TypeOf:
 case clang::Type::TypeOfExpr:
+case clang::Type::Using:
   type = type->getLocallyUnqualifiedSingleStepDesugaredType();
   break;
 default:
@@ -4063,6 +4064,7 @@
   case clang::Type::Paren:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
@@ -4722,6 +4724,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
 
   case clang::Type::UnaryTransform:
@@ -5104,6 +5107,7 @@
   case clang::Type::Typedef:
   case clang::Type::TypeOf:
   case clang::Type::TypeOfExpr:
+  case clang::Type::Using:
 llvm_unreachable("Handled in RemoveWrappingTypes!");
   case clang::Type::UnaryTransform:
 break;
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -562,6 +562,15 @@
   functionDecl(hasDescendant(typedefDecl(has(atomicType());
 }
 
+TEST_P(ImportType, ImportUsingType) {
+  MatchVerifier Verifier;
+  testImport("struct C {};"
+ "void declToImport() { using ::C; new C{}; }",
+ Lang_CXX11, "", Lang_CXX11, Verifier,
+ functionDecl(hasDescendant(
+ cxxNewExpr(hasType(pointerType(pointee(usingType(;
+}
+
 TEST_P(ImportDecl, ImportFunctionTemplateDecl) {
   MatchVerifier Verifier;
   testImport("template  void declToImport() { };", Lang_CXX03, "",
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1666,6 +1666,8 @@
   return Visit(TL.getPointeeLoc());
 }
 
+bool CursorVisitor::VisitUsingTypeLoc(UsingTypeLoc TL) { return false; }
+
 bool CursorVisitor::VisitAttributedTypeLoc(AttributedTypeLoc TL) {
   return Visit(TL.getModifiedLoc());
 }
Index: clang/test/AST/ast-dump-using.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-using.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck -strict-whitespace %s
+
+namespace a {
+struct S;
+}
+namespace b {
+using a::S;
+// CHECK:  UsingDecl {{.*}} a::S
+// CHECK-NEXT: UsingShadowDecl {{.*}} implicit CXXRecord {{.*}} 'S'
+// CHECK-NEXT: `-RecordType {{.*}} 'a::S'
+typedef S f; // t

[Lldb-commits] [PATCH] D114251: [AST] Add a sugar type for types found via UsingDecl

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

In D114251#3197713 , @davrec wrote:

> One other thought: can we add diagnostic notes using this new information, 
> e.g.

This sounds plausible, though I didn't want to extend the scope of this patch 
further.
I'm also not sure how often this will be signal rather than noise - I would 
expect in a large majority of cases that understanding exactly how the name was 
brought into scope isn't going to be important to understanding or fixing the 
error. I may be missing something, but the example seems a bit contrived.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114251

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

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

In D112374#3657640 , @mizvekov wrote:

> In general, I would not expect external tools to care about the shape of the 
> AST. I would expect the type API would be used in a way where we ignore a 
> type sugar node we have no reason to acknowledge.
> Ie you query if some type is a (possible sugar to) X, and you would either 
> get X or nothing. The type sugar over it would just be skipped over and you 
> would have no reason to know what was in there or what shape it had.

I'm afraid your expectations are wrong, and not by a little bit :-)

I totally agree this is the best way to the use the AST: understanding what you 
want to depend on and what groups of differences (e.g. sugar) to ignore, and 
writing code that expresses that intent.

However this is empirically not how lots of downstream (and plenty of in-tree) 
code is written, because:

- it requires a deep understanding of the "philosophy" of the AST to understand 
where extensions are possible in future
- many people write AST traversals using matchers, which constrains and 
obscures exactly what's being matched
- many people write AST trawling code based on AST dumps of examples, rather 
than a first-principles approach
- it is difficult and tedious to test
- people are often only as careful as they're incentivized to be

I've seen plenty of (useful) out-of-tree tidy checks written by people fuzzy on 
the difference between a Type and a TypeLoc, or what sugar is. Clang makes it 
(almost) easy to write tools but hard to write robust tools.

All of this is to say I like this change & appreciate how willing you are to 
help out-of-tree tools (which is best-effort), but I expect a lot of churn 
downstream. (And LLVM has a clear policy that that's OK).

(BTW, last time I landed such a change, investigating the LLDB tests was indeed 
the most difficult part, and I'm not even on windows. Running a linux VM of 
some sort might be your best bet, unfortunately)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[Lldb-commits] [PATCH] D121746: Use lit_config.substitute instead of foo % lit_config.params everywhere

[Sam McCall via Phabricator via lldb-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a reviewer: bollu.
Herald added subscribers: ayermolo, sdasgup3, wenzhicui, wrengr, Chia-hungDuan, 
dcaballe, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, 
shauheen, rriddle, mehdi_amini, delcypher.
Herald added a reviewer: sscalpone.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: Flang, All.
sammccall requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, Sanitizers, cfe-commits, 
yota9, stephenneuendorffer, nicolasvasilache, jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, MLIR, LLVM.

This mechanically applies the same changes from D121427 
 everywhere.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121746

Files:
  bolt/test/Unit/lit.site.cfg.py.in
  bolt/test/lit.site.cfg.py.in
  clang/test/Unit/lit.site.cfg.py.in
  clang/test/lit.site.cfg.py.in
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.site.cfg.in
  compiler-rt/test/lit.common.configured.in
  compiler-rt/unittests/lit.common.unit.configured.in
  cross-project-tests/lit.site.cfg.py.in
  flang/test/NonGtestUnit/lit.site.cfg.py.in
  flang/test/Unit/lit.site.cfg.py.in
  flang/test/lit.site.cfg.py.in
  lld/test/Unit/lit.site.cfg.py.in
  lld/test/lit.site.cfg.py.in
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/test/Unit/lit.site.cfg.py.in
  lldb/test/lit.site.cfg.py.in
  llvm/test/Unit/lit.site.cfg.py.in
  llvm/test/lit.site.cfg.py.in
  llvm/utils/lit/tests/lit.site.cfg.in
  mlir/examples/standalone/test/lit.site.cfg.py.in
  mlir/test/Unit/lit.site.cfg.py.in
  mlir/test/lit.site.cfg.py.in
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in

Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -2,8 +2,8 @@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
@@ -21,16 +21,6 @@
 for arch in config.targets_to_build.split():
 config.available_features.add(arch.lower() + '-registered-target')
 
-# Support substitution of the tools and libs dirs with user parameters. This is
-# used when we can't determine the tool dir at configuration time.
-try:
-config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-except KeyError:
-e = sys.exc_info()[1]
-key, = e.args
-lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
-
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -4,9 +4,9 @@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
-config.llvm_build_mode = "@LLVM_BUILD_MODE@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.enable_shared = @ENABLE_SHARED@
@@ -16,17 +16,5 @@
 config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
-# Support substitution of the tools_dir, libs_dirs, and build_mode with user
-# parameters. This is used when we can't determine the tool dir at
-# configuration time.
-try:
-config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-config.llvm_build_mode = config.llvm_build_mode % lit_config.params
-except KeyError:
-e = sys.exc_info()[1]
-key, = e.args
-lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
-
 # Let the main config do the real work.
 lit_config.load_config(config, "@POLLY_SOURCE_DIR@/test/Unit/lit.cfg")
Index: mlir/test/lit.site.cfg.py.in
===
--- mlir/test/li

[Lldb-commits] [PATCH] D121746: Use lit_config.substitute instead of foo % lit_config.params everywhere

[Sam McCall via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG75acad41bcd6: Use lit_config.substitute instead of foo % 
lit_config.params everywhere (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121746

Files:
  bolt/test/Unit/lit.site.cfg.py.in
  bolt/test/lit.site.cfg.py.in
  clang/test/Unit/lit.site.cfg.py.in
  clang/test/lit.site.cfg.py.in
  clang/utils/perf-training/lit.site.cfg.in
  clang/utils/perf-training/order-files.lit.site.cfg.in
  compiler-rt/test/lit.common.configured.in
  compiler-rt/unittests/lit.common.unit.configured.in
  cross-project-tests/lit.site.cfg.py.in
  flang/test/NonGtestUnit/lit.site.cfg.py.in
  flang/test/Unit/lit.site.cfg.py.in
  flang/test/lit.site.cfg.py.in
  lld/test/Unit/lit.site.cfg.py.in
  lld/test/lit.site.cfg.py.in
  lldb/test/API/lit.site.cfg.py.in
  lldb/test/Shell/lit.site.cfg.py.in
  lldb/test/Unit/lit.site.cfg.py.in
  lldb/test/lit.site.cfg.py.in
  llvm/test/Unit/lit.site.cfg.py.in
  llvm/test/lit.site.cfg.py.in
  llvm/utils/lit/tests/lit.site.cfg.in
  mlir/examples/standalone/test/lit.site.cfg.py.in
  mlir/test/Unit/lit.site.cfg.py.in
  mlir/test/lit.site.cfg.py.in
  polly/test/Unit/lit.site.cfg.in
  polly/test/lit.site.cfg.in

Index: polly/test/lit.site.cfg.in
===
--- polly/test/lit.site.cfg.in
+++ polly/test/lit.site.cfg.in
@@ -2,8 +2,8 @@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
@@ -21,16 +21,6 @@
 for arch in config.targets_to_build.split():
 config.available_features.add(arch.lower() + '-registered-target')
 
-# Support substitution of the tools and libs dirs with user parameters. This is
-# used when we can't determine the tool dir at configuration time.
-try:
-config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-except KeyError:
-e = sys.exc_info()[1]
-key, = e.args
-lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
-
 # excludes: A list of directories to exclude from the testsuite. The 'Inputs'
 # subdirectories contain auxiliary inputs for various tests in their parent
 # directories.
Index: polly/test/Unit/lit.site.cfg.in
===
--- polly/test/Unit/lit.site.cfg.in
+++ polly/test/Unit/lit.site.cfg.in
@@ -4,9 +4,9 @@
 
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_libs_dir = "@LLVM_LIBS_DIR@"
-config.llvm_build_mode = "@LLVM_BUILD_MODE@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_libs_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_build_mode = lit_config.substitute("@LLVM_BUILD_MODE@")
 config.polly_obj_root = "@POLLY_BINARY_DIR@"
 config.polly_lib_dir = "@POLLY_LIB_DIR@"
 config.enable_shared = @ENABLE_SHARED@
@@ -16,17 +16,5 @@
 config.llvm_polly_link_into_tools = "@LLVM_POLLY_LINK_INTO_TOOLS@"
 config.has_unittests = @POLLY_GTEST_AVAIL@
 
-# Support substitution of the tools_dir, libs_dirs, and build_mode with user
-# parameters. This is used when we can't determine the tool dir at
-# configuration time.
-try:
-config.llvm_tools_dir = config.llvm_tools_dir % lit_config.params
-config.llvm_libs_dir = config.llvm_libs_dir % lit_config.params
-config.llvm_build_mode = config.llvm_build_mode % lit_config.params
-except KeyError:
-e = sys.exc_info()[1]
-key, = e.args
-lit_config.fatal("unable to find %r parameter, use '--param=%s=VALUE'" % (key,key))
-
 # Let the main config do the real work.
 lit_config.load_config(config, "@POLLY_SOURCE_DIR@/test/Unit/lit.cfg")
Index: mlir/test/lit.site.cfg.py.in
===
--- mlir/test/lit.site.cfg.py.in
+++ mlir/test/lit.site.cfg.py.in
@@ -6,9 +6,9 @@
 config.target_triple = "@LLVM_TARGET_TRIPLE@"
 config.llvm_src_root = "@LLVM_SOURCE_DIR@"
 config.llvm_obj_root = "@LLVM_BINARY_DIR@"
-config.llvm_tools_dir = "@LLVM_TOOLS_DIR@"
-config.llvm_lib_dir = "@LLVM_LIBS_DIR@"
-config.llvm_shlib_dir = "@SHLIBDIR@"
+config.llvm_tools_dir = lit_config.substitute("@LLVM_TOOLS_DIR@")
+config.llvm_lib_dir = lit_config.substitute("@LLVM_LIBS_DIR@")
+config.llvm_shlib_dir = lit_config.substitute("@SHLIBDIR@")
 config.llvm_shlib_ext = "@SHLIBEXT@"
 config.llvm_exe

[Lldb-commits] [PATCH] D125012: [clang] createInvocationFromCommandLine -> createInvocation, delete former. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
sammccall requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

(Followup from 40c13720a4b977d4347bbde53c52a4d0703823c2 
)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D125012

Files:
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/diagtool/ShowEnabledWarnings.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CompilerInstanceTest.cpp
  clang/unittests/Serialization/ModuleCacheTest.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -663,9 +663,11 @@
llvm::make_range(compiler_invocation_arguments.begin(),
 compiler_invocation_arguments.end()));
 
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = diagnostics_engine;
   std::shared_ptr invocation =
-  clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs,
- diagnostics_engine);
+  clang::createInvocation(compiler_invocation_argument_cstrs,
+  std::move(CIOpts));
 
   if (!invocation)
 return nullptr;
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -134,7 +134,10 @@
 ArgsCStr.push_back(arg.c_str());
   }
 
-  Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
+  CIOpts.VFS = FS;
+  Invocation = createInvocation(ArgsCStr, std::move(CIOpts));
   assert(Invocation);
   Invocation->getFrontendOpts().DisableFree = false;
   Invocation->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -125,7 +125,10 @@
   Diags->setClient(new IgnoringDiagConsumer);
 std::vector Args = {"tok-test", "-std=c++03", "-fsyntax-only",
   FileName};
-auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+CreateInvocationOptions CIOpts;
+CIOpts.Diags = Diags;
+CIOpts.VFS = FS;
+auto CI = createInvocation(Args, std::move(CIOpts));
 assert(CI);
 CI->getFrontendOpts().DisableFree = false;
 CI->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Serialization/ModuleCacheTest.cpp
===
--- clang/unittests/Serialization/ModuleCacheTest.cpp
+++ clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
@@ -95,13 +96,15 @@
   MCPArg.append(ModuleCachePath);
   IntrusiveRefCntPtr Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
 
   // First run should pass with no errors
   const char *Args[] = {"clang","-fmodules",  "-Fframeworks",
 MCPArg.c_str(), "-working-directory", TestDir.c_str(),
 "test.m"};
   std::shared_ptr Invocation =
-  createInvocationFromCommandLine(Args, Diags);
+  createInvocation(Args, CIOpts);
   ASSERT_TRUE(Invocation);
   CompilerInstance Instance;
   Instance.setDiagnostics(Diags.get());
@@ -124,7 +127,7 @@
  "-Fframeworks",  MCPArg.c_str(), "-working-directory",
  TestDir.c_str(), "test.m"};
   std::shared_ptr Invocation2 =
-  createInvocationFromCommandLine(Args2, Diags);
+  createInvocation(Args2, CIOpts);
   ASSERT_TRUE(Invocation2);
   CompilerInstance Instance2(Instance.getPCHContainerOperations(),
  &Instance.getModuleCache());
@@ -142,13 +145,15 @@
   MCPArg.append(ModuleCachePath);
  

[Lldb-commits] [PATCH] D125012: [clang] createInvocationFromCommandLine -> createInvocation, delete former. NFC

[Sam McCall via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG499d0b96cb52: [clang] createInvocationFromCommandLine -> 
createInvocation, delete former. NFC (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D125012?vs=427314&id=427634#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125012

Files:
  clang/include/clang/Frontend/Utils.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CreateInvocationFromCommandLine.cpp
  clang/tools/c-index-test/core_main.cpp
  clang/tools/diagtool/ShowEnabledWarnings.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  clang/unittests/Frontend/CompilerInstanceTest.cpp
  clang/unittests/Serialization/ModuleCacheTest.cpp
  clang/unittests/Tooling/Syntax/TokensTest.cpp
  clang/unittests/Tooling/Syntax/TreeTestBase.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp
@@ -663,9 +663,11 @@
llvm::make_range(compiler_invocation_arguments.begin(),
 compiler_invocation_arguments.end()));
 
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = diagnostics_engine;
   std::shared_ptr invocation =
-  clang::createInvocationFromCommandLine(compiler_invocation_argument_cstrs,
- diagnostics_engine);
+  clang::createInvocation(compiler_invocation_argument_cstrs,
+  std::move(CIOpts));
 
   if (!invocation)
 return nullptr;
Index: clang/unittests/Tooling/Syntax/TreeTestBase.cpp
===
--- clang/unittests/Tooling/Syntax/TreeTestBase.cpp
+++ clang/unittests/Tooling/Syntax/TreeTestBase.cpp
@@ -134,7 +134,10 @@
 ArgsCStr.push_back(arg.c_str());
   }
 
-  Invocation = createInvocationFromCommandLine(ArgsCStr, Diags, FS);
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
+  CIOpts.VFS = FS;
+  Invocation = createInvocation(ArgsCStr, std::move(CIOpts));
   assert(Invocation);
   Invocation->getFrontendOpts().DisableFree = false;
   Invocation->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Tooling/Syntax/TokensTest.cpp
===
--- clang/unittests/Tooling/Syntax/TokensTest.cpp
+++ clang/unittests/Tooling/Syntax/TokensTest.cpp
@@ -125,7 +125,10 @@
   Diags->setClient(new IgnoringDiagConsumer);
 std::vector Args = {"tok-test", "-std=c++03", "-fsyntax-only",
   FileName};
-auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+CreateInvocationOptions CIOpts;
+CIOpts.Diags = Diags;
+CIOpts.VFS = FS;
+auto CI = createInvocation(Args, std::move(CIOpts));
 assert(CI);
 CI->getFrontendOpts().DisableFree = false;
 CI->getPreprocessorOpts().addRemappedFile(
Index: clang/unittests/Serialization/ModuleCacheTest.cpp
===
--- clang/unittests/Serialization/ModuleCacheTest.cpp
+++ clang/unittests/Serialization/ModuleCacheTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/Utils.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
@@ -95,13 +96,15 @@
   MCPArg.append(ModuleCachePath);
   IntrusiveRefCntPtr Diags =
   CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CreateInvocationOptions CIOpts;
+  CIOpts.Diags = Diags;
 
   // First run should pass with no errors
   const char *Args[] = {"clang","-fmodules",  "-Fframeworks",
 MCPArg.c_str(), "-working-directory", TestDir.c_str(),
 "test.m"};
   std::shared_ptr Invocation =
-  createInvocationFromCommandLine(Args, Diags);
+  createInvocation(Args, CIOpts);
   ASSERT_TRUE(Invocation);
   CompilerInstance Instance;
   Instance.setDiagnostics(Diags.get());
@@ -124,7 +127,7 @@
  "-Fframeworks",  MCPArg.c_str(), "-working-directory",
  TestDir.c_str(), "test.m"};
   std::shared_ptr Invocation2 =
-  createInvocationFromCommandLine(Args2, Diags);
+  createInvocation(Args2, CIOpts);
   ASSERT_TRUE(Invocation2);
   CompilerInstance Instance2(Instance.getPCHContainerOperations(),
  &Instance.getModuleCache());
@@ -142,13 +145,15 @@
   MCPArg.append(

[Lldb-commits] [PATCH] D125012: [clang] createInvocationFromCommandLine -> createInvocation, delete former. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

Sorry, bad merge and this broke everything.
Fixed in f44552ab387b9087fb815251064782f8fb60e643


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125012

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

[Lldb-commits] [PATCH] D88103: [JSON] Add error reporting facility, used in fromJSON and ObjectMapper.

[Sam McCall via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfa69b608063e: [JSON] Add error reporting to fromJSON and 
ObjectMapper (authored by sammccall).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D88103?vs=293879&id=293891#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88103

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  lldb/source/Utility/StructuredData.cpp
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Analysis/TFUtils.cpp
  llvm/unittests/Support/JSONTest.cpp

Index: llvm/unittests/Support/JSONTest.cpp
===
--- llvm/unittests/Support/JSONTest.cpp
+++ llvm/unittests/Support/JSONTest.cpp
@@ -373,14 +373,21 @@
   return OS << "(" << S.S << ", " << (S.I ? std::to_string(*S.I) : "None")
 << ", " << S.B << ")";
 }
-bool fromJSON(const Value &E, CustomStruct &R) {
-  ObjectMapper O(E);
+bool fromJSON(const Value &E, CustomStruct &R, Path P) {
+  ObjectMapper O(E, P);
   if (!O || !O.map("str", R.S) || !O.map("int", R.I))
 return false;
   O.map("bool", R.B);
   return true;
 }
 
+static std::string errorContext(const Value &V, const Path::Root &R) {
+  std::string Context;
+  llvm::raw_string_ostream OS(Context);
+  R.printErrorContext(V, OS);
+  return OS.str();
+}
+
 TEST(JSONTest, Deserialize) {
   std::map> R;
   CustomStruct ExpectedStruct = {"foo", 42, true};
@@ -404,16 +411,58 @@
   CustomStruct("bar", llvm::None, false),
   CustomStruct("baz", llvm::None, false),
   };
-  ASSERT_TRUE(fromJSON(J, R));
+  Path::Root Root("CustomStruct");
+  ASSERT_TRUE(fromJSON(J, R, Root));
   EXPECT_EQ(R, Expected);
 
+  (*J.getAsObject()->getArray("foo"))[0] = 123;
+  ASSERT_FALSE(fromJSON(J, R, Root));
+  EXPECT_EQ("expected object at CustomStruct.foo[0]",
+toString(Root.getError()));
+  const char *ExpectedDump = R"({
+  "foo": [
+/* error: expected object */
+123,
+{ ... },
+{ ... }
+  ]
+})";
+  EXPECT_EQ(ExpectedDump, errorContext(J, Root));
+
   CustomStruct V;
-  EXPECT_FALSE(fromJSON(nullptr, V)) << "Not an object " << V;
-  EXPECT_FALSE(fromJSON(Object{}, V)) << "Missing required field " << V;
-  EXPECT_FALSE(fromJSON(Object{{"str", 1}}, V)) << "Wrong type " << V;
+  EXPECT_FALSE(fromJSON(nullptr, V, Root));
+  EXPECT_EQ("expected object when parsing CustomStruct",
+toString(Root.getError()));
+
+  EXPECT_FALSE(fromJSON(Object{}, V, Root));
+  EXPECT_EQ("missing value at CustomStruct.str", toString(Root.getError()));
+
+  EXPECT_FALSE(fromJSON(Object{{"str", 1}}, V, Root));
+  EXPECT_EQ("expected string at CustomStruct.str", toString(Root.getError()));
+
   // Optional must parse as the correct type if present.
-  EXPECT_FALSE(fromJSON(Object{{"str", 1}, {"int", "string"}}, V))
-  << "Wrong type for Optional " << V;
+  EXPECT_FALSE(fromJSON(Object{{"str", "1"}, {"int", "string"}}, V, Root));
+  EXPECT_EQ("expected integer at CustomStruct.int", toString(Root.getError()));
+}
+
+TEST(JSONTest, ParseDeserialize) {
+  auto E = parse>(R"json(
+[{"str": "foo", "int": 42}, {"int": 42}]
+  )json");
+  EXPECT_THAT_EXPECTED(E, FailedWithMessage("missing value at (root)[1].str"));
+
+  E = parse>(R"json(
+[{"str": "foo", "int": 42}, {"str": "bar"}
+  )json");
+  EXPECT_THAT_EXPECTED(
+  E,
+  FailedWithMessage("[3:2, byte=50]: Expected , or ] after array element"));
+
+  E = parse>(R"json(
+[{"str": "foo", "int": 42}]
+  )json");
+  EXPECT_THAT_EXPECTED(E, Succeeded());
+  EXPECT_THAT(*E, testing::SizeIs(1));
 }
 
 TEST(JSONTest, Stream) {
@@ -481,9 +530,6 @@
{"f", "a moderately long string: 48 characters in total"},
,
   };
-  std::string Err;
-  raw_string_ostream OS(Err);
-  R.printErrorContext(V, OS);
   const char *Expected = R"({
   "a": [ ... ],
   "b": {
@@ -496,7 +542,7 @@
 }
   }
 })";
-  EXPECT_EQ(Expected, OS.str());
+  EXPECT_EQ(Expected, errorContext(V, R));
 }
 
 } // namespace
Index: llvm/lib/Analysis/TFUtils.cpp
===
--- llvm/lib/Analysis/TFUtils.cpp
+++ llvm/lib/Analysis/TFUtils.cpp
@@ -104,7 +104,9 @@
 Ctx.emitError("Unable to parse JSON Value as spec (" + Message + "): " + S);
 return None;
   };
-  json::ObjectMapper Mapper(Value);
+  // FIXME: accept a Path as a parameter, and use it for error reporting.
+  json::Path::Root Root("tensor_spec");
+  json::ObjectMapper Mapper(Value, Root);
   if (!Mapper)
 return EmitError("Value is not a dict");
 
Index: llvm/inclu

[Lldb-commits] [PATCH] D88103: [JSON] Add error reporting facility, used in fromJSON and ObjectMapper.

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 293892.
sammccall added a comment.

This was landed as 4 commits, this diff is all 4 as committed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88103

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  lldb/source/Utility/StructuredData.cpp
  llvm/include/llvm/Support/JSON.h
  llvm/lib/Analysis/TFUtils.cpp
  llvm/lib/Support/JSON.cpp
  llvm/unittests/Support/JSONTest.cpp

Index: llvm/unittests/Support/JSONTest.cpp
===
--- llvm/unittests/Support/JSONTest.cpp
+++ llvm/unittests/Support/JSONTest.cpp
@@ -8,6 +8,7 @@
 
 #include "llvm/Support/JSON.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Error.h"
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -372,14 +373,21 @@
   return OS << "(" << S.S << ", " << (S.I ? std::to_string(*S.I) : "None")
 << ", " << S.B << ")";
 }
-bool fromJSON(const Value &E, CustomStruct &R) {
-  ObjectMapper O(E);
+bool fromJSON(const Value &E, CustomStruct &R, Path P) {
+  ObjectMapper O(E, P);
   if (!O || !O.map("str", R.S) || !O.map("int", R.I))
 return false;
   O.map("bool", R.B);
   return true;
 }
 
+static std::string errorContext(const Value &V, const Path::Root &R) {
+  std::string Context;
+  llvm::raw_string_ostream OS(Context);
+  R.printErrorContext(V, OS);
+  return OS.str();
+}
+
 TEST(JSONTest, Deserialize) {
   std::map> R;
   CustomStruct ExpectedStruct = {"foo", 42, true};
@@ -403,16 +411,58 @@
   CustomStruct("bar", llvm::None, false),
   CustomStruct("baz", llvm::None, false),
   };
-  ASSERT_TRUE(fromJSON(J, R));
+  Path::Root Root("CustomStruct");
+  ASSERT_TRUE(fromJSON(J, R, Root));
   EXPECT_EQ(R, Expected);
 
+  (*J.getAsObject()->getArray("foo"))[0] = 123;
+  ASSERT_FALSE(fromJSON(J, R, Root));
+  EXPECT_EQ("expected object at CustomStruct.foo[0]",
+toString(Root.getError()));
+  const char *ExpectedDump = R"({
+  "foo": [
+/* error: expected object */
+123,
+{ ... },
+{ ... }
+  ]
+})";
+  EXPECT_EQ(ExpectedDump, errorContext(J, Root));
+
   CustomStruct V;
-  EXPECT_FALSE(fromJSON(nullptr, V)) << "Not an object " << V;
-  EXPECT_FALSE(fromJSON(Object{}, V)) << "Missing required field " << V;
-  EXPECT_FALSE(fromJSON(Object{{"str", 1}}, V)) << "Wrong type " << V;
+  EXPECT_FALSE(fromJSON(nullptr, V, Root));
+  EXPECT_EQ("expected object when parsing CustomStruct",
+toString(Root.getError()));
+
+  EXPECT_FALSE(fromJSON(Object{}, V, Root));
+  EXPECT_EQ("missing value at CustomStruct.str", toString(Root.getError()));
+
+  EXPECT_FALSE(fromJSON(Object{{"str", 1}}, V, Root));
+  EXPECT_EQ("expected string at CustomStruct.str", toString(Root.getError()));
+
   // Optional must parse as the correct type if present.
-  EXPECT_FALSE(fromJSON(Object{{"str", 1}, {"int", "string"}}, V))
-  << "Wrong type for Optional " << V;
+  EXPECT_FALSE(fromJSON(Object{{"str", "1"}, {"int", "string"}}, V, Root));
+  EXPECT_EQ("expected integer at CustomStruct.int", toString(Root.getError()));
+}
+
+TEST(JSONTest, ParseDeserialize) {
+  auto E = parse>(R"json(
+[{"str": "foo", "int": 42}, {"int": 42}]
+  )json");
+  EXPECT_THAT_EXPECTED(E, FailedWithMessage("missing value at (root)[1].str"));
+
+  E = parse>(R"json(
+[{"str": "foo", "int": 42}, {"str": "bar"}
+  )json");
+  EXPECT_THAT_EXPECTED(
+  E,
+  FailedWithMessage("[3:2, byte=50]: Expected , or ] after array element"));
+
+  E = parse>(R"json(
+[{"str": "foo", "int": 42}]
+  )json");
+  EXPECT_THAT_EXPECTED(E, Succeeded());
+  EXPECT_THAT(*E, testing::SizeIs(1));
 }
 
 TEST(JSONTest, Stream) {
@@ -420,15 +470,19 @@
 std::string S;
 llvm::raw_string_ostream OS(S);
 OStream J(OS, Indent);
+J.comment("top*/level");
 J.object([&] {
   J.attributeArray("foo", [&] {
 J.value(nullptr);
+J.comment("element");
 J.value(42.5);
 J.arrayBegin();
 J.value(43);
 J.arrayEnd();
   });
+  J.comment("attribute");
   J.attributeBegin("bar");
+  J.comment("attribute value");
   J.objectBegin();
   J.objectEnd();
   J.attributeEnd();
@@ -437,22 +491,60 @@
 return OS.str();
   };
 
-  const char *Plain = R"({"foo":[null,42.5,[43]],"bar":{},"baz":"xyz"})";
+  const char *Plain =
+  R"(/*top* /level*/{"foo":[null,/*element*/42.5,[43]],/*attribute*/"bar":/*attribute value*/{},"baz":"xyz"})";
   EXPECT_EQ(Plain, StreamStuff(0));
-  const char *Pretty = R"({
+  const char *Pretty = R"(/* top* /level */
+{
   "foo": [
 null,
+/* element */
 42.5,
 [
   

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

Mostly LG, just a couple of possible logic bugs.

Apologies, I was out on vacation and hoped someone else would see this.




Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:650
 
+  bool Fallthrough() const { return ExternalFSValidWD && IsFallthrough; }
+

this name seems less than ideal:
 - it's very similar to `IsFallthrough` but has different semantics
 - "fallthrough" itself is not a very clear description of this functionality 
IMO
 - it's spelled wrong per the style guide

I'd suggest `shouldUseExternalFS()` or so



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1057
+auto EC = ExternalFS->setCurrentWorkingDirectory(Path);
+ExternalFSValidWD = static_cast(EC);
+  }

this seems backwards - error_code converts to true if it's an *error*

add tests?



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1061
+  // Don't change the working directory if the path doesn't exist.
+  if (!exists(Path))
+return errc::no_such_file_or_directory;

this seems like it should go at the top? cd to a nonexistent directory should 
avoid changing state and return an error (which means not marking 
ExternalFSValidWD as false, I think)



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1065
+  // Non-absolute paths are relative to the current working directory.
+  if (!sys::path::is_absolute(Path)) {
+SmallString<128> AbsolutePath;

makeAbsolute already does this check


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65185: Let tablegen generate property definitions

[Sam McCall via Phabricator via lldb-commits]

sammccall added subscribers: chandlerc, sammccall.
sammccall added inline comments.



Comment at: 
lldb/trunk/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp:77
+#define LLDB_PROPERTIES_dynamicloaderdarwinkernel
+#include "Properties.inc"
+};

These unqualified `#include` directives are causing us some problems 
downstream. What do you think about qualifying these like other includes in the 
file? i.e. `#include "Plugins/DynamicLoader/Darwin-Kernel/Properties.inc"`

The problem is that these "local" includes can only resolve to the right thing 
if the build system specifies a different `-I` line to each relevant cpp file 
(or the header lives next to the source file, which isn't the case for 
generated files).
Many build systems (including both CMake and bazel) only support doing this for 
separate libraries, however lldb's source isn't sufficiently clearly layered to 
allow splitting into fine-grained libraries in bazel (CMake is much more lax 
here).

While CMake is the gold standard here, it'd be helpful to make it possible to 
build with other systems out-of-tree. @chandlerc was looking at this issue and 
has more context and opinions here.

It also seems a bit less magic and more consistent with the rest of the 
includes in the file, as well as with the project. (Most, though sadly not all, 
.inc files in llvm are included by qualified paths).

I'll send a patch shortly after verifying it fixes the issue.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65185



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

[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall created this revision.
sammccall added reviewers: JDevlieghere, labath, chandlerc.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
labath added a comment.

I am not sure about the consistency argument (on the other CL), as it seems to 
me that most/all llvm .inc files *which are produced by tblgen" are included by 
via just their bare names. However, these files generally have unique names, 
which is usually achieved by just embedding (a derivative of) the folder name 
into the .inc file name (e.g. AArch64GenAsmWriter.inc, AMDGPUGenAsmWriter.inc, 
...). So, I think the most consistent approach here would be to rename lldb 
files into something like `TargetProperties.inc`, `CoreProperties.inc` etc.

Consistency considerations aside, I do think that having a bunch of files 
called `Properties.inc` is a bit confusing, and it would be good to 
disambiguate between them. I'm fairly ambivalent about whether that is achieved 
by prefixing the include directives with the folder name, or by renaming the 
files so that they have unique names.


This is a bit more explicit, and makes it possible to build LLDB without
varying the -I lines per-directory.
(The latter is useful because many build systems only allow this to be
configured per-library, and LLDB is insufficiently layered to be split into
multiple libraries on stricter build systems).

(My comment on D65185  has some more context)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65397

Files:
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -65,12 +65,12 @@
 
 static constexpr PropertyDefinition g_properties[] = {
 #define LLDB_PROPERTIES_thread
-#include "Properties.inc"
+#include "Target/Properties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_thread
-#include "PropertiesEnum.inc"
+#include "Target/PropertiesEnum.inc"
 };
 
 class ThreadOptionValueProperties : public OptionValueProperties {
Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3275,12 +3275,12 @@
 
 static constexpr PropertyDefinition g_properties[] = {
 #define LLDB_PROPERTIES_target
-#include "Properties.inc"
+#include "Target/Properties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_target
-#include "PropertiesEnum.inc"
+#include "Target/PropertiesEnum.inc"
   ePropertyExperimental,
 };
 
@@ -3358,12 +3358,12 @@
 // TargetProperties
 static constexpr PropertyDefinition g_experimental_properties[]{
 #define LLDB_PROPERTIES_experimental
-#include "Properties.inc"
+#include "Target/Properties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_experimental
-#include "PropertiesEnum.inc"
+#include "Target/PropertiesEnum.inc"
 };
 
 class TargetExperimentalOptionValueProperties : public OptionValueProperties {
Index: lldb/source/Target/Process.cpp
===
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -114,12 +114,12 @@
 
 static constexpr PropertyDefinition g_properties[] = {
 #define LLDB_PROPERTIES_process
-#include "Properties.inc"
+#include "Target/Properties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_process
-#include "PropertiesEnum.inc"
+#include "Target/PropertiesEnum.inc"
 };
 
 ProcessProperties::ProcessProperties(lldb_private::Process *process)
Index: lldb/source/Target/Platform.cpp
===
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -65,12 +65,12 @@
 
 static constexpr PropertyDefinition g_properties[] = {
 #define LLDB_PROPERTIES_platform
-#include "Properties.inc"
+#include "Target/Properties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_platform
-#include "PropertiesEnum.inc"
+#include "Target/PropertiesEnum.inc"
 };
 
 } // namespace
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -115,12 +115,12 @@

[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 212166.
sammccall added a comment.
Herald added a subscriber: mgorny.
Herald added a reviewer: jdoerfert.

Give the files distinct names instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65397

Files:
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/Properties.td
  lldb/source/Interpreter/CMakeLists.txt
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/InterpreterProperties.td
  lldb/source/Interpreter/Properties.td
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernelProperties.td
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/Properties.td
  lldb/source/Plugins/JITLoader/GDB/CMakeLists.txt
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDBProperties.td
  lldb/source/Plugins/JITLoader/GDB/Properties.td
  lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
  lldb/source/Plugins/Platform/MacOSX/Properties.td
  lldb/source/Plugins/Process/MacOSX-Kernel/CMakeLists.txt
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDPProperties.td
  lldb/source/Plugins/Process/MacOSX-Kernel/Properties.td
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
  lldb/source/Plugins/Process/gdb-remote/Properties.td
  lldb/source/Plugins/StructuredData/DarwinLog/CMakeLists.txt
  lldb/source/Plugins/StructuredData/DarwinLog/Properties.td
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLogProperties.td
  lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  lldb/source/Plugins/SymbolFile/DWARF/Properties.td
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Properties.td
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -65,12 +65,12 @@
 
 static constexpr PropertyDefinition g_properties[] = {
 #define LLDB_PROPERTIES_thread
-#include "Properties.inc"
+#include "TargetProperties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_thread
-#include "PropertiesEnum.inc"
+#include "TargetPropertiesEnum.inc"
 };
 
 class ThreadOptionValueProperties : public OptionValueProperties {
Index: lldb/source/Target/Properties.td
===
--- /dev/null
+++ lldb/source/Target/Properties.td
@@ -1,234 +0,0 @@
-include "../../include/lldb/Core/PropertiesBase.td"
-
-let Definition = "experimental" in {
-  def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
-Global, DefaultTrue,
-Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
-  def UseModernTypeLookup : Property<"use-modern-type-lookup", "Boolean">,
-Global, DefaultFalse,
-Desc<"If true, use Clang's modern type lookup infrastructure.">;
-}
-
-let Definition = "target" in {
-  def DefaultArch: Property<"default-arch", "Arch">,
-Global,
-DefaultStringValue<"">,
-Desc<"Default architecture to choose, when there's a choice.">;
-  def MoveToNearestCode: Property<"move-to-nearest-code", "Boolean">,
-DefaultTrue,
-Desc<"Move breakpoints to nearest code.">;
-  def Language: Property<"language", "Language">,
-DefaultEnumValue<"eLanguageTypeUnknown">,
-Desc<"The language to use when interpreting expressions entered in commands.">;
-  def ExprPrefix: Property<"expr-prefix", "FileSpec">,
-DefaultStringValue<"">,
-Desc<"Path to a file containing expressions to be prepended to all expressions.">;
-  def PreferDynamic: Property<"prefer-dynamic-value", "Enum">,
-DefaultEnumValue<"eDynamicDontRunTarget">,
-EnumValues<"OptionEnumValues(g_dynamic_value_types)">,
-Desc<"Should printed values be shown as their dynamic value.">;
-  def EnableSynthetic: Property<"enable-synthetic-value", "Boolean">,
-Default

[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall updated this revision to Diff 212167.
sammccall added a comment.

Fix one straggler


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65397

Files:
  lldb/source/Core/CMakeLists.txt
  lldb/source/Core/CoreProperties.td
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Core/Properties.td
  lldb/source/Interpreter/CMakeLists.txt
  lldb/source/Interpreter/CommandInterpreter.cpp
  lldb/source/Interpreter/InterpreterProperties.td
  lldb/source/Interpreter/Properties.td
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/CMakeLists.txt
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernel.cpp
  
lldb/source/Plugins/DynamicLoader/Darwin-Kernel/DynamicLoaderDarwinKernelProperties.td
  lldb/source/Plugins/DynamicLoader/Darwin-Kernel/Properties.td
  lldb/source/Plugins/JITLoader/GDB/CMakeLists.txt
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDB.cpp
  lldb/source/Plugins/JITLoader/GDB/JITLoaderGDBProperties.td
  lldb/source/Plugins/JITLoader/GDB/Properties.td
  lldb/source/Plugins/Platform/MacOSX/CMakeLists.txt
  lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
  lldb/source/Plugins/Platform/MacOSX/PlatformMacOSXProperties.td
  lldb/source/Plugins/Platform/MacOSX/Properties.td
  lldb/source/Plugins/Process/MacOSX-Kernel/CMakeLists.txt
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
  lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDPProperties.td
  lldb/source/Plugins/Process/MacOSX-Kernel/Properties.td
  lldb/source/Plugins/Process/gdb-remote/CMakeLists.txt
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemoteProperties.td
  lldb/source/Plugins/Process/gdb-remote/Properties.td
  lldb/source/Plugins/StructuredData/DarwinLog/CMakeLists.txt
  lldb/source/Plugins/StructuredData/DarwinLog/Properties.td
  lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  
lldb/source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLogProperties.td
  lldb/source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  lldb/source/Plugins/SymbolFile/DWARF/Properties.td
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/Properties.td
  lldb/source/Target/Target.cpp
  lldb/source/Target/TargetProperties.td
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -65,12 +65,12 @@
 
 static constexpr PropertyDefinition g_properties[] = {
 #define LLDB_PROPERTIES_thread
-#include "Properties.inc"
+#include "TargetProperties.inc"
 };
 
 enum {
 #define LLDB_PROPERTIES_thread
-#include "PropertiesEnum.inc"
+#include "TargetPropertiesEnum.inc"
 };
 
 class ThreadOptionValueProperties : public OptionValueProperties {
Index: lldb/source/Target/Properties.td
===
--- /dev/null
+++ lldb/source/Target/Properties.td
@@ -1,234 +0,0 @@
-include "../../include/lldb/Core/PropertiesBase.td"
-
-let Definition = "experimental" in {
-  def InjectLocalVars : Property<"inject-local-vars", "Boolean">,
-Global, DefaultTrue,
-Desc<"If true, inject local variables explicitly into the expression text. This will fix symbol resolution when there are name collisions between ivars and local variables. But it can make expressions run much more slowly.">;
-  def UseModernTypeLookup : Property<"use-modern-type-lookup", "Boolean">,
-Global, DefaultFalse,
-Desc<"If true, use Clang's modern type lookup infrastructure.">;
-}
-
-let Definition = "target" in {
-  def DefaultArch: Property<"default-arch", "Arch">,
-Global,
-DefaultStringValue<"">,
-Desc<"Default architecture to choose, when there's a choice.">;
-  def MoveToNearestCode: Property<"move-to-nearest-code", "Boolean">,
-DefaultTrue,
-Desc<"Move breakpoints to nearest code.">;
-  def Language: Property<"language", "Language">,
-DefaultEnumValue<"eLanguageTypeUnknown">,
-Desc<"The language to use when interpreting expressions entered in commands.">;
-  def ExprPrefix: Property<"expr-prefix", "FileSpec">,
-DefaultStringValue<"">,
-Desc<"Path to a file containing expressions to be prepended to all expressions.">;
-  def PreferDynamic: Property<"prefer-dynamic-value", "Enum">,
-DefaultEnumValue<"eDynamicDontRunTarget">,
-EnumValues<"OptionEnumValues(g_dynamic_value_types)">,
-Desc<"Should printed values be shown as their dynamic value.">;
-  def EnableSynthetic: Property<"enable-synthetic-value", "Boolean">,
-DefaultTrue,
-Desc<"Should synthetic values be used by default whenever available.">;
-  def Sk

[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

In D65397#1604416 , @labath wrote:

> I am not sure about the consistency argument (on the other CL), as it seems 
> to me that most/all llvm .inc files *which are produced by tblgen" are 
> included by via just their bare names. However, these files generally have 
> unique names, which is usually achieved by just embedding (a derivative of) 
> the folder name into the .inc file name (e.g. AArch64GenAsmWriter.inc, 
> AMDGPUGenAsmWriter.inc, ...). So, I think the most consistent approach here 
> would be to rename lldb files into something like `TargetProperties.inc`, 
> `CoreProperties.inc` etc.
>
> Consistency considerations aside, I do think that having a bunch of files 
> called `Properties.inc` is a bit confusing, and it would be good to 
> disambiguate between them. I'm fairly ambivalent about whether that is 
> achieved by prefixing the include directives with the folder name, or by 
> renaming the files so that they have unique names.


Fair enough, I've given them names that match the main file in that directory 
(thanks for pointing out this convention!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65397



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

[Lldb-commits] [PATCH] D65397: Qualify includes of Properties[Enum].inc files. NFC

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

Thanks @rupprecht!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65397



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Support encoding a current working directory in a VFS mapping.

[Sam McCall via Phabricator via lldb-commits]

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

It seems conceptually a little strange to have the working directory be part of 
a serialized "FS", as it's fundamentally a property of a process and only 
transiently a property of the VFS.
I don't know this is fatal (not sure what else the RedirectingFileSystem might 
be used for.

What's the reason to store this as part of the VFS rather than as a separate 
attribute of the larger reproducer structure?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Support encoding a current working directory in a VFS mapping.

[Sam McCall via Phabricator via lldb-commits]

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

Oops, didn't mean to mark as accepted.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Support encoding a current working directory in a VFS mapping.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

> I'd like to know if you tried other approaches and why they failed.

+1. In particular, what goes wrong if you try to make the working directory a 
sibling of VFS (within the reproducer container) rather than a child of it 
(within shared infrastructure)?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Support encoding working directories in a VFS mapping.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

In D65677#1627576 , @JDevlieghere 
wrote:

> After some brainstorming I've identified a few other approaches that should 
> better reflect the transience of the current working directory:
>
> - We can modify the VFS to have a notion of //search paths//. The 
> `adjustPath` function could iterate over the search paths until it finds an 
> absolute path that exists. If none exist we'd return the same path we return 
> today. This would be the most general approach.
> - We could create a new virtual file system that we put on top of the 
> RedirectingFileSystem which does something similar to what I've described 
> above. This would require us to override every function that calls 
> `adjustPath`, so it would be pretty heavyweight and rather inflexible.
>
>   I'd like to get your feedback before I start implementing these. What do 
> you think? Is there another approach that's worth considering?


I'm really sorry for missing this comment, I was out sick and missed the email.

> I'd like to get your feedback before I start implementing these.

Honestly, this still seems way too complicated and this doesn't seem like a 
feature that needs to be part of VFS.

>   What do you think? Is there another approach that's worth considering?

Per my previous comment, what goes wrong if you try to make the working 
directory a sibling of VFS (within the reproducer container) rather than a 
child of it (within shared infrastructure)?


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Support encoding working directories in a VFS mapping.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

In D65677#1649291 , @JDevlieghere 
wrote:

> In D65677#1648506 , @sammccall wrote:
>
> > In D65677#1627576 , @JDevlieghere 
> > wrote:
> >
> > > After some brainstorming I've identified a few other approaches that 
> > > should better reflect the transience of the current working directory:
> > >
> > > - We can modify the VFS to have a notion of //search paths//. The 
> > > `adjustPath` function could iterate over the search paths until it finds 
> > > an absolute path that exists. If none exist we'd return the same path we 
> > > return today. This would be the most general approach.
> > > - We could create a new virtual file system that we put on top of the 
> > > RedirectingFileSystem which does something similar to what I've described 
> > > above. This would require us to override every function that calls 
> > > `adjustPath`, so it would be pretty heavyweight and rather inflexible.
> > >
> > >   I'd like to get your feedback before I start implementing these. What 
> > > do you think? Is there another approach that's worth considering?
> >
> >
> > I'm really sorry for missing this comment, I was out sick and missed the 
> > email.
> >
> > > I'd like to get your feedback before I start implementing these.
> >
> > Honestly, this still seems way too complicated and this doesn't seem like a 
> > feature that needs to be part of VFS.
>
>
> It's funny you say that, because the code to resolve relative paths is almost 
> identical to the thing you added for supporting different working directories 
> in different threads. :-)


Right! I think the key distinction is that there wasn't any functional change 
to the APIs, because the abstraction didn't change.
(There was a second factory function, which basically lets callers choose 
between the two implementations that have different sets of bugs)

>>>   What do you think? Is there another approach that's worth considering?
>> 
>> Per my previous comment, what goes wrong if you try to make the working 
>> directory a sibling of VFS (within the reproducer container) rather than a 
>> child of it (within shared infrastructure)?
> 
> Oops, seems like I didn't see your question either :-) Please clarify what 
> you mean by sibling and child. Do you mean that the working directories are 
> part of the mapping or that the redirecting file system knows about it? I 
> don't care where we store the entries, I'm happy to have a separate YAML 
> mapping that only the LLDB reproducers know about. However, I do need the 
> underlying logic to be part of the (redirecting) VFS. Just like clang, LLDB 
> is agnostic to the VFS, so this whole thing should be transparent. The only 
> reason I didn't keep them separate is because then you need a way to tell the 
> redirecting file system about the working directories, which means you need 
> to get the concrete VFS, i.e. casting the VFS to a RedirectingVFS, which I 
> try to avoid.

I mean why can't you just call `setCurrentWorkingDirectory` before starting? 
something like this:

  struct Reproducer { string FSYaml; string CWD; ... };
  void run(Reproducer &R) {
auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);
FS->setCurrentWorkingDirectory(R.CWD);
runLLDBWith(FS, ...);
  }


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Support encoding working directories in a VFS mapping.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

(I replied by email, it seems phabricator doesn't pick it up.)

In D65677#1649731 , @JDevlieghere 
wrote:

> In D65677#1649329 , @sammccall wrote:
>
> > In D65677#1649291 , @JDevlieghere 
> > wrote:
> >
> > > It's funny you say that, because the code to resolve relative paths is 
> > > almost identical to the thing you added for supporting different working 
> > > directories in different threads. :-)
> >
> >
> > Right! I think the key distinction is that there wasn't any functional 
> > change to the APIs, because the abstraction didn't change.
> >  (There was a second factory function, which basically lets callers choose 
> > between the two implementations that have different sets of bugs)
> >
> > >>>   What do you think? Is there another approach that's worth considering?
> > >> 
> > >> Per my previous comment, what goes wrong if you try to make the working 
> > >> directory a sibling of VFS (within the reproducer container) rather than 
> > >> a child of it (within shared infrastructure)?
> > > 
> > > Oops, seems like I didn't see your question either :-) Please clarify 
> > > what you mean by sibling and child. Do you mean that the working 
> > > directories are part of the mapping or that the redirecting file system 
> > > knows about it? I don't care where we store the entries, I'm happy to 
> > > have a separate YAML mapping that only the LLDB reproducers know about. 
> > > However, I do need the underlying logic to be part of the (redirecting) 
> > > VFS. Just like clang, LLDB is agnostic to the VFS, so this whole thing 
> > > should be transparent. The only reason I didn't keep them separate is 
> > > because then you need a way to tell the redirecting file system about the 
> > > working directories, which means you need to get the concrete VFS, i.e. 
> > > casting the VFS to a RedirectingVFS, which I try to avoid.
> >
> > I mean why can't you just call `setCurrentWorkingDirectory` before 
> > starting? something like this:
> >
> >   struct Reproducer { string FSYaml; string CWD; ... };
> >   void run(Reproducer &R) {
> > auto* FS = RedirectingFileSystem::create(R.FSYaml, ...);
> > FS->setCurrentWorkingDirectory(R.CWD);
> > runLLDBWith(FS, ...);
> >   }
> >  
> >
>
>
> That doesn't work for the reproducer because the working directory likely 
> doesn't exist during replay. The redirecting file system just forwards the 
> `setCurrentWorkingDirectory` call to it's underlying (real) FS, so this 
> becomes a no-op.


Can we fix that instead? The RedirectingVFS already knows about virtual 
directory entries, being able to cd to them makes sense.

This seems mostly like fixing a bug/limitation that wouldn't involve API 
changes.

Is there a part of the problem that needs the new search path concept/multiple 
paths?


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a subscriber: benlangmuir.
sammccall added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1051
 std::error_code
 RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
+  // Don't change the working directory if the path doesn't exist.

This is a change in behavior, in that the WD of the underlying filesystem is no 
longer set, so e.g. in overlay mode RedirectingFileSystem::status() will look 
for non-overlaid relative paths in the wrong directory.

e.g. if we're using an overlay VFS with no files mapped and external WD is 
/foo, `cd /bar; stat baz` reads /bar/baz before this patch and reads /foo/baz 
after it. The old behavior seems more logical.

I think the right thing to do is to have setCurrentWorkingDirectory() call 
ExternalFS->setCurrentWorkingDirectory() if overlay mode is on. It shouldn't 
propagate failure, but rather record it `bool ExternalFSValidWD`, and only 
allow fallthrough operations when this flag is true.
This means cd into a dir which is invalid in the underlying FS would put it 
into a "broken" state where it's no longer consulted, but an (absolute) cd to a 
valid directory would fix it. (Variations are possible, depending on how you 
want to handle relative cd when the CWD is invalid in the underlying FS).

If you would rather just change the behavior here, you probably need to talk to 
someone familiar with the existing clients. It seems mostly used by clang's 
`-ivfsoverlay`, which was added by @benlangmuir way back when. I don't really 
know what it's used for.


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

https://reviews.llvm.org/D65677



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

[Lldb-commits] [PATCH] D65677: [VirtualFileSystem] Make the RedirectingFileSystem hold on to its own working directory.

[Sam McCall via Phabricator via lldb-commits]

sammccall added a comment.

BTW, This approach does look much better to me, it fits well into the VFS 
abstraction and I'd argue is fixing a bug in RedirectingFileSystem.
We do need to be careful about introducing new bugs, though. (or avoid the 
generality and just implement the parts LLDB needs, as a separate vfs::FS)


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

https://reviews.llvm.org/D65677



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