https://github.com/HighCommander4 requested changes to this pull request.
I like the simplicity of this approach, especially that the desired behaviour for index-based completions also falls out of this! I do think it's worth adding a bit of extra logic to ensure that all our parameter names come from the same function declaration. While it's definitely an edge case, the possibility of mixing and matching parameter names from different declarations has the potential to cause an unnecessary amount of confusion. I don't think the logic for choosing the function declaration to use needs to be more complex than "the first one we encounter that has at least one nonempty parameter name". That handles the important case at issue here (declarations with no parameter names), and anything else (i.e. some parameters have names, others don't) seems like an unusual edge case. > It might be useful to do what c++ reflection is doing here, ie only produce a > name if all declarations introduce the same name (or no name) I think this stricter behaviour makes sense for reflection, where the parameter name potentially becomes runtime data for the program (and so inconsistent names is a signal that something might be wrong), but less so for code completion which is just a programming aid. For code completion, I'd say any parameter name (even if it's one of a conflicting set) is better than no name. https://github.com/llvm/llvm-project/pull/206716 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
