Make source_location use string_view instead of const string&

Summary:
`const std::string&` requires the caller to copy if all they have is `std::string_view`. Let's modernize the code here.

Heterogenous lookups are not supported by `std::unordered_map` until C++20 so I've had to switch to `std::map`. This shouldn't be a problem in practice.

Change is no-op.

Reviewed By: iahs

Differential Revision: D69197835

fbshipit-source-id: 83b0849a5bada97f9c41b13845a5567aeb2f2694

Author: praihan

thrift/compiler/source_location.cc

@@ -19,10 +19,10 @@
 
 #include <fmt/core.h>
 
-#include <assert.h>
-#include <errno.h>
-#include <string.h>
 #include <algorithm>
+#include <cassert>
+#include <cerrno>
+#include <cstring>
 #include <filesystem>
 #include <stdexcept>
 #include <string>
@@ -84,24 +84,24 @@ std::vector<uint_least32_t> get_line_offsets(std::string_view sv) {
 } // namespace
 
 source source_manager::add_source(
-    const std::string& file_name, std::vector<char> text) {
+    std::string_view file_name, std::vector<char> text) {
   assert(text.back() == '\0');
   std::string_view sv(text.data(), text.size());
-  sources_.push_back(
-      source_info{file_name, std::move(text), get_line_offsets(sv)});
+  sources_.push_back(source_info{
+      std::string(file_name), std::move(text), get_line_offsets(sv)});
   return {/* .start = */
           source_location(/* source_id= */ sources_.size(), /** offset= */ 0),
           /* .text = */ sv};
 }
 
-std::string source_manager::get_file_path(const std::string& file_name) const {
+std::string source_manager::get_file_path(std::string_view file_name) const {
   if (file_source_map_.find(file_name) != file_source_map_.end()) {
-    return file_name;
+    return std::string(file_name);
   }
   return std::filesystem::absolute(file_name).string();
 }
 
-std::optional<source> source_manager::get_file(const std::string& file_name) {
+std::optional<source> source_manager::get_file(std::string_view file_name) {
   if (auto source = file_source_map_.find(file_name);
       source != file_source_map_.end()) {
     return source->second;
@@ -156,11 +156,11 @@ std::optional<source> source_manager::get_file(const std::string& file_name) {
 }
 
 source source_manager::add_virtual_file(
-    const std::string& file_name, const std::string& src) {
+    std::string_view file_name, std::string_view src) {
   if (file_source_map_.find(file_name) != file_source_map_.end()) {
-    throw std::runtime_error(std::string("file already added: ") + file_name);
+    throw std::runtime_error(fmt::format("file already added: {}", file_name));
   }
-  const char* start = src.c_str();
+  const char* start = src.data();
   auto source =
       add_source(file_name, std::vector<char>(start, start + src.size() + 1));
   file_source_map_.emplace(file_name, source);
@@ -202,21 +202,21 @@ resolved_location::resolved_location(
 }
 
 source_manager::path_or_error source_manager::find_include_file(
-    const std::string& filename,
-    const std::string& parent_path,
+    std::string_view filename,
+    std::string_view parent_path,
     const std::vector<std::string>& search_paths) {
   if (auto itr = found_includes_.find(filename); itr != found_includes_.end()) {
     return source_manager::path_or_error{std::in_place_index<0>, itr->second};
   }
 
   auto found = [&](std::string path) {
-    found_includes_[filename] = path;
+    found_includes_[std::string(filename)] = path;
     return source_manager::path_or_error{
         std::in_place_index<0>, std::move(path)};
   };
 
   if (file_source_map_.find(filename) != file_source_map_.end()) {
-    return found(filename);
+    return found(std::string(filename));
   }
 
   // Absolute path? Just try that.
@@ -236,7 +236,7 @@ source_manager::path_or_error source_manager::find_include_file(
   // new search path with current dir global
   std::vector<std::string> sp = search_paths;
   auto itr = found_includes_.find(parent_path);
-  const std::string& resolved_parent_path =
+  std::string_view resolved_parent_path =
       itr != found_includes_.end() ? itr->second : parent_path;
   auto dir = std::filesystem::path(resolved_parent_path).parent_path().string();
   dir = dir.empty() ? "." : dir;
@@ -271,7 +271,7 @@ source_manager::path_or_error source_manager::find_include_file(
 }
 
 std::optional<std::string> source_manager::found_include_file(
-    const std::string& filename) const {
+    std::string_view filename) const {
   if (auto itr = found_includes_.find(filename); itr != found_includes_.end()) {
     return itr->second;
   }

thrift/compiler/source_location.h

@@ -16,13 +16,13 @@
 
 #pragma once
 
-#include <stdint.h>
-
 #include <cstdint>
 #include <deque>
+#include <functional>
+#include <map>
 #include <optional>
 #include <string>
-#include <unordered_map>
+#include <string_view>
 #include <variant>
 #include <vector>
 
@@ -115,10 +115,10 @@ class source_manager {
   // sources_ grows.
   std::deque<source_info> sources_;
 
-  std::unordered_map<std::string, source> file_source_map_;
+  std::map<std::string, source, std::less<>> file_source_map_;
 
   // Maps from filepaths present in the AST to filepaths on disk.
-  std::unordered_map<std::string, std::string> found_includes_;
+  std::map<std::string, std::string, std::less<>> found_includes_;
 
   const source_info* get_source(uint_least32_t source_id) const {
     return source_id > 0 && source_id <= sources_.size()
@@ -128,20 +128,20 @@ class source_manager {
 
   friend class resolved_location;
 
-  source add_source(const std::string& file_name, std::vector<char> text);
+  source add_source(std::string_view file_name, std::vector<char> text);
 
  public:
   // Loads a file and returns a source object representing its content.
   // The file can be a real file or a virtual one previously registered with
   // add_virtual_file.
   // Returns an empty optional if opening or reading the file fails.
   // Makes use of the result of previous calls to find_include_file.
-  std::optional<source> get_file(const std::string& file_name);
+  std::optional<source> get_file(std::string_view file_name);
 
-  std::string get_file_path(const std::string& file_name) const;
+  std::string get_file_path(std::string_view file_name) const;
 
   // Adds a virtual file with the specified name and content.
-  source add_virtual_file(const std::string& file_name, const std::string& src);
+  source add_virtual_file(std::string_view file_name, std::string_view src);
 
   // Returns the start location of a source containing the specified location.
   // It is a member function in case we add clang-like compression of locations.
@@ -171,12 +171,12 @@ class source_manager {
   // The resolved path is made available to get_file.
   using path_or_error = std::variant<std::string, std::string>;
   path_or_error find_include_file(
-      const std::string& filename,
-      const std::string& program_path,
+      std::string_view filename,
+      std::string_view program_path,
       const std::vector<std::string>& search_paths);
   // Queries for a file previously found by find_include_file.
   std::optional<std::string> found_include_file(
-      const std::string& filename) const;
+      std::string_view filename) const;
 };
 
 } // namespace apache::thrift::compiler