Add Expr.valueOrAbort and reflect.report.errorAndAbort
#12056
Conversation
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
from
3514ee2 to
2705092
Compare
|
@liufengyun we also need to take into account the discussion in #12022 |
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
from
51fd7af to
4586e7a
Compare
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
from
4586e7a to
4d28db1
Compare
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
from
4d28db1 to
cd70cb2
Compare
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
2 times, most recently
from
65d2eb4 to
9d2fd94
Compare
|
Now the added methods are marked as experimental and we do not need to wait until 3.1 to add them. |
|
Will need rebase on #12371 |
| * Otherwise returns the value. | ||
| */ | ||
| @experimental | ||
| def valueOrAbort(using FromExpr[T]): T | ||
|
|
There was a problem hiding this comment.
I'd suggest the method to take a message for indicating the reason why it's aborted. It helps end-users in debugging and saves the efforts of macro-authors in writing error-handling code.
There was a problem hiding this comment.
The question then is if we should keep the version with the default error message. The idea if this signature was also to make the authors not need to write the error message.
Currently the can do
x.value match
case Some(n) => ...
case None => reflect.report.error("......", x)or just
if x.valueOrAbort == 0 then ...
else the proposed one would need to add the message
if x.valueOrAbort(".........") == 0 then
else ...There was a problem hiding this comment.
Indeed, the version with the error message does not look better in code and can be a little annoying for prototyping. For macro authors, it's not clear that it's better.
Macros can be subtle to use, more debuggability to end-users will make them more friendly for usage. However, if the gain for end-users is small, maybe it's not worth the complication.
The question then is if we should keep the version with the default error message
We probably only want to keep one of the two.
There was a problem hiding this comment.
We could add an valueOrAbortWith("....") later or users will just define their own in the meantime. We should probably wait and see what users define themselves to know what is preferable in paractice.
There was a problem hiding this comment.
Meanwhile, they can just define something like this in their projects
extension [T: FromExpr](expr: Expr[T])
def valueOrAbortWith(msg: String)(using Quotes) =
x.value match
case Some(n) => n
case None => reflect.report.errorAndAbort(msg, x)Expr(1).valueOrAbortWith("expected a constant")There was a problem hiding this comment.
I agree, it's better to wait and do it later.
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
2 times, most recently
from
55de88a to
21bc7e3
Compare
|
@liufengyun could you review the changes. I had to move some implementations to make MiMa happy. |
Provides a homogeneous and unambiguous concept for stopping the expansion of a macro. The advantages of this naming are * Consistent name suffixes `xyzAbort` * `report.e` will show auto-completion for all variants of `error` and `errorAndAbort` * `Abort` cannot be confused with other kinds of error handling in this API
smarter
added
the
needs-minor-release
nicolasstucki
force-pushed
the
use-abort-as-concept-for-stopping-macro-expansion
branch
from
21bc7e3 to
43fec8f
Compare
nicolasstucki
removed
the
needs-minor-release
nicolasstucki
deleted the
use-abort-as-concept-for-stopping-macro-expansion
branch
Provides a homogeneous and unambiguous concept for stopping the expansion of a macro.
The advantages of this naming are
xyzAbortreport.ewill show auto-completion for all variants oferroranderrorAndAbortAbortcannot be confused with other kinds of error handling in this APIThis is an alternative to #12022