Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter compilation settings in local dev #8625

Open
roberth opened this issue Jul 1, 2023 · 5 comments
Open

Stricter compilation settings in local dev #8625

roberth opened this issue Jul 1, 2023 · 5 comments
Labels
contributor-experience Developer experience for Nix contributors feature Feature request or proposal portability Supporting more platforms

Comments

@roberth
Copy link
Member

roberth commented Jul 1, 2023

Is your feature request related to a problem? Please describe.

We have issues that can be caught be compilers.

We want to catch these mistakes in a local build during development, and we also want to catch them in CI

We do not want to fail the package build when the issue is insignificant, such as a warning. Otherwise build success will be too sensitive to compiler updates and such that users may want to apply through input overriding, and we'd make life unnecessarily hard on distributions such as Nixpkgs.

Describe the solution you'd like

A pedantic attribute in the derivation, so that the derived dev shell can enable that. When the pedantic flag is set, the configure script picks that up and sets a bunch of flags that make the compiler check for various things and fail if those checks are violated.

Describe alternatives you've considered

Additional context

Priorities

Add 👍 to issues you find important.

@roberth roberth added feature Feature request or proposal contributor-experience Developer experience for Nix contributors labels Jul 1, 2023
@roberth roberth added the portability Supporting more platforms label Jul 1, 2023
@inclyc
Copy link
Member

inclyc commented Jul 5, 2023

The clangd language server reads .clangd file for customization flags, including -Wxxx && .clang-tidy provides more static analysis. Might be a simple solution that does not need to change current building system (it actually works fine). Shall we make these checks mandatory?

Related: #6721

@roberth
Copy link
Member Author

roberth commented Jul 5, 2023

That could be a good first step. Not everyone will use and see that, so it leaves a bit of a gap, but even after adding CI for this, it will be useful to have the info asap.

I've tried to give clangd a spin just now, but it doesn't seem to have been packaged, and also the binary downloaded by the vscode plugin doesn't work. How would I use it?

@inclyc
Copy link
Member

inclyc commented Jul 6, 2023

The flake provided by this repository offers a devShell that includes clangd. My .envrc:

use flake .#native-clangStdenvPackages

It was introduced in #7433, and works perfect for me.

but it doesn't seem to have been packaged

The pname is clang-tools in nixpkgs.

@roberth
Copy link
Member Author

roberth commented Jul 6, 2023

Thanks! I'd been using https://github.com/microsoft/vscode-cpptools until now, and I was unaware of #7433 somehow. clangd wasn't listed on search.nixos.org when I checked yesterday, but it is today 🤔
I've switched just now and I'm curious which setup takes more effort to maintain.

@inclyc
Copy link
Member

inclyc commented Jul 6, 2023

I've switched just now and I'm curious which setup takes more effort to maintain.

Clangd gives us more accurate warning & language hints. But requires the compilation database compile_commands.json and some customization flags. ms-cpptools might work out of box for normal projects. I would like prefer clangd because it gives me more information on this code base.

Screenshot_20230706_170222

clangd also provides uniform range-based auto-formatting, I'm submitting PRs formatted by .clang-format cherry-picked from #6721:

BasedOnStyle: LLVM
IndentWidth: 4
BreakBeforeBraces: Custom
BraceWrapping:
  AfterStruct: true
  AfterClass: true
  AfterFunction: true
  AfterUnion: true
  SplitEmptyRecord: false
PointerAlignment: Middle
FixNamespaceComments: false
SortIncludes: Never
#IndentPPDirectives: BeforeHash
SpaceAfterCStyleCast: true
SpaceAfterTemplateKeyword: false
AccessModifierOffset: -4
AlignAfterOpenBracket: AlwaysBreak
AlignEscapedNewlines: DontAlign
ColumnLimit: 120
BreakStringLiterals: false
BitFieldColonSpacing: None
AllowShortFunctionsOnASingleLine: Empty
AlwaysBreakTemplateDeclarations: Yes
BinPackParameters: false
BreakConstructorInitializers: BeforeComma
EmptyLineAfterAccessModifier: Leave # change to always/never later?
EmptyLineBeforeAccessModifier: Leave
#PackConstructorInitializers: BinPack
BreakBeforeBinaryOperators: NonAssignment
AlwaysBreakBeforeMultilineStrings: true

The hacking guide described how to setup clangd correctly for nix: https://nixos.org/manual/nix/unstable/contributing/hacking.html#editor-integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor-experience Developer experience for Nix contributors feature Feature request or proposal portability Supporting more platforms
Projects
None yet
Development

No branches or pull requests

2 participants