aaron.ballman added a comment. In D147073#4258426 <https://reviews.llvm.org/D147073#4258426>, @zequanwu wrote:
> In D147073#4258396 <https://reviews.llvm.org/D147073#4258396>, @aaron.ballman > wrote: > >> In D147073#4258384 <https://reviews.llvm.org/D147073#4258384>, @hans wrote: >> >>> Again not an expert here, but lgtm. >>> >>> (Nit: the >>> https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExprCXX.cpp#L1528-L1530 >>> link in the description seems to point to the wrong code now, since main >>> changed. Here is a link for 16.0.1: >>> https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536) >> >> I'm confused -- I thought D147569 <https://reviews.llvm.org/D147569> >> resolved the issue and so this patch is no longer needed? > > D147569 <https://reviews.llvm.org/D147569> fixes > https://github.com/llvm/llvm-project/issues/45481. This one fixes another > issue crbug.com/1427933. Their stack trace look similar but not caused by the > same issue. > > Updated the link in summary to: > https://github.com/llvm/llvm-project/blob/llvmorg-16.0.1/clang/lib/Sema/SemaExprCXX.cpp#L1536 Thank you for clarifying, I was confused. :-) I don't think the changes here are correct either -- it's glossing over an issue that we're not properly tracking the source location in the AST. Playing around with the reduced example is interesting though. If you remove the default argument in the `T` constructor, the issue goes away. If you stop using a forward declaration in the instantiation of `T`, the issue goes away. If `S1` isn't a template, the issue goes away (but the issue will come back if you then make `foo()` a template instead of `S1`). So it seems that something about template instantiation is dropping the source location information (perhaps) and we should be trying to track down where that is to fix the root cause rather than work around it here for coverage mapping alone. (The end location being incorrect can impact other things that are harder to test because it'll be for things like fix-its that don't work properly, which are easy to miss.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147073/new/ https://reviews.llvm.org/D147073 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
