Re: [PATCH] v2 Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Bernhard Reutner-Fischer]

On December 9, 2015 2:07:05 AM GMT+01:00, David Malcolm  
wrote:

>I can't comment on Mikael's observations, but here's an updated version
>of Bernhard's patch which moves the duplicated code into a new
>"find_closest_string" function in gcc/spellcheck.c.  
>With that, the lookup_*_fuzzy functions are all of the form:
>
>{
>  auto_vec  candidates;
>
>  /* call something to populate candidates e.g.: */
>  lookup_function_fuzzy_find_candidates (fun, );
>
>  return find_closest_string (fn, );
>}
>
>where, as before, the auto_vec is implicitly cleaned up via a
>C++ destructor as the function exits.  Hopefully with this change it
>reduces the amount of proposed C++ in the fortran subdirectory to an
>palatable amount.
>
>That's all I did; I didn't address the other issues seen in this thread
>(e.g. Mikael's notes above).
>
>Not yet well-tested; it compiles and passes the new test cases; I'm
>posting it here in case someone more familiar with the Fortran FE wants
>to take this forward (Bernhard?)

I have rewritten the autovec to plain c, will send an updated patch including 
current comments and maybe the parameter handling as suggested by Joost when 
done.

Thanks,
>
>Hope this is constructive
>Dave

Re: [PATCH] v2 Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Tobias Burnus]

David Malcolm wrote:
> On Sat, 2015-12-05 at 20:53 +0100, Mikael Morin wrote:
> > to get things moving again, a few comments on top of David Malcolm's:
[...]
> > It seems you are considering some candidates more than once here.
[...]
> > You have to start the lookup with the current namespace's sym_root (not 
> > with fun), otherwise you'll miss some candidates.
> > You may also want to query parent namespaces for host-associated symbols.
[...]

I think the current patch doesn't not address those (as stated) and I think
that some suggestions should honour the attributes better (variable vs.
subroutine vs. function etc.). But I very much like the general patch.

Regarding Malcolm's update:
> I can't comment on Mikael's observations, but here's an updated version
> of Bernhard's patch which moves the duplicated code into a new
> "find_closest_string" function in gcc/spellcheck.c.

That change looks good to me.

BTW: I think you should write a quip for https://gcc.gnu.org/gcc-6/changes.html

Tobias

PS: Talking about the release notes, my feeling is that both the wiki and
the release notes miss some changes, but I have to admit that I am really
out of sync. It currently only lists Submodules at the Wiki,
   https://gcc.gnu.org/wiki/GFortran/News#GCC6
and https://gcc.gnu.org/gcc-6/changes.html has a few other items. (Both
should be synced crosswise.) As additional item, I know of coarray events
but there must be more items.

[PATCH] v2 Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[David Malcolm]

On Sat, 2015-12-05 at 20:53 +0100, Mikael Morin wrote:
> Hello,
> 
> to get things moving again, a few comments on top of David Malcolm's:
> 
> Le 01/12/2015 13:55, Bernhard Reutner-Fischer a écrit :
> >
> > David Malcolm nice Levenshtein distance spelling check helpers
> > were used in some parts of other frontends. This proposed patch adds
> > some spelling corrections to the fortran frontend.
> >
> > Suggestions are printed if we can find a suitable name, currently
> > perusing a very simple cutoff factor:
> > /* If more than half of the letters were misspelled, the suggestion is
> > likely to be meaningless.  */
> > cutoff = MAX (strlen (typo), strlen (best_guess)) / 2;
> > which effectively skips names with less than 4 characters.
> > For e.g. structures, one could try to be much smarter in an attempt to
> > also provide suggestions for single-letter members/components.
> >
> > This patch covers (at least partly):
> > - user-defined operators
> > - structures (types and their components)
> > - functions
> > - symbols (variables)
> >
> > I do not immediately see how to handle subroutines. Ideas?
> >
> Not sure what you are looking for; I can get an error generated in 
> gfc_procedure_use if using IMPLICIT NONE (EXTERNAL)
> 
> > If anybody has a testcase where a spelling-suggestion would make sense
> > then please pass it along so we maybe can add support for GCC-7.
> >
> 
> 
> > diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
> > index 685e3f5..6e1f63c 100644
> > --- a/gcc/fortran/resolve.c
> > +++ b/gcc/fortran/resolve.c
> > @@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "data.h"
> >   #include "target-memory.h" /* for gfc_simplify_transfer */
> >   #include "constructor.h"
> > +#include "spellcheck.h"
> >
> >   /* Types used in equivalence statements.  */
> >
> > @@ -2682,6 +2683,61 @@ resolve_specific_f (gfc_expr *expr)
> > return true;
> >   }
> >
> > +/* Recursively append candidate SYM to CANDIDATES.  */
> > +
> > +static void
> > +lookup_function_fuzzy_find_candidates (gfc_symtree *sym,
> > +   vec *candidates)
> > +{
> > +  gfc_symtree *p;
> > +  for (p = sym->right; p; p = p->right)
> > +{
> > +  lookup_function_fuzzy_find_candidates (p, candidates);
> > +  if (p->n.sym->ts.type != BT_UNKNOWN)
> > +   candidates->safe_push (p->name);
> > +}
> > +  for (p = sym->left; p; p = p->left)
> > +{
> > +  lookup_function_fuzzy_find_candidates (p, candidates);
> > +  if (p->n.sym->ts.type != BT_UNKNOWN)
> > +   candidates->safe_push (p->name);
> > +}
> > +}
> 
> It seems you are considering some candidates more than once here.
> The first time through the recursive call you will consider say 
> sym->right->right, and with the loop, you'll consider it again after 
> returning from the recursive call.
> The usual way to traverse the whole tree is to handle the current 
> pointer and recurse on left and right pointers.  So without loop.
> There is gfc_traverse_ns that you might find handy to do that (no 
> obligation).
> 
> Same goes for the user operators below.
> 
> > +
> > +
> > +/* Lookup function FN fuzzily, taking names in FUN into account.  */
> > +
> > +const char*
> > +gfc_lookup_function_fuzzy (const char *fn, gfc_symtree *fun)
> > +{
> > +  auto_vec  candidates;
> > +  lookup_function_fuzzy_find_candidates (fun, );
> 
> You have to start the lookup with the current namespace's sym_root (not 
> with fun), otherwise you'll miss some candidates.
> You may also want to query parent namespaces for host-associated symbols.
> 
> > +
> > +  /* Determine closest match.  */
> > +  int i;
> > +  const char *name, *best = NULL;
> > +  edit_distance_t best_distance = MAX_EDIT_DISTANCE;
> > +
> 
> [...]
> 
> > diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
> > index ff9aff9..212f7d8 100644
> > --- a/gcc/fortran/symbol.c
> > +++ b/gcc/fortran/symbol.c
> > @@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
> >   #include "parse.h"
> >   #include "match.h"
> >   #include "constructor.h"
> > +#include "spellcheck.h"
> >
> >
> >   /* Strings for all symbol attributes.  We use these for dumping the
> > @@ -235,6 +236,62 @@ gfc_get_default_type (const char *name, gfc_namespace 
> > *ns)
> >   }
> >
> >
> > +/* Recursively append candidate SYM to CANDIDATES.  */
> > +
> > +static void
> > +lookup_symbol_fuzzy_find_candidates (gfc_symtree *sym,
> > +   vec *candidates)
> > +{
> > +  gfc_symtree *p;
> > +  for (p = sym->right; p; p = p->right)
> > +{
> > +  lookup_symbol_fuzzy_find_candidates (p, candidates);
> > +  if (p->n.sym->ts.type != BT_UNKNOWN)
> > +   candidates->safe_push (p->name);
> > +}
> > +  for (p = sym->left; p; p = p->left)
> > +{
> > +  lookup_symbol_fuzzy_find_candidates (p, candidates);
> > +  if (p->n.sym->ts.type != BT_UNKNOWN)
> > +   candidates->safe_push (p->name);
> > +}
> > +}
> 

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Mikael Morin]

Hello,

to get things moving again, a few comments on top of David Malcolm's:

Le 01/12/2015 13:55, Bernhard Reutner-Fischer a écrit :


David Malcolm nice Levenshtein distance spelling check helpers
were used in some parts of other frontends. This proposed patch adds
some spelling corrections to the fortran frontend.

Suggestions are printed if we can find a suitable name, currently
perusing a very simple cutoff factor:
/* If more than half of the letters were misspelled, the suggestion is
likely to be meaningless.  */
cutoff = MAX (strlen (typo), strlen (best_guess)) / 2;
which effectively skips names with less than 4 characters.
For e.g. structures, one could try to be much smarter in an attempt to
also provide suggestions for single-letter members/components.

This patch covers (at least partly):
- user-defined operators
- structures (types and their components)
- functions
- symbols (variables)

I do not immediately see how to handle subroutines. Ideas?

Not sure what you are looking for; I can get an error generated in 
gfc_procedure_use if using IMPLICIT NONE (EXTERNAL)



If anybody has a testcase where a spelling-suggestion would make sense
then please pass it along so we maybe can add support for GCC-7.





diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 685e3f5..6e1f63c 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "data.h"
  #include "target-memory.h" /* for gfc_simplify_transfer */
  #include "constructor.h"
+#include "spellcheck.h"

  /* Types used in equivalence statements.  */

@@ -2682,6 +2683,61 @@ resolve_specific_f (gfc_expr *expr)
return true;
  }

+/* Recursively append candidate SYM to CANDIDATES.  */
+
+static void
+lookup_function_fuzzy_find_candidates (gfc_symtree *sym,
+   vec *candidates)
+{
+  gfc_symtree *p;
+  for (p = sym->right; p; p = p->right)
+{
+  lookup_function_fuzzy_find_candidates (p, candidates);
+  if (p->n.sym->ts.type != BT_UNKNOWN)
+   candidates->safe_push (p->name);
+}
+  for (p = sym->left; p; p = p->left)
+{
+  lookup_function_fuzzy_find_candidates (p, candidates);
+  if (p->n.sym->ts.type != BT_UNKNOWN)
+   candidates->safe_push (p->name);
+}
+}


It seems you are considering some candidates more than once here.
The first time through the recursive call you will consider say 
sym->right->right, and with the loop, you'll consider it again after 
returning from the recursive call.
The usual way to traverse the whole tree is to handle the current 
pointer and recurse on left and right pointers.  So without loop.
There is gfc_traverse_ns that you might find handy to do that (no 
obligation).


Same goes for the user operators below.


+
+
+/* Lookup function FN fuzzily, taking names in FUN into account.  */
+
+const char*
+gfc_lookup_function_fuzzy (const char *fn, gfc_symtree *fun)
+{
+  auto_vec  candidates;
+  lookup_function_fuzzy_find_candidates (fun, );


You have to start the lookup with the current namespace's sym_root (not 
with fun), otherwise you'll miss some candidates.

You may also want to query parent namespaces for host-associated symbols.


+
+  /* Determine closest match.  */
+  int i;
+  const char *name, *best = NULL;
+  edit_distance_t best_distance = MAX_EDIT_DISTANCE;
+


[...]


diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index ff9aff9..212f7d8 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
  #include "parse.h"
  #include "match.h"
  #include "constructor.h"
+#include "spellcheck.h"


  /* Strings for all symbol attributes.  We use these for dumping the
@@ -235,6 +236,62 @@ gfc_get_default_type (const char *name, gfc_namespace *ns)
  }


+/* Recursively append candidate SYM to CANDIDATES.  */
+
+static void
+lookup_symbol_fuzzy_find_candidates (gfc_symtree *sym,
+   vec *candidates)
+{
+  gfc_symtree *p;
+  for (p = sym->right; p; p = p->right)
+{
+  lookup_symbol_fuzzy_find_candidates (p, candidates);
+  if (p->n.sym->ts.type != BT_UNKNOWN)
+   candidates->safe_push (p->name);
+}
+  for (p = sym->left; p; p = p->left)
+{
+  lookup_symbol_fuzzy_find_candidates (p, candidates);
+  if (p->n.sym->ts.type != BT_UNKNOWN)
+   candidates->safe_push (p->name);
+}
+}

This looks like the same as lookup_function_fuzzy_find_candidates, isn't it?
Maybe have a general symbol traversal function with a selection callback 
argument to test whether the symbol is what you want, depending on the 
context (is it a function? a subroutine? etc).



+
+
+/* Lookup symbol SYM fuzzily, taking names in SYMBOL into account.  */
+
+static const char*
+lookup_symbol_fuzzy (const char *sym, gfc_symbol *symbol)
+{
+  auto_vec  candidates;
+  lookup_symbol_fuzzy_find_candidates (symbol->ns->sym_root, 

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Janne Blomqvist]

On Tue, Dec 1, 2015 at 7:51 PM, Bernhard Reutner-Fischer
 wrote:
> As said, we could as well use a list of candidates with NULL as record marker.
> Implementation cosmetics. Steve seems to not be thrilled by the
> overall idea in the first place, so unless there is clear support by
> somebody else i won't pursue this any further, it's not that i'm bored
> or ran out of stuff i should do.. ;)

FWIW, I think the idea of this patch is quite nice, and I'd like to
see it in the compiler.

I'm personally Ok with "C++-isms", but nowadays my contributions are
so minor that my opinion shouldn't carry that much weight on this
matter.


-- 
Janne Blomqvist

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Mikael Morin]

Le 03/12/2015 10:29, Janne Blomqvist a écrit :

On Tue, Dec 1, 2015 at 7:51 PM, Bernhard Reutner-Fischer
 wrote:

As said, we could as well use a list of candidates with NULL as record marker.
Implementation cosmetics. Steve seems to not be thrilled by the
overall idea in the first place, so unless there is clear support by
somebody else i won't pursue this any further, it's not that i'm bored
or ran out of stuff i should do.. ;)


FWIW, I think the idea of this patch is quite nice, and I'd like to
see it in the compiler.


I like this feature as well.


I'm personally Ok with "C++-isms", but nowadays my contributions are
so minor that my opinion shouldn't carry that much weight on this
matter.


Same here.
David Malcolm suggested to move the candidate selection code to the 
common middle-end infrastructure, which would move half of the so-called 
"bloat" there.  Steve, would that work for you?


It seems to me that the remaining C++-isms are rather acceptable.
I do agree that the vec implementation details seem overly complex for 
something whose job is just the memory management of a growing (or 
shrinking) vector.  However, the API is consistent and self-explanatory, 
and the usage of it that is made here (just a few "safe_push") is not 
more complex than what would be done with a C-only API.


Mikael

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Steve Kargl]

On Thu, Dec 03, 2015 at 02:53:06PM +0100, Mikael Morin wrote:
> Le 03/12/2015 10:29, Janne Blomqvist a écrit :
> > On Tue, Dec 1, 2015 at 7:51 PM, Bernhard Reutner-Fischer
> >  wrote:
> >> As said, we could as well use a list of candidates with NULL as record 
> >> marker.
> >> Implementation cosmetics. Steve seems to not be thrilled by the
> >> overall idea in the first place, so unless there is clear support by
> >> somebody else i won't pursue this any further, it's not that i'm bored
> >> or ran out of stuff i should do.. ;)
> >
> > FWIW, I think the idea of this patch is quite nice, and I'd like to
> > see it in the compiler.
> >
> I like this feature as well.
> 
> > I'm personally Ok with "C++-isms", but nowadays my contributions are
> > so minor that my opinion shouldn't carry that much weight on this
> > matter.
> >
> Same here.
> David Malcolm suggested to move the candidate selection code to the 
> common middle-end infrastructure, which would move half of the so-called 
> "bloat" there.  Steve, would that work for you?

Fine with me.

When debugging, if I run into C++isms, I'll stop and move to
a new bug.  We certainly have enough open bugs to choose from. 

-- 
Steve

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Steve Kargl]

On Tue, Dec 01, 2015 at 12:58:28PM -0500, David Malcolm wrote:
> On Tue, 2015-12-01 at 18:51 +0100, Bernhard Reutner-Fischer wrote:
> > As said, we could as well use a list of candidates with NULL as record 
> > marker.
> > Implementation cosmetics. Steve seems to not be thrilled by the
> > overall idea in the first place, so unless there is clear support by
> > somebody else i won't pursue this any further, it's not that i'm bored
> > or ran out of stuff i should do.. ;)
> 
> (FWIW I liked the idea, but I'm not a Fortran person so my opinion
> counts much less that Steve's)
> 

Your opinion is as valid as mine.

My only concern is code maintenance.  Injection of C++ (or any
other language) into C code seems to add possible complications
when something needs to be fix or changed to accommodate a new
Fortran freature.

-- 
Steve

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[David Malcolm]

On Tue, 2015-12-01 at 13:55 +0100, Bernhard Reutner-Fischer wrote:
> gcc/fortran/ChangeLog
> 
> 2015-11-29  Bernhard Reutner-Fischer  
> 
>   * gfortran.h (gfc_lookup_function_fuzzy): New declaration.
>   * resolve.c: Include spellcheck.h.
>   (lookup_function_fuzzy_find_candidates): New static function.
>   (lookup_uop_fuzzy_find_candidates): Likewise.
>   (lookup_uop_fuzzy): Likewise.
>   (resolve_operator) : Call lookup_uop_fuzzy.
>   (gfc_lookup_function_fuzzy): New definition.
>   (resolve_unknown_f): Call gfc_lookup_function_fuzzy.
>   * interface.c (check_interface0): Likewise.
>   * symbol.c: Include spellcheck.h.
>   (lookup_symbol_fuzzy_find_candidates): New static function.
>   (lookup_symbol_fuzzy): Likewise.
>   (gfc_set_default_type): Call lookup_symbol_fuzzy.
>   (lookup_component_fuzzy_find_candidates): New static function.
>   (lookup_component_fuzzy): Likewise.
>   (gfc_find_component): Call lookup_component_fuzzy.
> 
> gcc/testsuite/ChangeLog
> 
> 2015-11-29  Bernhard Reutner-Fischer  
> 
>   * gfortran.dg/spellcheck-operator.f90: New testcase.
>   * gfortran.dg/spellcheck-procedure.f90: New testcase.
>   * gfortran.dg/spellcheck-structure.f90: New testcase.
> 
> ---
> 
> David Malcolm nice Levenshtein distance spelling check helpers
> were used in some parts of other frontends. This proposed patch adds
> some spelling corrections to the fortran frontend.
> 
> Suggestions are printed if we can find a suitable name, currently
> perusing a very simple cutoff factor:
> /* If more than half of the letters were misspelled, the suggestion is
>likely to be meaningless.  */
> cutoff = MAX (strlen (typo), strlen (best_guess)) / 2;
> which effectively skips names with less than 4 characters.
> For e.g. structures, one could try to be much smarter in an attempt to
> also provide suggestions for single-letter members/components.
> 
> This patch covers (at least partly):
> - user-defined operators
> - structures (types and their components)
> - functions
> - symbols (variables)
> 
> I do not immediately see how to handle subroutines. Ideas?
> 
> If anybody has a testcase where a spelling-suggestion would make sense
> then please pass it along so we maybe can add support for GCC-7.
> 
> Signed-off-by: Bernhard Reutner-Fischer 
> ---
>  gcc/fortran/gfortran.h |   1 +
>  gcc/fortran/interface.c|  16 ++-
>  gcc/fortran/resolve.c  | 135 
> -
>  gcc/fortran/symbol.c   | 129 +++-
>  gcc/testsuite/gfortran.dg/spellcheck-operator.f90  |  30 +
>  gcc/testsuite/gfortran.dg/spellcheck-procedure.f90 |  41 +++
>  gcc/testsuite/gfortran.dg/spellcheck-structure.f90 |  35 ++
>  7 files changed, 376 insertions(+), 11 deletions(-)
>  create mode 100644 gcc/testsuite/gfortran.dg/spellcheck-operator.f90
>  create mode 100644 gcc/testsuite/gfortran.dg/spellcheck-procedure.f90
>  create mode 100644 gcc/testsuite/gfortran.dg/spellcheck-structure.f90
> 
> diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
> index 5487c93..cbfd592 100644
> --- a/gcc/fortran/gfortran.h
> +++ b/gcc/fortran/gfortran.h
> @@ -3060,6 +3060,7 @@ bool gfc_type_is_extensible (gfc_symbol *);
>  bool gfc_resolve_intrinsic (gfc_symbol *, locus *);
>  bool gfc_explicit_interface_required (gfc_symbol *, char *, int);
>  extern int gfc_do_concurrent_flag;
> +const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *);
>  
> 
>  /* array.c */
> diff --git a/gcc/fortran/interface.c b/gcc/fortran/interface.c
> index 30cc522..19f800f 100644
> --- a/gcc/fortran/interface.c
> +++ b/gcc/fortran/interface.c
> @@ -1590,10 +1590,18 @@ check_interface0 (gfc_interface *p, const char 
> *interface_name)
> if (p->sym->attr.external)
>   gfc_error ("Procedure %qs in %s at %L has no explicit interface",
>  p->sym->name, interface_name, >sym->declared_at);
> -   else
> - gfc_error ("Procedure %qs in %s at %L is neither function nor "
> -"subroutine", p->sym->name, interface_name,
> -   >sym->declared_at);
> +   else {
> + const char *guessed
> +   = gfc_lookup_function_fuzzy (p->sym->name, p->sym->ns->sym_root);
> + if (guessed)
> +   gfc_error ("Procedure %qs in %s at %L is neither function nor "
> +  "subroutine; did you mean %qs?", p->sym->name,
> + interface_name, >sym->declared_at, guessed);
> + else
> +   gfc_error ("Procedure %qs in %s at %L is neither function nor "
> +  "subroutine", p->sym->name, interface_name,
> + >sym->declared_at);
> +   }
> return 1;
>   }
>  
> diff --git a/gcc/fortran/resolve.c 

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Bernhard Reutner-Fischer]

On 1 December 2015 at 17:41, Steve Kargl
 wrote:
> On Tue, Dec 01, 2015 at 05:12:57PM +0100, Bernhard Reutner-Fischer wrote:
>> On 1 December 2015 at 16:01, Steve Kargl
>>  wrote:
>> > On Tue, Dec 01, 2015 at 01:55:01PM +0100, Bernhard Reutner-Fischer wrote:
>> >>
>> >> David Malcolm nice Levenshtein distance spelling check helpers
>> >> were used in some parts of other frontends. This proposed patch adds
>> >> some spelling corrections to the fortran frontend.
>>
>> > What problem are you trying to solve here?  The patch looks like
>>
>> The idea is to improve the programmer experience when writing code.
>> See the testcases enclosed in the patch. I consider this a feature :)
>
> Opinions differ.  I consider it unnecessary bloat.

Fair enough.
I fully agree that it's bloat.

The compiler is so tremendously bloated by now anyway that i consider
these couple of kilobyte to have a nice bloat/user friendliness
factor, overall ;)
I can imagine that people code their fortran programs in an IDE (the
bloated variant of an editor, mine is ~20518 bytes of text, no data,
no bss) and IDEs will sooner or later support fixit-hints. Even the
console/terminal users might enjoy to safe them a cycle of opening a
different file, looking up the type/module/etc name and then going
back to the source-file to correct their typo. *I* would welcome that
sometimes for sure :)

>> > unneeded complexity with the result of injecting C++ idioms into
>> > the Fortran FE.
>>
>> What C++ idioms are you referring to? The autovec?
>> AFAIU the light use of C++ in GCC is deemed OK. I see usage of
>> std::swap and std::map in the FE, not to mention the wide-int uses
>> (wi::). Thus we don't have to realloc/strcat but can use vectors to
>> the same effect, just as other frontends, including the C frontend,
>> do.
>> I take it you remember that we had to change all "try" to something
>> C++ friendly. If the Fortran FE meant to opt-out of being compiled
>> with a C++ compiler in the first place, why were all the C++ clashes
>> rewritten, back then? :)
>
> Yes, I know there are other C++ (mis)features within the
> Fortran FE especially in the trans-*.c files.  Those are
> accepted (by some) as necessary evils to interface with
> the ME.  Your patch injects C++ into otherwise perfectly
> fine C code, which makes it more difficult for those with
> no or very limited C++ knowledge to maintain the gfortran.

So you're in favour of using realloc and strcat, ok. I can use that.
Let me see if ipa-icf can replace all the identical tails of the
lookup_*_fuzzy into a common helper.
Shouldn't rely on LTO anyway nor ipa-icf i suppose.

>
> There are currently 806 open bug reports for gfortran.
> AFAIK, your patch does not address any of those bug reports.

I admit i didn't look..

> The continued push to inject C++ into the Fortran FE will
> have the (un)intentional consequence of forcing at least one
> active gfortran contributor to stop.

That was not my intention for sure.

cheers,

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Bernhard Reutner-Fischer]

On 1 December 2015 at 18:28, David Malcolm  wrote:
> On Tue, 2015-12-01 at 13:55 +0100, Bernhard Reutner-Fischer wrote:


>> +/* Lookup function FN fuzzily, taking names in FUN into account.  */
>> +
>> +const char*
>> +gfc_lookup_function_fuzzy (const char *fn, gfc_symtree *fun)
>> +{
>> +  auto_vec  candidates;
>> +  lookup_function_fuzzy_find_candidates (fun, );
>> +
>> +  /* Determine closest match.  */
>> +  int i;
>> +  const char *name, *best = NULL;
>> +  edit_distance_t best_distance = MAX_EDIT_DISTANCE;
>> +
>> +  FOR_EACH_VEC_ELT (candidates, i, name)
>> +{
>> +  edit_distance_t dist = levenshtein_distance (fn, name);
>> +  if (dist < best_distance)
>> + {
>> +   best_distance = dist;
>> +   best = name;
>> + }
>> +}
>> +  /* If more than half of the letters were misspelled, the suggestion is
>> + likely to be meaningless.  */
>> +  if (best)
>> +{
>> +  unsigned int cutoff = MAX (strlen (fn), strlen (best)) / 2;
>> +  if (best_distance > cutoff)
>> + return NULL;
>> +}
>> +  return best;
>> +}
>
>
> Caveat: I'm not very familiar with the Fortran FE, so take the following
> with a pinch of salt.
>
> If I'm reading things right, here, and in various other places, you're
> building a vec of const char *, and then seeing which one of those
> candidates is the best match for another const char *.
>
> You could simplify things by adding a helper function to spellcheck.h,
> akin to this one:
>
> extern tree
> find_closest_identifier (tree target, const auto_vec *candidates);

I was hoping for ipa-icf to fix that up on my behalf. I'll try to see
if it does. Short of that: yes, should do that.

>
> This would reduce the amount of duplication in the patch (and slightly
> reduce the amount of C++).

As said, we could as well use a list of candidates with NULL as record marker.
Implementation cosmetics. Steve seems to not be thrilled by the
overall idea in the first place, so unless there is clear support by
somebody else i won't pursue this any further, it's not that i'm bored
or ran out of stuff i should do.. ;)
>
> [are there IDENTIFIER nodes in the Fortran FE, or is it all const char
> *? this would avoid some strlen calls]

