llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.17k stars 12.04k forks source link

clang-tidy readability-else-after-return false positive #110694

Open LegalizeAdulthood opened 1 month ago

LegalizeAdulthood commented 1 month ago

Suppose you introduce a variable in the scope of the if statement like so:

    size_t index{};
    if( const auto pos{ std::find( geom->materialIds.begin(), geom->materialIds.end(), proxyMaterialId ) };
        pos == geom->materialIds.end() )
    {
        throw std::runtime_error( "Mismatched material id; expected one of " + toString( geom->materialIds ) + ", got "
                                  + std::to_string( proxyMaterialId ) );
    }
    else
    {
        index = std::distance( geom->materialIds.begin(), pos );
    }

readability-else-after-return doesn't recognize the use of the scoped variable in the else and flags the else after return as a violation. This is a false positive.

llvmbot commented 1 month ago

@llvm/issue-subscribers-bug

Author: Richard Thomson (LegalizeAdulthood)

Suppose you introduce a variable in the scope of the `if` statement like so: ``` size_t index{}; if( const auto pos{ std::find( geom->materialIds.begin(), geom->materialIds.end(), proxyMaterialId ) }; pos == geom->materialIds.end() ) { throw std::runtime_error( "Mismatched material id; expected one of " + toString( geom->materialIds ) + ", got " + std::to_string( proxyMaterialId ) ); } else { index = std::distance( geom->materialIds.begin(), pos ); } ``` `readability-else-after-return` doesn't recognize the use of the scoped variable in the else and flags the else after return as a violation. This is a false positive.
llvmbot commented 1 month ago

@llvm/issue-subscribers-clang-tidy

Author: Richard Thomson (LegalizeAdulthood)

Suppose you introduce a variable in the scope of the `if` statement like so: ``` size_t index{}; if( const auto pos{ std::find( geom->materialIds.begin(), geom->materialIds.end(), proxyMaterialId ) }; pos == geom->materialIds.end() ) { throw std::runtime_error( "Mismatched material id; expected one of " + toString( geom->materialIds ) + ", got " + std::to_string( proxyMaterialId ) ); } else { index = std::distance( geom->materialIds.begin(), pos ); } ``` `readability-else-after-return` doesn't recognize the use of the scoped variable in the else and flags the else after return as a violation. This is a false positive.
chrchr-github commented 1 month ago

Reduced/compilable:

#include <cstdlib>

int main() {
    int i{};
    if (int x = rand(); x == 0) {
        throw 0;
    }
    else {
        i = x;
    }
    return i;
}

https://godbolt.org/z/oPbWxaEdK

To be fair, it is possible to rewrite this without else after return:

int main() {
    int i{};
    {
        int x = rand();
        if (x == 0) {
            throw 0;
        i = x;
    }
    return i;
}