|
Notes |
|
|
(0001074)
|
|
zs
|
|
2020-02-05 11:04
|
|
The cause of this bug is the fix for another bug: github 64.
That fix did come with an announcement that reqressions like this
would happen.
I also think this bug is probably a duplicate of Mantis bug 480.
The problem is that the code that constructs the final value of !:Actions
gives it an inst that contains unique components (specifically, the constants
action_canonicalise and action_split). In the presence of insts containing unique
components, cse_detection.m refuses to automatically transform the switch
on AllActions to the commented-out version, which directly leads to the bug.
As mentioned in the discussion of bug 480, fixing this properly is hard.
I can see a possible band-aid solution that *could* fix this and similar instances,
though of course not all others. This would involve changing the test in
cse_detection.m from "does the inst of the variant concerned (AllActions) include
any unique components" to "does it contains any unique components on
non-constants" (since constants do not have cells on the heap). The "could"
part above is because I am pretty sure this change would revive github bug 64,
but don't know whether fixing that in the constants-only case is possible.
Even if it didn't, it would have the downside that it would make the rules
followed by determinism analysis (when viewed from a user's point of view,
and so including cse) more complex still.
Who thinks this tradeoff is worth making? |
|
|
|
|
I don't think the tradeoff is worth making; it's going to make an already complicated situation even more complicated.
Question: how feasible is it to detect the above situation and make the error message suggest the refactoring? |
|
|
|
(0001076)
|
|
zs
|
|
2020-02-05 11:45
|
|
It should be easy to
- have cse record, in the proc_info the fact that it declined to pull a common unification
out of a disjunction due to uniqueness concerns, and
- have determinism analysis look at this flag, and if set, and there are determinism errors,
print a message about this fact being a possible cause of the errors.
What I am not sure about is whether we can give this message a useful context,
for two reasons. One: by definition, a common deconstruction involves at least
two contexts, maybe more, and listing them all probably wouldn't be useful.
We can probably use the smallest context (i.e. the one with the smallest
line number). Two, when cse pulls a deconstruction unification out of the arms
of a disjunction, the pulled-out unification does not come from any one place,
so there is no one "right" context it can record for it. Again, we can probably
use the smallest context among the pulled-out unifications. This would then
be a context we could report if there is a problem in the *next* invocation
of cse on that procedure body. But given the approximations above, I am not sure
whether the context they would yield would useful in the common cases.
However, given that the pointer to the cause of the error would be useful
even without a specific context, I will implement it. |
|
|
|
(0001079)
|
|
zs
|
|
2020-04-15 12:56
|
|
The reminder about this possible cause of the determinism error,
based on a flag set by cse_detection.m, was added to the compiler
on 2020 feb 7. |
|
bug496.m (1,807) 2020-02-05 10:20