dblaikie marked an inline comment as done. dblaikie added a comment. In D61357#1488839 <https://reviews.llvm.org/D61357#1488839>, @rsmith wrote:
> In D61357#1485581 <https://reviews.llvm.org/D61357#1485581>, @dblaikie wrote: > > > Oh, @rsmith - if there's any better/different testing (if you can figure > > out how to reduce the test case down further, now that we know the cause - > > or if you'd like testing for other codepaths I've touched in this patch) > > I'm all ears. (also naming/API wrangling - my changes were just the minimal > > sort of "this works" based on what we discussed, but totally happy to do > > more involved/different things if there's such to be done) > > > Perhaps we could add a diagnostic scope object of some kind, which would > assert (or reorder the diagnostics?) if a diagnostic is produced while > producing notes for a different diagnostic. That might help for new code and > for places where we're aware -- or suspect -- there is a problem, but we > don't know where the existing bugs are and that doesn't help us find them. :( Yeah, I was thinking about something like that. Yeah, doesn't seem any reason we couldn't retrofit something on top of the diagnostics infratsructure - or a utility in it that connects notes to their diagnostic & either asserts, or delays emission, etc. I was thinking about that in the context of cleaning up /everything/ to use that, but yeah - we could do it as an incremental migration/as needed sort of thing. ================ Comment at: lib/Sema/SemaOverload.cpp:3518-3519 << false << From->getType() << From->getSourceRange() << ToType; } else return false; + ---------------- rsmith wrote: > Can we avoid calling `CompleteCandidates` on this path? Yeah, I raised up the two if conditions to a pre-check: if (!(OvResult == OR_Ambiguous || (OvResult == OR_No_Viable_Function && !CandidateSet.empty()))) return false; auto Cands = ... if (...Ambiguous) ... else { // No_Viable_Function && !empty() ... NoteCandidates (open to other ideas, couldn't quite think of anything that didn't involve some duplication - either duplicating the conditions or the CompleteCandidates call, etc) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61357/new/ https://reviews.llvm.org/D61357 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits