Dependent member function name is resolved incorrectly

[Endilll]

Consider the following one-liner:

llvm-project/llvm/include/llvm/ADT/PointerIntPair.h

Line 94 in fa2b038

PointerTy getPointer() const { return Info::getPointer(Value); }

Info::getPointer is incorrectly resolved to be the function defined in this very line.
It's observable by jumping to definition (jumps to the same line), and by asking for information about Info::getPointer symbol:

instance-method getPointer
→ PointerTy

// In PointerIntPair<PointerTy, IntBits, IntType, PtrTraits, Info>
public: PointerTy getPointer() const

Function is actually defined 100 lines below, in PointerIntPairInfo:

llvm-project/llvm/include/llvm/ADT/PointerIntPair.h

Lines 190 to 193 in fa2b038

	static PointerT getPointer(intptr_t Value) {
	return PtrTraits::getFromVoidPointer(
	reinterpret_cast<void *>(Value & PointerBitMask));
	}

I'm using VS Code with clangd extension, if that matters.

[llvmbot]

@llvm/issue-subscribers-clangd

[Endilll]

To be clear, detail::PunnedPointer has an operator intptr_t():

llvm-project/llvm/include/llvm/ADT/PointerIntPair.h

Line 45 in fa2b038

constexpr operator intptr_t() const { return asInt(); }

[Endilll]

Given that it's a dependent context, ideally I'd like clangd to list candidate implementations for me to pick from, ranking them by popularity, or at the very least give up and not mislead the user.

[HighCommander4]

Minimized testcase:

struct PointerIntPairInfo {
  static void *getPointer(void *Value);
};

template <typename Info = PointerIntPairInfo>
struct PointerIntPair {
  void *Value;
  void *getPointer() const { return Info::getPointer(Value); }
};

[HighCommander4]

So my first thought was that this is one of our textual heuristics (findNearbyIdentifier or locateSymbolTextually) acting up, but nope: our AST-based logic itself is returning this incorrect result! 🙀

[HighCommander4]

I think what's happening here is that HeuristicResolver::resolveMemberExpr() is being too promiscuous: if it fails to find the member in the qualifier's scope, it... ignores the qualifier and tries looking up the member in the current scope, which is incorrect in general.

[HighCommander4]

I tried just returning early if we have a qualifier and couldn't find anything in the qualifier's scope, expecting that some testcases will break, but nothing did. So that's seems like a good enough fix for now.

[HighCommander4]

Proposed fix: https://reviews.llvm.org/D158873

[HighCommander4]

After the fix, the behaviour is that AST-based resolution produces no results and we fall back on our textual heuristics.

On the minimized testcase, the textual heuristics offer both PointerIntPair::getPointer (the incorrect result), and PointerIntPairInfo::getPointer (the intended result).

This could be improved further (e.g. for calls, we could do arity-based filtering on function results founds by the textual heuristics), but I'm going to leave that to future issues and consider the original bug here fixed.

[HighCommander4]

On the minimized testcase, the textual heuristics offer both PointerIntPair::getPointer (the incorrect result), and PointerIntPairInfo::getPointer (the intended result).

(I don't have a local opt build handy to try the updated behaviour on the original testcase in the LLVM source, but the result is presumably similar, except that perhaps some additional functions named getPointer found in the index may be offered as well.)

[Endilll]

Thank you for addressing it so quickly!

On the minimized testcase, the textual heuristics offer both PointerIntPair::getPointer (the incorrect result), and PointerIntPairInfo::getPointer (the intended result).

(I don't have a local opt build handy to try the updated behaviour on the original testcase in the LLVM source, but the result is presumably similar, except that perhaps some additional functions named getPointer found in the index may be offered as well.)

That sounds good enough for me as well. Definitely an improvement over (what was) status quo.

[HighCommander4]

On the minimized testcase, the textual heuristics offer both PointerIntPair::getPointer (the incorrect result), and PointerIntPairInfo::getPointer (the intended result).

(I don't have a local opt build handy to try the updated behaviour on the original testcase in the LLVM source, but the result is presumably similar, except that perhaps some additional functions named getPointer found in the index may be offered as well.)

For completeness, I did check the behaviour on the original testcase. What actually happens is that the textual heuristic finds more than 5 candidate functions named getPointer in the index, and we bail and return no results.