subject:"\[Lldb\-commits\] \[PATCH\] D68609\: Replace regex match with rfind \(NFCish\)"

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

2019-10-08 Thread Adrian Prantl via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGea6377505435: Replace regex match with rfind (NFCish) 
(authored by aprantl).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D68609?vs=223695=223887#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68609/new/

https://reviews.llvm.org/D68609

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Symbol/ObjectFile.cpp


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -19,7 +19,6 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
-#include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/lldb-private.h"
 
@@ -83,9 +82,8 @@
 
   if (!data_sp || data_sp->GetByteSize() == 0) {
 // Check for archive file with format "/path/to/archive.a(object.o)"
-char path_with_object[PATH_MAX * 2];
-module_sp->GetFileSpec().GetPath(path_with_object,
- sizeof(path_with_object));
+llvm::SmallString<256> path_with_object;
+module_sp->GetFileSpec().GetPath(path_with_object);
 
 ConstString archive_object;
 const bool must_exist = true;
@@ -571,21 +569,22 @@
   }
 }
 
-bool ObjectFile::SplitArchivePathWithObject(const char *path_with_object,
+bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
 FileSpec _file,
 ConstString _object,
 bool must_exist) {
-  llvm::SmallVector matches;
-  RegularExpression g_object_regex(llvm::StringRef("(.*)\\(([^\\)]+)\\)$"));
-  if 
(g_object_regex.Execute(llvm::StringRef::withNullAsEmpty(path_with_object),
- )) {
-std::string path = matches[1].str();
-std::string obj = matches[2].str();
-archive_file.SetFile(path, FileSpec::Style::native);
-archive_object.SetCString(obj.c_str());
-return !(must_exist && !FileSystem::Instance().Exists(archive_file));
-  }
-  return false;
+  size_t len = path_with_object.size();
+  if (len < 2 || path_with_object.back() != ')')
+return false;
+  llvm::StringRef archive = path_with_object.substr(0, 
path_with_object.rfind('('));
+  if (archive.empty())
+return false;
+  llvm::StringRef object = path_with_object.substr(archive.size() + 
1).drop_back();
+  archive_file.SetFile(archive, FileSpec::Style::native);
+  if (must_exist && !FileSystem::Instance().Exists(archive_file))
+return false;
+  archive_object.SetString(object);
+  return true;
 }
 
 void ObjectFile::ClearSymtab() {
Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -201,7 +201,7 @@
   /// \b false otherwise and \a archive_file and \a archive_object
   /// are guaranteed to be remain unchanged.
   static bool SplitArchivePathWithObject(
-  const char *path_with_object, lldb_private::FileSpec _file,
+  llvm::StringRef path_with_object, lldb_private::FileSpec _file,
   lldb_private::ConstString _object, bool must_exist);
 
   // LLVM RTTI support


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -19,7 +19,6 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
-#include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/lldb-private.h"
 
@@ -83,9 +82,8 @@
 
   if (!data_sp || data_sp->GetByteSize() == 0) {
 // Check for archive file with format "/path/to/archive.a(object.o)"
-char path_with_object[PATH_MAX * 2];
-module_sp->GetFileSpec().GetPath(path_with_object,
- sizeof(path_with_object));
+llvm::SmallString<256> path_with_object;
+module_sp->GetFileSpec().GetPath(path_with_object);
 
 ConstString archive_object;
 const bool must_exist = true;
@@ -571,21 +569,22 @@
   }
 }
 
-bool ObjectFile::SplitArchivePathWithObject(const char *path_with_object,
+bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
 FileSpec _file,
 ConstString _object,
 bool must_exist) {
-  llvm::SmallVector matches;
-  RegularExpression g_object_regex(llvm::StringRef("(.*)\\(([^\\)]+)\\)$"));
-  if

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

2019-10-08 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/source/Symbol/ObjectFile.cpp:85
 // Check for archive file with format "/path/to/archive.a(object.o)"
-char path_with_object[PATH_MAX * 2];
-module_sp->GetFileSpec().GetPath(path_with_object,
- sizeof(path_with_object));
+llvm::SmallString path_with_object;
+module_sp->GetFileSpec().GetPath(path_with_object);

now that this is self-resizing, this can probably be something smaller 
(PATH_MAX on windows is 32k, sort of)..



Comment at: lldb/source/Symbol/ObjectFile.cpp:577
+  size_t len = path_with_object.size();
+  if (len < 2 || path_with_object[len-1] != ')')
+return false;

`.back()` ?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68609/new/

https://reviews.llvm.org/D68609



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

2019-10-07 Thread Adrian Prantl via Phabricator via lldb-commits

aprantl created this revision.
aprantl added reviewers: JDevlieghere, friss.

This change is mostly performance-neutral since our regex engine is fast, but 
it's IMHO slightly more readable.
Also, matching matching parenthesis is not a great match for regular 
expressions.


https://reviews.llvm.org/D68609

Files:
  lldb/include/lldb/Symbol/ObjectFile.h
  lldb/source/Symbol/ObjectFile.cpp


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -19,7 +19,6 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
-#include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/lldb-private.h"
 
@@ -83,9 +82,8 @@
 
   if (!data_sp || data_sp->GetByteSize() == 0) {
 // Check for archive file with format "/path/to/archive.a(object.o)"
-char path_with_object[PATH_MAX * 2];
-module_sp->GetFileSpec().GetPath(path_with_object,
- sizeof(path_with_object));
+llvm::SmallString path_with_object;
+module_sp->GetFileSpec().GetPath(path_with_object);
 
 ConstString archive_object;
 const bool must_exist = true;
@@ -571,21 +569,22 @@
   }
 }
 
