[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.
Yitzhak Mandelbaum via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 25 06:14:17 PDT 2019
ymandel added a comment.
In D61015#1478539 <https://reviews.llvm.org/D61015#1478539>, @ilya-biryukov wrote:
> Why would we consider this a legitimate failure, rather than a programming error?
> Same argument could be made about any form of format-string-like functions, e.g. `llvm::formatv` or `sprintf`.
> Yet, they return strings and not `Expected<string>` or their equivalent.
It's not that it's a legitimate failure so much as an invalid argument -- that is, the caller caused it and therefore should handle the failure. For a standalone tool making the call, I'd argue they should treat it as a programming error and assert to crash. However, a server that takes the input from, say, a user will want to propagate that back to the user.
As for `formatv` and `sprintf` -- I think the difference is that this has a more dynamic design. Both of those take explicit arguments at the callsite, vs. TextGenerator which takes a `MatchResult`. But, that's just a detail -- the key thing is that we're trying to support a usecase where the input may be flawed and we want to fail gracefully. We do so at the cost of the added complexity of `Expected<>`.
1. add a separate validation function. But, this will complicate the design since we'd need to pass two functions rather than one. That is, `TextGenerator` would need to be a pair of functions.
2. return an empty string on failure. This has the typical tradeoffs of using a sentinel value.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits