[PATCH] D70625: [DebugInfo][BPF] Support to emit debugInfo for extern variables
    Yonghong Song via Phabricator via llvm-commits 
    llvm-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 llvm-commits
mailing list