Skip to content

Conversation

@danilaml
Copy link
Collaborator

@danilaml danilaml commented Nov 24, 2025

Adds an explicit include of <cassert> in StringTable.h rather than relying on the one in StringRef.h. Fixes potential compile errors if assert() was undef'ed between StringRef.h and StringTable.h inclusion.

Noticed the issue after c9f5734.

Adds an explicit include of <cassert> in StringTable.h rather than
relying on the one in StringRef.h. Fixes potential compile errors if
assert() was undef'ed between StringRef.h and StringTable.h inclusion.
@llvmbot
Copy link
Member

llvmbot commented Nov 24, 2025

@llvm/pr-subscribers-llvm-adt

Author: Danila Malyutin (danilaml)

Changes

Adds an explicit include of <cassert> in StringTable.h rather than relying on the one in StringRef.h. Fixes potential compile errors if assert() was undef'ed between StringRef.h and StringTable.h inclusion.

Noticed the issue after c9f5734.


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

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/StringTable.h (+1)
diff --git a/llvm/include/llvm/ADT/StringTable.h b/llvm/include/llvm/ADT/StringTable.h
index 9422a6da1ce8e..3a08e56e8f501 100644
--- a/llvm/include/llvm/ADT/StringTable.h
+++ b/llvm/include/llvm/ADT/StringTable.h
@@ -11,6 +11,7 @@
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator.h"
+#include <cassert>
 #include <iterator>
 #include <limits>
 

@arsenm
Copy link
Contributor

arsenm commented Nov 24, 2025

assert() was undef'ed between StringRef.h and StringTable.h inclusion.

Is it even legal to do that?

@danilaml
Copy link
Collaborator Author

danilaml commented Nov 24, 2025

@arsenm can't see why not. It's not a great idea but it's not a reserved name or anything. In any case, relying on transitive includes is not ideal.

@danilaml danilaml merged commit 51d93e7 into llvm:main Nov 24, 2025
12 checks passed
@danilaml danilaml deleted the fix-missing-include branch November 24, 2025 20:48
@arsenm
Copy link
Contributor

arsenm commented Nov 24, 2025

@arsenm can't see why not. It's not a great idea but it's not a reserved name or anything. In any case, relying on transitive includes is not ideal.

C++11 states: "A translation unit that includes a standard library header shall not #define or #undef names declared in
any standard library header."

@danilaml
Copy link
Collaborator Author

@arsenm doesn't change much in practice. Just like windows headers having min/max macros, unity builds and countless other such things on a smaller scale (wouldn't surprise me if there is some enum value or macro somewhere in llvm sources that "conflicts" with some std name).

aadeshps-mcw pushed a commit to aadeshps-mcw/llvm-project that referenced this pull request Nov 26, 2025
Adds an explicit include of `<cassert>` in StringTable.h rather than
relying on the one in StringRef.h. Fixes potential compile errors if
assert() was undef'ed between StringRef.h and StringTable.h inclusion.
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