This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: C++ PATCH for c++/86608, reading constexpr volatile variable


OK.
On Tue, Dec 11, 2018 at 1:35 PM Marek Polacek <polacek@redhat.com> wrote:
>
> On Tue, Dec 11, 2018 at 10:26:00AM -0500, Jason Merrill wrote:
> > On 12/10/18 8:48 PM, Marek Polacek wrote:
> > > A template-argument for a non-type template-parameter shall be a converted
> > > constant expression.  But an lvalue-to-rvalue conversion applied to a volatile
> > > glvalue is not allowed to be part of the evaluation of a constant expression.
> > > So this test should be rejected.
> >
> > It occurred to me after my note on IRC that the
> > potential_constant_expression_1 test we were talking about,
> >
> >   if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
> >
> > ought to test want_rval rather than !DECL_P so that we consistently reject
> > the lvalue-to-rvalue conversion, and not other uses of a volatile lvalue.
> > And the diagnostic ought to talk about that rather than "side-effects".
>
> Done.
>
> > It might still be appropriate to change non_const_var_error, but I'd think
> > it could check TREE_THIS_VOLATILE itself, rather than the caller; I don't
> > see a need for the two calls to differ in their handling of volatile
> > variables.
>
> This change was no longer needed...
>
> > Perhaps decl_maybe_constant_var_p should return false for constexpr
> > volatile, as well.
>
> ...neither was this but I did it all the same because I think it's reasonable.
>
> Like I said, not planning to backport it, but I could be easily convinced.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2018-12-11  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/86608 - reading constexpr volatile variable.
>         * constexpr.c (potential_constant_expression_1): Check want_rval
>         instead of checking if we have a decl.
>         * decl2.c (decl_maybe_constant_var_p): Don't consider volatile
>         constexpr variables as maybe constant.
>
>         * g++.dg/cpp0x/constexpr-volatile2.C: New test.
>         * g++.dg/cpp0x/pr65327.C: Add dg-error.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 1c844a8c2ef..44db38029bf 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -5476,10 +5476,11 @@ potential_constant_expression_1 (tree t, bool want_rval, bool strict, bool now,
>         available, so we don't bother with switch tracking.  */
>      return true;
>
> -  if (TREE_THIS_VOLATILE (t) && !DECL_P (t))
> +  if (TREE_THIS_VOLATILE (t) && want_rval)
>      {
>        if (flags & tf_error)
> -        error_at (loc, "expression %qE has side-effects", t);
> +       error_at (loc, "lvalue-to-rvalue conversion of a volatile lvalue "
> +                 "%qE with type %qT", t, TREE_TYPE (t));
>        return false;
>      }
>    if (CONSTANT_CLASS_P (t))
> diff --git gcc/cp/decl2.c gcc/cp/decl2.c
> index a8bf28a0cd9..1b3e758b625 100644
> --- gcc/cp/decl2.c
> +++ gcc/cp/decl2.c
> @@ -4313,7 +4313,7 @@ decl_maybe_constant_var_p (tree decl)
>    tree type = TREE_TYPE (decl);
>    if (!VAR_P (decl))
>      return false;
> -  if (DECL_DECLARED_CONSTEXPR_P (decl))
> +  if (DECL_DECLARED_CONSTEXPR_P (decl) && !TREE_THIS_VOLATILE (decl))
>      return true;
>    if (DECL_HAS_VALUE_EXPR_P (decl))
>      /* A proxy isn't constant.  */
> diff --git gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
> new file mode 100644
> index 00000000000..0def8d73731
> --- /dev/null
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-volatile2.C
> @@ -0,0 +1,13 @@
> +// PR c++/86608
> +// { dg-do compile { target c++11 } }
> +
> +template<typename T, T v> struct X {};
> +
> +int
> +main ()
> +{
> +  static constexpr volatile int a = 3;
> +  constexpr volatile int b = 2;
> +  return (sizeof(X<decltype(a), a>) // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
> +         + sizeof(X<decltype(b), b>)); // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }
> +}
> diff --git gcc/testsuite/g++.dg/cpp0x/pr65327.C gcc/testsuite/g++.dg/cpp0x/pr65327.C
> index c6cefaba692..5176b3c3204 100644
> --- gcc/testsuite/g++.dg/cpp0x/pr65327.C
> +++ gcc/testsuite/g++.dg/cpp0x/pr65327.C
> @@ -15,4 +15,4 @@ constexpr volatile int
>  bar ()
>  {
>    return i;
> -}
> +} // { dg-error "lvalue-to-rvalue conversion of a volatile lvalue" }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]