mrcvtl wrote:
> I have debug the issue, seems we cannot move the `__range` variable check to
> the start of `checkExprLifetimeImpl`, we may missing extending lifetime of
> temporaries. Eg.
>
> ```c++
> template <typename T>
> struct ListWrapper {
> ListWrapper() {}
> ~ListWrapper() {}
> const T *begin() const;
> const T *end() const;
> ListWrapper& r();
> ListWrapper g();
> };
>
> using A = ListWrapper<int>;
>
> A g() { return A(); }
> const A &f1(const A &t) { return t; }
>
> void member_call() {
> // CHECK-CXX23: void @_ZN7P2718R011member_call11member_callEv()
> // CHECK-CXX23-LABEL: for.cond.cleanup:
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> for (auto e : g().r().g().r().g().r().g()) {}
> }
> ```
>
> I think we should revert to the previous approach, what do you think?
>
> ```c++
> if (IsGslPtrValueFromGslTempOwner && DiagLoc.isValid()) {
> if (const auto *VD =
> dyn_cast_if_present<VarDecl>(ExtendingEntity->getDecl());
> SemaRef.getLangOpts().CPlusPlus23 && VD &&
> VD->isCXXForRangeImplicitVar())
> return true;
> SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
> << DiagRange;
> return false;
> }
> ```
Agree, there are some side effect that I can't fully understand.
> I have debug the issue, seems we cannot move the `__range` variable check to
> the start of `checkExprLifetimeImpl`, we may missing extending lifetime of
> temporaries. Eg.
>
> ```c++
> template <typename T>
> struct ListWrapper {
> ListWrapper() {}
> ~ListWrapper() {}
> const T *begin() const;
> const T *end() const;
> ListWrapper& r();
> ListWrapper g();
> };
>
> using A = ListWrapper<int>;
>
> A g() { return A(); }
> const A &f1(const A &t) { return t; }
>
> void member_call() {
> // CHECK-CXX23: void @_ZN7P2718R011member_call11member_callEv()
> // CHECK-CXX23-LABEL: for.cond.cleanup:
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> for (auto e : g().r().g().r().g().r().g()) {}
> }
> ```
>
> I think we should revert to the previous approach, what do you think?
>
> ```c++
> if (IsGslPtrValueFromGslTempOwner && DiagLoc.isValid()) {
> if (const auto *VD =
> dyn_cast_if_present<VarDecl>(ExtendingEntity->getDecl());
> SemaRef.getLangOpts().CPlusPlus23 && VD &&
> VD->isCXXForRangeImplicitVar())
> return true;
> SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
> << DiagRange;
> return false;
> }
> ```
I agree. There are so
> I have debug the issue, seems we cannot move the `__range` variable check to
> the start of `checkExprLifetimeImpl`, we may missing extending lifetime of
> temporaries. Eg.
>
> ```c++
> template <typename T>
> struct ListWrapper {
> ListWrapper() {}
> ~ListWrapper() {}
> const T *begin() const;
> const T *end() const;
> ListWrapper& r();
> ListWrapper g();
> };
>
> using A = ListWrapper<int>;
>
> A g() { return A(); }
> const A &f1(const A &t) { return t; }
>
> void member_call() {
> // CHECK-CXX23: void @_ZN7P2718R011member_call11member_callEv()
> // CHECK-CXX23-LABEL: for.cond.cleanup:
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> // CHECK-CXX23-NEXT: call void
> @_ZN7P2718R011member_call11ListWrapperIiED1Ev(
> for (auto e : g().r().g().r().g().r().g()) {}
> }
> ```
>
> I think we should revert to the previous approach, what do you think?
>
> ```c++
> if (IsGslPtrValueFromGslTempOwner && DiagLoc.isValid()) {
> if (const auto *VD =
> dyn_cast_if_present<VarDecl>(ExtendingEntity->getDecl());
> SemaRef.getLangOpts().CPlusPlus23 && VD &&
> VD->isCXXForRangeImplicitVar())
> return true;
> SemaRef.Diag(DiagLoc, diag::warn_dangling_lifetime_pointer)
> << DiagRange;
> return false;
> }
> ```
Thanks for debugging that!
I agree btw, didn't think about this side effect before.
https://github.com/llvm/llvm-project/pull/145164
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
