================
@@ -39,20 +40,24 @@ Hints declHints(const Decl *D) {
}
std::vector<Hinted<SymbolLocation>> locateDecl(const Decl &D) {
- std::vector<Hinted<SymbolLocation>> Result;
- // FIXME: Should we also provide physical locations?
- if (auto SS = tooling::stdlib::Recognizer()(&D)) {
- Result.push_back({*SS, Hints::CompleteSymbol});
- if (!D.hasBody())
- return Result;
- }
+ SourceManager &SM = D.getASTContext().getSourceManager();
+ bool HasBodyInMainFile = llvm::any_of(D.redecls(), [&](Decl *Redecl) {
+ return Redecl->hasBody() && SM.isInMainFile(Redecl->getLocation());
+ });
+ // if decl has body and in main file, we don't need to do further search.
+ if (!HasBodyInMainFile)
----------------
kadircet wrote:
i don't think it's actually "right" layering-wise to perform any
`sourcelocation <-> fileid` mappings here. it's subtle and expensive,
especially for every-redecl of stdlib symbols, which might be forward declared
many times in STL (but also for any other symbol).
Hence we actually have logic down the line that maps sourcelocations to files
(it's also expensive today, but is in a single place and takes care of edge
cases, like macros). at least this layering ensures that if we want to
introduce caching/optimizations one day, we can rely on having certain
separation between different kinds of mappings.
Moreover, I don't follow the check around a redeclaration having body only in
the main file. I feel like you're focusing on a very specific issue you have
and not solving the issue in general. e.g. what if the user only has forward
declaration of the symbol in the main file? why don't we want to suppress in
that case? or what if the user has body/declaration in a different header file,
which also seems pretty common.
I think if you want to improve this case, you really need to address `// FIXME:
Should we also provide physical locations?`, which is definitely doable, we
just need to make sure ranking is still done accordingly.
I am still strongly arguing that we should never insert a custom header if
there's a name-collision with a stdlib symbol. Hence we should make sure all
physical locations for a stdlib symbol is downranked, i'd recommend stripping
completesymbol signal from these symbols.
You also need to consider the macro-side of this. you're addressing this
situation for decls, but the user might as well have `#define assert(X) X` in
their main file (or some of the other headers). we need to make sure this same
logic also works for that.
https://github.com/llvm/llvm-project/pull/95797
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
