This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Enable -Wunused-template
ClosedPublic

Authored by philnik on Feb 23 2023, 12:17 PM.

Details

Reviewers
ldionne
Mordante
var-const
EricWF
Group Reviewers
Restricted Project
Restricted Project
Commits
rGe65cd4ce832b: [libc++] Enable -Wunused-template
Summary

Clang wants to enable this flag by default, but libc++ isn't working with it yet.

Diff Detail

Event Timeline

philnik created this revision.Feb 23 2023, 12:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2023, 12:17 PM
Herald added a subscriber: arichardson. · View Herald Transcript
philnik requested review of this revision.Feb 23 2023, 12:17 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 23 2023, 12:18 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik updated this revision to Diff 500169.Feb 24 2023, 5:53 AM

Try to fix CI

philnik added inline comments.Feb 24 2023, 5:54 AM
libcxx/test/std/time/time.hms/time.hms.nonmembers/ostream.pass.cpp
56 ↗(On Diff #500169)

@Mordante Did you forget to use this function while implementing the test?

Mordante accepted this revision.Feb 24 2023, 8:22 AM

LGTM, please wait for D144737 before landing this.

libcxx/test/std/time/time.hms/time.hms.nonmembers/ostream.pass.cpp
56 ↗(On Diff #500169)

Yes the good old copy paste bugs ;-) See D144737 for the fix.

This revision is now accepted and ready to land.Feb 24 2023, 8:22 AM

FYI I landed the Japanese fixes.

ldionne added inline comments.Feb 27 2023, 10:34 AM
libcxx/include/__algorithm/make_projected.h
63–76

Those are templates, so they are implicitly inline. I would suggest not marking them at all.

89

Here too.

libcxx/include/__memory/shared_ptr.h
436–439

Here too.

libcxx/include/__type_traits/is_nothrow_convertible.h
30–33

Here too.

libcxx/src/filesystem/filesystem_common.h
51

Which functions in this file trigger the warning?

philnik updated this revision to Diff 501156.Feb 28 2023, 8:24 AM
philnik marked 5 inline comments as done.

Address comments

Herald added a project: Restricted Project. · View Herald TranscriptFeb 28 2023, 8:24 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
This revision now requires review to proceed.Feb 28 2023, 8:24 AM
philnik added inline comments.Feb 28 2023, 8:24 AM
libcxx/src/filesystem/filesystem_common.h
51

I don't know exactly, but this has to be cleaned up anyways. Currently, everything in this file is inside an anonymous namespace and marked static. I don't know why it's this way, but I don't think there is a good reason for it.

EricWF accepted this revision.Mar 7 2023, 1:51 PM
This revision is now accepted and ready to land.Mar 7 2023, 1:51 PM
EricWF added a comment.Mar 7 2023, 1:52 PM

How is libc++ not working with it yet a blocker for them? We disable warnings in our headers except outside of our build?

How is libc++ not working with it yet a blocker for them? We disable warnings in our headers except outside of our build?

TBH I don't know exactly why. It's a good idea to enable anyways.

philnik updated this revision to Diff 503350.Mar 8 2023, 6:25 AM

Try to fix CI

philnik updated this revision to Diff 503393.Mar 8 2023, 8:35 AM

Next try

This revision was landed with ongoing or failed builds.Mar 8 2023, 10:27 AM
This revision was automatically updated to reflect the committed changes.