Pass and move some strings as std::string arround

bertwesarg · bertwesarg · commit 7ec85af36584 · 2023-11-20T21:48:42.000+01:00
diff --git a/src/methods.cpp b/src/methods.cpp
@@ -69,20 +69,20 @@ extern "C"
             return NULL;
         }
 
-        std::string_view module(module_cstr, module_len);
+        std::string module(module_cstr, module_len);
         std::string_view function_name(function_name_cstr, function_name_len);
-        std::string_view file_name(file_name_cstr, file_name_len);
-
-        std::string file_name_abs = scorepy::abspath(file_name);
+        auto const file_name_abs =
+            scorepy::abspath(std::string_view(file_name_cstr, file_name_len));
 
         if (identifier == nullptr or identifier == Py_None)
         {
-            scorepy::region_begin(function_name, module, file_name_abs, line_number);
+            scorepy::region_begin(function_name, std::move(module), std::move(file_name_abs),
+                                  line_number);
         }
         else
         {
-            scorepy::region_begin(function_name, module, file_name_abs, line_number,
-                                  reinterpret_cast<PyCodeObject*>(identifier));
+            scorepy::region_begin(function_name, std::move(module), std::move(file_name_abs),
+                                  line_number, reinterpret_cast<PyCodeObject*>(identifier));
         }
 
         Py_RETURN_NONE;
@@ -124,16 +124,17 @@ extern "C"
             return NULL;
         }
 
-        std::string_view module(module_cstr, module_len);
+        std::string module(module_cstr, module_len);
         std::string_view function_name(function_name_cstr, function_name_len);
 
         if (identifier == nullptr or identifier == Py_None)
         {
-            scorepy::region_end(function_name, module);
+            scorepy::region_end(function_name, std::move(module));
         }
         else
         {
-            scorepy::region_end(function_name, module, reinterpret_cast<PyCodeObject*>(identifier));
+            scorepy::region_end(function_name, std::move(module),
+                                reinterpret_cast<PyCodeObject*>(identifier));
         }
 
         Py_RETURN_NONE;
diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp
@@ -124,8 +124,8 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*)
             if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0)
             {
                 const int line_number = code->co_firstlineno;
-                const std::string file_name = get_file_name(frame);
-                region_begin(name, module_name, file_name, line_number, code);
+                const auto file_name = get_file_name(frame);
+                region_begin(name, std::move(module_name), std::move(file_name), line_number, code);
             }
         }
         Py_DECREF(code);
@@ -142,7 +142,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*)
             // TODO: Use string_view/CString comparison?
             if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0)
             {
-                region_end(name, module_name, code);
+                region_end(name, std::move(module_name), code);
             }
         }
         Py_DECREF(code);
diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp
@@ -28,44 +28,45 @@ static compat::RegisterCodeDealloc register_dealloc(on_dealloc);
 
 // Used for regions, that have an identifier, aka a code object id. (instrumenter regions and
 // some decorated regions)
-void region_begin(std::string_view function_name, std::string_view module,
-                  const std::string& file_name, const std::uint64_t line_number,
-                  compat::PyCodeObject* identifier)
+void region_begin(std::string_view function_name, std::string module, std::string file_name,
+                  const std::uint64_t line_number, compat::PyCodeObject* identifier)
 {
     region_handle& region = regions[identifier];
 
     if (region == uninitialised_region_handle)
     {
-        const auto region_name = make_region_name(module, function_name);
+        const auto region_name = make_region_name(std::move(module), function_name);
         SCOREP_User_RegionInit(&region.value, NULL, NULL, region_name.c_str(),
                                SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);
 
-        SCOREP_User_RegionSetGroup(region.value, std::string(module, 0, module.find('.')).c_str());
+        SCOREP_User_RegionSetGroup(region.value,
+                                   std::string(region_name, 0, region_name.find('.')).c_str());
     }
     SCOREP_User_RegionEnter(region.value);
 }
 
 // Used for regions, that only have a function name, a module, a file and a line number (user
 // regions)
