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

This revision was automatically updated to reflect the committed changes. Closed by commit rL331049: Always normalize FileSpec paths. (authored by gclayton, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D45977?vs=144021=144353#toc

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

labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. Looks fine to me. Normalization, at least as it is implemented now in `remove_dots`, is a fairly heavy operation, so it makes sense to avoid it when possible. And the extra speedup is great.

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

clayborg updated this revision to Diff 144021. clayborg added a comment. After doing performance tests, the code was 7 to 10 % slower if we didn't check if a path needs normalization due to the llvm code making arrays of StringRef objects and appending a path together. Restored and even

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

clayborg added a comment. In https://reviews.llvm.org/D45977#1077719, @labath wrote: > This code itself looks fine, I have just two minor comments. > > However, I do have a question about performance. I remember us being very > worried about performance in the past, so we ended up putting in

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

> On Apr 25, 2018, at 12:54 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > This code itself looks fine, I have just two minor comments. > > However, I do have a question about performance. I remember us being very > worried about

[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

[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:

[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

[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

[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 > >

[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

[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`

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

aprantl added a comment. I'll take that back: make_absolute only copies when the path isn't already absolute. https://reviews.llvm.org/D45977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

aprantl added a comment. > Also not sure if the LLVM stuff will try to substitute in the current working > directory for "."? No it won't, remove_dots() has no side effects. There's a separate make_absolute() function. That one will cause another copy. Personally, if I have a choice between a

[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: > clayborg wrote:

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

clayborg 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); aprantl wrote: > clayborg wrote:

[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: > I am fine

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

clayborg added a comment. In https://reviews.llvm.org/D45977#1076048, @aprantl wrote: > One general question: why is this form of normalization preferred over > calling realpath? Normalization is everything we can do to fix up a path without knowing anything about the current working

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

clayborg 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); I am fine switching to using the

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

aprantl added a comment. One general question: why is this form of normalization preferred over calling realpath? https://reviews.llvm.org/D45977 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

[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

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

clayborg updated this revision to Diff 143643. clayborg added a comment. Fix comment typo https://reviews.llvm.org/D45977 Files: include/lldb/Core/FileSpecList.h include/lldb/Utility/FileSpec.h source/Breakpoint/BreakpointResolverFileLine.cpp source/Core/FileSpecList.cpp

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

clayborg added inline comments. Comment at: source/Utility/FileSpec.cpp:105 +//-- +bool needsNormalization(const llvm::StringRef ) { + for (auto i = path.find('/'); i != llvm::StringRef::npos; I

[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

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

clayborg updated this revision to Diff 143609. clayborg added a comment. Remove commented out code. https://reviews.llvm.org/D45977 Files: include/lldb/Core/FileSpecList.h include/lldb/Utility/FileSpec.h source/Breakpoint/BreakpointResolverFileLine.cpp source/Core/FileSpecList.cpp

[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

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

clayborg updated this revision to Diff 143608. clayborg added a comment. Remove unwanted file changes. https://reviews.llvm.org/D45977 Files: include/lldb/Core/FileSpecList.h include/lldb/Utility/FileSpec.h source/Breakpoint/BreakpointResolverFileLine.cpp source/Core/FileSpecList.cpp

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

clayborg created this revision. clayborg added reviewers: labath, jingham, davide, pranavb. Herald added subscribers: JDevlieghere, aprantl. Always normalizing lldb_private::FileSpec paths will help us get a consistent results from comparisons when setting breakpoints and when looking for source