[clang-tidy] Request for dangling reference check for (ranges) min, max, minmax

[olologin]

Hi, is it possible to implement dangling reference clang-tidy check that identifies bad calls to std::min, std::max, std::minmax, and potentially for std::ranges versions of these functions. For example on inputs like this:

[astdump from godbolt](https://godbolt.org/#g:!((g:!((g:!((h:codeEditor,i:(filename:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,selection:(endColumn:16,endLineNumber:12,positionColumn:16,positionLineNumber:12,selectionStartColumn:16,selectionStartLineNumber:12,startColumn:16,startLineNumber:12),source:'template%3Cclass+T%3E%0Aconst+T%26+min(const+T%26+a,+const+T%26+b)%0A%7B%0A++++return+(b+%3C+a)+%3F+b+:+a%3B%0A%7D%0A%0Aint+normal()%0A%7B%0A++++int+a+%3D+0%3B%0A++++int+b+%3D+0%3B%0A++++const+auto+c+%3D+min(a,+b)%3B%0A++++const+auto%26+d+%3D+min(a,+b)%3B%0A%7D%0A%0Aint+dangling()%0A%7B%0A++++const+auto%26+c+%3D+min(1,+2)%3B%0A++++int+a+%3D+2%3B%0A++++const+auto%26+d+%3D+min(a,+normal())%3B%0A++++const+auto%26+e+%3D+min(1,+a*2)%3B%0A%7D'),l:'5',n:'1',o:'C%2B%2B+source+%231',t:'0')),k:35.81213307240704,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((g:!((h:compiler,i:(compiler:clang_trunk,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'1',intel:'1',libraryCode:'0',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'-Xclang+-ast-dump+-std%3Dc%2B%2B20',overrides:!(),selection:(endColumn:1,endLineNumber:1,positionColumn:1,positionLineNumber:1,selectionStartColumn:1,selectionStartLineNumber:1,startColumn:1,startLineNumber:1),source:1),l:'5',n:'0',o:'+x86-64+clang+(trunk)+(Editor+%231)',t:'0')),k:50.82493349378958,l:'4',m:25.357710651828295,n:'0',o:'',s:0,t:'0'),(g:!((h:output,i:(compilerName:'x86-64+clang+18.1.0',editorid:1,fontScale:14,fontUsePx:'0',j:1,wrap:'0'),l:'5',n:'0',o:'Output+of+x86-64+clang+(trunk)+(Compiler+%231)',t:'0')),header:(),l:'4',m:74.6422893481717,n:'0',o:'',s:0,t:'0')),k:64.18786692759295,l:'3',n:'0',o:'',t:'0')),l:'2',n:'0',o:'',t:'0')),version:4)

Code from godbolt:

// self-implemented version of min to avoid inclusion of <algorithm>
// this should generate smaller AST tree
template<class T>
const T& min(const T& a, const T& b)
{
    return (b < a) ? b : a;
}

int normal()
{
    int a = 0;
    int b = 0;
    const auto c = min(a, b);
    const auto& d = min(a, b);
}

int dangling()
{
    const auto& c = min(1, 2);
    int a = 2;
    const auto& d = min(a, normal());
    const auto& e = min(1, a*2);
}

I guess we need to match for function min, max, minmax calls that store results to reference and has MaterializeTemporaryExpr child nodes? Will that be good enough?

Or maybe it is already implemented somewhere as clang warning? I tried to google it but could not find anything.

[5chmidti]

Agreed. And std::clamp suffers from the same issue as well (https://en.cppreference.com/w/cpp/algorithm/clamp).

I guess we need to match for function min, max, minmax calls that have MaterializeTemporaryExpr child nodes? Will that be good enough?

That would only match some of the problematic cases. Instead, we would want to check the value category of the argument. In this case, we are looking for xvalues. Here is an example of how a dangling reference can be produced without the argument being wrapped inside a MaterializeTemporaryExpr:

struct S {
    int val;
};

void f(const int &);

void foo() {
    f(S{}.val); // only `S{}` is inside a `MaterializeTemporaryExpr`
}

https://godbolt.org/z/fzGMvvbqz

Or maybe it is already implemented somewhere as clang warning?

Clang does have dangling reference warnings, but they dont warn on this issue. There is also bugprone-return-const-ref-from-parameter, but that is for authors of functions that may produce dangling references.

[olologin]

If somebody wants to take it over - be free. I don't think I know AST well enough for this. I guess people can start from initially looking at the implementation of bugprone-return-const-ref-from-parameter: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp

Update: Actually, looking at the description of bugprone-return-const-ref-from-parameter it seems like it should be able to catch issues in std::min/max/... functions. I understand that this would probably require symbolic execution and other complex stuff, but maybe potentially bad functions can just be hardcoded into that existing check? :)