Update macro spec to allow more introspection in Phase 2. #3480
Conversation
| finish. | ||
|
|
||
| If a cycle arises in macro applications waiting for other macro applications to | ||
| complete then a special value is returned indicating that introspection is not |
There was a problem hiding this comment.
I do think it makes sense to throw an error here, we can't easily model a special value otherwise (I don't want to go down the path of Option types or something). How about a MemberIntrospectionCycleException?
There was a problem hiding this comment.
For what it's worth I'm not fully convinced that the "recover and succeed" path is an important one: it means that adding one macro can cause another macro (the one that does the "recover and succeed") to act as if declarations are missing. It sounds potentially confusing: I changed something with Macro X, why did macro Y stop doing what I expected? It's possible this is useful in some cases, but I could also believe that always bailing out and telling the user "macros X and Y can't be used together in this way" is a better.
If we do want "recover and succeed":
We could just make the return value nullable? I don't think there's any information we want to convey on failure. We could call the method try... by analogy to int.tryParse.
I think if we use an exception it's pretty likely that authors will simply not handle this case, it is unlikely to show up in their automated or manual testing. Naming the method to indicate that it could fail would help in this case too, though.
I dunno, possibly I'm overthinking it ;) up to you. Thanks!
There was a problem hiding this comment.
Fwiw I am totally fine with making this an Error instead of an Exception, to indicate that it should not be caught/handled? I don't think many macros would be able to actually do anything meaningful.
I mostly just think the easiest way to surface this error (and terminate the macro), is to just have the API throw. I don't think we really have to worry if some macro author does actually try to handle the error, they can't do anything harmful, its just that they probably can't do anything useful (but maybe some could, who knows).
There was a problem hiding this comment.
Sounds good--how about like this? (just throw StateError).
Thanks.
davidmorgan
force-pushed
the
introspect-members-of-other-types
branch
from
2ef1d6c to
5622032
Compare
|
Thanks :) |
Fix #3442