| Issue |
108145
|
| Summary |
getRawCommentForAnyRedecl can fail to return definition comment
|
| Labels |
clang:frontend
|
| Assignees |
|
| Reporter |
HighCommander4
|
While working on a clangd enhancement (https://github.com/llvm/llvm-project/pull/67802), @ckandeler and I have run into what I believe is a bug in `ASTContext::getRawCommentForAnyRedecl()`: when called on the **definition** of a function or class, it can fail to return the comment above the definition (i.e. incorrectly return null instead), if it was previously called on a (non-defining) declaration at a time when the definition wasn't yet added to the redecl chain.
I wrote a (failing) unit test demonstrating the issue at https://github.com/HighCommander4/llvm-project/commit/99330759463a902e16b0d5976557077b6e06c938.
The bug is a regression from https://github.com/llvm/llvm-project/commit/f31d8df1c8c69e7a787c1c1c529a524f3001c66a, which introduced the cache `CommentlessRedeclChains` and related data structures. (I verified this with a local backout of the patch, which makes my unit test pass.)
@jkorous-apple and @gribozavr, as author and reviewer of the regressing patch, could you advise what an appropriate fix would be?
---
Here are some more details regarding the bug:
The failing unit test processes the following code:
```c++
void f();
// f is the best
void f() {}
```
with an `ASTConsumer` that calls `ASTContext::getRawCommentForAnyRedecl()` as soon as a declaration is parsed and given to the consumer.
(I assume this is a valid use of `getRawCommentForAnyRedecl()`, i.e. it's not the intention to require that the function only be called once the whole AST is built, otherwise I don't see what would be the purpose of having caches like `CommentlessRedeclChains` at all. Please let me know if I misunderstood.)
When `getRawCommentForAnyRedecl()` is first called, for the non-defining declaration of `f()`, the definition has not been parsed and added to the redecl chain yet. This results in `CommentslessRedeclChains` containing an entry mapping this declaration to itself [here](https://searchfox.org/llvm/rev/b0ffaa79050460d724eec1b12363c439b43d5ae5/clang/lib/AST/ASTContext.cpp#464).
When the function is called a second time for the definition of `f()`, the redecl chain of that contains itself (the definition) as the first element, and the non-defining declaration as the second. The previously stored mapping is looked up [here](https://searchfox.org/llvm/rev/b0ffaa79050460d724eec1b12363c439b43d5ae5/clang/lib/AST/ASTContext.cpp#445), and results in the loop skipping the definition [here](https://searchfox.org/llvm/rev/b0ffaa79050460d724eec1b12363c439b43d5ae5/clang/lib/AST/ASTContext.cpp#455), and therefore never checking the definition for a comment.
Based on the above debugging, I don't quite understand how this caching mechanism is supposed to work.
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
