Brain storming - modernizing source code

[Daniel Marjamäki]

If we "brain storm" for a moment. What would some good c++ modernizing refactorings be for the Cppcheck source code?

This is a "brain storming" so well.. feel free to suggest latest C++21 features ... it might be good to know about them now already but it will take a while until those can be used.

[Markus Elfring]

If we "brain storm" for a moment.

How does this description fit to clarification requests around a topic like “Selection of a newer C++ version for a software build variant with CMake”?

[Daniel Marjamäki]

I think it fits. As far as I see you suggest std::make_unique in that thread. Off the top of my head I don't see a use case for it. But if there is a proper use case, yes I agree!

[Markus Elfring]

I think it fits.

I am curious on how this view will evolve. I am trying to increase the usage of smart pointers generally besides other functionality from the current C++ version.

[Rainer Wiesenfarth]

Not exactly "modernizing", but more harmonizing: I'd like to be able to feed a GUI-defined project to the CLI. That'll allow to define an "test-run" settings on the GUI and add them to a CI build step once you are satisfied with them.

Would require:
- base ProjectFile on tinyxml and std rather than on QXmlStreamReader and QString, QStringList,...
- extract and unify ProjectFile to ImportProject conversions (excludedPaths, analyseAllVsConfigs)
- move ProjectFile / ThreadHandler handling of addonsAndTools to some common implementation
- extract and unify ProjectFile to Settings conversions (includeDirs, defines, undefines, libraries,...)

In general, I get the feeling that whoever implemented the GUI part chose to ignore what existed for the command line...

[Daniel Marjamäki]

Yes I agree totally.

Nowadays you can run clang-tidy and addons from the GUI. We do not want that these results are mixed, so we might need to disable this in the CLI.

[Daniel Marjamäki]

I have created this ticket: https://trac.cppcheck.net/ticket/8820

[Rainer Wiesenfarth]

I created a feature branch on my clone (https://github.com/rwiesenfarth/cppcheck/tree/feature/importGuiProject) where I try to get this implemented.

For now, the cli is "polluted" with Qt, as converting ProjectFile from Qt to std poses the lowest risk (at least for me). I'll keep you updated about the progress (and/or bother you with questions :-) ).

[Daniel Marjamäki]

start using enum classes. I am not sure if the compilers have the support but if they do we should start using these asap.

[Daniel Marjamäki]

use std::regex instead of pcre. we need to determine if there is any significant difference first though.

[Daniel Marjamäki]

use stl algorithms. Our stlAlgorithm checker can suggest candidates.

[Paul Fultz]

We can make a Token iterator. Then we can convert it to a range and traverse tokens using a range-base for loop:

void foo(TokenIterator start, TokenIterator end)
{
    for(const Token& t:range(start, end)) {
        // Do some stuff
    }    
}

Or we could have an outdirected function to use the iterators directly:

void foo(TokenIterator start, TokenIterator end)
{
    for(TokenIterator t:outdirect(range(start, end))) {
        // Do some stuff
    }    
}

Of course, it we would be good to update our functions to use references or iterators of tokens instead of pointers.

[Daniel Marjamäki]

We can make a Token iterator.

I am skeptic. I am not sure how you mean. I need to see it to believe in it. It does seem nice to be able to use range for loops.

Of course, it we would be good to update our functions to use references or iterators of tokens instead of pointers.

I am skeptic about this one also. I've played with Token references for functions if they expect a non-NULL pointer... it just become inconsistent and weird.

For Token::link, Token::astParent, Token::astOperand1, Token::astOperand2 I believe that pointers work very well. References would not work at all as far as I see, these are not constant they are modified and must be able to contain "nothing". Iterators.. I think it could work in theory but the code would be more clumpsy.

Imho, iterator code can be clumpsy compared to pointers. The declaration const Token *tok is shorter and nicer than Token::const_iterator tok. And you can write if (!tok) instead of if (tok != mTokenList.end()).

[Markus Elfring]

Imho, iterator code can be clumpsy compared to pointers.

Can this software development concern be adjusted by making the affected programming interfaces more convenient to use?

[Markus Elfring]

Of course, it we would be good to update our functions to use references or iterators of tokens instead of pointers.

Would you like to continue the development discussion also for a topic like “Increasing usage of C++ references?”?

[Paul Fultz]

Also we can look at using HOFs to walk the ast:

template<class F>
void visitAst(const Token* tok, F f)
{
    if(!tok)
        return;
    visitAst(tok->astOperand1(), f);
    f(tok);
    visitAst(tok->astOperand2(), f);
}

visitAst(tok, [&](const Token* tok2) {
    // Do something with tok2
});

We could tweak it more to control how we traverse as well, such returning a value to control whether to stop traversing or stop traversing deeper.

[Daniel Marjamäki]

I like this suggestion.