Skip to content

Commit 044723a

Browse files
aristidispfacebook-github-bot
authored andcommitted
Readability nit: rename source to source_view
Summary: Motivation: `source` is overly broad and confusing, whereas `source_view` better captures its nature. Note that, for now, both names are still valid and aliases of each other, to allow for a migration period. Also added some documentation. Reviewed By: vitaut Differential Revision: D79928195 fbshipit-source-id: f40a6da51b5fae11e912ee49aa98594e4d971043
1 parent e9148d6 commit 044723a

File tree

8 files changed

+59
-24
lines changed

8 files changed

+59
-24
lines changed

third-party/thrift/src/thrift/compiler/codemod/test/file_manager_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class FileManagerTest : public ::testing::Test {
5656
}
5757

5858
source_manager source_manager_;
59-
source source_;
59+
source_view source_;
6060
std::unique_ptr<t_program_bundle> program_bundle_;
6161
std::unique_ptr<file_manager> file_manager_;
6262
};

third-party/thrift/src/thrift/compiler/generate/t_whisker_generator.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -570,7 +570,7 @@ class t_whisker_generator::whisker_source_parser
570570
return &cached->second.value();
571571
}
572572

573-
std::optional<source> source_code = src_manager_.get_file(path);
573+
std::optional<source_view> source_code = src_manager_.get_file(path);
574574
if (!source_code.has_value()) {
575575
return nullptr;
576576
}

third-party/thrift/src/thrift/compiler/parse/lexer.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,9 @@ const std::unordered_map<std::string_view, tok> keywords = {
175175
} // namespace
176176

177177
lexer::lexer(
178-
source src, diagnostics_engine& diags, doc_comment_handler on_doc_comment)
178+
source_view src,
179+
diagnostics_engine& diags,
180+
doc_comment_handler on_doc_comment)
179181
: source_(src.text),
180182
start_(src.start),
181183
diags_(&diags),

