Re: [PATCH] fortran: revise non-char pointer handling in gfc_conv_constant

[Tobias Burnus]

Yuao Ma wrote:

It is indeed simpler. I have used this api to update the patch.

diff --git a/gcc/fortran/trans-const.cc b/gcc/fortran/trans-const.cc
index f70f36284a3..966a837c25c 100644
--- a/gcc/fortran/trans-const.cc
+++ b/gcc/fortran/trans-const.cc
@@ -444,6 +444,10 @@ gfc_conv_constant (gfc_se * se, gfc_expr * expr)
        if (expr->ts.type == BT_CHARACTER)
     gfc_conv_string_parameter (se);
        else
-   se->expr = gfc_build_addr_expr (NULL_TREE, se->expr);
+   {
+     se->expr
+       = gfc_build_addr_expr (NULL_TREE,
+                  gfc_trans_force_lval (&se->pre, se->expr));
+   }

This is mostly fine. Except:

* It seems as if the email program converted tabs to single
  spaces – or messed up whitespace in general? Unless this only
  my mailer.

* The { } are no longer needed (they are a leftover of version 1 of
the patch that had 3 lines of code and, hence, required them).

* I think it is more readable to use 2 lines, i.e.
        se->expr
          = gfc_build_addr_expr (NULL_TREE,
                                 gfc_trans_force_lval (&se->pre, se->expr));

i.e. put the 'gfc_' under the '('; the ';' is in column 76, which is <= 80.

Or alternatively:
        se->expr = gfc_build_addr_expr (NULL_TREE,
                     gfc_trans_force_lval (&se->pre, se->expr));

No idea what's better.

In any case the actual change is OK (→ LGTM) and can be committed,
taking the nits into account.

Thanks for the patch!

Tobias