[PATCH] D92936: [Sema] Fix deleted function problem in implicitly movable test
Arthur O'Dwyer via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 5 07:20:16 PST 2021
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
More information about the cfe-commits
mailing list