github-mirror/fish-shell
2
0
Fork 0
mirror of https://github.com/fish-shell/fish-shell.git synced 2024-11-23 06:05:39 +08:00
Code Issues Actions Packages Projects Releases Wiki Activity

Switch filenames from intern'd strings to shared_ptr

Browse Source 
We store filenames in function definitions to indicate where the
function comes from. Previously these were intern'd strings. Switch them
to a shared_ptr<wcstring>, intending to remove intern'd strings.
This commit is contained in:
ridiculousfish 2022-08-13 12:15:23 -07:00
parent 20a3599b10
commit 082f074bb1
10 changed files with 53 additions and 54 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
src/builtins/functions.cpp
View File

@ -137,15 +137,15 @@ static int parse_cmd_opts(functions_cmd_opts_t &opts, int *optind, //!OCLINT(hi
static int report_function_metadata(const wcstring &funcname, bool verbose, io_streams_t &streams,
parser_t &parser, bool metadata_as_comments) {
const wchar_t *path = L"n/a";
wcstring path = L"n/a";
const wchar_t *autoloaded = L"n/a";
const wchar_t *shadows_scope = L"n/a";
wcstring description = L"n/a";
int line_number = 0;
if (auto props = function_get_props_autoload(funcname, parser)) {
path = props->definition_file;
if (path) {
if (props->definition_file) {
path = *props->definition_file;
autoloaded = props->is_autoload ? L"autoloaded" : L"not-autoloaded";
line_number = props->definition_lineno();
} else {
@ -159,12 +159,12 @@ static int report_function_metadata(const wcstring &funcname, bool verbose, io_s
// "stdin" means it was defined interactively, "-" means it was defined via `source`.
// Neither is useful information.
wcstring comment;
if (!std::wcscmp(path, L"stdin")) {
if (path == L"stdin") {
append_format(comment, L"# Defined interactively\n");
} else if (!std::wcscmp(path, L"-")) {
} else if (path == L"-") {
append_format(comment, L"# Defined via `source`\n");
} else {
append_format(comment, L"# Defined in %ls @ line %d\n", path, line_number);
append_format(comment, L"# Defined in %ls @ line %d\n", path.c_str(), line_number);
}
if (!streams.out_is_redirected && isatty(STDOUT_FILENO)) {
@ -175,7 +175,7 @@ static int report_function_metadata(const wcstring &funcname, bool verbose, io_s
streams.out.append(comment);
}
} else {
streams.out.append_format(L"%ls\n", path);
streams.out.append_format(L"%ls\n", path.c_str());
if (verbose) {
streams.out.append_format(L"%ls\n", autoloaded);
streams.out.append_format(L"%d\n", line_number);

19
src/builtins/source.cpp
View File

@ -43,7 +43,7 @@ maybe_t<int> builtin_source(parser_t &parser, io_streams_t &streams, const wchar
int fd = -1;
struct stat buf;
const wchar_t *fn, *fn_intern;
filename_ref_t func_filename{};
if (argc == optind || std::wcscmp(argv[optind], L"-") == 0) {
if (streams.stdin_fd < 0) {
@ -55,8 +55,7 @@ maybe_t<int> builtin_source(parser_t &parser, io_streams_t &streams, const wchar
// Don't implicitly read from the terminal.
return STATUS_CMD_ERROR;
}
fn = L"-";
fn_intern = fn;
func_filename = std::make_shared<wcstring>(L"-");
fd = streams.stdin_fd;
} else {
opened_fd = autoclose_fd_t(wopen_cloexec(argv[optind], O_RDONLY));
@ -83,13 +82,14 @@ maybe_t<int> builtin_source(parser_t &parser, io_streams_t &streams, const wchar
return STATUS_CMD_ERROR;
}
fn_intern = intern(argv[optind]);
func_filename = std::make_shared<wcstring>(argv[optind]);
}
assert(fd >= 0 && "Should have a valid fd");
assert(func_filename && "Should have valid function filename");
const block_t *sb = parser.push_block(block_t::source_block(fn_intern));
const block_t *sb = parser.push_block(block_t::source_block(func_filename));
auto &ld = parser.libdata();
scoped_push<const wchar_t *> filename_push{&ld.current_filename, fn_intern};
scoped_push<filename_ref_t> filename_push{&ld.current_filename, func_filename};
// Construct argv from our null-terminated list.
// This is slightly subtle. If this is a bare `source` with no args then `argv + optind` already
@ -106,10 +106,9 @@ maybe_t<int> builtin_source(parser_t &parser, io_streams_t &streams, const wchar
parser.pop_block(sb);
if (retval != STATUS_CMD_OK) {
wcstring esc = escape_string(fn_intern);
streams.err.append_format(
_(L"%ls: Error while reading file '%ls'\n"), cmd,
escape_string(fn_intern) == intern_static(L"-") ? L"<stdin>" : esc.c_str());
wcstring esc = escape_string(*func_filename);
streams.err.append_format(_(L"%ls: Error while reading file '%ls'\n"), cmd,
esc == L"-" ? L"<stdin>" : esc.c_str());
} else {
retval = parser.get_last_status();
}

2
src/builtins/status.cpp
View File

@ -370,7 +370,7 @@ maybe_t<int> builtin_status(parser_t &parser, io_streams_t &streams, const wchar
case STATUS_FILENAME: {
CHECK_FOR_UNEXPECTED_STATUS_ARGS(opts.status_cmd)
auto res = parser.current_filename();
wcstring fn = res ? res : L"";
wcstring fn = res ? *res : L"";
if (!fn.empty() && opts.status_cmd == STATUS_DIRNAME) {
fn = wdirname(fn);
} else if (!fn.empty() && opts.status_cmd == STATUS_BASENAME) {

8
src/builtins/type.cpp
View File

@ -134,7 +134,7 @@ maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t
auto path = func->definition_file;
if (opts.path) {
if (path) {
streams.out.append(path);
streams.out.append(*path);
streams.out.append(L"\n");
}
} else if (!opts.short_output) {
@ -146,8 +146,8 @@ maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t
if (path) {
int line_number = func->definition_lineno();
wcstring comment;
if (std::wcscmp(path, L"-") != 0) {
append_format(comment, L"# Defined in %ls @ line %d\n", path,
if (*path != L"-") {
append_format(comment, L"# Defined in %ls @ line %d\n", path->c_str(),
line_number);
} else {
append_format(comment, L"# Defined via `source`\n");
@ -169,7 +169,7 @@ maybe_t<int> builtin_type(parser_t &parser, io_streams_t &streams, const wchar_t
streams.out.append_format(_(L"%ls is a function"), name);
auto path = func->definition_file;
if (path) {
streams.out.append_format(_(L" (defined in %ls)"), path);
streams.out.append_format(_(L" (defined in %ls)"), path->c_str());
}
streams.out.append(L"\n");
}

3
src/common.h
View File

@ -337,6 +337,9 @@ void format_ullong_safe(wchar_t buff[64], unsigned long long val);
/// "Narrows" a wide character string. This just grabs any ASCII characters and trunactes.
void narrow_string_safe(char buff[64], const wchar_t *s);
/// Stored in blocks to reference the file which created the block.
using filename_ref_t = std::shared_ptr<const wcstring>;
using scoped_lock = std::lock_guard<std::mutex>;
// An object wrapping a scoped lock and a value

4
src/exec.cpp
View File

@ -522,7 +522,7 @@ static launch_result_t exec_external_command(parser_t &parser, const std::shared
std::string actual_cmd_str = wcs2string(p->actual_cmd);
const char *actual_cmd = actual_cmd_str.c_str();
const wchar_t *file = parser.libdata().current_filename;
filename_ref_t file = parser.libdata().current_filename;
#if FISH_USE_POSIX_SPAWN
// Prefer to use posix_spawn, since it's faster on some systems like OS X.
@ -544,7 +544,7 @@ static launch_result_t exec_external_command(parser_t &parser, const std::shared
// usleep(10000);
FLOGF(exec_fork, L"Fork #%d, pid %d: spawn external command '%s' from '%ls'",
int(s_fork_count), *pid, actual_cmd, file ? file : L"<no file>");
int(s_fork_count), *pid, actual_cmd, file ? file->c_str() : L"<no file>");
// these are all things do_fork() takes care of normally (for forked processes):
p->pid = *pid;

8
src/fish.cpp
View File

@ -585,13 +585,11 @@ int main(int argc, char **argv) {
parser.vars().set(L"argv", ENV_DEFAULT, std::move(list));
auto &ld = parser.libdata();
wcstring rel_filename = str2wcstring(file);
scoped_push<const wchar_t *> filename_push{&ld.current_filename,
intern(rel_filename.c_str())};
filename_ref_t rel_filename = std::make_shared<wcstring>(str2wcstring(file));
scoped_push<filename_ref_t> filename_push{&ld.current_filename, rel_filename};
res = reader_read(parser, fd.fd(), {});
if (res) {
FLOGF(warning, _(L"Error while reading file %ls\n"),
ld.current_filename ? ld.current_filename : _(L"Standard input"));
FLOGF(warning, _(L"Error while reading file %ls\n"), rel_filename->c_str());
}
}
}

5
src/function.h
View File

@ -44,9 +44,8 @@ struct function_properties_t {
/// Whether the function was autoloaded.
bool is_autoload{false};
/// The file from which the function was created (intern'd string), or nullptr if not from a
/// file.
const wchar_t *definition_file{};
/// The file from which the function was created, or nullptr if not from a file.
filename_ref_t definition_file{};
/// \return the description, localized via _.
const wchar_t *localized_description() const;

31
src/parser.cpp
View File

@ -241,9 +241,9 @@ static void append_block_description_to_stack_trace(const parser_t &parser, cons
break;
}
case block_type_t::source: {
const wchar_t *source_dest = b.sourced_file;
const filename_ref_t &source_dest = b.sourced_file;
append_format(trace, _(L"from sourcing file %ls\n"),
user_presentable_path(source_dest, parser.vars()).c_str());
user_presentable_path(*source_dest, parser.vars()).c_str());
print_call_site = true;
break;
}
@ -268,10 +268,10 @@ static void append_block_description_to_stack_trace(const parser_t &parser, cons
if (print_call_site) {
// Print where the function is called.
const wchar_t *file = b.src_filename;
const auto &file = b.src_filename;
if (file) {
append_format(trace, _(L"\tcalled on line %d of file %ls\n"), b.src_lineno,
user_presentable_path(file, parser.vars()).c_str());
user_presentable_path(*file, parser.vars()).c_str());
} else if (parser.libdata().within_fish_init) {
append_format(trace, _(L"\tcalled during startup\n"));
}
@ -377,7 +377,7 @@ int parser_t::get_lineno() const {
return lineno;
}
const wchar_t *parser_t::current_filename() const {
filename_ref_t parser_t::current_filename() const {
for (const auto &b : block_list) {
if (b.is_function_call()) {
auto props = function_get_props(b.function_name);
@ -415,7 +415,7 @@ wcstring parser_t::current_line() {
}
const int lineno = this->get_lineno();
const wchar_t *file = this->current_filename();
filename_ref_t file = this->current_filename();
wcstring prefix;
@ -423,7 +423,7 @@ wcstring parser_t::current_line() {
if (!is_interactive() || is_function()) {
if (file) {
append_format(prefix, _(L"%ls (line %d): "),
user_presentable_path(file, vars()).c_str(), lineno);
user_presentable_path(*file, vars()).c_str(), lineno);
} else if (libdata().within_fish_init) {
append_format(prefix, L"%ls (line %d): ", _(L"Startup"), lineno);
} else {
@ -637,14 +637,15 @@ void parser_t::get_backtrace(const wcstring &src, const parse_error_list_t &erro
}
wcstring prefix;
const wchar_t *filename = this->current_filename();
filename_ref_t filename = this->current_filename();
if (filename) {
if (which_line > 0) {
prefix = format_string(_(L"%ls (line %lu): "),
user_presentable_path(filename, vars()).c_str(), which_line);
prefix =
format_string(_(L"%ls (line %lu): "),
user_presentable_path(*filename, vars()).c_str(), which_line);
} else {
prefix =
format_string(_(L"%ls: "), user_presentable_path(filename, vars()).c_str());
format_string(_(L"%ls: "), user_presentable_path(*filename, vars()).c_str());
}
} else {
prefix = L"fish: ";
@ -724,8 +725,8 @@ wcstring block_t::description() const {
if (this->src_lineno >= 0) {
append_format(result, L" (line %d)", this->src_lineno);
}
if (this->src_filename != nullptr) {
append_format(result, L" (file %ls)", this->src_filename);
if (this->src_filename) {
append_format(result, L" (file %ls)", this->src_filename->c_str());
}
return result;
}
@ -747,9 +748,9 @@ block_t block_t::function_block(wcstring name, wcstring_list_t args, bool shadow
return b;
}
block_t block_t::source_block(const wchar_t *src) {
block_t block_t::source_block(filename_ref_t src) {
block_t b{block_type_t::source};
b.sourced_file = src;
b.sourced_file = std::move(src);
return b;
}

13
src/parser.h
View File

@ -66,8 +66,8 @@ class block_t {
const block_type_t block_type;
public:
/// Name of file that created this block. This string is intern'd.
const wchar_t *src_filename{nullptr};
/// Name of file that created this block.
filename_ref_t src_filename{};
/// Line number where this block was created.
int src_lineno{0};
/// Whether we should pop the environment variable stack when we're popped off of the block
@ -83,7 +83,7 @@ class block_t {
// If this is a source block, the source'd file, interned.
// Otherwise nothing.
const wchar_t *sourced_file{};
filename_ref_t sourced_file{};
// If this is an event block, the event. Otherwise ignored.
maybe_t<event_t> event;
@ -103,7 +103,7 @@ class block_t {
static block_t if_block();
static block_t event_block(event_t evt);
static block_t function_block(wcstring name, wcstring_list_t args, bool shadows);
static block_t source_block(const wchar_t *src);
static block_t source_block(filename_ref_t src);
static block_t for_block();
static block_t while_block();
static block_t switch_block();
@ -201,8 +201,7 @@ struct library_data_t {
size_t read_limit{0};
/// The current filename we are evaluating, either from builtin source or on the command line.
/// This is an intern'd string.
const wchar_t *current_filename{};
filename_ref_t current_filename{};
/// List of events that have been sent but have not yet been delivered because they are blocked.
std::vector<shared_ptr<const event_t>> blocked_events{};
@ -431,7 +430,7 @@ class parser_t : public std::enable_shared_from_this<parser_t> {
/// Returns the file currently evaluated by the parser. This can be different than
/// reader_current_filename, e.g. if we are evaluating a function defined in a different file
/// than the one curently read.
const wchar_t *current_filename() const;
filename_ref_t current_filename() const;
/// Return if we are interactive, which means we are executing a command that the user typed in
/// (and not, say, a prompt).