Configure script breakage with the new -Werror=implicit-function-declaration

To this end, I’ve started ⚙ D133771 Add a "Potentially Breaking Changes" section to the Clang release notes to improve the release notes and see about exposing some of that information more visibly.

Did I predict it? Yes and no. I had off-list conversations with @jwakely about it and so I was aware that GCC had issues they were trying to address before they rolled out the same changes. I mentioned that the situation sounded rather awful for them and that I hoped we were avoiding similar problems based on the silence about the change after it landed. But I didn’t know about (or if I knew, I didn’t appreciate) the silent breakage issues. Mea culpa!

Agreed, but that wasn’t really what I was after. I was suggesting that when we announce release candidates, someone run a smoke test against an RC to see if everything explodes or not like it did here. Given how quickly after the release you were able to spot a “stop the world” problem for you, I’m trying to find out if there’s a way we could get that information before we release rather than after as that seems like it will be the least disruptive for all of us.

That may be true, but I don’t know that this part of the discussion is besides the point; to me, it’s the primary point. This unmaintained software is pulling back a conforming change to a compiler, and it’s a change that comes from multiple user requests to please improve the security posture of C and because the C language is evolving into these areas based on the fact that this code isn’t valid. Continuing to not maintain the code is not going to be an option at some point because someday we will switch the default language mode to C23. That’s why I’m super happy to hear @thesamesam (and others) are actively working on trying to get fixes upstreamed; I think that’s going to ultimately be time very well spent. I greatly appreciate those efforts!

Fantastic! Out of curiosity, what are your thoughts on the suggestion from @MaskRay? Does that give you enough breathing room, or does it cause other hardships? (I was thinking this would also help with implicit int issues that @thesamesam said they’ve also hit a few of.)

Just a gentle reminder that 15.0.1 is due on Tuesday next week (the 20th September) - so if we need to do anything for that release it would be nice if it landed in the release branch this week.

1 Like

Thank you for the reminder! If the recommendation from @MaskRay will work, then I think that should be the recommended approach for folks in this situation. If that recommendation is not workable for some reason, I think I can downgrade the diagnostic to be a warning-only diagnostic relatively easily in the 15.0.1 branch directly this week.

1 Like

I’m afraid this is going to be really hard. LLVM overall has very high commit activity and the development of next release overlaps with bugfix releases of the previous branch. I think it would be helpful if we packaged 16.x snapshots and encourage users to test that long before the first RCs. Unfortunately, I don’t really know how to choose a good snapshot point for this and I’m afraid I would end up packaging code with known regressions.

I think config files are a good solution to test changes like this one, i.e. to easily test “before” and “after” without having to keep two builds of clang around or rely on CFLAGS being reliably respected. However, I don’t think this is something we should use as a default.

One thing we’d really like to avoid is changing the defaults in Gentoo, as that would mean that people using clang on Gentoo get different results from people using clang on other distros. I’d really prefer if we could settle on consistent behavior everywhere.

My preference is that the change is reverted for 15.x globally but that’s just me. However, what’s even more important here is to reach some consensus ASAP. If the behavior is going to be reverted, I’d be happy to patch the Gentoo version sooner to reduce the timespan where they are exposed to the problem. If it’s not going to be reverted, I’d like to publish an appropriate announcement to Gentoo users, so they know what to expect. We’re currently in the worst situation possible where we know that things are broken and we are unable to tell users what to expect.

To this end, I’ve started an RFC at [RFC] Add new discourse channel for "Potentially Breaking/Disruptive Changes" for Clang to hopefully give folks a way to subscribe to notifications about potentially disruptive changes earlier.

Ah, thank you for helping me to understand the situation better!

Making sure I understand the concern here: because Gentoo is a source distribution, this isn’t a matter of patching the Clang used to build the packages in the distro, it’s a matter of patching the system-installed Clang which is then used to build the packages in the distro?

If that’s somewhat accurate, is it possible to configure the compiler in this special way only when it’s used to build the packages so that the system-installed Clang still behaves like it does everywhere else?

Agreed!

Is that desirable though? I think the distro angle here is a bit of a red herring – yes, that’s where the effects are most strongly felt, but people can and will run into this when manually building projects – and they will likely not be in a position to diagnose such failures, especially if they are silent.

Unless the message we want to send here is that if you grab some random library/project/repo off the internet and try to build it, you should really use gcc, because using clang is prone to crash and burn (or worse, not crash and burn), this is a problem that needs to be addressed generally, and not just inside distro build systems.

So +1 on downgrading these to warnings in 15.0.1.

Exactly this.

To some degree, yes. Probably via injecting the -Wno-error=... flags straight into the CC variable. However, this also involves potential confusion since package manager builds will now be different from the same builds performed manually.

1 Like

I propose that for 15.0.1, we downgrade both -Wimplicit-int and -Wimplicit-function-declaration from error to default-on warning, for all C compilation modes prior to C23. (We can leave them as errors in devhead, but I also think we should plan to explicitly re-visit the question before the Clang 16 release, and not just assume it’ll be OK.)

I think it’s a particularly good idea to revert because people are actively volunteering to work on fixing the problems in software upstream. It certainly is helpful to our users to break less software, if possible, so if we can expect a lot of these problems to “go away” over the next few months, that’s a great justification to delay the change.

That we’re reverting this for 15.0.1 instead of 15.0.0 is unfortunate, but I expect there will be very few users of 15.0.0 as soon as 15.0.1 comes out, so I think that will be a short-lived issue, and not an argument to avoid the change.

3 Likes

Agreed; I’m working on a patch for this currently and hope to post it today or tomorrow.

I’m never opposed to reacting to more information, so I’m happy to re-visit the decision before we ship again. However, I’m strongly opposed to kicking this can down the road indefinitely as that limits our ability to ever set C2x as the default language mode, it makes for worse security posture with things known to cause issues, and it steps directly on the toes of the language committees. Giving some wiggle room for people to react right now is defensible, but this code hasn’t been valid for 20 years and at some point we can no longer let it keep constraining us because it does not meet the bar for an extension in Clang (specifically, there are no proposals in front of any language committee to keep this behavior and one of those committees has been actively reclaiming these design spaces for new functionality).

Adding this to my list of issues that should be revisited before 16.x :slight_smile:

1 Like

Downgrading this to warning-by-default for 15.0.1 looks fine to me as well.
There are multiple folks on IRC who express the concern whether Clang would be asked to kick the can down the road again (for 16.0.0).

I am with them: the main branch should keep error-by-default:
there are vendors who have passed the stage adding -Wno-error=implicit-function-declaration to random projects and are now enjoying the benefits
(new projects not implicit-function-declaration clean cannot be sneak into their repositories)
As jyknight said, we can revisit the distribution packages and check what default 16.0.0 needs to use. If a distro finds at 16.0.0 there are still significant straggling users, I’d suggest they use the configuration file idea (Configure script breakage with the new -Werror=implicit-function-declaration - #13 by MaskRay) instead of holding back the error-by-default state.

In addition, I am glad that the main branch has made progress in eliminating implicit function declarations
and the work shall benefit distributions using GCC as the primary compiler.
(Thanks to Florian for the Linux Plumbers Conference 2021 link Eliminating implicit function declarations. I did not know it). And I am thankful to folks who have raise upstream package issues or submitted patches.

libc++ has a Phabricator Project “libc++ vendors” (Login) to notify impacted vendors.
We can use “clang-vendors” (Login) for Clang.

I agree with the plan to make these warnings in 15.0.1, given that maintainers are now making an active effort to fix these problems.

1 Like

The patch is now up for review at: ⚙ D133800 [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

2 Likes

FWIW, I think @MaskRay’s --config idea would be something we could pursue if clang defaulted to searching somewhere (see ⚙ D109621 [clang][Driver] Default to loading clang.cfg if config file not specified). But not something to block on for now anyway.

And the changes are now landed in the 15.0.1 branch: Downgrade implicit int and implicit function declaration to warning only · llvm/llvm-project-release-prs@c0141f3 · GitHub

Thank you to everyone for the discussion around this!

3 Likes

On a semi-related note, I’ve added yesterday’s snapshot of LLVM 16.x to Gentoo now. It’s not visible to our users by default but hopefully it’ll encourage more of them to test it. Having to push new snapshots should also keep me motivated to ensure that LLVM 16.x builds continuously, and get quicker reports for build and test failures.

I’ll also try to help getting config support ship-shape, so we could use it effectively to test configure scripts. I’m going to make a bigger announcement and request for testing in Gentoo once all’s ready.

1 Like

Is it intentional that while -Wimplicit-int has been downgraded to a warning, -Wint-conversion still defaults to an error?

I hadn’t heard that there were any concerns with -Wint-conversion, so I left it as-is. However, if there are issues with it, it’d be good to know ASAP!