Sorry it's taken so long to respond.
On Wed, Jun 24, 2015 at 05:43:05PM +0300, Mikhail Maltsev wrote:
> > That looks wrong, because with TREE_CONSTANT we'd warn in C but not in C++
> > for the following:
> >
> > const int a = 4;
> > void
> > f (void)
> > {
> > const int b = 4;
> > static const int c = 5;
> > if (a && a) {}
> > if (b && b) {}
> > if (c && c) {}
> > }
> >
> Actually for this case the patch silences the warning both for C and
> C++. It's interesting that Clang warns like this:
That's really not what I see. With your patch, cc1 warns on that testcase
while cc1plus does not.
> test.c:7:10: warning: use of logical '&&' with constant operand
> [-Wconstant-logical-operand]
>
> It does not warn for my testcase with templates. It also does not warn
> about:
>
> void
> bar (const int parm_a)
> {
> const bool a = parm_a;
> if (a && a) {}
> if (a || a) {}
> if (parm_a && parm_a) {}
> if (parm_a || parm_a) {}
> }
I think we want 4 warnings here, but vanilla cc1 only issues 2 warnings.
> Maybe my snippet does not express clearly enough what it was supposed to
> express. I actually meant something like this:
>
> template<class _U1, class _U2, class = typename
> enable_if<__and_<is_convertible<_U1, _T1>,
> is_convertible<_U2, _T2>>::value>::type>
> constexpr pair(pair<_U1, _U2>&& __p)
> : first(std::forward<_U1>(__p.first)),
> second(std::forward<_U2>(__p.second)) { }
>
> (it's std::pair move constructor)
> It is perfectly possible that the user will construct an std::pair<T, T>
> object from an std::pair<U, U>. In this case we get an "and" of two
> identical is_convertible instantiations. The difference is that here
> there is a clever __and_ template which helps to avoid warnings. Well,
> at least I now know a good way to suppress them in my code :).
>
> Though I still think that this warning is bogus. Probably the correct
> (and the hard) way to check templates is to compare ASTs of the operands
> before any substitutions.
>
> But for now I could try to implement an idea, which I mentioned in the
> bug report: add a new flag to enum tsubst_flags, and set it when we
> check ASTs which depend on parameters of a template being instantiated
> (we already have similar checks for macro expansions). What do you think
> about such approach?
Ok, in that case I think easiest would the following (I hit the same issue
when writing the -Wtautological-compare patch):
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2015-07-14 Marek Polacek <pola...@redhat.com>
PR c++/66572
* pt.c (tsubst_copy_and_build): Add warn_logical_op sentinel.
* g++.dg/warn/Wlogical-op-2.C: New test.
diff --git gcc/cp/pt.c gcc/cp/pt.c
index 2097963..0c9712a 100644
--- gcc/cp/pt.c
+++ gcc/cp/pt.c
@@ -14893,6 +14893,7 @@ tsubst_copy_and_build (tree t,
{
warning_sentinel s1(warn_type_limits);
warning_sentinel s2(warn_div_by_zero);
+ warning_sentinel s3(warn_logical_op);
tree op0 = RECUR (TREE_OPERAND (t, 0));
tree op1 = RECUR (TREE_OPERAND (t, 1));
tree r = build_x_binary_op
diff --git gcc/testsuite/g++.dg/warn/Wlogical-op-2.C
gcc/testsuite/g++.dg/warn/Wlogical-op-2.C
index e69de29..755db08 100644
--- gcc/testsuite/g++.dg/warn/Wlogical-op-2.C
+++ gcc/testsuite/g++.dg/warn/Wlogical-op-2.C
@@ -0,0 +1,30 @@
+// PR c++/66572
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wlogical-op" }
+
+struct false_type
+{
+ static constexpr bool value = false;
+};
+
+struct true_type
+{
+ static constexpr bool value = true;
+};
+
+template<typename T>
+struct is_unsigned : false_type {};
+
+template<>
+struct is_unsigned<unsigned> : true_type {};
+
+template<typename T1, typename T2>
+bool foo()
+{
+ return is_unsigned<T1>::value && is_unsigned<T2>::value;
+}
+
+int main()
+{
+ foo<unsigned, unsigned>();
+}
Marek
