llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.09k stars 11.09k forks source link

clang-format. SeparateDefinitionBlocks inserts spaces after attribute like macro #69591

Open yrHeTaTeJlb opened 9 months ago

yrHeTaTeJlb commented 9 months ago

My previous issue was closed with an incorrect resolution.

The problem is that SeparateDefinitionBlocks: Always treats attribute-like macro incorrectly. USTRUCT(), UCLASS(), UPROPERTY() and UENUM() are not definitions, and clang format should not insert an emty line after them.

$ cat .clang-format
SeparateDefinitionBlocks: Always
BreakBeforeBraces: Allman

######################################

$ cat test.cpp
UENUM()
enum class EEnum
{
    Foo,
    Bar,
};
USTRUCT()
struct FStruct
{
    UPROPERTY()
    int baz = 0;
    UFUNCTION()
    int quux(){}
};
UCLASS()
class FClass
{
    void foo(){}
    void bar(){}
};

######################################

$ clang-format.exe --version
clang-format version 17.0.1 (git://code.qt.io/clang/llvm-project.git 1eee682c1df3ce22d1c53c36f9c44cb4bf3df615)

######################################

$ clang-format.exe test.cpp
UENUM()
enum class EEnum
{
  Foo,
  Bar,
};
USTRUCT()

struct FStruct
{
  UPROPERTY()
  int baz = 0;
  UFUNCTION()

  int quux() {}
};
UCLASS()

class FClass
{
  void foo() {}

  void bar() {}
};

######################################

$ cat expected.cpp
UENUM()
enum class EEnum
{
  Foo,
  Bar,
};

USTRUCT()
struct FStruct
{
  UPROPERTY()
  int baz = 0;

  UFUNCTION()
  int quux() {}
};

UCLASS()
class FClass
{
  void foo() {}

  void bar() {}
};

MaxEmptyLinesToKeep has nothing to do there. I neither want my code to get squashed into a single block, nor to align everything manually.

HazardyKnusperkeks commented 9 months ago

First you need to declare them as AttributeMacros:

clang-format --style="{SeparateDefinitionBlocks: Always, BreakBeforeBraces: Allman, AttributeMacros: [USTRUCT, UENUM, UCLASS, UPROPERTY, UFUNCTION]}" test.cpp 

UENUM() enum class EEnum {
  Foo,
  Bar,
};

USTRUCT() struct FStruct
{
  UPROPERTY() int baz = 0;

  UFUNCTION() int quux() {}
};

UCLASS() class FClass
{
  void foo() {}

  void bar() {}
};

Now you most likely want a line break there? That is a feature request, and not a bug report against a existing feature.

yrHeTaTeJlb commented 9 months ago

Well, if AttributeMacros makes clang-format to treat defined values as attributes, then half of my problem is solved. The second half is to make BreakAfterAttributes: Always also respect this setting. I'm not sure if that would be a bug or feature request.

$ clang-format --style="{BreakAfterAttributes: Always, SeparateDefinitionBlocks: Always, BreakBeforeBraces: Allman, AttributeMacros: [USTRUCT, UENUM, UCLASS, UPROPERTY, UFUNCTION]}" test.cpp
UENUM()
enum class EEnum {
  Foo,
  Bar,
};

USTRUCT() struct FStruct
{
  UPROPERTY() int baz = 0;

  UFUNCTION() int quux() {}
};

UCLASS() class FClass
{
  void foo() {}

  void bar() {}
};

Apart from that, UENUM() is left intact in my case for some reason

HazardyKnusperkeks commented 9 months ago

The problem is, your attribute has parenthesis. If you look at the documentation of BreakAfterAttributes it clearly states C++11 attributes, which yours aren't and never have parenthesis (outside of [[ ]]).

So I'd say it's a feature request. Or you could go with Macros: USTRUCT()=[[some_attribute]], that also should work.

yrHeTaTeJlb commented 9 months ago

I'm not sure if Unreal Engine build system will be happy about such tricks. And in general adding hundreds of hacks is a tedious job.

As I mentioned in my previous issue, this syntax is not my invention, this is a canonical UE code, I believe I'm not alone who would make use of such feature. It'd be cool to have it

HazardyKnusperkeks commented 9 months ago

I'm not sure if Unreal Engine build system will be happy about such tricks. And in general adding hundreds of hacks is a tedious job.

Nothing would land in the build system, just the .clang-format file. Which you have to edit anyway, even if a feature request would be posted and implemented.

As I mentioned in my previous issue, this syntax is not my invention, this is a canonical UE code, I believe I'm not alone who would make use of such feature. It'd be cool to have it

No doubt about it.

yrHeTaTeJlb commented 9 months ago

Nothing would land in the build system, just the .clang-format file

Oh, now I see. Didn't get it from the first time. This actually looks as a decent workaround. Thanks!

Leroy231 commented 5 months ago

Not sure I understand the workaround. I have following in my .clang-format yet I still get an empty line after UCLASS:

BreakAfterAttributes: Always
Macros:
- USTRUCT()=[[some_attribute]]
- UCLASS()=[[some_attribute]]
SeparateDefinitionBlocks: Always

I also tried it with BreakAfterAttributes: Never but got the same thing.

pid011 commented 2 weeks ago

It seems like it's too late to respond, but how about trying it this way?

ColumnLimit: 0
SeparateDefinitionBlocks: Always
AttributeMacros:
  - 'UPROPERTY'
  - 'UFUNCTION'
  - 'UCLASS'
  - 'USTRUCT'
  - 'UENUM'
  - 'UINTERFACE'

I don't know the reason, but set to ColumnLimit: 0 seems to work well regardless of the BreakAfterAttributes value.