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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
