Ensure that extern allocator declaration is visible before definition
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Do you split want the non-#include changes split out?
compiler-rt/lib/scudo/standalone/string_utils.cpp | ||
---|---|---|
13 | Same thing, common.h already provides it. | |
compiler-rt/lib/scudo/standalone/vector.h | ||
14 | common.h includes string.h already, so this additional include is unnecessary. We have some custom library paths in our build system, so I needed to go through each include. | |
compiler-rt/lib/scudo/standalone/wrappers_c.h | ||
22 ↗ | (On Diff #415970) | -Wmissing-variable-declarations complains about no previous extern declaration for Allocator in wrappers_c.cpp, so the intention was to centralize the duplicate extern declarations between wrappers_c.cpp and wrapper_cpp.cpp, but I think that got lost while splitting the commits, and I missed that there's a SCUDO_PREFIX macro here, so I'll just add a declaration to wrappers_c.cpp instead. |
compiler-rt/lib/scudo/standalone/vector.h | ||
---|---|---|
14 | Unless there is a strong reason to do so, I am for IWYU. |
Revert header file removal
compiler-rt/lib/scudo/standalone/vector.h | ||
---|---|---|
14 | Ok, I've dropped the header removals from this patch |
compiler-rt/lib/scudo/standalone/wrappers_c.h | ||
---|---|---|
22 ↗ | (On Diff #415970) | Cab we move extern to the patch where actually it's necessary? |
compiler-rt/lib/scudo/standalone/wrappers_c.cpp | ||
---|---|---|
25 | @vitalybuka Are you saying to split this change off into another patch? |
compiler-rt/lib/scudo/standalone/common.h | ||
---|---|---|
120 | There is the same in cpp file. | |
compiler-rt/lib/scudo/standalone/wrappers_c.cpp | ||
31–32 | if we need extern, I assumed there is a some new user outside this cpp file. I believe correct approach is how it's done in wrappers_c_bionic.cpp. Separate patch for unrelated stuff will be nice. Actually each of this fixes better to have as a separate patch with "why" in description. |
compiler-rt/lib/scudo/standalone/wrappers_c.cpp | ||
---|---|---|
31–32 | The comment above this line seems to suggest that it's not feasible to have separate allocators for C and C++. I did try it and it seemed to work fine locally, but I suspect there may be other issues that haven't yet appeared. For example, what if a heap allocation from the C++ scudo allocator is freed by C code or vice versa? |
compiler-rt/lib/scudo/standalone/wrappers_c.cpp | ||
---|---|---|
31–32 | I see, I missed cpp file then probably in wrappers_c.h and remove extern in cpp file |
There is the same in cpp file.
I believe the point was to hide declaration from the header to avoid use by mistake.