[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: aaron.ballman.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

attributes after an if like:

if (...) [[likely]]

are now applied on the if instead of the following statement.

i added the likely attribute in the necessary places Attr.td, AttrDocs.td.

i added a diagnostics kind for C++2a and for likely not used on an if statement.


Repository:
  rC Clang

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp

Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,22 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && A.isCXX11Attribute() &&
+  !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +352,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,12 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3858,6 +3858,7 @@
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
+  case ParsedAttr::AT_Likely:
 return true;
   case ParsedAttr::AT_WarnUnusedResult:
 return !ScopeName && AttrName->getName().equals("nodiscard");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
   "use of the %0 attribute is a C++14 extension">, InGroup;
 def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup;
+def ext_cxx2a_attr : Extension<
+  "use of the %0 attribute is a C++2a extension">, InGroup;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,9 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't apear here">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -1492,6 +1492,26 @@
   }];
 }
 
+def LikelyDocs : Documentation {
+  let Category = DocCatStmt;
+  let Heading = "likely";
+  let Content = [{
+The ``likely`` (or ``clang::likely``) attribute is used to annotate that a condition is likely to be true
+this is supposed to be used as a hint by the optimizer (not yet implemented)
+
+Here is an example:
+
+.. c

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191031.
Tyker added a comment.

i added tests as you requested. i didn't add test for the codegen as it wasn't 
affected.


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp

Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -23,6 +23,21 @@
 // CHECK-NEXT:   FallThroughAttr
 // CHECK-NEXT:   NullStmt
 
+int TestLikelyAttribute(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[likely]]
+return 1;
+  return 2;
+}
+// CHECK:  FunctionDecl{{.*}}TestLikelyAttribute
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+
 [[clang::warn_unused_result]] int TestCXX11DeclAttr();
 // CHECK:  FunctionDecl{{.*}}TestCXX11DeclAttr
 // CHECK-NEXT:   WarnUnusedResultAttr
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,21 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +351,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,13 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3858,6 +3858,7 @@
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
+  case ParsedAttr::AT_Likely:
 return true;
   case ParsedAttr::AT_WarnUnusedResult:
 return !ScopeName && AttrName->getName().equals("nodiscard");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
   "use of the %0 attribute is a C++14 extension">, InGroup;
 def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup;
+def ext_cxx2a_attr : Extension<
+  "use of the %0 attribute is a C++2a extension">, InGroup;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,9 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't appear here">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup, Default

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191034.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp

Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -23,6 +23,21 @@
 // CHECK-NEXT:   FallThroughAttr
 // CHECK-NEXT:   NullStmt
 
+int TestLikelyAttribute(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[likely]]
+return 1;
+  return 2;
+}
+// CHECK:  FunctionDecl{{.*}}TestLikelyAttribute
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+
 [[clang::warn_unused_result]] int TestCXX11DeclAttr();
 // CHECK:  FunctionDecl{{.*}}TestCXX11DeclAttr
 // CHECK-NEXT:   WarnUnusedResultAttr
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,21 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +351,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,13 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -3858,6 +3858,7 @@
   case ParsedAttr::AT_Deprecated:
   case ParsedAttr::AT_FallThrough:
   case ParsedAttr::AT_CXX11NoReturn:
+  case ParsedAttr::AT_Likely:
 return true;
   case ParsedAttr::AT_WarnUnusedResult:
 return !ScopeName && AttrName->getName().equals("nodiscard");
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -7339,6 +7339,8 @@
   "use of the %0 attribute is a C++14 extension">, InGroup;
 def ext_cxx17_attr : Extension<
   "use of the %0 attribute is a C++17 extension">, InGroup;
+def ext_cxx2a_attr : Extension<
+  "use of the %0 attribute is a C++2a extension">, InGroup;
 
 def warn_unused_comparison : Warning<
   "%select{equality|inequality|relational|three-way}0 comparison result unused">,
@@ -8158,6 +8160,9 @@
   "fallthrough annotation in unreachable code">,
   InGroup, DefaultIgnore;
 
+def err_likely_attr_invalid_placement : Error<
+  "likely annotation can't appear here">;
+
 def warn_unreachable_default : Warning<
   "default label in switch which covers all enumeration values">,
   InGroup, DefaultIgnore;
Index: clang/include/clang/Basic/AttrDocs.td
==

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191035.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/SemaCXX/cxx2a-likely-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likely-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likely-attr.cpp
@@ -0,0 +1,37 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1)
+  {
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+void g(void) {
+  [[likely]] int n; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[likely]] ++n; // expected-error {{likely annotation can't appear here}}
+  
+  switch (n) {
+[[likely]] return; // expected-error {{likely annotation can't appear here}}
+
+  case 0:
+if (1) [[likely, likely]] // expected-error {{attribute 'likely' cannot appear multiple times in an attribute specifier}}
+  return ;
+if (1) [[likely(0)]] // expected-error {{attribute 'likely' cannot have an argument list}}
+  return ;
+break;
+  }
+}
+
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -23,6 +23,21 @@
 // CHECK-NEXT:   FallThroughAttr
 // CHECK-NEXT:   NullStmt
 
+int TestLikelyAttribute(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[likely]]
+return 1;
+  return 2;
+}
+// CHECK:  FunctionDecl{{.*}}TestLikelyAttribute
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelyAttr
+// CHECK-NEXT: IfStmt
+
 [[clang::warn_unused_result]] int TestCXX11DeclAttr();
 // CHECK:  FunctionDecl{{.*}}TestCXX11DeclAttr
 // CHECK-NEXT:   WarnUnusedResultAttr
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,21 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelyAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+   SourceRange Range) {
+  LikelyAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  if (!llvm::isa(St)) {
+S.Diag(A.getLoc(), diag::err_likely_attr_invalid_placement) << A.getName();
+  }
+
+  if (!S.getLangOpts().CPlusPlus2a && !A.getScopeName())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +351,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likely:
+return handleLikelyAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Parse/ParseStmt.cpp
===
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1216,6 +1216,9 @@
 : Sema::ConditionKind::Boolean))
 return StmtError();
 
+  ParsedAttributesWithRange Attrs(AttrFactory);
+  MaybeParseCXX11Attributes(Attrs);
+
   llvm::Optional ConstexprCondition;
   if (IsConstexpr)
 ConstexprCondition = Cond.getKnownValue();
@@ -1314,8 +1317,13 @@
   if (ElseStmt.isInvalid())
 ElseStmt = Actions.ActOnNullStmt(ElseStmtLoc);
 
-  return Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
- ThenStmt.get(), ElseLoc, ElseStmt.get());
+  StmtResult Stmt = Actions.ActOnIfStmt(IfLoc, IsConstexpr, InitStmt.get(), Cond,
+ ThenStmt.get(), ElseLoc, ElseStmt.get());
+
+  if (!Attrs.empty())
+return Actions.ProcessStmtAttributes(Stmt.get(), Attrs, Attrs.Range);
+
+  return Stmt;
 }
 
 /// ParseSwitchStatement
Index: clang/lib/Parse/Parse

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker marked an inline comment as done.
Tyker added a comment.

I added the sema testes


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

if likely/unlikely can be applied to any statement or label what should be 
generated when it is applied to a statement that don't have any conditional 
branch ? should it be ignored without warning or error ?


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191115.
Tyker added a comment.

i think we are suppose to hook likely/unlikely on builtin_expected, for if, 
for, while, switch. but i have no idea how we could hook it if we need to 
support catch.

i added a revision with likely/unlikely and the correct semantic (i think).

i also added the required checks. i didn't find a way to detect when we are in 
a catch block. but it should perhaps be allowded.


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1) [[unlikely]]
+{
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]] // expected-error {{'likely' attribute can only appear in if, while, switch and for}}
+if (1) [[likely, likely]] {
+  // expected-error@-1 {{there can only be one likely attribue in any attribute list}}
+  // expected-note@-2 {{previously used likely attribue}}
+  [[unlikely]] return ; // expected-error {{'unlikely' attribute can only appear in if, while, switch and for}}
+}
+  else [[unlikely]] if (1) {
+  while (1) [[likely]] {
+  switch (i) {
+[[likely]] case 1:
+  default: [[likely]]
+return ;
+  }
+}
+  for (;;) [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely are mutually exclusive}}
+  // expected-note@-2 {{previously used likely attribue}}
+[[likely]] return ;
+  // expected-error@-1 {{likely and unlikely are mutually exclusive}}
+  // expected-note@-5 {{previously used unlikely attribue}}
+}
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]]; // expected-error {{'likely' attribute can only appear in if, while, switch and for}}
+  } catch (int) {
+  [[likely]] test: // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[unlikely]] return ;
+  }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,31 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelihoodAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+   SourceRange Range) {
+  LikelihoodAttr Attr(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  Scope* scope = S.getCurScope();
+  Scope* previousScope = nullptr;
+
+  if (scope)
+previousScope = scope->getParent();
+
+  //check that ths attribute is used in an if, while, for, switch or catch
+  if (!previousScope || 
+  !(previousScope->getFlags() & Scope::ControlScope) ||
+   previousScope->getFlags() & Scope::SEHExceptScope ||
+   previousScope->getFlags() & Scope::SEHTryScope ||
+   previousScope->getFlags() & Scope::FnTryCatchScope)
+ S.Diag(A.getLoc(), diag::err_likelihood_outside_control_scope) << A.getName();
+
+  if (!S.getLangOpts().CPlusPlus2a)
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  return ::new (S.Context) auto(Attr);
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -201,7 +226,21 @@
   } HintAttrs[] = {{nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr},
{nullptr, nullptr}, {nullptr, nullptr}, {nullptr, nullptr}};
 
+  //there can be only one likelyhood attribute
+  const Attr* likelihoodAttr = nullptr;
+
   for (const auto *I : Attrs) {
+if (llvm::isa(I)) {
+  if (likelihoodAttr) {
+if (std::strcmp(I->getSpelling(), likelihoodAttr->getSpelling()))
+  S.Diag(I->getLocation(), diag::err_mutuably_exclusive_likelihood) << I->getSpelling() << likelihoodAttr->getSpelling();
+else
+  S.Diag(I->getLocation(), diag::err_multiple_likelihood) << I->getSpelling();
+

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191408.
Tyker added a comment.
Herald added a subscriber: jdoerfert.

added diagnostics for contradictory attributes like for if:

  if (...) [[likely]]
return 1;
  else [[likely]]
return 2;

handled the codegen for If to generate builtin_expect but i probably didn't do 
it the canonical way. i am awaiting advice of how to do it right.


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1) [[unlikely]]
+{
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]] // expected-error {{'likely' attribute can only appear in if, while, switch and for}}
+if (1) [[likely, likely]] {
+  // expected-error@-1 {{there can only be one likely attribue in any attribute list}}
+  // expected-note@-2 {{previously used likely attribue}}
+  [[unlikely]] return ; // expected-error {{'unlikely' attribute can only appear in if, while, switch and for}}
+}
+  else [[unlikely]] if (1) {
+  while (1) [[likely]] {
+  switch (i) {
+[[likely]] case 1:
+  default: [[likely]]
+return ;
+  }
+}
+  for (;;) [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely are mutually exclusive}}
+  // expected-note@-2 {{previously used likely attribue}}
+[[likely]] return ;
+  // expected-error@-1 {{likely and unlikely are mutually exclusive}}
+  // expected-note@-5 {{previously used unlikely attribue}}
+}
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]]; // expected-error {{'likely' attribute can only appear in if, while, switch and for}}
+  } catch (int) {
+  [[likely]] test: // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[unlikely]] return ;
+  }
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,50 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelihoodAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+   SourceRange Range) {
+  LikelihoodAttr Attribute(A.getRange(), S.Context,
+   A.getAttributeSpellingListIndex());
+
+  Scope* scope = S.getCurScope();
+  Scope* previousScope = nullptr;
+
+  if (scope)
+previousScope = scope->getParent();
+
+  //check that ths attribute is used in an if, while, for, switch or catch
+  if (!previousScope || 
+  !(previousScope->getFlags() & Scope::ControlScope) ||
+   previousScope->getFlags() & Scope::SEHExceptScope ||
+   previousScope->getFlags() & Scope::SEHTryScope ||
+   previousScope->getFlags() & Scope::FnTryCatchScope)
+ S.Diag(A.getLoc(), diag::err_likelihood_outside_control_scope) << A.getName();
+
+  if (!S.getLangOpts().CPlusPlus2a)
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  clang::Attr* result = ::new (S.Context) auto(Attribute);
+
+  auto* FnScope = S.getCurFunction();
+
+  /// don't need to emit any diagnostics if we aren't in a function or the stack is empty as this is already covered by the checks above
+  if (FnScope && !FnScope->PathLikelyhoodAttrStack.empty()) {
+Attr*& TopStackAttr = FnScope->PathLikelyhoodAttrStack.back();
+if (TopStackAttr) {
+  /// only need to compare first character to differenciate between 'likely' and 'unlikely'
+  if (result->getSpelling()[0] == TopStackAttr->getSpelling()[0])
+S.Diag(result->getLocation(), diag::err_multiple_li

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191862.
Tyker added a comment.

handled codegen for if, while, for and do/while, it will generate a 
@llvm.expect before the condition based on the attribute
i changed slithly the semantic

  if (...) {  //error
  [[likely]] ...
  }
  
  if (...) [[likely]]  { // ok
   ...
  }

and added tests for AST, Semantic and codegen


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1) [[unlikely]]
+{
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]] // expected-error {{'likely' can only appear after a selection or iteration statement}}
+if (1) [[likely, likely]] {
+  // expected-warning@-1 {{was already marked likely}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[unlikely]] return ; // expected-error {{'unlikely' can only appear after a selection or iteration statement}}
+}
+  else [[unlikely]] if (1) {
+  while (1) [[likely]] {
+  // switch (i) { switch support isn't implemented yet
+  //   [[likely]] case 1:
+  // default: [[likely]]
+  //   return ;
+  // }
+}
+  for (;;) [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely are mutually exclusive}}
+  // expected-note@-2 {{previous attribute is here}}
+[[likely]] return ;
+  // expected-warning@-1 {{was already marked likely}}
+  // expected-note@-5 {{previous attribute is here}}
+}
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]]; // expected-error {{'likely' can only appear after a selection or iteration statement}}
+  } catch (int) {
+  [[likely]] test: // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[unlikely]] return ; // expected-error {{'unlikely' can only appear after a selection or iteration statement}}
+  }
+}
\ No newline at end of file
Index: clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++2a -cc1 -emit-llvm -disable-llvm-passes -O3 %s -o - -triple %itanium_abi_triple | FileCheck %s
+
+int test(int i) {
+  if (i == 0) {
+i = i + 1;
+  } else [[likely]]
+return 1;
+  // CHECK: %expval = call i1 @llvm.expect.i1(i1 %cmp, i1 false)
+  while (i == 1) [[unlikely]] {
+return 2;
+  }
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 false)
+  for (;i == 4;) [[unlikely]] {
+return 2;
+  }
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 false)
+  do [[likely]] {
+return 2;
+  } while (i == 3);
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 true)
+  return 0;
+}
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -1,5 +1,39 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux -std=c++11 -Wno-deprecated-declarations -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s
 
+int TestLikelyAttributeIf(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[unlikely]]
+return 1;
+  return 2;
+}
+// CHECK:  IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelihoodAttr 0x{{[^ ]*}}  likely
+// CHECK:  IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelihoodAttr 0x{{[^ ]*}}  unlikely
+
+int TestLikelyAttributeLoops(int i) {
+  while (i == 1) [[likely]] {
+return 0;
+  }
+  for (;;) [[unlikely]]
+do [[likely]] {
+  return 1;
+} while (i == 2);
+  re

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

about the semantic issue. we will need for diagnostics purposes to detect 
attribute in branches during the semantic phase. so where and how can we store 
this information in the AST so that the CodeGen doesn't have to look through 
branches again for this attributes ?


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 192004.
Tyker added a comment.

i implemented the semantic the changes for if for, while and do while statement 
and the AST change to if. can you review it and tell me if it is fine so i 
implement the rest. i didn't update the test so they will fail.


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Stmt.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1) [[unlikely]]
+{
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]] // expected-error {{'likely' can only appear after a selection or iteration statement}}
+if (1) [[likely, likely]] {
+  // expected-warning@-1 {{was already marked likely}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[unlikely]] return ; // expected-error {{'unlikely' can only appear after a selection or iteration statement}}
+}
+  else [[unlikely]] if (1) {
+  while (1) [[likely]] {
+  // switch (i) { switch support isn't implemented yet
+  //   [[likely]] case 1:
+  // default: [[likely]]
+  //   return ;
+  // }
+}
+  for (;;) [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely are mutually exclusive}}
+  // expected-note@-2 {{previous attribute is here}}
+[[likely]] return ;
+  // expected-warning@-1 {{was already marked likely}}
+  // expected-note@-5 {{previous attribute is here}}
+}
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]]; // expected-error {{'likely' can only appear after a selection or iteration statement}}
+  } catch (int) {
+  [[likely]] test: // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[unlikely]] return ; // expected-error {{'unlikely' can only appear after a selection or iteration statement}}
+  }
+}
\ No newline at end of file
Index: clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++2a -cc1 -emit-llvm -disable-llvm-passes -O3 %s -o - -triple %itanium_abi_triple | FileCheck %s
+
+int test(int i) {
+  if (i == 0) {
+i = i + 1;
+  } else [[likely]]
+return 1;
+  // CHECK: %expval = call i1 @llvm.expect.i1(i1 %cmp, i1 false)
+  while (i == 1) [[unlikely]] {
+return 2;
+  }
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 false)
+  for (;i == 4;) [[unlikely]] {
+return 2;
+  }
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 false)
+  do [[likely]] {
+return 2;
+  } while (i == 3);
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 true)
+  return 0;
+}
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -1,5 +1,39 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux -std=c++11 -Wno-deprecated-declarations -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s
 
+int TestLikelyAttributeIf(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[unlikely]]
+return 1;
+  return 2;
+}
+// CHECK:  IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelihoodAttr 0x{{[^ ]*}}  likely
+// CHECK:  IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelihoodAttr 0x{{[

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 192012.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Stmt.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -139,6 +139,7 @@
   Record.push_back(HasElse);
   Record.push_back(HasVar);
   Record.push_back(HasInit);
+  Record.push_back(S->getBranchHint());
 
   Record.AddStmt(S->getCond());
   Record.AddStmt(S->getThen());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -222,6 +222,7 @@
   bool HasElse = Record.readInt();
   bool HasVar = Record.readInt();
   bool HasInit = Record.readInt();
+  S->setBranchHint(static_cast(Record.readInt()));
 
   S->setCond(Record.readSubExpr());
   S->setThen(Record.readSubStmt());
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,46 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelihoodAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+  SourceRange Range) {
+  Attr *Attr = ::new (S.Context) LikelihoodAttr(
+  A.getRange(), S.Context, A.getAttributeSpellingListIndex());
+
+  if (!S.getLangOpts().CPlusPlus2a && A.isCXX11Attribute())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  Scope *CurScope = S.getCurScope();
+
+  if (CurScope) {
+Scope *ControlScope = CurScope->getParent();
+if (!ControlScope ||
+!(ControlScope->getFlags() & Scope::ControlScope ||
+  ControlScope->getFlags() & Scope::BreakScope) ||
+ControlScope->getFlags() & Scope::SEHExceptScope ||
+ControlScope->getFlags() & Scope::SEHTryScope ||
+ControlScope->getFlags() & Scope::TryScope ||
+ControlScope->getFlags() & Scope::FnTryCatchScope) {
+  S.Diag(A.getLoc(), diag::warn_no_associated_branch) << A.getName();
+  return Attr;
+}
+
+if (ControlScope->getBranchAttr()) {
+  if (ControlScope->getBranchAttr()->getSpelling()[0] ==
+  Attr->getSpelling()[0])
+S.Diag(Attr->getLocation(), diag::err_repeat_attribute)
+<< Attr->getSpelling();
+  else
+S.Diag(Attr->getLocation(), diag::err_attributes_are_not_compatible)
+<< Attr->getSpelling()
+<< ControlScope->getBranchAttr()->getSpelling();
+  S.Diag(ControlScope->getBranchAttr()->getLocation(),
+ diag::note_previous_attribute);
+} else
+  ControlScope->setBranchAttr(Attr);
+  }
+  return Attr;
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +376,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likelihood:
+return handleLikelihoodAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -521,11 +521,48 @@
 };
 }
 
-StmtResult
-Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
-  ConditionResult Cond,
-  Stmt *thenStmt, SourceLocation ElseLoc,
-  Stmt *elseStmt) {
+BranchHint Sema::HandleIfStmtHint(Stmt *thenStmt, Stmt *elseStmt,
+  Attr *ThenAttr, Attr *ElseAttr) {
+  // diagnose branch with attribute and null statement as empty body
+  if (thenStmt && isa(thenStmt) &&
+  isa(dyn_cast(thenStmt)->getSubStmt()))
+DiagnoseEmptyStmtBody(
+dyn_cast(thenStmt)->getSubStmt()->getBeginLoc(),
+dyn_cast(thenStmt)->getSubStmt(),
+diag::warn_empty_if_body);
+  if (elseStmt && isa(elseStmt) &&
+  isa(dyn_cast(elseStmt)->getSubStmt()))
+DiagnoseEmptyStmtBody(
+dyn_cast(elseStmt)->getSubStmt()->getBeginLoc(),
+dyn_ca

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

@riccibruno Done


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

@lebedev.ri where are tests for AST serialization are located ? i didn't find 
them.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 192811.
Tyker retitled this revision from "[clang] Adding the Likely Attribute from 
C++2a to AST" to "[clang] Adding the Likelihood Attribute from C++2a".
Tyker added a comment.

@aaron.ballman fixed based on feedback.
added semantic support for switch statment.


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -139,6 +139,7 @@
   Record.push_back(HasElse);
   Record.push_back(HasVar);
   Record.push_back(HasInit);
+  Record.push_back(S->getBranchHint());
 
   Record.AddStmt(S->getCond());
   Record.AddStmt(S->getThen());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -222,6 +222,7 @@
   bool HasElse = Record.readInt();
   bool HasVar = Record.readInt();
   bool HasInit = Record.readInt();
+  S->setBranchHint(static_cast(Record.readInt()));
 
   S->setCond(Record.readSubExpr());
   S->setThen(Record.readSubStmt());
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,55 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelihoodAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+  SourceRange Range) {
+  LikelihoodAttr *Attr = ::new (S.Context) LikelihoodAttr(
+  A.getRange(), S.Context, A.getAttributeSpellingListIndex());
+
+  if (!S.getLangOpts().CPlusPlus2a && A.isCXX11Attribute())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  if (isa(St) || isa(St)) {
+auto *FnScope = S.getCurFunction();
+if (FnScope->SwitchStack.empty()) {
+  S.Diag(A.getLoc(), diag::warn_likelihood_on_case_outside_switch) << Attr->getSpelling()
+<< (isa(St) ? "case" : "default");
+}
+return Attr;
+  }
+
+  Scope *CurScope = S.getCurScope();
+  if (CurScope) {
+Scope *ControlScope = CurScope->getParent();
+if (!ControlScope ||
+ !(ControlScope->getFlags() & Scope::ControlScope ||
+   ControlScope->getFlags() & Scope::BreakScope) ||
+ ControlScope->getFlags() & Scope::SEHExceptScope ||
+ ControlScope->getFlags() & Scope::SEHTryScope ||
+ ControlScope->getFlags() & Scope::TryScope ||
+ ControlScope->getFlags() & Scope::FnTryCatchScope) {
+  S.Diag(A.getLoc(), diag::warn_no_likelihood_attr_associated_branch)
+  << A.getName();
+  return Attr;
+}
+
+if (ControlScope->getBranchAttr()) {
+  if (ControlScope->getBranchAttr()->getSpelling()[0] ==
+  Attr->getSpelling()[0])
+S.Diag(Attr->getLocation(), diag::err_repeat_attribute)
+<< Attr->getSpelling();
+  else
+S.Diag(Attr->getLocation(), diag::err_attributes_are_not_compatible)
+<< Attr->getSpelling()
+<< ControlScope->getBranchAttr()->getSpelling();
+  S.Diag(ControlScope->getBranchAttr()->getLocation(),
+ diag::note_previous_attribute);
+} else
+  ControlScope->setBranchAttr(Attr);
+  }
+  return Attr;
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +385,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likelihood:
+return handleLikelihoodAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -521,11 +521,27 @@
 };
 }
 
-StmtResult
-Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
-  ConditionResult Cond,
-  Stmt *thenStmt, SourceLocation ElseLoc,
-

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 192979.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -140,6 +140,7 @@
   Record.push_back(HasElse);
   Record.push_back(HasVar);
   Record.push_back(HasInit);
+  Record.push_back(S->getBranchHint());
 
   Record.AddStmt(S->getCond());
   Record.AddStmt(S->getThen());
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -223,6 +223,7 @@
   bool HasElse = Record.readInt();
   bool HasVar = Record.readInt();
   bool HasInit = Record.readInt();
+  S->setBranchHint(static_cast(Record.readInt()));
 
   S->setCond(Record.readSubExpr());
   S->setThen(Record.readSubStmt());
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -51,6 +51,55 @@
   return ::new (S.Context) auto(Attr);
 }
 
+static Attr *handleLikelihoodAttr(Sema &S, Stmt *St, const ParsedAttr &A,
+  SourceRange Range) {
+  LikelihoodAttr *Attr = ::new (S.Context) LikelihoodAttr(
+  A.getRange(), S.Context, A.getAttributeSpellingListIndex());
+
+  if (!S.getLangOpts().CPlusPlus2a && A.isCXX11Attribute())
+S.Diag(A.getLoc(), diag::ext_cxx2a_attr) << A.getName();
+
+  if (isa(St) || isa(St)) {
+auto *FnScope = S.getCurFunction();
+if (FnScope->SwitchStack.empty()) {
+  S.Diag(A.getLoc(), diag::warn_likelihood_on_case_outside_switch) << Attr->getSpelling()
+<< (isa(St) ? "case" : "default");
+}
+return Attr;
+  }
+
+  Scope *CurScope = S.getCurScope();
+  if (CurScope) {
+Scope *ControlScope = CurScope->getParent();
+if (!ControlScope ||
+ !(ControlScope->getFlags() & Scope::ControlScope ||
+   ControlScope->getFlags() & Scope::BreakScope) ||
+ ControlScope->getFlags() & Scope::SEHExceptScope ||
+ ControlScope->getFlags() & Scope::SEHTryScope ||
+ ControlScope->getFlags() & Scope::TryScope ||
+ ControlScope->getFlags() & Scope::FnTryCatchScope) {
+  S.Diag(A.getLoc(), diag::warn_no_likelihood_attr_associated_branch)
+  << A.getName();
+  return Attr;
+}
+
+if (ControlScope->getBranchAttr()) {
+  if (ControlScope->getBranchAttr()->getSpelling()[0] ==
+  Attr->getSpelling()[0])
+S.Diag(Attr->getLocation(), diag::err_repeat_attribute)
+<< Attr->getSpelling();
+  else
+S.Diag(Attr->getLocation(), diag::err_attributes_are_not_compatible)
+<< Attr->getSpelling()
+<< ControlScope->getBranchAttr()->getSpelling();
+  S.Diag(ControlScope->getBranchAttr()->getLocation(),
+ diag::note_previous_attribute);
+} else
+  ControlScope->setBranchAttr(Attr);
+  }
+  return Attr;
+}
+
 static Attr *handleSuppressAttr(Sema &S, Stmt *St, const ParsedAttr &A,
 SourceRange Range) {
   if (A.getNumArgs() < 1) {
@@ -336,6 +385,8 @@
 return nullptr;
   case ParsedAttr::AT_FallThrough:
 return handleFallThroughAttr(S, St, A, Range);
+  case ParsedAttr::AT_Likelihood:
+return handleLikelihoodAttr(S, St, A, Range);
   case ParsedAttr::AT_LoopHint:
 return handleLoopHintAttr(S, St, A, Range);
   case ParsedAttr::AT_OpenCLUnrollHint:
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -521,11 +521,27 @@
 };
 }
 
-StmtResult
-Sema::ActOnIfStmt(SourceLocation IfLoc, bool IsConstexpr, Stmt *InitStmt,
-  ConditionResult Cond,
-  Stmt *thenStmt, SourceLocation ElseLoc,
-  Stmt *elseStmt) {
+BranchHint Sema::HandleIfStmtHint(Stmt *ThenStmt, Stmt *elseStmt,
+  LikelihoodAttr *ThenAttr, LikelihoodAttr *ElseAttr) {
+  BranchHint Hint = NoHint;
+  // diagnose conflicting attribute and

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked 2 inline comments as done.
Tyker added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:2208
+}
+
 CFGBlock *CFGBuilder::VisitStmt(Stmt *S, AddStmtChoice asc) {

riccibruno wrote:
> I don't understand why this is needed. Can you explain it ? Also I think that 
> someone familiar with this code should comment on this (maybe @NoQ ?)
the detail of why are complicated and i don't have them all in head but without 
this edit in cases like 

```
switch (...) {
[[likely]] case 1:
...
[[fallthrough]];
default:
...
}
```
the fallthrough attribute emitted a diagnostic because is wasn't handling 
attributed case statement. the edit i performed is probably not the optimal way 
to solve the issue as it only solves the issue for likelihood attribute. but i 
don't know any other attribute that can be applied on a case statement but if 
they were others they would probably have the same issue. but the code is quite 
hard to follow and i didn't wanted to break anything. so this is what i came up 
with.
i am going to look into it to find a better solution.



Comment at: clang/lib/CodeGen/CGStmt.cpp:705
+}
+
 void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,

riccibruno wrote:
> I believe that the lowering is incorrect. I applied your patch and here 
> ({F8571803}) is the IR that clang generates (obtained with `-O1 -S -emit-llvm 
> -Xclang -disable-llvm-passes -g0`) for this code:
> 
> ```
> bool f(bool i);
> bool g(bool i);
> 
> bool h1(bool i) {
>   if (i) [[likely]]
> return f(i);
>   return g(i);
> }
> 
> bool h2(bool i) {
>   if (__builtin_expect(i, true))
> return f(i);
>   return g(i);
> }
> ```
> 
> In particular for the branch in `h1` we have:
> ```
>   %tobool = trunc i8 %0 to i1
>   %expval = call i1 @llvm.expect.i1(i1 %tobool, i1 true)
>   br i1 %tobool, label %if.then, label %if.end
> ```
> Note that `%expval` is not used. Compare this to the branch in `h2`:
> ```
>   %tobool = trunc i8 %0 to i1
>   %conv = zext i1 %tobool to i64
>   %expval = call i64 @llvm.expect.i64(i64 %conv, i64 1)
>   %tobool1 = icmp ne i64 %expval, 0
>   br i1 %tobool1, label %if.then, label %if.end
> ```
> where the extra conversions are because of the signature of 
> `__builtin_expect`.
from reading the documentation it seemed to me that both were equivalent. but 
after further checking there aren't.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 193015.
Tyker added a comment.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.

@riccibruno i fixed based on feedback everything except the CFG edit as i still 
need to analyse the situation.

added AST and CodeGen for For, While, Do and CXXFor.
added tests for Semantic, AST, PCH


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/PCH/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -std=c++2a
+
+// formating this file will break the test
+// clang-format off
+
+int f(int);
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]] { f(i); }
+  else if (i == 2)
+[[likely]] return f(i);
+  else
+[[unlikely]] { return f(i + 1); }
+  return i;
+}
+
+[[likely]] typedef int n1;
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2;
+// expected-error@-1 {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]];
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E{One};
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+[[likely]]
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]]
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  if (f(i)) [[likely, likely]] {
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-2 {{previous attribute is here}}
+[[unlikely]] return;
+// expected-warning@-1 {{attribute 'unlikely' is not associated with a branch and is ignored}}
+  }
+  else [[unlikely]] if (f(i)) {
+while (f(i))
+  [[likely]] {
+switch (i) {
+  [[likely]] case 1 : default : [[likely]];
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  [[unlikely]] case 3 : f(i);
+  [[fallthrough]];
+case 4:
+  return;
+}
+  }
+for (;;)
+  [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely attributes are not compatible}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[likely]] return;
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-5 {{previous attribute is here}}
+  }
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]];
+// expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  } catch (int) {
+[[likely]] test :
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+[[unlikely]] return;
+  }
+}
+
+// clang-format off
Index: clang/test/SemaCXX/cxx17-compat.cpp
===
--- clang/test/SemaCXX/cxx17-compat.cpp
+++ clang/test/SemaCXX/cxx17-compat.cpp
@@ -56,10 +56,33 @@
 };
 
 void ForRangeInit() {
-  for (int arr[3] = {1, 2, 3}; int n : arr) {}
+  for (int arr[3] = {1, 2, 3}; int n : arr) {
+  }
 #if __cplusplus <= 201703L
-// expected-warning@-2 {{range-based for loop initialization statements are a C++2a extension}}
+  // expected-warning@-2 {{range-based for loop initialization statements are a
+  // C++2a extension}}
 #else
-// expected-warning@-4 {{range-based for loop initialization statements are incompatible with C++ standards before C++2a}}
+  // expected-warning@-4 {{range-based for loop initialization statements are
+  // incompatible with C++ standards before C++2a}}
+#endif
+}
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'unlikely' attribute is a

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked an inline comment as done.
Tyker added inline comments.
Herald added a subscriber: rnkovacs.



Comment at: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp:70
+
+// clang-format off

just saw the //clang format off at the bottom ill remove it


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked an inline comment as done.
Tyker added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:528
+if (ElseLikelihoodAttr && ThenLikelihoodAttr->isEqual(ElseLikelihoodAttr)) 
{
+  Diag(ElseLikelihoodAttr->getLocation(),
+   diag::warn_conflicting_likelihood_attrs)

clang-format passed on the whole file i am fixing it


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 193016.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/PCH/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,68 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -std=c++2a
+
+// formating this file will break the test
+// clang-format off
+
+int f(int);
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]] { f(i); }
+  else if (i == 2)
+[[likely]] return f(i);
+  else
+[[unlikely]] { return f(i + 1); }
+  return i;
+}
+
+[[likely]] typedef int n1;
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2;
+// expected-error@-1 {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]];
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E{One};
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+[[likely]]
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]]
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  if (f(i)) [[likely, likely]] {
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-2 {{previous attribute is here}}
+[[unlikely]] return;
+// expected-warning@-1 {{attribute 'unlikely' is not associated with a branch and is ignored}}
+  }
+  else [[unlikely]] if (f(i)) {
+while (f(i))
+  [[likely]] {
+switch (i) {
+  [[likely]] case 1 : default : [[likely]];
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  [[unlikely]] case 3 : f(i);
+  [[fallthrough]];
+case 4:
+  return;
+}
+  }
+for (;;)
+  [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely attributes are not compatible}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[likely]] return;
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-5 {{previous attribute is here}}
+  }
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]];
+// expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  } catch (int) {
+[[likely]] test :
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+[[unlikely]] return;
+  }
+}
\ No newline at end of file
Index: clang/test/SemaCXX/cxx17-compat.cpp
===
--- clang/test/SemaCXX/cxx17-compat.cpp
+++ clang/test/SemaCXX/cxx17-compat.cpp
@@ -56,10 +56,33 @@
 };
 
 void ForRangeInit() {
-  for (int arr[3] = {1, 2, 3}; int n : arr) {}
+  for (int arr[3] = {1, 2, 3}; int n : arr) {
+  }
 #if __cplusplus <= 201703L
-// expected-warning@-2 {{range-based for loop initialization statements are a C++2a extension}}
+  // expected-warning@-2 {{range-based for loop initialization statements are a
+  // C++2a extension}}
 #else
-// expected-warning@-4 {{range-based for loop initialization statements are incompatible with C++ standards before C++2a}}
+  // expected-warning@-4 {{range-based for loop initialization statements are
+  // incompatible with C++ standards before C++2a}}
+#endif
+}
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'unlikely' attribute is a C++2a
+// extension}}
+#endif
+{
+  f(i);
+}
+  else if (i == 2)
+[[likely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'likely' attribute is a C++2a
+// extension}}
 #endif
+return f(i);
+  return i;
 }
Index: clang/test/PCH/cxx2a-l

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked an inline comment as done.
Tyker added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:2208
+}
+
 CFGBlock *CFGBuilder::VisitStmt(Stmt *S, AddStmtChoice asc) {

NoQ wrote:
> Tyker wrote:
> > riccibruno wrote:
> > > I don't understand why this is needed. Can you explain it ? Also I think 
> > > that someone familiar with this code should comment on this (maybe @NoQ ?)
> > the detail of why are complicated and i don't have them all in head but 
> > without this edit in cases like 
> > 
> > ```
> > switch (...) {
> > [[likely]] case 1:
> > ...
> > [[fallthrough]];
> > default:
> > ...
> > }
> > ```
> > the fallthrough attribute emitted a diagnostic because is wasn't handling 
> > attributed case statement. the edit i performed is probably not the optimal 
> > way to solve the issue as it only solves the issue for likelihood 
> > attribute. but i don't know any other attribute that can be applied on a 
> > case statement but if they were others they would probably have the same 
> > issue. but the code is quite hard to follow and i didn't wanted to break 
> > anything. so this is what i came up with.
> > i am going to look into it to find a better solution.
> The [[likely]] attribute should not affect the overall topology of the CFG. 
> It might be a nice piece of metadata to add to a CFG edge or to a CFG 
> terminator, but for most consumers of the CFG (various static analyses such 
> as analysis-based warnings or the Static Analyzer) the attribute should have 
> little to no effect - the tool would still need to explore both branches. I 
> don't know how exactly the fallthrough warning operates, but i find it likely 
> (no pun intended) that the fallthrough warning itself should be updated, not 
> the CFG.
> 
> It is probable that for compiler warnings it'll only cause false negatives, 
> which is not as bad as false positives, but i wouldn't rely on that. 
> Additionally, false negatives in such rare scenarios will be very hard to 
> notice later. So i'm highly in favor of aiming for the correct solution in 
> this patch.
> 
> 
i think we all agree that the CFG structure shouldn't be affected by the 
presence or absence of the likely attribute. but in the current state(no 
changes to the CFG) it does for example. 

the CFG were obtained without the edit in CFG.cpp but with the added likely 
attribute
using: clang -cc1 -analyze test.cpp -analyzer-checker=debug.DumpCFG

input:

```
int f(int i) {
switch (i) {
[[likely]] case 1:
return 1;
}
return i;
}

```
outputs:

```
 [B5 (ENTRY)]
   Succs (1): B2

 [B1]
   1: i
   2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
   3: return [B1.2];
   Preds (2): B3 B2
   Succs (1): B0

 [B2]
   1: i
   2: [B2.1] (ImplicitCastExpr, LValueToRValue, int)
   T: switch [B2.2]
   Preds (1): B5
   Succs (2): B4 B1

 [B3]
   1:  [[likely]]case 1:
[B4.2]   Succs (1): B1

 [B4]
  case 1:
   1: 1
   2: return [B4.1];
   Preds (1): B2
   Succs (1): B0

 [B0 (EXIT)]
   Preds (2): B1 B4

```
and
input:

```
int f(int i) {
switch (i) {
 case 1:
return 1;
}
return i;
}

```
outputs:

```
 [B4 (ENTRY)]
   Succs (1): B2

 [B1]
   1: i
   2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
   3: return [B1.2];
   Preds (1): B2
   Succs (1): B0

 [B2]
   1: i
   2: [B2.1] (ImplicitCastExpr, LValueToRValue, int)
   T: switch [B2.2]
   Preds (1): B4
   Succs (2): B3 B1

 [B3]
  case 1:
   1: 1
   2: return [B3.1];
   Preds (1): B2
   Succs (1): B0

 [B0 (EXIT)]
   Preds (2): B1 B3
```
i think think this is the underlying issue. the false diagnostic from 
fallthrough previously mentioned is a consequence of this


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked an inline comment as done.
Tyker added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:2208
+}
+
 CFGBlock *CFGBuilder::VisitStmt(Stmt *S, AddStmtChoice asc) {

NoQ wrote:
> Tyker wrote:
> > NoQ wrote:
> > > Tyker wrote:
> > > > riccibruno wrote:
> > > > > I don't understand why this is needed. Can you explain it ? Also I 
> > > > > think that someone familiar with this code should comment on this 
> > > > > (maybe @NoQ ?)
> > > > the detail of why are complicated and i don't have them all in head but 
> > > > without this edit in cases like 
> > > > 
> > > > ```
> > > > switch (...) {
> > > > [[likely]] case 1:
> > > > ...
> > > > [[fallthrough]];
> > > > default:
> > > > ...
> > > > }
> > > > ```
> > > > the fallthrough attribute emitted a diagnostic because is wasn't 
> > > > handling attributed case statement. the edit i performed is probably 
> > > > not the optimal way to solve the issue as it only solves the issue for 
> > > > likelihood attribute. but i don't know any other attribute that can be 
> > > > applied on a case statement but if they were others they would probably 
> > > > have the same issue. but the code is quite hard to follow and i didn't 
> > > > wanted to break anything. so this is what i came up with.
> > > > i am going to look into it to find a better solution.
> > > The [[likely]] attribute should not affect the overall topology of the 
> > > CFG. It might be a nice piece of metadata to add to a CFG edge or to a 
> > > CFG terminator, but for most consumers of the CFG (various static 
> > > analyses such as analysis-based warnings or the Static Analyzer) the 
> > > attribute should have little to no effect - the tool would still need to 
> > > explore both branches. I don't know how exactly the fallthrough warning 
> > > operates, but i find it likely (no pun intended) that the fallthrough 
> > > warning itself should be updated, not the CFG.
> > > 
> > > It is probable that for compiler warnings it'll only cause false 
> > > negatives, which is not as bad as false positives, but i wouldn't rely on 
> > > that. Additionally, false negatives in such rare scenarios will be very 
> > > hard to notice later. So i'm highly in favor of aiming for the correct 
> > > solution in this patch.
> > > 
> > > 
> > i think we all agree that the CFG structure shouldn't be affected by the 
> > presence or absence of the likely attribute. but in the current state(no 
> > changes to the CFG) it does for example. 
> > 
> > the CFG were obtained without the edit in CFG.cpp but with the added likely 
> > attribute
> > using: clang -cc1 -analyze test.cpp -analyzer-checker=debug.DumpCFG
> > 
> > input:
> > 
> > ```
> > int f(int i) {
> > switch (i) {
> > [[likely]] case 1:
> > return 1;
> > }
> > return i;
> > }
> > 
> > ```
> > outputs:
> > 
> > ```
> >  [B5 (ENTRY)]
> >Succs (1): B2
> > 
> >  [B1]
> >1: i
> >2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
> >3: return [B1.2];
> >Preds (2): B3 B2
> >Succs (1): B0
> > 
> >  [B2]
> >1: i
> >2: [B2.1] (ImplicitCastExpr, LValueToRValue, int)
> >T: switch [B2.2]
> >Preds (1): B5
> >Succs (2): B4 B1
> > 
> >  [B3]
> >1:  [[likely]]case 1:
> > [B4.2]   Succs (1): B1
> > 
> >  [B4]
> >   case 1:
> >1: 1
> >2: return [B4.1];
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B0 (EXIT)]
> >Preds (2): B1 B4
> > 
> > ```
> > and
> > input:
> > 
> > ```
> > int f(int i) {
> > switch (i) {
> >  case 1:
> > return 1;
> > }
> > return i;
> > }
> > 
> > ```
> > outputs:
> > 
> > ```
> >  [B4 (ENTRY)]
> >Succs (1): B2
> > 
> >  [B1]
> >1: i
> >2: [B1.1] (ImplicitCastExpr, LValueToRValue, int)
> >3: return [B1.2];
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B2]
> >1: i
> >2: [B2.1] (ImplicitCastExpr, LValueToRValue, int)
> >T: switch [B2.2]
> >Preds (1): B4
> >Succs (2): B3 B1
> > 
> >  [B3]
> >   case 1:
> >1: 1
> >2: return [B3.1];
> >Preds (1): B2
> >Succs (1): B0
> > 
> >  [B0 (EXIT)]
> >Preds (2): B1 B3
> > ```
> > i think think this is the underlying issue. the false diagnostic from 
> > fallthrough previously mentioned is a consequence of this
> Hmm, i see. I got it all wrong. Please forgive me!
> 
> You're just trying to make `CFGBuilder` support `AttributedStmt` correctly in 
> general. And the logic you're writing says "support it as any other generic 
> Stmt that doesn't have any control flow in it, unless it's a 
> `[[likely]]`-attributed `AttributedStmt`, in which case jump straight to 
> children".
> 
> Could you instead do this by implementing `CFGBuilder::VisitAttributedStmt` 
> with that logic?
> 
> I'm also not sure this logic is actually specific to `[[likely]]`. Maybe we 
> should unwrap all `AttributedStmt`s similarly?
we shouldn't handle all Attribu

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 193269.
Tyker added a comment.

fixed the CFG issue is an proper way


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/PCH/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -std=c++2a
+
+// formating this file will break the test
+// clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]] { f(i); }
+  else if (i == 2)
+[[likely]] return f(i);
+  else
+[[unlikely]] { return f(i + 1); }
+  return i;
+}
+
+[[likely]] typedef int n1;
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2;
+// expected-error@-1 {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]];
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E{One};
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+[[likely]]
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]]
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  if (f(i)) [[likely, likely]] {
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-2 {{previous attribute is here}}
+[[unlikely]] return;
+// expected-warning@-1 {{attribute 'unlikely' is not associated with a branch and is ignored}}
+  }
+  else [[unlikely]] if (f(i)) {
+while (f(i))
+  [[likely]] {
+switch (i) {
+  [[likely]] case 1 : default : [[likely]];
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  [[unlikely]] case 3 : f(i);
+  [[fallthrough]];
+case 4:
+  return;
+}
+  }
+for (;;)
+  [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely attributes are not compatible}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[likely]] return;
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-5 {{previous attribute is here}}
+  }
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]];
+// expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  } catch (int) {
+[[likely]] test :
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+[[unlikely]] return;
+  }
+}
Index: clang/test/SemaCXX/cxx17-compat.cpp
===
--- clang/test/SemaCXX/cxx17-compat.cpp
+++ clang/test/SemaCXX/cxx17-compat.cpp
@@ -63,3 +63,23 @@
 // expected-warning@-4 {{range-based for loop initialization statements are incompatible with C++ standards before C++2a}}
 #endif
 }
