Skip to content

Removing StringReader::retokenize #76046

Closed
@matklad

Description

@matklad

StringReader contains a retokenize function, which allows to to, well, retokenize existing span:

pub fn retokenize(sess: &'a ParseSess, mut span: Span) -> Self {
let begin = sess.source_map().lookup_byte_offset(span.lo());
let end = sess.source_map().lookup_byte_offset(span.hi());
// Make the range zero-length if the span is invalid.
if begin.sf.start_pos != end.sf.start_pos {
span = span.shrink_to_lo();
}
let mut sr = StringReader::new(sess, begin.sf, None);
// Seek the lexer to the right byte range.
sr.end_src_index = sr.src_index(span.hi());
sr
}

There's only caller of this function:

pub fn retokenise_span(&self, span: Span) -> StringReader<'a> {
lexer::StringReader::retokenize(&self.sess.parse_sess, span)
}
pub fn sub_span_of_token(&self, span: Span, tok: TokenKind) -> Option<Span> {
let mut toks = self.retokenise_span(span);
loop {
let next = toks.next_token();
if next == token::Eof {
return None;
}
if next == tok {
return Some(next.span);
}
}
}

And there's only one caller of that caller:

hir::ItemKind::Use(path, hir::UseKind::Glob) => {
// Make a comma-separated list of names of imported modules.
let def_id = self.tcx.hir().local_def_id(item.hir_id);
let names = self.tcx.names_imported_by_glob_use(def_id);
let names: Vec<_> = names.iter().map(|n| n.to_string()).collect();
// Otherwise it's a span with wrong macro expansion info, which
// we don't want to track anyway, since it's probably macro-internal `use`
if let Some(sub_span) =
self.span.sub_span_of_token(item.span, token::BinOp(token::Star))
{

In other words, the only reason why we need StringReader::retokenize is to get the span of * from hir::Use which happens to be a glob import.

(
rls needs to know the span of * to implement "replace use foo::* with explicit list of imported items" code action. This was the first demoed code action and is still the only one that is implemented
)

I'd like to remove the retokenize function, as it is one of the few remaining things that requires StringReader to be public, and as it is very much a hack anyway.

Additionally, I believe I've broken this function accidently a while ago: 395ee0b#diff-d06ad31c547d2d94c8b8770006977767L69. Rather than retokenizing a span, it retokenized file from the begining until the end of the span (whcih, as uses are usually on top, worked correctly most of the times by accident).

What would be the best way to fix this?

  • Add * span to hir::Use -- this seem to be the "proper" solution here
  • Move hack's implementation to where it belongs: in librutc_save_analysis, just fetch the text of the file and look for *, using rustc_lexer or maybe just .rfind('*')
  • Remove the deglog functionlity from RLS? I don't like this option as it handicaps the RLS before we formally decided to deprecated it. But I don't completely disregard it either, as, as a whole, it seems to be a full-stack one off hack penetrating every layer of abstraction.

@petrochenkov @Xanewok WDYT?

Metadata

Metadata

Assignees

Labels

A-parserArea: The lexing & parsing of Rust source code to an ASTA-save-analysisArea: saving results of analyses such as inference and borrowck results to a file.C-cleanupCategory: PRs that clean code up or issues documenting cleanup.

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions