Agreed -- diagnostic wording is usually trying to tell the user what they did wrong to guide them how to fix it, but this reads more like the diagnostic explains what the code does. This one will be especially interesting because people would naturally expect that comparison to work per the standard. And if enabling ICF breaks that then ICF is non-conforming because it makes valid code silently invalid. I think this is an ICF bug (it may still be worth warning users about though because you can use newer Clang with an older lld and hit the issue even if lld addresses this issue).

CC @lhames @sbc100 @ruiu in to try to scare up an lld code owner to see if this is known and if there's something that can be done on the lld side to not break valid code.

Ohhh interesting, so it's probably known that -icf=all will break code because it's not conforming and thus isn't an lld bug at all. Do (most?) other linkers also have the same functionality as -icf=all? I'm trying to understand whether this should be added to clang at all as it seems like it's a warning for a very particular usage pattern (a specific linker with a specific option ), which make it more reasonable for clang-tidy instead of the compiler.

Thanks for the details, that makes this seem more reasonable in Clang itself IMO.

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).