+
+//clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'unlikely' attribute is a C++2a extension}}
+#endif
+{
+  f(i);
+}
+  else if (i == 2)
+[[likely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'likely' attribute is a C++2a extension}}
+#endif
+return f(i);
+  return i;
+}
Index: clang/test/PCH/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,57 @@
+// Test this without pch.
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s -ast-dump | FileCheck %s
+
+// Test with pch. Use '-ast-dump' to force deserialization of function bodies.
+// RUN: %clang_cc1 -x c++-header -std=c++2a -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -fsyntax-only -verify 

[PATCH] D59467: [clang] Adding the Likelihood Attribute from C++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 193284.

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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/StmtCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Scope.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/AST/StmtCXX.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/CFG.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/Scope.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/PCH/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx17-compat.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,66 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify -std=c++2a
+
+// formating this file will break the test
+// clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]] { f(i); }
+  else if (i == 2)
+[[likely]] return f(i);
+  else
+[[unlikely]] { return f(i + 1); }
+  return i;
+}
+
+[[likely]] typedef int n1;
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2;
+// expected-error@-1 {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]];
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E{One};
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+[[likely]]
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]]
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  if (f(i)) [[likely, likely]] {
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-2 {{previous attribute is here}}
+[[unlikely]] return;
+// expected-warning@-1 {{attribute 'unlikely' is not associated with a branch and is ignored}}
+  }
+  else [[unlikely]] if (f(i)) {
+while (f(i))
+  [[likely]] {
+switch (i) {
+  [[likely]] case 1 : default : [[likely]];
+  // expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  [[unlikely]] case 3 : f(i);
+  [[fallthrough]];
+case 4:
+  return;
+}
+  }
+for (;;)
+  [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely attributes are not compatible}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[likely]] return;
+// expected-error@-1 {{likely attribute cannot be repeated}}
+// expected-note@-5 {{previous attribute is here}}
+  }
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]];
+// expected-warning@-1 {{attribute 'likely' is not associated with a branch and is ignored}}
+  } catch (int) {
+[[likely]] test :
+// expected-error@-1 {{'likely' attribute cannot be applied to a declaration}}
+[[unlikely]] return;
+  }
+}
Index: clang/test/SemaCXX/cxx17-compat.cpp
===
--- clang/test/SemaCXX/cxx17-compat.cpp
+++ clang/test/SemaCXX/cxx17-compat.cpp
@@ -63,3 +63,23 @@
 // expected-warning@-4 {{range-based for loop initialization statements are incompatible with C++ standards before C++2a}}
 #endif
 }
+
+//clang-format off
+
+int f(int i) {
+  if (i == 1)
+[[unlikely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'unlikely' attribute is a C++2a extension}}
+#endif
+{
+  f(i);
+}
+  else if (i == 2)
+[[likely]]
+#if __cplusplus <= 201703L
+// expected-warning@-2 {{use of the 'likely' attribute is a C++2a extension}}
+#endif
+return f(i);
+  return i;
+}
Index: clang/test/PCH/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/PCH/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,57 @@
+// Test this without pch.
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s -ast-dump | FileCheck %s
+
+// Test with pch. Use '-ast-dump' to force deserialization of function bodies.
+// RUN: %clang_cc1 -x c++-header -std=c++2a -emit-pch -o %t %s
+// RUN: %clang_cc1 -std=c++2a -include-pch %t -fsyntax-only -verify %s
+// -ast-dump-all | FileCheck %s
+
+// expected-no-diagnost

[PATCH] D60523: [clang] Bugfixe for 41400

[Gauthier via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

this is a bugfixe for bugtracker 

added nullptr check at the relevent place and test


Repository:
  rC Clang

https://reviews.llvm.org/D60523

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/using-decl-1.cpp


Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -396,3 +396,10 @@
   using N::Y;
   using N::Z;
 }
+
+// expected-error@+5 {{requires a qualified name}}
+// expected-error@+4 {{expected ';'}}
+// expected-error@+3 {{expected '}'}}
+// expected-note@+2 {{to match this '{'}}
+// expected-error@+1 {{expected ';'}}
+template struct S { using S
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -90,7 +90,7 @@
   // When naming a constructor as a member of a dependent context (eg, in a
   // friend declaration or an inherited constructor declaration), form an
   // unresolved "typename" type.
-  if (CurClass->isDependentContext() && !EnteringContext) {
+  if (CurClass->isDependentContext() && !EnteringContext && SS.getScopeRep()) {
 QualType T = Context.getDependentNameType(ETK_None, SS.getScopeRep(), &II);
 return ParsedType::make(T);
   }


Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -396,3 +396,10 @@
   using N::Y;
   using N::Z;
 }
+
+// expected-error@+5 {{requires a qualified name}}
+// expected-error@+4 {{expected ';'}}
+// expected-error@+3 {{expected '}'}}
+// expected-note@+2 {{to match this '{'}}
+// expected-error@+1 {{expected ';'}}
+template struct S { using S
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -90,7 +90,7 @@
   // When naming a constructor as a member of a dependent context (eg, in a
   // friend declaration or an inherited constructor declaration), form an
   // unresolved "typename" type.
-  if (CurClass->isDependentContext() && !EnteringContext) {
+  if (CurClass->isDependentContext() && !EnteringContext && SS.getScopeRep()) {
 QualType T = Context.getDependentNameType(ETK_None, SS.getScopeRep(), &II);
 return ParsedType::make(T);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

this is a bugfix for this 

added a constant copy of argument to CallStackFrame.
and use it in describeCall instead of the modifiable version od arguments.
added test for the diagnostic


Repository:
  rC Clang

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,25 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i) {
+  i -= 1;
+  return 1 << i; // expected-note {{negative shift count -8}}
+}
+
+constexpr int f2(int i) {
+  i -= 1;
+  if (i < -6)
+return f1(i); // expected-note {{'f1(-7)}}
+  // expected-note@+6 {{'f2(-6)}}
+  // expected-note@+5 {{'f2(-5)}}
+  // expected-note@+4 {{'f2(-4)}}
+  // expected-note@+3 {{'f2(-3)}}
+  // expected-note@+2 {{'f2(-2)}}
+  // expected-note@+1 {{'f2(-1)}}
+  return f2(i);
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -476,9 +476,15 @@
 const LValue *This;
 
 /// Arguments - Parameter bindings for this function call, indexed by
-/// parameters' function scope indices.
+/// parameters' function scope indices. may be modified by called function
 APValue *Arguments;
 
+using ConstArgs = APValue const *const;
+/// ConstantArgs - Parameter bindings for this function call, indexed by
+/// parameters' function scope indices. can't be modified. used for
+/// diagnostics
+ConstArgs ConstantArgs;
+
 // Note that we intentionally use std::map here so that references to
 // values are stable.
 typedef std::pair MapKeyTy;
@@ -519,7 +525,7 @@
 
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments);
+   APValue *Arguments, ConstArgs ConstantArgs);
 ~CallStackFrame();
 
 // Return the temporary for Key whose version number is Version.
@@ -789,14 +795,15 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr,
+  nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
   EvaluatingDecl = Base;
@@ -1235,9 +1242,10 @@
 
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments)
+   APValue *Arguments, ConstArgs CArgs)
 : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+  Arguments(Arguments), ConstantArgs(CArgs), CallLoc(CallLoc),
+  Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1679,7 +1687,7 @@
   Out << ", ";
 
 const ParmVarDecl *Param = *I;
-const APValue &Arg = Frame->Arguments[ArgIndex];
+const APValue &Arg = Frame->ConstantArgs[ArgIndex];
 Arg.printPretty(Out, Frame->Info.Ctx, Param->getType());
 
 if (ArgIndex == 0 && IsMemberCall)
@@ -4455,7 +4463,11 @@
   if (!Info.CheckCallLimit(CallLoc))
 return false;
 
-  CallStackFrame Frame(Info, CallL

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 194684.
Tyker added a comment.

@Quuxplusone  edited based of feedback

i simplified the test but didn't put the original because this one test 
arguments for variable and literal and there may be corner case differences


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

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,18 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i) {
+  i -= 1;
+  return 1 << i; // expected-note {{negative shift count -2}}
+}
+
+constexpr int f2(int i) {
+  i -= 1;
+  // expected-note@+1 {{'f1(-1)}}
+  return f1(i);
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{'f2(0)}}
+constexpr int a = f2(0);
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -476,9 +476,14 @@
 const LValue *This;
 
 /// Arguments - Parameter bindings for this function call, indexed by
-/// parameters' function scope indices.
+/// parameters' function scope indices. may be modified by called function
 APValue *Arguments;
 
+/// ConstantArgs - Parameter bindings for this function call, indexed by
+/// parameters' function scope indices. can't be modified. used for
+/// diagnostics
+const APValue *ConstantArgs;
+
 // Note that we intentionally use std::map here so that references to
 // values are stable.
 typedef std::pair MapKeyTy;
@@ -519,7 +524,7 @@
 
 CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments);
+   APValue *Arguments, const APValue *ConstantArgs);
 ~CallStackFrame();
 
 // Return the temporary for Key whose version number is Version.
@@ -789,14 +794,15 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr,
+  nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
   EvaluatingDecl = Base;
@@ -1235,9 +1241,10 @@
 
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
const FunctionDecl *Callee, const LValue *This,
-   APValue *Arguments)
+   APValue *Arguments, const APValue *CArgs)
 : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-  Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+  Arguments(Arguments), ConstantArgs(CArgs), CallLoc(CallLoc),
+  Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1679,7 +1686,7 @@
   Out << ", ";
 
 const ParmVarDecl *Param = *I;
-const APValue &Arg = Frame->Arguments[ArgIndex];
+const APValue &Arg = Frame->ConstantArgs[ArgIndex];
 Arg.printPretty(Out, Frame->Info.Ctx, Param->getType());
 
 if (ArgIndex == 0 && IsMemberCall)
@@ -4455,7 +4462,11 @@
   if (!Info.CheckCallLimit(CallLoc))
 return false;
 
-  CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data());
+  // ArgValues - may be modified by the called function.
+  // ConstantArgs - can't be modified by the called function
+  const ArgVector ConstantArgs(ArgValues.begin(), ArgValues.end());
+  CallStackFrame Frame(Info, CallLoc, Callee, This, ArgValues.data(),
+   ConstantArgs.data());
 
   // For a trivial copy or move assignment, perform an APValue copy. This is
   // 

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

@rsmith i don't think collecting theses values is expansive compared to 
evaluating the expression.
but i agree that we can disable the collection of these values when it isn't 
needed.


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

https://reviews.llvm.org/D60561



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

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

the impact was much higher than i expected,  around 1% slower in average on 50 
compilation of SemaDecl with -fsyntax-only.


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

https://reviews.llvm.org/D60561



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

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 195055.
Tyker added a comment.

i changed the way arguments are stored. to make it more controllable.
added an argument call stack where it is needed.
this version slows down compilation by around 0.5% in average over 200 run for 
SemaDecl -fsyntax-only


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

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,32 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i, int b) {
+  i += 4;
+  b -= 4;
+  // expected-note@+1 {{negative shift count -7}}
+  return i << b;
+}
+
+constexpr int f2(int i) {
+  int b = 0;
+  i += 1;
+  b -= 1;
+  // expected-note@+1 {{in call to 'f1(3, -3)'}}
+  return f1(i + 2, b -= 2);
+}
+
+constexpr int f(int i) {
+  i -= 1;
+  //expected-note@+1 {{negative shift count -1}}
+  return 1 << i;
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f2(0)'}}
+constexpr int val = f2(0);
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f(0)'}}
+static_assert(f(0), "");
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -661,6 +661,10 @@
 /// CurrentCall - The top of the constexpr call stack.
 CallStackFrame *CurrentCall;
 
+/// ArgumentCallStack - Used to store Arguments to function calls
+/// only required if diagnostics should produce a callstack
+SmallVectorImpl *ArgumentCallStack;
+
 /// CallStackDepth - The number of calls in the call stack right now.
 unsigned CallStackDepth;
 
@@ -789,14 +793,14 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  ArgumentCallStack(nullptr), CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
   EvaluatingDecl = Base;
@@ -1240,10 +1244,20 @@
   Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
+  if (Info.ArgumentCallStack) {
+Info.ArgumentCallStack->reserve(Info.ArgumentCallStack->size() +
+Callee->param_size());
+std::copy(Arguments, Arguments + Callee->param_size(),
+  std::back_inserter(*Info.ArgumentCallStack));
+  }
 }
 
 CallStackFrame::~CallStackFrame() {
   assert(Info.CurrentCall == this && "calls retired out of order");
+  if (Info.ArgumentCallStack && Caller) {
+Info.ArgumentCallStack->resize(Info.ArgumentCallStack->size() -
+   Callee->param_size());
+  }
   --Info.CallStackDepth;
   Info.CurrentCall = Caller;
 }
@@ -1257,9 +1271,12 @@
   return Result;
 }
 
-static void describeCall(CallStackFrame *Frame, raw_ostream &Out);
+static void describeCall(CallStackFrame *Frame, raw_ostream &Out,
+ SmallVectorImpl *ArgumentCallStack,
+ unsigned &Pos);
 
 void EvalInfo::addCallStack(unsigned Limit) {
+  assert(ArgumentCallStack && "needed to produce a call stack");
   // Determine which calls to skip, if any.
   unsigned ActiveCalls = CallStackDepth - 1;
   unsigned SkipStart = ActiveCalls, SkipEnd = SkipStart;
@@ -1270,8 +1287,10 @@
 
   // Walk the call stack and add the diagnostics.
   unsigned CallIdx = 0;
+  unsigned Pos = ArgumentCallStack->size();
   for (CallStackFrame *Frame = CurrentCall; Frame != &BottomFrame;
Frame = Frame->Caller, ++CallIdx) {
+Pos -= Frame->Callee->param_size();
 // Skip thi

[PATCH] D60561: [clang] fixing diagnostics of constexpr callstack

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 195056.

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

https://reviews.llvm.org/D60561

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx1y.cpp

Index: clang/test/SemaCXX/constant-expression-cxx1y.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx1y.cpp
+++ clang/test/SemaCXX/constant-expression-cxx1y.cpp
@@ -1159,3 +1159,32 @@
 enum class InEnum3 {
   THREE = indirect_builtin_constant_p("abc")
 };
+
+constexpr int f1(int i, int b) {
+  i += 4;
+  b -= 4;
+  // expected-note@+1 {{negative shift count -7}}
+  return i << b;
+}
+
+constexpr int f2(int i) {
+  int b = 0;
+  i += 1;
+  b -= 1;
+  // expected-note@+1 {{in call to 'f1(3, -3)'}}
+  return f1(i + 2, b -= 2);
+}
+
+constexpr int f(int i) {
+  i -= 1;
+  //expected-note@+1 {{negative shift count -1}}
+  return 1 << i;
+}
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f2(0)'}}
+constexpr int val = f2(0);
+
+// expected-error@+2 {{constant expression}}
+// expected-note@+1 {{in call to 'f(0)'}}
+static_assert(f(0), "");
\ No newline at end of file
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -661,6 +661,10 @@
 /// CurrentCall - The top of the constexpr call stack.
 CallStackFrame *CurrentCall;
 
+/// ArgumentCallStack - Used to store Arguments to function calls
+/// only required if diagnostics should produce a callstack
+SmallVectorImpl *ArgumentCallStack;
+
 /// CallStackDepth - The number of calls in the call stack right now.
 unsigned CallStackDepth;
 
@@ -789,14 +793,14 @@
 bool checkingForOverflow() { return EvalMode == EM_EvaluateForOverflow; }
 
 EvalInfo(const ASTContext &C, Expr::EvalStatus &S, EvaluationMode Mode)
-  : Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
-CallStackDepth(0), NextCallIndex(1),
-StepsLeft(getLangOpts().ConstexprStepLimit),
-BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
-EvaluatingDecl((const ValueDecl *)nullptr),
-EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
-HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
-InConstantContext(false), EvalMode(Mode) {}
+: Ctx(const_cast(C)), EvalStatus(S), CurrentCall(nullptr),
+  ArgumentCallStack(nullptr), CallStackDepth(0), NextCallIndex(1),
+  StepsLeft(getLangOpts().ConstexprStepLimit),
+  BottomFrame(*this, SourceLocation(), nullptr, nullptr, nullptr),
+  EvaluatingDecl((const ValueDecl *)nullptr),
+  EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
+  HasFoldFailureDiagnostic(false), IsSpeculativelyEvaluating(false),
+  InConstantContext(false), EvalMode(Mode) {}
 
 void setEvaluatingDecl(APValue::LValueBase Base, APValue &Value) {
   EvaluatingDecl = Base;
@@ -1240,10 +1244,20 @@
   Arguments(Arguments), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
+  if (Info.ArgumentCallStack) {
+Info.ArgumentCallStack->reserve(Info.ArgumentCallStack->size() +
+Callee->param_size());
+std::copy(Arguments, Arguments + Callee->param_size(),
+  std::back_inserter(*Info.ArgumentCallStack));
+  }
 }
 
 CallStackFrame::~CallStackFrame() {
   assert(Info.CurrentCall == this && "calls retired out of order");
+  if (Info.ArgumentCallStack && Caller) {
+Info.ArgumentCallStack->resize(Info.ArgumentCallStack->size() -
+   Callee->param_size());
+  }
   --Info.CallStackDepth;
   Info.CurrentCall = Caller;
 }
@@ -1257,9 +1271,12 @@
   return Result;
 }
 
-static void describeCall(CallStackFrame *Frame, raw_ostream &Out);
+static void describeCall(CallStackFrame *Frame, raw_ostream &Out,
+ SmallVectorImpl *ArgumentCallStack,
+ unsigned Pos);
 
 void EvalInfo::addCallStack(unsigned Limit) {
+  assert(ArgumentCallStack && "needed to produce a call stack");
   // Determine which calls to skip, if any.
   unsigned ActiveCalls = CallStackDepth - 1;
   unsigned SkipStart = ActiveCalls, SkipEnd = SkipStart;
@@ -1270,8 +1287,10 @@
 
   // Walk the call stack and add the diagnostics.
   unsigned CallIdx = 0;
+  unsigned Pos = ArgumentCallStack->size();
   for (CallStackFrame *Frame = CurrentCall; Frame != &BottomFrame;
Frame = Frame->Caller, ++CallIdx) {
+Pos -= Frame->Callee->param_size();
 // Skip this call?
 if (CallIdx >= SkipStart && CallIdx < SkipEnd) {
   if (CallIdx == SkipStart) {
@@ -1294,7 +1313,7 @@
 
 SmallVector Buffer;
 llvm::raw_svector_ostream Out(Buffer);
-describeCall(Frame, Out);
+describeCall(Frame, Ou

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gauthier via Phabricator via cfe-commits]

Tyker created this revision.
Tyker added a reviewer: rsmith.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

this patch adds support for the explicit bool specifier.

added parsing for explicit bool specifier in ParseDecl.cpp
adapted storage of explicit specifier in AST and DeclSpec.

  was boolean value not it is a PointerIntPair with a flag and a potential 
expression.
  was stored in every function declaration. now it is stored only in 
CXXConstructorDecl, CXXDeductionGuideDecl and CXXConversionDecl.

adpated AddOverloadCandidate, AddTemplateOverloadCandidate, 
AddConversionCandidate and AddTemplateConversionCandidate
adapted template instatiation to instantiate explicit bool expressions.
to receive a boolean idicating if function resolved to be explicit should be 
removed.
adapted Serialization, ASTMatchers, ASTComparator and ASTPrinter
added test for semantic and serialization.

this patch is not yet complete. i still need to check the interaction with CTAD 
and deduction guides.
and add more tests for AST operations. but i wanted first feedback.

perhaps this patch should be splitted in smaller patchs. but making each patch 
testable as a standalone may be tricky.


Repository:
  rC Clang

https://reviews.llvm.org/D60934

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/cxx-explicit-spec.cpp
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/explicit.cpp

Index: clang/test/SemaCXX/explicit.cpp
===
--- clang/test/SemaCXX/explicit.cpp
+++ clang/test/SemaCXX/explicit.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+
 namespace Constructor {
 struct A {
   A(int);
@@ -183,7 +185,8 @@
 const int ©List7 = {b};
 const int ©List8 = {n}; // expected-error {{no viable conversion}}
   }
-  
+
+#if __cplusplus < 201707L
   void testNew()
   {
 // 5.3.4p6:
@@ -200,7 +203,8 @@
 new int[i];
 new int[ni]; // expected-error {{array size expression of type 'NotInt' requires explicit conversion to type 'int'}}
   }
-  
+#endif
+
   void testDelete()
   {
 // 5.3.5pp2:
Index: clang/test/SemaCXX/cxx2a-explicit-bool.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -0,0 +1,336 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s -verify
+
+template
+struct enable_ifv {};
+
+template
+struct enable_ifv {
+  static constexpr auto value = val;
+};
+
+template
+struct is_same {
+static constexpr bool value = false;
+};
+
+template
+struct is_same {
+static constexpr bool value = true;
+};
+
+namespace test0
+{
+
+template
+struct A {
+  explicit(1 << a)
+  //expected-error@-1 {{argument to explicit specifier is not a valid constant expression}}
+  //expected-note@-2 {{negative shift count -1}}
+  A(int);
+};
+
+A<-1> a(0); //expected-note {{in instantiation of template class}}
+
+template
+struct B {
+  explicit(b)
+  // expected-error@-1 {{use of undeclared identifier}}
+  // expected-note@-2 {{this expression is parsed as explicit(bool) since c++2a}}
+  B(int);
+};
+
+}
+
+
+namespace test1 {
+
+template
+struct A {
+  // expected-note@-1 {{candidate constructor}}
+  // expected-note@-2 {{candidate constructor}}
+  // expected-note@-3 {{candidate function}} expected-note@-3 {{candidate function}}
+  // expected-note@-4 {{candidate function}} expected-note@-4 {{candidate function}}
+  explicit(b) A(int, int = 0);
+  // expected-note@-1 {{explicit constructor declared here}}
+  // expected-note@-2 {{explicit constructor declared here}}
+  // expected-note@-3 {{explicit constructor declared here}}
+};
+
+template
+A::A(int, int) {}
+
+void f()
+{
+  A a0 = 0; // expected-error {{no viable conversion}}
+  A a1( 0);
+  A && a2 = 0;

[PATCH] D60523: [clang] Bugfixe for 41400

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

awaiting feedback on this


Repository:
  rC Clang

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

https://reviews.llvm.org/D60523



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked 2 inline comments as done.
Tyker added inline comments.



Comment at: clang/lib/Sema/DeclSpec.cpp:959
+  // Each decl-specifier shall appear at most once in a complete
+  // decl-specifier-seq, except that long may appear twice.
+  if (hasExplicitSpecifier()) {

Quuxplusone wrote:
> Spelling/grammar/capitalization-of-C++2a.
> Also, it seems to me that you've got a CWG wording issue here: what does 
> N4810 mean by "Each //decl-specifier// shall appear at most once in a 
> complete //decl-specifier-seq//, except that `long` may appear twice"? What 
> is "each" decl-specifier? Is `explicit(true)` a different decl-specifier from 
> `explicit(1+1==2)`? Is `explicit(true)` different from `explicit(false)`?
I agree that it isn't clear. but i assumed that each counts every 
explicit-specifier as one.



Comment at: clang/test/SemaCXX/explicit.cpp:189
+
+#if __cplusplus < 201707L
   void testNew()

Quuxplusone wrote:
> Why?
i added a run line with the -std=c++2a flag to ensure no regression in c++2a 
mode as well as in c++11 mode. the warning that is being tested doesn't appear 
with this flag. the #if is to prevent the test from failling with the -stdc++2a 
flag. i don't know if the flag's behavior is intended or not.
[[ https://godbolt.org/z/df_rvX | godbolt example ]]



Repository:
  rC Clang

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

https://reviews.llvm.org/D60934



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 195973.

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

https://reviews.llvm.org/D60934

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/cxx-explicit-spec.cpp
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/explicit.cpp

Index: clang/test/SemaCXX/explicit.cpp
===
--- clang/test/SemaCXX/explicit.cpp
+++ clang/test/SemaCXX/explicit.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+
 namespace Constructor {
 struct A {
   A(int);
@@ -183,7 +185,8 @@
 const int ©List7 = {b};
 const int ©List8 = {n}; // expected-error {{no viable conversion}}
   }
-  
+
+#if __cplusplus < 201707L
   void testNew()
   {
 // 5.3.4p6:
@@ -200,7 +203,8 @@
 new int[i];
 new int[ni]; // expected-error {{array size expression of type 'NotInt' requires explicit conversion to type 'int'}}
   }
-  
+#endif
+
   void testDelete()
   {
 // 5.3.5pp2:
Index: clang/test/SemaCXX/cxx2a-explicit-bool.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -0,0 +1,335 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s -verify
+
+template
+struct enable_ifv {};
+
+template
+struct enable_ifv {
+  static constexpr auto value = val;
+};
+
+template
+struct is_same {
+static constexpr bool value = false;
+};
+
+template
+struct is_same {
+static constexpr bool value = true;
+};
+
+namespace test0
+{
+
+template
+struct A {
+  explicit(1 << a)
+  //expected-error@-1 {{argument to explicit specifier is not a valid constant expression}}
+  //expected-note@-2 {{negative shift count -1}}
+  A(int);
+};
+
+A<-1> a(0); //expected-note {{in instantiation of template class}}
+
+template
+struct B {
+  explicit(b)
+  // expected-error@-1 {{use of undeclared identifier}}
+  // expected-note@-2 {{this expression is parsed as explicit(bool) since c++2a}}
+  B(int);
+};
+
+}
+
+namespace test1 {
+
+template
+struct A {
+  // expected-note@-1 {{candidate constructor}}
+  // expected-note@-2 {{candidate constructor}}
+  // expected-note@-3 {{candidate function}} expected-note@-3 {{candidate function}}
+  // expected-note@-4 {{candidate function}} expected-note@-4 {{candidate function}}
+  explicit(b) A(int, int = 0);
+  // expected-note@-1 {{explicit constructor declared here}}
+  // expected-note@-2 {{explicit constructor declared here}}
+  // expected-note@-3 {{explicit constructor declared here}}
+};
+
+template
+A::A(int, int) {}
+
+void f()
+{
+  A a0 = 0; // expected-error {{no viable conversion}}
+  A a1( 0);
+  A && a2 = 0;// expected-error {{could not bind}}
+  A && a3( 0);// expected-error {{could not bind}}
+  A a4{ 0};
+  A && a5 = { 0};// expected-error {{chosen constructor is explicit}}
+  A && a6{ 0};
+  A a7 = { 0}; // expected-error {{chosen constructor is explicit in copy-initialization}}
+
+  a0 = 0;
+  a1 = { 0}; // expected-error {{no viable overloaded '='}}
+  a2 = A( 0);
+  a3 = A{ 0};
+
+  A c0 =  ((short)0);
+  A c1( ((short)0));
+  A && c2 =  ((short)0);
+  A && c3( ((short)0));
+  A c4{ ((short)0)};
+  A && c5 = { ((short)0)};
+  A && c6{ ((short)0)};
+
+  A d1( 0, 0);
+  A d2{ 0, 0};
+  A d3 = { 0, 0}; // expected-error {{chosen constructor is explicit in copy-initialization}}
+
+  d1 = { 0, 0}; // expected-error {{no viable overloaded '='}}
+  d2 = A( 0, 0);
+  d3 = A{ 0, 0};
+}
+}
+
+namespace test2 {
+
+template
+struct A {
+// expected-note@-1 {{candidate constructor}} expected-note@-1 {{candidate constructor}}
+// expected-note@-2 {{candidate constructor}} expected-note@-2 {{candidate constructor}}
+  template
+  explicit(a ^ is_same::value)
+// expected-note@-1 {{explicit(bool) specifier resolved to true}} expected-note@-1 {{explicit(bool) specifier resolved to true}}
+  

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 195990.
Tyker added a comment.

@Quuxplusone @riccibruno  fixed / answered feedback


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

https://reviews.llvm.org/D60934

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTReader.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaLookup.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/PCH/cxx-explicit-spec.cpp
  clang/test/SemaCXX/cxx2a-explicit-bool.cpp
  clang/test/SemaCXX/explicit.cpp

Index: clang/test/SemaCXX/explicit.cpp
===
--- clang/test/SemaCXX/explicit.cpp
+++ clang/test/SemaCXX/explicit.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
+
 namespace Constructor {
 struct A {
   A(int);
@@ -183,7 +185,8 @@
 const int ©List7 = {b};
 const int ©List8 = {n}; // expected-error {{no viable conversion}}
   }
-  
+
+#if __cplusplus < 201707L
   void testNew()
   {
 // 5.3.4p6:
@@ -200,7 +203,8 @@
 new int[i];
 new int[ni]; // expected-error {{array size expression of type 'NotInt' requires explicit conversion to type 'int'}}
   }
-  
+#endif
+
   void testDelete()
   {
 // 5.3.5pp2:
Index: clang/test/SemaCXX/cxx2a-explicit-bool.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-explicit-bool.cpp
@@ -0,0 +1,335 @@
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s -verify
+
+template
+struct enable_ifv {};
+
+template
+struct enable_ifv {
+  static constexpr auto value = val;
+};
+
+template
+struct is_same {
+static constexpr bool value = false;
+};
+
+template
+struct is_same {
+static constexpr bool value = true;
+};
+
+namespace test0
+{
+
+template
+struct A {
+  explicit(1 << a)
+  //expected-error@-1 {{argument to explicit specifier is not a valid constant expression}}
+  //expected-note@-2 {{negative shift count -1}}
+  A(int);
+};
+
+A<-1> a(0); //expected-note {{in instantiation of template class}}
+
+template
+struct B {
+  explicit(b)
+  // expected-error@-1 {{use of undeclared identifier}}
+  // expected-note@-2 {{this expression is parsed as explicit(bool) since c++2a}}
+  B(int);
+};
+
+}
+
+namespace test1 {
+
+template
+struct A {
+  // expected-note@-1 {{candidate constructor}}
+  // expected-note@-2 {{candidate constructor}}
+  // expected-note@-3 {{candidate function}} expected-note@-3 {{candidate function}}
+  // expected-note@-4 {{candidate function}} expected-note@-4 {{candidate function}}
+  explicit(b) A(int, int = 0);
+  // expected-note@-1 {{explicit constructor declared here}}
+  // expected-note@-2 {{explicit constructor declared here}}
+  // expected-note@-3 {{explicit constructor declared here}}
+};
+
+template
+A::A(int, int) {}
+
+void f()
+{
+  A a0 = 0; // expected-error {{no viable conversion}}
+  A a1( 0);
+  A && a2 = 0;// expected-error {{could not bind}}
+  A && a3( 0);// expected-error {{could not bind}}
+  A a4{ 0};
+  A && a5 = { 0};// expected-error {{chosen constructor is explicit}}
+  A && a6{ 0};
+  A a7 = { 0}; // expected-error {{chosen constructor is explicit in copy-initialization}}
+
+  a0 = 0;
+  a1 = { 0}; // expected-error {{no viable overloaded '='}}
+  a2 = A( 0);
+  a3 = A{ 0};
+
+  A c0 =  ((short)0);
+  A c1( ((short)0));
+  A && c2 =  ((short)0);
+  A && c3( ((short)0));
+  A c4{ ((short)0)};
+  A && c5 = { ((short)0)};
+  A && c6{ ((short)0)};
+
+  A d1( 0, 0);
+  A d2{ 0, 0};
+  A d3 = { 0, 0}; // expected-error {{chosen constructor is explicit in copy-initialization}}
+
+  d1 = { 0, 0}; // expected-error {{no viable overloaded '='}}
+  d2 = A( 0, 0);
+  d3 = A{ 0, 0};
+}
+}
+
+namespace test2 {
+
+template
+struct A {
+// expected-note@-1 {{candidate constructor}} expected-note@-1 {{candidate constructor}}
+// expected-note@-2 {{candidate constructor}} expected-note@-2 {{candidate constructor}}
+  template
+  explicit(a ^ is_same::value)
+// expected-note@-1 {{explicit(bool) specifier resolved

[PATCH] D60523: [clang] Don't segfault on incorrect using directive (PR41400)

[Gauthier via Phabricator via cfe-commits]

Tyker added a subscriber: jkooker.
Tyker added a comment.

@jkooker
i don't think it is possible for `ASTContext::getDependentNameType` to deal 
with `NSS = nullptr` except by reporting the error. we probably don't want to 
just report the error because the error could have been handled before invoking 
`ASTContext::getDependentNameType`.
and i agree that it should be reflected in the interface of those functions 
because pointer are often used a optional references. but wouldn't an assert + 
doxygen comment suffice ? instead of changing the interface, implementation and 
all callsites. Pointers as argument that are required not to be null are used 
in a lot of places in LLVM. for example `DependentNameType::DependentNameType` 
takes NNS as a pointer even thought it requires it to be non-null(calls a 
method on it). mixing pointers and references adds a lot of `&` and `*` which 
makes the code harder to read.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60523



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

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

[Gauthier via Phabricator via cfe-commits]

Tyker marked 2 inline comments as done.
Tyker added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2033
+
+  void setExplicitSpecifier(ExplicitSpecInfo ESI);
+

rsmith wrote:
> Generally we don't want to have setters in the AST; the AST is intended to be 
> immutable after creation. Is this necessary?
this is used in 2 cases:
- for deserialization.
- for in `Sema::ActOnFunctionDeclarator` to make every declaration of the same 
function have the same explicit specifier as the canonical one. there is 
probably a better way to do this but i didn't find how.

the second use case will need to be changed because the constructor's explicit 
specifier will be tail-allocated so the explicit specifier from the canonical 
declaration will need to be recovered before construction to allocate storage. 

how can we find the canonical declaration of a function before it is 
constructed ?



Comment at: clang/lib/Sema/SemaInit.cpp:3819
+AllowExplicit = AllowExplicit && !CopyInitializing;
+if (AllowExplicit || Conv->isMaybeNotExplicit()) {
   if (ConvTemplate)

rsmith wrote:
> Consider just removing this `if`. It's not clear that it carries its weight.
this if prevent conversion that are already known not to be valid from being 
added to the overload set. removing it is still correct because it will be 
removed later. but we would be doing "useless" work because we are adding the 
to the overload set knowing they will be removed.
the only benefit i see of removing this if is to make all explicit conversion 
appear in overload resolution failure messages. is it the intent ?


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

https://reviews.llvm.org/D60934



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