MaskRay added inline comments.
================ Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7007 + "comparison of function pointers (%0 and %1)">, + InGroup<CompareFunctionPointers>, DefaultIgnore; def warn_typecheck_ordered_comparison_of_function_pointers : Warning< ---------------- aaron.ballman wrote: > aaron.ballman wrote: > > cjdb wrote: > > > aaron.ballman wrote: > > > > cjdb wrote: > > > > > cjdb wrote: > > > > > > It's very rare that we set a warning to `DefaultIgnore`. What's the > > > > > > motivation for this? > > > > > > This warning is disabled by default, since it's only relevant if > > > > > > ICF is explicitly enabled. > > > > > > > > > > I see why now. Perhaps this warning should be enabled by default when > > > > > ICF is also enabled, and an error otherwise. > > > > The problem is: ICF is an lld thing, it's not a Clang thing; so Clang > > > > has no idea if ICF will or won't be enabled. > > > Okay, that sounds like we can't make it an error to turn this warning on > > > when ICF isn't enabled, but what about turning it on when the driver sees > > > `-icf=all`? Or does that bypass `clang_cc1` altogether? > > The diagnostic serves no purpose unless the user is linking with > > `-icf=all`, so agreed we can't enable this by default. We might be able to > > do something by looking at linker flags passed through clang on to the > > driver, but it's not going to be perfect (users can link manually without > > invoking through the compiler, and I'm not certain what IDEs do when > > driving builds with Clang). > @MaskRay -- do you think we should try to enable this diagnostic by default > by looking at driver flags that will be passed to the linker, or does it seem > more reasonable to you to have this ignored by default? I think it should remain ignored by default. Link actions happen separate at a late stage and we generally don't want linker options to affect compiles. (e.g. We haven't allowed -fuse-ld= to affect compiles). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141310/new/ https://reviews.llvm.org/D141310 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
