[PATCH] D61015: [LibTooing] Change Transformer's TextGenerator to a partial function.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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<>`.

Alternatives:

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61015/new/

https://reviews.llvm.org/D61015