Quuxplusone accepted this revision. Quuxplusone added a comment. This revision is now accepted and ready to land.
I don't fully understand the new control flow, but at least the new //behavior// (after applying this patch) looks like an improvement to me. I recommend rebasing on top-of-tree, mainly so that the buildbots will run again and presumably be greener this time. I still doubt that I have the authority to "accept" this patch, and am hoping to see someone like @zygoloid weigh in, even if it's only to say "meh." I //am// willing and able to //physically// land this patch, if someone says it's appropriate for me to do so. ================ Comment at: clang/lib/Sema/SemaInit.cpp:4181 + // For deleted functions in other contexts, there is no need to get the + // initialization sequence. + if (Result == OR_Deleted && Kind.getKind() != InitializationKind::IK_Copy) ---------------- IIUC, this comment is explaining the motivation for repeating `if (Result == OR_Deleted) // don't return quite yet` twice in the code above — line 4123 and line 4146. It might be better to move the comment higher, then. ================ Comment at: clang/lib/Sema/SemaInit.cpp:5285 + // need the initialization sequence to decide whether to perform the second + // overload resolution. + if (!(Result == OR_Deleted && ---------------- Checking my understanding: If the first resolution selects a deleted function which is a constructor whose first parameter is an rvalue reference to T, then we don't perform the second resolution. If the first resolution selects a deleted function which is not a constructor, or whose parameter is of the wrong type, then (in C++11 through C++17 but not in C++20) we //do// perform the second resolution. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92936/new/ https://reviews.llvm.org/D92936 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
