Represent nodes with Node_ instead of Option<Node_>
commitfd6aebf3b1254f32e131492304151e5b272fe62e
authorJake Bailey (Hacklang) <jakebailey@fb.com>
Thu, 4 Jun 2020 17:57:29 +0000 (4 10:57 -0700)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Thu, 4 Jun 2020 18:11:41 +0000 (4 11:11 -0700)
tree1acfaefe0e4d6be70f8061ee0b69915ccc0facd3
parentea794e0abe19500e8a699bb40e8a2b79e5696921
Represent nodes with Node_ instead of Option<Node_>

Summary:
Representing error-nodes with None was a helpful intermediate step away from Result, but the `None` value is a bit redundant. We already have a representation for an empty node--`Node_::Ignored`.

This diff removes the distinction between `None` (which means some unspecified error occurred when computing that node) and `Some(Node_::Ignored)` (which means the node is one the direct decl parser does not care about). The direct decl parser does not care about errors, so it seems reasonable to coalesce the former category into the latter.

Because it is no longer possible to use the `?` operator (it can be used only with `Option` and `Result` for the time being), this diff introduces an `unwrap_or_return!` macro, which returns `Node_::Ignored` if the argument is `None`. This looks a little ugly, but I think it might be a small improvement in one way--returning Ignored just because a helper like node_to_ty returned None/Err is a pretty coarse strategy for error-recovery.

For instance, if we have a class which implements some interface, but `node_to_ty` returns None for the interface type due to some parse error in it, we should just omit that type from the class's implements list. But as implemented currently, we ignore the entire class. Maybe `ignore_none!` will call out these situations a little more clearly than `?` did.

Reviewed By: Wilfred

Differential Revision: D21695605

fbshipit-source-id: f4eac23bcc155aa23d3ceba05f7f4371b816b01b
hphp/hack/src/decl/direct_decl_smart_constructors.rs