From 26ad09eafd311c837e83f49f2b5048d468870bff Mon Sep 17 00:00:00 2001 From: Michael Lill Date: Tue, 3 Sep 2024 16:51:26 +0200 Subject: [PATCH 1/2] fe: fix check condition failure The check condition expects duplicate arg names error to have been raised but in this case the duplicate feature sneaks in via free type. fixes #3647 --- src/dev/flang/fe/SourceModule.java | 4 +-- tests/reg_issue3647/Makefile | 25 ++++++++++++++++ tests/reg_issue3647/reg_issue3647.fz | 30 +++++++++++++++++++ .../reg_issue3647.fz.expected_err | 21 +++++++++++++ .../reg_issue3647.fz.expected_out | 0 5 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 tests/reg_issue3647/Makefile create mode 100644 tests/reg_issue3647/reg_issue3647.fz create mode 100644 tests/reg_issue3647/reg_issue3647.fz.expected_err create mode 100644 tests/reg_issue3647/reg_issue3647.fz.expected_out diff --git a/src/dev/flang/fe/SourceModule.java b/src/dev/flang/fe/SourceModule.java index ad09321fc..272cad4fe 100644 --- a/src/dev/flang/fe/SourceModule.java +++ b/src/dev/flang/fe/SourceModule.java @@ -729,7 +729,7 @@ public void addTypeParameter(AbstractFeature outer, if (doi != null) { if (CHECKS) check - (!doi.containsKey(fn) || doi.get(fn).size() == 1 && doi.get(fn).getFirst() == typeParameter); + (Errors.any() || !doi.containsKey(fn) || doi.get(fn).size() == 1 && doi.get(fn).getFirst() == typeParameter); add(doi, fn, typeParameter); } } @@ -1010,7 +1010,7 @@ void addDeclaredInnerFeature(AbstractFeature outer, Feature f) } else { - if (existing instanceof Feature ef && ef.isArgument() && f.isArgument()) + if (existing instanceof Feature ef && ef.isArgument() && f.isArgument() && !f.isTypeParameter()) { // NYI: CLEANUP: there should not be two places where // similar error is raised. diff --git a/tests/reg_issue3647/Makefile b/tests/reg_issue3647/Makefile new file mode 100644 index 000000000..4b3c6760e --- /dev/null +++ b/tests/reg_issue3647/Makefile @@ -0,0 +1,25 @@ +# This file is part of the Fuzion language implementation. +# +# The Fuzion language implementation is free software: you can redistribute it +# and/or modify it under the terms of the GNU General Public License as published +# by the Free Software Foundation, version 3 of the License. +# +# The Fuzion language implementation is distributed in the hope that it will be +# useful, but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public +# License for more details. +# +# You should have received a copy of the GNU General Public License along with The +# Fuzion language implementation. If not, see . + + +# ----------------------------------------------------------------------- +# +# Tokiwa Software GmbH, Germany +# +# Source code of Fuzion test Makefile +# +# ----------------------------------------------------------------------- + +override NAME = reg_issue3647 +include ../simple.mk diff --git a/tests/reg_issue3647/reg_issue3647.fz b/tests/reg_issue3647/reg_issue3647.fz new file mode 100644 index 000000000..806540b68 --- /dev/null +++ b/tests/reg_issue3647/reg_issue3647.fz @@ -0,0 +1,30 @@ +# This file is part of the Fuzion language implementation. +# +# The Fuzion language implementation is free software: you can redistribute it +# and/or modify it under the terms of the GNU General Public License as published +# by the Free Software Foundation, version 3 of the License. +# +# The Fuzion language implementation is distributed in the hope that it will be +# useful, but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public +# License for more details. +# +# You should have received a copy of the GNU General Public License along with The +# Fuzion language implementation. If not, see . + + +# ----------------------------------------------------------------------- +# +# Tokiwa Software GmbH, Germany +# +# Source code of Fuzion test reg_issue3647 +# +# ----------------------------------------------------------------------- + +# NYI: BUG: should not display error twice. + +reg_issue3647 => + + Node(T Type, a Node T) ref is + + say (type_as_value (Node i32)) diff --git a/tests/reg_issue3647/reg_issue3647.fz.expected_err b/tests/reg_issue3647/reg_issue3647.fz.expected_err new file mode 100644 index 000000000..1643979b4 --- /dev/null +++ b/tests/reg_issue3647/reg_issue3647.fz.expected_err @@ -0,0 +1,21 @@ + +--CURDIR--/reg_issue3647.fz:28:23: error 1: Duplicate feature declaration + Node(T Type, a Node T) ref is +----------------------^ +Feature that was declared repeatedly: 'reg_issue3647.Node.T' +originally declared at --CURDIR--/reg_issue3647.fz:28:8: + Node(T Type, a Node T) ref is +-------^ +To solve this, consider renaming one of these two features, e.g., as 'Tʼ' (using a unicode modifier letter apostrophe 'ʼ' U+02BC) or adding an additional argument (e.g. '_ unit' for an ignored unit argument used only to disambiguate these two). + + +--CURDIR--/reg_issue3647.fz:28:8: error 2: Duplicate feature declaration + Node(T Type, a Node T) ref is +-------^ +Feature that was declared repeatedly: 'reg_issue3647.Node.T' +originally declared at --CURDIR--/reg_issue3647.fz:28:23: + Node(T Type, a Node T) ref is +----------------------^ +To solve this, consider renaming one of these two features, e.g., as 'Tʼ' (using a unicode modifier letter apostrophe 'ʼ' U+02BC) or adding an additional argument (e.g. '_ unit' for an ignored unit argument used only to disambiguate these two). + +2 errors. diff --git a/tests/reg_issue3647/reg_issue3647.fz.expected_out b/tests/reg_issue3647/reg_issue3647.fz.expected_out new file mode 100644 index 000000000..e69de29bb From bfd0574f2016d313e84dd224cf2c72503fac0236 Mon Sep 17 00:00:00 2001 From: Michael Lill Date: Mon, 9 Sep 2024 12:30:07 +0200 Subject: [PATCH 2/2] ast: duplicateFeatureDeclaration, deduplicate if a,b are switched around --- src/dev/flang/ast/AstErrors.java | 23 +++++++++++-------- src/dev/flang/fe/SourceModule.java | 4 ++-- tests/reg_issue3647/reg_issue3647.fz | 2 -- .../reg_issue3647.fz.expected_err | 16 +++---------- 4 files changed, 19 insertions(+), 26 deletions(-) diff --git a/src/dev/flang/ast/AstErrors.java b/src/dev/flang/ast/AstErrors.java index db2132869..dc47664b1 100644 --- a/src/dev/flang/ast/AstErrors.java +++ b/src/dev/flang/ast/AstErrors.java @@ -944,23 +944,28 @@ public static void repeatedInheritanceCannotBeResolved(SourcePosition pos, Abstr "To solve this, you could add a redefinition of " + sbnf(f1) + " to " + s(heir) + "."); } - public static void duplicateFeatureDeclaration(SourcePosition pos, AbstractFeature f, AbstractFeature existing) + public static void duplicateFeatureDeclaration(AbstractFeature a, AbstractFeature b) { // suppress error message if errors were reported already and any feature // involved is f_ERROR - if (!any() || (f != Types.f_ERROR && - f .outer() != Types.f_ERROR && - existing != Types.f_ERROR && - existing.outer() != Types.f_ERROR )) + if (!any() || (a != Types.f_ERROR && + a .outer() != Types.f_ERROR && + b != Types.f_ERROR && + b.outer() != Types.f_ERROR )) { - var of = f.isTypeFeature() ? f.typeFeatureOrigin() : f; - error(pos, + // report in source code order to avoid symmetric error with exchanged roles of f and existing + var cmpRes = a.pos().show().compareTo(b.pos().show())>0; + var aa = cmpRes ? a : b; + var bb = cmpRes ? b : a; + + var of = aa.isTypeFeature() ? aa.typeFeatureOrigin() : aa; + error(bb.pos(), "Duplicate feature declaration", "Feature that was declared repeatedly: " + s(of) + "\n" + - "originally declared at " + existing.pos().show() + "\n" + + "originally declared at " + aa.pos().show() + "\n" + "To solve this, consider renaming one of these two features, e.g., as " + sbn(of.featureName().baseNameHuman() + "ʼ") + " (using a unicode modifier letter apostrophe " + sbn("ʼ")+ " U+02BC) "+ - (f.isTypeFeature() + (aa.isTypeFeature() ? ("or changing it into a routine by returning a " + sbn("unit") + " result, i.e., adding " + sbn("unit") + " before " + code("is") + " or using " + code("=>") + " instead of "+ code("is") + ".") diff --git a/src/dev/flang/fe/SourceModule.java b/src/dev/flang/fe/SourceModule.java index 272cad4fe..1d05b2dad 100644 --- a/src/dev/flang/fe/SourceModule.java +++ b/src/dev/flang/fe/SourceModule.java @@ -1020,7 +1020,7 @@ void addDeclaredInnerFeature(AbstractFeature outer, Feature f) } else { - AstErrors.duplicateFeatureDeclaration(f.pos(), f, existing); + AstErrors.duplicateFeatureDeclaration(f, existing); } } } @@ -1901,7 +1901,7 @@ private void checkDuplicateFeatures(AbstractFeature outer, FeatureName fn, List< else { // NYI: if (!isInherited && !sameModule(f, outer)) - AstErrors.duplicateFeatureDeclaration(f1.pos(), f1, f2); + AstErrors.duplicateFeatureDeclaration(f1, f2); } } } diff --git a/tests/reg_issue3647/reg_issue3647.fz b/tests/reg_issue3647/reg_issue3647.fz index 806540b68..c956e461b 100644 --- a/tests/reg_issue3647/reg_issue3647.fz +++ b/tests/reg_issue3647/reg_issue3647.fz @@ -21,8 +21,6 @@ # # ----------------------------------------------------------------------- -# NYI: BUG: should not display error twice. - reg_issue3647 => Node(T Type, a Node T) ref is diff --git a/tests/reg_issue3647/reg_issue3647.fz.expected_err b/tests/reg_issue3647/reg_issue3647.fz.expected_err index 1643979b4..7a1437ccc 100644 --- a/tests/reg_issue3647/reg_issue3647.fz.expected_err +++ b/tests/reg_issue3647/reg_issue3647.fz.expected_err @@ -1,21 +1,11 @@ ---CURDIR--/reg_issue3647.fz:28:23: error 1: Duplicate feature declaration +--CURDIR--/reg_issue3647.fz:26:23: error 1: Duplicate feature declaration Node(T Type, a Node T) ref is ----------------------^ Feature that was declared repeatedly: 'reg_issue3647.Node.T' -originally declared at --CURDIR--/reg_issue3647.fz:28:8: +originally declared at --CURDIR--/reg_issue3647.fz:26:8: Node(T Type, a Node T) ref is -------^ To solve this, consider renaming one of these two features, e.g., as 'Tʼ' (using a unicode modifier letter apostrophe 'ʼ' U+02BC) or adding an additional argument (e.g. '_ unit' for an ignored unit argument used only to disambiguate these two). - ---CURDIR--/reg_issue3647.fz:28:8: error 2: Duplicate feature declaration - Node(T Type, a Node T) ref is --------^ -Feature that was declared repeatedly: 'reg_issue3647.Node.T' -originally declared at --CURDIR--/reg_issue3647.fz:28:23: - Node(T Type, a Node T) ref is -----------------------^ -To solve this, consider renaming one of these two features, e.g., as 'Tʼ' (using a unicode modifier letter apostrophe 'ʼ' U+02BC) or adding an additional argument (e.g. '_ unit' for an ignored unit argument used only to disambiguate these two). - -2 errors. +one error.