[Lldb-commits] [lldb] r335432 - [FileSpec] Always normalize
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 25 03:16:49 PDT 2018
Thanks. Reverted in r335448.
On Mon, Jun 25, 2018 at 9:59 AM Pavel Labath <labath at 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 at 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 at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180625/15c18588/attachment-0001.html>
More information about the lldb-commits
mailing list