[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

Reverted in 4098e13a7146 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Timo Stripf via cfe-commits]

Sry for breaking builds. Can someone please revert it, I am currently not at 
work.

Am 28.07.2023 20:27 schrieb Nico Weber via Phabricator 
:
thakis added a comment.

This breaks tests on Mac: http://45.33.8.238/macm1/65934/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests on Mac: http://45.33.8.238/macm1/65934/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

  error: aliases are not supported on darwin

Maybe create a different testcase that is not supported on Darwin?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Steven Wu via Phabricator via cfe-commits]

steven_wu added a comment.

This breaks Darwin bots: 
https://green.lab.llvm.org/green/job/clang-stage1-RA/35102/testReport/junit/Clang/AST/ast_print_method_decl_cpp/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Timo Stripf via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa3da6284c23a: [clang][DeclPrinter] Fix missing semicolon in 
AST print for methods that are… (authored by strimo378).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/test/AST/ast-print-method-decl.cpp


Index: clang/test/AST/ast-print-method-decl.cpp
===
--- clang/test/AST/ast-print-method-decl.cpp
+++ clang/test/AST/ast-print-method-decl.cpp
@@ -85,3 +85,18 @@
 
   // CHECK-NEXT: };
 };
+
+
+// CHECK: struct DefMethodsWithoutBody {
+struct DefMethodsWithoutBody {
+  // CHECK-NEXT: DefMethodsWithoutBody() = delete;
+  DefMethodsWithoutBody() = delete;
+
+  // CHECK-NEXT: DefMethodsWithoutBody() = default;
+  ~DefMethodsWithoutBody() = default;
+
+  // CHECK-NEXT: void m1() __attribute__((alias("X")));
+  void m1() __attribute__((alias("X")));
+
+  // CHECK-NEXT: };
+};
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -463,12 +463,12 @@
 else if (isa(*D) && cast(*D)->hasBody())
   Terminator = nullptr;
 else if (auto FD = dyn_cast(*D)) {
-  if (FD->isThisDeclarationADefinition())
+  if (FD->doesThisDeclarationHaveABody() && !FD->isDefaulted())
 Terminator = nullptr;
   else
 Terminator = ";";
 } else if (auto TD = dyn_cast(*D)) {
-  if (TD->getTemplatedDecl()->isThisDeclarationADefinition())
+  if (TD->getTemplatedDecl()->doesThisDeclarationHaveABody())
 Terminator = nullptr;
   else
 Terminator = ";";


Index: clang/test/AST/ast-print-method-decl.cpp
===
--- clang/test/AST/ast-print-method-decl.cpp
+++ clang/test/AST/ast-print-method-decl.cpp
@@ -85,3 +85,18 @@
 
   // CHECK-NEXT: };
 };
+
+
+// CHECK: struct DefMethodsWithoutBody {
+struct DefMethodsWithoutBody {
+  // CHECK-NEXT: DefMethodsWithoutBody() = delete;
+  DefMethodsWithoutBody() = delete;
+
+  // CHECK-NEXT: DefMethodsWithoutBody() = default;
+  ~DefMethodsWithoutBody() = default;
+
+  // CHECK-NEXT: void m1() __attribute__((alias("X")));
+  void m1() __attribute__((alias("X")));
+
+  // CHECK-NEXT: };
+};
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -463,12 +463,12 @@
 else if (isa(*D) && cast(*D)->hasBody())
   Terminator = nullptr;
 else if (auto FD = dyn_cast(*D)) {
-  if (FD->isThisDeclarationADefinition())
+  if (FD->doesThisDeclarationHaveABody() && !FD->isDefaulted())
 Terminator = nullptr;
   else
 Terminator = ";";
 } else if (auto TD = dyn_cast(*D)) {
-  if (TD->getTemplatedDecl()->isThisDeclarationADefinition())
+  if (TD->getTemplatedDecl()->doesThisDeclarationHaveABody())
 Terminator = nullptr;
   else
 Terminator = ";";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM! When you land, please be sure to add `Fixes 
https://github.com/llvm/llvm-project/issues/62996` to the commit message so the 
issue is automatically closed (and it makes it easier for people doing code 
archeology later). Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Timo Stripf via Phabricator via cfe-commits]

