Questions about pedantic_non_lvalue_loc and related code

["Bin.Cheng" <amker dot cheng at gmail dot com>]

Hi,
I am trying to move simplifications in fold_cond_expr_with_comparison
to match.pd and had below questions about pedantic_non_lvalue_loc.

Last change for the function is:

commit 565353e8cf24046c034ecaf04dcb69ea8319a57c
Author: rguenth <rguenth@138bc75d-0d04-0410-961f-82ee72b054a4>
Date:   Tue Nov 11 15:21:12 2014 +0000

    2014-11-11  Richard Biener  <rguenther@suse.de>

        * tree-core.h (pedantic_lvalues): Remove.
        * fold-const.c (pedantic_lvalues): Likewise.
        (pedantic_non_lvalue_loc): Remove conditional non_lvalue_loc call.

        c/
        * c-decl.c (c_init_decl_processing): Do not set pedantic_lvalues
        to true.

It removes call to non_lvalue_loc in pedantic_non_lvalue_loc.  IIUC,
pedantic_lvalues was introduced because of Generic Lvalue extension
originally, and it is set to true after that feature was removed long
ago.  So before the aforementioned commit, pedantic_non_lvalue_loc is
like:
 static tree
 pedantic_non_lvalue_loc (location_t loc, tree x)
 {
   if (pedantic_lvalues)
     return non_lvalue_loc (loc, x);

   return protected_set_expr_location_unshare (x, loc);
 }

The net effect of this function is actually always returning what
non_lvalue_loc returns, it should be changed into below?
 static tree
 pedantic_non_lvalue_loc (location_t loc, tree x)
 {
   return non_lvalue_loc (loc, x);
 }
Given the change is made in 2014 and not causing any issue after it, I
can only assuming two possible reasons:
A) expressions passed to pedantic_non_lvalue_loc will never be lvalue,
thus no need to wrap it with NON_LVALUE by calling non_lvalue_loc.
B) expressions passed to pedantic_non_lvalue_loc could be lvalue, and
the lvalue is kept by not calling non_lvalue_loc as it is now.

Question 1) is, which one of above is correct?  After going through
all calls to pedantic_non_lvalue_loc, I tend to agree A) is correct.
Most calls in fold_cond_expr_with_comparison are lvalue-safe, and the
only exception is now also guarded by fix from PR19199.

Well, there are two calls outside of fold_cond_expr_with_comparison
which I am not sure: The one in fold_binary_loc for COMPOUND_EXPR, the
other one in fold_tenary_loc for COND_EXPR, the two calls are both
(somehow) guarded by !TREE_SIDE_EFFECTS, for example:
    case COMPOUND_EXPR:
      /* When pedantic, a compound expression can be neither an lvalue
     nor an integer constant expression.  */
      if (TREE_SIDE_EFFECTS (arg0) || TREE_CONSTANT (arg1))
    return NULL_TREE;
      /* Don't let (0, 0) be null pointer constant.  */
      tem = integer_zerop (arg1) ? build1 (NOP_EXPR, type, arg1)
                 : fold_convert_loc (loc, type, arg1);
      return pedantic_non_lvalue_loc (loc, tem);

The comment leads me to
Question 2) Does this mean a lvalue expression must have side_effects_flag set?
If answer is yes, then question 1.A) will hold.

Get back to current implementation of pedantic_non_lvalue_loc:
static tree
pedantic_non_lvalue_loc (location_t loc, tree x)
{
  return protected_set_expr_location_unshare (x, loc);
}

Function protected_set_expr_location_unshare was introduced by
PR45700.  It I read it correctly, the change is to, when necessarily,
set different loc information only after copying the original
expression.  Again if Question 2) is a yes, pedantic_non_lvalue_loc
can be cleaned up, based on the code before 2014's change.  Even
Question 2) is not always yes, calls to pedantic_non_lvalue_loc in
fold_cond_expr_with_comparison are all dead because it's for sure the
expression can't be lvalue.

So any explanation?  Thanks very much in advance.

Thanks,
bin