From 0661b27c749e303f143e285fc57f723e393f4b41 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Fri, 2 Oct 2026 00:02:40 +0200 Subject: [PATCH] clean-up: resolve some Clang conformance divergences (see #928) Knowledge-Technology(Claude Opus 5) used - to analyse and reassess the situation - to discuss and weigh the findings and develop solutions - to perform the selected mitigation and verify success ---- = Report by Claude = A sweep with Clang-22 over all 597 translation units (driven from the newly generated compilation database) showed 271 of them failing, yet with the 2502 diagnostics tracing back to very few root sites. This resolves those where GCC is simply the more permissive of the two; the remaining clusters are a matter of design and are left alone for now. * `using Alias::OriginalName;` does *not* declare inherited constructors. Per [namespace.udecl] a using-declarator names constructors only when its terminal name equals the last component of the nested-name-specifier -- so through an alias, the terminal name is read as the injected-class-name, i.e. a type, and Clang then demands `typename`. Clang is right here. Affects `hetero-data.hpp` (19 TUs) and `iter-source.hpp` (62 TUs); in the latter the alias is renamed `_I` -> `Adapter` while we are at it. Note `iter-source` as a whole awaits a design review, see #1125 * P1787R6 requires a template-argument-list after the `template` keyword. `holder.template emplace (...)` in `opaque-holder.hpp` and two sites in `node-builder.hpp` supply none, so the keyword has to go (83 TUs). * A default template argument may be given only once per scope. The out-of-class definition of `OutputSlot::AllocState` repeated the default from its declaration in 'output-slot.hpp' -- note the friend-declaration further down in the same header already got this right. * `constexpr auto ROUGH_PRECISION = pow (10, -3)` is not a constant expression by the standard; only GCC accepts it, through a builtin. Hence `util::ipow()` in 'util-quant.hpp', an exponentiation with integral exponent by square-and-multiply, as a counterpart to `ilog2()`. Exact for representable base and result, and the static_assert documents that `ipow(10.0,-3)` is bit-identical to `1e-3`. 193 TUs depended on this single line. Furthermore, two findings made while surveying: * `lib/stat/statistic.hpp` held a second, dormant instance of the same `pow` problem, in a `round()` helper that nothing in Lumiera ever instantiates -- which is why Clang never saw it. Since the helper is also the naive and imprecise formulation, it is removed here; a note records that it stays in use upstream in yoshimi-test, where this header comes from, and that decimal rounding belongs to #1360 if ever wanted here. * The alias `DataBuilder` was defined twice, identically, in 'weaving-pattern-builder.hpp' and in 'node-builder.hpp' which includes the former. Both compilers tolerate the duplicate definition and only Clang objects to the repeated default argument, which is why this went unnoticed. The duplicate is dropped and its documentation moved to the surviving definition -- together with a @todo, since `DataBuilder`, `SizMark` and `UseHeapAlloc` are vocabulary of the node-builder DSL and reside where they do by accident of include order. --- src/lib/hetero-data.hpp | 2 +- src/lib/iter-source.hpp | 6 +-- src/lib/opaque-holder.hpp | 2 +- src/lib/stat/statistic.hpp | 25 +++++++------ src/lib/util-quant.hpp | 39 +++++++++++++++++++- src/steam/engine/node-builder.hpp | 18 ++------- src/steam/engine/weaving-pattern-builder.hpp | 11 ++++++ src/test/test-helper.hpp | 3 +- src/vault/out/output-slot-connection.hpp | 2 +- 9 files changed, 74 insertions(+), 34 deletions(-) diff --git a/src/lib/hetero-data.hpp b/src/lib/hetero-data.hpp index 900a24874..b0ecd3860 100644 --- a/src/lib/hetero-data.hpp +++ b/src/lib/hetero-data.hpp @@ -108,7 +108,7 @@ namespace lib { { using Tuple = std::tuple; - using Tuple::tuple; + using Tuple::Tuple; template void linkInto (HeteroData&); diff --git a/src/lib/iter-source.hpp b/src/lib/iter-source.hpp index 352a88f66..baaa651d7 100644 --- a/src/lib/iter-source.hpp +++ b/src/lib/iter-source.hpp @@ -138,10 +138,10 @@ namespace lib { struct iterator : IterAdapter { ////////////////////////////////////////////////////TICKET #1125 : should be build on top of IterStateWrapper rather than IterAdapter! - using _I = IterAdapter; - using _I::IterAdapter; + using Adapter = IterAdapter; + using Adapter::Adapter; // expose inherited ctors - operator string() const {return _I::source()? string(*_I::source()) : "⟂"; } + operator string() const {return Adapter::source()? string(*Adapter::source()) : "⟂"; } }; /** build an iterator frontend for the given source, diff --git a/src/lib/opaque-holder.hpp b/src/lib/opaque-holder.hpp index f6ea4d0aa..c36553f5b 100644 --- a/src/lib/opaque-holder.hpp +++ b/src/lib/opaque-holder.hpp @@ -810,7 +810,7 @@ namespace lib { using Holder = InPlaceBuffer; Holder& holder = *static_cast (buffer_); - return holder.template emplace (std::forward (implementation)); + return holder.emplace (std::forward (implementation)); } /** Abbreviation for placement new of a subclass SUB into the opaque buffer*/ diff --git a/src/lib/stat/statistic.hpp b/src/lib/stat/statistic.hpp index c4326938f..c768d0576 100644 --- a/src/lib/stat/statistic.hpp +++ b/src/lib/stat/statistic.hpp @@ -17,7 +17,19 @@ ** - average over the N last elements in a data sequence ** - simple linear regression with weights (single predictor variable) ** - also over a time series with zero-based indices - ** + ** + ** @remark this header was imported from the [yoshimi-test] acceptance suite, where it + ** is maintained as `src/util/statistic.hpp` within namespace `util` -- so the + ** two copies are kept deliberately similar, to allow carrying fixes either way. + ** Note in particular that a helper `round(double)` for decimal rounding + ** was **removed here** (10/2026): it was never instantiated within Lumiera, while + ** its `constexpr double shift{pow(10.0,places)}` is not a constant expression by + ** the standard (while GCC accepts it, relying on a _builtin_). It remains in use + ** upstream, in `suite/step/TimingObservation.cpp`. Should decimal rounding be + ** wanted here, settle it together with the other half-solutions collected in #1360 + ** -- and note util::ipow() now provides the constexpr exponentiation. + ** + ** [yoshimi-test]: https://github.com/Ichthyostega/yoshimi-test/ */ @@ -70,17 +82,6 @@ namespace stat{ return std::apply (makeArray, forward (tuple)); } - template - inline double - round (double val) - { - constexpr double shift{pow(10.0, places)}; - return std::round(val*shift) / shift; - } - - - - /** * Read-only view into a segment within a sequence of data * @tparam D value type of the data series diff --git a/src/lib/util-quant.hpp b/src/lib/util-quant.hpp index a171bce1c..a3c40b052 100644 --- a/src/lib/util-quant.hpp +++ b/src/lib/util-quant.hpp @@ -224,6 +224,43 @@ namespace util { return logB; } - + + + /** + * Exponentiation with integral exponent (counterpart to #ilog2). + * @param base the value to be raised + * @param exponent integral exponent; a negative exponent yields the reciprocal + * @remark `std::pow` can **not** be used in a constant expression: the standard does + * not mark it `constexpr` and only GCC accepts it there, through a _builtin_. + * Besides, for a small integral exponent, repeated multiplication is exact + * whenever base and result are representable -- unlike the generic + * implementation by `exp(ln(base)·exponent)`. + * @note the implementation uses [square-and-multiply], so it performs `ilog2(exponent)` squarings; + * the reciprocal is formed only once, on the final result, and is thus correctly rounded + * @see ilog2() + * [square-and-multiply]: https://en.wikipedia.org/wiki/Exponentiation_by_squaring + */ + template + inline constexpr F + ipow (F base, int exponent) noexcept + { + bool reciprocal = exponent < 0; + uint64_t exp = reciprocal? -int64_t(exponent) // (widen first, to cover INT_MIN) + : int64_t(exponent); + F res{1}; + for ( ; exp; exp >>= 1, base *= base) + if (exp & 1) + res *= base; + return reciprocal? F(1)/res + : res; + } + + static_assert (1.0 == ipow (10.0, 0)); + static_assert (1e3 == ipow (10.0, 3)); + static_assert (1e-3 == ipow (10.0,-3)); // ◁───── exactly the nearest double to 0.001 + static_assert (0.25 == ipow ( 2.0,-2)); + static_assert (3125 == ipow ( 5.0, 5)); + + } // namespace util #endif /*UTIL_QUANT_H*/ diff --git a/src/steam/engine/node-builder.hpp b/src/steam/engine/node-builder.hpp index 955867244..0efe12210 100644 --- a/src/steam/engine/node-builder.hpp +++ b/src/steam/engine/node-builder.hpp @@ -155,16 +155,6 @@ namespace engine { - /** - * A builder to collect working data. - * Implemented through a suitable configuration of lib::SeveralBuilder, - * with a policy configuration parameter to define the allocator to use. - */ - template - using DataBuilder = lib::SeveralBuilder; - - - template class NodeBuilder; @@ -506,8 +496,8 @@ namespace engine { closeParamFront (PAR v1, PARS ...vs) { return adaptParam( - WAB::ParamClosure::template closeFront (forward (v1) - ,forward(vs)...)); + WAB::ParamClosure::closeFront (forward (v1) + ,forward(vs)...)); } /** immediately close the rightmost parameter positions, @@ -517,8 +507,8 @@ namespace engine { closeParamBack (PAR v1, PARS ...vs) { return adaptParam( - WAB::ParamClosure::template closeBack (forward (v1) - ,forward(vs)...)); + WAB::ParamClosure::closeBack (forward (v1) + ,forward(vs)...)); } /** immediately close a single parameter at designated position diff --git a/src/steam/engine/weaving-pattern-builder.hpp b/src/steam/engine/weaving-pattern-builder.hpp index eb2937a4e..9ae2cd565 100644 --- a/src/steam/engine/weaving-pattern-builder.hpp +++ b/src/steam/engine/weaving-pattern-builder.hpp @@ -163,6 +163,17 @@ namespace engine { + /** + * A builder to collect working data. + * Implemented through a suitable configuration of lib::SeveralBuilder, + * with a policy configuration parameter to define the allocator to use. + * @todo 10/2026 extract the builder-DSL definitions? `DataBuilder` and `SizMark` + * are vocabulary of the node-builder language and are used from + * 'node-builder.hpp' as well -- they reside here merely because this is + * the upstream header in the include order, not because the concepts + * belong to the weaving pattern. The same holds for `UseHeapAlloc`, + * which sits in an anonymous namespace over there. + */ template using DataBuilder = lib::SeveralBuilder; diff --git a/src/test/test-helper.hpp b/src/test/test-helper.hpp index 7cb136b60..b5fc2ed9f 100644 --- a/src/test/test-helper.hpp +++ b/src/test/test-helper.hpp @@ -33,6 +33,7 @@ #include "lib/symbol.hpp" #include "lib/meta/trait.hpp" +#include "lib/util-quant.hpp" #include "lib/time/timevalue.hpp" #include "test/transiently.hpp" #include "lib/format-obj.hpp" @@ -62,7 +63,7 @@ namespace test{ using util::showTypes; - constexpr auto ROUGH_PRECISION = pow (10, -3); + constexpr auto ROUGH_PRECISION = util::ipow (10.0, -3); constexpr auto EPSILON_ULP = 5; diff --git a/src/vault/out/output-slot-connection.hpp b/src/vault/out/output-slot-connection.hpp index d4eb91f7c..53cbd8a96 100644 --- a/src/vault/out/output-slot-connection.hpp +++ b/src/vault/out/output-slot-connection.hpp @@ -167,7 +167,7 @@ namespace out { * handle is retained. * @tparam isTest use a simple BufferMetadata table in OutputBufferProxy. */ - template + template // ◁───── default defined at declaration (output-slot.hpp) class OutputSlot::AllocState : public OutputSlot::Allocation {