<table border="1" cellspacing="0" cellpadding="8">
    <tr>
        <th>Issue</th>
        <td>
            <a href=https://github.com/llvm/llvm-project/issues/108145>108145</a>
        </td>
    </tr>

    <tr>
        <th>Summary</th>
        <td>
            getRawCommentForAnyRedecl can fail to return definition comment
        </td>
    </tr>

    <tr>
      <th>Labels</th>
      <td>
            clang:frontend
      </td>
    </tr>

    <tr>
      <th>Assignees</th>
      <td>
      </td>
    </tr>

    <tr>
      <th>Reporter</th>
      <td>
          HighCommander4
      </td>
    </tr>
</table>

<pre>
    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.
</pre>
<img width="1px" height="1px" alt="" src="http://email.email.llvm.org/o/eJzMV01v4zgS_TXMpRBDpmTJOvjgpGFMbovpAHMukSWLY4rUkJTd2V-_KElO7PTHzu5pAMOJZJH16tWrxxLGaI6OaCc2T2Lz5QHH1Pmw-80cu2ff9-g0heKh8fpt90dnLMHFh5NxR_AOEJRFd9RArkOnqCeXQMhtl9IQRb4X8iDk4WhSNzYr5XshD9aer38eh-D_JJWEPAyjtUIeymqbSSFrIZ9BFJk6cXRLAdBpeIEOzwRhdGBc8nDpMMELNGQNnQlMBIRmPIJxIMps__X12btE3xLjyPdHSr_jhTMilw4-7N3b76RJWSG3HLHMRL6HS0cOFFpLmvNLHYGQeyH3mlrjTDLezdfgW0BoR6f4HvjATMTIwE0ChQ5aNBaSh0BpDPNWao4O2PgzTXc-tmXazIpWYJzyIZBK9u261o3WgnExEeqFHdNynAtGGAKdjR-jfbsBjryd8-5xDuCOQtbA2WLAKRomQEimpznlT1guGJ2QVYI3SoBak-ZM-KEwcQaqQ-NWIvsisv38_QKX4BPNkTn3JejoTIJEMYGm3ruYGIA7TpuZGEdiKL_Qy70Ov1cOc2r4n7rO86za1EWZY51JWpdNpjd1VW42VVZVTUlZqep8e4f6taNZMyyeQMdAMTIBbfD9r1D9WMXvWNp8rbe6XautKmuqsNpWaq3WaiNr3MiizbNsrcoSuZKXzqiOFR28HhUzzUpBxdIrs0WwlmKc5frMzEdRZlNPBLKYSIPGhBBTGFUaA8UVF-EFzhRMa6YtTYSLSR0gWK_QQoPq5MfEMuZ4AybVfaDp8UQR-reb6g0Y44rFd8OeKLI_Tz74MT7iMFiaIIkiOwbT-H_jOfCOGGG2lAXw2dCFwjXwlXN3_MCg_Gg1vPkRUJ9NpLnT0QEOQ_BDMJgIWvMNLtODDYn8cAvr8fHx9vI3CgQYCKLvCXofWOoJjY0cHYO-yrEZj1ztT_JYtHxLRfCKYqQ4rWq9tf7CTyiv6dMGbCvTRwn5xJ_p7tkbDe1iPPnT3YJJa9CyIidMFNP3i0BUT6L68inG7T5zsW-cMI49BZZNYjLZKeL_Z5Nc0OjZQrhnbj3FRBgwRNJTpY_mzMbiF-ebAazuc92-AEb-YVbo1IVntEbDGIklIsrsbxg3--FknUnIKoLzabYXl8hNyCYb_ms0geb0p7q9e7ezb9DQh3uq2ZsvnbcE-6-vjKsZjU0cyKeOwoVV-QLazy4ZadHoVZBzT41h8HMaHZ4ngXBXR7Dm9F97OwFau4J_WcJIYClBT3By_sLO_wK9iSMbYkze689t-Qc7-t9ijjNrTYhpSZ4TbH2Y4N-eHndlnsvS3tH_6fzocK5CQ-RuNfHTo4QPmhW8sggCxdGmuBzjC_r4I5KUdwlnfOiAXApv0OMwzP1s4h3q5MGkSLYFsXnqKJDYfPluUomEQXWt_7by4fhh84HOQh6arG0RqzrbZEWZ6UoWRGrdrGVe5qrI66bI9QZpwwcBT0W83jRCHvZfX-fvpdNWahiEzIuyELJefVe5O3GaeNUlQiTlnZ5P7WuVbkj_WV3ueJ5slw1g5i6-kyK399uxy-DicJNCyE7j3WTpTv9aIsvCGTDXlW7HlJh8IP1eKhPBen8iDePwzylOsVkmrfnUetck52W9HyCezFVq93PcPyWDzW0G7FrU8tnn6EwBVEfq9AP0rCu8Dqp32nzC-DEUzxOspmY8HqdB7_nGDv8aTSJY_ImDd_4yNyQbIAftSXXoTOy5-HEc2CcnX-B3i9WD3uW6zmt8oN26kmWWr-U2e-h2WLVUNG1LZVGW660sdLMt6qauSBeZrtSD2clMFlm9XmebbJtlq4pqjflW5WWxabCsRZFRj8aumHquwsM0g-7W2XZdbB4sNmTj9CYk5cxyvm8D8-q0kJLfj8Jumvqa8RhFkVkTU_zYLZlkafdT4_3Ri8EN-QvtD2Owu_95-JwSiUIellzOO_mfAAAA__-_3JBv">