<div dir="ltr">> Do you think that will actually be useful to the static analyzer use-case? I'd expect that in most use-cases, you'd want to provide the changes via some mechanism other than searching for a file named "APINotes.apinotes" in the same directory as you found the header. Other than for Swift with Apple frameworks, it seems likely to be pretty rare that the producer of a particular library would be the same entity providing a file of extra attributes on the side?<br><div><br></div><div>It might be convenient for a library author to attach a YAML file that describes which functions should be checked e.g. for taint analysis. This file could be placed conveniently right next to the headers. On the other hand, this is not an option for some system libraries (e.g. libc), we definitely don't want libc authors to provide this data, it should be the role of the analyzer developers. Actually we do that today for many standard C/C++ or POSIX functions, but we simply <a href="https://github.com/llvm/llvm-project/blob/08097fc6a9746370e073e617036847d9ea46c9f4/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L1778">hardcode the information into the source code of the analyzer</a> itself.</div><div><br></div><div><div>> I wouldn't think so, since this mechanism modifies the nodes as part of their initial creation, not as a subsequent mutation?</div><div><br></div><div>My understanding is that we create a Decl and then a bit later we create an Attr and attach that to it, so this seems to be a mutation to me. However, I am not sure, perhaps this should not be interpreted as such.</div><div class="gmail-yj6qo"></div><div class="gmail-adL"><br></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Oct 12, 2020 at 9:30 PM James Y Knight <<a href="mailto:jyknight@google.com">jyknight@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Mon, Oct 12, 2020 at 12:46 PM Gábor Márton <<a href="mailto:martongabesz@gmail.com" target="_blank">martongabesz@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">+1 for separating the Swift specific Sema logic the way you described.<br><div>However, there are valuable implementations in <a href="https://github.com/apple/llvm-project/tree/apple/main/clang/lib/APINotes" target="_blank">lib/APINotes/*</a> which we could use for a generic APINotes framework in Clang. For instance, <a href="https://github.com/apple/llvm-project/blob/apple/main/clang/lib/APINotes/APINotesManager.cpp#L325" target="_blank">APINotesManager::findAPINotes</a> encapsulates the logic for finding the APINotes files, this seems to be totally independent from Swift.<br></div></div></blockquote><div><br></div><div>Do you think that will actually be useful to the static analyzer use-case? I'd expect that in most use-cases, you'd want to provide the changes via some mechanism other than searching for a file named "APINotes.apinotes" in the same directory as you found the header. Other than for Swift with Apple frameworks, it seems likely to be pretty rare that the producer of a particular library would be the same entity providing a file of extra attributes on the side?</div><div><br></div><div>Also, I believe that particular function is unused in Swift. AFAICT, Swift only uses the modules-based apinotes loading (loadCurrentModuleAPINotes), not the header-path-based loading (that is: it passes "-fapinotes-modules" to its internal Clang invocation, not "-fapinotes").</div><div><br></div><div><div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>A side note: <a href="https://clang.llvm.org/doxygen/classclang_1_1ASTMutationListener.html" target="_blank">ASTMutationListener</a> is the interface which notifies any consumers when an AST node is changed after its creation. I think we should notify all clients once we add a new APINote Attribute (e.g. see <a href="https://clang.llvm.org/doxygen/classclang_1_1ASTMutationListener.html#abd04809c814e5ea2af681465004f32fe" target="_blank">AddedAttributeToRecord</a>), shouldn't we?</div></div></blockquote><div><br></div><div>I wouldn't think so, since this mechanism modifies the nodes as part of their initial creation, not as a subsequent mutation?</div><div><br></div></div></div>
</blockquote></div>