Skip to content

Commit ade1c56

Browse files
steven-vargasteven varga
steven-varga
authored and
steven varga
committed
[#34]:svarga:fix, finish generated.h chicken-and-egg fix and reformat emitted producer indentation
Two changes: * src/h5cpp.cpp — finish the chicken-and-egg started in 76c38d8 (deferred OutputFile open to destructor). The user's source already carries `#include "generated.h"` before the compiler's AST-scan pass runs, so clang's preprocessor errored out before the AST matchers could fire and the producer never got a chance to emit. Two pieces of plumbing fix this: - Inject -DH5CPP_BUILDING_TYPE_INFO into the clang tool's argument list. The h5cpp headers gate their "storage_representation_v<T> == unsupported" static_asserts on this define so they don't fire during the bootstrap pass (where H5CPP_REGISTER_STRUCT hasn't materialised the specialisations yet). Regular user compilation keeps the asserts active. - Map OutputFile to a virtual empty header ("#pragma once\n") so clang sees a stub during the scan; the real generated.h overwrites the on-disk path when the consumer destructs. * src/producer_h5.hpp — reformat the emitted producer output to 4-space namespace-body indentation and collapse a few one-line if/throw blocks. Pure whitespace / style on the generated artifact, no functional change.
1 parent e5d9094 commit ade1c56

2 files changed

Lines changed: 110 additions & 83 deletions

File tree

