Author: mydeveloperday Date: 2021-11-29T14:27:16Z New Revision: 814aabae37757d21d96e22608ccb98e91c1f3a06
URL: https://github.com/llvm/llvm-project/commit/814aabae37757d21d96e22608ccb98e91c1f3a06 DIFF: https://github.com/llvm/llvm-project/commit/814aabae37757d21d96e22608ccb98e91c1f3a06.diff LOG: [clang-format] regressed default behavior for operator parentheses {D110833} regressed behavior of spaces before parentheses for operators, this revision reverts that so that operators are handled as they were before. I think in hindsight it was a mistake to try and consume operator behaviour in with the function behaviour, I think Operators can be considered a special style. Its seems the code is getting confused as to if this is a function declaration or definition. I think latterly we can consider adding an operator parentheses specific custom option but this should have been explicitly called out as it can impact projects. Reviewed By: HazardyKnusperkeks, curdeius Differential Revision: https://reviews.llvm.org/D114696 Added: Modified: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index fba55fb32da2d..22faf54b017da 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -3169,9 +3169,13 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, if (Left.isIf(Line.Type != LT_PreprocessorDirective)) return Style.SpaceBeforeParensOptions.AfterControlStatements || spaceRequiredBeforeParens(Right); + + // TODO add Operator overloading specific Options to + // SpaceBeforeParensOptions + if (Right.is(TT_OverloadedOperatorLParen)) + return spaceRequiredBeforeParens(Right); // Function declaration or definition - if (Line.MightBeFunctionDecl && (Left.is(TT_FunctionDeclarationName) || - Right.is(TT_OverloadedOperatorLParen))) { + if (Line.MightBeFunctionDecl && (Left.is(TT_FunctionDeclarationName))) { if (Line.mightBeFunctionDefinition()) return Style.SpaceBeforeParensOptions.AfterFunctionDefinitionName || spaceRequiredBeforeParens(Right); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 845b1e5e8809a..39f1b835fb766 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -9185,6 +9185,11 @@ TEST_F(FormatTest, UnderstandsOverloadedOperators) { // https://llvm.org/PR50629 // verifyFormat("void f() { operator*(a & a); }"); // verifyFormat("void f() { operator&(a, b * b); }"); + + verifyFormat("::operator delete(foo);"); + verifyFormat("::operator new(n * sizeof(foo));"); + verifyFormat("foo() { ::operator delete(foo); }"); + verifyFormat("foo() { ::operator new(n * sizeof(foo)); }"); } TEST_F(FormatTest, UnderstandsFunctionRefQualification) { @@ -14117,8 +14122,9 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) { verifyFormat("static_assert (sizeof (char) == 1, \"Impossible!\");", Space); verifyFormat("int f () throw (Deprecated);", Space); verifyFormat("typedef void (*cb) (int);", Space); - verifyFormat("T A::operator() ();", Space); - verifyFormat("X A::operator++ (T);", Space); + // FIXME these tests regressed behaviour. + // verifyFormat("T A::operator() ();", Space); + // verifyFormat("X A::operator++ (T);", Space); verifyFormat("auto lambda = [] () { return 0; };", Space); verifyFormat("int x = int (y);", Space); @@ -14174,7 +14180,8 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) { verifyFormat("int f() throw (Deprecated);", SomeSpace); verifyFormat("typedef void (*cb) (int);", SomeSpace); verifyFormat("T A::operator()();", SomeSpace); - verifyFormat("X A::operator++ (T);", SomeSpace); + // FIXME these tests regressed behaviour. + // verifyFormat("X A::operator++ (T);", SomeSpace); verifyFormat("int x = int (y);", SomeSpace); verifyFormat("auto lambda = []() { return 0; };", SomeSpace); @@ -14230,8 +14237,9 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) { SpaceFuncDecl); verifyFormat("int f () throw(Deprecated);", SpaceFuncDecl); verifyFormat("typedef void (*cb)(int);", SpaceFuncDecl); - verifyFormat("T A::operator() ();", SpaceFuncDecl); - verifyFormat("X A::operator++ (T);", SpaceFuncDecl); + // FIXME these tests regressed behaviour. + // verifyFormat("T A::operator() ();", SpaceFuncDecl); + // verifyFormat("X A::operator++ (T);", SpaceFuncDecl); verifyFormat("T A::operator()() {}", SpaceFuncDecl); verifyFormat("auto lambda = []() { return 0; };", SpaceFuncDecl); verifyFormat("int x = int(y);", SpaceFuncDecl); @@ -14266,7 +14274,7 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) { verifyFormat("typedef void (*cb)(int);", SpaceFuncDef); verifyFormat("T A::operator()();", SpaceFuncDef); verifyFormat("X A::operator++(T);", SpaceFuncDef); - verifyFormat("T A::operator() () {}", SpaceFuncDef); + // verifyFormat("T A::operator() () {}", SpaceFuncDef); verifyFormat("auto lambda = [] () { return 0; };", SpaceFuncDef); verifyFormat("int x = int(y);", SpaceFuncDef); verifyFormat("M(std::size_t R, std::size_t C) : C(C), data(R) {}", @@ -14341,7 +14349,7 @@ TEST_F(FormatTest, ConfigurableSpaceBeforeParens) { verifyFormat("int f() throw (Deprecated);", SomeSpace2); verifyFormat("typedef void (*cb) (int);", SomeSpace2); verifyFormat("T A::operator()();", SomeSpace2); - verifyFormat("X A::operator++ (T);", SomeSpace2); + // verifyFormat("X A::operator++ (T);", SomeSpace2); verifyFormat("int x = int (y);", SomeSpace2); verifyFormat("auto lambda = []() { return 0; };", SomeSpace2); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
