Skip to content

Coding standards: Add guideline for variable scope in if statements#346

Open
nyalldawson wants to merge 2 commits intomasterfrom
nyalldawson-patch-1
Open

Coding standards: Add guideline for variable scope in if statements#346
nyalldawson wants to merge 2 commits intomasterfrom
nyalldawson-patch-1

Conversation

@nyalldawson
Copy link
Contributor

Adds a coding guideline to limit the leakage of variables used for if statements by initializing them within the if statement scope

Co-authored-by: Julien Cabieces <julien.cabieces@oslandia.com>
@nyalldawson
Copy link
Contributor Author

Thanks @troopa81 !

c++17 [if statement with initializer](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0305r0.html) form can be used to limit the scope:

``` cpp
if ( QgsMarkerSymbol *marker = renderer->symbol; marker->symbolLayerCount() > 1 )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better add the pointer checking to this example too:

Suggested change
if ( QgsMarkerSymbol *marker = renderer->symbol; marker->symbolLayerCount() > 1 )
if ( QgsMarkerSymbol *marker = renderer->symbol; marker && marker->symbolLayerCount() > 1 )

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After encountering a real bug because of this (qgis/QGIS#64871), I would suggest we also add a rule to always check the pointer initialized within the if initializer.
I couldn't find a related check in clang-tidy, however we could craft something with regular expressions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a related check in clang-tidy, however we could craft something with regular expressions.

I think we could write something with clang API to be more robust. Something like that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

In Discussion QEPs currently in discussion stage Policy

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants