llvm / llvm-project

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

[clang-tidy][false negative] bugprone-integer-division not detected inside std::make_unique #110961

Open hexagonrecursion opened 1 week ago

hexagonrecursion commented 1 week ago

[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:7,positionColumn:16,positionLineNumber:7,selectionStartColumn:16,selectionStartLineNumber:7,startColumn:16,startLineNumber:7),source:'%23include+%3Cmemory%3E%0A%0Afloat+floatFunc(float)%3B%0A%0Aclass+C+%7B%0Apublic:%0A++++C(float)+%7B%7D%0A%7D%3B%0A%0Aint+main()+%7B%0A++++std::make_unique%3CC%3E(7+/+2)%3B%0A%7D%0A'),l:'5',n:'1',o:'C%2B%2B+source+%231',t:'0'),(h:compiler,i:(compiler:clang_trunk,filters:(b:'0',binary:'1',binaryObject:'1',commentOnly:'0',debugCalls:'1',demangle:'0',directives:'0',execute:'1',intel:'0',libraryCode:'0',trim:'1',verboseDemangling:'0'),flagsViewOpen:'1',fontScale:14,fontUsePx:'0',j:1,lang:c%2B%2B,libs:!(),options:'',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:39.1644908616188,l:'4',n:'0',o:'',s:0,t:'0'),(g:!((h:tool,i:(args:'--checks%3D!'-*,bugprone-integer-division!'',argsPanelShown:'0',compilerName:'x86-64+clang+(trunk)',editorid:1,fontScale:14,fontUsePx:'0',j:1,monacoEditorHasBeenAutoOpened:'1',monacoEditorOpen:'1',monacoStdin:'1',stdin:'',stdinPanelShown:'1',toolId:clangtidytrunk,treeid:0,wrap:'1'),l:'5',n:'0',o:'clang-tidy+(trunk)+x86-64+clang+(trunk)+(Editor+%231,+Compiler+%231)',t:'0')),k:60.835509138381205,l:'4',n:'0',o:'',s:0,t:'0')),l:'2',n:'0',o:'',t:'0')),version:4)

#include <memory>

float floatFunc(float);

class C {
public:
    C(float) {}
};

int main() {
    std::make_unique<C>(7 / 2);  // should warn here, but does not
}

Expected:

<source>:11:25: warning: result of integer division used in a floating point context; possible loss of precision [bugprone-integer-division]
   11 |     std::make_unique<C>(7 / 2);
      |                         ^
nicovank commented 6 days ago

Technically, this is still in integer context I think. An int is passed to make_unique, which converts it during overload resolution.

Godbolt

chrchr-github commented 5 days ago

But this is just what bugprone-integer-division is intended for. The reason for the FN seems to be the template parameter pack used by std::make_unique, same as in #110960 and #110964.

nicovank commented 5 days ago

Right, I'm not saying a warning wouldn't be nice, I think this will require much more extensive inter-procedural static analysis. "Is this integer division ever used in a float context?" Any function that takes in arguments and later constructs the value with those arguments is affected, notably emplace-style functions as well.

#include <vector>

int main() {
    std::vector<float> v;
    v.emplace_back(5/3); // FN.
}
chrchr-github commented 5 days ago

Good point about emplace(), so there are even more opportunities for FNs. bugprone-narrowing-conversions is also affected:

void f(std::vector<float>& v, int i) {
    v.push_back(i); // bugprone-narrowing-conversions,cppcoreguidelines-narrowing-conversions
    v.emplace_back(i); // FN
}
hexagonrecursion commented 5 days ago

bugprone-narrowing-conversions is also affected

There might be many more affected checks. I just stopped searching after finding three because I did not want to spam