davide added a comment. Some basic comments. Haven't looked at the implementation very closely, I'll do that probably tomorrow. Thanks for working on this!
================ Comment at: include/lldb/Target/CPPLanguageRuntime.h:59-60 + lldb::ThreadPlanSP GetStepThroughTrampolinePlan(Thread &thread, + bool stop_others); + ---------------- Please add a doxygen comment to this function. (I understand the surrounding ones are not commented, but we should start somewhere)/ ================ Comment at: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:1-3 +""" +Test lldb data formatter subsystem. +""" ---------------- This comment seems a little outdated. ================ Comment at: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:8-9 + +import os +import time +import lldb ---------------- I'm not sure you need these. ================ Comment at: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:27-28 + + @add_test_categories(["libc++"]) + def test(self): + """Test that std::function as defined by libc++ is correctly printed by LLDB""" ---------------- Is this affected by the debug info variant used? (i.e. dSYM/gmodules/DWARF/dwo). If not, this could be a `NO_DEBUG_INFO` test ================ Comment at: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/TestStdFunctionStepIntoCallable.py:56-73 + self.assertEqual( self.thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), 13 ) ; + self.process.Continue() + + self.thread.StepInto() + self.assertEqual( self.thread.GetFrameAtIndex(0).GetLineEntry().GetLine(), 28 ) ; + self.process.Continue() + ---------------- All these lines check hardcoded make me a little nervous, as they're really fragile. Can you make them parametric at least? so that if somebody slides down the test case they're still valid (i.e. base + offset) ================ Comment at: packages/Python/lldbsuite/test/lang/cpp/std-function-step-into-callable/main.cpp:1-9 +//===-- main.cpp --------------------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// ---------------- You don't need a license for the test case, I think. ================ Comment at: source/Target/ThreadPlanStepThrough.cpp:99-105 + + CPPLanguageRuntime *cpp_runtime = + m_thread.GetProcess()->GetCPPLanguageRuntime(); + + if (cpp_runtime) + m_sub_plan_sp = + cpp_runtime->GetStepThroughTrampolinePlan(m_thread, m_stop_others); ---------------- Can you add a comment here? https://reviews.llvm.org/D52851 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
