Skip to content

Conversation

@mikomikotaishi
Copy link

@mikomikotaishi mikomikotaishi commented Dec 5, 2025

Summary

Several of the C++ source files have inconsistent styles (such as varying alignment for pointer asterisks). This PR aims to eliminate the differences so that the code base is more consistent.

This also adds the [[nodiscard]] attribute to several methods in the code base, especially those which return a value and should be considered.

How did you test this change?

Run the following from ./scripts/perf-counters:

make

Output:

mkdir -p build
g++ -std=c++11 -I/usr/include/webkitgtk-1.0/ -ljavascriptcoregtk-1.0 src/jsc-perf.cpp src/hardware-counter.cpp src/thread-local.cpp -o build/jsc-perf
src/jsc-perf.cpp: In function ‘const OpaqueJSValue* js_perf_counters_init(JSContextRef, JSObjectRef, JSObjectRef, size_t, const OpaqueJSValue* const*, const OpaqueJSValue**)’:
src/jsc-perf.cpp:92:38: warning: ignoring return value of ‘T* HPHP::ThreadLocalNoCheck<T>::getCheck() const [with T = HPHP::HardwareCounter]’, declared with attribute ‘nodiscard’ [-Wunused-result]
   92 |   HardwareCounter::s_counter.getCheck();
      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
In file included from src/hardware-counter.h:11,
                 from src/jsc-perf.cpp:18:
src/thread-local.h:298:4: note: declared here
  298 | T* ThreadLocalNoCheck<T>::getCheck() const {
      |    ^~~~~~~~~~~~~~~~~~~~~

There is now a warning for the line HardwareCounter::s_counter.getCheck(), but I'm not sure what to replace it with.

Aside from these changes, there is no change to the logic or code itself.

@meta-cla meta-cla bot added the CLA Signed label Dec 5, 2025
@mikomikotaishi
Copy link
Author

I'm not sure why the Vercel workflow is failing, I've clicked on it to authorise but nothing seems to happen.

@eps1lon eps1lon closed this Dec 6, 2025
@mikomikotaishi
Copy link
Author

@eps1lon May I get a reason/explanation why this was closed?

@eps1lon
Copy link
Collaborator

eps1lon commented Dec 6, 2025

Sorry, the code hasn't been used for the longest time and I assumed this is AI slop 😬 We'll be removing it instead.

@mikomikotaishi
Copy link
Author

mikomikotaishi commented Dec 6, 2025

I assumed this is AI slop

That doesn't make any sense, why would anyone need to use AI to add [[nodiscard]] to a few function signatures or move the placement of an asterisk? It probably takes more time to feed all of this into an AI and ask for it and await a response than it does to just do it manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants