This is an archive of the discontinued LLVM Phabricator instance.

[libc++][nfc] Include test_macros.h in more tests.
ClosedPublic

Authored by Mordante on Jan 24 2022, 10:06 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG5d3ab6a2bb36: [libc++][nfc] Include test_macros.h in more tests.
Summary

This should fix the regressions detected in D117992.

Note this should land before D117992, the dependency is added to test
the combination of the two patches.

Depends on D117992

Diff Detail

Event Timeline

Mordante requested review of this revision.Jan 24 2022, 10:06 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 24 2022, 10:06 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne added inline comments.
libcxx/test/std/concepts/concepts.lang/concept.assignable/assignable_from.compile.pass.cpp
26 ↗(On Diff #402579)

I don't understand why this is needed in several of the places where you're including the file. For example, here, I don't see anything that uses a macro in test_macros.h. Am I missing it?

Mordante updated this revision to Diff 403343.Jan 26 2022, 11:12 AM

Addresses review comment.

Mordante marked an inline comment as done.Jan 26 2022, 11:13 AM
Mordante added inline comments.
libcxx/test/std/concepts/concepts.lang/concept.assignable/assignable_from.compile.pass.cpp
26 ↗(On Diff #402579)

As discussed on Discord, there was some confusion about the policy whether or not to always include this header. The header should only be included when needed. So the unneeded occurrences have been removed.

ldionne accepted this revision.Jan 26 2022, 1:16 PM
This revision is now accepted and ready to land.Jan 26 2022, 1:16 PM
This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.