[PATCH] D54943: [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth marked an inline comment as done.
JonasToth added inline comments.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp:608
+}
+
+template <typename L, typename R>
----------------
0x8000-0000 wrote:
> 0x8000-0000 wrote:
> > JonasToth wrote:
> > > JonasToth wrote:
> > > > 0x8000-0000 wrote:
> > > > > Please insert the this test code here:
> > > > > 
> > > > > ```
> > > > > struct IntWrapper {
> > > > > 
> > > > >    unsigned low;
> > > > >    unsigned high;
> > > > > 
> > > > >    IntWrapper& operator=(unsigned value) {
> > > > >       low = value & 0xffff;
> > > > >       high = (value >> 16) & 0xffff;
> > > > >    }
> > > > > 
> > > > >    template<typename Istream>
> > > > >    friend Istream& operator>>(Istream& is, IntWrapper& rhs) {
> > > > >       unsigned someValue = 0;       // false positive now
> > > > >       if (is >> someValue) {
> > > > >          rhs = someValue;
> > > > >       }
> > > > >       return is;
> > > > >    }
> > > > > };
> > > > > 
> > > > > unsigned TestHiddenFriend(IntMaker& im) {
> > > > >    IntWrapper iw;
> > > > > 
> > > > >    im >> iw;
> > > > > 
> > > > >    return iw.low;
> > > > > }
> > > > > ```
> > > > clang gives me no error when I add the const there. sure it does reproduce properly?
> > > Here for reference: https://godbolt.org/z/DXKMYh
> > Probably I wasn't clear - I suggested adding my test code at line 608, because it needs the "IntMaker" definition at line 594.
> > 
> > The false positive from this const check is reported on the "unsigned someValue = 0;" line inside the template extraction operator.
> Oh, I got it - don't think "shift" think "overloaded extraction operator".
> 
> In my code above, you don't know what ">>" does, and it clearly takes a mutable reference as the right side.
> 
> ```
> const int foo;
> std::cin >> foo;
> ```
> 
> should not compile, either :)
no. something else is going on.
https://godbolt.org/z/xm8eVC
```
| |   |-CXXOperatorCallExpr <line:21:5, col:11> '<dependent type>'
| |   | |-UnresolvedLookupExpr <col:8> '<overloaded function type>' lvalue (ADL) = 'operator>>' 0x55a26b885938 0x55a26b857238
| |   | |-DeclRefExpr <col:5> 'Istream' lvalue ParmVar 0x55a26b885748 'is' 'Istream &'
| |   | `-DeclRefExpr <col:11> 'const unsigned int' lvalue Var 0x55a26b885a38 'someValue' 'const unsigned int'
```
This code is only a false positive in the sense, that the "we can not know if something bad happens" is not detected. The operator>> is not resolved. That is because the template is not instantiated and the expressions can therefore not be resolved yet.
There seems to be no instantiation of this template function.

Somehow the `im >> iw;` does not instantiate the `friend operator<<`. I reduced it to something i think is minimal and that shows the same behaviour. (https://godbolt.org/z/MMG_4q)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54943/new/

https://reviews.llvm.org/D54943