‎src/h5cpp.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,17 @@ int main(int argc, const char **argv) {
129129
clang::tooling::ClangTool Tool(OptionsParser.getCompilations(),
130130
OptionsParser.getSourcePathList());
131131

132+
// During the AST-scan pass that produces generated.h, the user's
133+
// H5CPP_REGISTER_STRUCT macros haven't fired yet (generated.h is the
134+
// empty virtual stub mapped below). The h5cpp headers gate their
135+
// "storage_representation_v<T> == unsupported" static_asserts on
136+
// H5CPP_BUILDING_TYPE_INFO so they don't fire in this bootstrap context;
137+
// they remain active for regular user compilation where generated.h exists.
138+
Tool.appendArgumentsAdjuster(
139+
clang::tooling::getInsertArgumentAdjuster(
140+
"-DH5CPP_BUILDING_TYPE_INFO",
141+
clang::tooling::ArgumentInsertPosition::BEGIN));
142+
132143
// Issue #32: rewrite [[h5::xxx(...)]] → [[clang::annotate("h5::xxx", ...)]]
133144
// for each source path before Clang sees it.
134145
std::vector<std::string> _h5_attr_storage;
@@ -141,6 +152,18 @@ int main(int argc, const char **argv) {
141152
pb_attr_translator::install_virtual_files(
142153
Tool, OptionsParser.getSourcePathList(), _pb_attr_storage);
143154

155+
// Issue #34 follow-up: break the chicken-and-egg between the source's
156+
// `#include "generated.h"` and the consumer (which writes the file at
157+
// destructor time, per 76c38d8). Without this, clang's preprocessor errors
158+
// out before the AST matchers ever fire and no output is produced.
159+
// Install an empty virtual header at OutputFile; the real content overwrites
160+
// the on-disk file when the consumer destructs.
161+
std::string _empty_generated_storage;
162+
if (!OutputFile.empty()) {
163+
_empty_generated_storage = "#pragma once\n";
164+
Tool.mapVirtualFile(OutputFile, _empty_generated_storage);
165+
}
166+
144167
std::string work_path = OutputFile;
145168
if (CheckMode) {
146169
work_path = OutputFile + ".h5cpp-check";

‎src/producer_h5.hpp‎

Lines changed: 87 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -127,115 +127,113 @@ struct H5Producer : Producer<H5Producer> {
127127
std::replace(ns_name.begin(), ns_name.end(), ':', '_');
128128

129129
// --- generated namespace: row_t + compound_type() ---
130+
// Contents indented one level (4 spaces) inside the namespace block.
130131
if( !doc.empty() ) io << "// doc: \"" << doc << "\"\n";
131132
if( !alias.empty() ) io << "// alias: \"" << alias << "\"\n";
132133
if( !version.empty() ) io << "// version: \"" << version << "\"\n";
133-
io << "namespace h5::generated::" << ns_name << " {\n\n";
134+
io << "namespace h5::generated::" << ns_name << " {\n";
134135

135-
// row_t struct
136-
io << "struct row_t {\n";
136+
// row_t struct (4-space indent for struct, 8-space for members)
137+
io << " struct row_t {\n";
137138
for (const auto& f : fields) {
138139
if (f.is_vlen) {
139140
if (f.is_string) {
140-
io << " char* " << f.cpp_name << ";\n";
141+
io << " char* " << f.cpp_name << ";\n";
141142
} else {
142-
io << " hvl_t " << f.cpp_name << ";\n";
143+
io << " hvl_t " << f.cpp_name << ";\n";
143144
}
144145
} else {
145-
io << " " << f.cpp_type << " " << f.cpp_name << ";\n";
146+
io << " " << f.cpp_type << " " << f.cpp_name << ";\n";
146147
}
147148
}
148-
io << "};\n\n";
149+
io << " };\n";
149150

150-
// compound_type() factory
151-
io << "inline hid_t compound_type() {\n"
152-
<< " static const hid_t ct = []{\n";
151+
// compound_type() factory (4-space for function, 8-space inside, 12-space inside lambda)
152+
io << " inline hid_t compound_type() {\n"
153+
<< " static const hid_t ct = []{\n";
153154

154155
// Emit VLEN / string type helpers for vlen fields
155156
for (const auto& f : fields) {
156157
if (!f.is_vlen) continue;
157158
std::string var = "v_" + f.cpp_name;
158159
if (f.is_string) {
159-
io << " hid_t " << var << " = H5Tcopy(H5T_C_S1);\n"
160-
<< " H5Tset_size(" << var << ", H5T_VARIABLE);\n";
160+
io << " hid_t " << var << " = H5Tcopy(H5T_C_S1);\n"
161+
<< " H5Tset_size(" << var << ", H5T_VARIABLE);\n";
161162
} else {
162-
io << " hid_t " << var << " = H5Tvlen_create(" << f.h5_type << ");\n";
163+
io << " hid_t " << var << " = H5Tvlen_create(" << f.h5_type << ");\n";
163164
}
164165
}
165166

166-
io << " hid_t ct = H5Tcreate(H5T_COMPOUND, sizeof(row_t));\n";
167+
io << " hid_t ct = H5Tcreate(H5T_COMPOUND, sizeof(row_t));\n";
167168
for (const auto& f : fields) {
168169
std::string type_expr;
169170
if (f.is_vlen) {
170171
type_expr = "v_" + f.cpp_name;
171172
} else {
172173
type_expr = f.h5_type;
173174
}
174-
io << " H5Tinsert(ct, \"" << f.h5_name << "\", HOFFSET(row_t, "
175+
io << " H5Tinsert(ct, \"" << f.h5_name << "\", HOFFSET(row_t, "
175176
<< f.cpp_name << "), " << type_expr << ");\n";
176177
}
177-
io << " return ct;\n"
178-
<< " }();\n"
179-
<< " return ct;\n"
180-
<< "}\n\n"
178+
io << " return ct;\n"
179+
<< " }();\n"
180+
<< " return ct;\n"
181+
<< " }\n"
181182
<< "} // namespace h5::generated::" << ns_name << "\n\n";
182183

183184
// --- scatter<T> specialization ---
185+
// Template + body indented one level (4 spaces) inside `namespace h5 {`.
184186
io << "namespace h5 {\n"
185-
<< "template<> inline h5::ds_t scatter<" << record_name << ">(\n"
186-
<< " hid_t fd, const std::string& path, const " << record_name << "& obj) {\n"
187-
<< " using namespace ::h5::generated::" << ns_name << ";\n"
188-
<< " h5::ds_t ds;\n"
189-
<< " h5::mute();\n"
190-
<< " bool exists = H5Lexists(fd, path.c_str(), H5P_DEFAULT) > 0;\n"
191-
<< " h5::unmute();\n";
187+
<< " template<> inline h5::ds_t scatter<" << record_name << ">(\n"
188+
<< " hid_t fd, const std::string& path, const " << record_name << "& obj) {\n"
189+
<< " using namespace ::h5::generated::" << ns_name << ";\n"
190+
<< " h5::ds_t ds;\n"
191+
<< " h5::mute();\n"
192+
<< " bool exists = H5Lexists(fd, path.c_str(), H5P_DEFAULT) > 0;\n"
193+
<< " h5::unmute();\n";
192194
if( on_missing == "error" ){
193-
io << " if (!exists) {\n"
194-
<< " throw std::runtime_error(\"dataset not found: \" + path);\n"
195-
<< " }\n"
196-
<< " ds = h5::open(fd, path, h5::default_dapl);\n";
195+
io << " if (!exists) throw std::runtime_error(\"dataset not found: \" + path);\n"
196+
<< " ds = h5::open(fd, path, h5::default_dapl);\n";
197197
} else if( on_missing == "ignore" ){
198-
io << " if (!exists) {\n"
199-
<< " return ds;\n"
200-
<< " }\n"
201-
<< " ds = h5::open(fd, path, h5::default_dapl);\n";
198+
io << " if (!exists) return ds;\n"
199+
<< " ds = h5::open(fd, path, h5::default_dapl);\n";
202200
} else {
203-
io << " if (exists) {\n"
204-
<< " ds = h5::open(fd, path, h5::default_dapl);\n"
205-
<< " } else {\n"
206-
<< " h5::dcpl_t dcpl{H5Pcreate(H5P_DATASET_CREATE)};\n"
207-
<< " hsize_t chunk = " << chunk_size << ";\n"
208-
<< " H5Pset_chunk(dcpl, 1, &chunk);\n";
201+
// Flipped: larger branch (create) on top; smaller (open) becomes a
202+
// single-statement trailing else with no braces.
203+
io << " if (!exists) {\n"
204+
<< " h5::dcpl_t dcpl{H5Pcreate(H5P_DATASET_CREATE)};\n"
205+
<< " hsize_t chunk = " << chunk_size << ";\n"
206+
<< " H5Pset_chunk(dcpl, 1, &chunk);\n";
209207
if (!compress_algo.empty()) {
210208
if (compress_algo == "gzip") {
211-
io << " H5Pset_deflate(dcpl, " << compress_level << ");\n";
209+
io << " H5Pset_deflate(dcpl, " << compress_level << ");\n";
212210
}
213211
}
214212
io
215-
<< " hsize_t cur = 0;\n"
216-
<< " hsize_t max = H5S_UNLIMITED;\n"
217-
<< " hid_t space = H5Screate_simple(1, &cur, &max);\n"
218-
<< " ds = h5::createds(fd, path, compound_type(), h5::sp_t{space},\n"
219-
<< " h5::default_lcpl, dcpl, h5::default_dapl);\n"
220-
<< " }\n";
213+
<< " hsize_t cur = 0;\n"
214+
<< " hsize_t max = H5S_UNLIMITED;\n"
215+
<< " hid_t space = H5Screate_simple(1, &cur, &max);\n"
216+
<< " ds = h5::createds(fd, path, compound_type(), h5::sp_t{space},\n"
217+
<< " h5::default_lcpl, dcpl, h5::default_dapl);\n"
218+
<< " } else ds = h5::open(fd, path, h5::default_dapl);\n";
221219
}
222-
io << " hsize_t row = h5::detail::next_row(ds);\n";
220+
io << " hsize_t row = h5::detail::next_row(ds);\n";
223221

224222
// Pre-compute vlen helper variables (size + data pointer)
225223
for (const auto& f : fields) {
226224
if (!f.is_vlen) continue;
227-
io << " hsize_t " << f.cpp_name << "_len = obj." << f.cpp_name << ".size();\n";
225+
io << " hsize_t " << f.cpp_name << "_len = obj." << f.cpp_name << ".size();\n";
228226
if (f.is_string) {
229-
io << " const char* " << f.cpp_name << "_ptr = obj." << f.cpp_name << ".c_str();\n";
227+
io << " const char* " << f.cpp_name << "_ptr = obj." << f.cpp_name << ".c_str();\n";
230228
} else {
231-
io << " auto* " << f.cpp_name << "_ptr = obj." << f.cpp_name << ".data();\n";
229+
io << " auto* " << f.cpp_name << "_ptr = obj." << f.cpp_name << ".data();\n";
232230
}
233231
}
234232

235-
io << " row_t r{\n";
233+
io << " row_t r{\n";
236234
for (std::size_t i = 0; i < fields.size(); ++i) {
237235
const auto& f = fields[i];
238-
io << " ";
236+
io << " ";
239237
if (f.is_vlen) {
240238
if (f.is_string) {
241239
io << "(char*)" << f.cpp_name << "_ptr";
@@ -247,58 +245,64 @@ struct H5Producer : Producer<H5Producer> {
247245
}
248246
io << (i + 1 < fields.size() ? ",\n" : "\n");
249247
}
250-
io << " };\n";
248+
io << " };\n";
251249

252-
io << " herr_t err = h5::detail::write_one_row(ds, compound_type(), row, &r);\n"
253-
<< " (void)err;\n"
254-
<< " return ds;\n"
255-
<< "}\n"
250+
io << " herr_t err = h5::detail::write_one_row(ds, compound_type(), row, &r);\n"
251+
<< " (void)err;\n"
252+
<< " return ds;\n"
253+
<< " }\n"
256254
<< "} // namespace h5\n\n";
257255

258256
// --- gather<T> specialization ---
257+
// Same indentation scheme as scatter above.
259258
io << "namespace h5 {\n"
260-
<< "template<> inline void gather<" << record_name << ">(\n"
261-
<< " hid_t fd, const std::string& path, " << record_name << "& obj) {\n"
262-
<< " using namespace ::h5::generated::" << ns_name << ";\n";
259+
<< " template<> inline void gather<" << record_name << ">(\n"
260+
<< " hid_t fd, const std::string& path, " << record_name << "& obj) {\n"
261+
<< " using namespace ::h5::generated::" << ns_name << ";\n";
263262
if( on_missing == "error" || on_missing == "ignore" ){
264-
io << " h5::mute();\n"
265-
<< " bool exists = H5Lexists(fd, path.c_str(), H5P_DEFAULT) > 0;\n"
266-
<< " h5::unmute();\n";
263+
io << " h5::mute();\n"
264+
<< " bool exists = H5Lexists(fd, path.c_str(), H5P_DEFAULT) > 0;\n"
265+
<< " h5::unmute();\n";
267266
if( on_missing == "error" ){
268-
io << " if (!exists) {\n"
269-
<< " throw std::runtime_error(\"dataset not found: \" + path);\n"
270-
<< " }\n";
267+
io << " if (!exists) throw std::runtime_error(\"dataset not found: \" + path);\n";
271268
} else {
272-
io << " if (!exists) {\n"
273-
<< " return;\n"
274-
<< " }\n";
269+
io << " if (!exists) return;\n";
275270
}
276271
}
277-
io << " h5::ds_t ds = h5::open(fd, path, h5::default_dapl);\n"
278-
<< " hsize_t nrows = h5::detail::next_row(ds);\n"
279-
<< " if (nrows == 0) return;\n"
280-
<< " row_t r{};\n"
281-
<< " herr_t err = h5::detail::read_one_row(ds, compound_type(), nrows - 1, &r);\n"
282-
<< " (void)err;\n";
272+
io << " h5::ds_t ds = h5::open(fd, path, h5::default_dapl);\n"
273+
<< " hsize_t nrows = h5::detail::next_row(ds);\n"
274+
<< " if (nrows == 0) return;\n"
275+
<< " row_t r{};\n"
276+
<< " herr_t err = h5::detail::read_one_row(ds, compound_type(), nrows - 1, &r);\n"
277+
<< " (void)err;\n";
283278

284279
for (const auto& f : fields) {
285280
if (f.is_vlen) {
286281
if (f.is_string) {
287-
io << " if (r." << f.cpp_name << ") obj." << f.cpp_name << ".assign(r."
282+
io << " if (r." << f.cpp_name << ") obj." << f.cpp_name << ".assign(r."
288283
<< f.cpp_name << ");\n";
289284
} else {
290-
io << " obj." << f.cpp_name << ".assign(static_cast<" << f.cpp_type
285+
io << " obj." << f.cpp_name << ".assign(static_cast<" << f.cpp_type
291286
<< "*>(r." << f.cpp_name << ".p), static_cast<" << f.cpp_type
292287
<< "*>(r." << f.cpp_name << ".p) + r." << f.cpp_name << ".len);\n";
293288
}
294289
} else {
295-
io << " obj." << f.cpp_name << " = r." << f.cpp_name << ";\n";
290+
io << " obj." << f.cpp_name << " = r." << f.cpp_name << ";\n";
296291
}
297292
}
298-
io << " hid_t reclaim_space = H5Screate(H5S_SCALAR);\n"
299-
<< " H5Dvlen_reclaim(compound_type(), reclaim_space, H5P_DEFAULT, &r);\n"
300-
<< " H5Sclose(reclaim_space);\n"
301-
<< "}\n"
293+
// Use the HDF5 version-aware reclaim API:
294+
// HDF5 >= 1.12: H5Treclaim (canonical name; the old H5Dvlen_reclaim is
295+
// deprecated and may be hidden when the library is built
296+
// with H5_NO_DEPRECATED_SYMBOLS).
297+
// HDF5 < 1.12: H5Dvlen_reclaim (the only available spelling).
298+
io << " hid_t reclaim_space = H5Screate(H5S_SCALAR);\n"
299+
<< " #if H5_VERSION_GE(1,12,0)\n"
300+
<< " H5Treclaim(compound_type(), reclaim_space, H5P_DEFAULT, &r);\n"
301+
<< " #else\n"
302+
<< " H5Dvlen_reclaim(compound_type(), reclaim_space, H5P_DEFAULT, &r);\n"
303+
<< " #endif\n"
304+
<< " H5Sclose(reclaim_space);\n"
305+
<< " }\n"
302306
<< "} // namespace h5\n\n";
303307

304308
// --- registration macro ---

0 commit comments

Comments
 (0)