[Lldb-commits] [PATCH] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 445202.
Michael137 added a comment.

- Add ValueObjectProvider so materializer doesn't use incorrect ValueObject 
when re-using UserExpressions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

Files:
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Expression/UserExpression.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/commands/expression/expr_inside_lambda/Makefile
  lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
  lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
  lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile
  
lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
  lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp

Index: lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
@@ -0,0 +1,32 @@
+#include 
+#include 
+#include 
+
+struct Foo {
+  bool enable = true;
+  uint32_t offset = 0;
+
+  void usleep_helper(uint32_t usec) {
+[this, &usec] {
+  puts("Break here in the helper");
+  std::this_thread::sleep_for(
+  std::chrono::duration(offset + usec));
+}();
+  }
+};
+
+void *background_thread(void *) {
+  Foo f;
+  for (;;) {
+f.usleep_helper(2);
+  }
+}
+
+int main() {
+  std::puts("First break");
+  std::thread main_thread(background_thread, nullptr);
+  Foo f;
+  for (;;) {
+f.usleep_helper(1);
+  }
+}
Index: lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
@@ -0,0 +1,54 @@
+"""
+Test that if we hit a breakpoint on a lambda capture
+on two threads at the same time we stop only for
+the correct one.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestBreakOnLambdaCapture(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_break_on_lambda_capture(self):
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+
+(target, process, main_thread, _) = lldbutil.run_to_source_breakpoint(self,
+"First break", self.main_source_file)
+
+# FIXME: This is working around a separate bug. If you hit a breakpoint and
+# run an expression and it is the first expression you've ever run, on
+# Darwin that will involve running the ObjC runtime parsing code, and we'll
+# be in the middle of that when we do PerformAction on the other thread,
+# which will cause the condition expression to fail.  Calling another
+# expression first works around this.
+val_obj = main_thread.frame[0].EvaluateExpression("true")
+self.assertSuccess(val_obj.GetError(), "Ran our expression successfully")
+self.assertEqual(val_obj.value, "true", "Value was true.")
+
+bkpt = target.BreakpointCreateBySourceRegex("Break here in the helper",
+self.main_source_file);
+
+bkpt.SetCondition("enable && usec == 1")
+process.Continue()
+
+# This is hard to test definitively, becuase it requires hitting
+# a breakpoint on multiple threads at the same time.  On Darwin, this
+# will happen pretty much ever time we continue.  What we are really
+# asserting is that we only ever stop on one thread, so we approximate that
+# by continuing 20 times and assert we only ever hit the first thread.  Either
+# this is a platform that only reports one hit at a time, in which case all
+# this code is unused, or we actually didn't hit the other thread.
+
+  

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

[Kim Gräsman via Phabricator via lldb-commits]

kimgr added a comment.

It's a little confusing, because it now looks like _every_ `Type` in the AST is 
wrapped in an `ElaboratedTypeLoc` + `ElaboratedType`. IWYU's debug AST dump 
shows this (excerpt):

  tests/cxx/sizeof_reference.cc:51:8: (1) [ VarDecl ] size_t s2 

 
  tests/cxx/sizeof_reference.cc:51:1: (2) [ ElaboratedTypeLoc ] size_t  

 
  tests/cxx/sizeof_reference.cc:51:1: (2) [ ElaboratedType ] size_t 

 
  tests/cxx/sizeof_reference.cc:51:1: (3) [ TypedefTypeLoc ] size_t 

 
  tests/cxx/sizeof_reference.cc:51:1: (3) [ TypedefType ] size_t

 
  Marked full-info use of decl size_t (from 
/home/kimgr/code/iwyu/out/main/lib/clang/15.0.0/include/stddef.h:46:23) at 
tests/cxx/sizeof_reference.cc:51:1
  tests/cxx/sizeof_reference.cc:51:13: (2) [ UnaryExprOrTypeTraitExpr ] 
UnaryExprOrTypeTraitExpr 0x5589fd2a4230 'unsigned long' sizeof 
'IndirectTemplateStruct &' 


 
  (For type IndirectTemplateStruct): 

 
  Marked full-info use of decl IndirectTemplateStruct (from 
tests/cxx/sizeof_reference.cc:18:30) at tests/cxx/sizeof_reference.cc:51:20 
 
  tests/cxx/sizeof_reference.cc:51:20: (3) [ LValueReferenceTypeLoc ] 
IndirectTemplateStruct & 
   
  tests/cxx/sizeof_reference.cc:51:20: (3) [ LValueReferenceType ] 
IndirectTemplateStruct & 
  
  tests/cxx/sizeof_reference.cc:51:20: (4) [ ElaboratedTypeLoc ] 
IndirectTemplateStruct   

  tests/cxx/sizeof_reference.cc:51:20: (4) [ ElaboratedType ] 
IndirectTemplateStruct   
   
  tests/cxx/sizeof_reference.cc:51:20: (5) [ TemplateSpecializationTypeLoc ] 
IndirectTemplateStruct   

  tests/cxx/sizeof_reference.cc:51:20: (5) [ TemplateSpecializationType ] 
IndirectTemplateStruct   
   
  Marked fwd-decl use of decl IndirectTemplateStruct (from 
tests/cxx/sizeof_reference.cc:18:30) at tests/cxx/sizeof_reference.cc:51:20 
  
  tests/cxx/sizeof_reference.cc:51:20: (6, fwd decl) [ TemplateName ] 
IndirectTemplateStruct  
   
  tests/cxx/sizeof_reference.cc:51:43: (6, fwd decl) [ TemplateArgumentLoc ] 
 

  tests/cxx/sizeof_reference.cc:51:43: (7, fwd decl) [ ElaboratedTypeLoc ] 
IndirectClass   
  
  tests/cxx/sizeof_reference.cc:51:43: (7, fwd decl) [ ElaboratedType ] 
IndirectClass   
 
  tests/cxx/sizeof_reference.cc:51:43: (8, fwd decl) [ RecordTypeLoc ] class 
IndirectClass   

  tests/cxx/sizeof_reference.cc:51:43: (8, fwd decl) [ RecordType ] class 
IndirectClass   
   
  Marked fwd-decl use

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

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov reopened this revision.
mizvekov added a comment.

In D112374#3653967 , @JDevlieghere 
wrote:

> I'm sorry to hear you're having trouble building LLDB. The LLDB website has 
> quite an elaborate guide with instructions in how to build LLDB: 
> https://lldb.llvm.org/resources/build.html, including specific instructions 
> on Windows.

The instructions exist, doesn't mean they work or that they are a fair burden 
on the developers of the other projects.

The LLDB build / test system is made of the same 'stuff' as the rest of LLVM 
sure, but it does a lot more 'questionable' things which makes it look more 
hazardous.
For example, opening hundreds of frozen terminal windows or creating paths in 
the build directory which contain unsubstituted variables.
So it seems to me that for me to be comfortable working with this, I would have 
to adjust my workflow to build LLVM in a sandbox instead.

If we tried polling other clang devs, we might find that standard practice is 
that we are not really even trying to build and run LLDB tests locally anymore.

libcxx devs have a CI pre-commit system which only runs when a path in their 
sub-project is touched. I think this would be reasonable start for LLDB.

Lastly, my main concern is that by keeping this patch off, even if we don't 
suspect a problem in it, this will create a very long tail. The users affected 
don't know about it yet, and they will keep coming
with a delay one by one as we re-land and revert.

> I'm happy to help out. I personally don't know if we should go with (1) or 
> (2), both sound reasonable in their own way. I'd like @teemperor, who's the 
> author of the feature and the affected tests, to weigh in.

I think, but can't confirm, that this is just a case of (1) and for what is 
being tested we don't really care how the return type of those size() methods 
is written.
I would like some way to test that the functionality is not really broken, we 
just changed that test expectation, but alas I can't.

> I don't. I think reverting your change was well within the guidelines 
> outlined by LLVM's patch reversion policy: 
> https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy
>
> Additionally, I think you could've given me a little bit more time to catch 
> up on the discussion here. The code review policy and practices 
> (https://llvm.org/docs/CodeReview.html#code-reviews-speed-and-reciprocity) 
> recommend pinging every few days to once per week depending on how urgent the 
> patch is.

I think, given the size of this patch and the other points I made, we could 
have simply fixed those issues post-commit, if I had received any prior 
notification.

In D112374#3657332 , @kimgr wrote:

> It's a little confusing, because it now looks like _every_ `Type` in the AST 
> is wrapped in an `ElaboratedTypeLoc` + `ElaboratedType`. IWYU's debug AST 
> dump shows this (excerpt):
> I'm not sure I understand why the elaborated type nodes are necessary even 
> when they don't seem to add any additional information?

It's the difference in knowing the type was written without any tag or 
nested-name specifier, and having a type that you are not sure how it was 
written.

When we are dealing with a type which we are not sure, we would like to print 
it fully qualified, with a synthetic nested name specifier computed from it's 
DC, because otherwise it could be confusing as the type could come from 
somewhere very distant from the context we are printing the type from. We would 
not want to assume that a type which has been desugared was written how it's 
desugared state would seem to imply.

FWIW, in the state of affairs we leave clang after this patch, I don't think 
it's worth keeping a separate ElaboratedType anymore, we might as well fuse 
it's functionality into the type nodes which could be wrapped in it. Taking 
care to optimize storage when not used otherwise, I think we can recoup the 
performance lost in this patch, perhaps even end in a better state overall.

But I think doing these two steps in one go would not be sensibly incremental. 
We have in this patch here a very simple core change, which is very unlikely to 
have bugs in itself, but creates enormous test churn.

The second step of eliminating ElaboratedType could be a less simple core 
change with almost zero test churn, which makes it less risky that it would 
introduce a bug that escapes review.


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] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Kim Gräsman via Phabricator via lldb-commits]

kimgr added a comment.

> It's the difference in knowing the type was written without any tag or 
> nested-name specifier, and having a type that you are not sure how it was 
> written.
>
> When we are dealing with a type which we are not sure, we would like to print 
> it fully qualified, with a synthetic nested name specifier computed from it's 
> DC,
> because otherwise it could be confusing as the type could come from somewhere 
> very distant from the context we are printing the type from. We would not
> want to assume that a type which has been desugared was written how it's 
> desugared state would seem to imply.

I'm coming at this from pretty far away, so there's very likely lots of details 
that I'm overlooking. But it seems to me the mainline had only had an 
`ElaboratedType` node if there was elaboration, and not otherwise. And that 
makes a lot more sense to me than having 2 `ElaboratedType*` nodes _for every 
type in the AST_, just to express that "hey, by the way, this type had no 
elaboration".

> FWIW, in the state of affairs we leave clang after this patch, I don't think 
> it's worth keeping a separate ElaboratedType anymore, we might as 
> well fuse it's functionality into the type nodes which could be wrapped in 
> it. Taking care to optimize storage when not used otherwise, I think
> we can recoup the performance lost in this patch, perhaps even end in a 
> better state overall.
>
> But I think doing these two steps in one go would not be sensibly 
> incremental. We have in this patch here a very simple core change, which
> is very unlikely to have bugs in itself, but creates enormous test churn.
>
> The second step of eliminating ElaboratedType could be a less simple core 
> change with almost zero test churn, which makes it less risky that
> it would introduce a bug that escapes review.

That sounds good at face value, but if you're planning to remove these nodes 
again, that would create enormous churn for out-of-tree tools to re-adjust to 
the new shape of the tree.

I can't say what the best solution is, but this patch generates quite a lot of 
work for me, and I would really hope that catching up with the new AST does not 
generate even more work down the line.


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] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 445259.
Michael137 added a comment.

- Remove now redundant m_lldb_value_object


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

Files:
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Expression/UserExpression.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/commands/expression/expr_inside_lambda/Makefile
  lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
  lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
  lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile
  
lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
  lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp

Index: lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
@@ -0,0 +1,32 @@
+#include 
+#include 
+#include 
+
+struct Foo {
+  bool enable = true;
+  uint32_t offset = 0;
+
+  void usleep_helper(uint32_t usec) {
+[this, &usec] {
+  puts("Break here in the helper");
+  std::this_thread::sleep_for(
+  std::chrono::duration(offset + usec));
+}();
+  }
+};
+
+void *background_thread(void *) {
+  Foo f;
+  for (;;) {
+f.usleep_helper(2);
+  }
+}
+
+int main() {
+  std::puts("First break");
+  std::thread main_thread(background_thread, nullptr);
+  Foo f;
+  for (;;) {
+f.usleep_helper(1);
+  }
+}
Index: lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
@@ -0,0 +1,54 @@
+"""
+Test that if we hit a breakpoint on a lambda capture
+on two threads at the same time we stop only for
+the correct one.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestBreakOnLambdaCapture(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_break_on_lambda_capture(self):
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+
+(target, process, main_thread, _) = lldbutil.run_to_source_breakpoint(self,
+"First break", self.main_source_file)
+
+# FIXME: This is working around a separate bug. If you hit a breakpoint and
+# run an expression and it is the first expression you've ever run, on
+# Darwin that will involve running the ObjC runtime parsing code, and we'll
+# be in the middle of that when we do PerformAction on the other thread,
+# which will cause the condition expression to fail.  Calling another
+# expression first works around this.
+val_obj = main_thread.frame[0].EvaluateExpression("true")
+self.assertSuccess(val_obj.GetError(), "Ran our expression successfully")
+self.assertEqual(val_obj.value, "true", "Value was true.")
+
+bkpt = target.BreakpointCreateBySourceRegex("Break here in the helper",
+self.main_source_file);
+
+bkpt.SetCondition("enable && usec == 1")
+process.Continue()
+
+# This is hard to test definitively, becuase it requires hitting
+# a breakpoint on multiple threads at the same time.  On Darwin, this
+# will happen pretty much ever time we continue.  What we are really
+# asserting is that we only ever stop on one thread, so we approximate that
+# by continuing 20 times and assert we only ever hit the first thread.  Either
+# this is a platform that only reports one hit at a time, in which case all
+# this code is unused, or we actually didn't hit the other thread.
+
+for idx in range(0, 20):
+process.Continue()
+

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

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov added a comment.

In D112374#3657472 , @kimgr wrote:

> I'm coming at this from pretty far away, so there's very likely lots of 
> details that I'm overlooking. But it seems to me the mainline had only had an 
> `ElaboratedType` node if there was elaboration, and not otherwise. And that 
> makes a lot more sense to me than having 2 `ElaboratedType*` nodes _for every 
> type in the AST_, just to express that "hey, by the way, this type had no 
> elaboration".

There are no 2 `ElaboratedType` nodes, there is only one. If you are seeing 
something like an ElaboratedType wrapping directly over another ElaboratedType, 
that would seem to be a bug.

To the second point, it's a problem of representation. Having no elaboration is 
not the same thing as having no information about elaboration, so we better not 
represent both things with the same state.

> That sounds good at face value, but if you're planning to remove these nodes 
> again, that would create enormous churn for out-of-tree tools to re-adjust to 
> the new shape of the tree.
>
> I can't say what the best solution is, but this patch generates quite a lot 
> of work for me, and I would really hope that catching up with the new AST 
> does not generate even more work down the line.

That part I don't understand why. Before this patch, clang can produce a bunch 
of type nodes wrapped in an ElTy, or not. After this patch, we add ElTys in 
more cases, but the basic situation remains the same.

Why IWYU would even care about ElaboratedTypes at all? I would have expected a 
`git grep ElaboratedType` on IWYU sources to have no matches.

Can you elaborate on that?

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.

Of course that was not what happened in practice. A lot of code used `dyn_cast` 
where it should have used `getAs`. Just look at all the fixes in this patch for 
examples.
And fixing that, besides making that code compatible with this patch, also 
fixed other bugs where it would not properly ignore other pre-existing type 
sugar.

If IWYU has unit tests that test too much clang implementation details, that 
would generate unneeded burden on both sides.  Is it for example doing AST dump 
tests and expecting exact outputs?
Not even the clang test suite does that too much.
Is it to compensate for any perceived lack of testing on mainline side?


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] D129078: [LLDB][ClangExpression] Allow expression evaluation from within C++ Lambdas

