Skip to content

[libc] lfind_test.cpp: put helpers in an anonymous namespace. #137821

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 29, 2025

Conversation

enh-google
Copy link
Contributor

This matches other tests and allows the tests to be built together (as Android is doing).

This matches other tests and allows the tests to be built together (as Android is doing).
@llvmbot
Copy link
Member

llvmbot commented Apr 29, 2025

@llvm/pr-subscribers-libc

Author: None (enh-google)

Changes

This matches other tests and allows the tests to be built together (as Android is doing).


Full diff: https://github.com/llvm/llvm-project/pull/137821.diff

1 Files Affected:

  • (modified) libc/test/src/search/lfind_test.cpp (+4)
diff --git a/libc/test/src/search/lfind_test.cpp b/libc/test/src/search/lfind_test.cpp
index 00384f7eec14e..8c67bd4aefe1c 100644
--- a/libc/test/src/search/lfind_test.cpp
+++ b/libc/test/src/search/lfind_test.cpp
@@ -9,10 +9,14 @@
 #include "src/search/lfind.h"
 #include "test/UnitTest/Test.h"
 
+namespace {
+
 int compar(const void *a, const void *b) {
   return *reinterpret_cast<const int *>(a) != *reinterpret_cast<const int *>(b);
 }
 
+};
+
 TEST(LlvmLibcLfindTest, SearchHead) {
   int list[3] = {1, 2, 3};
   size_t len = 3;

Copy link

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

Copy link

github-actions bot commented Apr 29, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@lntue lntue changed the title lfind_test.cpp: put helpers in an anonymous namespace. [libc] lfind_test.cpp: put helpers in an anonymous namespace. Apr 29, 2025
Comment on lines 12 to 19
namespace {

int compar(const void *a, const void *b) {
return *reinterpret_cast<const int *>(a) != *reinterpret_cast<const int *>(b);
}

}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
namespace {
int compar(const void *a, const void *b) {
return *reinterpret_cast<const int *>(a) != *reinterpret_cast<const int *>(b);
}
}
static int compar(const void *a, const void *b) {
return *reinterpret_cast<const int *>(a) != *reinterpret_cast<const int *>(b);
}

Honestly, this is more in-line with LLVM's coding style, but I don't know how much libc differs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, i'd have just used static myself but other llvm-libc tests seemed to have an anonymous namespace...

@lntue lntue merged commit a24457e into llvm:main Apr 29, 2025
16 checks passed
@jhuber6
Copy link
Contributor

jhuber6 commented Apr 29, 2025

Is the function supposed to becalled compar?

@enh-google
Copy link
Contributor Author

Is the function supposed to becalled compar?

that looks weird to me, but that does seem to be used consistently throughout the test, implementation, and header.

fwiw, that's also what the original BSD implementation called it.

over in bionic's headers, i call it __comparator because that's probably more useful to someone in a 21st century ide, who can't even comprehend the idea of needing to keep identifier lengths down to 6 characters :-)

bruce tognazzini had a story in one of his books where someone bought a designer jacket in milan, sent it to the far east to be duplicated, and when the first shipment came back wondered why there was a weird brown mark on the cuff of every single one. they contacted the factory, and they said "that was on the original --- we'll send it back for you to see". turns out they'd copied the accidental cigarette burn too...

@enh-google enh-google deleted the enh-google-patch-1 branch May 5, 2025 20:20
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…37821)

This matches other tests and allows the tests to be built together (as
Android is doing).
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…37821)

This matches other tests and allows the tests to be built together (as
Android is doing).
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
…37821)

This matches other tests and allows the tests to be built together (as
Android is doing).
GeorgeARM pushed a commit to GeorgeARM/llvm-project that referenced this pull request May 7, 2025
…37821)

This matches other tests and allows the tests to be built together (as
Android is doing).
Ankur-0429 pushed a commit to Ankur-0429/llvm-project that referenced this pull request May 9, 2025
…37821)

This matches other tests and allows the tests to be built together (as
Android is doing).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants