JonasToth added inline comments.
================ Comment at: clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp:178 + return; + // Conversions to unsigned integer are well defined and follow modulo 2 + // arithmetic. ---------------- gchatelet wrote: > JonasToth wrote: > > I am surprised by `following modulo 2 arithmetic` and think it's a bit > > misleading. Writing just `module arithmetic` is probably better, as `module > > 2` somewhat implies there a only 2 valid values (0, 1). > > > > Is this the `int` -> `unsigned int` case path? That seems worth diagnosing > > too. > Yes, thx for noticing. I updated the comment, I think it's better now. > > Indeed this is the `int` -> `unsigned int` case path. Warning here would lead > to a lot of noise for everybody doing bitwise operations since `-1` is a > compact way to represent the maximum value. Since the semantic is valid and > well defined by the standard I'm unsure it's worth the pain. I'm open to > suggestions though. > > Maybe a good way to figure out is what would be the correct fix for say > `unsigned long AllBits = -1;` Comment is fine :) I though that we have check that diagnoses `unsigned i = -1;` but I don't find it right now (maybe its still in review or so, i belive it was related to introducing `std::numeric_limits<>`). As its well defined and not narrowing and not mentionend by the CPPCG in that section its ok, maybe worth an option in the future? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D53488 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
