dblaikie added inline comments.
================ Comment at: clang/lib/CodeGen/CGExpr.cpp:2843-2844 + CGDebugInfo *DI = CGM.getModuleDebugInfo(); + auto *Fn = dyn_cast<llvm::Function>(LV.getPointer(*this)); + if (DI && Fn && !Fn->getSubprogram()) + DI->EmitFunctionDecl(FD, FD->getLocation(), T, Fn); ---------------- yonghong-song wrote: > dblaikie wrote: > > It looks like there isn't any test coverage for the case where Fn is null > > here (I added an assertion that Fn is non-null and it didn't fire when > > running check-clang) - please add some to this patch. (I'd like to then > > look at those cases to better understand when they come up and whether > > there's a different/better way to phrase this code to handle those cases) > Sorry. It is my fault. Tested with a slight different test case than this one > in my actual test setup. Now the new test should reflect null Fn. Thanks! Thanks for the updated test case! I think for generality, either this code shouldn't use the `LV` value - instead calling `GetOrCreateLLVMFunction` (though that's probably not great - since that would involve things like going through ConvertType and other shenanigans in `GetAddrOfFunction` - I guess if pieces of `GetAddrToFunction`, `EmitFunctionDeclPointer`, and `EmitFunctionDeclPointer` were split up maybe it could be done - or changet he return value to include the raw llvm::Function result in addition to the LValue (& keep the existing function name returning just the LValue for other callers)) - or doing more work to unwrap the Constant - maybe it would be enough to use "stripPointerCasts" & that'd get you something you can cast to llvm::Function? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100567/new/ https://reviews.llvm.org/D100567 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
