Skip to content

refactor: replace private access hacks with accessor pattern#564

Merged
zccrs merged 1 commit into
linuxdeepin:masterfrom
zccrs:master
May 25, 2026
Merged

refactor: replace private access hacks with accessor pattern#564
zccrs merged 1 commit into
linuxdeepin:masterfrom
zccrs:master

Conversation

@zccrs
Copy link
Copy Markdown
Member

@zccrs zccrs commented May 20, 2026

Remove all '#define private/protected public' hacks and replace them with a proper C++ template-based private accessor pattern using explicit template instantiation (friend injection trick), mirroring the approach used in linuxdeepin/treeland#875.

Add src/util/dprivateaccessor_p.h with Accessor/AccessorImpl templates and D_DECLARE_PRIVATE_MEMBER, D_DECLARE_PRIVATE_METHOD, D_DECLARE_PRIVATE_CONST_METHOD, D_PRIVATE_MEMBER, D_PRIVATE_CALL macros. All helpers are in global namespace so ADL correctly finds the friend-injected get() function.

@zccrs zccrs requested a review from 18202781743 May 20, 2026 08:45
@zccrs zccrs force-pushed the master branch 4 times, most recently from 0d1a967 to 7144e90 Compare May 21, 2026 08:23
18202781743
18202781743 previously approved these changes May 21, 2026
@deepin-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zccrs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

1 similar comment
@deepin-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zccrs

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Remove all '#define private/protected public' hacks and replace them
with a proper C++ template-based private accessor pattern using
explicit template instantiation (friend injection trick), mirroring
the approach used in linuxdeepin/treeland#875.

Add src/util/dprivateaccessor_p.h with Accessor/AccessorImpl
templates and D_DECLARE_PRIVATE_MEMBER, D_DECLARE_PRIVATE_METHOD,
D_DECLARE_PRIVATE_CONST_METHOD, D_PRIVATE_MEMBER, D_PRIVATE_CALL
macros. All helpers are in global namespace so ADL correctly finds
the friend-injected get() function.
@deepin-ci-robot
Copy link
Copy Markdown
Contributor

deepin pr auto review

你好!我是CodeGeeX。我已仔细审查了你提供的Git Diff,该变更主要包含三个部分:新增Arch Linux和Deepin的CI构建工作流、引入基于显式模板实例化的私有成员访问机制替代危险的宏、以及相关的CMake和版权声明调整。

以下是我对语法逻辑、代码质量、代码性能和代码安全方面的详细审查意见和改进建议:

一、 语法与逻辑

  1. D_DECLARE_PRIVATE_METHOD 宏的变参缺陷(严重)
    dprivateaccessor_p.h 中,D_DECLARE_PRIVATE_METHODD_DECLARE_PRIVATE_CONST_METHOD 使用了 __VA_ARGS__ 来接收方法的参数。但是,C++中指向成员函数的指针声明要求参数列表必须完全匹配。

    • 问题:如果被访问的私有方法没有参数,__VA_ARGS__ 展开为空,会导致语法错误,如 RetType (ClassName::*)() const 是合法的,但宏展开可能产生多余的逗号或格式错误。
    • 建议:由于C++不支持宏的可选参数,建议针对无参和有参方法提供不同的宏(如 D_DECLARE_PRIVATE_METHOD_0D_DECLARE_PRIVATE_METHOD_N),或者使用更现代的C++20的 __VA_OPT__ 来处理多余逗号。
  2. D_PRIVATE_CALL 宏的传参逻辑

    • 问题:在 ddcifileengine.cpp 中的调用 D_PRIVATE_CALL(*D_PRIVATE_CALL(realDciFile, QFile_d_func_tag{}), QFilePrivate_engine_tag{}),外层解引用了内层调用的结果。这要求 QFile::d_func() 返回的必须是对象或引用,而实际上 Qt 的 d_func() 通常返回的是指针(QFilePrivate *d_ptr)。
    • 建议:如果 d_func() 返回指针,内层调用应该使用 ->* 而不是 .*。你可能需要为指针类型重载一个 D_PRIVATE_PTR_CALL 宏,或者确保解引用逻辑正确:D_PRIVATE_PTR_CALL(D_PRIVATE_CALL(realDciFile, QFile_d_func_tag{}), QFilePrivate_engine_tag{})
  3. Arch Linux CI 的依赖顺序(逻辑优化)

    • 问题:在 dtkcore-archlinux-build.yml 中,actions/checkout@v4 位于“Install dependencies”步骤之后。如果代码仓库内部包含依赖源码或需要根据仓库内的配置来决定安装什么依赖,这种顺序会导致问题。
    • 建议:将 actions/checkout@v4 移到系统更新和依赖安装之前,这是CI的标准最佳实践。

