ilya-biryukov added a comment. In https://reviews.llvm.org/D44480#1116359, @nik wrote:
> I've stumbled about this bug too and was looking into it and then I saw the > mail about this change being submitted :) > > I've ended up with a slightly different change (https://dpaste.de/PSpM/raw , > instead of FD->getReturnType()->getContainedDeducedType() I have > FD->getReturnType()->getAs<AutoType>()). It looks like the tests from this > change pass with my change and my tests pass with this change (consider to > add decltype(auto) tests, too!). So I'm wondering whether there is a test > case that fails for one change but not the other (if there is, the test case > should probably be added), or if there are differences in the performance. > > I'm not familiar enough with the code base to justify the getAs<AutoType>(), > I've just observed that it made my tests pass. I'm sure there should be no significant differences in performance. Adding `decltype(auto)` to the test would certainly be useful. W.r.t. correctness I think at this point checks for DeducedType and AutoType are equivalent. There is only one other inheritor of DeducedType, the DeducedTemplateSpecializationType, and it should not appear in the function return type IIUC. When the concepts get into the standard and clangd implements them, this might change, as the comment of `DeducedType` suggests: /// Common base class for placeholders for types that get replaced by /// placeholder type deduction: C++11 auto, C++14 decltype(auto), C++17 deduced /// class template types, and (eventually) constrained type names from the C++ /// Concepts TS. I don't know the exact details of the latest Concepts proposal, but would assume usages of constrained template parameters from Concepts TS shouldn't block skipping of function bodies. In which case, your fix seems to do the right thing and this one doesn't. @rsmith, WDYT? Should we check for AutoType instead? Repository: rL LLVM https://reviews.llvm.org/D44480 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
