Merge remote-tracking branch 'origin/fix_3359_abstract_missing_cause_…

…cryptic_error'

* origin/fix_3359_abstract_missing_cause_cryptic_error:
  tests: trick DFA to believe execution continues after call to abstract feature
  src: Fix constructor call source position in tests/abstractfeatures_negative
  air/dfa: Move reporting missing implementations of abstracts to DFA, fix #3359

src/dev/flang/air/Clazz.java

@@ -251,20 +251,6 @@ public void action(AbstractCase c)
   boolean _isCalledAsOuter = false;
 
 
-  /**
-   * If instances of this class are created, this gives a source code position
-   * that does create such an instance.  To be used in error messages.
-   */
-  HasSourcePosition _instantiationPos = null;
-
-
-  /**
-   * In case abstract methods are called on this, this lists the abstract
-   * methods that have been found to do so.
-   */
-  TreeSet<AbstractFeature> _abstractCalled = null;
-
-
   /**
    * Set of all heirs of this clazz.
    */
@@ -1215,20 +1201,6 @@ Clazz lookup(FeatureAndActuals fa,
             if (f != Types.f_ERROR && (af != null || !isEffectivelyAbstract(f)))
               {
                 var aaf = af != null ? af : f;
-                if (isEffectivelyAbstract(aaf)
-                    /* NYI: WORKAROUND!
-                    tests/lib_container_set fails without this:
-                    "Used abstract feature 'container.Set.type.new' is not implemented by 'container.Set.type'"
-                    */
-                   && !_type.feature().isTypeFeature() )
-                  {
-                    if (_abstractCalled == null)
-                      {
-                        _abstractCalled = new TreeSet<>();
-                      }
-                    _abstractCalled.add(aaf);
-                  }
-
                 t = aaf.selfType().applyTypePars(aaf, fa._tp);
               }
           }
@@ -1913,7 +1885,6 @@ void instantiated(HasSourcePosition at)
     if (!_isInstantiated && !isVoidType())
       {
         _isInstantiated = true;
-        _instantiationPos = at;
       }
   }
 
@@ -2039,20 +2010,6 @@ private boolean allOutersInstantiated2()
   }
 
 
-  /**
-   * Perform checks on classes such as that an instantiated clazz is not the
-   * target of any calls to abstract methods that are not implemented by this
-   * clazz.
-   */
-  public void check()
-  {
-    if (isInstantiated() && _abstractCalled != null)
-      {
-        AirErrors.abstractFeatureNotImplemented(feature(), _abstractCalled, _instantiationPos);
-      }
-  }
-
-
   /**
    * In case this is a Clazz of value type, create the corresponding reference clazz.
    */

src/dev/flang/air/Clazzes.java

@@ -429,10 +429,6 @@ public static void findAllClasses(Clazz main)
     if (CHECKS) check
       (clazzesToBeVisited.size() == 0);
     closed = true;
-    for (var cl : clazzes.keySet())
-      {
-        cl.check();
-      }
   }
 
 

src/dev/flang/fuir/FUIR.java

@@ -35,6 +35,7 @@
 import java.util.TreeSet;
 
 
+import dev.flang.air.AirErrors;
 import dev.flang.air.Clazz;
 import dev.flang.air.Clazzes;
 import dev.flang.air.FeatureAndActuals;
@@ -2594,6 +2595,72 @@ public SourcePosition declarationPos(int cl)
   }
 
 
+  /*---------------------------------------------------------------------
+   *
+   * handling of abstract missing errors.
+   *
+   * NYI: This still uses AirErrors.abstractFeatureNotImplemented, which should
+   * eventually be moved to DFA or somewhere else when DFA is joined with AIR
+   * phase.
+   */
+
+
+  /**
+   * tuple of clazz, called abstrct features and location where the clazz was
+   * instantiated.
+   */
+  record AbsMissing(Clazz clazz,
+                    TreeSet<AbstractFeature> called,
+                    SourcePosition instantiationPos)
+  {
+  };
+
+
+  /**
+   * Set of missing implementations of abstract features
+   */
+  TreeMap<Clazz, AbsMissing> _abstractMissing = new TreeMap<>();
+
+
+  /**
+   * If a called to an abstract feature was found, the DFA will use this to
+   * record the missing implementation of an abstract features.
+   *
+   * Later, this will be reported as an error via `reportAbstractMissing()`.
+   *
+   * @param cl clazz is of the clazz that is missing an implementation of an
+   * abstract features.
+   *
+   * @param f the inner clazz that is called and that is missing an implementation
+   *
+   * @param instantiationPos if known, the site where `cl` was instantiated,
+   * `NO_SITE` if unknown.
+   */
+  public void recordAbstractMissing(int cl, int f, int instantiationSite)
+  {
+    var cc = clazz(cl);
+    var cf = clazz(f);
+    var r = _abstractMissing.computeIfAbsent(cc, ccc -> new AbsMissing(ccc, new TreeSet<>(), sitePos(instantiationSite)));
+    r.called.add(cf.feature());
+  }
+
+
+  /**
+   * In case any errors were recorded via `recordAbstractMissing` this will
+   * create the corresponding error messages.  The errors reported will be
+   * cumulative, i.e., if a clazz is missing several implementations of abstract
+   * features, there will be only one error for that clazz.
+   */
+  public void reportAbstractMissing()
+  {
+    _abstractMissing.values()
+      .stream()
+      .forEach(r -> AirErrors.abstractFeatureNotImplemented(r.clazz.feature(),
+                                                            r.called,
+                                                            r.instantiationPos));
+  }
+
+
 }
 
 /* end of file */

src/dev/flang/fuir/analysis/dfa/DFA.java

@@ -264,45 +264,43 @@ Val access(int s, Val tvalue, List<Val> args)
       var tc = _fuir.accessTargetClazz(s);
       var cc0 = _fuir.accessedClazz  (s);
       var ccs = _fuir.accessedClazzes(s);
-      var found = new boolean[] { false };
       var resf = new Val[] { null };
-      for (var cci = 0; cci < ccs.length; cci += 2)
+      tvalue.value().forAll(t ->
         {
-          var tt = ccs[cci  ];
-          var cc = ccs[cci+1];
-          tvalue.value().forAll(t -> {
-              check
-                (t != Value.UNIT || AbstractInterpreter.clazzHasUnitValue(_fuir, tt));
-              if (t == Value.UNIT ||
-                  t._clazz == tt ||
+          if (CHECKS) check
+            (t != Value.UNIT || AbstractInterpreter.clazzHasUnitValue(_fuir, tc));
+          var t_cl = t == Value.UNIT ? tc : t._clazz;
+          var found = false;
+          for (var cci = 0; cci < ccs.length; cci += 2)
+            {
+              var tt = ccs[cci  ];
+              var cc = ccs[cci+1];
+              if (t_cl == tt ||
                   t != Value.UNDEFINED && _fuir.clazzAsValue(t._clazz) == tt)
                 {
-                  found[0] = true;
+                  found = true;
                   var r = access0(s, t, args, cc, tvalue);
                   if (r != null)
                     {
                       resf[0] = resf[0] == null ? r : resf[0].joinVal(DFA.this, r);
                     }
                 }
-            });
-        }
-      var res = resf[0];
-      if (!found[0])
-        { // NYI: proper error reporting
-          var detail = new StringBuilder("Considered targets: ");
-          for (var ccii = 0; ccii < ccs.length; ccii += 2)
+            }
+          if (!found)
             {
-              detail.append(_fuir.clazzAsString(ccs[ccii])).append(ccii+2 < ccs.length ? ", " : "\n");
+              var instantiatedAt = _calls.keySet().stream()
+                .filter(c -> c._cc == t_cl && c._site != NO_SITE)
+                .map(c -> c._site)
+                .findAny()
+                .orElse(NO_SITE);
+              _fuir.recordAbstractMissing(t_cl, cc0, instantiatedAt);
             }
-          var ignore = _call.showWhy(detail);
-          Errors.error(_fuir.sitePos(s),
-                       "NYI: in "+_fuir.siteAsString(s)+" no targets for "+_fuir.codeAtAsString(s)+" target "+tvalue,
-                       detail.toString());
-        }
-      else if (res != null &&
-               tvalue instanceof EmbeddedValue &&
-               !_fuir.clazzIsRef(tc) &&
-               _fuir.clazzKind(cc0) == FUIR.FeatureKind.Field)
+        });
+      var res = resf[0];
+      if (res != null &&
+          tvalue instanceof EmbeddedValue &&
+          !_fuir.clazzIsRef(tc) &&
+          _fuir.clazzKind(cc0) == FUIR.FeatureKind.Field)
         { // an embedded field in a value instance, so keep tvalue's
           // embedding. For chained embedded fields in value instances like
           // `t.f.g.h`, the embedding remains `t` for `f`, `g` and `h`.
@@ -1019,6 +1017,7 @@ public void dfa()
             Context._MAIN_ENTRY_POINT_);
 
     findFixPoint();
+    _fuir.reportAbstractMissing();
     Errors.showAndExit();
   }
 

tests/abstractfeatures_negative/abstractfeatures_negative.fz

@@ -25,6 +25,9 @@
 
 abstractfeatures_negative is
 
+  # flag that may be true of false in a way that is unknown to the compiler.
+  maybe => envir.args.count > 3
+
   opengenerics12 =>
     F(A type...) ref is
       f(a A) is abstract                         //  1. should flag an error: abstract feature not implemented
@@ -34,7 +37,7 @@ abstractfeatures_negative is
 
     x G := G                                     //  2. should flag an error: abstract feature f not implemented
     y F i32 bool i32 := x
-    y.f 3 true 5
+    if maybe then y.f 3 true 5
 
   opengenerics12
 
@@ -50,9 +53,9 @@ abstractfeatures_negative is
 
     x G := G                                       //  6. should flag an error: abstract features f1, f2, f3 not implemented
     y F i32 bool i32 := x
-    y.f1 3 true 5
-    y.f2 3 true 5
-    y.f3 3 true 5
+    if maybe then y.f1 3 true 5
+    if maybe then y.f2 3 true 5
+    if maybe then y.f3 3 true 5
 
   opengenerics12a
 
@@ -64,7 +67,7 @@ abstractfeatures_negative is
     G ref : F i32 bool i32 is
 
     x G := G                                       //  8. should flag an error: abstract feature f not implemented
-    x.f 3 true 5
+    if maybe then x.f 3 true 5
 
   opengenerics12b
 
@@ -79,8 +82,8 @@ abstractfeatures_negative is
     G ref : F i32 bool i32 is
 
     x G := G                                       // 12. should flag an error: abstract features f1, f2, f3 not implemented
-    x.f1 3 true 5
-    x.f2 3 true 5
-    x.f3 3 true 5
+    if maybe then x.f1 3 true 5
+    if maybe then x.f2 3 true 5
+    if maybe then x.f3 3 true 5
 
   opengenerics12c

tests/reg_issue3359/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 <https://www.gnu.org/licenses/>.
+
+
+# -----------------------------------------------------------------------
+#
+#  Tokiwa Software GmbH, Germany
+#
+#  Source code of Fuzion test Makefile
+#
+# -----------------------------------------------------------------------
+
+override NAME = reg_issue3359
+include ../simple.mk