Skip to content

Commit

Permalink
Re-implement rust-in-macro for performance
Browse files Browse the repository at this point in the history 
rust-in-macro could cause significant performance problems resulting
in a very choppy user experience. Reimplement rust-in macro in a
somewhat simpler manner and in way which allows both allows
restriction to parts of the buffer and caching of buffer analysis.
Optimize rust-syntax-propertize to use this caching mechanism.

Fixes #208
Fixes #288
  • Loading branch information
@phillord @mookid
phillord authored and mookid committed May 13, 2020
1 parent eca55c0 commit bfe4056
Show file tree
Hide file tree
Showing 2 changed files with 225 additions and 40 deletions.
110 changes: 108 additions & 2 deletions rust-mode-tests.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2139,8 +2139,8 @@ fn main() {
(should (equal (scan-sexps (+ 1 close-pos) -1) open-pos))))
(dolist (nonpar-pos nonparen-positions)
(let ((nonpar-syntax-class (syntax-class (syntax-after nonpar-pos))))
(should (not (equal 4 nonpar-syntax-class)))
(should (not (equal 5 nonpar-syntax-class)))))))
(should-not (equal 4 nonpar-syntax-class))
(should-not (equal 5 nonpar-syntax-class))))))

(ert-deftest rust-test-unmatched-single-quote-in-comment-paren-matching ()
;; This was a bug from the char quote handling that affected the paren
Expand Down Expand Up @@ -2923,6 +2923,112 @@ macro_c!{
125 ;; macro_d >
)))

(ert-deftest rust-test-paren-matching-no-angle-brackets-in-macro-rules ()
(rust-test-matching-parens
"
fn foo<A>(a:A) {
macro_rules! foo ( foo::<ignore the bracets> );
macro_rules! bar [ foo as Option<B> ];
}
macro_c!{
struct Boo<D> {}
}"
'((8 10))
;; Inside macros, it should not find any angle brackets, even if it normally
;; would
'(47 ;; foo <
62 ;; foo >
107 ;; bar <
109 ;; bar >
141 ;; macro_c <
143 ;; macro_c >
)))

