From c71089abe9cb4ad169bccfb7cdb8d0d42523a148 Mon Sep 17 00:00:00 2001 From: Dario Lipicar Date: Tue, 2 Jun 2026 15:54:54 -0300 Subject: [PATCH] remove legacy emitEvent (#69) --- README.md | 2 -- cpp-generator/docs/project.md | 6 ++---- cpp-generator/docs/spec.md | 5 +---- .../experimental/impl_header_parser.cpp | 14 +++++--------- cpp-generator/experimental/lidl_ast.h | 7 +------ .../experimental/lidl_gen_provider.cpp | 18 ------------------ tests/experimental/fixtures/universal_impl.h | 3 +++ tests/experimental/test_impl_header_parser.cpp | 7 +++++-- tests/experimental/test_lidl_gen_provider.cpp | 18 +++--------------- 9 files changed, 20 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 10aedcb..069bea4 100644 --- a/README.md +++ b/README.md @@ -300,8 +300,6 @@ modules().my_module.onUserLoggedIn( The accessor's parameter types follow the consumer's own `--api-style` (so a `universal` consumer sees `const std::string&` / `int64_t`, a handcrafted Qt consumer sees `const QString&` / `int`). -**Legacy** — modules that haven't migrated keep the old `std::function emitEvent` member working. The codegen still detects it and wires the lambda in the provider constructor. New code should prefer `logos_events:`. - ### API #### LogosResult diff --git a/cpp-generator/docs/project.md b/cpp-generator/docs/project.md index 0ea2ea3..e160149 100644 --- a/cpp-generator/docs/project.md +++ b/cpp-generator/docs/project.md @@ -39,7 +39,7 @@ Shared data model used by all pipelines: - **`EventDecl`** — event name + params - **`FieldDecl`** — struct field name, type, optional flag - **`TypeDecl`** — named struct type with fields -- **`ModuleDecl`** — complete module: name, version, description, category, depends, types, methods, events, `hasEmitEvent` flag +- **`ModuleDecl`** — complete module: name, version, description, category, depends, types, methods, events All types have `operator==` for testing. @@ -125,7 +125,6 @@ Flag plumbing: - `lidlIsStdConvertible(TypeExpr)` — checks if a type has a pure C++ representation - `lidlMakeProviderHeader(ModuleDecl, implClass, implHeader)` — generates Qt glue header - Emits `nlohmannToQVariant()` helper when any method has `jsonReturn = true` - - Legacy path: wires `m_impl.emitEvent` → `LogosProviderBase::emitEvent` in the constructor when `hasEmitEvent` is set (un-migrated modules using the old `std::function emitEvent` member) - Always emits an `onInit(LogosAPI*) override` that, via SFINAE'd helpers in `logos_module_context.h`, (a) copies the three runtime-injected properties (`modulePath`, `instanceId`, `instancePersistencePath`) into the impl, (b) constructs a per-module `LogosModules` aggregate and threads its pointer through the same base, and (c) installs the typed-event callback (`maybeSetEmitEvent`) consumed by `_events.cpp` method bodies. Impls that don't inherit `LogosModuleContext` compile unchanged — the helper overloads collapse to no-ops. The full `LogosAPI` is never exposed past the provider boundary. - Always emits `#include "logos_sdk.h"` and a `std::unique_ptr m_logosModules` member; ownership lives on the provider, the context base sees only a non-owning `void*` reinterpreted in `LogosModuleContext::modules()` (which depends on the impl's TU having included `logos_sdk.h`). - `lidlMakeProviderDispatch(ModuleDecl)` — generates callMethod/getMethods dispatch @@ -138,7 +137,6 @@ Flag plumbing: - State machine: `LookingForClass` → `InClass` → `InPublic`/`InPrivate`/`InLogosEvents` - The literal `logos_events:` token (defined in `logos_module_context.h` as `#define logos_events public`) opens an events section; bare prototypes inside become `EventDecl{name, params}` entries appended to `ModuleDecl.events` - Skips: constructors, destructors, typedefs, using, friend, enum, struct, `std::function` declarations -- Legacy: still detects `std::function<...> emitEvent` members and sets `ModuleDecl.hasEmitEvent = true` so un-migrated modules keep working through the provider constructor's lambda wiring - Recognizes `LogosMap` and `LogosList` return types (nlohmann::json aliases) and sets `MethodDecl.jsonReturn = true` - Template-aware parameter splitting (handles `std::vector` correctly) @@ -242,7 +240,7 @@ Fixture files in `tests/experimental/fixtures/`: - Method definitions in the header (only declarations ending with `;`) - Nested classes - Template methods - - `std::function` members other than `emitEvent` are silently skipped + - `std::function` members are silently skipped (never treated as methods) - LIDL does not support generic/parameterized types or inheritance - Only the `qt` backend is implemented for `--from-header`; future backends (CBOR, Rust) are planned - Client stub generation (`lidlMakeHeader`/`lidlMakeSource`) is only available from LIDL files, not from `--from-header` diff --git a/cpp-generator/docs/spec.md b/cpp-generator/docs/spec.md index f594dec..e1c7f36 100644 --- a/cpp-generator/docs/spec.md +++ b/cpp-generator/docs/spec.md @@ -18,7 +18,7 @@ The goal is to decouple module business logic from the Qt framework. Module auth | **Dispatch** | The generated `callMethod()` function that maps string method names to typed method calls on the provider object | | **Impl Header** | The pure C++header file (`_impl.h`) that declares a module's public methods using standard C++ types | | **TypeExpr** | The AST node representing a type in the LIDL type system | -| **ModuleDecl** | The AST node representing a complete module declaration (name, version, methods, events, types, `hasEmitEvent` flag) | +| **ModuleDecl** | The AST node representing a complete module declaration (name, version, methods, events, types) | ## Domain Model @@ -196,8 +196,6 @@ logos_events: // expands to `public:`; recog `buildPlugin.nix` ships this at `$out/share/logos/.lidl`. `buildHeaders.nix` passes it to the consumer-side codegen via `--events-from`, which adds typed `on(callback)` accessors to the generated `` wrapper (one per declared event, callback-arg types respect `--api-style`). -**Legacy backward-compat**: the older `std::function emitEvent` member is still detected by the parser and wired in the provider constructor — un-migrated modules (e.g. logos-package-manager-module) keep working through their existing `emitEvent("name", "json")` call sites. New code should prefer `logos_events:`. - Module metadata (name, version, description, dependencies) still comes from `metadata.json`, not from the header. ### Generated Output @@ -211,7 +209,6 @@ Contains two classes: - Call `m_impl.method(...)` - C++ std return → Qt return (e.g., `QString::fromStdString(result)`) - For `jsonReturn` methods (returning `LogosMap`/`LogosList`), the glue calls a generated `nlohmannToQVariant()` recursive helper to convert `nlohmann::json` → `QVariant`/`QVariantMap`/`QVariantList` - - If the impl declares an `emitEvent` callback (`hasEmitEvent`), the constructor wires it to `LogosProviderBase::emitEvent` - Always overrides `onInit(LogosAPI*)` to (a) copy the three runtime-injected properties (`modulePath`, `instanceId`, `instancePersistencePath`) into the impl when it inherits from `LogosModuleContext`, and (b) construct a per-module `LogosModules` (from `generated_code/logos_sdk.h`) owned by the provider, threading its pointer through the same context base. Both wire-ups go through SFINAE'd helpers in `logos_module_context.h` (`_logos_codegen_::maybeSetContext` / `maybeSetLogosModules`), so non-inheriting impls compile unchanged and the `LogosAPI` never escapes the provider. 2. **Plugin** — `QObject` subclass implementing `PluginInterface` and `LogosProviderPlugin`. Carries `Q_PLUGIN_METADATA` and `Q_INTERFACES`. Its `createProviderObject()` factory returns a new ProviderObject instance. diff --git a/cpp-generator/experimental/impl_header_parser.cpp b/cpp-generator/experimental/impl_header_parser.cpp index 896d469..a769869 100644 --- a/cpp-generator/experimental/impl_header_parser.cpp +++ b/cpp-generator/experimental/impl_header_parser.cpp @@ -344,15 +344,11 @@ ImplParseResult parseImplHeader(const QString& headerPath, if (state != InPublic) break; if (line.contains("std::function<")) { - // Legacy `std::function<…> emitEvent` member — predates - // the typed `logos_events:` mechanism. Still recognised - // for backward compat: modules that haven't migrated yet - // (e.g. logos-package-manager-module) keep their existing - // emit("name", "json") call sites working through the - // provider constructor's lambda wiring. New code should - // prefer `logos_events:`. - if (line.contains("emitEvent")) - result.module.hasEmitEvent = true; + // A std::function member is not a method — skip it so the + // `parseMethodLine` path below doesn't choke on the nested + // parens in its type. (Events are declared in a typed + // `logos_events:` section, parsed above — there is no longer + // any special `std::function emitEvent` member to detect.) break; } diff --git a/cpp-generator/experimental/lidl_ast.h b/cpp-generator/experimental/lidl_ast.h index 9fcb28b..136f20f 100644 --- a/cpp-generator/experimental/lidl_ast.h +++ b/cpp-generator/experimental/lidl_ast.h @@ -80,17 +80,12 @@ struct ModuleDecl { QVector types; QVector methods; QVector events; - // True when the impl header declares a public - // std::function emitEvent; - // The generator will wire it to LogosProviderBase::emitEvent in the glue. - bool hasEmitEvent = false; bool operator==(const ModuleDecl& o) const { return name == o.name && version == o.version && description == o.description && category == o.category && depends == o.depends && types == o.types - && methods == o.methods && events == o.events - && hasEmitEvent == o.hasEmitEvent; + && methods == o.methods && events == o.events; } }; diff --git a/cpp-generator/experimental/lidl_gen_provider.cpp b/cpp-generator/experimental/lidl_gen_provider.cpp index 6f091f3..c3bbe68 100644 --- a/cpp-generator/experimental/lidl_gen_provider.cpp +++ b/cpp-generator/experimental/lidl_gen_provider.cpp @@ -270,24 +270,6 @@ QString lidlMakeProviderHeader(const ModuleDecl& module, << module.name << "\", \"" << (module.version.isEmpty() ? "0.0.0" : module.version) << "\")\n\n"; s << "public:\n"; - // Legacy backward-compat: if the impl still declares a - // `std::function<…> emitEvent` member (the old text-pattern path), - // wire it in the provider's constructor so existing modules - // (e.g. logos-package-manager-module) keep working through their - // `emitEvent("name", "json")` call sites. New universal modules - // should declare events in a typed `logos_events:` section — that - // path goes through `maybeSetEmitEvent` in onInit (below) and - // doesn't touch this constructor. - if (module.hasEmitEvent) { - s << " " << providerObjectClass << "() {\n"; - s << " m_impl.emitEvent = [this](const std::string& name, const std::string& data) {\n"; - s << " QVariantList args;\n"; - s << " if (!data.empty()) args << QString::fromStdString(data);\n"; - s << " emitEvent(QString::fromStdString(name), args);\n"; - s << " };\n"; - s << " }\n\n"; - } - for (const MethodDecl& md : module.methods) { QString qtRet = lidlTypeToQt(md.returnType); bool retConvertible = lidlIsStdConvertible(md.returnType); diff --git a/tests/experimental/fixtures/universal_impl.h b/tests/experimental/fixtures/universal_impl.h index 7d46f33..ab96d6a 100644 --- a/tests/experimental/fixtures/universal_impl.h +++ b/tests/experimental/fixtures/universal_impl.h @@ -15,5 +15,8 @@ public: QVariantList anyList(); StdLogosResult fetchResult(); + // A std::function member: the parser must skip it (not treat it as a + // callable method). Used to predate the typed `logos_events:` mechanism + // as an event hook; that special handling is gone. std::function emitEvent; }; diff --git a/tests/experimental/test_impl_header_parser.cpp b/tests/experimental/test_impl_header_parser.cpp index 7f4c48d..e8a6ed2 100644 --- a/tests/experimental/test_impl_header_parser.cpp +++ b/tests/experimental/test_impl_header_parser.cpp @@ -262,14 +262,17 @@ TEST_F(ImplHeaderParserTest, UniversalTypesAndMetadataEvents) EXPECT_EQ(r.module.events[0].params[0].name, "info"); EXPECT_EQ(r.module.events[0].params[0].type.name, "tstr"); - EXPECT_TRUE(r.module.hasEmitEvent); - auto findMethod = [&](const QString& name) -> const MethodDecl* { for (const auto& m : r.module.methods) if (m.name == name) return &m; return nullptr; }; + // The fixture declares a `std::function<…> emitEvent` member. The old + // legacy hook treated it specially; now such members are simply skipped + // and never mistaken for a callable method. + EXPECT_EQ(findMethod("emitEvent"), nullptr); + auto fetchMap = findMethod("fetchMap"); ASSERT_NE(fetchMap, nullptr); EXPECT_EQ(fetchMap->returnType.kind, TypeExpr::Map); diff --git a/tests/experimental/test_lidl_gen_provider.cpp b/tests/experimental/test_lidl_gen_provider.cpp index e0b9e9a..efc23f4 100644 --- a/tests/experimental/test_lidl_gen_provider.cpp +++ b/tests/experimental/test_lidl_gen_provider.cpp @@ -273,26 +273,14 @@ TEST(LidlGenProvider, EventEmitters) EXPECT_TRUE(h.contains("emitEvent(\"onUpdate\"")); // Events declared in ModuleDecl.events flow through the // `_logos_codegen_::maybeSetEmitEvent` SFINAE helper invoked from - // the generated `onInit` — NOT the legacy `m_impl.emitEvent = …` - // constructor wiring (that's gated on `hasEmitEvent` for back-compat - // with un-migrated modules — see ConstructorWiresEmitEventWhenHasEmitEventOnly). + // the generated `onInit`. There is no constructor-based wiring — the + // old `m_impl.emitEvent = …` path was removed along with the legacy + // `std::function emitEvent` member it supported. EXPECT_TRUE(h.contains("_logos_codegen_::maybeSetEmitEvent(m_impl")); EXPECT_TRUE(h.contains("emitEvent(QString::fromStdString(name)")); EXPECT_FALSE(h.contains("m_impl.emitEvent =")); } -TEST(LidlGenProvider, ConstructorWiresEmitEventWhenHasEmitEventOnly) -{ - ModuleDecl m; - m.name = "emitonly"; - m.version = "1.0.0"; - m.hasEmitEvent = true; - - QString h = lidlMakeProviderHeader(m, "EmitOnlyImpl", "emitonly_impl.h"); - EXPECT_TRUE(h.contains("m_impl.emitEvent =")); - EXPECT_TRUE(h.contains("emitEvent(QString::fromStdString(name)")); -} - TEST(LidlGenProvider, HeaderIncludesNlohmannConversionForJsonMapReturn) { ModuleDecl m;