Decorate values with arbitrary warnings #3248
Conversation
|
Forgot to mention, there's also a new test utility: if an env variable called |
kustosz
requested review from
farmaazon,
MichaelMauderer,
mwu-tow and
wdanilo
as code owners
| Test.specify "should allow to attach multiple warnings and read them back" <| | ||
| x = 1233 | ||
| y = Warning.attach x "don't do this" | ||
| z = Warning.attach y "I'm serious" | ||
| here.get_warnings z . should_equal ["I'm serious", "don't do this"] |
There was a problem hiding this comment.
Shouldn't attach at the new warnings at the back instead of at the front?
Makes sense if we want warnings to go LIFO, but then it seems inconsistent with how in the expression: a+b technically a is computed first, but its warning also goes first.
There was a problem hiding this comment.
Newest warning goes first. Though, I'll probably end up having a logical clock for these, when I actually implement the warnings functionality. This is just to have a notion of tainting values with stuff, the user-facing part is to be implemented.
@wdanilo yeah, you're missing the fact this is an internal API only, mostly to make sure I can actually get away with value-tainting. This does not implement the user-facing API yet. |
kustosz
force-pushed
the
compiler/wip/mk/dataflow-warnings
branch
from
5b62f24
to
24179c1
Compare
...untime/src/main/java/org/enso/interpreter/node/expression/builtin/warning/GetOriginNode.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/enso/interpreter/node/expression/builtin/warning/GetReassignmentsNode.java
Outdated
Show resolved
Hide resolved
lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/AcceptsWarning.java
Outdated
Show resolved
Hide resolved
| attach value warning = @Builtin_Method "Prim_Warning.attach" | ||
|
|
||
| get_all : Any -> Vector.Vector | ||
| get_all value = @Builtin_Method "Prim_Warning.get_all" |
There was a problem hiding this comment.
There are a few methods missing from this list get_origin and get_reassignments.
|
|
||
| public static WithWarnings appendTo(Object target, ArrayRope<Warning> warnings) { | ||
| if (target instanceof WithWarnings) { | ||
| return new WithWarnings(((WithWarnings) target).warnings.append(warnings)); |
There was a problem hiding this comment.
Think this should just be:
return ((WithWarnings) target).warnings.append(warnings);
If I am reading it right otherwise would double nest the value
There was a problem hiding this comment.
right, something went very wrong here, good catch :)
Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>
Pull Request Description
This introduces the runtime support for transparent values decorated with arbitrary warning values.
It is the first part of the warnings task, as these warnings do not currently carry any meta-information regarding code locations. This will be added in the next PR.
Important Notes
There are no slowdowns in the java benchmark suite.
Checklist
Please include the following checklist in your PR: