DeinAlptraum wrote: My thoughts on this PR in its current state are as follows - this is more or less trying to achieve the same thing as #78114 - that said, the annotations are somewhat different. #78114 annotates exactly as much/little as is necessary to pass a strict type-check. This PR in comparisons annotates more symbols, and it does not pass a (strict or non-strict) type-check yet - imo we would first need to decide which approach is preferable here, or if we prefer #78114 - possibly this should be split into smaller PRs - if we decide to go with this PR, the string formatting changes from #173861 should still be removed from this change
Some more background on #78114: I've been working on typing annotations for the bindings for two or three years now, and #78114 is where that started. I've been slowly splitting off chunks from #78114 (it was originally a few thousand lines of changes) for different areas of the code in order to keep the changes small and focused (see 2nd point above) and fixing all sorts of other issues we encounter along the way with @Endilll's support. I'm currently going through fixing https://github.com/llvm/llvm-project/issues/156680, though it's taking me a while since I'm quite busy with work recently. That's why there hasn't been much progress on typing annotations themselves since May. Having said that, I am obviously somewhat biased towards my own annotations in #78114 but I believe that a) passing a (strict) type check is preferable, and b) I'm not sure I find it desirable to annotate nearly all symbols, e.g. constant assignments don't need type annotations in my opinion, and neither do many of the other cases you've annotated. In particular, I didn't annotate them because the type checker infers that much, and so do the IDEs using them (as I've confirmed with some of your annotations in my IDE locally). One could argue that having them is better for clarity even when they are not necessary, I would be somewhat against that but I don't have a strong opinion here and would leave judgement to @Endilll anyway. https://github.com/llvm/llvm-project/pull/173845 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
