I have been looking at this commit (https://github.com/asmwarrior/codeblocks_sfmirror/commit/9dc5777ddf05aeb599c72dc8bbfd8d03e14b68f0) (the exact location of the changes was not specified).
PPToken looks to me just a way to pack token information, so I assume the real benefit is using the deque afterwards.
I just suggest changing
if (m_PPTokenStream.size() > 0)
if (!m_PPTokenStream.empty())
else
;// peekToken.Clear();
I very briefly glanced across the commit pointed to by the link.
findings:
Thanks for the comment.
class PToken: data-member m_Kind is left uninitialized when PToken is default initialized and when initialized via the ctor with 4 args (while having 5 data-member).
The call to the latter one is also a little errorprone. It might easily be used incorrectly because it has 3 int args. I'd consider member-initializer for the 4 integral data
member and delete the default-ctor.
Oh, yes, I should initialize the m_Kind member variable in the default constructor and other constructors.
About the argument: "PPToken(wxString lexeme, int charIndex, int lineIndex, int nestLevel)", I really don't know where does the "charIndex" come from, I will looked into it.
The cctor of PToken is unecessary and breaks the rule-of-0 without need. It might also copy m_Kind from
an uninitialized data-member. That is undefined behaviour. IMHO the cctor should be removed.
My initial though is that PPToken's copy constructor is used because I think it need to construct the element in the deque, in some cases, the PPToken get copied to the deque. Am I wrong?
Oh, you are correct,
In C++, the "Rule of Zero" is a guideline that suggests avoiding writing custom constructors, destructors, or copy/move assignment operators if the default compiler-generated versions will suffice. The rule states that if a class does not need custom resource management (like dynamic memory allocation), it can rely on the compiler-generated special member functions.
So, the copy constructor is not needed here, because the compiler will generate the same one if I remove it.
The compound statement after if (IsEOF()) is repeated. It sets 2 data-members of PToken and should be delegated to PToken.
Do you mean that the
/** Check whether the Tokenizer reaches the end of the buffer (file) */
bool IsEOF() const
{
return m_TokenIndex >= m_BufferLen;
}
should be removed from the high level parser, but we can return a PPToken which has m_Kind field set as "EOF"?
Thanks.
No, I mean this:
if (IsEOF())
{
m_Lex = wxEmptyString;
m_Lex.m_Lexeme = wxEmptyString;
m_Lex.m_Kind = PPTokenKind::EndOfFile;
class PToken
{
// ...
void setEof()
{
m_Lexeme = wxEmptyString;
m_Kind = PPTokenKind::EndOfFile;
}
// ...
};
if (IsEOF())
{
m_Lex.setEof();
return false;
}