二、 代码质量

  1. 消除危险的 #define private public(极佳改进)

    • 亮点:使用显式模板实例化技术访问私有成员,不仅消除了未定义行为(UB),还避免了宏污染全局符号空间。这是非常高质量的改进,符合C++标准 [temp.explicit]/12 的规定。
    • 建议dprivateaccessor_p.h 中的注释非常清晰,但建议在 ddcifileengine.cpp 使用 D_PRIVATE_CALL 的地方加上简短的注释,说明为什么这里需要访问私有函数(如:由于Qt未暴露同步到磁盘的引擎接口),以便后续维护者理解。
  2. Deepin CI 的 APT 源配置安全性

    • 问题:在 dtkcore-deepin-build.yml 中,使用了 echo "deb [trusted=yes] ..."trusted=yes 会跳过 GPG 签名验证,这在生产环境中是不安全的,容易遭受中间人攻击。
    • 建议:虽然CI环境相对隔离,但更规范的做法是添加Deepin的GPG Key后再使用源,或者至少在注释中说明此处为了简化CI流程而牺牲了签名校验。
  3. Arch Linux CI 的打包工具选择

    • 问题:在 Arch Linux CI 中,使用了 zip 来打包安装产物。Arch Linux 原生使用 pacmanmakepkg(基于 PKGEXT=.pkg.tar.zst)。
    • 建议:如果该产物仅用于存档或下载测试,zip 无可厚非;但如果要用于实际的 Arch 环境部署,建议生成标准的 .pkg.tar.zst 包。

三、 代码性能

  1. CI 并行构建优化

    • 问题:Arch CI 中使用了 -j$(nproc),这是好的。但在 Deepin CI 中,dpkg-buildpackage 默认可能不会利用所有核心。
    • 建议:在 Deepin CI 的构建步骤中,添加 DEB_BUILD_OPTIONS="parallel=$(nproc)" 环境变量,以启用 dpkg 的并行编译,显著缩短构建时间。
  2. find 命令的性能开销

    • 问题:Arch CI 中 find /tmp/dtkcore-install -type f | wc -l 用于统计文件数。对于大量小文件,遍历整个目录树有一定I/O开销。
    • 建议:此开销在CI中通常可接受,但若追求极致,可考虑在 cmake --install 时通过解析输出日志来统计,或直接保留当前写法(因为其直观性更好)。

四、 代码安全

  1. SPDX 版权年份的修改

    • 问题ddcifileengine.cpp 的版权声明从 2021 - 2022 修改为了 2021 - 2026。包含未来的年份在开源协议中是不常见且不推荐的。
    • 建议:如果是今年修改,应改为 2021 - 2023(或当前实际年份)。2026 可能是笔误,建议修正。
  2. CI 中的 if: ${{ !env.ACT }} 逻辑

    • 问题:Arch CI 的上传步骤有 if: ${{ !env.ACT }},这是为了兼容 nektos/act(本地运行CI的工具),但在生产环境的 GitHub Actions 中,如果无意间设置了 ACT 环境变量,会导致构建产物丢失。
    • 建议:保留此逻辑无大碍,但需确保仓库的 Settings 中没有误配 ACT 环境变量。

综合修改建议代码示例

针对 dprivateaccessor_p.hddcifileengine.cpp 的关键问题,建议修改如下:

1. 完善 dprivateaccessor_p.h (增加针对指针的访问宏):

// ... 前面保持不变 ...

// 针对对象实例的访问 (.*)
#define D_PRIVATE_MEMBER(obj, tag) ((obj).*dtk_private_detail::access(tag))
#define D_PRIVATE_CALL(obj, tag, ...) ((obj).*dtk_private_detail::access(tag))(__VA_ARGS__)

// 针对指针实例的访问 (->*) - 新增
#define D_PRIVATE_PTR_MEMBER(obj_ptr, tag) ((obj_ptr)->*dtk_private_detail::access(tag))
#define D_PRIVATE_PTR_CALL(obj_ptr, tag, ...) ((obj_ptr)->*dtk_private_detail::access(tag))(__VA_ARGS__)

2. 修正 ddcifileengine.cpp 中的调用逻辑:

// 假设 d_func() 返回的是 QFilePrivate* 指针
bool DDciFileEngine::syncToDisk()
{
    if (!flush())
        return false;
        
    // 使用 D_PRIVATE_PTR_CALL 来通过指针调用私有方法
    return D_PRIVATE_PTR_CALL(
        D_PRIVATE_CALL(realDciFile, QFile_d_func_tag{}), 
        QFilePrivate_engine_tag{}
    )->syncToDisk();
}

3. 修正版权声明:

// SPDX-FileCopyrightText: 2021 - 2023 UnionTech Software Technology Co., Ltd.

整体来看,这是一次非常棒的重构,特别是使用模板技术替换宏的黑魔法,极大提升了代码的健壮性。只需在CI流程和宏的边界条件上稍加打磨即可。

@zccrs zccrs merged commit fe3f8ff into linuxdeepin:master May 25, 2026
21 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants