This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Allow configuring list of macros that map to attributes
ClosedPublic

Authored by arichardson on Aug 28 2020, 6:07 AM.

Details

Summary

This adds a AttributeMacros configuration option that causes certain
identifiers to be parsed like a attribute((foo)) annotation.
This is motivated by our CHERI C/C++ fork which adds a capability
qualifier for pointer/reference. Without this change clang-format parses
many type declarations as multiplications/bitwise-and instead.
I initially considered adding "
capability" as a new clang-format keyword,
but having a list of macros that should be treated as attributes is more
flexible since it can be used e.g. for static analyzer annotations or other language
extensions.

Example: std::vector<foo * capability> -> std::vector<foo *capability>

Depends on D86775 (to apply cleanly)

Diff Detail

Event Timeline

arichardson created this revision.Aug 28 2020, 6:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 28 2020, 6:07 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
arichardson requested review of this revision.Aug 28 2020, 6:07 AM

fix formatting and rebase

I'm wondering if this could be used to help the CUDA usages of clang-format

I'm wondering if this could be used to help the CUDA usages of clang-format

Do you have any examples that are not formatted sensibly? I was also considering adding another list for attribute macros with arguments (e.g. foo *addrspace(1))

jrtc27 requested changes to this revision.Sep 1 2020, 7:13 AM

The documentation currently shows __capability being included, but from looking at this patch does the configuration file not append (which I think makes sense, at least for __capability) rather than replace?

clang/docs/ClangFormatStyleOptions.rst
776
clang/include/clang/Format/Format.h
599
This revision now requires changes to proceed.Sep 1 2020, 7:13 AM

The documentation currently shows __capability being included, but from looking at this patch does the configuration file not append (which I think makes sense, at least for __capability) rather than replace?

I just wrote a test to check what clang-format does and it appears it's neither: D86941

  • fix name of key in config file
  • Add test for config parsing

fix configuration parsing test

arichardson marked 2 inline comments as done.Sep 4 2020, 9:17 AM
MyDeveloperDay accepted this revision.Sep 4 2020, 10:22 AM

LGTM, I think we need something like this for those keywords/macros

jrtc27 accepted this revision.Sep 4 2020, 4:36 PM
This revision is now accepted and ready to land.Sep 4 2020, 4:36 PM