[PATCH] D94391: CGDebugInfo: Drop Loc.isInvalid() special case from getLineNumber

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D94391#2493978 <https://reviews.llvm.org/D94391#2493978>, @dblaikie wrote:

> In D94391#2491229 <https://reviews.llvm.org/D94391#2491229>, @MaskRay wrote:
>
>> In D94391#2491178 <https://reviews.llvm.org/D94391#2491178>, @dblaikie wrote:
>>
>>> Any particular bug you're trying to fix? (this looks like it changes the debug locations, so it would need tests)
>>>
>>> Not necessarily clear that these are better debug locations?
>>
>> No particular bug. `CGDebugInfo::getLineNumber` returning CurLoc just looks strange. (The CurLoc may be far below (irrelevant) when getLineNumber is called.)
>>
>> This patch is hopefully NFC. Dropping `CurLoc` from `getLineNumber(RD->getLocation().isValid() ? RD->getLocation() : CurLoc)` will cause a difference, which may reveal missing `SourceLocation` information in ObjC constructs.
>
> Ah, OK. Yeah, I don't have strong feelings about this - a bit hesitant to make the change without a stronger motivation though. Perhaps other reviewers have some thoughts to contribute (also maybe @JDevlieghere ).

Using `CurLoc` in `getLineNumber` is not ideal. Trying to fix one call site reveals a probably unsatisfactory debug location for ObjC `ImplicitParamDecl`.
The location is set to the closing brace. I do not investigate further as I know nearly nothing about ObjC...

  --- a/clang/test/CodeGenObjC/debug-info-blocks.m
  +++ b/clang/test/CodeGenObjC/debug-info-blocks.m
  @@ -65,7 +65,7 @@ static void run(void (^block)(void))
         // CHECK-DAG: !DILocalVariable(arg: 2, scope: ![[COPY_SP]], {{.*}}, flags: DIFlagArtificial)
         // CHECK-DAG: !DILocalVariable(arg: 1, scope: ![[DESTROY_SP]], {{.*}}, flags: DIFlagArtificial)
         run(^{
  -          // CHECK-DAG: ![[SELF]] = !DILocalVariable(name: "self", scope:{{.*}}, line: [[@LINE+4]],
  +          // CHECK-DAG: ![[SELF]] = !DILocalVariable(name: "self", scope:{{.*}}, line: [[@LINE-7]],
             // CHECK-DAG: ![[D]] = !DILocalVariable(name: "d", scope:{{.*}}, line: [[@LINE+1]],
             NSMutableDictionary *d = [[NSMutableDictionary alloc] init]; 
             ivar = 42 + (int)[d count];


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94391