CMAKE_CXX_FLAGS not correctly set in CMakeLists.txt

hotwords123 commented 4 months ago

在项目根目录下的 CMakeLists.txt 中，先使用了 add_subdirectory 添加子目录，然后才将 CMAKE_CXX_FLAGS 设置为 CMAKE_COMMON_FLAGS 的值，相关代码如下：

https://github.com/THSS-DB/TDB/blob/a4537c07cd8cb37c599f34b8c74e7e3fb2657eb4/CMakeLists.txt#L95-L109

本地测试时（cmake 版本为 3.29.3），开启 verbose 选项构建发现 CMAKE_CXX_FLAGS 并没有被应用到编译过程中，查看 build/src/server/CMakeFiles/server.dir/flags.make 也发现 CXX_FLAGS 仅为 -g -std=gnu++2a，但 configure 时的输出为

CMAKE_CXX_FLAGS is  -Wall -Werror  -O0 -g -DDEBUG  -fno-omit-frame-pointer -fsanitize=address -fprofile-arcs -ftest-coverage

如果把 SET(CMAKE_CXX_FLAGS ${CMAKE_COMMON_FLAGS}) 移到 add_subdirectory 前面，这些 flags 就能被正常传入编译过程中。因此可以推测，只有在 add_subdirectory 之前设置的 CMAKE_CXX_FLAGS 才能对子文件夹中的 target 正常生效。从 CMakeLists.txt 的编写逻辑上看，目前观察到的构建过程应该不是预期的行为。

这些编译开关的缺失导致了一些问题：

没有 -Wall，编译器不会对一些常见的失误发出警告，增大了静态查错的难度；
没有 -DDEBUG，代码中的 ASSERT 宏即使在默认的 Debug 模式下也不会检查断言条件，不符合常理。

不过直接应用这些编译开关也有一些问题（更严格说来应该是现有代码的问题）：

由于使用了 -Wall -Werror，所有警告被视为错误，这样会导致现有代码无法编译（大部分警告是 -Wsign-compare，例如 for (int i = 0; i < vector.size(); i++) 这种代码）。
打开 -fsanitize=address 后，运行时定位问题点更方便，不过原来隐藏的一些问题也被揪了出来（例如一些内存泄漏，以及内存分配和释放不匹配），可能会对同学们造成一些困扰。
打开 -DDEBUG 后，原先代码中 B+ 树的实现有一些 Frame 没有 unpin，以及 #17 中提到的 bug，在清理时如果严格执行检查（FrameManager::free_internal() 中的 ASSERT）会 crash（取决于 Lab1 的实现中如何处理这类错误）。

感觉 codebase 比较庞大，处理这个问题有点棘手……

hotwords123 commented 4 months ago

相关问题在 #19 中修复了一些，#17 中提到的还没有改，可能需要讨论一下修改方法。

RkGrit commented 4 months ago

你说的是对的，这些问题确实存在，可以提一个pr修复一下，最晚会在明年的lab中更新这部分

hotwords123 commented 4 months ago

大部分找到的 bug 和内存泄漏问题已经在 #19 中提交了，麻烦助教再检查一下，辛苦了！

RkGrit commented 3 months ago

检查了没啥问题👍🏻

THSS-DB / TDB

CMAKE_CXX_FLAGS not correctly set in CMakeLists.txt #18