Quuxplusone added inline comments.
================
Comment at: clang/lib/Sema/SemaInit.cpp:4020
bool IsListInit = false,
bool IsInitListCopy = false) {
assert(((!IsListInit && !IsInitListCopy) ||
----------------
Two comments you could act on if you want, or just ignore if they seem too
scope-creepy:
(1) This function is //crazy// long and complicated. It would be great to break
it down into named steps or subtasks.
(2) I'm still concerned about the repetition of `if (Result == OR_Deleted but
Kind == IK_Copy) keep going`. IIUC, the intent is that this function should
return the `InitializationSequence` that is found by overload resolution... but
sometimes we can tell that overload resolution is going to find an
inappropriate sequence (e.g. an explicit conversion, or a sequence that depends
on a deleted function, or maybe some other situations) and in that case we want
to short-circuit, because the caller is going to treat "No unique sequence" in
the same way as "The unique sequence was inappropriate". **Would it be better
to** have the caller pass in a new function parameter, `bool
ShortCircuitUnusableSequences`, which is usually `true` but can be explicitly
set to `false` in the three cases (`return`, `co_return`, `throw`) where we
currently want to treat "No unique sequence" differently from "The unique
sequence was inappropriate"?
Then the repeated check would be `if (Result == OR_Deleted &&
ShortCircuitUnusableSequences) return;` and it would be a little clearer what's
going on.
(Sidenote: Default function arguments, //especially// trailing boolean ones,
[are the
devil](https://quuxplusone.github.io/blog/2020/04/18/default-function-arguments-are-the-devil/#the-boolean-parameter-tarpit).)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D92936/new/
https://reviews.llvm.org/D92936
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
