[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
labath added inline comments.
Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:130-131
+// we must leave the slash character though.
+while (relative_path.consume_front("."))
+ /* Do nothing. */;
+ }
What about paths like `.foo/bar.c` and `.../bar.c`. These don't contain `.` or
`..` components. so you'll want to avoid the consuming here. (You can use
`*llvm::sys::path::begin(path)` to get the first path component and check that).
Also, I'm not sure what is the behavior we want for paths like `../../foo.cpp`.
The present behavior of matching the path as it was `/../foo.cpp` does not seem
entirely useful.
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);
aprantl wrote:
> clayborg wrote:
> > clayborg wrote:
> > > aprantl wrote:
> > > > clayborg wrote:
> > > > > I am fine switching to using the llvm functions for removing, this is
> > > > > only detecting if any normalization needs to happen. If there is an
> > > > > equivalent LLVM function that will only run through the string one
> > > > > time to detect all needs for normalization (no multiple passes
> > > > > looking for "..", then for "." etc like we used to have), I would be
> > > > > happy to use it, but there doesn't seem to be. We are trying to avoid
> > > > > having to create a "SmallVectorImpl< char >" for all paths if they
> > > > > don't need fixing here. This function is just iterating through an
> > > > > llvm::StringRef just to see if normalization needs to happen. If it
> > > > > does, then we make a SmallVectorImpl< char > and we fix the path. We
> > > > > can easily use llvm functions for the fixing part.
> > > > I see, you want to avoid copying the string when it isn't necessary to
> > > > do so. IMHO the best way to do this is to add a `bool hasDots(const
> > > > Twine &)` function to llvm::sys::path and add a `bool remove_dot_dot`
> > > > parameter to `removeDots()`. Since you have already written the
> > > > unittests, adding the function to LLVM shouldn't be any extra work
> > > > (I'll help reviewing), and we'll have 100 LoC less to maintain.
> > > >
> > > > This way we can avoid having two functions that perform path
> > > > normalization that superficially look like they might do the same
> > > > thing, but a year from now nobody can really tell whether they behave
> > > > exactly the same, and whether a bug found in one implementation should
> > > > also be fixed in the other one, etc... .
> > > I want to know if there are redundant slashes as well. I want something
> > > like "bool llvm::sys::has(const Twine &, bool dots, bool dot_dots, bool
> > > redundant_slashes)". But I don't see that getting accepted? I just want a
> > > "bool llvm:sys::path::contains_redundant_stuff(const Twine &)". Not sure
> > > on the name though. needs_normalization? can_be_shortened? Everything in
> > > LLVM right now assumes that dots, dot dots, slashes and realpath stuff
> > > happen in the same function. Not sure how to break that up without
> > > ruining the performance of the one and only loop that should be
> > > happening. I like the current approach since it doesn't require chopping
> > > up the string into an array and iterating through the array.
> > Also not sure if the LLVM stuff will try to substitute in the current
> > working directory for "."?
> I just read the source code of remove_dots()
> (http://llvm.org/doxygen/Path_8cpp_source.html#l00699) and if I'm not
> mistaken, it actually also removes double-separators as a side-effect, since
> it iterates over the path components of the string as it constructs the copy.
> It also seems to avoid the copy operation if it isn't necessary.
>
> Could you take another look? Perhaps I'm missing something, (or perhaps we
> can just turn this into a small addition to remove_dots).
>
> > Everything in LLVM right now assumes that dots, dot dots, slashes and
> > realpath stuff happen in the same function.
> I'm not sure I understand.
FWIW, this could be implemented in a much simpler way, while still making sure
we run through the string only once. I'm thinking of something like:
```
bool first = true;
for(it = llvm::sys::path::begin(path, style), end = llvm::sys::path::end(path);
it != end; ++it) {
if (!first && (*it == "." || *it == "..")) return true;
first = false;
}
return false;
```
Comment at: source/Utility/FileSpec.cpp:184-187
+if (component == ".") {
+ if (!normalized.empty())
+continue; // Skip '.' unless it is the first thing
+}
This looks wrong, because then for a path like `./../whatever`, you will end up
with both `.` and `..` in the "normalized" path.
Comment at: source/Utility/FileSpec.cpp:401-406
+// Make sure we don
[Lldb-commits] [lldb] r330708 - [dotest] Make the set of tests independent of the test configuration
Author: labath
Date: Tue Apr 24 03:51:44 2018
New Revision: 330708
URL: http://llvm.org/viewvc/llvm-project?rev=330708&view=rev
Log:
[dotest] Make the set of tests independent of the test configuration
Summary:
In the magic test duplicator, we were making the decision whether to
create a test variant based on the compiler and the target platform.
This meant that the set of known tests was different for each test
configuration.
This patch makes the set of generated test variants static and handles
the skipping via runtime checks instead. This is more consistent with
how we do other test-skipping decision (e.g. for libc++ tests), and
makes it easier to expose the full set of tests to lit, which now does
not need to know anything about what things can potentially cause tests
to appear or disappear.
Reviewers: JDevlieghere, aprantl
Subscribers: eraman, lldb-commits
Differential Revision: https://reviews.llvm.org/D45949
Modified:
lldb/trunk/packages/Python/lldbsuite/test/dotest.py
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=330708&r1=330707&r2=330708&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Tue Apr 24 03:51:44 2018
@@ -1104,6 +1104,22 @@ def checkLibcxxSupport():
print("Libc++ tests will not be run because: " + reason)
configuration.skipCategories.append("libc++")
+def checkDebugInfoSupport():
+import lldb
+
+platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
+compiler = configuration.compiler
+skipped = []
+for cat in test_categories.debug_info_categories:
+if cat in configuration.categoriesList:
+continue # Category explicitly requested, let it run.
+if test_categories.is_supported_on_platform(cat, platform, compiler):
+continue
+configuration.skipCategories.append(cat)
+skipped.append(cat)
+if skipped:
+print("Skipping following debug info categories:", skipped)
+
def run_suite():
# On MacOS X, check to make sure that domain for com.apple.DebugSymbols
defaults
# does not exist before proceeding to running the test suite.
@@ -1212,6 +1228,7 @@ def run_suite():
target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
checkLibcxxSupport()
+checkDebugInfoSupport()
# Don't do debugserver tests on everything except OS X.
configuration.dont_do_debugserver_test = "linux" in target_platform or
"freebsd" in target_platform or "windows" in target_platform
Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=330708&r1=330707&r2=330708&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Tue Apr 24 03:51:44
2018
@@ -241,23 +241,14 @@ def MakeInlineTest(__file, __globals, de
test = type(test_name, (InlineTest,), {'using_dsym': None})
test.name = test_name
-target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
-if test_categories.is_supported_on_platform(
-"dsym", target_platform, configuration.compiler):
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-if test_categories.is_supported_on_platform(
-"dwarf", target_platform, configuration.compiler):
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-if test_categories.is_supported_on_platform(
-"dwo", target_platform, configuration.compiler):
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-if test_categories.is_supported_on_platform(
-"gmodules", target_platform, configuration.compiler):
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
+test.test_with_dsym = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_dsym, decorators)
+test.test_with_dwarf = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_dwarf, decorators)
+test.test_with_dwo = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_dwo, decorators)
+test.test_with_gmodules = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_gmodules, decorators)
# Add the test case to the globals, and hide InlineTest
[Lldb-commits] [PATCH] D45949: [dotest] Make the set of tests independent of the test configuration
This revision was automatically updated to reflect the committed changes.
Closed by commit rL330708: [dotest] Make the set of tests independent of the
test configuration (authored by labath, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D45949
Files:
lldb/trunk/packages/Python/lldbsuite/test/dotest.py
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
@@ -241,23 +241,14 @@
test = type(test_name, (InlineTest,), {'using_dsym': None})
test.name = test_name
-target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
-if test_categories.is_supported_on_platform(
-"dsym", target_platform, configuration.compiler):
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-if test_categories.is_supported_on_platform(
-"dwarf", target_platform, configuration.compiler):
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-if test_categories.is_supported_on_platform(
-"dwo", target_platform, configuration.compiler):
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-if test_categories.is_supported_on_platform(
-"gmodules", target_platform, configuration.compiler):
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
+test.test_with_dsym = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_dsym, decorators)
+test.test_with_dwarf = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_dwarf, decorators)
+test.test_with_dwo = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_dwo, decorators)
+test.test_with_gmodules = ApplyDecoratorsToFunction(
+test._InlineTest__test_with_gmodules, decorators)
# Add the test case to the globals, and hide InlineTest
__globals.update({test_name: test})
Index: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py
@@ -1104,6 +1104,22 @@
print("Libc++ tests will not be run because: " + reason)
configuration.skipCategories.append("libc++")
+def checkDebugInfoSupport():
+import lldb
+
+platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
+compiler = configuration.compiler
+skipped = []
+for cat in test_categories.debug_info_categories:
+if cat in configuration.categoriesList:
+continue # Category explicitly requested, let it run.
+if test_categories.is_supported_on_platform(cat, platform, compiler):
+continue
+configuration.skipCategories.append(cat)
+skipped.append(cat)
+if skipped:
+print("Skipping following debug info categories:", skipped)
+
def run_suite():
# On MacOS X, check to make sure that domain for com.apple.DebugSymbols defaults
# does not exist before proceeding to running the test suite.
@@ -1212,6 +1228,7 @@
target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
checkLibcxxSupport()
+checkDebugInfoSupport()
# Don't do debugserver tests on everything except OS X.
configuration.dont_do_debugserver_test = "linux" in target_platform or "freebsd" in target_platform or "windows" in target_platform
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -1732,65 +1732,29 @@
for attrname, attrvalue in attrs.items():
if attrname.startswith("test") and not getattr(
attrvalue, "__no_debug_info_test__", False):
-target_platform = lldb.DBG.GetSelectedPlatform(
-).GetTriple().split('-')[2]
# If any debug info categories were explicitly tagged, assume that list to be
# authoritative. If none were specified, try with all debug
# info formats.
-all_dbginfo_categories = set(
-test_categories.debug_info_categories) - set(configuration.skipCategories)
+all_dbginfo_categories = set(test_categories.debug_info_categories)
categories = set(
getattr(
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
labath created this revision.
labath added reviewers: JDevlieghere, aprantl, amccarth.
The utility iterates through the tests in the suite, much like dotest
does, and prints all tests it found. So that it can set up the
environment properly, it needs to be called with a single argument - the
path to the lldb executable.
Right now it's not wired up to anything, but the idea is to call this
from the lldbtest lit format, to enable running tests with a finer
granularity.
The main reason I do this as a separate utility (and not inside the test format)
is that importing our tests requires a lot of fiddling with the import path and
has a bunch of side effects (e.g., loading liblldb into the address space), and
I did not want to expose the main lit process to this.
https://reviews.llvm.org/D46005
Files:
lit/Suite/lldbtestdumper.py
Index: lit/Suite/lldbtestdumper.py
===
--- /dev/null
+++ lit/Suite/lldbtestdumper.py
@@ -0,0 +1,78 @@
+"""
+A utility that prints all tests in the dotest suite. It expects to be called
+with a single argument - path to the lldb executable.
+"""
+
+from __future__ import print_function
+import subprocess
+import os
+import os.path
+import sys
+
+lldb_test_root = None
+
+
+def get_tests(test_or_suite):
+import unittest2
+
+if not isinstance(test_or_suite, unittest2.suite.TestSuite):
+yield test_or_suite
+return
+
+for nested in test_or_suite:
+for test in get_tests(nested):
+yield test
+
+
+def set_up_environment():
+global lldb_test_root
+
+this_file_dir = os.path.abspath(os.path.dirname(__file__))
+lldb_root = os.path.abspath(os.path.join(this_file_dir, '..', '..'))
+lldb_test_root = os.path.join(
+lldb_root,
+'packages',
+'Python',
+'lldbsuite',
+'test')
+use_lldb_suite_dir = os.path.join(lldb_root, "scripts")
+
+lldb_executable = os.path.abspath(sys.argv[1])
+site_packages = subprocess.check_output([lldb_executable, '-P']).strip()
+
+sys.path[0:0] = [use_lldb_suite_dir, site_packages]
+import use_lldb_suite
+
+import lldb
+lldb.DBG = lldb.SBDebugger.Create()
+
+os.environ["LLDB_TEST"] = lldb_test_root
+
+
+def main():
+set_up_environment()
+
+import unittest2
+
+sys.path[0:0] = [""] # Placeholder for dirpath below
+for dirpath, _, filenames in os.walk(lldb_test_root):
+sys.path[0] = dirpath
+dirpath = os.path.relpath(dirpath, lldb_test_root)
+for name in filenames:
+base, ext = os.path.splitext(name)
+if ext != ".py":
+continue
+if not name.startswith("Test"):
+continue
+
+suite = unittest2.defaultTestLoader.loadTestsFromName(base)
+for test in get_tests(suite):
+print(
+dirpath.replace(os.sep, ' '),
+name,
+test.__class__.__name__,
+test._testMethodName)
+
+
+if __name__ == '__main__':
+main()
Index: lit/Suite/lldbtestdumper.py
===
--- /dev/null
+++ lit/Suite/lldbtestdumper.py
@@ -0,0 +1,78 @@
+"""
+A utility that prints all tests in the dotest suite. It expects to be called
+with a single argument - path to the lldb executable.
+"""
+
+from __future__ import print_function
+import subprocess
+import os
+import os.path
+import sys
+
+lldb_test_root = None
+
+
+def get_tests(test_or_suite):
+import unittest2
+
+if not isinstance(test_or_suite, unittest2.suite.TestSuite):
+yield test_or_suite
+return
+
+for nested in test_or_suite:
+for test in get_tests(nested):
+yield test
+
+
+def set_up_environment():
+global lldb_test_root
+
+this_file_dir = os.path.abspath(os.path.dirname(__file__))
+lldb_root = os.path.abspath(os.path.join(this_file_dir, '..', '..'))
+lldb_test_root = os.path.join(
+lldb_root,
+'packages',
+'Python',
+'lldbsuite',
+'test')
+use_lldb_suite_dir = os.path.join(lldb_root, "scripts")
+
+lldb_executable = os.path.abspath(sys.argv[1])
+site_packages = subprocess.check_output([lldb_executable, '-P']).strip()
+
+sys.path[0:0] = [use_lldb_suite_dir, site_packages]
+import use_lldb_suite
+
+import lldb
+lldb.DBG = lldb.SBDebugger.Create()
+
+os.environ["LLDB_TEST"] = lldb_test_root
+
+
+def main():
+set_up_environment()
+
+import unittest2
+
+sys.path[0:0] = [""] # Placeholder for dirpath below
+for dirpath, _, filenames in os.walk(lldb_test_root):
+sys.path[0] = dirpath
+dirpath = os.path.relpath(dirpath, lldb_test_root)
+for name in filenames:
+base, ext = os.path.splitext(name)
+if ext != ".py":
+continue
+if not name.startswith("Test"):
+
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
JDevlieghere added inline comments. Comment at: lit/Suite/lldbtestdumper.py:27 + +def set_up_environment(): +global lldb_test_root A comment describing what and why this is necessary might be nice. Comment at: lit/Suite/lldbtestdumper.py:40 + +lldb_executable = os.path.abspath(sys.argv[1]) +site_packages = subprocess.check_output([lldb_executable, '-P']).strip() Any objection to parsing the arg in main and passing it as an argument? https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
clayborg added a comment.
I am trying to switch to using llvm::sys::path::remove_dots() and we will see
where we end up. By switching to this it means:
"./foo.c" --> "foo.c"
"./foo/bar.c" --> "foo/bar.c"
This makes is easier to see if a relative path matches another FileSpec since
we don't have to remove the "./" from the beginning of the path.
Comment at: source/Breakpoint/BreakpointResolverFileLine.cpp:131
+while (relative_path.consume_front("."))
+ /* Do nothing. */;
+ }
I will be removing this after I switch to using llvm::sys::path::remove_dots()
instead of the Normalize() I converted.
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);
labath wrote:
> aprantl wrote:
> > clayborg wrote:
> > > clayborg wrote:
> > > > aprantl wrote:
> > > > > clayborg wrote:
> > > > > > I am fine switching to using the llvm functions for removing, this
> > > > > > is only detecting if any normalization needs to happen. If there is
> > > > > > an equivalent LLVM function that will only run through the string
> > > > > > one time to detect all needs for normalization (no multiple passes
> > > > > > looking for "..", then for "." etc like we used to have), I would
> > > > > > be happy to use it, but there doesn't seem to be. We are trying to
> > > > > > avoid having to create a "SmallVectorImpl< char >" for all paths if
> > > > > > they don't need fixing here. This function is just iterating
> > > > > > through an llvm::StringRef just to see if normalization needs to
> > > > > > happen. If it does, then we make a SmallVectorImpl< char > and we
> > > > > > fix the path. We can easily use llvm functions for the fixing part.
> > > > > I see, you want to avoid copying the string when it isn't necessary
> > > > > to do so. IMHO the best way to do this is to add a `bool
> > > > > hasDots(const Twine &)` function to llvm::sys::path and add a `bool
> > > > > remove_dot_dot` parameter to `removeDots()`. Since you have already
> > > > > written the unittests, adding the function to LLVM shouldn't be any
> > > > > extra work (I'll help reviewing), and we'll have 100 LoC less to
> > > > > maintain.
> > > > >
> > > > > This way we can avoid having two functions that perform path
> > > > > normalization that superficially look like they might do the same
> > > > > thing, but a year from now nobody can really tell whether they behave
> > > > > exactly the same, and whether a bug found in one implementation
> > > > > should also be fixed in the other one, etc... .
> > > > I want to know if there are redundant slashes as well. I want something
> > > > like "bool llvm::sys::has(const Twine &, bool dots, bool dot_dots, bool
> > > > redundant_slashes)". But I don't see that getting accepted? I just want
> > > > a "bool llvm:sys::path::contains_redundant_stuff(const Twine &)". Not
> > > > sure on the name though. needs_normalization? can_be_shortened?
> > > > Everything in LLVM right now assumes that dots, dot dots, slashes and
> > > > realpath stuff happen in the same function. Not sure how to break that
> > > > up without ruining the performance of the one and only loop that should
> > > > be happening. I like the current approach since it doesn't require
> > > > chopping up the string into an array and iterating through the array.
> > > Also not sure if the LLVM stuff will try to substitute in the current
> > > working directory for "."?
> > I just read the source code of remove_dots()
> > (http://llvm.org/doxygen/Path_8cpp_source.html#l00699) and if I'm not
> > mistaken, it actually also removes double-separators as a side-effect,
> > since it iterates over the path components of the string as it constructs
> > the copy. It also seems to avoid the copy operation if it isn't necessary.
> >
> > Could you take another look? Perhaps I'm missing something, (or perhaps we
> > can just turn this into a small addition to remove_dots).
> >
> > > Everything in LLVM right now assumes that dots, dot dots, slashes and
> > > realpath stuff happen in the same function.
> > I'm not sure I understand.
> FWIW, this could be implemented in a much simpler way, while still making
> sure we run through the string only once. I'm thinking of something like:
> ```
> bool first = true;
> for(it = llvm::sys::path::begin(path, style), end =
> llvm::sys::path::end(path); it != end; ++it) {
> if (!first && (*it == "." || *it == "..")) return true;
> first = false;
> }
> return false;
> ```
This will all go away, I am just going to call llvm::sys::path::remove_dots()...
Comment at: source/Utility/FileSpec.cpp:187
+continue; // Skip '.' unless it is the first thing
+}
+else if (component != "..") {
Yes we would need to remove this. Again, I wil
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
labath added inline comments.
Comment at: source/Utility/FileSpec.cpp:406
+ m_directory.Clear();
+ }
}
clayborg wrote:
> > they resolve to the same file in a virtual filesystem, where all referenced
> > path components exist and are directories
>
> Normalizing happens after resolving and if we don't resolve a path, we have
> no way to know what is a directory and what isn't. We will be setting
> breakpoints for remote targets quite a bit in LLDB and ww can't assume or
> stat anything in a path. So I would say FileSpec's are equivalent if the
> relative paths match all components.
>
> > Now the (c) part would imply that foo/.. should normalize to ./. and not .,
> > which is a bit odd, but it is consistent with our stated intention of
> > preserving directory information. If we do not want to have ./. here, then
> > we need to come up with a different definition of what it means to be
> > "normalized".
>
> I guess we could make the m_directory contain "." and m_filename contain
> nothing for the "./." case. It doesn 't make sense to have "." in both
> places.
>> they resolve to the same file in a virtual filesystem, where all referenced
>> path components exist and are directories
> Normalizing happens after resolving and if we don't resolve a path, we have
> no way to know what is a directory and what isn't. We will be setting
> breakpoints for remote targets quite a bit in LLDB and ww can't assume or
> stat anything in a path.
Yes, I am aware of that. I am not saying this is how we should actually
implement the normalization algorithm. I am trying define what a
"normalization" is in the first place, so that we can then judge whether a
particular normalization algorithm is good or not. I think defining
normalization in terms of an actual filesystem makes sense, since at the end of
the day, our algorithm should somehow approximate what happens in real file
systems. I am not saying the algorithm should be doing any stats, but for the
verification (either in our heads or in the tests) we can use certainly use
stats or actual file systems.
> So I would say FileSpec's are equivalent if the relative paths match all
> components.
This is too vague to be useful. I have no idea how I would apply this
definition to determine if e.g. "/foo/../bar.txt" and "./bar.txt" are
equivalent. And you didn't say anything about how to derive the normal form for
a `FileSpec`.
>> Now the (c) part would imply that foo/.. should normalize to ./. and not .,
>> which is a bit odd, but it is consistent with our stated intention of
>> preserving directory information. If we do not want to have ./. here, then
>> we need to come up with a different definition of what it means to be
>> "normalized".
> I guess we could make the m_directory contain "." and m_filename contain
> nothing for the "./." case. It doesn 't make sense to have "." in both places.
I don't think that is very useful, as then this would be the only special case
where normalization would produce a FileSpec without a filename component.
Comment at: source/Utility/FileSpec.cpp:857
+!IsPathSeparator(components[i].back(), syntax) &&
+(result.empty() || !IsPathSeparator(result.back(), syntax)))
result += GetPreferredPathSeparator(syntax);
clayborg wrote:
> labath wrote:
> > Why is this necessary? `result` can't be empty because we have just
> > appended something to it (and we've checked that `components[i]` is
> > non-empty).
> Because if you don't check this joining {"/', "foo.txt"} will result in
> "//foo.txt" which is wrong,.
Yes, but didn't the original condition guard against that already?
We know that `components[i]` is non-empty, and we have just appended it to
`result` two lines above. So, unless I am missing something, `result.back()`
should be the same as `components[i].back()` and this additional check does not
buy us anything.
https://reviews.llvm.org/D45977
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
clayborg added inline comments.
Comment at: source/Utility/FileSpec.cpp:406
+ m_directory.Clear();
+ }
}
labath wrote:
> clayborg wrote:
> > > they resolve to the same file in a virtual filesystem, where all
> > > referenced path components exist and are directories
> >
> > Normalizing happens after resolving and if we don't resolve a path, we have
> > no way to know what is a directory and what isn't. We will be setting
> > breakpoints for remote targets quite a bit in LLDB and ww can't assume or
> > stat anything in a path. So I would say FileSpec's are equivalent if the
> > relative paths match all components.
> >
> > > Now the (c) part would imply that foo/.. should normalize to ./. and not
> > > ., which is a bit odd, but it is consistent with our stated intention of
> > > preserving directory information. If we do not want to have ./. here,
> > > then we need to come up with a different definition of what it means to
> > > be "normalized".
> >
> > I guess we could make the m_directory contain "." and m_filename contain
> > nothing for the "./." case. It doesn 't make sense to have "." in both
> > places.
> >> they resolve to the same file in a virtual filesystem, where all
> >> referenced path components exist and are directories
>
> > Normalizing happens after resolving and if we don't resolve a path, we have
> > no way to know what is a directory and what isn't. We will be setting
> > breakpoints for remote targets quite a bit in LLDB and ww can't assume or
> > stat anything in a path.
>
> Yes, I am aware of that. I am not saying this is how we should actually
> implement the normalization algorithm. I am trying define what a
> "normalization" is in the first place, so that we can then judge whether a
> particular normalization algorithm is good or not. I think defining
> normalization in terms of an actual filesystem makes sense, since at the end
> of the day, our algorithm should somehow approximate what happens in real
> file systems. I am not saying the algorithm should be doing any stats, but
> for the verification (either in our heads or in the tests) we can use
> certainly use stats or actual file systems.
>
> > So I would say FileSpec's are equivalent if the relative paths match all
> > components.
>
> This is too vague to be useful. I have no idea how I would apply this
> definition to determine if e.g. "/foo/../bar.txt" and "./bar.txt" are
> equivalent. And you didn't say anything about how to derive the normal form
> for a `FileSpec`.
>
> >> Now the (c) part would imply that foo/.. should normalize to ./. and not
> >> ., which is a bit odd, but it is consistent with our stated intention of
> >> preserving directory information. If we do not want to have ./. here, then
> >> we need to come up with a different definition of what it means to be
> >> "normalized".
>
> > I guess we could make the m_directory contain "." and m_filename contain
> > nothing for the "./." case. It doesn 't make sense to have "." in both
> > places.
>
> I don't think that is very useful, as then this would be the only special
> case where normalization would produce a FileSpec without a filename
> component.
>> So I would say FileSpec's are equivalent if the relative paths match all
>> components.
> This is too vague to be useful. I have no idea how I would apply this
> definition to determine if e.g. "/foo/../bar.txt" and "./bar.txt" are
> equivalent. And you didn't say anything about how to derive the normal form
> for a FileSpec.
"/foo/../bar.txt" would be normalized to "/bar.txt" and "./bar.txt" will, after
switching to llvm's remove_dots, be normalized to "bar.txt". So those could be
though of as equivalent since one is only a basename and would only need to
match the filename. If you have "/foo/bar/baz.txt", it could be equivalent to
"bar/baz.txt" by making sure the filename's match, and if either or both path
is relative, then matching as many directories as are specified.
>> I guess we could make the m_directory contain "." and m_filename contain
>> nothing for the "./." case. It doesn 't make sense to have "." in both
>> places.
> I don't think that is very useful, as then this would be the only special
> case where normalization would produce a FileSpec without a filename
> component.
Ok, then leave it as is with "." in filename, not directory. We don't need it
in both places IMHO
Comment at: source/Utility/FileSpec.cpp:857
+!IsPathSeparator(components[i].back(), syntax) &&
+(result.empty() || !IsPathSeparator(result.back(), syntax)))
result += GetPreferredPathSeparator(syntax);
labath wrote:
> clayborg wrote:
> > labath wrote:
> > > Why is this necessary? `result` can't be empty because we have just
> > > appended something to it (and we've checked that `components[i]` is
> > > non-empty).
> > Because if you don't check this joining {"
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
labath added inline comments. Comment at: source/Utility/FileSpec.cpp:406 + m_directory.Clear(); + } } clayborg wrote: > labath wrote: > > clayborg wrote: > > > > they resolve to the same file in a virtual filesystem, where all > > > > referenced path components exist and are directories > > > > > > Normalizing happens after resolving and if we don't resolve a path, we > > > have no way to know what is a directory and what isn't. We will be > > > setting breakpoints for remote targets quite a bit in LLDB and ww can't > > > assume or stat anything in a path. So I would say FileSpec's are > > > equivalent if the relative paths match all components. > > > > > > > Now the (c) part would imply that foo/.. should normalize to ./. and > > > > not ., which is a bit odd, but it is consistent with our stated > > > > intention of preserving directory information. If we do not want to > > > > have ./. here, then we need to come up with a different definition of > > > > what it means to be "normalized". > > > > > > I guess we could make the m_directory contain "." and m_filename contain > > > nothing for the "./." case. It doesn 't make sense to have "." in both > > > places. > > >> they resolve to the same file in a virtual filesystem, where all > > >> referenced path components exist and are directories > > > > > Normalizing happens after resolving and if we don't resolve a path, we > > > have no way to know what is a directory and what isn't. We will be > > > setting breakpoints for remote targets quite a bit in LLDB and ww can't > > > assume or stat anything in a path. > > > > Yes, I am aware of that. I am not saying this is how we should actually > > implement the normalization algorithm. I am trying define what a > > "normalization" is in the first place, so that we can then judge whether a > > particular normalization algorithm is good or not. I think defining > > normalization in terms of an actual filesystem makes sense, since at the > > end of the day, our algorithm should somehow approximate what happens in > > real file systems. I am not saying the algorithm should be doing any stats, > > but for the verification (either in our heads or in the tests) we can use > > certainly use stats or actual file systems. > > > > > So I would say FileSpec's are equivalent if the relative paths match all > > > components. > > > > This is too vague to be useful. I have no idea how I would apply this > > definition to determine if e.g. "/foo/../bar.txt" and "./bar.txt" are > > equivalent. And you didn't say anything about how to derive the normal form > > for a `FileSpec`. > > > > >> Now the (c) part would imply that foo/.. should normalize to ./. and not > > >> ., which is a bit odd, but it is consistent with our stated intention of > > >> preserving directory information. If we do not want to have ./. here, > > >> then we need to come up with a different definition of what it means to > > >> be "normalized". > > > > > I guess we could make the m_directory contain "." and m_filename contain > > > nothing for the "./." case. It doesn 't make sense to have "." in both > > > places. > > > > I don't think that is very useful, as then this would be the only special > > case where normalization would produce a FileSpec without a filename > > component. > >> So I would say FileSpec's are equivalent if the relative paths match all > >> components. > > > This is too vague to be useful. I have no idea how I would apply this > > definition to determine if e.g. "/foo/../bar.txt" and "./bar.txt" are > > equivalent. And you didn't say anything about how to derive the normal form > > for a FileSpec. > > "/foo/../bar.txt" would be normalized to "/bar.txt" and "./bar.txt" will, > after switching to llvm's remove_dots, be normalized to "bar.txt". So those > could be though of as equivalent since one is only a basename and would only > need to match the filename. If you have "/foo/bar/baz.txt", it could be > equivalent to "bar/baz.txt" by making sure the filename's match, and if > either or both path is relative, then matching as many directories as are > specified. > > > >> I guess we could make the m_directory contain "." and m_filename contain > >> nothing for the "./." case. It doesn 't make sense to have "." in both > >> places. > > > I don't think that is very useful, as then this would be the only special > > case where normalization would produce a FileSpec without a filename > > component. > > Ok, then leave it as is with "." in filename, not directory. We don't need it > in both places IMHO Ok, if we start using llvm's `remove_dots` as our normalization algorithm, then both of these issues will become moot (and I believe I have a fairly good understanding of how `remove_dots` works). Comment at: source/Utility/FileSpec.cpp:857 +!IsPathSeparator(components[i].back(), syntax) && +(result.
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
zturner added a comment. Does it print just the names of the .py files, or does it print the actual test classes and methods in the Class.TestName format, evaluate XFAIL and SKIPs, etc? I'm also not entirely clear why `set_up_environment` is even necessary. Nothing below the call to `set_up_environment` even seems to depend on the environment being set up. https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
labath added a comment. In https://reviews.llvm.org/D46005#1076978, @zturner wrote: > Does it print just the names of the .py files, or does it print the actual > test classes and methods in the Class.TestName format, evaluate XFAIL and > SKIPs, etc? It prints everything: test file (including the relative path in the test suite), test class name and test method name. It does not try to evaluate skips or xfails, but that was never the intention. The idea is to feed this information to lit (as it needs to know about the set of all tests), which can then run them with a bigger (smaller?) granularity. The interpretation of the test results (skip,fail, ...) will still be done by the lit test format, only now it could do it at a test-method level instead of file-level. > I'm also not entirely clear why `set_up_environment` is even necessary. > Nothing below the call to `set_up_environment` even seems to depend on the > environment being set up. In the main function I call `unittest2.defaultTestLoader.loadTestsFromName(base)`, which deep down does an `__import__(base)`. We need the environment of the import statement to be reasonably close to the the environment that `dotest` provides so that the import will succeed. https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
zturner added a comment. In https://reviews.llvm.org/D46005#1077000, @labath wrote: > In https://reviews.llvm.org/D46005#1076978, @zturner wrote: > > > Does it print just the names of the .py files, or does it print the actual > > test classes and methods in the Class.TestName format, evaluate XFAIL and > > SKIPs, etc? > > > It prints everything: test file (including the relative path in the test > suite), test class name and test method name. > > It does not try to evaluate skips or xfails, but that was never the > intention. The idea is to feed this information to lit (as it needs to know > about the set of all tests), which can then run them with a bigger (smaller?) > granularity. The interpretation of the test results (skip,fail, ...) will > still be done by the lit test format, only now it could do it at a > test-method level instead of file-level. I thought the intention was going to be parallelize at file-granularity rather than method granularity, since the whole point of grouping tests together into classes is that they can share similar set up and tear down which may be expensive to perform multiple times. If we're going to parallelize at method-granularity, I would rather have one .py file per method. https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
zturner added a comment. In https://reviews.llvm.org/D46005#1077013, @zturner wrote: > In https://reviews.llvm.org/D46005#1077000, @labath wrote: > > > In https://reviews.llvm.org/D46005#1076978, @zturner wrote: > > > > > Does it print just the names of the .py files, or does it print the > > > actual test classes and methods in the Class.TestName format, evaluate > > > XFAIL and SKIPs, etc? > > > > > > It prints everything: test file (including the relative path in the test > > suite), test class name and test method name. > > > > It does not try to evaluate skips or xfails, but that was never the > > intention. The idea is to feed this information to lit (as it needs to know > > about the set of all tests), which can then run them with a bigger > > (smaller?) granularity. The interpretation of the test results (skip,fail, > > ...) will still be done by the lit test format, only now it could do it at > > a test-method level instead of file-level. > > > I thought the intention was going to be parallelize at file-granularity > rather than method granularity, since the whole point of grouping tests > together into classes is that they can share similar set up and tear down > which may be expensive to perform multiple times. If we're going to > parallelize at method-granularity, I would rather have one .py file per > method. Note that there's currently no precedent (that i'm aware of anwyay) in LLVM or any of its subprojects for splitting the running of a single file into multiple parallel jobs. All of LLVM's other projects parallelize at file granularity, and so it's up to to the person writing tests to ensure that the file granularity matches that with which they want tests to be parallelized at. That doesn't mean it's not possible (as you've shown here), but it adds some additional complexity and I'm not sure it's worth it. There are currently two use cases I can come up with for grouping lots of tests together in a single class. 1. Sharing common set up and tear down code that is expensive. 2. Sharing common utility functions that all test cases need to use. For the first case, we don't *want* to parallelize at a finer granularity because it will cause extra work to be done. And for the second case, we can solve this by having a file in a directory named `util.py` and writing `from . import util` at the top of each test case file. Then have the lit config skip files named `util.py` in its discovery. https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
zturner added a comment. Actually, for the first case, I'm thinking we can extend lit to support somethign like per-directory set up. Like if there is a file present called `lit.init.cfg`, then this file contains some code that is run once per directory before any of the tests. Then we could still split up the test cases into multiple ones and parallelize finer. https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
clayborg updated this revision to Diff 143777.
clayborg added a comment.
Switch over to using llvm::sys::path::remove_dots(), remove the ::Normalize()
function and fix a few issue discovered during testing.
https://reviews.llvm.org/D45977
Files:
include/lldb/Core/FileSpecList.h
include/lldb/Utility/FileSpec.h
source/Breakpoint/BreakpointResolverFileLine.cpp
source/Core/FileSpecList.cpp
source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Symbol/CompileUnit.cpp
source/Symbol/Declaration.cpp
source/Utility/FileSpec.cpp
unittests/Utility/FileSpecTest.cpp
Index: unittests/Utility/FileSpecTest.cpp
===
--- unittests/Utility/FileSpecTest.cpp
+++ unittests/Utility/FileSpecTest.cpp
@@ -54,17 +54,14 @@
FileSpec fs_posix_trailing_slash("/foo/bar/", false,
FileSpec::ePathSyntaxPosix);
- EXPECT_STREQ("/foo/bar/.", fs_posix_trailing_slash.GetCString());
- EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetDirectory().GetCString());
- EXPECT_STREQ(".", fs_posix_trailing_slash.GetFilename().GetCString());
+ EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetCString());
+ EXPECT_STREQ("/foo", fs_posix_trailing_slash.GetDirectory().GetCString());
+ EXPECT_STREQ("bar", fs_posix_trailing_slash.GetFilename().GetCString());
FileSpec fs_windows_trailing_slash("F:\\bar\\", false,
FileSpec::ePathSyntaxWindows);
- EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString());
- // EXPECT_STREQ("F:\\bar",
- // fs_windows_trailing_slash.GetDirectory().GetCString()); // It returns
- // "F:/bar"
- EXPECT_STREQ(".", fs_windows_trailing_slash.GetFilename().GetCString());
+ EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetCString());
+ EXPECT_STREQ("bar", fs_windows_trailing_slash.GetFilename().GetCString());
}
TEST(FileSpecTest, AppendPathComponent) {
@@ -131,32 +128,13 @@
EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
}
-static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
-bool remove_backup_dots, bool result) {
- EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
- << "File one: " << one.GetCString() << "\nFile two: " << two.GetCString()
- << "\nFull match: " << full_match
- << "\nRemove backup dots: " << remove_backup_dots;
-}
-
TEST(FileSpecTest, EqualSeparator) {
FileSpec backward("C:\\foo\\bar", false, FileSpec::ePathSyntaxWindows);
FileSpec forward("C:/foo/bar", false, FileSpec::ePathSyntaxWindows);
EXPECT_EQ(forward, backward);
-
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
- Compare(forward, backward, full_match, remove_backup_dots, match);
- Compare(forward, backward, full_match, !remove_backup_dots, match);
- Compare(forward, backward, !full_match, remove_backup_dots, match);
- Compare(forward, backward, !full_match, !remove_backup_dots, match);
}
TEST(FileSpecTest, EqualDotsWindows) {
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
std::pair tests[] = {
{R"(C:\foo\bar\baz)", R"(C:\foo\foo\..\bar\baz)"},
{R"(C:\bar\baz)", R"(C:\foo\..\bar\baz)"},
@@ -170,18 +148,11 @@
for (const auto &test : tests) {
FileSpec one(test.first, false, FileSpec::ePathSyntaxWindows);
FileSpec two(test.second, false, FileSpec::ePathSyntaxWindows);
-EXPECT_NE(one, two);
-Compare(one, two, full_match, remove_backup_dots, match);
-Compare(one, two, full_match, !remove_backup_dots, !match);
-Compare(one, two, !full_match, remove_backup_dots, match);
-Compare(one, two, !full_match, !remove_backup_dots, !match);
+EXPECT_EQ(one, two);
}
}
TEST(FileSpecTest, EqualDotsPosix) {
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
std::pair tests[] = {
{R"(/foo/bar/baz)", R"(/foo/foo/../bar/baz)"},
{R"(/bar/baz)", R"(/foo/../bar/baz)"},
@@ -193,18 +164,11 @@
for (const auto &test : tests) {
FileSpec one(test.first, false, FileSpec::ePathSyntaxPosix);
FileSpec two(test.second, false, FileSpec::ePathSyntaxPosix);
-EXPECT_NE(one, two);
-Compare(one, two, full_match, remove_backup_dots, match);
-Compare(one, two, full_match, !remove_backup_dots, !match);
-Compare(one, two, !full_match, remove_backup_dots, match);
-Compare(one, two, !full_match, !remove_backup_dots, !match);
+EXPECT_EQ(one, two);
}
}
TEST(FileSpecTest, EqualDotsPosixRoot) {
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
std::pair tests[] = {
{R"(/)", R"(/..)"},
{R"(/)", R"(/
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
labath added a comment. In https://reviews.llvm.org/D46005#1077013, @zturner wrote: > I thought the intention was going to be parallelize at file-granularity > rather than method granularity, since the whole point of grouping tests > together into classes is that they can share similar set up and tear down > which may be expensive to perform multiple times. If we're going to > parallelize at method-granularity, I would rather have one .py file per > method. Hm... good question. I don't think any of our current tests take advantage of this possibility (sharing set-up code). Well.. except if you don't count bringing up the entire dotest machinery as set-up code (which does take a non-zero amount of time, but hopefully that can be brought down once we move everything to lit). However, maybe this does need more discussion. FWIW, I think that we shouldn't enforce the one-test-method-per-file completely rigidly. It may make sense to separate out some of the things, but I think that our tests often need some similar utility functions, which are too specific to put in some generic library, and it's easier to share these if they are in the same file. Having multiple tests in the same file also reduces the boiler plate and avoids another round of modifying each test file. (I think even lit acknowledges the usefulness of having logically separate tests in the same file (CHECK-LABEL)). https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46005: [test] Add a utility for dumping all tests in the dotest suite
zturner added a comment. In https://reviews.llvm.org/D46005#1077029, @labath wrote: > In https://reviews.llvm.org/D46005#1077013, @zturner wrote: > > > I thought the intention was going to be parallelize at file-granularity > > rather than method granularity, since the whole point of grouping tests > > together into classes is that they can share similar set up and tear down > > which may be expensive to perform multiple times. If we're going to > > parallelize at method-granularity, I would rather have one .py file per > > method. > > > Hm... good question. I don't think any of our current tests take advantage of > this possibility (sharing set-up code). Well.. except if you don't count > bringing up the entire dotest machinery as set-up code (which does take a > non-zero amount of time, but hopefully that can be brought down once we move > everything to lit). > > However, maybe this does need more discussion. > > FWIW, I think that we shouldn't enforce the one-test-method-per-file > completely rigidly. It may make sense to separate out some of the things, but > I think that our tests often need some similar utility functions, which are > too specific to put in some generic library, and it's easier to share these > if they are in the same file. Having multiple tests in the same file also > reduces the boiler plate and avoids another round of modifying each test > file. (I think even lit acknowledges the usefulness of having logically > separate tests in the same file (CHECK-LABEL)). I agree we shouldn't enforce one test per file. I just think it's may be worth only //parallelizing// at the file level. If you want two tests to be able to run in parallel, perhaps we should say that those tests must be in different files (perhaps even in the same directory). LLVM's lit / FileCheck tests often also have multiple tests in one file. But all tests in the same file run sequentially. https://reviews.llvm.org/D46005 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40474: DWZ 05/07: Main functionality
jankratochvil updated this revision to Diff 143785.
https://reviews.llvm.org/D40474
Files:
include/lldb/Utility/ConstString.h
include/lldb/Utility/FileSpec.h
source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.h
source/Plugins/SymbolFile/DWARF/DWARFDataExtractor.h
source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
source/Plugins/SymbolFile/DWARF/DWARFFormValue.cpp
source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFUnit.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
source/Utility/DataEncoder.cpp
source/Utility/DataExtractor.cpp
Index: source/Utility/DataExtractor.cpp
===
--- source/Utility/DataExtractor.cpp
+++ source/Utility/DataExtractor.cpp
@@ -232,7 +232,8 @@
if (data != nullptr) {
const uint8_t *data_bytes = data->GetBytes();
if (data_bytes != nullptr) {
-assert(m_start >= data_bytes);
+// For DWARFDataExtractor::OffsetData we need to return negative value.
+// assert(m_start >= data_bytes);
return m_start - data_bytes;
}
}
Index: source/Utility/DataEncoder.cpp
===
--- source/Utility/DataEncoder.cpp
+++ source/Utility/DataEncoder.cpp
@@ -81,7 +81,8 @@
if (data != nullptr) {
const uint8_t *data_bytes = data->GetBytes();
if (data_bytes != nullptr) {
-assert(m_start >= data_bytes);
+// For DWARFDataExtractor::OffsetData we need to return negative value.
+// assert(m_start >= data_bytes);
return m_start - data_bytes;
}
}
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -18,10 +18,12 @@
#include
#include
#include
+#include
// Other libraries and framework includes
#include "llvm/ADT/DenseMap.h"
#include "llvm/Support/Threading.h"
+#include "llvm/Support/RWMutex.h"
#include "lldb/Utility/Flags.h"
@@ -312,6 +314,13 @@
// the method returns a pointer to the base compile unit.
virtual DWARFUnit *GetBaseCompileUnit();
+ SymbolFileDWARF *GetDWZSymbolFile() const {
+if (!m_dwz_common_file)
+ return nullptr;
+return m_dwz_common_file->SymbolFile();
+ }
+ bool GetIsDWZ() const { return m_is_dwz; }
+
protected:
typedef llvm::DenseMap
DIEToTypePtr;
@@ -473,6 +482,45 @@
SymbolFileDWARFDwp *GetDwpSymbolFile();
+ void InitializeDWZ();
+
+ class DWZCommonFile {
+ public:
+// C++14: Use heterogenous lookup.
+DWZCommonFile(const lldb_private::FileSpec &filespec_ref);
+DWZCommonFile(std::unique_ptr symbol_file,
+lldb::ObjectFileSP obj_file, lldb::ModuleSP module);
+SymbolFileDWARF *SymbolFile() const { return m_symbol_file.get(); }
+
+bool operator==(const DWZCommonFile &rhs) const {
+ return m_filespec_ref == rhs.m_filespec_ref;
+}
+bool operator!=(const DWZCommonFile &rhs) const { return !(*this == rhs); }
+class Hasher {
+public:
+ size_t operator()(const DWZCommonFile &key) const {
+return lldb_private::FileSpec::Hasher()(key.m_filespec_ref);
+ }
+};
+
+size_t m_use_count = 0;
+
+ private:
+std::unique_ptr m_symbol_file;
+lldb::ObjectFileSP m_obj_file;
+lldb::ModuleSP m_module;
+const lldb_private::FileSpec &m_filespec_ref;
+
+DISALLOW_COPY_AND_ASSIGN(DWZCommonFile);
+ };
+ DWZCommonFile *m_dwz_common_file = nullptr;
+ void DWZCommonFileSet(DWZCommonFile *dwz_common_file);
+ void DWZCommonFileClear();
+ static std::unordered_set
+ m_filespec_to_dwzcommonfile;
+ static llvm::sys::RWMutex m_filespec_to_dwzcommonfile_mutex;
+ bool m_is_dwz = false;
+
lldb::ModuleWP m_debug_map_module_wp;
SymbolFileDWARFDebugMap *m_debug_map_symfile;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -410,7 +410,9 @@
m_supports_DW_AT_APPLE_objc_complete_type(eLazyBoolCalculate), m_ranges(),
m_unique_ast_type_map() {}
-SymbolFileDWARF::~SymbolFileDWARF() {}
+SymbolFileDWARF::~SymbolFileDWARF() {
+ DWZCommonFileClear();
+}
static const ConstString &GetDWARFMachOSegmentName() {
static ConstString g_dwarf_section_name("__DWARF");
@@ -426,6 +428,7 @@
}
TypeSystem *SymbolFileDWARF::GetTypeSystemForLanguage(LanguageType language) {
+ lldbassert(!GetIsDWZ());
SymbolFileDWARFDebugMap *debug_map_sy
[Lldb-commits] [PATCH] D40473: DWZ 04/07: Adjust existing code for the DWZ support.
jankratochvil updated this revision to Diff 143784.
https://reviews.llvm.org/D40473
Files:
source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
source/Plugins/SymbolFile/DWARF/DWARFDIE.h
source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
source/Plugins/SymbolFile/DWARF/DWARFUnit.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -734,6 +734,8 @@
DWARFUnit *dwarf_cu =
info->GetCompileUnit((dw_offset_t)comp_unit->GetID());
+if (dwarf_cu)
+ dwarf_cu = dwarf_cu->GetMainCU();
if (dwarf_cu && dwarf_cu->GetUserData() == NULL)
dwarf_cu->SetUserData(comp_unit);
return dwarf_cu;
@@ -763,6 +765,7 @@
uint32_t cu_idx) {
CompUnitSP cu_sp;
if (dwarf_cu) {
+dwarf_cu = dwarf_cu->GetMainCU();
CompileUnit *comp_unit = (CompileUnit *)dwarf_cu->GetUserData();
if (comp_unit) {
// We already parsed this compile unit, had out a shared pointer to it
@@ -1370,6 +1373,9 @@
Type *SymbolFileDWARF::ResolveTypeUID(const DWARFDIE &die,
bool assert_not_being_parsed) {
+ // this can be neither die.GetDWARF() nor die.GetMainDWARF().
+ if (die.GetMainDWARF() != this)
+return die.GetMainDWARF()->ResolveTypeUID(die, assert_not_being_parsed);
if (die) {
Log *log(LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO));
if (log)
@@ -1482,6 +1488,10 @@
Type *SymbolFileDWARF::ResolveType(const DWARFDIE &die,
bool assert_not_being_parsed,
bool resolve_function_context) {
+ // this can be neither die.GetDWARF() nor die.GetMainDWARF().
+ if (die.GetMainDWARF() != this)
+return die.GetMainDWARF()->ResolveType(
+die, assert_not_being_parsed, resolve_function_context);
if (die) {
Type *type = GetTypeForDIE(die, resolve_function_context).get();
@@ -1502,6 +1512,7 @@
CompileUnit *
SymbolFileDWARF::GetCompUnitForDWARFCompUnit(DWARFUnit *dwarf_cu,
uint32_t cu_idx) {
+ dwarf_cu = dwarf_cu->GetMainCU();
// Check if the symbol vendor already knows about this compile unit?
if (dwarf_cu->GetUserData() == NULL) {
// The symbol vendor doesn't know about this compile unit, we
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -132,6 +132,11 @@
dw_offset_t GetBaseObjOffset() const;
+ // DW_TAG_compile_unit with DW_TAG_imported_unit for this DW_TAG_partial_unit.
+ DWARFUnit *GetMainCU() const {
+return const_cast(this);
+ }
+
protected:
DWARFUnit(SymbolFileDWARF *dwarf);
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -445,6 +445,9 @@
}
TypeSystem *DWARFUnit::GetTypeSystem() {
+ if (GetMainCU() != this)
+return GetMainCU()->GetTypeSystem();
+
if (m_dwarf)
return m_dwarf->GetTypeSystemForLanguage(GetLanguageType());
else
@@ -495,7 +498,7 @@
// Don't specify the compile unit offset as we don't know it because the
// DIE belongs to
// a different compile unit in the same symbol file.
- return m_dwarf->DebugInfo()->GetDIEForDIEOffset(die_offset);
+ return GetMainCU()->m_dwarf->DebugInfo()->GetDIEForDIEOffset(die_offset);
}
}
m_dwarf->GetObjectFile()->GetModule()->ReportError(
@@ -633,6 +636,9 @@
}
LanguageType DWARFUnit::GetLanguageType() {
+ if (GetMainCU() != this)
+return GetMainCU()->GetLanguageType();
+
if (m_language_type != eLanguageTypeUnknown)
return m_language_type;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -569,7 +569,8 @@
if (ranges.IsEmpty() || name == NULL || mangled == NULL) {
for (const DIERef &die_ref : die_refs) {
if (die_ref.die_offset != DW_INVALID_OFFSET) {
-DWARFDIE die = dwarf2Data->GetDIE(die_ref);
+DWARFDIE die = cu->GetMainCU()->GetSymbolFileDWARF()
+->GetDIE(die_ref);
if (die)
die.GetDIE()->GetDIENamesAndRanges(
die.GetDWARF(), die.GetCU(), name, mangled, ranges, decl_file,
Index: source/Plu
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
aprantl added inline comments. Comment at: source/Utility/FileSpec.cpp:241 } + llvm::sys::path::remove_dots(resolved, true, LLVMPathSyntax(syntax)); `// Normalize the path by removing './' and other redundant components.` Comment at: source/Utility/FileSpec.cpp:242 - Normalize(resolved, m_syntax); + llvm::sys::path::remove_dots(resolved, true, LLVMPathSyntax(syntax)); Thanks! https://reviews.llvm.org/D45977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D40475: DWZ 06/07: DWZ test mode
jankratochvil updated this revision to Diff 143799.
https://reviews.llvm.org/D40475
Files:
packages/Python/lldbsuite/test/dotest.py
packages/Python/lldbsuite/test/lldbinline.py
packages/Python/lldbsuite/test/lldbtest.py
packages/Python/lldbsuite/test/make/Makefile.rules
packages/Python/lldbsuite/test/plugins/builder_base.py
packages/Python/lldbsuite/test/test_categories.py
unittests/SymbolFile/CMakeLists.txt
unittests/SymbolFile/DWZ/CMakeLists.txt
unittests/SymbolFile/DWZ/Inputs/dwztest.c
unittests/SymbolFile/DWZ/Inputs/dwztest.debug
unittests/SymbolFile/DWZ/Inputs/dwztest.debug.dwz
unittests/SymbolFile/DWZ/Inputs/dwztest.out
unittests/SymbolFile/DWZ/SymbolFileDWZTests.cpp
Index: unittests/SymbolFile/DWZ/SymbolFileDWZTests.cpp
===
--- /dev/null
+++ unittests/SymbolFile/DWZ/SymbolFileDWZTests.cpp
@@ -0,0 +1,89 @@
+//===-- SymbolFileDWZTests.cpp --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "gtest/gtest.h"
+
+#include "TestingSupport/TestUtilities.h"
+
+#include "Plugins/SymbolFile/DWARF/SymbolFileDWARF.h"
+#include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "lldb/Core/Module.h"
+#include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/ClangASTContext.h"
+#include "lldb/Symbol/SymbolVendor.h"
+#include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/FileSpec.h"
+
+#if defined(_MSC_VER)
+#include "lldb/Host/windows/windows.h"
+#include
+#endif
+
+#include
+
+using namespace lldb_private;
+
+class SymbolFileDWZTests : public testing::Test {
+public:
+ void SetUp() override {
+// Initialize and TearDown the plugin every time, so we get a brand new
+// AST every time so that modifications to the AST from each test don't
+// leak into the next test.
+#if defined(_MSC_VER)
+::CoInitializeEx(nullptr, COINIT_MULTITHREADED);
+#endif
+
+HostInfo::Initialize();
+SymbolFileDWARF::Initialize();
+ClangASTContext::Initialize();
+ObjectFileELF::Initialize();
+SymbolVendorELF::Initialize();
+
+m_dwztest_out = GetInputFilePath("dwztest.out");
+ }
+
+ void TearDown() override {
+SymbolVendorELF::Terminate();
+ObjectFileELF::Terminate();
+ClangASTContext::Terminate();
+SymbolFileDWARF::Terminate();
+HostInfo::Terminate();
+
+#if defined(_MSC_VER)
+::CoUninitialize();
+#endif
+ }
+
+protected:
+ std::string m_dwztest_out;
+};
+
+TEST_F(SymbolFileDWZTests, TestSimpleClassTypes) {
+ FileSpec fspec(m_dwztest_out.c_str(), false);
+ ArchSpec aspec("x86_64-pc-linux");
+ lldb::ModuleSP module = std::make_shared(fspec, aspec);
+
+ SymbolVendor *plugin = module->GetSymbolVendor();
+ EXPECT_NE(nullptr, plugin);
+ SymbolFile *symfile = plugin->GetSymbolFile();
+ EXPECT_NE(nullptr, symfile);
+ EXPECT_EQ(symfile->GetPluginName(), SymbolFileDWARF::GetPluginNameStatic());
+ SymbolContext sc;
+ llvm::DenseSet searched_files;
+ TypeMap results;
+ EXPECT_EQ(1u,
+ symfile->FindTypes(sc, ConstString("StructMovedToDWZCommonFile"), nullptr,
+ false, 0, searched_files, results));
+ EXPECT_EQ(1u, results.GetSize());
+ lldb::TypeSP udt_type = results.GetTypeAtIndex(0);
+ EXPECT_EQ(ConstString("StructMovedToDWZCommonFile"), udt_type->GetName());
+ CompilerType compiler_type = udt_type->GetForwardCompilerType();
+ EXPECT_TRUE(ClangASTContext::IsClassType(compiler_type.GetOpaqueQualType()));
+}
Index: unittests/SymbolFile/DWZ/Inputs/dwztest.c
===
--- /dev/null
+++ unittests/SymbolFile/DWZ/Inputs/dwztest.c
@@ -0,0 +1,9 @@
+// gcc -Wall -g -o dwztest.out dwztest.c; eu-strip --remove-comment -f dwztest.debug dwztest.out; cp -p dwztest.debug dwztest.debug.dup; dwz -m dwztest.debug.dwz dwztest.debug dwztest.debug.dup;rm dwztest.debug.dup; /usr/lib/rpm/sepdebugcrcfix . dwztest.out
+
+struct StructMovedToDWZCommonFile {
+ int i1, i2, i3, i4, i5, i6, i7, i8, i9;
+} VarWithStructMovedToDWZCommonFile;
+static const int sizeof_StructMovedToDWZCommonFile = sizeof(struct StructMovedToDWZCommonFile);
+int main() {
+ return sizeof_StructMovedToDWZCommonFile;
+}
Index: unittests/SymbolFile/DWZ/CMakeLists.txt
===
--- /dev/null
+++ unittests/SymbolFile/DWZ/CMakeLists.txt
@@ -0,0 +1,21 @@
+add_lldb_unittest(SymbolFileDWZTests
+ SymbolFileDWZTests.cpp
+
+ LINK_LIBS
+lldbCore
+lldbHost
+lldbSymbol
+lldbPluginSymbolFileDWARF
+lldbUtilityHelpers
+lldbPluginObjectFileELF
+lldbPluginSymbolVendorELF
+ LINK_COMPONENTS
+Support
+ )
+
+set(test_inputs
+ dwztest.out
+ dwztest.debug
+ dwztest.debug
[Lldb-commits] [PATCH] D45977: Always normalize FileSpec paths.
clayborg updated this revision to Diff 143801.
clayborg added a comment.
Added comment before llvm::sys::path_remove_dots(...)
https://reviews.llvm.org/D45977
Files:
include/lldb/Core/FileSpecList.h
include/lldb/Utility/FileSpec.h
source/Breakpoint/BreakpointResolverFileLine.cpp
source/Core/FileSpecList.cpp
source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Symbol/CompileUnit.cpp
source/Symbol/Declaration.cpp
source/Utility/FileSpec.cpp
unittests/Utility/FileSpecTest.cpp
Index: unittests/Utility/FileSpecTest.cpp
===
--- unittests/Utility/FileSpecTest.cpp
+++ unittests/Utility/FileSpecTest.cpp
@@ -54,17 +54,14 @@
FileSpec fs_posix_trailing_slash("/foo/bar/", false,
FileSpec::ePathSyntaxPosix);
- EXPECT_STREQ("/foo/bar/.", fs_posix_trailing_slash.GetCString());
- EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetDirectory().GetCString());
- EXPECT_STREQ(".", fs_posix_trailing_slash.GetFilename().GetCString());
+ EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetCString());
+ EXPECT_STREQ("/foo", fs_posix_trailing_slash.GetDirectory().GetCString());
+ EXPECT_STREQ("bar", fs_posix_trailing_slash.GetFilename().GetCString());
FileSpec fs_windows_trailing_slash("F:\\bar\\", false,
FileSpec::ePathSyntaxWindows);
- EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString());
- // EXPECT_STREQ("F:\\bar",
- // fs_windows_trailing_slash.GetDirectory().GetCString()); // It returns
- // "F:/bar"
- EXPECT_STREQ(".", fs_windows_trailing_slash.GetFilename().GetCString());
+ EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetCString());
+ EXPECT_STREQ("bar", fs_windows_trailing_slash.GetFilename().GetCString());
}
TEST(FileSpecTest, AppendPathComponent) {
@@ -131,32 +128,13 @@
EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
}
-static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
-bool remove_backup_dots, bool result) {
- EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
- << "File one: " << one.GetCString() << "\nFile two: " << two.GetCString()
- << "\nFull match: " << full_match
- << "\nRemove backup dots: " << remove_backup_dots;
-}
-
TEST(FileSpecTest, EqualSeparator) {
FileSpec backward("C:\\foo\\bar", false, FileSpec::ePathSyntaxWindows);
FileSpec forward("C:/foo/bar", false, FileSpec::ePathSyntaxWindows);
EXPECT_EQ(forward, backward);
-
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
- Compare(forward, backward, full_match, remove_backup_dots, match);
- Compare(forward, backward, full_match, !remove_backup_dots, match);
- Compare(forward, backward, !full_match, remove_backup_dots, match);
- Compare(forward, backward, !full_match, !remove_backup_dots, match);
}
TEST(FileSpecTest, EqualDotsWindows) {
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
std::pair tests[] = {
{R"(C:\foo\bar\baz)", R"(C:\foo\foo\..\bar\baz)"},
{R"(C:\bar\baz)", R"(C:\foo\..\bar\baz)"},
@@ -170,18 +148,11 @@
for (const auto &test : tests) {
FileSpec one(test.first, false, FileSpec::ePathSyntaxWindows);
FileSpec two(test.second, false, FileSpec::ePathSyntaxWindows);
-EXPECT_NE(one, two);
-Compare(one, two, full_match, remove_backup_dots, match);
-Compare(one, two, full_match, !remove_backup_dots, !match);
-Compare(one, two, !full_match, remove_backup_dots, match);
-Compare(one, two, !full_match, !remove_backup_dots, !match);
+EXPECT_EQ(one, two);
}
}
TEST(FileSpecTest, EqualDotsPosix) {
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
std::pair tests[] = {
{R"(/foo/bar/baz)", R"(/foo/foo/../bar/baz)"},
{R"(/bar/baz)", R"(/foo/../bar/baz)"},
@@ -193,18 +164,11 @@
for (const auto &test : tests) {
FileSpec one(test.first, false, FileSpec::ePathSyntaxPosix);
FileSpec two(test.second, false, FileSpec::ePathSyntaxPosix);
-EXPECT_NE(one, two);
-Compare(one, two, full_match, remove_backup_dots, match);
-Compare(one, two, full_match, !remove_backup_dots, !match);
-Compare(one, two, !full_match, remove_backup_dots, match);
-Compare(one, two, !full_match, !remove_backup_dots, !match);
+EXPECT_EQ(one, two);
}
}
TEST(FileSpecTest, EqualDotsPosixRoot) {
- const bool full_match = true;
- const bool remove_backup_dots = true;
- const bool match = true;
std::pair tests[] = {
{R"(/)", R"(/..)"},
{R"(/)", R"(/.)"},
@@ -214,11 +178,7 @@
for (const auto &test : tests) {
FileSpec one(
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
alur added a comment. Friendly ping, is there anything else I need to do for this to get submitted? https://reviews.llvm.org/D45628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
Hi Erik, the review is still marked as requiring changes. Once that is sorted out I'd be happy to submit this on your behalf (what is the base SVN revision for the latest patch?) Davide Italiano, is all the CR feedback addressed in the latest revision? On Tue, Apr 24, 2018 at 1:38 PM, Erik Welander via Phabricator via lldb-commits wrote: > alur added a comment. > > Friendly ping, is there anything else I need to do for this to get > submitted? > > > https://reviews.llvm.org/D45628 > > > > ___ > lldb-commits mailing list > lldb-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D45628: [LLDB] Support compressed debug info sections (.zdebug*)
lemo added subscribers: bgianfo, clayborg, alur, labath, penryu, lemo. lemo added a comment. Hi Erik, the review is still marked as requiring changes. Once that is sorted out I'd be happy to submit this on your behalf (what is the base SVN revision for the latest patch?) Davide Italiano, is all the CR feedback addressed in the latest revision? https://reviews.llvm.org/D45628 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
