Re: [PATCH] D49175: [clangd] Ignore sema code complete callback with recovery context.
Sounds good. Thanks for the suggestion! On Wed, Jul 11, 2018 at 3:45 PM Sam McCall wrote: > > > On Wed, Jul 11, 2018, 15:18 Eric Liu via Phabricator < > revi...@reviews.llvm.org> wrote: > >> ioeric added a comment. >> >> In https://reviews.llvm.org/D49175#1158562, @sammccall wrote: >> >> > I like this idea, of course hard to know how it will affect all >> practical cases. >> > >> > Is it easy to get have internal stats on how many wins/losses this has? >> >> >> This would fix bad completion results when sema could recover to the >> normal mode (e.g. completion in macros like EXPECT_THAT, expressions in if >> statements). From the code completion metrics I collected from existing >> code, the number of completions where no result was found dropped by 30%. >> The loss seems to be we will get no result in recovery mode rather than bad >> results (e.g. in excluded preprocessor branches). >> > Thanks! I have no doubt this is positive overall, so just thinking out > loud here. > > For changes like this, we can show that quantitavely by running > before/after completions at the same sites, and reporting e.g. > Percent of all sites that are better vs worse vs neutral (in terms of rank > of desired result) > Change in MRR: mean/std deviation > > For our internal quality evaluation tools, we could think about how to > make it easy to get these numbers for a candidate change. > Having the raw results as before/after pairs is great for investigating > wins/losses too. > > > >> >> Repository: >> rCTE Clang Tools Extra >> >> https://reviews.llvm.org/D49175 >> >> >> >> ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D49175: [clangd] Ignore sema code complete callback with recovery context.
On Wed, Jul 11, 2018, 15:18 Eric Liu via Phabricator < revi...@reviews.llvm.org> wrote: > ioeric added a comment. > > In https://reviews.llvm.org/D49175#1158562, @sammccall wrote: > > > I like this idea, of course hard to know how it will affect all > practical cases. > > > > Is it easy to get have internal stats on how many wins/losses this has? > > > This would fix bad completion results when sema could recover to the > normal mode (e.g. completion in macros like EXPECT_THAT, expressions in if > statements). From the code completion metrics I collected from existing > code, the number of completions where no result was found dropped by 30%. > The loss seems to be we will get no result in recovery mode rather than bad > results (e.g. in excluded preprocessor branches). > Thanks! I have no doubt this is positive overall, so just thinking out loud here. For changes like this, we can show that quantitavely by running before/after completions at the same sites, and reporting e.g. Percent of all sites that are better vs worse vs neutral (in terms of rank of desired result) Change in MRR: mean/std deviation For our internal quality evaluation tools, we could think about how to make it easy to get these numbers for a candidate change. Having the raw results as before/after pairs is great for investigating wins/losses too. > > Repository: > rCTE Clang Tools Extra > > https://reviews.llvm.org/D49175 > > > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
