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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
