aaron.ballman added inline comments.
================ Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:276 // extern "C" int atexit(void (*f)(void)); - assert(cast<llvm::Function>(dtorStub)->getFunctionType() == - llvm::FunctionType::get(CGM.VoidTy, false) && + llvm::PointerType *Expected = + llvm::PointerType::get(llvm::FunctionType::get(CGM.VoidTy, false), ---------------- This will likely cause unused variable warnings in release builds due to the `assert` macro. ================ Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:303 // extern "C" int unatexit(void (*f)(void)); - assert(dtorStub->getFunctionType() == - llvm::FunctionType::get(CGM.VoidTy, false) && + llvm::PointerType *Expected = + llvm::PointerType::get(llvm::FunctionType::get(CGM.VoidTy, false), ---------------- Same here. ================ Comment at: clang/lib/CodeGen/CodeGenModule.h:1482 + void AddGlobalDtor(llvm::Function *Dtor, int Priority = 65535, + bool IsDtorAttrFunc = false); ---------------- There's a fixme comment a few lines up about hardcoding priority being gross and this sort of extends the grossness a bit. Perhaps these functions should accept a `DestructorAttr *`/`ConstructorAttr *` that can be null? ================ Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2537 + // priority. + CodeGenFunction CGF(CGM); + ---------------- Do you need this? I think you can get `VoidTy` off `CGM` already which seems to be the only use of `CGF`. ================ Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2584 + // We're assuming that the destructor function is something we can + // reasonably call with the default CC. Go ahead and cast it to the + // right prototype. ---------------- Is this assumption safe though given that there are calling convention attributes that can be written on the function? ================ Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2648 + // We're assuming that the destructor function is something we can + // reasonably call with the default CC. Go ahead and cast it to the + // right prototype. ---------------- Same question here as above. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90892/new/ https://reviews.llvm.org/D90892 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
