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