Skip to content

Commit

Permalink
4/n Pass file contents to erlang_service with parse request
Browse files Browse the repository at this point in the history
Summary:
When requesting the erlang service to parse a file, pass the file contents with the request.  We sent it directly as bytes on the wire, for simplicity and performance.

Unfortunately `elp_epp` processing needs a `pid()` for the open file, so we provide a gen_server to do this.

I suspect the performance can be improved.

Note: It seems that the one glean test is now unexpectedly getting types from the eqwalizer setup, that it did not before.

Reviewed By: michalmuskala

Differential Revision: D60764433

fbshipit-source-id: ab2c1d830387705b167f52bd80af4a8be4bd7046
  • Loading branch information
alanz authored and facebook-github-bot committed Sep 5, 2024
1 parent 536c67f commit 4873369
Show file tree
Hide file tree
Showing 9 changed files with 268 additions and 26 deletions.
4 changes: 4 additions & 0 deletions crates/elp/src/bin/glean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1931,6 +1931,10 @@ mod tests {
doc_foo(Bar) -> [Bar].
%% ^^^^^^^^^^^^^^^^^^^^^^ func/doc_foo/1/not_deprecated/exported
%% ^^^^^^^^^^^^^^^^^^^^^^ doc/-spec doc_foo(integer()) -> [integer()].
%% ^^^ var/Bar :: number()
%% ^^^ doc/Bar :: number()
%% ^^^ var/Bar :: number()
%% ^^^ doc/Bar :: number()
main(A) -> A.
%% ^^^^^^^^^^^^^ func/main/1/not_deprecated/not_exported
Expand Down
15 changes: 15 additions & 0 deletions crates/erlang_service/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ pub struct ParseRequest {
pub file_id: FileId,
pub path: PathBuf,
pub format: Format,
pub file_text: Arc<str>,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -808,6 +809,11 @@ impl ParseRequest {
eetf::List::from(override_options).into(),
]);
let mut buf = Vec::new();
// We first pass a length-preceded text buffer, then the options.
buf.write_u32::<BigEndian>(self.file_text.len() as u32)
.expect("buf write failed");
buf.write_all(self.file_text.as_bytes())
.expect("buf write failed");
eetf::Term::from(list).encode(&mut buf).unwrap();
buf
}
Expand Down Expand Up @@ -866,6 +872,7 @@ fn path_into_list(path: PathBuf) -> eetf::ByteList {

#[cfg(test)]
mod tests {
use std::fs;
use std::str;

use expect_test::expect_file;
Expand Down Expand Up @@ -1025,11 +1032,15 @@ mod tests {
lazy_static! {
static ref CONN: Connection = Connection::start().unwrap();
}
let file_text = Arc::from(
fs::read_to_string(path.clone()).expect("Should have been able to read the file"),
);
let request = ParseRequest {
options: vec![],
override_options,
file_id: FileId::from_raw(0),
path,
file_text,
format: Format::Text,
};
let response = CONN.request_parse(request, || (), &|_, _, _| None);
Expand All @@ -1050,11 +1061,15 @@ mod tests {
lazy_static! {
static ref CONN: Connection = Connection::start().unwrap();
}
let file_text = Arc::from(
fs::read_to_string(path.clone()).expect("Should have been able to read the file"),
);
let request = ParseRequest {
options: vec![],
override_options,
file_id: FileId::from_raw(0),
path,
file_text,
format: Format::Text,
};
let response = CONN.request_parse(request, || (), &|_, _, _| None);
Expand Down
18 changes: 18 additions & 0 deletions crates/ide/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2360,4 +2360,22 @@ baz(1)->4.
"#,
);
}

#[test]
fn erlang_service_file_open_encoding() {
check_diagnostics(
// Note: \~ gets replaced by ~ in the fixture parsing
r#"
//- erlang_service
//- /src/main.erl
-module(main).
-export([foo/0]).
foo() ->
\~"\"\\µA\"" = \~/"\\µA"/
X = 3.
%% ^ error: syntax error before: X
"#,
);
}
}
3 changes: 3 additions & 0 deletions crates/ide_db/src/erl_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use elp_base_db::salsa::Database;
use elp_base_db::AbsPath;
use elp_base_db::AbsPathBuf;
use elp_base_db::FileId;
use elp_base_db::FileLoader;
use elp_base_db::IncludeCtx;
use elp_base_db::ProjectId;
use elp_base_db::SourceDatabase;
Expand Down Expand Up @@ -78,12 +79,14 @@ impl AstLoader for crate::RootDatabase {
override_options.push(option.clone());
}
let path: PathBuf = path.to_path_buf().into();
let file_text = self.file_text(file_id);
let req = ParseRequest {
options,
override_options,
file_id,
path: path.clone(),
format,
file_text,
};
let erlang_service = self.erlang_service_for(project_id);
let r = erlang_service.request_parse(
Expand Down
21 changes: 13 additions & 8 deletions erlang_service/src/elp_epp.erl
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

-export([open/3, open/4, open/5, close/1, format_error/1]).
-export([scan_erl_form/1, parse_erl_form/1, macro_defs/1]).
-export([scan_file/1, scan_file/4, parse_file/1, parse_file/4, parse_file/5]).
-export([scan_file/1, scan_file/4, parse_file/1, parse_file/5, parse_file/6]).
-export([
default_encoding/0,
encoding_to_string/1,
Expand Down Expand Up @@ -338,12 +338,13 @@ scan_file(Epp) ->
[{eof, {Offset, Offset}}]
end.

-spec parse_file(Id, FileName, FileId, IncludePath, PredefMacros) ->
-spec parse_file(Id, FileName, FileId, FileText, IncludePath, PredefMacros) ->
{'ok', [Form]} | {'ok', [Form], Extra} | {error, OpenError}
when
Id :: erlang_service_server:id(),
FileName :: file:name(),
FileId :: erlang_service_server:id(),
FileText :: erlang_service_server:file_text(),
IncludePath :: [DirectoryName :: file:name()],
PredefMacros :: macros(),
Form :: elp_parse:abstract_form() | {'error', ErrorInfo} | {'eof', Location},
Expand All @@ -352,15 +353,16 @@ when
ErrorInfo :: elp_scan:error_info() | elp_parse:error_info(),
OpenError :: term().

parse_file(Id, Ifile, FileId, Path, Predefs) ->
parse_file(Id, Ifile, FileId, [{includes, Path}, {macros, Predefs}]).
parse_file(Id, Ifile, FileId, FileText, Path, Predefs) ->
parse_file(Id, Ifile, FileId, FileText, [{includes, Path}, {macros, Predefs}]).

-spec parse_file(Id, FileName, FileId, Options) ->
-spec parse_file(Id, FileName, FileId, FileText, Options) ->
{'ok', [Form]} | {'ok', [Form], Extra} | {error, OpenError}
when
Id :: erlang_service_server:id(),
FileName :: file:name(),
FileId :: erlang_service_server:id(),
FileText :: erlang_service_server:file_text(),
Options :: [
{'includes', IncludePath :: [DirectoryName :: file:name()]}
| {'source_name', SourceName :: file:name()}
Expand All @@ -377,8 +379,9 @@ when
Extra :: [{'encoding', source_encoding() | 'none'}],
OpenError :: term().

parse_file(Id, Ifile, FileId, Options) ->
case open(Id, FileId, [{name, Ifile} | Options]) of
parse_file(Id, Ifile, FileId, FileText, Options) ->
Pid = elp_io_string:new(FileText),
case open(Id, FileId, [{name, Ifile}, {fd, Pid} | Options]) of
{ok, Epp} ->
Forms = parse_file(Epp),
close(Epp),
Expand Down Expand Up @@ -694,7 +697,9 @@ server(Pid, Id, Name, FileId, Options) ->
epp_reply(Pid, {error, E})
end;
Fd ->
init_server(Pid, Name, Options, St#epp{file = Fd, file_id = FileId, pre_opened = true})
%% We do not flag the file as pre_opened as we want to be
%% able to close the in-memory instance when done
init_server(Pid, Name, Options, St#epp{file = Fd, file_id = FileId })
end.

init_server(Pid, FileName, Options0, St0) ->
Expand Down
14 changes: 7 additions & 7 deletions erlang_service/src/elp_escript.erl
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

-module(elp_escript).

-export([extract/3]).
-export([extract/4]).

-record(state, {
file :: file:filename(),
Expand All @@ -44,9 +44,10 @@
-record(sections, {shebang :: shebang() | 'undefined'}).
-type sections() :: #sections{}.

-spec extract(erlang_service_server:id(), file:filename(), erlang_service_server:file_id()) -> any().
extract(Id, File, FileId) ->
{HeaderSz, Fd, _Sections} = parse_header(File),
-spec extract(erlang_service_server:id(), file:filename(), erlang_service_server:file_id(), erlang_service_server:file_text()) -> any().
extract(Id, File, FileId, FileText) ->
Pid = elp_io_string:new(FileText),
{HeaderSz, Fd, _Sections} = parse_header(Pid),
Forms = do_parse_file(Id, File, FileId, Fd, HeaderSz),
ok = file:close(Fd),
Forms.
Expand All @@ -67,9 +68,8 @@ initial_state(File, FileId) ->
}.

%% Skip header and make a heuristic guess about the script type
-spec parse_header(file:filename()) -> {any(), any(), sections()}.
parse_header(File) ->
{ok, Fd} = file:open(File, [read]),
-spec parse_header(file:io_device()) -> {any(), any(), sections()}.
parse_header(Fd) ->

%% Skip shebang on first line
Line1 = get_line(Fd),
Expand Down
Loading

0 comments on commit 4873369

Please sign in to comment.