rtrieu reopened this revision.
rtrieu added a comment.
This revision is now accepted and ready to land.

This warning is now flagging some code which I believe is a false positive.  In 
particular, when template arguments are involved, their values can be 
calculated during template instantiation, but they should be treated as 
variables instead of constants.  For example:

  template <int I, class T>
  void foo(int x) {
      bool b1 = (x & sizeof(T)) == 8;
      bool b2 = (x & I) == 8;
      bool b3 = (x & 4) == 8;
  }
  
  void run(int x) {
      foo<4, int>(8);
  }

In this instantiation, x is and'ed with the value 4 in different ways.  
`sizeof(T)` is type-dependent, `I` is value-dependent, and `4` is an integer 
literal.  With your code, each of them would produce a warning.  However, the 
first two values of 4 will change in different template instantiations, so the 
warning is not useful when it is okay for some instantiations to have constant 
values.  Because of this, warnings should treat dependent expressions as 
non-constant even when they can be evaluated, so only `b3` should get a 
warning.  This is causing the warning to be emitted on code heavy in template 
meta-programming, such as array libraries.  Please revert or fix.

I believe that evaluating the expression would make this warning too broad and 
would need more testing that what was included here.  Only handling 
UnaryOperator with IntegerLiteral sub-expression makes more sense for the 
warning, and adding in any new cases if we find them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to