(ert-deftest rust-test-in-macro-do-not-fail-on-unbalance ()
(should
;; We don't care about the results here, so long as they do not error
(with-temp-buffer
(insert
"fn foo<A>(a:A) {
macro_c!{
struct Boo<D> {}
")
(rust-mode)
(goto-char (point-max))
(syntax-ppss))))


(ert-deftest rust-test-in-macro-no-caching ()
(should-not
(with-temp-buffer
(insert
"fn foo<A>(a:A) {
macro_c!{
struct Boo<D> {}
")
(rust-mode)
(search-backward "macro")
;; do not use the cache
(let ((rust-macro-scopes nil))
(rust-in-macro)))))

(ert-deftest rust-test-in-macro-fake-cache ()
(should
(with-temp-buffer
(insert
"fn foo<A>(a:A) {
macro_c!{
struct Boo<D> {}
")
(rust-mode)
(search-backward "macro")
;; make the cache lie to make the whole buffer in scope
;; we need to be at paren level 1 for this to work
(let ((rust-macro-scopes `((,(point-min) ,(point-max)))))
(rust-in-macro)))))

(ert-deftest rust-test-in-macro-broken-cache ()
(should-error
(with-temp-buffer
(insert
"fn foo<A>(a:A) {
macro_c!{
struct Boo<D> {}
")
(rust-mode)
(search-backward "Boo")
;; do we use the cache at all
(let ((rust-macro-scopes '(I should break)))
(rust-in-macro)))))

(ert-deftest rust-test-in-macro-nested ()
(should
(equal
(with-temp-buffer
(insert
"macro_rules! outer {
() => { vec![] };
}")
(rust-mode)
(rust-macro-scope (point-min) (point-max)))
'((38 40) (20 45)))))

(ert-deftest rust-test-in-macro-not-with-space ()
(should
(equal
(with-temp-buffer
(insert
"fn foo<T>() {
if !(mem::size_of::<T>() > 8) {
bar()
}
}")
(rust-mode)
(rust-macro-scope (point-min) (point-max)))
'empty)))


(ert-deftest rust-test-paren-matching-type-with-module-name ()
(rust-test-matching-parens
"
Expand Down
155 changes: 117 additions & 38 deletions rust-mode.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,12 @@ symbols."
(looking-back rust-re-ident beg-of-symbol)))

(defun rust-looking-back-macro ()
"Non-nil if looking back at an ident followed by a !"
"Non-nil if looking back at an ident followed by a !
This is stricter than rust syntax which allows a space between
the ident and the ! symbol. If this space is allowed, then we
would also need a keyword check to avoid `if !(condition)` being
seen as a macro."
(if (> (- (point) (point-min)) 1)
(save-excursion
(backward-char)
Expand Down Expand Up @@ -260,18 +265,90 @@ to the function arguments. When nil, `->' will be indented one level."
;; Rewind until the point no longer moves
(setq continue (/= starting (point)))))))

(defun rust-in-macro ()
(defvar-local rust-macro-scopes nil
"Cache for the scopes calculated by `rust-macro-scope'.
This variable can be `let' bound directly or indirectly around
`rust-macro-scope' as an optimization but should not be otherwise
set.")

(defun rust-macro-scope (start end)
"Return the scope of macros in the buffer.
The return value is a list of (START END) positions in the
buffer.
If set START and END are optimizations which limit the return
value to scopes which are approximately with this range."
(save-excursion
(when (> (rust-paren-level) 0)
(backward-up-list)
(rust-rewind-irrelevant)
(or (rust-looking-back-macro)
(and (rust-looking-back-ident)
(save-excursion
(backward-sexp)
(rust-rewind-irrelevant)
(rust-looking-back-str "macro_rules!")))
(rust-in-macro)))))
;; need to special case macro_rules which has unique syntax
(let ((scope nil)
(start (or start (point-min)))
(end (or end (point-max))))
(goto-char start)
;; if there is a start move back to the previous top level,
;; as any macros before that must have closed by this time.
(let ((top (syntax-ppss-toplevel-pos (syntax-ppss))))
(when top
(goto-char top)))
(while
(and
;; The movement below may have moved us passed end, in
;; which case search-forward will error
(< (point) end)
(search-forward "!" end t))
(let ((pt (point)))
(cond
;; in a string or comment is boring, move straight on
((rust-in-str-or-cmnt))
;; in a normal macro,
((and (skip-chars-forward " \t\n\r")
(memq (char-after)
'(?\[ ?\( ?\{))
;; Check that we have a macro declaration after.
(rust-looking-back-macro))
(let ((start (point)))
(ignore-errors (forward-list))
(setq scope (cons (list start (point)) scope))))
;; macro_rules, why, why, why did you not use macro syntax??
((save-excursion
;; yuck -- last test moves point, even if it fails
(goto-char (- pt 1))
(skip-chars-backward " \t\n\r")
(rust-looking-back-str "macro_rules"))
(save-excursion
(when (re-search-forward "[[({]" nil t)
(backward-char)
(let ((start (point)))
(ignore-errors (forward-list))
(setq scope (cons (list start (point)) scope)))))))))
;; Return 'empty rather than nil, to indicate a buffer with no
;; macros at all.
(or scope 'empty))))

(defun rust-in-macro (&optional start end)
"Return non-nil when point is within the scope of a macro.
If START and END are set, minimize the buffer analysis to
approximately this location as an optimization.
Alternatively, if `rust-macro-scopes' is a list use the scope
information in this variable. This last is an optimization and
the caller is responsible for ensuring that the data in
`rust-macro-scopes' is up to date."
(when (> (rust-paren-level) 0)
(let ((scopes
(or
rust-macro-scopes
(rust-macro-scope start end))))
;; `rust-macro-scope' can return the symbol `empty' if the
;; buffer has no macros at all.
(when (listp scopes)
(seq-some
(lambda (sc)
(and (>= (point) (car sc))
(< (point) (cadr sc))))
scopes)))))

(defun rust-looking-at-where ()
"Return T when looking at the \"where\" keyword."
Expand Down Expand Up @@ -1208,32 +1285,34 @@ whichever comes first."

(defun rust-syntax-propertize (start end)
"A `syntax-propertize-function' to apply properties from START to END."
(goto-char start)
(let ((str-start (rust-in-str-or-cmnt)))
(when str-start
(rust--syntax-propertize-raw-string str-start end)))
(funcall
(syntax-propertize-rules
;; Character literals.
(rust--char-literal-rx (1 "\"") (2 "\""))
;; Raw strings.
("\\(r\\)#*\""
(0 (ignore
(goto-char (match-end 0))
(unless (save-excursion (nth 8 (syntax-ppss (match-beginning 0))))
(put-text-property (match-beginning 1) (match-end 1)
'syntax-table (string-to-syntax "|"))
(rust--syntax-propertize-raw-string (match-beginning 0) end)))))
("[<>]"
(0 (ignore
(when (save-match-data
(save-excursion
(goto-char (match-beginning 0))
(rust-ordinary-lt-gt-p)))
(put-text-property (match-beginning 0) (match-end 0)
'syntax-table (string-to-syntax "."))
(goto-char (match-end 0)))))))
(point) end))
;; Cache all macro scopes as an optimization. See issue #208
(let ((rust-macro-scopes (rust-macro-scope start end)))
(goto-char start)
(let ((str-start (rust-in-str-or-cmnt)))
(when str-start
(rust--syntax-propertize-raw-string str-start end)))
(funcall
(syntax-propertize-rules
;; Character literals.
(rust--char-literal-rx (1 "\"") (2 "\""))
;; Raw strings.
("\\(r\\)#*\""
(0 (ignore
(goto-char (match-end 0))
(unless (save-excursion (nth 8 (syntax-ppss (match-beginning 0))))
(put-text-property (match-beginning 1) (match-end 1)
'syntax-table (string-to-syntax "|"))
(rust--syntax-propertize-raw-string (match-beginning 0) end)))))
("[<>]"
(0 (ignore
(when (save-match-data
(save-excursion
(goto-char (match-beginning 0))
(rust-ordinary-lt-gt-p)))
(put-text-property (match-beginning 0) (match-end 0)
'syntax-table (string-to-syntax "."))
(goto-char (match-end 0)))))))
(point) end)))

(defun rust-fill-prefix-for-comment-start (line-start)
"Determine what to use for `fill-prefix' based on the text at LINE-START."
Expand Down

0 comments on commit bfe4056

Please sign in to comment.