Thread: [CEDET-devel] Two minor bugfixes
Brought to you by:
zappo
From: Martin S. <Mar...@gm...> - 2015-02-11 22:47:19
|
First bug: File: 0001-Check-that-bounds-is-non-nil-in-semantic-ia-complete.patch In semantic-ia-complete-symbol-menu in ia.el a check whether bounds are valid is missing. There is a check in semantic-ia-complete-symbol. The bug is triggered when completing something like myfun( with empty prefix (see example in commit message). Remark: In contrast to the code snippet in semantic-ia-complete-symbol I have used a let instead of having four (oref a bounds) evaluations. I also omitted the (goto-char (car (oref a bounds)), as delete-region already places point at this position. Second bug: File: 0001-Save-global-variable-wisent-lookahead-before-calling.patch The wisent parser uses some global variables, which is a bit of a hassle considering that it calls itself (via EXPANDFULL and the like). If the last token of a recursive call does not match, it can corrupt the unmatched-syntax-cache or even lead to wrong parse results. If recursive calls of the parser are only done with parenthesised blocks and rules are accordingly, then the last token must be something like ')' and thus always matches and the bug is not triggered. For languages like fortran this is different. I am very sure that the commit indeed fixes a bug and nothing more, but the wisent parser and its token stream organisation is a rather complex automaton, so I might have missed something. |
From: Eric L. <er...@si...> - 2015-02-16 13:52:30
|
On 02/11/2015 05:47 PM, Martin Stein wrote: > First bug: > File: 0001-Check-that-bounds-is-non-nil-in-semantic-ia-complete.patch > > In semantic-ia-complete-symbol-menu in ia.el a check whether bounds are > valid is missing. There is a check in semantic-ia-complete-symbol. The > bug is triggered when completing something like > myfun( > with empty prefix (see example in commit message). > Remark: In contrast to the code snippet in semantic-ia-complete-symbol I > have used a let instead of having four (oref a bounds) evaluations. I > also omitted the (goto-char (car (oref a bounds)), as delete-region > already places point at this position. Hi Martin, Thanks, I submitted a variant of your patch. This was also a problem for the non-menu version related to checking for case insensitivity and an empty prefix your test case found which I fixed. > Second bug: > File: 0001-Save-global-variable-wisent-lookahead-before-calling.patch > > The wisent parser uses some global variables, which is a bit of a hassle > considering that it calls itself (via EXPANDFULL and the like). If the > last token of a recursive call does not match, it can corrupt the > unmatched-syntax-cache or even lead to wrong parse results. If recursive > calls of the parser are only done with parenthesised blocks and rules > are accordingly, then the last token must be something like ')' and thus > always matches and the bug is not triggered. For languages like fortran > this is different. > I am very sure that the commit indeed fixes a bug and nothing more, but > the wisent parser and its token stream organisation is a rather complex > automaton, so I might have missed something. This is a great find. Thanks. I've installed it here and it passes the test suites. Was there an example that exposed this that is short you can share? Can the bug be exposed using some existing language? It would be nice if I could add a test for this. Thanks Eric |