ioeric added inline comments.

================
Comment at: lib/Sema/SemaCodeComplete.cpp:3744
     AddMacroResults(PP, Results, false, PreferredTypeIsPointer);
-  HandleCodeCompleteResults(this, CodeCompleter, 
-                CodeCompletionContext(CodeCompletionContext::CCC_Expression, 
----------------
ilya-biryukov wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > `ResultsBuilder`'s constructor accepts a `CodeCompletionContext`. Can 
> > > > we pass in the context with `PreferedType` there instead of 
> > > > reconstructing it later?
> > > > To make sure we don't miss other things (incl. any future additions) 
> > > > that `ResultsBuilder` puts into the context.
> > > The `PreferedType` should actually already be set in the `ResultsBuilder` 
> > > (line 3715). In thoery, `Results.getCompletionContext()` should work fine 
> > > here as well, but it would break some Index tests - it introduced some 
> > > inconsistency in sema scoring when running with and without result 
> > > caching 
> > > (https://github.com/llvm-mirror/clang/blob/master/test/Index/complete-exprs.c#L35).
> > >  This is probably a bug, but I'm not sure if I'm the right person to 
> > > chase it :( 
> > What kind of inconsistencies? Maybe we should just update the CHECKS in the 
> > test?
> After chatting offline: it seems passing in the context that has the 
> preferred type set into `ResultsBuilder` saves us from check failures.
> The fact that `ResultsBuilder` also has `setPreferrredType`, which can be 
> different from the one in the `CodeCompletionContext` is still confusing, but 
> that's unrelated to the problem at hand.
Sounds good. Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D48917



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to