Right, but in the Fortran FE these are const char*.

thanks for your comments!

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[David Malcolm]

On Tue, 2015-12-01 at 18:51 +0100, Bernhard Reutner-Fischer wrote:
> On 1 December 2015 at 18:28, David Malcolm  wrote:
> > On Tue, 2015-12-01 at 13:55 +0100, Bernhard Reutner-Fischer wrote:
> 
> 
> >> +/* Lookup function FN fuzzily, taking names in FUN into account.  */
> >> +
> >> +const char*
> >> +gfc_lookup_function_fuzzy (const char *fn, gfc_symtree *fun)
> >> +{
> >> +  auto_vec  candidates;
> >> +  lookup_function_fuzzy_find_candidates (fun, );
> >> +
> >> +  /* Determine closest match.  */
> >> +  int i;
> >> +  const char *name, *best = NULL;
> >> +  edit_distance_t best_distance = MAX_EDIT_DISTANCE;
> >> +
> >> +  FOR_EACH_VEC_ELT (candidates, i, name)
> >> +{
> >> +  edit_distance_t dist = levenshtein_distance (fn, name);
> >> +  if (dist < best_distance)
> >> + {
> >> +   best_distance = dist;
> >> +   best = name;
> >> + }
> >> +}
> >> +  /* If more than half of the letters were misspelled, the suggestion is
> >> + likely to be meaningless.  */
> >> +  if (best)
> >> +{
> >> +  unsigned int cutoff = MAX (strlen (fn), strlen (best)) / 2;
> >> +  if (best_distance > cutoff)
> >> + return NULL;
> >> +}
> >> +  return best;
> >> +}
> >
> >
> > Caveat: I'm not very familiar with the Fortran FE, so take the following
> > with a pinch of salt.
> >
> > If I'm reading things right, here, and in various other places, you're
> > building a vec of const char *, and then seeing which one of those
> > candidates is the best match for another const char *.
> >
> > You could simplify things by adding a helper function to spellcheck.h,
> > akin to this one:
> >
> > extern tree
> > find_closest_identifier (tree target, const auto_vec *candidates);
> 
> I was hoping for ipa-icf to fix that up on my behalf. I'll try to see
> if it does. Short of that: yes, should do that.

I was more thinking about code readability; don't rely on ipa-icf - fix
it in the source.

> > This would reduce the amount of duplication in the patch (and slightly
> > reduce the amount of C++).
> 
> As said, we could as well use a list of candidates with NULL as record marker.
> Implementation cosmetics. Steve seems to not be thrilled by the
> overall idea in the first place, so unless there is clear support by
> somebody else i won't pursue this any further, it's not that i'm bored
> or ran out of stuff i should do.. ;)

(FWIW I liked the idea, but I'm not a Fortran person so my opinion
counts much less that Steve's)

> > [are there IDENTIFIER nodes in the Fortran FE, or is it all const char
> > *? this would avoid some strlen calls]
> 
> Right, but in the Fortran FE these are const char*.
> 
> thanks for your comments!

RE: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[VandeVondele Joost]

So, I have tested the patch, it seems to work well.

I would really like to see this feature in the compiler, I'm sure it will help 
people developing Fortran code.

I have already an enhancement request, catching the name of 'Keyword argument' :

> cat test.f90
MODULE test
CONTAINS
  SUBROUTINE foo(bar)
 INTEGER :: bar
  END SUBROUTINE
END MODULE
USE test 
CALL foo(baz=1)
END

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Steve Kargl]

On Tue, Dec 01, 2015 at 06:34:57PM +0100, Bernhard Reutner-Fischer wrote:
> On 1 December 2015 at 17:41, Steve Kargl
> >
> > Yes, I know there are other C++ (mis)features within the
> > Fortran FE especially in the trans-*.c files.  Those are
> > accepted (by some) as necessary evils to interface with
> > the ME.  Your patch injects C++ into otherwise perfectly
> > fine C code, which makes it more difficult for those with
> > no or very limited C++ knowledge to maintain the gfortran.
> 
> So you're in favour of using realloc and strcat, ok. I can use that.
> Let me see if ipa-icf can replace all the identical tails of the
> lookup_*_fuzzy into a common helper.
> Shouldn't rely on LTO anyway nor ipa-icf i suppose.

Yes, I would prefer it, but certainly won't demand it.
There are other Fortran contributors/maintainers.  They
may prefer you approach, so give them time to speak up.

-- 
Steve

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[VandeVondele Joost]

Today, I ran 'gfortran -static-libfortran test.f90' and was very pleased with 
the answer:

gfortran: error: unrecognized command line option ‘-static-libfortran’; did you 
mean ‘-static-libgfortran’?

So thanks David, and hopefully we get this user experience for the FE as well.

Joost

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Steve Kargl]

On Tue, Dec 01, 2015 at 01:55:01PM +0100, Bernhard Reutner-Fischer wrote:
> 
> David Malcolm nice Levenshtein distance spelling check helpers
> were used in some parts of other frontends. This proposed patch adds
> some spelling corrections to the fortran frontend.
> 
> Suggestions are printed if we can find a suitable name, currently
> perusing a very simple cutoff factor:
> /* If more than half of the letters were misspelled, the suggestion is
>likely to be meaningless.  */
> cutoff = MAX (strlen (typo), strlen (best_guess)) / 2;
> which effectively skips names with less than 4 characters.
> For e.g. structures, one could try to be much smarter in an attempt to
> also provide suggestions for single-letter members/components.
> 
> This patch covers (at least partly):
> - user-defined operators
> - structures (types and their components)
> - functions
> - symbols (variables)
> 
> I do not immediately see how to handle subroutines. Ideas?
> 
> If anybody has a testcase where a spelling-suggestion would make sense
> then please pass it along so we maybe can add support for GCC-7.
> 

What problem are you trying to solve here?  The patch looks like
unneeded complexity with the result of injecting C++ idioms into
the Fortran FE.

-- 
Steve

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Bernhard Reutner-Fischer]

On 1 December 2015 at 16:01, Steve Kargl
 wrote:
> On Tue, Dec 01, 2015 at 01:55:01PM +0100, Bernhard Reutner-Fischer wrote:
>>
>> David Malcolm nice Levenshtein distance spelling check helpers
>> were used in some parts of other frontends. This proposed patch adds
>> some spelling corrections to the fortran frontend.

> What problem are you trying to solve here?  The patch looks like

The idea is to improve the programmer experience when writing code.
See the testcases enclosed in the patch. I consider this a feature :)

> unneeded complexity with the result of injecting C++ idioms into
> the Fortran FE.

What C++ idioms are you referring to? The autovec?
AFAIU the light use of C++ in GCC is deemed OK. I see usage of
std::swap and std::map in the FE, not to mention the wide-int uses
(wi::). Thus we don't have to realloc/strcat but can use vectors to
the same effect, just as other frontends, including the C frontend,
do.
I take it you remember that we had to change all "try" to something
C++ friendly. If the Fortran FE meant to opt-out of being compiled
with a C++ compiler in the first place, why were all the C++ clashes
rewritten, back then? :)

thanks,

Re: [PATCH] RFC: Use Levenshtein spelling suggestions in Fortran FE

[Steve Kargl]

On Tue, Dec 01, 2015 at 05:12:57PM +0100, Bernhard Reutner-Fischer wrote:
> On 1 December 2015 at 16:01, Steve Kargl
>  wrote:
> > On Tue, Dec 01, 2015 at 01:55:01PM +0100, Bernhard Reutner-Fischer wrote:
> >>
> >> David Malcolm nice Levenshtein distance spelling check helpers
> >> were used in some parts of other frontends. This proposed patch adds
> >> some spelling corrections to the fortran frontend.
> 
> > What problem are you trying to solve here?  The patch looks like
> 
> The idea is to improve the programmer experience when writing code.
> See the testcases enclosed in the patch. I consider this a feature :)

Opinions differ.  I consider it unnecessary bloat.

> > unneeded complexity with the result of injecting C++ idioms into
> > the Fortran FE.
> 
> What C++ idioms are you referring to? The autovec?
> AFAIU the light use of C++ in GCC is deemed OK. I see usage of
> std::swap and std::map in the FE, not to mention the wide-int uses
> (wi::). Thus we don't have to realloc/strcat but can use vectors to
> the same effect, just as other frontends, including the C frontend,
> do.
> I take it you remember that we had to change all "try" to something
> C++ friendly. If the Fortran FE meant to opt-out of being compiled
> with a C++ compiler in the first place, why were all the C++ clashes
> rewritten, back then? :)

Yes, I know there are other C++ (mis)features within the
Fortran FE especially in the trans-*.c files.  Those are
accepted (by some) as necessary evils to interface with 
the ME.  Your patch injects C++ into otherwise perfectly
fine C code, which makes it more difficult for those with
no or very limited C++ knowledge to maintain the gfortran.

There are currently 806 open bug reports for gfortran.
AFAIK, your patch does not address any of those bug reports.
The continued push to inject C++ into the Fortran FE will
have the (un)intentional consequence of forcing at least one
active gfortran contributor to stop.

--  
Steve