[Lldb-commits] [PATCH] D50478: Add support for artificial tail call frames

aprantl added inline comments. Comment at: lldb/include/lldb/Symbol/Function.h:331 + /// \ref resolved. + union { +const char *mangled_name; vsk wrote: > aprantl wrote: > > `llvm::PointerUnion` ? > It's not possible to use PointerUnion here because `const c

[Lldb-commits] [PATCH] D51227: [vscode] Skip the vscode tests on Linux

aprantl added a comment. Looks like we *really* need to figure out what the underlying bug is here, because there's not many platforms left after this patch. This patch is fine as a stopgap, but we can't have a feature like this be in the source code while being virtually untested, that's just a

[Lldb-commits] [PATCH] D51243: Disable use-color if the output stream is not a terminal with color support.

aprantl added inline comments. Comment at: source/Core/Debugger.cpp:805 const char *term = getenv("TERM"); if (term && !strcmp(term, "dumb")) SetUseColor(false); Shouldn't this check be obsolete now? Repository: rLLDB LLDB https://reviews.llvm.org

[Lldb-commits] [PATCH] D51243: Disable use-color if the output stream is not a terminal with color support.

aprantl added inline comments. Comment at: source/Core/Debugger.cpp:805 const char *term = getenv("TERM"); if (term && !strcmp(term, "dumb")) SetUseColor(false); aprantl wrote: > Shouldn't this check be obsolete now? You can probably test that by runni

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

aprantl created this revision. aprantl added a reviewer: jingham. Herald added a subscriber: mgrang. Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to read/understand/maintain. As a side-effect, this should also improve the performance by avoiding lots ofcostly vector element

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

aprantl added inline comments. Comment at: scripts/Python/python-typemaps.swig:89 + PyErr_SetString(PyExc_ValueError, "Not a valid StateType value"); + return nullptr ; +} nice! There's an extra space before the `;` Comment at: so

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

aprantl added inline comments. Comment at: include/lldb/lldb-enumerations.h:60 ///< or threads get the chance to run. + kNumStateType }; I think we usually do something like eLastsState = eStateSuspended. This avoid having to add the case t

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

aprantl updated this revision to Diff 163198. aprantl marked 2 inline comments as done. aprantl added a comment. Address review feedback. https://reviews.llvm.org/D51453 Files: include/lldb/Breakpoint/BreakpointResolver.h source/Breakpoint/BreakpointResolver.cpp Index: source/Breakpoint/Br

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

aprantl marked an inline comment as done. aprantl added a comment. I don't have any numbers to back up the performance claim. My primary motivation for this patch was to prepare the code for adding extra functionality; the performance gain was only side-effect. One way to measure it would be to

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint bile FileSpec+Line+Column in the SBAPI.

