New Beman Standard CMake Recommendations: Implicit Defaults and (No) Single-Use Variables

The Beman Standard CMake requirements and recommendations are in good shape, I would like to propose adding two more recommendations to guide the existing projects as we prepare them for packaging.

Proposal

[CMAKE.IMPLICIT_DEFAULTS] RECOMMENDATION: Where CMake commands have reasonable default values, and the project does not intend to change those values, the parameters should be left implicitly defaulted rather than enumerated in the command.

Example:

# Not recommended
# Explicitly enumerated defaults for install artifact destinations
install(
    TARGETS beman.project
    EXPORT beman.project-targets
    DESTINATION ${CMAKE_INSTALL_LIBDIR}
    RUNTIME 
        DESTINATION ${CMAKE_INSTALL_BINDIR}
    FILE_SET HEADERS
        DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
)

# Recommended, implicit defaults
install(
    TARGETS beman.project
    EXPORT beman.project-targets
    FILE_SET HEADERS
)

[CMAKE.NO_SINGLE_USE_VARS] RECOMMENDATION: set() should not be used to create variables that are only used once. Prefer using the value(s) directly in such cases.

Example:

# Not recommended, single-use variable for the source list
set(SOURCES
    a.cpp
    b.cpp
    c.cpp
)

# ...

target_sources(beman.project PRIVATE ${SOURCES})

# Recommended, list sources directly

target_sources(beman.project
    PRIVATE
        a.cpp
        b.cpp
        c.cpp
)

Rationale

[CMAKE.IMPLICIT_DEFAULTS]

CMake commands are complicated messes of fractal-like context specific syntax. When it comes to the more complicated commands like target_sources() and install(), less is definitely more.

When non-experts (which is most C++ programmers) have to edit a CML, they reasonably assume that everything in there was written for a reason. Enumerated defaults leave even build system veterans scratching their heads wondering what problem a given parameter was trying to solve. This leads to a Chesterton’s Fence scenario where developers are afraid to touch code they don’t understand.

Leaving defaults implicit increases CMake code clarity, readability, and allows developers to be more confident in making changes.

[CMAKE.NO_SINGLE_USE_VARS]

This is a simple locality problem. Code isn’t any shorter when creating these single-use variables, all it does is frustratingly decouple description from use.

This pattern is a holdover from ancient CMake days when such variables would actually be used a couple of times for reasons that are esoteric and boring to explain. It’s been 15 years since such patterns were relevant, and these style of variables are now thoroughly discouraged by the CMake orthodoxy.

Bonus: I really don’t like it when devs create a variable to hold a single short constant string that’s about the same length as the variable name. Just use the string. I don’t dislike it enough to codify it though.

2 Likes

These are difficult to disagree with. Just wondering what you think of this construct as opposed to just spelling it out:

Regardless, I’d say basically lift this post into a PR for the beman standard.

This is why they’re recommendations, not requirements. Obviously good taste is in order. Crowding out the logic of a for-loop is more confusing than the alternative of a single-use or near-single-use variable. Stuff like this is fine.

Another style is to do something like this:

function(add_example TARGET)
    add_executable(${TARGET})
    target_sources(${TARGET} PRIVATE ${TARGET}.cpp)
    target_link_libraries(${TARGET} PRIVATE beman::scope)
endfunction()

add_example(Larry)
add_example(Curly)
add_example(Moe)

Which is in better taste when the repetitive work has more knobs and buttons than this exact example, which only has a single string. When you’re dealing with calling a couple commands differentiated only by a single string, there’s basically no wrong answer. Every solution is good. (i am not issuing challenge to come up with the worst possible solution).

2 Likes

I’m also supportive of these additions. Very nice.

I do want to point out ASAIK there’s no way to lint for these rules now.

But yeah these sounds like good ideas.

do want to point out ASAIK there’s no way to lint for these rules now.

It looks like there is a basic cmake linter out there. I didn’t look deeply into its quality. Ideally, such a tool would use a CMake-provided library to parse.

There is no CMake linter. Every existing program amounts to a whitespace shuffler or a character case cop, sometimes both. Lot of reasons for that, short version is it’s never been at the top of anyone’s priority list.

The upside is that CMakeLang is very unpleasant, which encourages devs to write as little of it as possible and rarely change, minimizing the need of advanced tooling.

Oof, this sounds like such a cynical take.