Large number of -Wattributes warnings when compiled with GCC >= 14

[mstorsjo]

If compiling Clang with GCC 14 or newer (for Linux), the build produces a very large number of warnings - obscuring any other potential higher value warnings. Building with GCC seldom is warning free anyway, but ideally we'd keep it at least in manageable numbers.

There are around 600 instances of the same warning:

/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::UnusedFileScopedDecls' [-Wattributes]
  841 | class Sema : public SemaBase {
      |       ^~~~
/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::TentativeDefinitions' [-Wattributes]
/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::ExtVectorDecls' [-Wattributes]
/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::DelegatingCtorDecls' [-Wattributes]

(I.e this header gets included in 150 translation units, and each time it produces 4 warnings.)

In principle, this warning actually seems legitimate in this case. The warning is caused by us building with -fvisibility-inlines-hidden. The root cause of the issue can be observed in this reduced testcase:

class SomeClass {
public:
    void method() {}
};

template<typename Source, void (Source::*Loader)()>
class LazyVector { };

class TestClass {
  typedef LazyVector<SomeClass, &SomeClass::method> MyType;
  MyType vect;
};

$ g++-14 -c repro.cpp -Wextra -fvisibility-inlines-hidden
repro.cpp:9:7: warning: ‘TestClass’ declared with greater visibility than the type of its field ‘TestClass::vect’ [-Wattributes]
    9 | class TestClass {
      |       ^~~~~~~~~

https://gcc.godbolt.org/z/vhfcoaeoc

Now while the warning seems legitimate in its own right, it's probably not an issue we'd like to deal with here. Unfortunately, it seems to be stuck behind a very wide warning option - turning off all warnings about attributes seems like a very large hammer. But 600 warnings during the build is also quite unmanageable.

CC @MaskRay @firewave @DavidSpickett @tstellar @nikic

[firewave]

GitHub shows the wrong ticket in the duplication notice - it is actually #125552.

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Martin Storsjö (mstorsjo)

If compiling Clang with GCC 14 or newer (for Linux), the build produces a very large number of warnings - obscuring any other potential higher value warnings. Building with GCC seldom is warning free anyway, but ideally we'd keep it at least in manageable numbers.

There are around 600 instances of the same warning:

/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::UnusedFileScopedDecls' [-Wattributes]
  841 | class Sema : public SemaBase {
      |       ^~~~
/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::TentativeDefinitions' [-Wattributes]
/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::ExtVectorDecls' [-Wattributes]
/build/llvm-project/clang/include/clang/Sema/Sema.h:841:7: warning: 'clang::Sema' declared with greater visibility than the type of its field 'clang::Sema::DelegatingCtorDecls' [-Wattributes]

(I.e this header gets included in 150 translation units, and each time it produces 4 warnings.)

In principle, this warning actually seems legitimate in this case. The warning is caused by us building with -fvisibility-inlines-hidden. The root cause of the issue can be observed in this reduced testcase:

class SomeClass {
public:
    void method() {}
};

template<typename Source, void (Source::*Loader)()>
class LazyVector { };

class TestClass {
  typedef LazyVector<SomeClass, &SomeClass::method> MyType;
  MyType vect;
};

$ g++-14 -c repro.cpp -Wextra -fvisibility-inlines-hidden
repro.cpp:9:7: warning: ‘TestClass’ declared with greater visibility than the type of its field ‘TestClass::vect’ [-Wattributes]
    9 | class TestClass {
      |       ^~~~~~~~~

https://gcc.godbolt.org/z/vhfcoaeoc

Now while the warning seems legitimate in its own right, it's probably not an issue we'd like to deal with here. Unfortunately, it seems to be stuck behind a very wide warning option - turning off all warnings about attributes seems like a very large hammer. But 600 warnings during the build is also quite unmanageable.

CC @MaskRay @firewave @DavidSpickett @tstellar @nikic

[erichkeane]

It seems like this should be easy enough to fix? It seems to me that we just have to move the definitions of
ReadUnusedFileScopedDecls
ReadTentativeDefinitions
ReadExtVectorDecls and
ReadDelegatingConstructors
in include/clang/Sema/ExternalSemaSource.h

to be out-of-line, ala: https://godbolt.org/z/rv1Yjc6fK

I unfortunately don't have a GCC build I can repro this with locally, so I'll mark this as 'good first issue' and see if someone can take over this.

Alternatively, I could put together a quick candidate patch if @mstorsjo could validate it for me.

[mstorsjo]

It seems like this should be easy enough to fix? It seems to me that we just have to move the definitions of ReadUnusedFileScopedDecls ReadTentativeDefinitions ReadExtVectorDecls and ReadDelegatingConstructors in include/clang/Sema/ExternalSemaSource.h

to be out-of-line, ala: https://godbolt.org/z/rv1Yjc6fK

I unfortunately don't have a GCC build I can repro this with locally, so I'll mark this as 'good first issue' and see if someone can take over this.

Alternatively, I could put together a quick candidate patch if @mstorsjo could validate it for me.

Thanks, that sounds like a reasonable fix! It feels slightly weird/brittle - but I guess this construct is uncommon enough that it doesn’t pop up often anyway. And as a best-effort way to reduce warnings it’s probably good in any case. I’m happy to try out a potential fix!

[erichkeane]

It seems like this should be easy enough to fix? It seems to me that we just have to move the definitions of ReadUnusedFileScopedDecls ReadTentativeDefinitions ReadExtVectorDecls and ReadDelegatingConstructors in include/clang/Sema/ExternalSemaSource.h
to be out-of-line, ala: https://godbolt.org/z/rv1Yjc6fK
I unfortunately don't have a GCC build I can repro this with locally, so I'll mark this as 'good first issue' and see if someone can take over this.
Alternatively, I could put together a quick candidate patch if @mstorsjo could validate it for me.

Thanks, that sounds like a reasonable fix! It feels slightly weird/brittle - but I guess this construct is uncommon enough that it doesn’t pop up often anyway. And as a best-effort way to reduce warnings it’s probably good in any case. I’m happy to try out a potential fix!

Great! Yeah, it perhaps is? A comment explaining why they are out of line would be handy/useful. Feel free to add me and I'll review it ASAP (though note I'm out of office tomorrow/friday, returning Monday).

[mstorsjo]

Ah - to be clear - I’d be happy to try compile it you can provide a fix attempt patch, as I think you offered earlier :-)

[erichkeane]

Ah - to be clear - I’d be happy to try compile it you can provide a fix attempt patch, as I think you offered earlier :-)

Ah shoot, was hoping I'd successfully sniped you :D

See: #157906

I opted to suppress the warnings instead (or attempt to!). Let me know if it was successful.