[PATCH] D70625: [DebugInfo][BPF] Support to emit debugInfo for extern variables

Yonghong Song via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sat Nov 23 12:36:52 PST 2019


yonghong-song added a comment.

@dblaikie Thanks for the review. To address your comments below:

> the title talks about global variables
>  the description talks about extern types (guessing that's just a typo?)

The title is "Support to emit debugInfo for extern variables". In the description, I am using "extern" variables as well. But in the implementation, I am using "GlobalDecl" or "GlobalDeclOnly". I indeed need to make naming convention consistent.

>   the patch itself seems to have code that's visiting more functions? ( processFuncPrototypes )

The proceessFuncPrototypes filtered any function without debug info and defined. So based on my testing, it only emits extern functions. But do let me know if I miss anything here.

>   & also I'd generally still prefer to see two patches for each piece of new debug info - first adding the functionality to LLVM, then second using that functionality in Clang (even though we're in the monorepo - these are still divisible patches that makes code review, revert, root cause analysis, etc, easier)

Agree. I will separate the patch into two separate patches as you suggested, add more tests for the clang patch and resubmit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70625





More information about the cfe-commits mailing list