-bool ObjectFile::SplitArchivePathWithObject(const char *path_with_object,
+bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
 FileSpec _file,
 ConstString _object,
 bool must_exist) {
-  llvm::SmallVector matches;
-  RegularExpression g_object_regex(llvm::StringRef("(.*)\\(([^\\)]+)\\)$"));
-  if 
(g_object_regex.Execute(llvm::StringRef::withNullAsEmpty(path_with_object),
- )) {
-std::string path = matches[1].str();
-std::string obj = matches[2].str();
-archive_file.SetFile(path, FileSpec::Style::native);
-archive_object.SetCString(obj.c_str());
-return !(must_exist && !FileSystem::Instance().Exists(archive_file));
-  }
-  return false;
+  size_t len = path_with_object.size();
+  if (len < 2 || path_with_object[len-1] != ')')
+return false;
+  llvm::StringRef archive = path_with_object.substr(0, 
path_with_object.rfind('('));
+  if (archive.empty())
+return false;
+  llvm::StringRef object = path_with_object.substr(archive.size() + 
1).drop_back();
+  archive_file.SetFile(archive, FileSpec::Style::native);
+  if (must_exist && !FileSystem::Instance().Exists(archive_file))
+return false;
+  archive_object.SetString(object);
+  return true;
 }
 
 void ObjectFile::ClearSymtab() {
Index: lldb/include/lldb/Symbol/ObjectFile.h
===
--- lldb/include/lldb/Symbol/ObjectFile.h
+++ lldb/include/lldb/Symbol/ObjectFile.h
@@ -201,7 +201,7 @@
   /// \b false otherwise and \a archive_file and \a archive_object
   /// are guaranteed to be remain unchanged.
   static bool SplitArchivePathWithObject(
-  const char *path_with_object, lldb_private::FileSpec _file,
+  llvm::StringRef path_with_object, lldb_private::FileSpec _file,
   lldb_private::ConstString _object, bool must_exist);
 
   // LLVM RTTI support


Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -19,7 +19,6 @@
 #include "lldb/Utility/DataBuffer.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/Log.h"
-#include "lldb/Utility/RegularExpression.h"
 #include "lldb/Utility/Timer.h"
 #include "lldb/lldb-private.h"
 
@@ -83,9 +82,8 @@
 
   if (!data_sp || data_sp->GetByteSize() == 0) {
 // Check for archive file with format "/path/to/archive.a(object.o)"
-char path_with_object[PATH_MAX * 2];
-module_sp->GetFileSpec().GetPath(path_with_object,
- sizeof(path_with_object));
+llvm::SmallString path_with_object;
+module_sp->GetFileSpec().GetPath(path_with_object);
 
 ConstString archive_object;
 const bool must_exist = true;
@@ -571,21 +569,22 @@
   }
 }
 
-bool ObjectFile::SplitArchivePathWithObject(const char *path_with_object,
+bool ObjectFile::SplitArchivePathWithObject(llvm::StringRef path_with_object,
 FileSpec _file,
 ConstString _object,
 bool must_exist) {
-  llvm::SmallVector matches;
-  RegularExpression g_object_regex(llvm::StringRef("(.*)\\(([^\\)]+)\\)$"));
-  if (g_object_regex.Execute(llvm::StringRef::withNullAsEmpty(path_with_object),
- )) {
-std::string path = matches[1].str();
-std::string obj =

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

[Lldb-commits] [PATCH] D68609: Replace regex match with rfind (NFCish)

3 matches

Site Navigation

Mail list logo

Footer information