third-party/thrift/src/thrift/compiler/parse/lexer.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class lexer {
8585
// on_doc_comment is invoked on a documentation comment such as
8686
// `/** ... */` or `/// ...`.
8787
lexer(
88-
source src,
88+
source_view src,
8989
diagnostics_engine& diags,
9090
doc_comment_handler on_doc_comment = ignore_comments);
9191

third-party/thrift/src/thrift/compiler/source_location.cc

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,16 @@ class default_source_manager_backend final : public source_manager_backend {
133133
source_manager::source_manager()
134134
: backend_(std::make_unique<default_source_manager_backend>()) {}
135135

136-
source source_manager::add_source(
136+
source_view source_manager::add_source(
137137
std::string_view file_name, std::vector<char> text) {
138138
assert(text.back() == '\0');
139139
std::string_view sv(text.data(), text.size());
140140
sources_.push_back(source_info{
141141
std::string(file_name), std::move(text), get_line_offsets(sv)});
142-
return {/* .start = */
143-
source_location(/* source_id= */ sources_.size(), /** offset= */ 0),
144-
/* .text = */ sv};
142+
return source_view{
143+
/* .start = */
144+
source_location(/* source_id= */ sources_.size(), /* offset= */ 0),
145+
/* .text = */ sv};
145146
}
146147

147148
std::string source_manager::get_file_path(std::string_view file_name) const {
@@ -151,7 +152,8 @@ std::string source_manager::get_file_path(std::string_view file_name) const {
151152
return std::filesystem::absolute(file_name).string();
152153
}
153154

154-
std::optional<source> source_manager::get_file(std::string_view file_name) {
155+
std::optional<source_view> source_manager::get_file(
156+
std::string_view file_name) {
155157
if (auto source = file_source_map_.find(file_name);
156158
source != file_source_map_.end()) {
157159
return source->second;
@@ -182,7 +184,7 @@ std::optional<source> source_manager::get_file(std::string_view file_name) {
182184
return source;
183185
}
184186

185-
source source_manager::add_virtual_file(
187+
source_view source_manager::add_virtual_file(
186188
std::string_view file_name, std::string_view src) {
187189
if (file_source_map_.find(file_name) != file_source_map_.end()) {
188190
throw std::runtime_error(fmt::format("file already added: {}", file_name));

third-party/thrift/src/thrift/compiler/source_location.h

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,11 +98,14 @@ class resolved_location {
9898
};
9999

100100
// A view of a source owned by `source_manager`.
101-
struct source {
101+
struct source_view final {
102102
source_location start; // The source start location.
103103
std::string_view text; // The source text including a terminating '\0'.
104104
};
105105

106+
// DO_BEFORE(aristidis,20251001): Remove alias when all references are updated.
107+
using source = source_view;
108+
106109
// A class that abstracts the reading of files from the file system. The
107110
// backend could read from a real file system, or be an in-memory
108111
// representation itself.
@@ -132,11 +135,16 @@ class source_manager {
132135
std::vector<char> text;
133136
std::vector<uint_least32_t> line_offsets;
134137
};
138+
135139
// This is a deque to make sure that file_name is not reallocated when
136140
// sources_ grows.
137141
std::deque<source_info> sources_;
138142

139-
std::map<std::string, source, std::less<>> file_source_map_;
143+
/**
144+
* Maps the file_name of a source file owned by this source_manager (i.e., in
145+
* `sources_`) to its `source_view`.
146+
*/
147+
std::map<std::string, source_view, std::less<>> file_source_map_;
140148

141149
// Maps from filepaths present in the AST to filepaths on disk.
142150
std::map<std::string, std::string, std::less<>> found_includes_;
@@ -149,7 +157,18 @@ class source_manager {
149157

150158
friend class resolved_location;
151159

152-
source add_source(std::string_view file_name, std::vector<char> text);
160+
/**
161+
* Adds the given contents of a source file (with the given name) to this
162+
* manager (i.e., to `sources_`).
163+
*
164+
* @param file_name unique name of the file whose contents are being added.
165+
* Behavior is undefined if this name is not unique.
166+
*
167+
* @param text file contents. Last element MUST be the NUL character.
168+
*
169+
* @return a view into the (newly added) content.
170+
*/
171+
source_view add_source(std::string_view file_name, std::vector<char> text);
153172

154173
public:
155174
// Creates a source_manager with the default (filesystem-based) backend.
@@ -165,18 +184,30 @@ class source_manager {
165184
source_manager& operator=(source_manager&&) noexcept = default;
166185
~source_manager() noexcept = default;
167186

168-
// Loads a file and returns a source object representing its content.
169-
// The file can be a real file (provided by the backend), or a virtual one
170-
// previously registered with add_virtual_file.
171-
//
172-
// Returns an empty optional if opening or reading the file fails. Makes use
173-
// of the result of previous calls to find_include_file.
174-
std::optional<source> get_file(std::string_view file_name);
187+
/**
188+
* Returns a view into the contents of a file with the given name.
189+
*
190+
* The file can be a real file (provided by the backend), or a virtual one
191+
* previously registered with `add_virtual_file()`.
192+
*
193+
* The underlying contents are owned by this manager.
194+
*
195+
* Returns an empty optional if opening or reading the file fails. Makes use
196+
* of the result of previous calls to `find_include_file()`.
197+
*/
198+
std::optional<source_view> get_file(std::string_view file_name);
175199

176200
std::string get_file_path(std::string_view file_name) const;
177201

178-
// Adds a virtual file with the specified name and content.
179-
source add_virtual_file(std::string_view file_name, std::string_view src);
202+
/**
203+
* Adds a virtual file with the specified name and content.
204+
*
205+
* @return a view into the (newly added) content.
206+
*
207+
* @throws if a file with the given name already exists.
208+
*/
209+
source_view add_virtual_file(
210+
std::string_view file_name, std::string_view src);
180211

181212
// Returns the start location of a source containing the specified location.
182213
// It is a member function in case we add clang-like compression of locations.

third-party/thrift/src/thrift/compiler/test/diagnostic_test.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class DiagnosticsEngineTest : public ::testing::Test {
3737
source_manager source_mgr;
3838
diagnostic_results results;
3939
diagnostics_engine diags;
40-
source src;
40+
source_view src;
4141

4242
DiagnosticsEngineTest()
4343
: diags(source_mgr, results),

third-party/thrift/src/thrift/compiler/whisker/source_location.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ namespace whisker {
2323
using source_location = apache::thrift::compiler::source_location;
2424
using source_range = apache::thrift::compiler::source_range;
2525
using resolved_location = apache::thrift::compiler::resolved_location;
26-
using source = apache::thrift::compiler::source;
26+
using source = apache::thrift::compiler::source_view;
2727
using source_manager = apache::thrift::compiler::source_manager;
2828
} // namespace whisker

0 commit comments

Comments
 (0)