Discussion:
[ragel-users] ragel-6.8 bug report
David Binderman
2013-10-10 12:37:49 UTC
Permalink
Hello there,

I just ran the static analysis tool "cppcheck" over
the source code of ragel-6.8

It said

[parsedata.cpp:471]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmbase.cpp:139]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmbase.cpp:334]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmbase.cpp:428]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmgraph.cpp:301]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmgraph.cpp:1227]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmgraph.cpp:1228]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[fsmgraph.cpp:1406]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[rlparse.cpp:1413]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?
[rlparse.cpp:1429]: (warning) Ineffective call of function 'empty()'. Did you intend to call 'clear()' instead?

I looked at the first two and it did look like clear was intended.
Suggest code rework.

Regards

David Binderman
RU
2013-10-10 12:48:36 UTC
Permalink
Are you sure that cppcheck knows about the Aapl template library?

jg
David Binderman
2013-10-10 13:24:58 UTC
Permalink
Hello there,

----------------------------------------
> Are you sure that cppcheck knows about the Aapl template library?

I am sure that cppcheck knows nothing about the Aapl template library.

It does, however, know quite a bit about C++.

Hint: would a check of the C++ reference manual to
understand the difference between clear() and empty()
be a wise idea ?

Clue: empty() would be better named is_empty() and clear would
be better named do_clear().? Maybe in some better future version
of C++.

Regards

David Binderman
RU
2013-10-10 13:51:54 UTC
Permalink
Well, you would find empty() implementations like

/**
* \brief Empty the tree and delete all the element.
*
* Resets the tree to its initial state.
*/
template <AVLMEL_TEMPDEF> void AvlTree<AVLMEL_TEMPUSE>::empty()
{
if ( root ) {
/* Recursively delete from the tree structure. */
deleteChildrenOf(root);
delete root;
root = 0;
treeSize = 0;

#ifdef WALKABLE
BASELIST::abandon();
#endif
}

What is called empty() here, is called erase() in other places. I would assume that cppcheck
applies the rules for the STL libs, but this is misleading here. It's basically the author's
choice.

jg
David Binderman
2013-10-10 14:40:07 UTC
Permalink
Hello there,

----------------------------------------
> Well, you would find empty() implementations like

That's going to confuse any C++ STL programmer and the cppcheck tool too.

Would it be worthwhile to rename function to erase or pick another name ?

Regards

David Binderman
RU
2013-10-10 14:54:27 UTC
Permalink
David,
> That's going to confuse any C++ STL programmer and the cppcheck tool too.
>
> Would it be worthwhile to rename function to erase or pick another name ?
there are worse things in life than this. Since you have the source code, you could do it
yourself. I actually like the idea of using the verb "empty". Let's say your container models a
real life entity, beer for example. You would empty the container, but erasing it is not useful.

jg
Loading...