[Michael Buch via Phabricator via lldb-commits]

Michael137 updated this revision to Diff 445269.
Michael137 added a comment.

- Fix assertion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129078

Files:
  lldb/include/lldb/Expression/Materializer.h
  lldb/include/lldb/Expression/UserExpression.h
  lldb/include/lldb/lldb-private-types.h
  lldb/source/Expression/Materializer.cpp
  lldb/source/Expression/UserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionVariable.h
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangUserExpression.h
  lldb/test/API/commands/expression/expr_inside_lambda/Makefile
  lldb/test/API/commands/expression/expr_inside_lambda/TestExprInsideLambdas.py
  lldb/test/API/commands/expression/expr_inside_lambda/main.cpp
  lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/Makefile
  
lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
  lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp

Index: lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/main.cpp
@@ -0,0 +1,32 @@
+#include 
+#include 
+#include 
+
+struct Foo {
+  bool enable = true;
+  uint32_t offset = 0;
+
+  void usleep_helper(uint32_t usec) {
+[this, &usec] {
+  puts("Break here in the helper");
+  std::this_thread::sleep_for(
+  std::chrono::duration(offset + usec));
+}();
+  }
+};
+
+void *background_thread(void *) {
+  Foo f;
+  for (;;) {
+f.usleep_helper(2);
+  }
+}
+
+int main() {
+  std::puts("First break");
+  std::thread main_thread(background_thread, nullptr);
+  Foo f;
+  for (;;) {
+f.usleep_helper(1);
+  }
+}
Index: lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_on_lambda_capture/TestBreakOnLambdaCapture.py
@@ -0,0 +1,54 @@
+"""
+Test that if we hit a breakpoint on a lambda capture
+on two threads at the same time we stop only for
+the correct one.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+
+
+class TestBreakOnLambdaCapture(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_break_on_lambda_capture(self):
+self.build()
+self.main_source_file = lldb.SBFileSpec("main.cpp")
+
+(target, process, main_thread, _) = lldbutil.run_to_source_breakpoint(self,
+"First break", self.main_source_file)
+
+# FIXME: This is working around a separate bug. If you hit a breakpoint and
+# run an expression and it is the first expression you've ever run, on
+# Darwin that will involve running the ObjC runtime parsing code, and we'll
+# be in the middle of that when we do PerformAction on the other thread,
+# which will cause the condition expression to fail.  Calling another
+# expression first works around this.
+val_obj = main_thread.frame[0].EvaluateExpression("true")
+self.assertSuccess(val_obj.GetError(), "Ran our expression successfully")
+self.assertEqual(val_obj.value, "true", "Value was true.")
+
+bkpt = target.BreakpointCreateBySourceRegex("Break here in the helper",
+self.main_source_file);
+
+bkpt.SetCondition("enable && usec == 1")
+process.Continue()
+
+# This is hard to test definitively, becuase it requires hitting
+# a breakpoint on multiple threads at the same time.  On Darwin, this
+# will happen pretty much ever time we continue.  What we are really
+# asserting is that we only ever stop on one thread, so we approximate that
+# by continuing 20 times and assert we only ever hit the first thread.  Either
+# this is a platform that only reports one hit at a time, in which case all
+# this code is unused, or we actually didn't hit the other thread.
+
+for idx in range(0, 20):
+process.Continue()
+for thread in p

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

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov added a comment.

In D112374#3657472 , @kimgr wrote:

> I can't say what the best solution is, but this patch generates quite a lot 
> of work for me, and I would really hope that catching up with the new AST 
> does not generate even more work down the line.

Okay, I checked out IWYU and I see why you need to look at ElaboratedType in 
some cases. And that also answers a lot of my previous questions.

Some type nodes were before rarely ever elaborated, but will have an 
ElaboratedType over them consistently now.
Searching IWYU source code, some cases where dyn_cast is used in some of them:

iwyu.cc:

  // If we're a constructor, we also need to construct the entire class,
  // even typedefs that aren't used at construct time. Try compiling
  //template struct C { typedef typename T::a t; };
  //class S; int main() { C c; }
  if (isa(fn_decl)) {
CHECK_(parent_type && "How can a constructor have no parent?");
parent_type = RemoveElaboration(parent_type);
if (!TraverseDataAndTypeMembersOfClassHelper(
dyn_cast(parent_type)))
  return false;
  }
  return true;
  `

  if (const auto* enum_type = dyn_cast(type))
return !CanBeOpaqueDeclared(enum_type);


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] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via lldb-commits]

mizvekov added a comment.

@kimgr One other general comment.

The way this function is implemented is quite error prone:

  static const NamedDecl* TypeToDeclImpl(const Type* type, bool as_written) {
// Get past all the 'class' and 'struct' prefixes, and namespaces.
type = RemoveElaboration(type);
  
// Read past SubstTemplateTypeParmType (this can happen if a
// template function returns the tpl-arg type: e.g. for
// 'T MyFn() {...}; MyFn.a', the type of MyFn will be a Subst.
type = RemoveSubstTemplateTypeParm(type);
  
CHECK_(!isa(type) && "IWYU doesn't support Objective-C");

Ie the beginning is being too explicit, testing for very specific sugar type 
nodes kinds, in a very specific order, just to skip over them.

That makes it very fragile against clang changes.

You can instead just use `getAs` to step over them in a generic fashion.

I don't think this one gets broken by this MR, but I am very confident it will 
get broken by another patch I have.


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