[PATCH] D33644: Add default values for function parameter chunks

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In https://reviews.llvm.org/D33644#793601, @yvvan wrote:

> In https://reviews.llvm.org/D33644#793594, @klimek wrote:
>
> > In https://reviews.llvm.org/D33644#793577, @yvvan wrote:
> >
> > > In https://reviews.llvm.org/D33644#793573, @klimek wrote:
> > >
> > > > In https://reviews.llvm.org/D33644#783903, @yvvan wrote:
> > > >
> > > > > Do not evaluate numbers.
> > > > >  Check for != "=" is needed not to mess with invalid default arguments or their types (without it I get "const Bar& bar = =" when Bar is not defined)
> > > >
> > > >
> > > > Shouldn't we than instead check that error case?
> > >
> > >
> > > I don't know the proper way to do that :) I also don't know the full amount of errors that might cause such behavior.
> > >  You can suggest the solution if you have some idea. Current one is safe because we just ignore any case that causes empty default string.
> >
> >
> > Taking your example "const Bar& bar = =" when Bar is not defined:
> >  What happens now?  Will it be "const Bar& bar ="? I argue that is not better than "const Bar& bar = =".
> >  Btw, can you add tests? I think that will make it easier to see what's actually happening.
>
>
> It will be just "const Bar& bar" in that case with this patch applied. So it is better than both "const Bar& bar =" and "const Bar& bar = =" :)
>  About tests - I agree that it's nice to have them. Can I find something similar in the current test set (as an example) and where is it better to put my tests?


All of test/Index/complete-* basically test this, I think. Try to find a good spot or write a new test.


https://reviews.llvm.org/D33644