Developer forums (C::B DEVELOPMENT STRICTLY!) > Development
Several improvements to Code Completion plugin
Huki:
Thanks for the commit. 2 patches for parsing bugs:
1) Don't skip successive opening or closing brackets. For example, see any occurrence of "GetConfigManager(_T("code_completion"))->XXX" in CodeCompletion.cpp,
--- Code: ---void CodeCompletion::UpdateToolBar()
{
bool showScope = Manager::Get()->GetConfigManager(_T("code_completion"))->ReadBool(_T("/scope_filter"), true);
^^^^^^^^________________________________ // cannot use CC on ReadBool
[...]
--- End code ---
Fix:
--- Code: ---From dcc4ea033b398329915a900faada2d1254261313 Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Tue, 24 Jun 2014 08:11:28 +0530
Subject: [PATCH 04/30] CC: fix parser successive brackets
---
src/plugins/codecompletion/nativeparser_base.cpp | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/src/plugins/codecompletion/nativeparser_base.cpp b/src/plugins/codecompletion/nativeparser_base.cpp
index 89e252c..3c5cf75 100644
--- a/src/plugins/codecompletion/nativeparser_base.cpp
+++ b/src/plugins/codecompletion/nativeparser_base.cpp
@@ -561,6 +561,9 @@ unsigned int NativeParserBase::FindCCTokenStart(const wxString& line)
if (IsClosingBracket(startAt, line))
++nest;
+ //NOTE: do not skip successive opening brackets.
+ if (IsOpeningBracket(startAt, line))
+ --nest;
}
startAt = BeforeToken(startAt, line);
@@ -650,6 +653,15 @@ wxString NativeParserBase::GetNextCCToken(const wxString& line,
if (IsOpeningBracket(startAt, line))
++nest;
+ //NOTE: do not skip successive closing brackets. Eg,
+ // "GetConfigManager(_T("code_completion"))->ReadBool"
+ // ^
+ if (IsClosingBracket(startAt, line))
+ {
+ --nest;
+ if (nest == 0)
+ ++startAt;
+ }
}
}
if (IsOperatorBegin(startAt, line))
--
1.9.4.msysgit.0
--- End code ---
2) In DoParse(), when parsing "else" we try to eat the arguments. It breaks support for "else-if" (we will end up eating the "if"), and anyway there is no need to skip anything after "else".
--- Code: ---From 900b6df9304e30455438c362f2757c88bd690494 Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Tue, 24 Jun 2014 09:20:07 +0530
Subject: [PATCH 26/30] CC: parser: fixed 'else-if'
---
src/plugins/codecompletion/parser/parserthread.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/plugins/codecompletion/parser/parserthread.cpp b/src/plugins/codecompletion/parser/parserthread.cpp
index f87b1b7..938a134 100644
--- a/src/plugins/codecompletion/parser/parserthread.cpp
+++ b/src/plugins/codecompletion/parser/parserthread.cpp
@@ -714,8 +714,9 @@ void ParserThread::DoParse()
{
if (!m_Options.useBuffer || m_Options.bufferSkipBlocks)
SkipToOneOfChars(ParserConsts::semicolonclbrace, true, true);
- else
- m_Tokenizer.GetToken(); // skip arguments
+ //NOTE: this breaks support for 'else if' !
+ // else
+ // m_Tokenizer.GetToken(); // skip arguments
m_Str.Clear();
}
else if (token == ParserConsts::kw_enum)
--
1.9.4.msysgit.0
--- End code ---
ollydbg:
--- Quote from: Huki on June 26, 2014, 08:22:19 pm ---Thanks for the commit. 2 patches for parsing bugs:
...snip...
--- End quote ---
Hi, sorry for the late reply, I'm busy those days.
I tested and committed the two patches(rev9846 and rev9847), thanks for your contribution. ;)
Huki:
--- Quote from: ollydbg on July 16, 2014, 08:51:14 am ---
--- Quote from: Huki on June 26, 2014, 08:22:19 pm ---Thanks for the commit. 2 patches for parsing bugs:
...snip...
--- End quote ---
Hi, sorry for the late reply, I'm busy those days.
I tested and committed the two patches(rev9846 and rev9847), thanks for your contribution. ;)
--- End quote ---
Hi, thanks for the commit.
A small update to the undo token behavior fix.
There was some code I added to CalcConditionExpression() by mistake (assuming it is called directly inside SkipUnwanted()). See my comments below:
--- Quote from: Huki on June 24, 2014, 08:27:49 pm ---[...] A preprocessor condition is hit in SkipUnwanted() and we run CalcConditionExpression(). [...] We need to reset Undo and Saved positions to the proper place after CalcConditionExpression() (because it's not only expanding the macro but also parsing it fully, so we need to reset right after the preprocessor line has ended).
--- End quote ---
In fact the function which is called in SkipUnwanted() is HandleConditionPreprocessor(), that's where we should reset the undo and savetoken values. If we reset them in CalcConditionExpression() we will always reset to the start of the first conditional block even if it is false.
--- Code: ---#if 0| <---- adding the code in CalcConditionExpression() will reset undo vars to here which is wrong
// inactive code
#else| <---- adding the code in HandleConditionPreprocessor() will reset undo vars to here
// active code
#endif
--- End code ---
I've also added a small note to avoid recursive call to PeekToken() (which shouldn't really happen).
Here's the patch:
--- Code: ---From fd5af3421e85d9eff782c63ab07736cadf04d45c Mon Sep 17 00:00:00 2001
From: huki <gk7huki@gmail.com>
Date: Fri, 4 Jul 2014 02:32:16 +0530
Subject: CC: update to undo token fix
---
src/plugins/codecompletion/parser/tokenizer.cpp | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/plugins/codecompletion/parser/tokenizer.cpp b/src/plugins/codecompletion/parser/tokenizer.cpp
index 2bfb113..4a8f0cb 100644
--- a/src/plugins/codecompletion/parser/tokenizer.cpp
+++ b/src/plugins/codecompletion/parser/tokenizer.cpp
@@ -1043,7 +1043,6 @@ wxString Tokenizer::PeekToken()
{
if (!m_PeekAvailable)
{
- m_PeekAvailable = true;
//NOTE: The m_Saved... vars will be reset to the correct position
// as necessary when a ReplaceBufferText() is done.
m_SavedTokenIndex = m_TokenIndex;
@@ -1055,6 +1054,7 @@ wxString Tokenizer::PeekToken()
else
m_PeekToken.Clear();
+ m_PeekAvailable = true; //NOTE: Moved after DoGetToken() to avoid recursive PeekToken() calls.
m_PeekTokenIndex = m_TokenIndex;
m_PeekLineNumber = m_LineNumber;
m_PeekNestLevel = m_NestLevel;
@@ -1407,11 +1407,6 @@ bool Tokenizer::CalcConditionExpression(bool checkResult)
// reset tokenizer's functionality
m_State = oldState;
- // reset undo token
- m_SavedTokenIndex = m_UndoTokenIndex = m_TokenIndex;
- m_SavedLineNumber = m_UndoLineNumber = m_LineNumber;
- m_SavedNestingLevel = m_UndoNestLevel = m_NestLevel;
-
exp.ConvertInfixToPostfix();
if (exp.CalcPostfix())
{
@@ -1697,6 +1692,11 @@ void Tokenizer::HandleConditionPreprocessor(const PreprocessorType type)
default:
break;
}
+
+ // reset undo token
+ m_SavedTokenIndex = m_UndoTokenIndex = m_TokenIndex;
+ m_SavedLineNumber = m_UndoLineNumber = m_LineNumber;
+ m_SavedNestingLevel = m_UndoNestLevel = m_NestLevel;
}
void Tokenizer::SplitArguments(wxArrayString& results)
@@ -1826,6 +1826,7 @@ bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken)
{
m_PeekAvailable = false;
PeekToken(); // NOTE (ollydbg#1#): chance of recursive call of this function
+ // NOTE (huki): updated PeekToken() to prevent unnecessary recursive call
}
return true;
--
1.9.4.msysgit.0
--- End code ---
ollydbg:
Hi, Huki, thanks for the contribution, today, I looked at you patch, and I think that
--- Code: --- // Update the peek token
if (m_PeekAvailable && updatePeekToken)
{
m_PeekAvailable = false;
PeekToken(); // NOTE (ollydbg#1#): chance of recursive call of this function
// NOTE (huki): updated PeekToken() to prevent unnecessary recursive call
}
--- End code ---
I think here, with your patch, the m_PeekAvailable should always be false in the if condition, right?
ollydbg:
--- Quote from: ollydbg on August 27, 2014, 05:54:59 am ---Hi, Huki, thanks for the contribution, today, I looked at you patch, and I think that
--- Code: --- // Update the peek token
if (m_PeekAvailable && updatePeekToken)
{
m_PeekAvailable = false;
PeekToken(); // NOTE (ollydbg#1#): chance of recursive call of this function
// NOTE (huki): updated PeekToken() to prevent unnecessary recursive call
}
--- End code ---
I think here, with your patch, the m_PeekAvailable should always be false in the if condition, right?
--- End quote ---
Oh, I was wrong, I just debug the code and found that the function bool Tokenizer::ReplaceBufferText(const wxString& target, bool updatePeekToken) will be called from void ParserThread::HandleMacroExpansion(int id, const wxString &peek), which is called from DoParse() function.
I understand you idea, that is the function Tokenizer::ReplaceBufferText() can also be called inside the PeekToken() function.
The old way (without your patch) of PeekToken() could be:
--- Code: ---first set the flag: m_PeekAvailable = true;
some function will call ReplaceBufferText();
--- End code ---
Thus, the condition "if (m_PeekAvailable && updatePeekToken)" is true, and we comes with a recursive call to PeekToken().
Now, in your patch, you set the flag a bit later, which are something like:
--- Code: ---some function will call ReplaceBufferText();
set the flag: m_PeekAvailable = true;
--- End code ---
Here, the if condition is always false, so we can avoid the recursive call to PeekToken().
Question: what happens about old way (without your patch), do we enter a infinite recursive call to PeekToken()?
Navigation
[0] Message Index
[#] Next page
[*] Previous page
Go to full version