-void region_begin(std::string_view function_name, std::string_view module,
-                  const std::string& file_name, const std::uint64_t line_number)
+void region_begin(std::string_view function_name, std::string module, std::string file_name,
+                  const std::uint64_t line_number)
 {
-    const auto region_name = make_region_name(module, function_name);
+    const auto region_name = make_region_name(std::move(module), function_name);
     region_handle& region = user_regions[region_name];
 
     if (region == uninitialised_region_handle)
     {
         SCOREP_User_RegionInit(&region.value, NULL, NULL, region_name.c_str(),
                                SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);
 
-        SCOREP_User_RegionSetGroup(region.value, std::string(module, 0, module.find('.')).c_str());
+        SCOREP_User_RegionSetGroup(region.value,
+                                   std::string(region_name, 0, region_name.find('.')).c_str());
     }
     SCOREP_User_RegionEnter(region.value);
 }
 
 // Used for regions, that have an identifier, aka a code object id. (instrumenter regions and
 // some decorated regions)
-void region_end(std::string_view function_name, std::string_view module,
+void region_end(std::string_view function_name, std::string module,
                 compat::PyCodeObject* identifier)
 {
     const auto it_region = regions.find(identifier);
@@ -75,27 +76,27 @@ void region_end(std::string_view function_name, std::string_view module,
     }
     else
     {
-        const auto region_name = make_region_name(module, function_name);
-        region_end_error_handling(region_name);
+        const auto region_name = make_region_name(std::move(module), function_name);
+        region_end_error_handling(std::move(region_name));
     }
 }
 
 // Used for regions, that only have a function name, a module (user regions)
-void region_end(std::string_view function_name, std::string_view module)
+void region_end(std::string_view function_name, std::string module)
 {
-    const auto region_name = make_region_name(module, function_name);
+    const auto region_name = make_region_name(std::move(module), function_name);
     auto it_region = user_regions.find(region_name);
     if (it_region != user_regions.end())
     {
         SCOREP_User_RegionEnd(it_region->second.value);
     }
     else
     {
-        region_end_error_handling(region_name);
+        region_end_error_handling(std::move(region_name));
     }
 }
 
-void region_end_error_handling(const std::string& region_name)
+void region_end_error_handling(std::string region_name)
 {
     static region_handle error_region;
     static SCOREP_User_ParameterHandle scorep_param = SCOREP_USER_INVALID_PARAMETER;
diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp
@@ -30,13 +30,11 @@ struct region_handle
 constexpr region_handle uninitialised_region_handle = region_handle();
 
 /// Combine the arguments into a region name
-inline std::string make_region_name(std::string_view module_name, std::string_view name)
+inline std::string make_region_name(std::string module_name, std::string_view name)
 {
-    std::string region;
-    region += module_name;
-    region += ":";
-    region += name;
-    return std::move(region);
+    module_name += ':';
+    module_name += name;
+    return std::move(module_name);
 }
 
 extern std::unordered_map<compat::PyCodeObject*, region_handle> regions;
@@ -58,11 +56,10 @@ inline bool try_region_begin(compat::PyCodeObject* identifier)
     }
 }
 