aprantl created this revision. aprantl added a reviewer: jingham. Herald added subscribers: abidh, mgrang. Support setting a breakpoint bile FileSpec+Line+Column in the SBAPI. This patch extends the SBAPI to allow for setting a breakpoint not only at a specific line, but also at a specific (minim

[Lldb-commits] [PATCH] D51466: Move the column marking functionality to the Highlighter framework

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thank you, this looks great! Repository: rLLDB LLDB https://reviews.llvm.org/D51466 ___ lldb-commits mailing list lldb-commits@lists.llvm.or

[Lldb-commits] [PATCH] D51453: Refactor BreakpointResolver::SetSCMatchesByLine()

aprantl marked an inline comment as done. aprantl added a comment. FYI: I tried to benchmark this using `break set -A -p begin` and similar things, but in all my experiments the variation between test runs was much larger than any difference with or without my patch. The filtering of the breakp

[Lldb-commits] [PATCH] D51461: Support setting a breakpoint by FileSpec+Line+Column in the SBAPI.

aprantl updated this revision to Diff 163231. aprantl marked 4 inline comments as done. aprantl added a comment. Address review feedback. https://reviews.llvm.org/D51461 Files: include/lldb/API/SBTarget.h include/lldb/Breakpoint/BreakpointResolver.h include/lldb/Breakpoint/BreakpointResol

[Lldb-commits] [PATCH] D51445: Remove undefined behavior around the use of StateType

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! I assume there is a whole bunch of other enums for which we should be doing the same thing now? Also there might be some 1-based ones for which we also need to check the lower bound.

[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

aprantl created this revision. aprantl added reviewers: jingham, jasonmolenda. Herald added a subscriber: kubamracek. This patch allows LLDB to print column info in backtraces et al. if available, which is useful when the backtrace contains a frame like the following: `f(can_crash(0), can_crash(

[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

aprantl added inline comments. Comment at: source/Core/Debugger.cpp:123 +#define FILE_AND_LINE \ + "{ at ${line.file.basename}:${line.number}${line.column}}" #define IS_OPTIMIZED "{${function.is-optimized} [opt]}" --

[Lldb-commits] [PATCH] D51661: Print column info in backtraces et al. if available

aprantl updated this revision to Diff 164104. aprantl added a comment. Address review feedback. Thanks you, I was wondering how to properly implement an optional element! https://reviews.llvm.org/D51661 Files: include/lldb/Core/FormatEntity.h packages/Python/lldbsuite/test/functionalities/

[Lldb-commits] [PATCH] D52101: [lldb-mi] Correct regex in the symbol-list-lines test

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks, that looks like a safe change. Repository: rLLDB LLDB https://reviews.llvm.org/D52101 ___ lldb-commits mailing list lldb-commits@lis

[Lldb-commits] [PATCH] D52139: [lldb-mi] Improve lldb-mi LIT tests

aprantl added a comment. It looks like it would be easy to forget to add -gdb-exit to every test. Wouldn't it be better to have the lldb-mi automatically produce a -gdb-exit command when it reads EOF from stdin? Repository: rLLDB LLDB https://reviews.llvm.org/D52139 _

[Lldb-commits] [PATCH] D52270: TestMultilineExpr: validate evaluation for expressions spread over multiple lines

aprantl added inline comments. Comment at: lit/Expr/TestMultilineExpr.test:3 + +# In terminal sessions LLDB reverts input from subsequent lines so it doesn't show up in the output we check below. +expression aprantl wrote: > "reverts" -> "joins"? or did you mean

[Lldb-commits] [PATCH] D52270: TestMultilineExpr: validate evaluation for expressions spread over multiple lines

aprantl added inline comments. Comment at: lit/Expr/TestMultilineExpr.test:3 + +# In terminal sessions LLDB reverts input from subsequent lines so it doesn't show up in the output we check below. +expression "reverts" -> "joins"? Comment at: l

[Lldb-commits] [PATCH] D52270: TestMultilineExpr: validate evaluation for expressions spread over multiple lines

aprantl added a comment. Does that mean we can remove ./packages/Python/lldbsuite/test/expression_command/multiline/TestMultilineExpressions.py ? https://reviews.llvm.org/D52270 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://list

[Lldb-commits] [PATCH] D52270: TestMultilineExpr: validate evaluation for expressions spread over multiple lines

aprantl added a comment. For those following at home: the point of this exercise is to get rid of the -expect-based TestMultilineExpressions.py testcase that kept failing on build bots. https://reviews.llvm.org/D52270 ___ lldb-commits mailing list

[Lldb-commits] [PATCH] D52270: TestMultilineExpr: validate evaluation for expressions spread over multiple lines

aprantl added inline comments. Comment at: lit/Expr/TestMultilineExpr.test:9 +# CHECK: (lldb) expression +# CHECK-NEXT: Enter expressions, then terminate with an empty line to evaluate: +# CHECK-NEXT: (int) {{.*}} = 5 sgraenitz wrote: > Maybe it's nitpicking, but

[Lldb-commits] [PATCH] D52270: TestMultilineExpr: validate evaluation for expressions spread over multiple lines

aprantl added a comment. > LLDB still echoes all commands including comments What do you think about fixing that before landing this patch? Then we don't need to work around it. https://reviews.llvm.org/D52270 ___ lldb-commits mailing list lldb-co

[Lldb-commits] [PATCH] D40539: Allow using the object file name for debug symbol resolution

aprantl added a comment. yaml2obj sounds generally like a good solution. I should note that right now yaml2obj is only used/tested for the (narrow) use-case of testing the linker. For creating debug-info-related testcases, I found it both to high and too low an abstraction. Too low, because it

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

aprantl added a comment. In https://reviews.llvm.org/D40283#972622, @clayborg wrote: > Added Adrian Prantl as a reviewer in case he has any input. Adrian: is there > any way that a DIE is marked up with an extra attribute when the asm name is > explicitly set? It would be great to know this fro

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

aprantl created this revision. aprantl added reviewers: jasonmolenda, jingham. Herald added a subscriber: llvm-commits. I found a cheap and cross-platform way of detecting whether the target supports AVX2. Repository: rL LLVM https://reviews.llvm.org/D41962 Files: packages/Python/lldbsui

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

aprantl added a comment. In https://reviews.llvm.org/D41962#974656, @jasonmolenda wrote: > I suppose a possible alternative would be to figure out the avx2 / avx512 > features manually based on the cpuid instead of letting the compiler do it > for us. e.g. > https://stackoverflow.com/question

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

aprantl updated this revision to Diff 129639. aprantl added a comment. Uploaded a mildly better version that is NFC on MSVC. https://reviews.llvm.org/D41962 Files: packages/Python/lldbsuite/test/functionalities/register/intel_avx/TestYMMRegister.py packages/Python/lldbsuite/test/functional

[Lldb-commits] [PATCH] D41997: Build virtual override tables in DWARFASTParserClang::CompleteTypeFromDWARF

aprantl added a comment. Awesome! Comment at: Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:2103 +static bool isOverload(clang::CXXMethodDecl *m1, clang::CXXMethodDecl *m2) { + // FIXME: This should detect covariant return types, but currently doesn't. Co

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

aprantl added a comment. > Why not just look for the AVX registers by name that are only available if > they are correctly detected by the native lldb-server or debugserver? Then we > can avoid all of this. If we don't execute any instructions that crash the > program, we can stop before any sp

[Lldb-commits] [PATCH] D41962: Fix TestYMMRegisters for older machines without AVX2

aprantl added a comment. > there's a new skipUnlessFeature() method added to decorators.py which runs > sysctl to detect hardware features (in this case, hw.optional.avx512f) How does one execute a program like `sysctl` on the remote? I have seen code in TestLldbGdbServer.py that uses `platform

[Lldb-commits] [PATCH] D42215: [CMake] Make check-lldb work with LLDB_CODESIGN_IDENTITY=''

aprantl added a comment. How about printing a message (if CMAKE_HOST_APPLE) that the system debugserver is being used at configure time? https://reviews.llvm.org/D42215 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org

[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree

aprantl created this revision. This patch is the result of a discussion on lldb-dev, see http://lists.llvm.org/pipermail/lldb-dev/2018-January/013111.html for background. This is a first sketch of how to move building of the testcases in the LLDB testsuite out of the source tree. The patch is

[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts

aprantl added a comment. Thanks, this is a great start! Comment at: packages/Python/lldbsuite/test/make/Makefile.rules:240 +CLANG_MODULE_CACHE_DIR := module-cache + Is it safe that this is a relative path? Comment at: packages/Python/lldbsu

[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree

aprantl added a comment. In https://reviews.llvm.org/D42281#981969, @clayborg wrote: > Looks like a good start. It might be nice to validate that after "clean" that > we have no files that are untracked in the test directory? This could help us > to verify that we don't leave artifacts around.

[Lldb-commits] [PATCH] D42280: Wrap all references to build artifacts in the LLDB testsuite in TestBase::getBuildArtifact()

aprantl added a comment. In https://reviews.llvm.org/D42280#982229, @jingham wrote: > I didn't intend to block this patch, just to point out that this was really > work you shouldn't have had to do, and that as we touch these files we should > clean them up so next time we don't have to. It wi

[Lldb-commits] [PATCH] D42340: [modules] Fix missing includes/typo in LLDB's includes. [NFC]

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! This looks generally good/useful as a cleanup, too. Comment at: include/lldb/Core/ThreadSafeDenseSet.h:49 void Clear() { -stds::lock_guard<_MutexType> guard(

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

aprantl added a comment. In https://reviews.llvm.org/D40283#983908, @clayborg wrote: > I am fine with checking this. The only issue on my end is the extra memory > that will be needed to store these often huge mangled names in every AST > context for the 0.0001% of the time it is actually neede

[Lldb-commits] [PATCH] D40283: lldb: Use the DWARF linkage name when importing C++ methods

aprantl added a comment. As Greg pointed out C++ (and Swift) mangled names can be enormous, so it would definitely have an impact on the memory footprint (unless we are only passing `StringRef`s around. Are we?) https://reviews.llvm.org/D40283 ___

[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts

aprantl added a comment. > Skip tests which fail when -fmodules is passed > (https://bugs.llvm.org/show_bug.cgi?id=36048). Wait.. this patch is not supposed to change the set of tests that get -fmodules passed to them. It should only add -fmodules-cache-path to tests that do pass -fmodules. Wh

[Lldb-commits] [PATCH] D42281: WIP: compile the LLDB tests out-of-tree

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/dotest_args.py:166 +metavar='Test build directory', +help='The root build directory for the tests') clayborg wrote: > Maybe add a default right here?: > > ``` > defau

[Lldb-commits] [PATCH] D42277: Use test-specific module caches to avoid stale header conflicts

aprantl added a comment. Okay got it. I'll investigate the PR separately. https://reviews.llvm.org/D42277 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/api/check_public_api_headers/TestPublicAPIHeaders.py:46 self.line_to_break = line_number( -self.source, '// Set breakpoint here.') +self.getBuildArtifact(self.source), '//

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. I am now working on building each test variant (dwarf,dwo,dsym,...) in its own build directory so they can run in parallel and we can get rid of the lockfile. While doing this I discovered that certain tests (e.g., TestPublicAPIHeaders.py, the MI tests) are invoking bui

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. I found the mechanism I was looking for. Generic testcases set `NO_DEBUG_INFO_TESTCASE = True` https://reviews.llvm.org/D42281 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailm

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. In https://reviews.llvm.org/D42281#990296, @labath wrote: > In https://reviews.llvm.org/D42281#989793, @aprantl wrote: > > > I am now working on building each test variant (dwarf,dwo,dsym,...) in its > > own build directory so they can run in parallel and we can get rid

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. Let's see how this fares on the bots. As I mentioned in the commit message, please don't hesitate to revert this patch. I'm watching all bots connected to lab.llvm.org, and green.lab.llvm.org but please let me know if I'm missing something. Repository: rL LLVM http

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. There are a few test failures, but they look manageable. I will need help resolving them though: http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7843 FAIL: TestLoadUnload.LoadUnloadTestCase.test_lldb_process_load_and_unload_commands_dwarf http://lab.llvm

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. In https://reviews.llvm.org/D42281#992295, @aprantl wrote: > There are a few test failures, but they look manageable. I will need help > resolving them though: Updated list: http://lab.llvm.org:8011/builders/lldb-windows7-android/builds/7843 FAIL: TestLoadUnload.Load

[Lldb-commits] [PATCH] D42281: Compile the LLDB tests out-of-tree

aprantl added a comment. Thanks a lot for your help, Pavel! We also discovered the most amazing test failure on the green dragon bots yesterday (fix still underway) that was made much worse by my patch. On Darwin LLDB automatically tries to find a .dSYM bundle for an executable by querying the

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl created this revision. aprantl added reviewers: labath, zturner, jingham. Herald added subscribers: JDevlieghere, eraman. This patch creates a .dwarf, .dwo, etc., build directory for each testcase variant. Most importantly, this eliminates the need for the per-test lock file in the sourc

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl added a comment. > I am not sure this actually creates enough separation. That's a good point. If I manage to extract the testname somehow via Python reflection magic, I could also get rid of the unintuitive self.mydir tuple. I'll give that a try. Comment at: packag

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl updated this revision to Diff 132417. aprantl added a comment. Address review feedback from Alex. https://reviews.llvm.org/D42763 Files: packages/Python/lldbsuite/test/api/listeners/TestListener.py packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompD

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl added a comment. lib/IR -> test/IR https://reviews.llvm.org/D42763 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl updated this revision to Diff 132433. aprantl added a comment. Update the diff with Pavel's awesome suggestion of using self.testMethodName. Note that that I haven't fixed all the resulting test error yet, I'm working through those now. https://reviews.llvm.org/D42763 Files: package

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl added a comment. @jingham wrote: > Now that we aren't mixing variants, would it be possible to have a test class > claim that all the tests use the same binary file? At present to get > self-contained tests you often need to do roughly the same thing many times, > on the same binary.

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl updated this revision to Diff 132456. aprantl added a comment. Fix broken testcases. https://reviews.llvm.org/D42763 Files: packages/Python/lldbsuite/test/api/listeners/TestListener.py packages/Python/lldbsuite/test/functionalities/breakpoint/comp_dir_symlink/TestCompDirSymLink.py

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/api/listeners/TestListener.py:26 TestBase.setUp(self) -self.build() labath wrote: > I'm confused by these changes. I was under the impression that setUp() runs > before

[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()

aprantl added a comment. That looks great, to make sure I understand this correctly, is the following accurate? If we apply this on top of https://reviews.llvm.org/D42763, and we have tests/my/testA.py: class TestA(Base): def setUp(self): ... def testAone(self):

[Lldb-commits] [PATCH] D42836: [dotest] make debug info variant accessible in setUp()

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Great. The important thing for me was that a new object is constructed for each permutation of test function / variant. https://reviews.llvm.org/D42836 __

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl updated this revision to Diff 132875. aprantl added a comment. Updated to query self.getDebugInfo() in getBuildDir(). https://reviews.llvm.org/D42763 Files: packages/Python/lldbsuite/test/api/listeners/TestListener.py packages/Python/lldbsuite/test/functionalities/breakpoint/comp_d

[Lldb-commits] [PATCH] D42763: Build each testcase variant in its own subdirectory and remove the srcdir lock file

aprantl updated this revision to Diff 132888. aprantl marked 17 inline comments as done. aprantl added a comment. Cleanup and address outstanding review feedback. https://reviews.llvm.org/D42763 Files: packages/Python/lldbsuite/test/functionalities/load_unload/TestLoadUnload.py packages/Pyt

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

aprantl created this revision. aprantl added a reviewer: jingham. This patch makes LLDB's clang module cache path customizable via `settings set target.clang-modules-cache-path ` and uses it in the LLDB testsuite to reuse the same location inside the build directory for LLDB and clang. https:/

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

aprantl updated this revision to Diff 133636. aprantl added a comment. Herald added subscribers: hintonda, mgorny. Address most review feedback. https://reviews.llvm.org/D43099 Files: include/lldb/Target/Target.h packages/Python/lldbsuite/test/lldbtest.py source/Plugins/ExpressionParser/C

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

aprantl added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/ClangModulesDeclVendor.cpp:595-601 +if (Path.empty()) { + // This code is copied from the Clang driver. + const bool erased_on_reboot = false; + llvm::sys::path::system_temp_direc

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

aprantl added inline comments. Comment at: source/Plugins/ExpressionParser/Clang/CMakeLists.txt:26 clangCodeGen +clangDriver clangEdit I checked and this does not affects LLDB's binary size in any measurable way. https://reviews.llvm.org/D43099

[Lldb-commits] [PATCH] D43099: Make LLDB's clang module cache path customizable

aprantl added inline comments. Comment at: source/Target/Target.cpp:3949 + idx); + assert(option_value); + return option_value->GetCurrentValue(); davide wrote: > add an assertion message if you don'

[Lldb-commits] [PATCH] D43335: [dosep] Run tests in a more parallel fashion

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. This is very cool! I imagine that this will help a lot with TestIntegerTypes and other really long-running ones. Thanks. https://reviews.llvm.org/D43335 _

[Lldb-commits] [PATCH] D43464: Avoid dirtying the source tree in breakpoint command tests

aprantl added a comment. Awesome! https://reviews.llvm.org/D43464 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43506: Fix a couple of more tests to not create files in the source tree

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! By the way, just in case you are motivated :-), one outstanding task is also to replace all uses of the `clean` target in the makefiles with simply removing the test build directory

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

aprantl accepted this revision. aprantl added inline comments. This revision is now accepted and ready to land. Comment at: include/lldb/Core/MappedHash.h:156 - template - class ExportTable { - public: Yeah this looks like it was a dead end dating back to 201

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

aprantl added a comment. In https://reviews.llvm.org/D43596#1015903, @clayborg wrote: > The only input we currently use this on is DWARF and I believe all DWARF > strings are UTF8 so this should all work correctly, no? The hashing algorithms produced different hashes for characters with bit 7

[Lldb-commits] [PATCH] D43647: Generate most of the target properties from a central specification.

aprantl created this revision. aprantl added reviewers: jingham, jasonmolenda, clayborg. Herald added a subscriber: llvm-commits. I recently had to add a new property and was annoyed by how much repetitive code I needed to write manually. This patch applies a common pattern used all over LLVM to

[Lldb-commits] [PATCH] D43647: Generate most of the target properties from a central specification.

aprantl added a comment. In https://reviews.llvm.org/D43647#1016722, @jingham wrote: > What would I have to figure out if I wanted to stop where one of these > properties was actually set or read? That's something I've had to do many's > the time... This task is straightforward in the origina

[Lldb-commits] [PATCH] D43647: Generate most of the target properties from a central specification.

aprantl added a comment. I like Pavel's template syntax. It's not clear to me how to best implement the registering of the properties this way though. I think we want to avoid static intializers. Given how TargetProperties::TargetProperties() is implemented, I could imagine we could just make t

[Lldb-commits] [PATCH] D43596: Replace HashStringUsingDJB with llvm::djbHash

aprantl added a comment. Nice! https://reviews.llvm.org/D43596 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D43694: Add a sanity check for inline tests

aprantl created this revision. aprantl added reviewers: vsk, labath, jingham. Herald added a subscriber: eraman. When writing an inline test, there is no way to make sure that any of the inline commands are actually executed, so this patch adds a sanity check that at least one breakpoint was hit

[Lldb-commits] [PATCH] D32167: Add support for type units (.debug_types) to LLDB in a way that is compatible with DWARF 5

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/test_categories.py:27 'gmodules': 'Tests that can be run with -gmodules debug information', +'dwarf_type_units' : 'Tests using the DWARF type units (-fdebug-types-section)', 'expression': 'Te

[Lldb-commits] [PATCH] D43912: [Symbol] Add InvalidType, a force-checked recoverable error

aprantl added a comment. > Going forward, we should transition to a model in which CompilerTypes are > either valid or do not exist. I don't understand very well how the LLDB type system works so excuse my naive questions: Does this account for lazyness? I.e., could there be value in having an

[Lldb-commits] [PATCH] D43984: Make the clang module cache setting available without a target

aprantl created this revision. aprantl added a reviewer: jingham. It turns out that setting the clang module cache after LLDB has a Target can be too late. In particular, the Swift language plugin needs to know the setting without having access to a Target. This patch moves the setting into the

[Lldb-commits] [PATCH] D43984: Make the clang module cache setting available without a target

aprantl added a comment. The entire testsuite depends on being able to set this option (see the change in `packages/Python/lldbsuite/test/lldbtest.py`), but we don't verify that the directory is being used. I'll see if I can come up with a dedicated test. https://reviews.llvm.org/D43984 ___

[Lldb-commits] [PATCH] D43984: Make the clang module cache setting available without a target

aprantl updated this revision to Diff 136803. aprantl added a comment. Added a testcase. https://reviews.llvm.org/D43984 Files: include/lldb/Core/ModuleList.h include/lldb/Target/Target.h packages/Python/lldbsuite/test/lang/objc/modules-cache/Makefile packages/Python/lldbsuite/test/lan

[Lldb-commits] [PATCH] D44056: [lldb] Fix "code unreachable" warning in HostThreadPosix::Cancel

aprantl added inline comments. Comment at: source/Host/posix/HostThreadPosix.cpp:44 if (IsJoinable()) { #ifndef __ANDROID__ #ifndef __FreeBSD__ What about: ``` #ifdef __ANDROID__ error.SetErrorString("HostThreadPosix::Cancel() not supported on Android");

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

aprantl created this revision. aprantl added reviewers: jingham, vsk, labath. This is a more principled approach to disabling Spotlight .dSYM lookups while running the testsuite, most importantly it also works for the LIT-based tests, which I overlooked in my initial fix (renaming the test build

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

aprantl added a comment. In https://reviews.llvm.org/D44342#1034514, @labath wrote: > Does that mean we can remove the `.noindex` thingy from the build folder > name? (In any case, the change seems fine to me). Yes, but we could still keep it for performance reasons. Perhaps only on Darwin.

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

aprantl updated this revision to Diff 138082. aprantl added a comment. Renamed option to something more general. https://reviews.llvm.org/D44342 Files: include/lldb/Core/ModuleList.h lit/lit-lldb-init lit/lit.cfg packages/Python/lldbsuite/test/lldbtest.py source/Core/ModuleList.cpp

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

aprantl added a comment. Thanks for the suggestion, Greg! I renamed to option to use a more generic name and removed the `#if APPLE` guards. https://reviews.llvm.org/D44342 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm

[Lldb-commits] [PATCH] D44342: Introduce a setting to disable Spotlight while running the test suite

aprantl updated this revision to Diff 138086. aprantl added a comment. More review feedback. https://reviews.llvm.org/D44342 Files: include/lldb/Core/ModuleList.h lit/lit-lldb-init lit/lit.cfg packages/Python/lldbsuite/test/lldbtest.py source/Core/ModuleList.cpp source/Host/macosx/S

[Lldb-commits] [PATCH] D44526: [dotest] Clean up test folder clean-up

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Thanks! I think this makes sense. Comment at: packages/Python/lldbsuite/test/lldbtest.py:707 +"""Create the test-specific working directory, deleting any previous +

[Lldb-commits] [PATCH] D45215: RFC/WIP: Have lit run the lldb test suite

aprantl added a comment. So this is basically replacing the parallel test-driver functionality of dotest with lit and dotest is only used to invoke one test at a time. This way we (as the LLVM project) can avoid maintaining to test drivers implemented in python. That clearly sounds like the rig

[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

aprantl added a comment. I have only one general question and otherwise I'm fine with this: Does this bring LLDB's DWARF parser closer to the structure of LLVM's? We still want to long-term replace LLDB's DWARF parser with LLVM's so every refactoring should aim at making their interfaces more s

[Lldb-commits] [PATCH] D45170: Cleanup DWARFCompileUnit and DWARFUnit in preparation for adding DWARFTypeUnit

aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. That's great! Let's go for it then. Could you please doxygen'ify the comments? https://reviews.llvm.org/D45170 ___ lldb-commits mailing list ll

[Lldb-commits] [PATCH] D45518: [dotest] Use in-tree dsymutil on Darwin

aprantl added inline comments. Comment at: packages/Python/lldbsuite/test/dotest.py:306 + if args.dsymutil: +os.environ['DSYMUTIL'] = args.dsymutil + else: I suppose we could point to llvm-dsymutil even on non-darwin platforms? Might be useful

[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

aprantl added a comment. Which compilers / platforms generate / support this? Is this an ELF-only feature? https://reviews.llvm.org/D45628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb

[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)

aprantl added a comment. Thanks for clarifying. You'll also need to add a testcase. If clang supports this then I don't have a problem with supporting this in LLDB and adding a testcase should be easy. Comment at: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:1893

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added inline comments. Comment at: lldb.xcodeproj/xcshareddata/xcschemes/darwin-debug.xcscheme:29 selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.GDB" + language = "" shouldUseLaunchSchemeArgsEnv = "YES"> Is this an acci

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:189 + // Always normalize our compile unit directory to get rid of redundant + // slashes and other path anonalies before we use it for path prepending + FileSpec local_spec(local_pa

[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.

aprantl added inline comments. Comment at: source/Utility/FileSpec.cpp:107 + for (auto i = path.find('/'); i != llvm::StringRef::npos; + i = path.find('/', i + 1)) { +const auto nextChar = safeCharAtIndex(path, i+1); clayborg wrote: > no as the only ca

<    1   2   3   4   5   6   7   8   9   10   >