HighCommander4 wrote:
I've started to look at the implementation. I haven't looked through everything
in detail yet, but the main thing I'm noticing so far is that the patch doesn't
actually implement Sam's suggestion of having the parser pass in to
`CodeCompleteQualifiedId` a flag indicated that we are in a declaration context.
Instead, the patch is inferring this information inside
`CodeCompleteQualifiedId` by doing this:
```c++
if (!SemaRef.CurContext->isFunctionOrMethod() &&
!SemaRef.CurContext->isRecord()) {
// Assume we are completing a declaration
}
```
But this has false positives. For example:
```c++
namespace N {
int foo(int required, int optional = 42);
}
int foo = N::f^
```
Here, the behaviour before the patch was to produce a `DeclaringEntity=false`
completion, giving us just the required argument and making it a placeholder.
The behaviour after the patch is to produce a `DeclaringEntity=true`
completion, inserting both arguments with no placeholders.
I think it would be better to keep this a `DeclaringEntity=false` completion by
using a more accurate determination of whether we are in a declaration context,
by having the parser pass in a boolean to `CodeCompleteQualifiedId` indicating
this. (Note, this would be an additional boolean to `IsAddressOfOperand`,
something like `IsInDeclarationContext`.)
https://github.com/llvm/llvm-project/pull/165916
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
