Skip to content

Add BYN_WARN_UNUSED macro and apply to Name and IString #8973

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.

Sign up for GitHub

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
sbc100 merged 1 commit into from
Aug 6, 2026
+8 −4
Merged

Add BYN_WARN_UNUSED macro and apply to Name and IString #8973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/compiler-support.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,10 @@
#define WASM_BUILTIN_UNREACHABLE __assume(false)
#endif

#if defined(__GNUC__) || defined(__clang__)
#define BYN_WARN_UNUSED [[gnu::warn_unused]]
#else
#define BYN_WARN_UNUSED
#endif

#endif // wasm_compiler_support_h
1 change: 0 additions & 1 deletion src/ir/possible-contents.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -898,7 +898,6 @@ struct InfoCollector
}

void visitCall(Call* curr) {
Name targetName;
if (!Intrinsics(*getModule()).isCallWithoutEffects(curr)) {
// This is just a normal call.
handleDirectCall(curr, curr->target);
Expand Down
2 changes: 1 addition & 1 deletion src/support/istring.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

namespace wasm {

struct IString {
struct BYN_WARN_UNUSED IString {

@kripken kripken Aug 5, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This feels odd to me, to scatter these throughout the codebase. Is there no global flag we can set?

@sbc100 sbc100 Aug 5, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I mean its kind of like llvm/llvm-project#203084.

If there was a flag I guess it would be something like "-falways-assume-ctors-have-no-side-effects" but I'm pretty sure that is never always true so you wouldn't want it to be global.

@kripken kripken Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I see, so this is for the ctors? Can we annotate the ctors directly, then, not the entire class?

@sbc100 sbc100 Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It seems like the way to do this is to annotate the class itself.

Annotating a C++ class with [[gnu::warn_unused]] tells the compiler to issue a -Wunused-variable warning if a variable of that type is instantiated but never explicitly used.

@kripken kripken Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this documented somewhere? I can't seem to find GNU docs.

@sbc100 sbc100 Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The clang docs describe it: https://clang.llvm.org/docs/AttributeReference.html#warn-unused

"""
This attribute is available in both C and C++ language modes but is primarily useful in C++ for classes which have a non-trivial constructor or destructor but act as a value type rather than an RAII type.
"""

@kripken kripken Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks... funny that it is a gnu thing but clang documents it 😄

private:
static const char* interned(std::string_view s);

Expand Down
2 changes: 1 addition & 1 deletion src/support/name.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ namespace wasm {
// TODO: as an optimization, IString values < some threshold could be considered
// numerical indices directly.

struct Name : public IString {
struct BYN_WARN_UNUSED Name : public IString {
Name() : IString() {}
Name(std::string_view str) : IString(str) {}
Name(const char* str) : IString(str) {}
Expand Down
1 change: 0 additions & 1 deletion src/tools/wasm-opt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ willRemoveDebugInfo(const std::vector<OptimizationOptions::PassInfo>& passes) {
//

int main(int argc, const char* argv[]) {
Name entry;
bool emitBinary = true;
bool converge = false;
bool fuzzExecBefore = false;
Expand Down
Loading