https://github.com/ilovepi requested changes to this pull request.

While this is likely a step in the right direction, I'm a bit skeptical that 
this patch is entirely correct. 

First, I think we need to understand why we only want to cache/memoize the USR 
strings from definitions. 

Second, this requires some more significant testing than is currently present 
for clang-doc. Additionally, we'd need to evaluate such an invasive change on 
multiple projects, and ensure that the documentation output has not changed due 
to this patch. Even then, I'd wouldn't be confident that we'd gotten it correct 
until tests were in place, and we could examine the differences with, and 
without this patch.

Lastly, I think its worth discussing the memoization/caching strategy used 
here, and if its appropriate/scalable. Other clang tools use different 
mechanisms, and it would be good to avoid reinventing the wheel if we can just 
adopt one of those that also happens to be well tested, such as clangd's 
indexing and the search/caching mechanisms used in scandeps.

https://github.com/llvm/llvm-project/pull/96809
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to