Thanks. Reverted in r335448. On Mon, Jun 25, 2018 at 9:59 AM Pavel Labath <lab...@google.com> wrote:
> I believe the raison d'être of the needsNormalization function was the > normalization process is not exactly cheap (it constructs a vector of > StringRefs and whatnot), and measurements showed that this actually is > important for performance of lldb > <https://reviews.llvm.org/D45977#1078510>. > On Sun, 24 Jun 2018 at 14:36, Jonas Devlieghere via lldb-commits > <lldb-commits@lists.llvm.org> wrote: > > > > Author: jdevlieghere > > Date: Sun Jun 24 06:31:44 2018 > > New Revision: 335432 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=335432&view=rev > > Log: > > [FileSpec] Always normalize > > > > Removing redundant components from the path seems pretty harmless. > > Rather than checking whether this is necessary and then actually doing > > so, always invoke remove_dots to start with a normalized path. > > > > Modified: > > lldb/trunk/source/Utility/FileSpec.cpp > > > > Modified: lldb/trunk/source/Utility/FileSpec.cpp > > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=335432&r1=335431&r2=335432&view=diff > > > ============================================================================== > > --- lldb/trunk/source/Utility/FileSpec.cpp (original) > > +++ lldb/trunk/source/Utility/FileSpec.cpp Sun Jun 24 06:31:44 2018 > > @@ -119,100 +119,6 @@ FileSpec::FileSpec(const FileSpec *rhs) > > //------------------------------------------------------------------ > > FileSpec::~FileSpec() {} > > > > -namespace { > > -//------------------------------------------------------------------ > > -/// Safely get a character at the specified index. > > -/// > > -/// @param[in] path > > -/// A full, partial, or relative path to a file. > > -/// > > -/// @param[in] i > > -/// An index into path which may or may not be valid. > > -/// > > -/// @return > > -/// The character at index \a i if the index is valid, or 0 if > > -/// the index is not valid. > > -//------------------------------------------------------------------ > > -inline char safeCharAtIndex(const llvm::StringRef &path, size_t i) { > > - if (i < path.size()) > > - return path[i]; > > - return 0; > > -} > > - > > -//------------------------------------------------------------------ > > -/// Check if a path needs to be normalized. > > -/// > > -/// Check if a path needs to be normalized. We currently consider a > > -/// path to need normalization if any of the following are true > > -/// - path contains "/./" > > -/// - path contains "/../" > > -/// - path contains "//" > > -/// - path ends with "/" > > -/// Paths that start with "./" or with "../" are not considered to > > -/// need normalization since we aren't trying to resolve the path, > > -/// we are just trying to remove redundant things from the path. > > -/// > > -/// @param[in] path > > -/// A full, partial, or relative path to a file. > > -/// > > -/// @return > > -/// Returns \b true if the path needs to be normalized. > > -//------------------------------------------------------------------ > > -bool needsNormalization(const llvm::StringRef &path) { > > - if (path.empty()) > > - return false; > > - // We strip off leading "." values so these paths need to be > normalized > > - if (path[0] == '.') > > - return true; > > - for (auto i = path.find_first_of("\\/"); i != llvm::StringRef::npos; > > - i = path.find_first_of("\\/", i + 1)) { > > - const auto next = safeCharAtIndex(path, i+1); > > - switch (next) { > > - case 0: > > - // path separator char at the end of the string which should be > > - // stripped unless it is the one and only character > > - return i > 0; > > - case '/': > > - case '\\': > > - // two path separator chars in the middle of a path needs to be > > - // normalized > > - if (i > 0) > > - return true; > > - ++i; > > - break; > > - > > - case '.': { > > - const auto next_next = safeCharAtIndex(path, i+2); > > - switch (next_next) { > > - default: break; > > - case 0: return true; // ends with "/." > > - case '/': > > - case '\\': > > - return true; // contains "/./" > > - case '.': { > > - const auto next_next_next = safeCharAtIndex(path, i+3); > > - switch (next_next_next) { > > - default: break; > > - case 0: return true; // ends with "/.." > > - case '/': > > - case '\\': > > - return true; // contains "/../" > > - } > > - break; > > - } > > - } > > - } > > - break; > > - > > - default: > > - break; > > - } > > - } > > - return false; > > -} > > - > > - > > -} > > //------------------------------------------------------------------ > > // Assignment operator. > > //------------------------------------------------------------------ > > @@ -252,8 +158,7 @@ void FileSpec::SetFile(llvm::StringRef p > > } > > > > // Normalize the path by removing ".", ".." and other redundant > components. > > - if (needsNormalization(resolved)) > > - llvm::sys::path::remove_dots(resolved, true, m_style); > > + llvm::sys::path::remove_dots(resolved, true, m_style); > > > > // Normalize back slashes to forward slashes > > if (m_style == Style::windows) > > @@ -270,10 +175,10 @@ void FileSpec::SetFile(llvm::StringRef p > > // Split path into filename and directory. We rely on the underlying > char > > // pointer to be nullptr when the components are empty. > > llvm::StringRef filename = llvm::sys::path::filename(resolved, > m_style); > > - if(!filename.empty()) > > + if (!filename.empty()) > > m_filename.SetString(filename); > > llvm::StringRef directory = llvm::sys::path::parent_path(resolved, > m_style); > > - if(!directory.empty()) > > + if (!directory.empty()) > > m_directory.SetString(directory); > > } > > > > @@ -424,9 +329,8 @@ bool FileSpec::Equal(const FileSpec &a, > > // case sensitivity of equality test > > const bool case_sensitive = a.IsCaseSensitive() || > b.IsCaseSensitive(); > > > > - const bool filenames_equal = ConstString::Equals(a.m_filename, > > - b.m_filename, > > - case_sensitive); > > + const bool filenames_equal = > > + ConstString::Equals(a.m_filename, b.m_filename, case_sensitive); > > > > if (!filenames_equal) > > return false; > > @@ -712,9 +616,7 @@ bool FileSpec::IsSourceImplementationFil > > return g_source_file_regex.Execute(extension.GetStringRef()); > > } > > > > -bool FileSpec::IsRelative() const { > > - return !IsAbsolute(); > > -} > > +bool FileSpec::IsRelative() const { return !IsAbsolute(); } > > > > bool FileSpec::IsAbsolute() const { > > llvm::SmallString<64> current_path; > > > > > > _______________________________________________ > > 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
