incorrect source location for builtin functions in the clang AST.

[hokein]

Given the following case:

int test(unsigned __x)  { 
  return __builtin_popcount(1); // line 2
}
int test2(unsigned __x)  { 
  return __builtin_popcount(1);
}

void foo() {
  __builtin^_popcount(1ul); // calling go-to-definition in clangd will jump to line2.
}

The __builtin_popcount FunctionDecl looks like, its source location points to the first call site (line 2, col 10). The source location is suspicious (I think it should be an invalid source loc).

`-FunctionDecl 0x560139b6dd20 <col:10> col:10 implicit used __builtin_popcount 'int (unsigned int) noexcept' extern
|   |-ParmVarDecl 0x560139b6de20 <<invalid sloc>> <invalid sloc> 'unsigned int'
|   |-BuiltinAttr 0x560139b6ddc8 <<invalid sloc>> Implicit 378
|   |-NoThrowAttr 0x560139b6de90 <col:10> Implicit
|   `-ConstAttr 0x560139b6deb8 <col:10> Implicit

[llvmbot]

@llvm/issue-subscribers-clang-frontend

[shafik]

It looks like we do the same thing w/ other builtins, I am not sure if this is expected behavior or not though, CC @AaronBallman

[AaronBallman]

As best I can tell, this has been the behavior since at least Clang 3.5, so it's a bit tough to say whether it is or isn't expected. Sema::CreateBuiltin() takes a source location for where the implicit declaration lives, but https://github.com/llvm/llvm-project/blob/277b8989794f6a915edb519cdf8c937ffbdc83eb/clang/lib/Sema/SemaLookup.cpp#L952 is what picks the source location to pass, and it's passing the location of the name.

I'm not certain what will break if we change the logic, but I would not expect this to be an invalid source location, but instead be in the <built-in> location (same as predefined macros).