sammccall added a comment. In D83419#2141433 <https://reviews.llvm.org/D83419#2141433>, @hokein wrote:
> > Sorry, I should give some reasons here: > > These are sensible reasons. My only (not a strong) concern is that "error" is > quite special, we need to be careful to choose it -- note that there is an > `error` function in glibc which is used for error-reporting. Yeah. This is also true of `log` which is is a c standard library function. I've seen slightly funny diagnostics when the header is missing, but no worse than that. It's not possible to actually call the wrong function here - neither of the first two parameters of glibc `error()` can be a string. > maybe there are other better names (`stringError`?) other than > `createError`/`makeError`. Maybe, it's hard to know what's important enough to include in the name. Neither the fact that the implementation stores a string, or that we're allocating an object seem to qualify to me. Maybe something to do with the formatting like `errorV` or `fmtError` or something but they seem pretty ugly. The difficulty of being part of the `log` family, which doesn't have suffixes... >> I guess makeError is still better than the status quo, but not enough to >> feel motivated to clean this up right now. Maybe someone else wants to pick >> this up? > > unless you strongly insist on `error` name, I'm happy to help with (looks > like it's just a low-hanging rename fruit). I think this is a nice cleanup, > we should make it happen (either using `error` or other names). Sure. I'm probably letting the perfect be the enemy of the good here, still sad about the json::Value::asInt() -> json::Value::getAsInt() thing... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83419/new/ https://reviews.llvm.org/D83419 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
