Skip to content

Commit

Permalink
air/dfa: Move reporting missing implementations of abstracts to DFA, fix
Browse files Browse the repository at this point in the history
 #3359

The air phase is not accurate enough, which is why these errors were disabled
for type features.  Reporting them all during DFA avoids this and we can handle
missing abstract type features the same way as norml abstract features.

The code for handling this is still a bit spread out ofer packages air, fuir and
dfa, a NYI comment has been added to fix this when air and dfa phases are
joined.

Also adds a regression test.
  • Loading branch information
fridis committed Jul 16, 2024
1 parent 6608922 commit 8d6d201
Show file tree
Hide file tree
Showing 8 changed files with 321 additions and 57 deletions.
43 changes: 0 additions & 43 deletions src/dev/flang/air/Clazz.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -1913,7 +1885,6 @@ void instantiated(HasSourcePosition at)
if (!_isInstantiated && !isVoidType())
{
_isInstantiated = true;
_instantiationPos = at;
}
}

Expand Down Expand Up @@ -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.
*/
Expand Down
4 changes: 0 additions & 4 deletions src/dev/flang/air/Clazzes.java
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}


Expand Down
67 changes: 67 additions & 0 deletions src/dev/flang/fuir/FUIR.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -2599,6 +2600,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 */
18 changes: 8 additions & 10 deletions src/dev/flang/fuir/analysis/dfa/DFA.java
Original file line number Diff line number Diff line change
Expand Up @@ -288,16 +288,13 @@ Val access(int s, Val tvalue, List<Val> args)
}
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)
{
detail.append(_fuir.clazzAsStringNew(ccs[ccii])).append(ccii+2 < ccs.length ? ", " : "\n");
}
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());
{
var instantiatedAt = _calls.keySet().stream()
.filter(c -> c._cc == tc && c._site != NO_SITE)
.map(c -> c._site)
.findAny()
.orElse(NO_SITE);
_fuir.recordAbstractMissing(tc, cc0, instantiatedAt);
}
else if (res != null &&
tvalue instanceof EmbeddedValue &&
Expand Down Expand Up @@ -1019,6 +1016,7 @@ public void dfa()
Context._MAIN_ENTRY_POINT_);

findFixPoint();
_fuir.reportAbstractMissing();
Errors.showAndExit();
}

Expand Down
25 changes: 25 additions & 0 deletions tests/reg_issue3359/Makefile
Original file line number Diff line number Diff line change
@@ -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
109 changes: 109 additions & 0 deletions tests/reg_issue3359/reg_issue3359.fz
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# 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
#
# -----------------------------------------------------------------------

# Test errors reported for missing abstract (type) features
#
reg_issue3359 is

# ---------------------------------------------------------------------
#
# first, check errors for missing implementations of abstract type features

# f declares three abstract type features n, o, p
#
f is
type.n unit => abstract
type.o unit => abstract
type.p unit => abstract


# g does not implement any of the three inherited type features n, o, p
#
g : f is


# h implementes type feature n, but not o nor p
#
h : g is
redef type.n =>


# t will call T.n and T.o, but not T.p since this call is unreachable
#
t(T type : f) =>
if envir.args.count = 0 then T.n
else if envir.args.count = 1 then T.o
else if true then
else T.p # unreachable, will not be reported!


# f does not implement n nor o, so these two will be reported as not implemented abstract
t f

# g does not implement n nor o, so these two will be reported as not implemented abstract
t g

# h does not implement n, so this will be reported as not implemented abstract
t h


# ---------------------------------------------------------------------
#
# next, also check errors for missing implementations of abstract normal features

# i declares three abstract features q, r, s
#
i is
q unit => abstract
r unit => abstract
s unit => abstract


# j does not implement any of the three inherited features q, r, s
#
j : i is


# k implementes feature q, but not r nor s
#
k : j is
redef q =>


# u will call T.q and T.r, but not T.s since this call is unreachable
#
u(v T : i) =>
if envir.args.count = 0 then v.q
else if envir.args.count = 1 then v.r
else if true then
else v.s # unreachable, will not be reported!


# i does not implement q nor r, so these two will be reported as not implemented abstract
u i

# j does not implement q nor r, so these two will be reported as not implemented abstract
u j

# k does not implement q, so this will be reported as not implemented abstract
u k
Loading

0 comments on commit 8d6d201

Please sign in to comment.