[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

[Zequan Wu via Phabricator via cfe-commits]

zequanwu marked an inline comment as done.
zequanwu added a comment.

In D147073#4240017 <https://reviews.llvm.org/D147073#4240017>, @hans wrote:

> I'm not familiar with this code. I suppose the question is whether it's reasonable for this code to expect that the source locations are always valid or not?

Yes.

For `0 ? T<C>{} : T<C>{};`, the both branches have valid start location but invalid end location. See comments at https://github.com/llvm/llvm-project/issues/45481#issuecomment-1487267814.

For the `std::strong_ordering`, I found that `DeclRefExpr` in the ConditionalOperator's true branch has invalid start and end locations. It might because it's inside a `PseudoObjectExpr`. Maybe we should simply just skip visiting `PseudoObjectExpr`, I see that its begin and end location are the same. I'm not familiar with that expression, so, I just handled it by adding checks for validating begin and end locations.

  PseudoObjectExpr 0x5555629f3bf0 'const strong_ordering':'const class std::strong_ordering' lvalue
  |-BinaryOperator 0x5555629f3bd0 'const strong_ordering':'const class std::strong_ordering' lvalue '<=>'
  | |-MemberExpr 0x5555629f3848 'const S':'const struct S' lvalue .value 0x5555629f34d0
  | | `-DeclRefExpr 0x5555629f3808 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f31b0 'lhs' 'const MyStruct &'
  | `-MemberExpr 0x5555629f3878 'const S':'const struct S' lvalue .value 0x5555629f34d0
  |   `-DeclRefExpr 0x5555629f3828 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f3238 'rhs' 'const MyStruct &'
  |-OpaqueValueExpr 0x5555629f38a8 'const S':'const struct S' lvalue
  | `-MemberExpr 0x5555629f3848 'const S':'const struct S' lvalue .value 0x5555629f34d0
  |   `-DeclRefExpr 0x5555629f3808 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f31b0 'lhs' 'const MyStruct &'
  |-OpaqueValueExpr 0x5555629f38c0 'const S':'const struct S' lvalue
  | `-MemberExpr 0x5555629f3878 'const S':'const struct S' lvalue .value 0x5555629f34d0
  |   `-DeclRefExpr 0x5555629f3828 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f3238 'rhs' 'const MyStruct &'
  `-ConditionalOperator 0x5555629f3ba0 'const strong_ordering':'const class std::strong_ordering' lvalue
    |-CXXOperatorCallExpr 0x5555629f3970 '_Bool' '==' adl
    | |-ImplicitCastExpr 0x5555629f3958 '_Bool (*)(const S &, const S &)' <FunctionToPointerDecay>
    | | `-DeclRefExpr 0x5555629f38d8 '_Bool (const S &, const S &)' lvalue Function 0x5555629f2a40 'operator==' '_Bool (const S &, const S &)'
    | |-OpaqueValueExpr 0x5555629f38a8 'const S':'const struct S' lvalue
    | | `-MemberExpr 0x5555629f3848 'const S':'const struct S' lvalue .value 0x5555629f34d0
    | |   `-DeclRefExpr 0x5555629f3808 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f31b0 'lhs' 'const MyStruct &'
    | `-OpaqueValueExpr 0x5555629f38c0 'const S':'const struct S' lvalue
    |   `-MemberExpr 0x5555629f3878 'const S':'const struct S' lvalue .value 0x5555629f34d0
    |     `-DeclRefExpr 0x5555629f3828 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f3238 'rhs' 'const MyStruct &'
    |-DeclRefExpr 0x5555629f3b80 'const strong_ordering':'const class std::strong_ordering' lvalue Var 0x5555629a4ed8 'equal' 'const strong_ordering':'const class std::strong_ordering'
    `-ConditionalOperator 0x5555629f3b50 'const strong_ordering':'const class std::strong_ordering' lvalue
      |-CXXOperatorCallExpr 0x5555629f3ab0 '_Bool' '<' adl
      | |-ImplicitCastExpr 0x5555629f3a98 '_Bool (*)(const S &, const S &)' <FunctionToPointerDecay>
      | | `-DeclRefExpr 0x5555629f3a78 '_Bool (const S &, const S &)' lvalue Function 0x5555629f27a0 'operator<' '_Bool (const S &, const S &)'
      | |-OpaqueValueExpr 0x5555629f38a8 'const S':'const struct S' lvalue
      | | `-MemberExpr 0x5555629f3848 'const S':'const struct S' lvalue .value 0x5555629f34d0
      | |   `-DeclRefExpr 0x5555629f3808 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f31b0 'lhs' 'const MyStruct &'
      | `-OpaqueValueExpr 0x5555629f38c0 'const S':'const struct S' lvalue
      |   `-MemberExpr 0x5555629f3878 'const S':'const struct S' lvalue .value 0x5555629f34d0
      |     `-DeclRefExpr 0x5555629f3828 'const MyStruct':'const struct MyStruct' lvalue ParmVar 0x5555629f3238 'rhs' 'const MyStruct &'
      |-DeclRefExpr 0x5555629f3b30 'const strong_ordering':'const class std::strong_ordering' lvalue Var 0x5555629a4c50 'less' 'const strong_ordering':'const class std::strong_ordering'
      `-DeclRefExpr 0x5555629f3ae8 'const strong_ordering':'const class std::strong_ordering' lvalue Var 0x5555629a5388 'greater' 'const strong_ordering':'const class std::strong_ordering'



================
Comment at: clang/test/CoverageMapping/invalid_location.cpp:31
+//        now because 'T<C>{}' doesn't have a valid end location for now.
+// CHECK-NETX:   File 0, 13:9 -> 13:14 = #3
+// CHECK-NETX:   File 0, 13:18 -> 13:23 = (#0 - #3)
----------------
hans wrote:
> s/NETX/NEXT/ here and below?
These 2 "CHECK-NETX" are misspelled on purpose. Once the FIXME is fixed, we should correct the spelling here and below. 

Updated the FIXME.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147073/new/

https://reviews.llvm.org/D147073