-void region_begin(std::string_view function_name, std::string_view module,
-                  const std::string& file_name, const std::uint64_t line_number,
-                  compat::PyCodeObject* identifier);
-void region_begin(std::string_view function_name, std::string_view module,
-                  const std::string& file_name, const std::uint64_t line_number);
+void region_begin(std::string_view function_name, std::string module, std::string file_name,
+                  const std::uint64_t line_number, compat::PyCodeObject* identifier);
+void region_begin(std::string_view function_name, std::string module, std::string file_name,
+                  const std::uint64_t line_number);
 
 /** tries to end a region. Return true on success
  *
@@ -81,11 +78,11 @@ inline bool try_region_end(compat::PyCodeObject* identifier)
     }
 }
 
-void region_end(std::string_view function_name, std::string_view module,
+void region_end(std::string_view function_name, std::string module,
                 compat::PyCodeObject* identifier);
-void region_end(std::string_view function_name, std::string_view module);
+void region_end(std::string_view function_name, std::string module);
 
-void region_end_error_handling(const std::string& region_name);
+void region_end_error_handling(std::string region_name);
 
 void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number);
 void rewind_end(std::string region_name, bool value);
diff --git a/src/scorepy/pathUtils.cpp b/src/scorepy/pathUtils.cpp
@@ -25,7 +25,7 @@ static std::string getcwd()
         result.resize(result.find('\0', result.size() - chunk_size));
     else
         result.clear();
-    return result;
+    return std::move(result);
 }
 
 void normalize_path(std::string& path)
@@ -80,7 +80,7 @@ std::string abspath(std::string_view input_path)
         result = getcwd();
         // On error exit
         if (result.empty())
-            return result;
+            return {};
         // Prepend CWD
         result.append(1, '/').append(input_path);
     }
@@ -89,6 +89,6 @@ std::string abspath(std::string_view input_path)
         result = input_path;
     }
     normalize_path(result);
-    return result;
+    return std::move(result);
 }
 } // namespace scorepy
diff --git a/src/scorepy/pythonHelpers.cpp b/src/scorepy/pythonHelpers.cpp
@@ -7,23 +7,23 @@
 
 namespace scorepy
 {
-std::string_view get_module_name(PyFrameObject& frame)
+std::string get_module_name(PyFrameObject& frame)
 {
     PyObject* globals = PyFrame_GetGlobals(&frame);
     PyObject* module_name = PyDict_GetItemString(globals, "__name__");
     Py_DECREF(globals);
     if (module_name)
-        return compat::get_string_as_utf_8(module_name);
+        return std::move(std::string(compat::get_string_as_utf_8(module_name)));
 
     // this is a NUMPY special situation, see NEP-18, and Score-P issue #63
     // TODO: Use string_view/C-String to avoid creating 2 std::strings
     PyCodeObject* code = PyFrame_GetCode(&frame);
     std::string_view filename = compat::get_string_as_utf_8(code->co_filename);
     Py_DECREF(code);
     if ((filename.size() > 0) && (filename == "<__array_function__ internals>"))
-        return std::string_view("numpy.__array_function__");
+        return std::move(std::string("numpy.__array_function__"));
     else
-        return std::string_view("unkown");
+        return std::move(std::string("unkown"));
 }
 
 std::string get_file_name(PyFrameObject& frame)
@@ -33,9 +33,9 @@ std::string get_file_name(PyFrameObject& frame)
     Py_DECREF(code);
     if (filename == Py_None)
     {
-        return "None";
+        return std::move(std::string("None"));
     }
-    const std::string full_file_name = abspath(compat::get_string_as_utf_8(filename));
-    return !full_file_name.empty() ? full_file_name : "ErrorPath";
+    const auto full_file_name = abspath(compat::get_string_as_utf_8(filename));
+    return !full_file_name.empty() ? std::move(full_file_name) : "ErrorPath";
 }
 } // namespace scorepy
diff --git a/src/scorepy/pythonHelpers.hpp b/src/scorepy/pythonHelpers.hpp
@@ -73,7 +73,7 @@ auto cast_to_PyFunc(TFunc* func) -> detail::ReplaceArgsToPyObject_t<TFunc>*
 
 /// Return the module name the frame belongs to.
 /// The pointer is valid for the lifetime of the frame
-std::string_view get_module_name(PyFrameObject& frame);
+std::string get_module_name(PyFrameObject& frame);
 /// Return the file name the frame belongs to
 std::string get_file_name(PyFrameObject& frame);
 

Original file line number	Diff line number	Diff line change
`@@ -69,20 +69,20 @@ extern "C"`
`69`	`69`	`return NULL;`
`70`	`70`	`}`
`71`	`71`
`72`		`- std::string_view module(module_cstr, module_len);`
	`72`	`+ std::string module(module_cstr, module_len);`
`73`	`73`	`std::string_view function_name(function_name_cstr, function_name_len);`
`74`		`- std::string_view file_name(file_name_cstr, file_name_len);`
`75`		`-`
`76`		`- std::string file_name_abs = scorepy::abspath(file_name);`
	`74`	`+ auto const file_name_abs =`
	`75`	`+ scorepy::abspath(std::string_view(file_name_cstr, file_name_len));`
`77`	`76`
`78`	`77`	`if (identifier == nullptr or identifier == Py_None)`
`79`	`78`	`{`
`80`		`- scorepy::region_begin(function_name, module, file_name_abs, line_number);`
	`79`	`+ scorepy::region_begin(function_name, std::move(module), std::move(file_name_abs),`
	`80`	`+ line_number);`
`81`	`81`	`}`
`82`	`82`	`else`
`83`	`83`	`{`
`84`		`- scorepy::region_begin(function_name, module, file_name_abs, line_number,`
`85`		`- reinterpret_cast<PyCodeObject*>(identifier));`
	`84`	`+ scorepy::region_begin(function_name, std::move(module), std::move(file_name_abs),`
	`85`	`+ line_number, reinterpret_cast<PyCodeObject*>(identifier));`
`86`	`86`	`}`
`87`	`87`
`88`	`88`	`Py_RETURN_NONE;`
`@@ -124,16 +124,17 @@ extern "C"`
`124`	`124`	`return NULL;`
`125`	`125`	`}`
`126`	`126`
`127`		`- std::string_view module(module_cstr, module_len);`
	`127`	`+ std::string module(module_cstr, module_len);`
`128`	`128`	`std::string_view function_name(function_name_cstr, function_name_len);`
`129`	`129`
`130`	`130`	`if (identifier == nullptr or identifier == Py_None)`
`131`	`131`	`{`
`132`		`- scorepy::region_end(function_name, module);`
	`132`	`+ scorepy::region_end(function_name, std::move(module));`
`133`	`133`	`}`
`134`	`134`	`else`
`135`	`135`	`{`
`136`		`- scorepy::region_end(function_name, module, reinterpret_cast<PyCodeObject*>(identifier));`
	`136`	`+ scorepy::region_end(function_name, std::move(module),`
	`137`	`+ reinterpret_cast<PyCodeObject*>(identifier));`
`137`	`138`	`}`
`138`	`139`
`139`	`140`	`Py_RETURN_NONE;`
Original file line number	Diff line number	Diff line change
`@@ -124,8 +124,8 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*)`
`124`	`124`	`if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0)`
`125`	`125`	`{`
`126`	`126`	`const int line_number = code->co_firstlineno;`
`127`		`- const std::string file_name = get_file_name(frame);`
`128`		`- region_begin(name, module_name, file_name, line_number, code);`
	`127`	`+ const auto file_name = get_file_name(frame);`
	`128`	`+ region_begin(name, std::move(module_name), std::move(file_name), line_number, code);`
`129`	`129`	`}`
`130`	`130`	`}`
`131`	`131`	`Py_DECREF(code);`
`@@ -142,7 +142,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*)`
`142`	`142`	`// TODO: Use string_view/CString comparison?`
`143`	`143`	`if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0)`
`144`	`144`	`{`
`145`		`- region_end(name, module_name, code);`
	`145`	`+ region_end(name, std::move(module_name), code);`
`146`	`146`	`}`
`147`	`147`	`}`
`148`	`148`	`Py_DECREF(code);`
Original file line number	Diff line number	Diff line change
`@@ -28,44 +28,45 @@ static compat::RegisterCodeDealloc register_dealloc(on_dealloc);`
`28`	`28`
`29`	`29`	`// Used for regions, that have an identifier, aka a code object id. (instrumenter regions and`
`30`	`30`	`// some decorated regions)`
`31`		`-void region_begin(std::string_view function_name, std::string_view module,`
`32`		`- const std::string& file_name, const std::uint64_t line_number,`
`33`		`- compat::PyCodeObject* identifier)`
	`31`	`+void region_begin(std::string_view function_name, std::string module, std::string file_name,`
	`32`	`+ const std::uint64_t line_number, compat::PyCodeObject* identifier)`
`34`	`33`	`{`
`35`	`34`	`region_handle& region = regions[identifier];`
`36`	`35`
`37`	`36`	`if (region == uninitialised_region_handle)`
`38`	`37`	`{`
`39`		`- const auto region_name = make_region_name(module, function_name);`
	`38`	`+ const auto region_name = make_region_name(std::move(module), function_name);`
`40`	`39`	`SCOREP_User_RegionInit(&region.value, NULL, NULL, region_name.c_str(),`
`41`	`40`	`SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);`
`42`	`41`
`43`		`- SCOREP_User_RegionSetGroup(region.value, std::string(module, 0, module.find('.')).c_str());`
	`42`	`+ SCOREP_User_RegionSetGroup(region.value,`
	`43`	`+ std::string(region_name, 0, region_name.find('.')).c_str());`
`44`	`44`	`}`
`45`	`45`	`SCOREP_User_RegionEnter(region.value);`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`// Used for regions, that only have a function name, a module, a file and a line number (user`
`49`	`49`	`// regions)`
`50`		`-void region_begin(std::string_view function_name, std::string_view module,`
`51`		`- const std::string& file_name, const std::uint64_t line_number)`
	`50`	`+void region_begin(std::string_view function_name, std::string module, std::string file_name,`
	`51`	`+ const std::uint64_t line_number)`
`52`	`52`	`{`
`53`		`- const auto region_name = make_region_name(module, function_name);`
	`53`	`+ const auto region_name = make_region_name(std::move(module), function_name);`
`54`	`54`	`region_handle& region = user_regions[region_name];`
`55`	`55`
`56`	`56`	`if (region == uninitialised_region_handle)`
`57`	`57`	`{`
`58`	`58`	`SCOREP_User_RegionInit(&region.value, NULL, NULL, region_name.c_str(),`
`59`	`59`	`SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);`
`60`	`60`
`61`		`- SCOREP_User_RegionSetGroup(region.value, std::string(module, 0, module.find('.')).c_str());`
	`61`	`+ SCOREP_User_RegionSetGroup(region.value,`
	`62`	`+ std::string(region_name, 0, region_name.find('.')).c_str());`
`62`	`63`	`}`
`63`	`64`	`SCOREP_User_RegionEnter(region.value);`
`64`	`65`	`}`
`65`	`66`
`66`	`67`	`// Used for regions, that have an identifier, aka a code object id. (instrumenter regions and`
`67`	`68`	`// some decorated regions)`
`68`		`-void region_end(std::string_view function_name, std::string_view module,`
	`69`	`+void region_end(std::string_view function_name, std::string module,`
`69`	`70`	`compat::PyCodeObject* identifier)`
`70`	`71`	`{`
`71`	`72`	`const auto it_region = regions.find(identifier);`
`@@ -75,27 +76,27 @@ void region_end(std::string_view function_name, std::string_view module,`
`75`	`76`	`}`
`76`	`77`	`else`
`77`	`78`	`{`
`78`		`- const auto region_name = make_region_name(module, function_name);`
`79`		`- region_end_error_handling(region_name);`
	`79`	`+ const auto region_name = make_region_name(std::move(module), function_name);`
	`80`	`+ region_end_error_handling(std::move(region_name));`
`80`	`81`	`}`
`81`	`82`	`}`
`82`	`83`
`83`	`84`	`// Used for regions, that only have a function name, a module (user regions)`
`84`		`-void region_end(std::string_view function_name, std::string_view module)`
	`85`	`+void region_end(std::string_view function_name, std::string module)`
`85`	`86`	`{`
`86`		`- const auto region_name = make_region_name(module, function_name);`
	`87`	`+ const auto region_name = make_region_name(std::move(module), function_name);`
`87`	`88`	`auto it_region = user_regions.find(region_name);`
`88`	`89`	`if (it_region != user_regions.end())`
`89`	`90`	`{`
`90`	`91`	`SCOREP_User_RegionEnd(it_region->second.value);`
`91`	`92`	`}`
`92`	`93`	`else`
`93`	`94`	`{`
`94`		`- region_end_error_handling(region_name);`
	`95`	`+ region_end_error_handling(std::move(region_name));`
`95`	`96`	`}`
`96`	`97`	`}`
`97`	`98`
`98`		`-void region_end_error_handling(const std::string& region_name)`
	`99`	`+void region_end_error_handling(std::string region_name)`
`99`	`100`	`{`
`100`	`101`	`static region_handle error_region;`
`101`	`102`	`static SCOREP_User_ParameterHandle scorep_param = SCOREP_USER_INVALID_PARAMETER;`
Original file line number	Diff line number	Diff line change
`@@ -25,7 +25,7 @@ static std::string getcwd()`
`25`	`25`	`result.resize(result.find('\0', result.size() - chunk_size));`
`26`	`26`	`else`
`27`	`27`	`result.clear();`
`28`		`- return result;`
	`28`	`+ return std::move(result);`
`29`	`29`	`}`
`30`	`30`
`31`	`31`	`void normalize_path(std::string& path)`
`@@ -80,7 +80,7 @@ std::string abspath(std::string_view input_path)`
`80`	`80`	`result = getcwd();`
`81`	`81`	`// On error exit`
`82`	`82`	`if (result.empty())`
`83`		`- return result;`
	`83`	`+ return {};`
`84`	`84`	`// Prepend CWD`
`85`	`85`	`result.append(1, '/').append(input_path);`
`86`	`86`	`}`
`@@ -89,6 +89,6 @@ std::string abspath(std::string_view input_path)`
`89`	`89`	`result = input_path;`
`90`	`90`	`}`
`91`	`91`	`normalize_path(result);`
`92`		`- return result;`
	`92`	`+ return std::move(result);`
`93`	`93`	`}`
`94`	`94`	`} // namespace scorepy`