Re: [Patch, Fortran] PR fortran/37425: Finish type-bound operators

[Tobias Burnus <burnus at net-b dot de>]

Dear Daniel,

thanks for your patch!

Daniel Kraft wrote:
> * What's the usage of EXEC_ASSIGN_CALL instead of EXEC_CALL?
It was introduced with PR25746 - and there with the patch "[Patch,
fortran] PR25746 - operator assignment dependency checking",
http://gcc.gnu.org/ml/gcc-patches/2006-05/msg00296.html.

That patch introduces the distinction in gfc_trans_call whether a
dependency checking should be done (EXEC_ASSIGN_CALL) or not (EXEC_CALL).

> * Should we do ambiguity-checks while looking for matching operators?
> Like, continue always through-out all checks to see if we would have
> found another match?  Compare PR 37297, maybe we could leave that open
> but keep in mind to do something about it later.

I think we should defer it to a later patch / to a PR, but I think we
should do it. Depending on the order of USE statements or type
declarations we may use a different operator; such bugs are quite
difficult to find in user code.


+/* See if the arglist to an operator-call contains a derived-type argument
+   with a matching type-bound operator.  If so, return the matching specific
+   procedure defined as operator-target as well as the base-object to use
+   (which is the found derived-type argument with operator).  */
+
+static gfc_typebound_proc*
+matching_typebound_op (gfc_expr** tb_base, gfc_try* result,
[...]
+	if (result && *result == FAILURE)
+	  return NULL;


Can you add a word or so regarding the purpose of "result"? It obviously
does not just return a SUCCESS or FAILURE, but its presence might also
alter the program flow - depending whether it is present or not.


+   already_error is an additional output argument that specifies if FAILURE
+   is because of some real error and not because no match was found.  */

+gfc_extend_expr (gfc_expr *e, bool *already_error)


I would have guessed something different from the name than the comment
implies. Something like "real_error" or "not_matched" fits better my
expectations from the name with those of the comment.

Ditto in resolve.c

+! FIXME: Check that calls to inherited bindnigs work once CLASS allows that.

Typo: bindings


+! Here we see it also works for imported from module.


When reading "imported" I always thinks of the IMPORT statement thus I
would use "Here, we see that also use-associated operators work". But as
it is merely a comment in a test case, it really does not matter.

Otherwise, the patch looks okay to me.

Tobias