strimo378 added a comment.

https://github.com/llvm/llvm-project/issues/62996


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156533/new/

https://reviews.llvm.org/D156533

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156533: [clang][DeclPrinter] Fix missing semicolon in AST print for methods that are definitions without having a body

[Timo Stripf via Phabricator via cfe-commits]

strimo378 created this revision.
strimo378 added a reviewer: aaron.ballman.
Herald added a project: All.
strimo378 requested review of this revision.
Herald added subscribers: cfe-commits, wangpc.
Herald added a project: clang.

DeclPrinter used FunctionDecl::isThisDeclarationADefinition to decide if the 
decl requires a semicolon at the end. However, there are several methods 
without body (that require a semicolon) that are definitions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D156533

Files:
  clang/lib/AST/DeclPrinter.cpp
  clang/test/AST/ast-print-method-decl.cpp


Index: clang/test/AST/ast-print-method-decl.cpp
===
--- clang/test/AST/ast-print-method-decl.cpp
+++ clang/test/AST/ast-print-method-decl.cpp
@@ -85,3 +85,18 @@
 
   // CHECK-NEXT: };
 };
+
+
+// CHECK: struct DefMethodsWithoutBody {
+struct DefMethodsWithoutBody {
+  // CHECK-NEXT: DefMethodsWithoutBody() = delete;
+  DefMethodsWithoutBody() = delete;
+
+  // CHECK-NEXT: DefMethodsWithoutBody() = default;
+  ~DefMethodsWithoutBody() = default;
+
+  // CHECK-NEXT: void m1() __attribute__((alias("X")));
+  void m1() __attribute__((alias("X")));
+
+  // CHECK-NEXT: };
+};
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -463,12 +463,12 @@
 else if (isa(*D) && cast(*D)->hasBody())
   Terminator = nullptr;
 else if (auto FD = dyn_cast(*D)) {
-  if (FD->isThisDeclarationADefinition())
+  if (FD->doesThisDeclarationHaveABody() && !FD->isDefaulted())
 Terminator = nullptr;
   else
 Terminator = ";";
 } else if (auto TD = dyn_cast(*D)) {
-  if (TD->getTemplatedDecl()->isThisDeclarationADefinition())
+  if (TD->getTemplatedDecl()->doesThisDeclarationHaveABody())
 Terminator = nullptr;
   else
 Terminator = ";";


Index: clang/test/AST/ast-print-method-decl.cpp
===
--- clang/test/AST/ast-print-method-decl.cpp
+++ clang/test/AST/ast-print-method-decl.cpp
@@ -85,3 +85,18 @@
 
   // CHECK-NEXT: };
 };
+
+
+// CHECK: struct DefMethodsWithoutBody {
+struct DefMethodsWithoutBody {
+  // CHECK-NEXT: DefMethodsWithoutBody() = delete;
+  DefMethodsWithoutBody() = delete;
+
+  // CHECK-NEXT: DefMethodsWithoutBody() = default;
+  ~DefMethodsWithoutBody() = default;
+
+  // CHECK-NEXT: void m1() __attribute__((alias("X")));
+  void m1() __attribute__((alias("X")));
+
+  // CHECK-NEXT: };
+};
Index: clang/lib/AST/DeclPrinter.cpp
===
--- clang/lib/AST/DeclPrinter.cpp
+++ clang/lib/AST/DeclPrinter.cpp
@@ -463,12 +463,12 @@
 else if (isa(*D) && cast(*D)->hasBody())
   Terminator = nullptr;
 else if (auto FD = dyn_cast(*D)) {
-  if (FD->isThisDeclarationADefinition())
+  if (FD->doesThisDeclarationHaveABody() && !FD->isDefaulted())
 Terminator = nullptr;
   else
 Terminator = ";";
 } else if (auto TD = dyn_cast(*D)) {
-  if (TD->getTemplatedDecl()->isThisDeclarationADefinition())
+  if (TD->getTemplatedDecl()->doesThisDeclarationHaveABody())
 Terminator = nullptr;
   else
 Terminator = ";";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits