[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseDecl.cpp:6587
+return;
+  }
+

rjmccall wrote:
> Comment indentation.
> 
> Should we do this when starting to parse a function prototype instead of when 
> parsing a parameter?
Thanks I noticed I forgot to change the tabs to spaces.

I looked at your suggestion to move the code, but I think this is the proper 
place. Now it also validates whether lambdas exceed the limit, else we need to 
check at two places.
I'll also add a unit test to test for the lambda.


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

https://reviews.llvm.org/D63975



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

[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 209348.
Mordante added a comment.

tab -> space
adds an extra unit test for lambdas
fixes an off by one error found while testing the lambdas


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

https://reviews.llvm.org/D63975

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/test/Parser/nested_function_prototype_overflow.cpp
  clang/test/Parser/nested_lambda_overflow.cpp


Index: clang/test/Parser/nested_lambda_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_lambda_overflow.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+auto foo = [](void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(
+#ifdef FAIL
+void (*f)()
+#endif
+)
 {};
+// CHECK: fatal error: function scope depth exceeded maximum of 127
Index: clang/test/Parser/nested_function_prototype_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_function_prototype_overflow.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+void foo(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(
+#ifdef FAIL
+void (*f)()
+#endif
+);
+// CHECK: fatal error: function scope depth exceeded maximum of 127
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6576,6 +6576,18 @@
   Actions.containsUnexpandedParameterPacks(ParmDeclarator))
 DiagnoseMisplacedEllipsisInDeclarator(ConsumeToken(), ParmDeclarator);
 
+  // Avoid exceeding the maximum function scope depth.
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  // Note Sema::ActOnParamDeclarator calls ParmVarDecl::setScopeInfo with
+  // getFunctionPrototypeDepth() - 1.
+  if (getCurScope()->getFunctionPrototypeDepth() - 1 >
+  ParmVarDecl::getMaxFunctionScopeDepth()) {
+Diag(DSStart, diag::err_function_scope_depth_e

[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseDecl.cpp:6587
+return;
+  }
+

rjmccall wrote:
> Mordante wrote:
> > rjmccall wrote:
> > > Comment indentation.
> > > 
> > > Should we do this when starting to parse a function prototype instead of 
> > > when parsing a parameter?
> > Thanks I noticed I forgot to change the tabs to spaces.
> > 
> > I looked at your suggestion to move the code, but I think this is the 
> > proper place. Now it also validates whether lambdas exceed the limit, else 
> > we need to check at two places.
> > I'll also add a unit test to test for the lambda.
> I don't understand.  I'm just saying to put the check at the top of the 
> function instead of inside the loop.
I now understand what you mean. I'll upload a new patch.


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

https://reviews.llvm.org/D63975



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

[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 209513.
Mordante added a comment.

Moved the test out of the loop as suggested by rjmccall.


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

https://reviews.llvm.org/D63975

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/test/Parser/nested_function_prototype_overflow.cpp
  clang/test/Parser/nested_lambda_overflow.cpp


Index: clang/test/Parser/nested_lambda_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_lambda_overflow.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+auto foo = [](void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(
+#ifdef FAIL
+void (*f)()
+#endif
+)
 {};
+// CHECK: fatal error: function scope depth exceeded maximum of 127
Index: clang/test/Parser/nested_function_prototype_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_function_prototype_overflow.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+void foo(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(
+#ifdef FAIL
+void (*f)()
+#endif
+);
+// CHECK: fatal error: function scope depth exceeded maximum of 127
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6510,6 +6510,19 @@
ParsedAttributes &FirstArgAttrs,
SmallVectorImpl &ParamInfo,
SourceLocation &EllipsisLoc) {
+
+  // Avoid exceeding the maximum function scope depth.
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  // Note Sema::ActOnParamDeclarator calls ParmVarDecl::setScopeInfo with
+  // getFunctionPrototypeDepth() - 1.
+  if (getCurScope()->getFunctionPrototypeDepth() - 1 >
+  ParmVarDecl::getMaxFunctionScopeDepth()) {
+Diag(Tok.getLocation(), diag::err_function_scope_depth_exceeded)
+<< ParmVarDecl::getMaxFunctionScopeDepth();
+cutOffParsing();
+return;
+  }

[PATCH] D64644: Fixes a clang frontend assertion failure (bug 35682)

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

This fixes bug 35682.  I was not able to easily generate a test-case so I 
haven't been able to add unit tests. I'll update the bug in bugzilla with some 
additional information.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64644

Files:
  clang/lib/Sema/SemaTemplate.cpp


Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,15 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *TemplateArgs) {
-  return DependentScopeDeclRefExpr::Create(
-  Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
-  TemplateArgs);
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+return ExprError();
+  else
+return DependentScopeDeclRefExpr::Create(Context, std::move(QualifierLoc),
+ TemplateKWLoc, NameInfo,
+ TemplateArgs);
 }
 
 


Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,15 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *TemplateArgs) {
-  return DependentScopeDeclRefExpr::Create(
-  Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
-  TemplateArgs);
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+return ExprError();
+  else
+return DependentScopeDeclRefExpr::Create(Context, std::move(QualifierLoc),
+ TemplateKWLoc, NameInfo,
+ TemplateArgs);
 }
 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64644: Fixes a clang frontend assertion failure (bug 35682)

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

Thanks for feedback. I'll whether I can find a way to generate a nice test case.




Comment at: clang/lib/Sema/SemaTemplate.cpp:726
+return ExprError();
+  else
+return DependentScopeDeclRefExpr::Create(Context, std::move(QualifierLoc),

lebedev.ri wrote:
> no else after return
Will do in the next update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64644



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: gribozavr, rsmith.
Mordante added a project: clang.

It warns for for comments like
/** \pre \em */

where \em has no argument

This warning is enabled with the -Wdocumentation option.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64696

Files:
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1027,6 +1027,49 @@
 template
 void test_attach37::test_attach39(int aaa, int bbb) {}
 
+// The inline comments expect a string after the command.
+// expected-warning@+1 {{'\a' command does not have an argument}}
+/// \a
+int test_inline_no_argument_a_b(int);
+
+/// \a A
+int test_inline_no_argument_a_g(int);
+
+// expected-warning@+1 {{'@b' command does not have an argument}}
+/// @b
+int test_inline_no_argument_b_b(int);
+
+/// @b A
+int test_inline_no_argument_b_g(int);
+
+// expected-warning@+1 {{'\c' command does not have an argument}}
+/// \c
+int test_inline_no_argument_c_b(int);
+
+/// \c A
+int test_inline_no_argument_c_g(int);
+
+// expected-warning@+1 {{'\e' command does not have an argument}}
+/// \e
+int test_inline_no_argument_e_b(int);
+
+/// \e A
+int test_inline_no_argument_e_g(int);
+
+// expected-warning@+1 {{'\em' command does not have an argument}}
+/// \em
+int test_inline_no_argument_em_b(int);
+
+/// \em A
+int test_inline_no_argument_em_g(int);
+
+// expected-warning@+1 {{'\p' command does not have an argument}}
+/// \p
+int test_inline_no_argument_p_b(int);
+
+/// \p A
+int test_inline_no_argument_p_g(int);
+
 // We used to emit warning that parameter 'a' is not found because we parsed
 // the comment in context of the redeclaration which does not have parameter
 // names.
Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -422,6 +422,12 @@
 IC = S.actOnInlineCommand(CommandTok.getLocation(),
   CommandTok.getEndLocation(),
   CommandTok.getCommandID());
+
+Diag(CommandTok.getEndLocation().getLocWithOffset(1),
+ diag::warn_doc_inline_contents_no_argument)
+<< CommandTok.is(tok::at_command)
+<< Traits.getCommandInfo(CommandTok.getCommandID())->Name
+<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
   }
 
   Retokenizer.putBackLeftoverTokens();
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -153,6 +153,12 @@
 def note_add_deprecation_attr : Note<
   "add a deprecation attribute to the declaration to silence this warning">;
 
+// inline contents commands
+
+def warn_doc_inline_contents_no_argument : Warning<
+  "'%select{\\|@}0%1' command does not have an argument">,
+  InGroup, DefaultIgnore;
+
 // verbatim block commands
 
 def warn_verbatim_block_end_without_start : Warning<


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1027,6 +1027,49 @@
 template
 void test_attach37::test_attach39(int aaa, int bbb) {}
 
+// The inline comments expect a string after the command.
+// expected-warning@+1 {{'\a' command does not have an argument}}
+/// \a
+int test_inline_no_argument_a_b(int);
+
+/// \a A
+int test_inline_no_argument_a_g(int);
+
+// expected-warning@+1 {{'@b' command does not have an argument}}
+/// @b
+int test_inline_no_argument_b_b(int);
+
+/// @b A
+int test_inline_no_argument_b_g(int);
+
+// expected-warning@+1 {{'\c' command does not have an argument}}
+/// \c
+int test_inline_no_argument_c_b(int);
+
+/// \c A
+int test_inline_no_argument_c_g(int);
+
+// expected-warning@+1 {{'\e' command does not have an argument}}
+/// \e
+int test_inline_no_argument_e_b(int);
+
+/// \e A
+int test_inline_no_argument_e_g(int);
+
+// expected-warning@+1 {{'\em' command does not have an argument}}
+/// \em
+int test_inline_no_argument_em_b(int);
+
+/// \em A
+int test_inline_no_argument_em_g(int);
+
+// expected-warning@+1 {{'\p' command does not have an argument}}
+/// \p
+int test_inline_no_argument_p_b(int);
+
+/// \p A
+int test_inline_no_argument_p_g(int);
+
 // We used to emit warning that parameter 'a' is not found because we parsed
 // the comment in context of the redeclaration which does not have parameter
 // names.
Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/Comme

[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 209703.
Mordante added a comment.

Addresses @rjmccall's remarks.
Fixes the tests for the nested lambda's.
As suspected the blocks also have the same nesting limit, thus added a test for 
them.


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

https://reviews.llvm.org/D63975

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/test/Parser/nested_blocks_overflow.cpp
  clang/test/Parser/nested_function_prototype_overflow.cpp
  clang/test/Parser/nested_lambda_overflow.cpp

Index: clang/test/Parser/nested_lambda_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_lambda_overflow.cpp
@@ -0,0 +1,55 @@
+// RUN: %clang %s -fsyntax-only -fbracket-depth=512
+// RUN: not %clang %s -fsyntax-only -fbracket-depth=512 -DFAIL 2>&1 | FileCheck %s
+
+template  int foo(T &&t);
+
+void bar(int x = foo(
+
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+
+[](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo([](int x = foo(
+
+#ifdef FAIL
+[](int x = foo(
+#endif
+
+[](int x = foo(1)){}
+
+#ifdef FAIL
+)){}
+#endif
+
+)){})){})){})){})){})){}
+
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+)){})){})){})){})){})){})){})){}
+));
+
+// CHECK: fatal error: function scope depth exceeded maximum of 127
Index: clang/test/Parser/nested_function_prototype_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_function_prototype_overflow.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+void foo(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(v

[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for the review. I don't have SVN access, can you commit these changes?


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

https://reviews.llvm.org/D63975



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

[PATCH] D64644: Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 210128.
Mordante marked an inline comment as done.
Mordante retitled this revision from "Fixes a clang frontend assertion failure 
(bug 35682)" to "Fixes an assertion failure while instantiation a template with 
an incomplete typo corrected type".
Mordante edited the summary of this revision.
Mordante added a comment.

Fixes @lebedev.ri's remark and adds the requested test.
While working on the test I discovered the initial assumption in bug 35682 was 
incorrect. Updated the title and summary accordingly.


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

https://reviews.llvm.org/D64644

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp


Index: 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,47 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | 
FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+#include 
+#include 
+
+template 
+class allocator;
+
+template 
+struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template 
+struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template 
+static constexpr bool enable_implicit() {
+  return std::is_constructible::value;
+}
+  };
+
+  template (),
+bool>::type = false>
+  single(U1 &&u1) : first(std::forward(u1)) {}
+};
+
+using SetKeyType = String;
+single v;
+
+// CHECK: error: unknown type name 'String'; did you mean 'string'?
+// CHECK: fatal error: too many errors emitted, stopping now [-ferror-limit=]
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,14 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *TemplateArgs) {
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+return ExprError();
+
   return DependentScopeDeclRefExpr::Create(
-  Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
-  TemplateArgs);
+  Context, std::move(QualifierLoc), TemplateKWLoc, NameInfo, TemplateArgs);
 }
 
 


Index: clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,47 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+#include 
+#include 
+
+template 
+class allocator;
+
+template 
+struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template 
+struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template 
+static constexpr bool enable_implicit() {
+  return std::is_constructible::value;
+}
+  };
+
+  template (),
+bool>::type = false>
+  single(U1 &&u1) : first(std::forward(u1)) {}
+};
+
+using SetKeyType = String;
+single v;
+
+// CHECK: error: unknown type name 'String'; did you mean 'string'?
+// CHECK: fatal error: too many errors emitted, stopping now [-ferror-limit=]
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,14 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 

[PATCH] D64811: Warn when NumParams overflows

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rjmccall.
Mordante added a project: clang.

Before when the overflow occurred an assertion was triggered. Now check whether 
the maximum has been reached and warn properly.

This patch fixes bug 33162 which is marked as 'duplicate' of bug 19607. The 
original part of bug 19607 is fixed by D63975 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64811

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/Parser/function_parameter_overflow.cpp
  clang/test/Parser/lambda_function_parameter_overflow.cpp
  clang/test/Parser/parameter_overflow.h

Index: clang/test/Parser/parameter_overflow.h
===
--- /dev/null
+++ clang/test/Parser/parameter_overflow.h
@@ -0,0 +1,9 @@
+#pragma once
+
+#define I10 int, int, int, int, int, int, int, int, int, int
+#define I50 I10, I10, I10, I10, I10
+#define I500 I50, I50, I50, I50, I50, I50, I50, I50, I50, I50
+#define I5000 I500, I500, I500, I500, I500, I500, I500, I500, I500, I500
+#define I6 I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000, I5000
+
+#define  I65535 I6, I5000, I500, I10, I10, I10, int, int, int, int, int
Index: clang/test/Parser/lambda_function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/lambda_function_parameter_overflow.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#include "parameter_overflow.h"
+
+auto foo = [](I65535
+#ifdef FAIL
+, int
+#endif
+){};
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/test/Parser/function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/function_parameter_overflow.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#include "parameter_overflow.h"
+
+void foo(I65535
+#ifdef FAIL
+, int
+#endif
+);
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -1265,6 +1265,14 @@
 
   ParseParameterDeclarationClause(D, Attr, ParamInfo, EllipsisLoc);
 
+  // If the parameters could not be parsed stop processing. Proceeding is
+  // not an issue when the maximum scope depth is exceeded. But when the
+  // maximum number of parameters is exceeded the processing will still hit
+  // the assertion in FunctionProtoType's constructor.
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (Tok.is(tok::eof))
+return ExprError();
+
   // For a generic lambda, each 'auto' within the parameter declaration
   // clause creates a template type parameter, so increment the depth.
   // If we've parsed any explicit template parameters, then the depth will
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6668,6 +6668,15 @@
 
 // If the next token is a comma, consume it and keep reading arguments.
   } while (TryConsumeToken(tok::comma));
+
+  // Avoid exceeding the maximum function parameters
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (ParamInfo.size() > Type::getMaxNumParams()) {
+Diag(ParamInfo[Type::getMaxNumParams() - 1].IdentLoc,
+ diag::err_number_of_function_parameters_exceeded)
+<< Type::getMaxNumParams();
+cutOffParsing();
+  }
 }
 
 /// [C90]   direct-declarator '[' constant-expression[opt] ']'
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -326,6 +326,8 @@
 def err_argument_required_after_attribute : Error<
   "argument required after attribute">;
 def err_missing_param : Error<"expected parameter declarator">;
+def err_number_of_function_parameters_exceeded : Error<
+  "number of function parameters exceeded maximum of %0">, DefaultFatal;
 def err_missing_comma_before_ellipsis : Error<
   "C requires a comma prior to the ellipsis in a variadic function type">;
 def err_unexpected_typedef_ident : Error<
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -1520,6 +1520,8 @@
 unsigned Kind : 8;
   };
 
+  enum { Num

[PATCH] D64644: Fixes an assertion failure while instantiation a template with an incomplete typo corrected type

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 210146.
Mordante added a comment.

Remove the includes from the test.
Changed the `std::is_constructible` to `is_same` since the latter is easier to 
mock.


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

https://reviews.llvm.org/D64644

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp


Index: 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ 
clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,60 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | 
FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+template  class allocator;
+
+template  struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template  struct enable_if {};
+template  struct enable_if { typedef Tp type; };
+
+template  struct integral_constant {
+  static constexpr const Tp value = v;
+  typedef Tp value_type;
+  typedef integral_constant type;
+
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
+};
+
+template  constexpr const Tp integral_constant::value;
+
+using true_type = integral_constant;
+using false_type = integral_constant;
+
+template  struct is_same : public false_type {};
+template  struct is_same : public true_type {};
+
+template  struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template  static constexpr bool enable_implicit() {
+  return is_same::value;
+}
+  };
+
+  template (),
+   bool>::type = false>
+  single(U1 &&u1);
+};
+
+using SetKeyType = String;
+single v;
+
+// CHECK: error: unknown type name 'String'; did you mean 'string'?
+// CHECK: fatal error: too many errors emitted, stopping now [-ferror-limit=]
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -718,9 +718,14 @@
 SourceLocation TemplateKWLoc,
 const DeclarationNameInfo &NameInfo,
 const TemplateArgumentListInfo *TemplateArgs) {
+  // DependentScopeDeclRefExpr::Create requires a valid QualifierLoc
+  // See https://bugs.llvm.org/show_bug.cgi?id=35682
+  NestedNameSpecifierLoc QualifierLoc = SS.getWithLocInContext(Context);
+  if (!QualifierLoc)
+return ExprError();
+
   return DependentScopeDeclRefExpr::Create(
-  Context, SS.getWithLocInContext(Context), TemplateKWLoc, NameInfo,
-  TemplateArgs);
+  Context, std::move(QualifierLoc), TemplateKWLoc, NameInfo, TemplateArgs);
 }
 
 


Index: clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/instantiate-incomplete-typo-suggested-error-limit.cpp
@@ -0,0 +1,60 @@
+// RUN: not %clang -fsyntax-only -std=c++11 -ferror-limit=1 %s 2>&1 | FileCheck %s
+
+// Test case to test the error in https://bugs.llvm.org/show_bug.cgi?id=35682.
+// The issue be caused by the typo correction that changes String to the
+// incomplete type string. The example is based on the std::pair code and
+// reduced to a minimal test case. When using std::pair the issue can only be
+// reproduced when using the -stdlib=libc++ compiler option.
+
+template  class allocator;
+
+template  struct char_traits;
+
+template ,
+  class Allocator = allocator>
+class basic_string;
+typedef basic_string, allocator> string;
+
+template  struct enable_if {};
+template  struct enable_if { typedef Tp type; };
+
+template  struct integral_constant {
+  static constexpr const Tp value = v;
+  typedef Tp value_type;
+  typedef integral_constant type;
+
+  constexpr operator value_type() const noexcept { return value; }
+  constexpr value_type operator()() const noexcept { return value; }
+};
+
+template  constexpr const Tp integral_constant::value;
+
+using true_type = integral_constant;
+using false_type = integral_constant;
+
+template  struct is_same : public false_type {};
+template  struct is_same : public true_type {};
+
+template  struct single {
+  typedef T first_type;
+
+  T first;
+
+  struct CheckArgs {
+template  static constexpr bool enable_implicit() {
+  

[PATCH] D64820: Avoids an assertion failure when an invalid conversion declaration is used

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

When using a user-defined conversion function template with a deduced return 
type the compiler gives a set of warnings:

  bug.cc:252:44: error: cannot specify any part of a return type in the 
declaration of a conversion function; use an alias template to declare a 
conversion to 'auto (Ts &&...) const'
template  operator auto()(Ts &&... xs) const;
 ^~~
  bug.cc:252:29: error: conversion function cannot convert to a function type
template  operator auto()(Ts &&... xs) const;
  ^
  error: pointer to function type cannot have 'const' qualifier

after which it triggers an assertion failure. It seems the last error is 
incorrect and doesn't have any location information. This patch stops the 
compilation after the second warning.

Fixes bug 31422.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64820

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/conversion_function_to_function.cpp


Index: clang/test/Sema/conversion_function_to_function.cpp
===
--- /dev/null
+++ clang/test/Sema/conversion_function_to_function.cpp
@@ -0,0 +1,9 @@
+// RUN: not %clang_cc1 -fsyntax-only -std=c++14 %s 2>&1 | FileCheck %s
+
+struct S {
+  template  operator auto()(Ts &&... xs) const;
+};
+
+// CHECK: error: cannot specify any part of a return type in the declaration 
of a conversion function; use an alias template to declare a conversion to 
'auto (Ts &&...) const'
+// CHECK: error: conversion function cannot convert to a function type
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8066,6 +8066,9 @@
 }
 
 SemaRef.CheckConversionDeclarator(D, R, SC);
+if (D.isInvalidType())
+  return nullptr;
+
 IsVirtualOkay = true;
 return CXXConversionDecl::Create(
 SemaRef.Context, cast(DC), D.getBeginLoc(), NameInfo, R,


Index: clang/test/Sema/conversion_function_to_function.cpp
===
--- /dev/null
+++ clang/test/Sema/conversion_function_to_function.cpp
@@ -0,0 +1,9 @@
+// RUN: not %clang_cc1 -fsyntax-only -std=c++14 %s 2>&1 | FileCheck %s
+
+struct S {
+  template  operator auto()(Ts &&... xs) const;
+};
+
+// CHECK: error: cannot specify any part of a return type in the declaration of a conversion function; use an alias template to declare a conversion to 'auto (Ts &&...) const'
+// CHECK: error: conversion function cannot convert to a function type
+// CHECK-NOT: Assertion{{.*}}failed
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -8066,6 +8066,9 @@
 }
 
 SemaRef.CheckConversionDeclarator(D, R, SC);
+if (D.isInvalidType())
+  return nullptr;
+
 IsVirtualOkay = true;
 return CXXConversionDecl::Create(
 SemaRef.Context, cast(DC), D.getBeginLoc(), NameInfo, R,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64874: Improve handling of function pointer conversions

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

Starting with C++17 the `noexcept` is part of the function signature but a 
`noexcept` function can be converted to a `noexcept(false)` function. The 
overload resolution code handles it properly but expression builder doesn't. It 
causes the following code to trigger and assertion failure:

  struct S {
  int f(void) noexcept { return 110; }
  } s;
  
  template  int f10(void) { return (s.*a)(); }
  
  int foo(void)
  {
return f10<&S::f >();
  }

The fix adds an extra implicit cast when needed. I picked the `CK_NoOp` as 
`CastKind` since it seems to be the best fit. However I wonder whether it would 
be better to add a new value `CK_FunctionPointerConversion`.

This fixes bug 40024.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64874

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
  clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp


Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024
+// Tests the still valid function pointer conversions
+
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<&S::f>();
+}
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only %s
+// RUN: not %clang_cc1 -std=c++17 -fsyntax-only %s 2>&1 | FileCheck %s
+// RUN: not %clang_cc1 -std=c++2a -fsyntax-only %s 2>&1 | FileCheck %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024
+// Tests the no longer valid function pointer conversions
+
+struct S {
+  int f(void) { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<&S::f>();
+}
+
+// CHECK: error: no matching function for call to 'f10'
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,14 @@
 ObjCLifetimeConversion))
 RefExpr = ImpCastExprToType(RefExpr.get(), 
ParamType.getUnqualifiedType(), CK_NoOp);
 
+  // Starting with C++17 the noexcept is part of the function signature but
+  // a noexcept function can be converted to a noexcept(false) function.
+  QualType resultTy;
+  if (getLangOpts().CPlusPlus17 &&
+  IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
+   ParamType.getUnqualifiedType(), resultTy))
+RefExpr = ImpCastExprToType(RefExpr.get(), resultTy, CK_NoOp);
+
   assert(!RefExpr.isInvalid() &&
  Context.hasSameType(((Expr*) RefExpr.get())->getType(),
  ParamType.getUnqualifiedType()));


Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-valid.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for https://bugs.llvm.org/show_bug.cgi?id=40024
+// Tests the still valid function pointer conversions
+
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<&S::f>();
+}
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept-invalid.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -st

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

All inline commands defined in include/clang/AST/CommentCommands.td require an 
argument. The escape commands, like \&, are handled in the switch starting at 
lib/AST/CommentLexer.cpp:366. They are stored as Text and not as an 
InlineCommand.

If you want I can add an extra field in the Command class in 
include/clang/AST/CommentCommands.td. Something like `bit 
IsEmptyInlineCommandAllowed = 0;`.


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

https://reviews.llvm.org/D64696



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 211340.
Mordante added a comment.

Addresses @gribozavr comments.


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

https://reviews.llvm.org/D64696

Files:
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentParser.cpp
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1044,6 +1044,48 @@
 template 
 void test_attach38::test_attach39(int, B);
 
+// The inline comments expect a string after the command.
+// expected-warning@+1 {{'\a' command does not have an argument}}
+/// \a
+int test_inline_no_argument_a_bad(int);
+
+/// \a A
+int test_inline_no_argument_a_good(int);
+
+// expected-warning@+1 {{'@b' command does not have an argument}}
+/// @b
+int test_inline_no_argument_b_bad(int);
+
+/// @b A
+int test_inline_no_argument_b_good(int);
+
+// expected-warning@+1 {{'\c' command does not have an argument}}
+/// \c
+int test_inline_no_argument_c_bad(int);
+
+/// \c A
+int test_inline_no_argument_c_good(int);
+
+// expected-warning@+1 {{'\e' command does not have an argument}}
+/// \e
+int test_inline_no_argument_e_bad(int);
+
+/// \e A
+int test_inline_no_argument_e_good(int);
+
+// expected-warning@+1 {{'\em' command does not have an argument}}
+/// \em
+int test_inline_no_argument_em_bad(int);
+
+/// \em A
+int test_inline_no_argument_em_good(int);
+
+// expected-warning@+1 {{'\p' command does not have an argument}}
+/// \p
+int test_inline_no_argument_p_bad(int);
+
+/// \p A
+int test_inline_no_argument_p_good(int);
 
 // PR13411, reduced.  We used to crash on this.
 /**
Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -422,6 +422,12 @@
 IC = S.actOnInlineCommand(CommandTok.getLocation(),
   CommandTok.getEndLocation(),
   CommandTok.getCommandID());
+
+Diag(CommandTok.getEndLocation().getLocWithOffset(1),
+ diag::warn_doc_inline_contents_no_argument)
+<< CommandTok.is(tok::at_command)
+<< Traits.getCommandInfo(CommandTok.getCommandID())->Name
+<< SourceRange(CommandTok.getLocation(), CommandTok.getEndLocation());
   }
 
   Retokenizer.putBackLeftoverTokens();
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -153,6 +153,12 @@
 def note_add_deprecation_attr : Note<
   "add a deprecation attribute to the declaration to silence this warning">;
 
+// inline contents commands
+
+def warn_doc_inline_contents_no_argument : Warning<
+  "'%select{\\|@}0%1' command does not have an argument">,
+  InGroup, DefaultIgnore;
+
 // verbatim block commands
 
 def warn_verbatim_block_end_without_start : Warning<


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1044,6 +1044,48 @@
 template 
 void test_attach38::test_attach39(int, B);
 
+// The inline comments expect a string after the command.
+// expected-warning@+1 {{'\a' command does not have an argument}}
+/// \a
+int test_inline_no_argument_a_bad(int);
+
+/// \a A
+int test_inline_no_argument_a_good(int);
+
+// expected-warning@+1 {{'@b' command does not have an argument}}
+/// @b
+int test_inline_no_argument_b_bad(int);
+
+/// @b A
+int test_inline_no_argument_b_good(int);
+
+// expected-warning@+1 {{'\c' command does not have an argument}}
+/// \c
+int test_inline_no_argument_c_bad(int);
+
+/// \c A
+int test_inline_no_argument_c_good(int);
+
+// expected-warning@+1 {{'\e' command does not have an argument}}
+/// \e
+int test_inline_no_argument_e_bad(int);
+
+/// \e A
+int test_inline_no_argument_e_good(int);
+
+// expected-warning@+1 {{'\em' command does not have an argument}}
+/// \em
+int test_inline_no_argument_em_bad(int);
+
+/// \em A
+int test_inline_no_argument_em_good(int);
+
+// expected-warning@+1 {{'\p' command does not have an argument}}
+/// \p
+int test_inline_no_argument_p_bad(int);
+
+/// \p A
+int test_inline_no_argument_p_good(int);
 
 // PR13411, reduced.  We used to crash on this.
 /**
Index: clang/lib/AST/CommentParser.cpp
===
--- clang/lib/AST/CommentParser.cpp
+++ clang/lib/AST/CommentParser.cpp
@@ -422,6 +422,12 @@
 IC = S.actOnInlineCommand(CommandTok.getLocation(),
   CommandTok.getEndLocation(),
   CommandTok.getCommandID());
+
+Diag(CommandTok.getEndLocation().getLocWithOff

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for the review.

I don't have commit access. So yes please commit the patch for me.


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

https://reviews.llvm.org/D64696



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

[PATCH] D64811: Warn when NumParams overflows

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseDecl.cpp:6673
+  // Avoid exceeding the maximum function parameters
+  // See https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (ParamInfo.size() > Type::getMaxNumParams()) {

rjmccall wrote:
> rjmccall wrote:
> > We don't usually cite bugs like this in the main source code except as 
> > "there's a bug with this, here's where we tracking fixing it".  The comment 
> > explains the purpose of the diagnostic well enough (but please include a 
> > period).
> > 
> > Can you just drop the excess arguments here instead of cutting off parsing?
> Or actually — let's move this out of the parser entirely, because (unlike 
> prototype scope depth) we actually need to diagnose it in Sema as well 
> because of variadic templates.  `Sema::BuildFunctionType` seems like the 
> right place.
> 
> Also, can you `static_assert` that function declarations support at least as 
> many parameter declarations as we allow in types?
I tried your suggestion to move the test to `Sema::BuildFunctionType` but that 
function is only used for templates so it doesn't show the warning for other 
cases. Also the error generated will also print the 
`note_ovl_candidate_substitution_failure` diagnostic making the output rather 
unreadable. So I'll look for a better way to handle the variadic template case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64811



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

[PATCH] D65694: Properly instantiate a decltype in argument's default initializer

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: sepavloff, rsmith.
Mordante added a project: clang.

This fixes https://bugs.llvm.org/show_bug.cgi?id=28500


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65694

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaTemplate/default-arguments-cxx0x.cpp

Index: clang/test/SemaTemplate/default-arguments-cxx0x.cpp
===
--- clang/test/SemaTemplate/default-arguments-cxx0x.cpp
+++ clang/test/SemaTemplate/default-arguments-cxx0x.cpp
@@ -114,3 +114,34 @@
 S _a{};
   };
 }
+
+// https://bugs.llvm.org/show_bug.cgi?id=28500
+// Failure to resolve the decltype part during instantiation caused an
+// assertion failure
+namespace PR28500 {
+namespace original {
+template 
+void bar(T t = [](decltype(t) i) { return 0; }(0)) {}
+void foo() {
+  bar();
+}
+} // namespace original
+
+namespace cast {
+template 
+void bar(T t = decltype(t)(0)) {}
+void foo() {
+  bar();
+  bar();
+}
+} // namespace cast
+
+namespace value {
+template 
+void bar(T t = decltype(t)()) {}
+void foo() {
+  bar();
+  bar();
+}
+} // namespace value
+} // namespace PR28500
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -5287,29 +5287,33 @@
 //   }
 //
 // In this case instantiation of the type of 'g1' requires definition of
-// 'x1', which is defined later. Error recovery may produce an enum used
-// before definition. In these cases we need to instantiate relevant
-// declarations here.
+// 'x1', which is defined later.
+//
+// Error recovery may produce an enum used before definition.
+//
+// Default arguments with a decltype referencing arguments:
+//
+//   template  void bar(T t = decltype(t)()) {}
+//
+// Else we must have a label decl that hasn't been found yet.
+//
+// In these cases we need to instantiate relevant declarations here.
 bool NeedInstantiate = false;
 if (CXXRecordDecl *RD = dyn_cast(D))
   NeedInstantiate = RD->isLocalClass();
 else
-  NeedInstantiate = isa(D);
+  NeedInstantiate =
+  isa(D) || isa(D) || isa(D);
 if (NeedInstantiate) {
   Decl *Inst = SubstDecl(D, CurContext, TemplateArgs);
+  assert(Inst && "Failed to instantiate");
   CurrentInstantiationScope->InstantiatedLocal(D, Inst);
-  return cast(Inst);
+  return cast(Inst);
 }
 
-// If we didn't find the decl, then we must have a label decl that hasn't
-// been found yet.  Lazily instantiate it and return it now.
-assert(isa(D));
-
-Decl *Inst = SubstDecl(D, CurContext, TemplateArgs);
-assert(Inst && "Failed to instantiate label??");
+assert(0 && "Failed to instantiate");
 
-CurrentInstantiationScope->InstantiatedLocal(D, Inst);
-return cast(Inst);
+return nullptr;
   }
 
   if (CXXRecordDecl *Record = dyn_cast(D)) {
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2974,6 +2974,11 @@
   if (isa(D))
 return nullptr;
 
+  // Default arguments with a decltype referencing arguments prior to definition
+  // may require instantiation.
+  if (isa(D))
+return nullptr;
+
   // If we didn't find the decl, then we either have a sema bug, or we have a
   // forward reference to a label declaration.  Return null to indicate that
   // we have an uninstantiated label.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

The overload resolution for enums with a fixed underlaying type has changed in 
the C++14 standard. This patch implements the new rule.

  

Note: I don't have access to the CWG 1601 paper, but based on 
https://en.cppreference.com/w/cpp/language/overload_resolution I applied the 
fix retroactively to C++11.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65695

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/drs/dr16xx.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/www/cxx_dr_status.html

Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -9421,7 +9421,7 @@
 http://wg21.link/cwg1601";>1601
 C++14
 Promotion of enumeration with fixed underlying type
-Unknown
+SVN (C++11 onwards)
   
   
 http://wg21.link/cwg1602";>1602
Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1007,9 +1007,10 @@
   void j(long); // expected-note {{candidate}}
   int d = j(g); // expected-error {{ambiguous}}
 
-  int k(short); // expected-note {{candidate}}
-  void k(int); // expected-note {{candidate}}
-  int x = k(g); // expected-error {{ambiguous}}
+  // Valid per dr1601
+  int k(short);
+  void k(int);
+  int x = k(g);
 }
 #endif
 
Index: clang/test/CXX/drs/dr16xx.cpp
===
--- clang/test/CXX/drs/dr16xx.cpp
+++ clang/test/CXX/drs/dr16xx.cpp
@@ -23,6 +23,17 @@
 } // std
 #endif
 
+namespace dr1601 { // dr1601
+#if __cplusplus >= 201103L
+enum E : char { e };
+void f(char);
+void f(int);
+void g() {
+  f(e);
+}
+#endif
+} // namespace dr1601
+
 namespace dr1611 { // dr1611: dup 1658
   struct A { A(int); };
   struct B : virtual A { virtual void f() = 0; };
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3751,6 +3751,26 @@
   !SCS2.IsLvalueReference && SCS2.BindsToFunctionLvalue);
 }
 
+/// Returns the underlaying type of a fixed enum of the \a SCS's @c FromType
+/// if the \a SCS uses an integral promotion. Upon failure an empty type is
+/// returned.
+static QualType
+getFixedEnumUnderlayingType(const StandardConversionSequence &SCS) {
+
+  if (SCS.Second != ICK_Integral_Promotion)
+return QualType();
+
+  QualType FromType = SCS.getFromType();
+  if (!FromType->isEnumeralType())
+return QualType();
+
+  EnumDecl *Enum = FromType->getAs()->getDecl();
+  if (!Enum->isFixed())
+return QualType();
+
+  return Enum->getIntegerType();
+}
+
 /// CompareStandardConversionSequences - Compare two standard
 /// conversion sequences to determine whether one is better than the
 /// other or if they are indistinguishable (C++ 13.3.3.2p3).
@@ -3792,6 +3812,23 @@
  ? ImplicitConversionSequence::Better
  : ImplicitConversionSequence::Worse;
 
+  // C++14 [over.ics.rank]p4b2:
+  // This is retroactively applied to C++11 by CWG 1601.
+  //
+  //   A conversion that promotes an enumeration whose underlying type is fixed
+  //   to its underlying type is better than one that promotes to the promoted
+  //   underlying type, if the two are different.
+  QualType UnderlayingType1 = getFixedEnumUnderlayingType(SCS1);
+  QualType UnderlayingType2 = getFixedEnumUnderlayingType(SCS2);
+  if (!UnderlayingType1.isNull() && !UnderlayingType2.isNull()) {
+if (SCS1.getToType(1) == UnderlayingType1 &&
+SCS2.getToType(1) != UnderlayingType2)
+  return ImplicitConversionSequence::Better;
+else if (SCS1.getToType(1) != UnderlayingType1 &&
+ SCS2.getToType(1) == UnderlayingType2)
+  return ImplicitConversionSequence::Worse;
+  }
+
   // C++ [over.ics.rank]p4b2:
   //
   //   If class B is derived directly or indirectly from class A,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65696: Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: rsmith.
Mordante added a project: clang.

This implements the current standard wording for [dcl.fct.default]p9 and 
[dcl.fct.default]p7. This has been changed by CWG 2082.

  

Note: I don't have access to the paper therefore I assume it retroactively 
applies to all C++ standards.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65696

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
  clang/www/cxx_dr_status.html


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12307,7 +12307,7 @@
 http://wg21.link/cwg2082";>2082
 CD4
 Referring to parameters in unevaluated operands of default 
arguments
-Unknown
+SVN
   
   
 http://wg21.link/cwg2083";>2083
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,7 +1,8 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-void h()
+void f()
 {
   int i;
-  extern void h2(int x = sizeof(i)); // expected-error {{default argument 
references local variable 'i' of enclosing function}}
+  extern void g(int x = i); // expected-error {{default argument references 
local variable 'i' of enclosing function}}
+  extern void h(int x = sizeof(i));
 }
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int a;
+int f(int a, int b = a); // expected-error {{default argument references 
parameter 'a'}}
+typedef int I;
+int g(float I, int b = I(2)); // expected-error {{called object type 'float' 
is not a function or function pointer}}
+int h(int a, int b = sizeof(a));
+
+int b;
+class X {
+  int a;
+  int mem1(int i = a); // expected-error {{invalid use of non-static data 
member 'a'}}
+  int mem2(int i = b);
+  static int b;
+};
+
+int f(int = 0);
+void h() {
+  int j = f(1);
+  int k = f();
+}
+int (*p1)(int) = &f;
+int (*p2)() = &f; // expected-error {{cannot initialize a variable of type 
'int (*)()' with an rvalue of type 'int (*)(int)': different number of 
parameters (0 vs 1)}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,20 +86,24 @@
 NamedDecl *Decl = DRE->getDecl();
 if (ParmVarDecl *Param = dyn_cast(Decl)) {
   // C++ [dcl.fct.default]p9
-  //   Default arguments are evaluated each time the function is
-  //   called. The order of evaluation of function arguments is
-  //   unspecified. Consequently, parameters of a function shall not
-  //   be used in default argument expressions, even if they are not
-  //   evaluated. Parameters of a function declared before a default
-  //   argument expression are in scope and can hide namespace and
-  //   class member names.
+  //   A default argument is evaluated each time the function is called
+  //   with no argument for the corresponding parameter. A parameter shall
+  //   not appear as a potentially-evaluated expression in a default
+  //   argument. Parameters of a function declared before a default
+  //   argument are in scope and can hide namespace and class member
+  //   names.
+  if (DRE->isNonOdrUse() == NOUR_Unevaluated)
+return false;
+
   return S->Diag(DRE->getBeginLoc(),
  diag::err_param_default_argument_references_param)
  << Param->getDeclName() << DefaultArg->getSourceRange();
 } else if (VarDecl *VDecl = dyn_cast(Decl)) {
   // C++ [dcl.fct.default]p7
-  //   Local variables shall not be used in default argument
-  //   expressions.
+  //   A local variable cannot be odr-used (6.2) in a default argument.
+  if (DRE->isNonOdrUse() != NOUR_None)
+return false;
+
   if (VDecl->isLocalVarDecl())
 return S->Diag(DRE->getBeginLoc(),
diag::err_param_default_argument_references_local)


Index: clang/www/cxx_dr_status.html
===
--- clang/www/cxx_dr_status.html
+++ clang/www/cxx_dr_status.html
@@ -12307,7 +12307,7 @@
 http://wg21.link/cwg2082";>2082
 CD4
 Referring to parameters in unevaluated operands of default arguments
-Unknown
+SVN
   
   
 http://wg21.link/cwg2083";>2083
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp

[PATCH] D65696: Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:103-105
+  //   A local variable cannot be odr-used (6.2) in a default argument.
+  if (DRE->isNonOdrUse() != NOUR_None)
+return false;

rsmith wrote:
> Please add tests for the distinction between "potentially-evaluated" and 
> "odr-used" here, for example:
> 
> ```
> void f() {
>   const int n = 123;
>   void g(int k = n); // ok, not an odr-use
> }
> ```
I added the test but unfortunately clang disagrees with you and considers `n` 
ODR used. 
I'll have look how to teach clang `n` is not ODR used.



Comment at: clang/www/cxx_dr_status.html:3
   "http://www.w3.org/TR/html4/strict.dtd";>
 
 

rsmith wrote:
> Note that this is an auto-generated file. To update it, you need to add a 
> test to the relevant file (`test/CXX/drs/dr20xx.cpp`) with a suitable comment 
> (`// dr2082: 10` to mark this implemented in Clang 10), grab a recent 
> `cwg_index.html` file, and run the `make_cxx_dr_status` script.
Thanks for the info, I'll have a look at it after I fix the ODR used part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65696



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

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/www/cxx_dr_status.html:3
   "http://www.w3.org/TR/html4/strict.dtd";>
 
 

I'll properly update this file as explained in D65696.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65695



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I think it should warn; according to the documentation [1]  `\c` expects a 
word. Testing with Doxygen indeed gives a warning.

Can you post the real comment where this occurs?

[1] http://www.doxygen.nl/manual/commands.html#cmdc


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64696



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

[PATCH] D64696: Adds a warning when an inline Doxygen comment has no argument

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Unfortunately I'm not able to quickly find the Doxygen output of Swift online. 
When I process:
`A limited variant of \c @objc that's used for classes with generic ancestry.`
with Doxygen I get:
`A limited variant of that's used for classes with generic ancestry.`
Since the `@` is used for commands I think they should be escaped like:
include/swift/AST/Decl.h: /// such as \c \@testable and \c \@usableFromInline 
into account.

Can you verify whether the Doxygen output for the Swift documentation is 
correct without escaping the `\c @foo` ? Or do you have a link to the online 
version?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D64696



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

[PATCH] D64811: Warn when NumParams overflows

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 215425.
Mordante added a comment.

Moved the testing from Parse to Sema.
Added additional safeguards for template instantiation.
Added more unit tests.
The comments may be a bit noisy, but they explain why the templates need to be 
tested at two locations.


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

https://reviews.llvm.org/D64811

Files:
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/function_parameter_overflow.cpp
  clang/test/Sema/lambda_function_parameter_overflow.cpp
  clang/test/Sema/parameter_overflow.h
  clang/test/Sema/template_function_parameter_overflow.cpp
  clang/test/Sema/variadic_function_instantiation_parameter_overflow.cpp
  clang/test/Sema/variadic_function_parameter_overflow.cpp
  clang/test/Sema/variadic_template_function_parameter_overflow.cpp

Index: clang/test/Sema/variadic_template_function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/variadic_template_function_parameter_overflow.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#define ARG 42
+#include "parameter_overflow.h"
+
+template 
+void foo(P &&... p);
+
+void bar() {
+  foo(A65535
+#ifdef FAIL
+  , ARG
+#endif
+  );
+}
+
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/test/Sema/variadic_function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/variadic_function_parameter_overflow.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: %clang_cc1 %s -fsyntax-only -DFAIL 2>&1
+
+#define ARG 42
+#include "parameter_overflow.h"
+
+// Unlike the other parameter overflow tests this one is not limited by
+// NumParamsBits so the test does not generate an error.
+
+void foo(...);
+
+void bar() {
+  foo(A65535
+#ifdef FAIL
+  , ARG
+#endif
+  );
+}
Index: clang/test/Sema/variadic_function_instantiation_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/variadic_function_instantiation_parameter_overflow.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: %clang_cc1 %s -fsyntax-only -verify -DFAIL
+
+#define ARG int
+#include "parameter_overflow.h"
+
+// The expansion of T... and the existance of fp cause the overflow.
+
+void foo(A65534
+#ifdef FAIL
+  , ARG
+#endif
+);
+
+template  void foobar(void (*fp)(T...)); // expected-note {{number of function parameters exceeded maximum of 65535}}
+
+void bar() {
+  foobar(foo);
+}
Index: clang/test/Sema/template_function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/template_function_parameter_overflow.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#define ARG INT
+#include "parameter_overflow.h"
+
+template 
+void foo(A65535
+#ifdef FAIL
+, ARG
+#endif
+);
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/test/Sema/parameter_overflow.h
===
--- /dev/null
+++ clang/test/Sema/parameter_overflow.h
@@ -0,0 +1,10 @@
+#pragma once
+
+#define A10 ARG, ARG, ARG, ARG, ARG, ARG, ARG, ARG, ARG, ARG
+#define A50 A10, A10, A10, A10, A10
+#define A500 A50, A50, A50, A50, A50, A50, A50, A50, A50, A50
+#define A5000 A500, A500, A500, A500, A500, A500, A500, A500, A500, A500
+#define A6 A5000, A5000, A5000, A5000, A5000, A5000, A5000, A5000, A5000, A5000, A5000, A5000
+
+#define  A65534 A6, A5000, A500, A10, A10, A10, ARG, ARG, ARG, ARG
+#define  A65535 A65534, ARG
Index: clang/test/Sema/lambda_function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/lambda_function_parameter_overflow.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#define ARG int
+#include "parameter_overflow.h"
+
+auto foo = [](A65535
+#ifdef FAIL
+, ARG
+#endif
+){};
+// CHECK: fatal error: number of function parameters exceeded maximum of 65535
Index: clang/test/Sema/function_parameter_overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/function_parameter_overflow.cpp
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+#define ARG int
+#include "parameter_overflow.h"
+
+void foo(A65535
+#

[PATCH] D65696: Implements CWG 2082 Referring to parameters in unevaluated operands of default arguments

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 215448.
Mordante added a comment.

Updated the unit tests as requested. This required the 
`Sema::ActOnParamDefaultArgument` to delay a part of the ODR validation until 
the default argument has been 'instantiated'.
As discussed on IRC; the up to date `cwg_index.html` is not public, so I only 
updated the unit test and removed the changes to `cxx_dr_status.html`.


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

https://reviews.llvm.org/D65696

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
  clang/test/CXX/drs/dr20xx.cpp

Index: clang/test/CXX/drs/dr20xx.cpp
===
--- clang/test/CXX/drs/dr20xx.cpp
+++ clang/test/CXX/drs/dr20xx.cpp
@@ -8,6 +8,76 @@
 #define static_assert(...) _Static_assert(__VA_ARGS__)
 #endif
 
+
+namespace dr2082 { // dr2082: 10
+namespace local_var {
+void g() {
+  int k = 42;
+  void l(int m = k); // expected-error {{default argument references local variable 'k' of enclosing function}}
+}
+} // namespace local_var
+namespace local_const {
+void g() {
+  const int k = 42;
+  void l(int m = k);
+}
+} // namespace local_const
+#if __cplusplus >= 201103L
+namespace local_constexpr {
+void g() {
+  constexpr int k = 42;
+  void l(int m = k);
+}
+} // namespace local_constexpr
+#endif
+
+namespace local_const_float_to_integral {
+void g() {
+  const double k = 42;
+  void l(int m = k); // expected-error {{default argument references local variable 'k' of enclosing function}}
+}
+} // namespace local_const_float_to_integral
+#if __cplusplus >= 201103L
+namespace local_constexpr_float_to_integral {
+void g() {
+  constexpr double k = 42;
+  void l(int m = k);
+}
+} // namespace local_constexpr_float_to_integral
+
+namespace local_member_const {
+struct a {
+  int b;
+  int c;
+};
+void g() {
+  const a n{42, 42};
+  void l(int m = n.b); // expected-error {{default argument references local variable 'n' of enclosing function}}
+}
+} // namespace local_member_const
+namespace local_member_constexpr {
+struct a {
+  int b;
+  int c;
+};
+void g() {
+  constexpr a n{42, 42};
+  void l(int m = n.b);
+}
+} // namespace local_member_constexpr
+namespace local_member_mutable {
+struct a {
+  int b;
+  mutable int c;
+};
+void g() {
+  constexpr a n{42, 42};
+  void l(int m = n.b); // expected-error {{default argument references local variable 'n' of enclosing function}}
+}
+} // namespace local_member_mutable
+#endif
+}
+
 namespace dr2083 { // dr2083: partial
 #if __cplusplus >= 201103L
   void non_const_mem_ptr() {
Index: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
===
--- clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
+++ clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp
@@ -1,7 +1,8 @@
 // RUN: %clang_cc1 -fsyntax-only -verify %s
 
-void h()
+void f()
 {
   int i;
-  extern void h2(int x = sizeof(i)); // expected-error {{default argument references local variable 'i' of enclosing function}}
+  extern void g(int x = i); // expected-error {{default argument references local variable 'i' of enclosing function}}
+  extern void h(int x = sizeof(i));
 }
Index: clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p9.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+int a;
+int f(int a, int b = a); // expected-error {{default argument references parameter 'a'}}
+typedef int I;
+int g(float I, int b = I(2)); // expected-error {{called object type 'float' is not a function or function pointer}}
+int h(int a, int b = sizeof(a));
+
+int b;
+class X {
+  int a;
+  int mem1(int i = a); // expected-error {{invalid use of non-static data member 'a'}}
+  int mem2(int i = b);
+  static int b;
+};
+
+int f(int = 0);
+void h() {
+  int j = f(1);
+  int k = f();
+}
+int (*p1)(int) = &f;
+int (*p2)() = &f; // expected-error {{cannot initialize a variable of type 'int (*)()' with an rvalue of type 'int (*)(int)': different number of parameters (0 vs 1)}}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -86,29 +86,22 @@
 NamedDecl *Decl = DRE->getDecl();
 if (ParmVarDecl *Param = dyn_cast(Decl)) {
   // C++ [dcl.fct.default]p9
-  //   Default arguments are evaluated each time the function is
-  //   called. The order of evaluation of function arguments is
-  //   unspecified. Consequently, parameters of a function shall not
-  //   be used in default argument expressions, even if they are not
-  //   evaluated. Parameters of a function declared before a default

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 215457.
Mordante added a comment.

Add the proper markers in the unit test to update `cxx_dr_status.html`. As 
discussed on IRC; the up to date `cwg_index.html` is not public, so I only 
updated the unit test and removed the changes to `cxx_dr_status.html`.


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

https://reviews.llvm.org/D65695

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/drs/dr16xx.cpp
  clang/test/CXX/drs/dr6xx.cpp


Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1007,9 +1007,10 @@
   void j(long); // expected-note {{candidate}}
   int d = j(g); // expected-error {{ambiguous}}
 
-  int k(short); // expected-note {{candidate}}
-  void k(int); // expected-note {{candidate}}
-  int x = k(g); // expected-error {{ambiguous}}
+  // Valid per dr1601
+  int k(short);
+  void k(int);
+  int x = k(g);
 }
 #endif
 
Index: clang/test/CXX/drs/dr16xx.cpp
===
--- clang/test/CXX/drs/dr16xx.cpp
+++ clang/test/CXX/drs/dr16xx.cpp
@@ -23,6 +23,17 @@
 } // std
 #endif
 
+namespace dr1601 { // dr1601: 10 c++11
+#if __cplusplus >= 201103L
+enum E : char { e };
+void f(char);
+void f(int);
+void g() {
+  f(e);
+}
+#endif
+} // namespace dr1601
+
 namespace dr1611 { // dr1611: dup 1658
   struct A { A(int); };
   struct B : virtual A { virtual void f() = 0; };
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3751,6 +3751,26 @@
   !SCS2.IsLvalueReference && SCS2.BindsToFunctionLvalue);
 }
 
+/// Returns the underlaying type of a fixed enum of the \a SCS's \c FromType
+/// if the \a SCS uses an integral promotion. Upon failure an empty type is
+/// returned.
+static QualType
+getFixedEnumUnderlayingType(const StandardConversionSequence &SCS) {
+
+  if (SCS.Second != ICK_Integral_Promotion)
+return QualType();
+
+  QualType FromType = SCS.getFromType();
+  if (!FromType->isEnumeralType())
+return QualType();
+
+  EnumDecl *Enum = FromType->getAs()->getDecl();
+  if (!Enum->isFixed())
+return QualType();
+
+  return Enum->getIntegerType();
+}
+
 /// CompareStandardConversionSequences - Compare two standard
 /// conversion sequences to determine whether one is better than the
 /// other or if they are indistinguishable (C++ 13.3.3.2p3).
@@ -3792,6 +3812,23 @@
  ? ImplicitConversionSequence::Better
  : ImplicitConversionSequence::Worse;
 
+  // C++14 [over.ics.rank]p4b2:
+  // This is retroactively applied to C++11 by CWG 1601.
+  //
+  //   A conversion that promotes an enumeration whose underlying type is fixed
+  //   to its underlying type is better than one that promotes to the promoted
+  //   underlying type, if the two are different.
+  QualType UnderlayingType1 = getFixedEnumUnderlayingType(SCS1);
+  QualType UnderlayingType2 = getFixedEnumUnderlayingType(SCS2);
+  if (!UnderlayingType1.isNull() && !UnderlayingType2.isNull()) {
+if (SCS1.getToType(1) == UnderlayingType1 &&
+SCS2.getToType(1) != UnderlayingType2)
+  return ImplicitConversionSequence::Better;
+else if (SCS1.getToType(1) != UnderlayingType1 &&
+ SCS2.getToType(1) == UnderlayingType2)
+  return ImplicitConversionSequence::Worse;
+  }
+
   // C++ [over.ics.rank]p4b2:
   //
   //   If class B is derived directly or indirectly from class A,


Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -1007,9 +1007,10 @@
   void j(long); // expected-note {{candidate}}
   int d = j(g); // expected-error {{ambiguous}}
 
-  int k(short); // expected-note {{candidate}}
-  void k(int); // expected-note {{candidate}}
-  int x = k(g); // expected-error {{ambiguous}}
+  // Valid per dr1601
+  int k(short);
+  void k(int);
+  int x = k(g);
 }
 #endif
 
Index: clang/test/CXX/drs/dr16xx.cpp
===
--- clang/test/CXX/drs/dr16xx.cpp
+++ clang/test/CXX/drs/dr16xx.cpp
@@ -23,6 +23,17 @@
 } // std
 #endif
 
+namespace dr1601 { // dr1601: 10 c++11
+#if __cplusplus >= 201103L
+enum E : char { e };
+void f(char);
+void f(int);
+void g() {
+  f(e);
+}
+#endif
+} // namespace dr1601
+
 namespace dr1611 { // dr1611: dup 1658
   struct A { A(int); };
   struct B : virtual A { virtual void f() = 0; };
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3751,6 +3751,26 @@
   !SCS2.IsLvalueReference && SCS2.BindsToFunctionLvalue);
 }
 
+/// Returns the underlaying type of a fixed enum of the \a SCS's \c From

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 4 inline comments as done.
Mordante added inline comments.



Comment at: clang/test/CXX/drs/dr16xx.cpp:26
 
+namespace dr1601 { // dr1601: 10 c++11
+#if __cplusplus >= 201103L

rsmith wrote:
> No need for the "c++11" marker here. (We accept fixed underlying types in 
> C++98 as an extension, and your change applies there too.)
I will also update dr685.


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

https://reviews.llvm.org/D65695



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

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 215736.
Mordante marked an inline comment as done.
Mordante edited the summary of this revision.
Mordante added a comment.

Implemented the changes requested by @rsmith.


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

https://reviews.llvm.org/D65695

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/drs/dr16xx.cpp
  clang/test/CXX/drs/dr6xx.cpp

Index: clang/test/CXX/drs/dr6xx.cpp
===
--- clang/test/CXX/drs/dr6xx.cpp
+++ clang/test/CXX/drs/dr6xx.cpp
@@ -987,14 +987,19 @@
 }
 #endif
 
-#if __cplusplus >= 201103L
 namespace dr685 { // dr685: yes
   enum E : long { e };
+#if __cplusplus < 201103L
+// expected-error@-2 {{enumeration types with a fixed underlying type are a C++11 extension}}
+#endif
   void f(int);
   int f(long);
   int a = f(e);
 
   enum G : short { g };
+#if __cplusplus < 201103L
+// expected-error@-2 {{enumeration types with a fixed underlying type are a C++11 extension}}
+#endif
   int h(short);
   void h(long);
   int b = h(g);
@@ -1007,11 +1012,11 @@
   void j(long); // expected-note {{candidate}}
   int d = j(g); // expected-error {{ambiguous}}
 
-  int k(short); // expected-note {{candidate}}
-  void k(int); // expected-note {{candidate}}
-  int x = k(g); // expected-error {{ambiguous}}
+  // Valid per dr1601
+  int k(short);
+  void k(int);
+  int x = k(g);
 }
-#endif
 
 namespace dr686 { // dr686: yes
   void f() {
Index: clang/test/CXX/drs/dr16xx.cpp
===
--- clang/test/CXX/drs/dr16xx.cpp
+++ clang/test/CXX/drs/dr16xx.cpp
@@ -23,6 +23,18 @@
 } // std
 #endif
 
+namespace dr1601 { // dr1601: 10
+enum E : char { e };
+#if __cplusplus < 201103L
+// expected-error@-2 {{enumeration types with a fixed underlying type are a C++11 extension}}
+#endif
+void f(char);
+void f(int);
+void g() {
+  f(e);
+}
+} // namespace dr1601
+
 namespace dr1611 { // dr1611: dup 1658
   struct A { A(int); };
   struct B : virtual A { virtual void f() = 0; };
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -3751,6 +3751,34 @@
   !SCS2.IsLvalueReference && SCS2.BindsToFunctionLvalue);
 }
 
+enum class FixedEnumPromotion {
+  None,
+  ToUnderlyingType,
+  ToPromotedUnderlyingType
+};
+
+/// Returns kind of fixed enum promotion the \a SCS uses.
+static FixedEnumPromotion
+getFixedEnumPromtion(const StandardConversionSequence &SCS) {
+
+  if (SCS.Second != ICK_Integral_Promotion)
+return FixedEnumPromotion::None;
+
+  QualType FromType = SCS.getFromType();
+  if (!FromType->isEnumeralType())
+return FixedEnumPromotion::None;
+
+  EnumDecl *Enum = FromType->getAs()->getDecl();
+  if (!Enum->isFixed())
+return FixedEnumPromotion::None;
+
+  QualType UnderlyingType = Enum->getIntegerType();
+  if (SCS.getToType(1) == UnderlyingType)
+return FixedEnumPromotion::ToUnderlyingType;
+
+  return FixedEnumPromotion::ToPromotedUnderlyingType;
+}
+
 /// CompareStandardConversionSequences - Compare two standard
 /// conversion sequences to determine whether one is better than the
 /// other or if they are indistinguishable (C++ 13.3.3.2p3).
@@ -3792,6 +3820,23 @@
  ? ImplicitConversionSequence::Better
  : ImplicitConversionSequence::Worse;
 
+  // C++14 [over.ics.rank]p4b2:
+  // This is retroactively applied to C++11 by CWG 1601.
+  //
+  //   A conversion that promotes an enumeration whose underlying type is fixed
+  //   to its underlying type is better than one that promotes to the promoted
+  //   underlying type, if the two are different.
+  FixedEnumPromotion FEP1 = getFixedEnumPromtion(SCS1);
+  FixedEnumPromotion FEP2 = getFixedEnumPromtion(SCS2);
+  if (FEP1 != FixedEnumPromotion::None && FEP2 != FixedEnumPromotion::None) {
+if (FEP1 == FixedEnumPromotion::ToUnderlyingType &&
+FEP2 == FixedEnumPromotion::ToPromotedUnderlyingType)
+  return ImplicitConversionSequence::Better;
+else if (FEP1 == FixedEnumPromotion::ToPromotedUnderlyingType &&
+ FEP2 == FixedEnumPromotion::ToUnderlyingType)
+  return ImplicitConversionSequence::Worse;
+  }
+
   // C++ [over.ics.rank]p4b2:
   //
   //   If class B is derived directly or indirectly from class A,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64874: Improve handling of function pointer conversions

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64874



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

[PATCH] D64820: Avoids an assertion failure when an invalid conversion declaration is used

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64820



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

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: clang/test/SemaTemplate/default-arguments-cxx0x.cpp:1
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics

Wouldn't it be better to keep the original C++11 test and add a second `RUN:` 
for the C++14 test?
Then guard the new test with `#if __cplusplus >= 201402L`


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

https://reviews.llvm.org/D66067



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

[PATCH] D63975: Warn when ScopeDepthOrObjCQuals overflows

[Mark de Wever via Phabricator via cfe-commits]

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

Before when the overflow occurred an assertion as triggered. Now check
whether the maximum has been reached and warn properly.

This patch fixes the original submission of bug 19607. The part mentioned
in its 'duplicate' bug 33162 has not been fixed. I want to look at that
after this patch has been accepted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D63975

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/test/Parser/nested_function_prototype_overflow.cpp


Index: clang/test/Parser/nested_function_prototype_overflow.cpp
===
--- /dev/null
+++ clang/test/Parser/nested_function_prototype_overflow.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 %s -fsyntax-only
+// RUN: not %clang_cc1 %s -fsyntax-only -DFAIL 2>&1 | FileCheck %s
+
+void foo(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void 
(*f)(
+#ifdef FAIL
+void (*f)()
+#endif
+;
+
+// CHECK: fatal error: function scope depth exceeded maximum of 127
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -6576,6 +6576,16 @@
   Actions.containsUnexpandedParameterPacks(ParmDeclarator))
 DiagnoseMisplacedEllipsisInDeclarator(ConsumeToken(), ParmDeclarator);
 
+ // Avoid exceeding the maximum function scope depth.
+ // https://bugs.llvm.org/show_bug.cgi?id=19607
+  if (getCurScope()->getFunctionPrototypeDepth() - 1 >=
+  ParmVarDecl::getMaxFunctionScopeDepth()) {
+Diag(DSStart, diag::err_function_scope_depth_exceeded)
+<< ParmVarDecl::getMaxFunctionScopeDepth();
+cutOffParsing();
+return;
+  }
+
   // Inform the actions module about the parameter declarator, so it gets
   // added to the current scope.
   Decl *Param = Actions.ActOnParamDeclarator(getCurScope(), 
ParmDeclarator);
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -326,6 +326,8 @@
 def err_argument_required_after_attribute : Error<
   "argument required after attribute">;
 def err_missing_param : Error<"expected parameter declarator">;
+def err_function_scope_depth_exceeded : Error<
+  "function scope depth exceeded maximum of %0">, DefaultFatal;
 def err_missing_comma_before_ellipsis : Error<
   "C requires a comma prior to the ellipsis in a variadic function type">;
 def err_unexpected_typedef_ident : Error<
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -905,11 +905,13 @@
 /// Whether this parameter is an ObjC method parameter or not.
 unsigned IsObjCMethodParam : 1;
 
+   enum { NumScopeDepthOrObjCQualsBits = 7 };
+
 /// If IsObjCMethodParam, a Decl::ObjCDeclQualifier.
 /// Otherwise, the number of function parameter scopes enclosing
 /// the function parameter scope in which this parameter was
 /// declared.
-unsigned ScopeDepthOrObjCQuals : 7;
+unsigned ScopeDepthOrObjCQuals : NumScopeDepthOrObjCQualsBits;
 
 /// The number of parameters preceding this parameter in the
 /// function parameter s

[PATCH] D65695: Implements CWG 1601 in [over.ics.rank/4.2]

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for the review. Can you commit the patch since I don't have commit 
access?


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

https://reviews.llvm.org/D65695



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

[PATCH] D64820: [Sema] Avoids an assertion failure when an invalid conversion declaration is used

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for the review. Can you commit the patch since I don't have commit 
access?


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

https://reviews.llvm.org/D64820



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

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rtrieu, xbolva00.
Mordante added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68913

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 template 
 struct Iterator {
@@ -67,14 +68,17 @@
   for (const int &x : int_non_ref_container) {}
   // expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container' does not return a reference}}
   // expected-note@-2 {{use non-reference type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
 
   for (const double &x : int_container) {}
   // expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
   // expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
 
   for (const Bar x : bar_container) {}
   // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
   // expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
 }
 
 void test1() {
@@ -83,6 +87,7 @@
   for (const int &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (int &x : A) {}
@@ -93,6 +98,7 @@
   for (const double &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (double &x : A) {}
@@ -103,6 +109,7 @@
   for (const Bar &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : A) {}
@@ -126,6 +133,7 @@
   for (const double &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
   //for (double &x : B) {}
   // Binding error
@@ -135,6 +143,7 @@
   for (const Bar &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
   //for (Bar &x : B) {}
   // Binding error
@@ -148,6 +157,7 @@
   for (const Bar &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : C) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : C) {}
@@ -158,6 +168,7 @@
   for (const int &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : C) {}
   // No warning, copy made
   //for (int &x : C) {}
@@ -174,6 +185,7 @@
   for (const Bar x : D) {}
   // expected-warning@-1 {{creates a copy}}
   // expected-note@-2 {{'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
   for (Bar &x : D) {}
   // No warning
   for (Bar x : D) {}
@@ -182,6 +194,7 @@
   for (const int &x : D) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : D) {}
   // No warning
   //for (int &x : D) {}
@@ -196,6 +209,7 @@
   for (const Bar &x : E) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : E) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : E) {}
@@ -210,6 +224,7 @@
   for (const Bar &x : F) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
  

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rtrieu, xbolva00.
Mordante added a project: clang.

This makes the range loop warnings part of -Wall.

This 'fixes' https://bugs.llvm.org/show_bug.cgi?id=32823


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68912

Files:
  clang/include/clang/Basic/DiagnosticGroups.td


Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -818,11 +818,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,


Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -818,11 +818,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64874: [Sema] Improve handling of function pointer conversions

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 224753.
Mordante added a comment.

Removed the language version test.

I also added `clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp` to show 
this is not enough to allow `noreturn` to all language versions, so this file 
should not be committed.

Do you want me to look at a way to allow the `noreturn` for C++98/C++11? If yes 
I prefer to make that a separate commit.

AFAIK the `[[noreturn]]` conversion is a clang extension, should I file a DR 
for the standard? AFAIK there isn't one yet.

If you still like the patch could you commit it (except 
`clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp`) since I don't have 
commit access?


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

https://reviews.llvm.org/D64874

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
  clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp

Index: clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noreturn.cpp
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+typedef int (*fp)();
+typedef int (*fpnr)() __attribute__((noreturn));
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template 
+struct FP {};
+
+#if __cplusplus < 201703L
+// expected-note@+2 {{template parameter is declared here}}
+#endif
+template 
+struct FPNR {};
+
+int f();
+int fnr() __attribute__((noreturn));
+
+FP<&f> fp_valid;
+FPNR<&fnr> fpnr_valid;
+
+#if __cplusplus < 201703L
+// expected-error@+2 {{non-type template argument of type 'int (*)() __attribute__((noreturn))' cannot be converted to a value of type 'fp' (aka 'int (*)()')}}
+#endif
+FP<&fnr> fp_invalid_before_cxx17;
+
+#if __cplusplus < 201703L
+// expected-error@+4 {{non-type template argument of type 'int (*)()' cannot be converted to a value of type 'fpnr' (aka 'int (*)() __attribute__((noreturn))')}}
+#else
+// expected-error@+2 {{value of type 'int (*)()' is not implicitly convertible to 'fpnr' (aka 'int (*)() __attribute__((noreturn))')}}
+#endif
+FPNR<&f> fpnr_invalid;
Index: clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
===
--- /dev/null
+++ clang/test/CXX/conv/conv.fctptr/template-noexcept.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -verify %s
+
+// Starting with C++17 the noexcept is part of the function signature but
+// a noexcept function can be converted to a noexcept(false) function.
+// The tests were added for PR40024.
+
+#if __cplusplus < 201703L
+// expected-no-diagnostics
+#endif
+
+namespace valid_conversion {
+struct S {
+  int f(void) noexcept { return 110; }
+} s;
+
+template 
+int f10(void) { return (s.*a)(); }
+
+int foo(void) {
+  return f10<&S::f>();
+}
+} // namespace valid_conversion
+
+namespace invalid_conversion {
+struct S {
+  int f(void) { return 110; }
+} s;
+
+#if __cplusplus >= 201703L
+// expected-note@+3 {{candidate template ignored: invalid explicitly-specified argument for template parameter 'a'}}
+#endif
+template 
+int f10(void) { return (s.*a)(); }
+
+#if __cplusplus >= 201703L
+// expected-error@+3 {{no matching function for call to 'f10'}}
+#endif
+int foo(void) {
+  return f10<&S::f>();
+}
+} // namespace invalid_conversion
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -6991,6 +6991,13 @@
 ObjCLifetimeConversion))
 RefExpr = ImpCastExprToType(RefExpr.get(), ParamType.getUnqualifiedType(), CK_NoOp);
 
+  // Starting with C++17 the noexcept is part of the function signature but
+  // a noexcept function can be converted to a noexcept(false) function.
+  QualType ResultTy;
+  if (IsFunctionConversion(((Expr *)RefExpr.get())->getType(),
+   ParamType.getUnqualifiedType(), ResultTy))
+RefExpr = ImpCastExprToType(RefExpr.get(), ResultTy, CK_NoOp);
+
   assert(!RefExpr.isInvalid() &&
  Context.hasSameType(((Expr*) RefExpr.get())->getType(),
  ParamType.getUnqualifiedType()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 225782.
Mordante added a comment.

Undo the changes to the caret position as requested by @aaron.ballman .


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

https://reviews.llvm.org/D68913

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/warn-range-loop-analysis.cpp

Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 template 
 struct Iterator {
@@ -67,14 +68,17 @@
   for (const int &x : int_non_ref_container) {}
   // expected-warning@-1 {{loop variable 'x' is always a copy because the range of type 'Container' does not return a reference}}
   // expected-note@-2 {{use non-reference type 'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
 
   for (const double &x : int_container) {}
   // expected-warning@-1 {{loop variable 'x' has type 'const double &' but is initialized with type 'int' resulting in a copy}}
   // expected-note@-2 {{use non-reference type 'double' to keep the copy or type 'const int &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
 
   for (const Bar x : bar_container) {}
   // expected-warning@-1 {{loop variable 'x' of type 'const Bar' creates a copy from type 'const Bar'}}
   // expected-note@-2 {{use reference type 'const Bar &' to prevent copying}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
 }
 
 void test1() {
@@ -83,6 +87,7 @@
   for (const int &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (int &x : A) {}
@@ -93,6 +98,7 @@
   for (const double &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'double'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (double &x : A) {}
@@ -103,6 +109,7 @@
   for (const Bar &x : A) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : A) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : A) {}
@@ -126,6 +133,7 @@
   for (const double &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'double'{{.*}}'const int &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:21-[[@LINE-3]]:22}:""
   for (const double x : B) {}
   //for (double &x : B) {}
   // Binding error
@@ -135,6 +143,7 @@
   for (const Bar &x : B) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : B) {}
   //for (Bar &x : B) {}
   // Binding error
@@ -148,6 +157,7 @@
   for (const Bar &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : C) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : C) {}
@@ -158,6 +168,7 @@
   for (const int &x : C) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'int'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : C) {}
   // No warning, copy made
   //for (int &x : C) {}
@@ -174,6 +185,7 @@
   for (const Bar x : D) {}
   // expected-warning@-1 {{creates a copy}}
   // expected-note@-2 {{'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:18}:"&"
   for (Bar &x : D) {}
   // No warning
   for (Bar x : D) {}
@@ -182,6 +194,7 @@
   for (const int &x : D) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'int'{{.*}}'const Bar &'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const int x : D) {}
   // No warning
   //for (int &x : D) {}
@@ -196,6 +209,7 @@
   for (const Bar &x : E) {}
   // expected-warning@-1 {{always a copy}}
   // expected-note@-2 {{'Bar'}}
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:18-[[@LINE-3]]:19}:""
   for (const Bar x : E) {}
   // No warning, non-reference type indicates copy is made
   //for (Bar &x : E) {}
@@ -210,6 +224,7 @@
   for (const Bar &x : F) {}
   // expected-warning@-1 {{resulting in a copy}}
   // expected-note-re@-2 {{'Bar'{{.*}}'const Foo &'}}
+  // CHE

[PATCH] D69223: WDocumentation: Implement the \anchor.

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added a reviewer: gribozavr.
Mordante added a project: clang.
Herald added a subscriber: arphaman.

I'm not sure it should be added to the `InlineComment` group. It's not entirely 
a markup. Do you think it should be a in a separate group?

(I also have not yet posted code for `\emoji` which is also in the 
`InlineComment` group.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69223

Files:
  clang/bindings/xml/comment-xml-schema.rng
  clang/include/clang-c/Documentation.h
  clang/include/clang/AST/Comment.h
  clang/include/clang/AST/CommentCommands.td
  clang/lib/AST/CommentSema.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Index/CommentToXML.cpp
  clang/test/Index/Inputs/CommentXML/valid-function-02.xml
  clang/test/Index/comment-to-html-xml-conversion.cpp
  clang/test/Sema/warn-documentation.cpp
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CXComment.cpp

Index: clang/tools/libclang/CXComment.cpp
===
--- clang/tools/libclang/CXComment.cpp
+++ clang/tools/libclang/CXComment.cpp
@@ -159,6 +159,9 @@
 
   case InlineCommandComment::RenderEmphasized:
 return CXCommentInlineCommandRenderKind_Emphasized;
+
+  case InlineCommandComment::RenderAnchor:
+return CXCommentInlineCommandRenderKind_Anchor;
   }
   llvm_unreachable("unknown InlineCommandComment::RenderKind");
 }
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -497,6 +497,9 @@
 case CXCommentInlineCommandRenderKind_Emphasized:
   printf(" RenderEmphasized");
   break;
+case CXCommentInlineCommandRenderKind_Anchor:
+  printf(" RenderAnchor");
+  break;
 }
 for (i = 0, e = clang_InlineCommandComment_getNumArgs(Comment);
  i != e; ++i) {
Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -1057,6 +1057,13 @@
 /// \a A
 int test_inline_no_argument_a_good(int);
 
+// expected-warning@+1 {{'\anchor' command does not have a valid word argument}}
+/// \anchor
+int test_inline_no_argument_anchor_bad(int);
+
+/// \anchor A
+int test_inline_no_argument_anchor_good(int);
+
 // expected-warning@+1 {{'@b' command does not have a valid word argument}}
 /// @b
 int test_inline_no_argument_b_bad(int);
Index: clang/test/Index/comment-to-html-xml-conversion.cpp
===
--- clang/test/Index/comment-to-html-xml-conversion.cpp
+++ clang/test/Index/comment-to-html-xml-conversion.cpp
@@ -734,6 +734,16 @@
 // CHECK-NEXT: (CXComment_Text Text=[Aaa])
 // CHECK-NEXT: (CXComment_HTMLEndTag Name=[h1])))]
 
+/// \anchor A
+void comment_to_html_conversion_37();
+
+// CHECK: comment-to-html-xml-conversion.cpp:[[@LINE-2]]:6: FunctionDecl=comment_to_html_conversion_37:{{.*}} FullCommentAsHTML=[ ] FullCommentAsXML=[comment_to_html_conversion_37c:@F@comment_to_html_conversion_37#void comment_to_html_conversion_37() A]
+// CHECK-NEXT:  CommentAST=[
+// CHECK-NEXT:(CXComment_FullComment
+// CHECK-NEXT:   (CXComment_Paragraph
+// CHECK-NEXT: (CXComment_Text Text=[ ] IsWhitespace)
+// CHECK-NEXT: (CXComment_InlineCommand CommandName=[anchor] RenderAnchor Arg[0]=A)))]
+
 
 /// Aaa.
 class comment_to_xml_conversion_01 {
Index: clang/test/Index/Inputs/CommentXML/valid-function-02.xml
===
--- clang/test/Index/Inputs/CommentXML/valid-function-02.xml
+++ clang/test/Index/Inputs/CommentXML/valid-function-02.xml
@@ -9,6 +9,7 @@
 
 
 .
+hhh
   
 
 
Index: clang/lib/Index/CommentToXML.cpp
===
--- clang/lib/Index/CommentToXML.cpp
+++ clang/lib/Index/CommentToXML.cpp
@@ -297,6 +297,12 @@
 appendToResultWithHTMLEscaping(Arg0);
 Result << "";
 return;
+  case InlineCommandComment::RenderAnchor:
+assert(C->getNumArgs() == 1);
+Result << "";
+return;
   }
 }
 
@@ -641,6 +647,12 @@
 appendToResultWithXMLEscaping(Arg0);
 Result << "";
 return;
+  case InlineCommandComment::RenderAnchor:
+assert(C->getNumArgs() == 1);
+Result << "";
+appendToResultWithXMLEscaping(Arg0);
+Result << "";
+return;
   }
 }
 
Index: clang/lib/AST/TextNodeDumper.cpp
===
--- clang/lib/AST/TextNodeDumper.cpp
+++ clang/lib/AST/TextNodeDumper.cpp
@@ -489,6 +489,9 @@
   case comments::InlineCommandComment::RenderEmphasized:
 OS << " RenderEmphasized";
 break;
+  case comments::InlineCommandComment::RenderAnchor:
+OS << " Render

[PATCH] D69225: Sema: Fixes a crash with a templated destructor

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rcraik, hubert.reinterpretcast, aaron.ballman, rsmith.
Mordante added a project: clang.

This fixes PR38671.
The issue was introduced by D33833  which 
fixed PR33189.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69225

Files:
  clang/lib/Sema/SemaLookup.cpp
  clang/test/SemaTemplate/destructor-template.cpp


Index: clang/test/SemaTemplate/destructor-template.cpp
===
--- clang/test/SemaTemplate/destructor-template.cpp
+++ clang/test/SemaTemplate/destructor-template.cpp
@@ -92,3 +92,13 @@
   template 
   ~PR33189() { } // expected-error{{destructor cannot be declared as a 
template}}
 };
+
+namespace PR38671 {
+struct S {
+  template 
+  ~S(); // expected-error{{destructor cannot be declared as a template}}
+};
+struct T : S {// expected-note{{destructor of 'T' is implicitly deleted 
because base class 'PR38671::S' has no destructor}}
+  ~T() = default; // expected-warning{{explicitly defaulted destructor is 
implicitly deleted}}
+};
+} // namespace PR38671
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3101,11 +3101,10 @@
   });
 }
 CXXDestructorDecl *DD = RD->getDestructor();
-assert(DD && "record without a destructor");
 Result->setMethod(DD);
-Result->setKind(DD->isDeleted() ?
-SpecialMemberOverloadResult::NoMemberOrDeleted :
-SpecialMemberOverloadResult::Success);
+Result->setKind(DD && !DD->isDeleted()
+? SpecialMemberOverloadResult::Success
+: SpecialMemberOverloadResult::NoMemberOrDeleted);
 return *Result;
   }
 


Index: clang/test/SemaTemplate/destructor-template.cpp
===
--- clang/test/SemaTemplate/destructor-template.cpp
+++ clang/test/SemaTemplate/destructor-template.cpp
@@ -92,3 +92,13 @@
   template 
   ~PR33189() { } // expected-error{{destructor cannot be declared as a template}}
 };
+
+namespace PR38671 {
+struct S {
+  template 
+  ~S(); // expected-error{{destructor cannot be declared as a template}}
+};
+struct T : S {// expected-note{{destructor of 'T' is implicitly deleted because base class 'PR38671::S' has no destructor}}
+  ~T() = default; // expected-warning{{explicitly defaulted destructor is implicitly deleted}}
+};
+} // namespace PR38671
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -3101,11 +3101,10 @@
   });
 }
 CXXDestructorDecl *DD = RD->getDestructor();
-assert(DD && "record without a destructor");
 Result->setMethod(DD);
-Result->setKind(DD->isDeleted() ?
-SpecialMemberOverloadResult::NoMemberOrDeleted :
-SpecialMemberOverloadResult::Success);
+Result->setKind(DD && !DD->isDeleted()
+? SpecialMemberOverloadResult::Success
+: SpecialMemberOverloadResult::NoMemberOrDeleted);
 return *Result;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 226539.
Mordante added a comment.

Adds a test as requested by @xbolva00 .


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

https://reviews.llvm.org/D68912

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/SemaCXX/warn-range-loop-analysis.cpp


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -818,11 +818,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,


Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp
===
--- clang/test/SemaCXX/warn-range-loop-analysis.cpp
+++ clang/test/SemaCXX/warn-range-loop-analysis.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wall -Wno-unused -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wloop-analysis -verify %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -Wrange-loop-analysis -verify %s
 
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -818,11 +818,11 @@
 CharSubscript,
 Comment,
 DeleteNonVirtualDtor,
-ForLoopAnalysis,
 Format,
 Implicit,
 InfiniteRecursion,
 IntInBoolContext,
+LoopAnalysis,
 MismatchedTags,
 MissingBraces,
 Move,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rnk, rsmith, aaron.ballman.
Mordante added a project: clang.

This allows the caller of the function to add a note about how to fix the stack 
exhaustion. This can be useful to aid the user. For example it can be used to 
show a help message for https://bugs.llvm.org/show_bug.cgi?id=14030


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69478

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -386,17 +386,21 @@
   SemaPPCallbackHandler->reset();
 }
 
-void Sema::warnStackExhausted(SourceLocation Loc) {
+void Sema::warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID) {
   // Only warn about this once.
   if (!WarnedStackExhausted) {
 Diag(Loc, diag::warn_stack_exhausted);
+if (NoteDiagID)
+  Diag(Loc, NoteDiagID);
 WarnedStackExhausted = true;
   }
 }
 
 void Sema::runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn) {
-  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID) {
+  clang::runWithSufficientStackSpace(
+  [&] { warnStackExhausted(Loc, NoteDiagID); }, Fn);
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1307,14 +1307,17 @@
   void PrintStats() const;
 
   /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
+  void warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID = 0);
 
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
   /// in template instantiation) to avoid stack overflow.
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
   void runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID = 0);
 
   /// Helper class that creates diagnostics with optional
   /// template instantiation stacks.


Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -386,17 +386,21 @@
   SemaPPCallbackHandler->reset();
 }
 
-void Sema::warnStackExhausted(SourceLocation Loc) {
+void Sema::warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID) {
   // Only warn about this once.
   if (!WarnedStackExhausted) {
 Diag(Loc, diag::warn_stack_exhausted);
+if (NoteDiagID)
+  Diag(Loc, NoteDiagID);
 WarnedStackExhausted = true;
   }
 }
 
 void Sema::runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn) {
-  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID) {
+  clang::runWithSufficientStackSpace(
+  [&] { warnStackExhausted(Loc, NoteDiagID); }, Fn);
 }
 
 /// makeUnavailableInSystemHeader - There is an error in the current
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -1307,14 +1307,17 @@
   void PrintStats() const;
 
   /// Warn that the stack is nearly exhausted.
-  void warnStackExhausted(SourceLocation Loc);
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
+  void warnStackExhausted(SourceLocation Loc, unsigned NoteDiagID = 0);
 
   /// Run some code with "sufficient" stack space. (Currently, at least 256K is
   /// guaranteed). Produces a warning if we're low on stack space and allocates
   /// more in that case. Use this in code that may recurse deeply (for example,
   /// in template instantiation) to avoid stack overflow.
+  /// \param NoteDiagID Extra information of the user how to resolve the issue.
   void runWithSufficientStackSpace(SourceLocation Loc,
-   llvm::function_ref Fn);
+   llvm::function_ref Fn,
+   unsigned NoteDiagID = 0);
 
   /// Helper class that creates diagnostics with optional
   /// template instantiation stacks.
__

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rnk, rsmith, aaron.ballman.
Mordante added a project: clang.

When a long set of expressions is chained it may overflow the stack. This warns 
about the issue.

Note I'm not sure whether `AnalyzeImplicitConversions` is the best place to add 
this test, but it was the best I could find. Also not sure what the naming 
convention for these helpers is. Another option is to put the original code in 
a lambda in `AnalyzeImplicitConversions` and thus remove the extra function.

Fixes https://bugs.llvm.org/show_bug.cgi?id=14030

Depends on D69478  (if this patch is unwanted 
it is possible to remove the dependency.)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69479

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/stack-overflow-expr-bool.cpp
  clang/test/Sema/stack-overflow-expr-double.c
  clang/test/Sema/stack-overflow-expr-int.c

Index: clang/test/Sema/stack-overflow-expr-int.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-int.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1 +
+void foo() {
+  int i = A1 1;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-double.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-double.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1. +
+void foo() {
+  double f = A1 1.;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-bool.cpp
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG true &&
+void foo() {
+  bool b = A1 true;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,8 +10368,8 @@
   IsSameFloatAfterCast(value.getComplexFloatI

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

The use case and its test are in D69479 . Is 
that sufficient?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69478



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

[PATCH] D69481: [Sema] Fixes templated friend member assertion

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rjmccall, rsmith, aaron.ballman.
Mordante added a project: clang.
Mordante added a subscriber: rtrieu.

Fixes https://bugs.llvm.org/show_bug.cgi?id=41792


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69481

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp


Index: clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
===
--- clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
+++ clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
@@ -3,7 +3,7 @@
 void f(T);
 
 template
-struct A { };
+struct A { }; // expected-note{{template is declared here}}
 
 struct X {
   template<> friend void f(int); // expected-error{{in a friend}}
@@ -12,3 +12,12 @@
   friend void f(float); // okay
   friend class A; // okay
 };
+
+struct PR41792 {
+  // expected-error@+1{{cannot declare an explicit specialization in a friend}}
+  template <> friend void f<>(int);
+
+  // expected-error@+2{{template specialization declaration cannot be a 
friend}}
+  // expected-error@+1{{too few template arguments for class template 'A'}}
+  template <> friend class A<>;
+};
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2549,6 +2549,9 @@
 /// list.
 static bool
 DependsOnTemplateParameters(QualType T, TemplateParameterList *Params) {
+  if (!Params->size())
+return false;
+
   DependencyChecker Checker(Params, /*IgnoreNonTypeDependent*/false);
   Checker.TraverseType(T);
   return Checker.Match;


Index: clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
===
--- clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
+++ clang/test/CXX/temp/temp.spec/temp.expl.spec/p20.cpp
@@ -3,7 +3,7 @@
 void f(T);
 
 template
-struct A { };
+struct A { }; // expected-note{{template is declared here}}
 
 struct X {
   template<> friend void f(int); // expected-error{{in a friend}}
@@ -12,3 +12,12 @@
   friend void f(float); // okay
   friend class A; // okay
 };
+
+struct PR41792 {
+  // expected-error@+1{{cannot declare an explicit specialization in a friend}}
+  template <> friend void f<>(int);
+
+  // expected-error@+2{{template specialization declaration cannot be a friend}}
+  // expected-error@+1{{too few template arguments for class template 'A'}}
+  template <> friend class A<>;
+};
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -2549,6 +2549,9 @@
 /// list.
 static bool
 DependsOnTemplateParameters(QualType T, TemplateParameterList *Params) {
+  if (!Params->size())
+return false;
+
   DependencyChecker Checker(Params, /*IgnoreNonTypeDependent*/false);
   Checker.TraverseType(T);
   return Checker.Match;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 3 inline comments as done.
Mordante added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:12349
   // This is not the right CC for (e.g.) a variable initialization.
-  AnalyzeImplicitConversions(*this, E, CC);
+  analyzeImplicitConversionsWithSufficientStackSpace(*this, E, CC);
 }

xbolva00 wrote:
> xbolva00 wrote:
> > Please do not change function names.
> Ah, you had to change it.
> 
> Can this be solved maybe via new bool argument for 
> AnalyzeImplicitConversions(.., arg)? If arg is false, we could just call 
> S.runWithSufficientStackSpace(AnalyzeImplicitConversions(, true)), 
> otherwise continue in normal flow.
I like this idea better than my original approach. Thanks for the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69479



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

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 226590.
Mordante marked an inline comment as done.
Mordante added a comment.

Add an extra argument to `AnalyzeImplicitConversions` as suggested by @xbolva00 
.


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

https://reviews.llvm.org/D69479

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/stack-overflow-expr-bool.cpp
  clang/test/Sema/stack-overflow-expr-double.c
  clang/test/Sema/stack-overflow-expr-int.c

Index: clang/test/Sema/stack-overflow-expr-int.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-int.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1 +
+void foo() {
+  int i = A1 1;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-double.c
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-double.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG 1. +
+void foo() {
+  double f = A1 1.;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-bool.cpp
===
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-bool.cpp
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A10 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A100 A1 A1 A1 A1 A1 A1 A1 A1 A1 A1
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+
+#define ARG true &&
+void foo() {
+  bool b = A1 true;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,8 +10368,10 @@
   IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
 }
 
-static void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
-   bool IsListInit = false);
+static void
+AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
+   bool IsListInit = false,
+   bool CalledWithSufficientStackSpace = false);
 
 static bool IsEnumConstOrFromMacro(Sema &S, Expr *E) {
   // Suppress cases where we are comparing again

[PATCH] D69478: [Sema] Allow warnStackExhausted to show more info

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

@rsmith also doesn't like the approach `runWithSufficientStackSpace` in D69479 
. So I'll try to use a different approach. If 
that succeeds I have no use case for this patch and will probably abandon it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69478



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

[PATCH] D69479: [Sema] Warn about possible stack exhaution

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I'll look at a worklist first, before fixing the other issues. However it seems 
it's not as trivial as I hoped. The recursion occurs in the `SequenceChecker` 
which has a `WorkList` but that's not used for this part. I'll try to find a 
solution.


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

https://reviews.llvm.org/D69479



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

[PATCH] D68912: Adds -Wrange-loop-analysis to -Wall

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D68912#1723722 , @aaron.ballman 
wrote:

> In D68912#1723691 , @xbolva00 wrote:
>
> > >> Does this analysis require CFG support
> >
> > https://reviews.llvm.org/D69292 also requires CFG support.
>
>
> Yes, it does.
>
> > Generally, is CFG support ok for -Wall if the warning has few false 
> > positives?
>
> I think it also depends on the performance of the diagnostic.
>
> >>> I am wondering if the diagnostic was kept out of -Wall for a reason.
> > 
> > Yes, it would be good to find why this was disabled (never enabled?).


I don't know, @rtrieu originally added the warnings, maybe he can answer that 
question.

>> @Mordante can you compile LLVM itself with this patch?

I compiled LLVM and Clang and it adds 145 unique warnings. I wouldn't mind to 
fix those and the other subprojects, so I can test whether the tests have false 
positives.
Am I correct to assume these patches all need to be reviewed?

> I'd be curious to know how the timing changes between compilations as well; 
> does this regress performance considerably?

I did a partial build of LLVM the timings are quite similar:

Before:
real7m44.918s
user14m41.052s
sys 0m16.560s

After:
real7m52.934s
user14m39.968s
sys 0m16.932s


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

https://reviews.llvm.org/D68912



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

[PATCH] D71142: [Sema] Validate large bitfields

[Mark de Wever via Phabricator via cfe-commits]

Mordante planned changes to this revision.
Mordante added a comment.

In D71142#1967409 , @aaron.ballman 
wrote:

> In D71142#1967315 , @xbolva00 wrote:
>
> > Any comments?
> >
> > @rsmith @aaron.ballman
>
>
> There are outstanding review comments, so I'm waiting for the patch author to 
> address those, unless I've missed a question somewhere?


No I still need to address some issues, but I haven't found time for it yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71142



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

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rjmccall.
Mordante added a project: clang.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a reviewer: aaron.ballman.
Herald added a project: LLVM.
Mordante requested review of this revision.

This contains the initial part of the implementation for the C++20 likelihood 
attributes.
For now it only handles them in an IfStmt, I want to add support for other 
statements after this one is done.

I was unsure whether it's preferred to have one patch for both the Sema and 
CodeGen part. If wanted I can split them easily.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85091

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticGroups.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/ParseDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/CodeGenCXX/attr-likelihood-if-branch-weights.cpp
  clang/test/Misc/warning-wall.c
  clang/test/Preprocessor/has_attribute.cpp
  clang/test/SemaCXX/attr-likelihood.cpp
  clang/www/cxx_status.html
  llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
  llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp

Index: llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
===
--- llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
+++ llvm/lib/Transforms/Scalar/LowerExpectIntrinsic.cpp
@@ -24,7 +24,6 @@
 #include "llvm/IR/Metadata.h"
 #include "llvm/InitializePasses.h"
 #include "llvm/Pass.h"
-#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Transforms/Scalar.h"
 #include "llvm/Transforms/Utils/MisExpect.h"
@@ -48,10 +47,10 @@
 // 'select' instructions. It may be worthwhile to hoist these values to some
 // shared space, so they can be used directly by other passes.
 
-static cl::opt LikelyBranchWeight(
+cl::opt llvm::LikelyBranchWeight(
 "likely-branch-weight", cl::Hidden, cl::init(2000),
 cl::desc("Weight of the branch likely to be taken (default = 2000)"));
-static cl::opt UnlikelyBranchWeight(
+cl::opt llvm::UnlikelyBranchWeight(
 "unlikely-branch-weight", cl::Hidden, cl::init(1),
 cl::desc("Weight of the branch unlikely to be taken (default = 1)"));
 
Index: llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
===
--- llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
+++ llvm/include/llvm/Transforms/Scalar/LowerExpectIntrinsic.h
@@ -17,6 +17,7 @@
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/PassManager.h"
+#include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 
@@ -31,6 +32,8 @@
   PreservedAnalyses run(Function &F, FunctionAnalysisManager &);
 };
 
+extern cl::opt LikelyBranchWeight;
+extern cl::opt UnlikelyBranchWeight;
 }
 
 #endif
Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -987,7 +987,7 @@
 
   [[likely]] and [[unlikely]] attributes
   https://wg21.link/p0479r5";>P0479R5
-  No
+  Clang 12 (partial)
 
 
   typename optional in more contexts
Index: clang/test/SemaCXX/attr-likelihood.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/attr-likelihood.cpp
@@ -0,0 +1,101 @@
+// RUN: %clang_cc1 %s -verify -Wlikelihood-attribute-if
+// RUN: %clang_cc1 %s -verify -Wall
+// RUN: %clang_cc1 %s -DPEDANTIC -pedantic -verify -Wlikelihood-attribute-if
+
+#if PEDANTIC
+void g()
+{
+	if(true)
+		[[likely]] {}   // expected-warning {{use of the 'likely' attribute is a C++20 extension}}
+	else
+		[[unlikely]] {} // expected-warning {{use of the 'unlikely' attribute is a C++20 extension}}
+}
+#else
+void a()
+{
+	if(true)
+		[[likely]]   // expected-note {{attribute 'likely' declarered here}}
+		[[unlikely]] // expected-error {{incompatible attributes 'unlikely' and 'likely'}}
+		;
+}
+
+void b()
+{
+	if(true)
+		[[unlikely]] // expected-note {{attribute 'unlikely' declarered here}}
+		[[likely]]   // expected-error {{incompatible attributes 'likely' and 'unlikely'}}
+		;
+}
+
+void c()
+{
+	if(true) [[likely]] ;
+}
+
+void d()
+{
+	if(true) [[unlikely]] ;
+}
+
+void e()
+{
+	if(true) // expected-warning {{attribute 'likely' found in both true and false branch}}
+		[[likely]] {} // expected-note {{attribute 'likely' in true branch declared here}}
+	else
+		[[likely]] {} // expected-note {{attribute 'likely' in false branch declared here}}
+}
+
+void f()
+{
+	if(true) // expected-warning {{attribute 'unlikely' found in both true and false branch}}
+		[[unlikely]] {} // expected-note {{attribute 'unlikely' in true branch declared here}}
+	else
+		[[unli

[PATCH] D72103: [Sema] Avoid using an invalid InsertPos

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 282460.
Mordante added a comment.

Rebased the patch and updated the unit tests.


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

https://reviews.llvm.org/D72103

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp


Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -40,3 +40,68 @@
   template A models;
   template<> struct B models<>; // expected-error {{incomplete type 'struct 
B'}} expected-note {{forward declaration}}
 }
+
+namespace PR43221 {
+// This test crashed before so the main reason for the test is to see that it
+// no longer will crash.
+
+template 
+struct enable_if;
+
+template <>
+struct enable_if { using type = void; };
+
+template 
+constexpr bool always_false = false;
+
+// expected-error@+3 {{invalid operands to binary expression}}
+// expected-note@+2 {{in instantiation of member function}}
+template 
+constexpr bool always_false = T() == T();
+
+// expected-note@+2 {{candidate template ignored: substitution failure}}
+template >::type>
+bool operator==(T, T);
+
+// expected-note@+2 {{candidate template ignored: substitution failure}}
+template >::type>
+void data(T);
+
+template 
+struct S {};
+// expected-note@+2 {{candidate template ignored: could not match}}
+template 
+constexpr bool operator==(S, S) { return false; }
+
+template 
+constexpr bool s = S() == S() and s;
+
+template <>
+constexpr bool s<0> = S<0>() == S<0>();
+
+void test() { s<127>; } // expected-warning {{expression result unused}}
+
+// expected-note@+2 2 {{while substituting deduced template arguments into 
function template}}
+template 
+struct wrapper {
+  wrapper() = default;
+  wrapper(wrapper &&) = default;
+
+  // expected-note@+3 {{in instantiation of variable template specialization}}
+  // expected-note@+2 {{during template argument deduction for variable 
template partial specialization}}
+  // expected-note@+2 2 {{in instantiation of default argument}}
+  template >::type>
+  explicit wrapper(U);
+
+  // expected-error@+1 {{no matching function for call to 'data'}}
+  friend auto foo(wrapper w) { data(w.m); }
+
+  int m;
+};
+
+// expected-note@+1 2 {{while declaring the implicit copy constructor for}}
+struct wrapper2 {
+  wrapper m;
+};
+
+} // namespace PR43221
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -4465,6 +4465,17 @@
 }
   }
 
+  // The InsertPos needs to be reloaded:
+  // The call to TemplateSpecializationType::anyDependentTemplateArguments
+  // above may call ASTContext::getSubstTemplateTypeParmType. This function
+  // adds a new element in the FoldingSet InsertPos points at. If the
+  // FoldingSet resizes due to this insertion the 'iterator' is no longer
+  // valid.
+  VarTemplateSpecializationDecl *Spec =
+  Template->findSpecialization(Converted, InsertPos);
+  assert(!Spec && "Reloading InsertPos found a template specialization");
+  (void)Spec;
+
   // 2. Create the canonical declaration.
   // Note that we do not instantiate a definition until we see an odr-use
   // in DoMarkVarDeclReferenced().


Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -40,3 +40,68 @@
   template A models;
   template<> struct B models<>; // expected-error {{incomplete type 'struct B'}} expected-note {{forward declaration}}
 }
+
+namespace PR43221 {
+// This test crashed before so the main reason for the test is to see that it
+// no longer will crash.
+
+template 
+struct enable_if;
+
+template <>
+struct enable_if { using type = void; };
+
+template 
+constexpr bool always_false = false;
+
+// expected-error@+3 {{invalid operands to binary expression}}
+// expected-note@+2 {{in instantiation of member function}}
+template 
+constexpr bool always_false = T() == T();
+
+// expected-note@+2 {{candidate template ignored: substitution failure}}
+template >::type>
+bool operator==(T, T);
+
+// expected-note@+2 {{candidate template ignored: substitution failure}}
+template >::type>
+void data(T);
+
+template 
+struct S {};
+// expected-note@+2 {{candidate template ignored: could not match}}
+template 
+constexpr bool operator==(S, S) { return false; }
+
+template 
+constexpr bool s = S() == S() and s;
+
+template <>
+constexpr bool s<0> = S<0>() == S<0>();
+
+void test() { s<127>; } // expected-warning {{expression result unused}}
+
+// expected-note@+2 2 {{while substituting deduced template arguments into function template}}
+template 
+struct wrapper {
+  wrapper() = default;

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: rsmith, rjmccall.
Mordante added a project: clang.
Mordante requested review of this revision.

Fixes PR46484: Clang crash in clang/lib/Sema/SemaChecking.cpp:10028


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85601

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/conditional-expr.cpp


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,17 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10161,7 +10161,13 @@
   return IntRange(EIT->getNumBits(), EIT->isUnsigned());
 
 const BuiltinType *BT = cast(T);
-assert(BT->isInteger());
+if (!BT->isInteger()) {
+  // This can happen in a conditional expression with a throw statement
+  // C++11 [expr.cond]p2
+  //   If either the second or the third operand has type (cv) void, ...
+  assert(BT->isVoidType());
+  IntRange(1, true /*NonNegative*/);
+}
 
 return IntRange(C.getIntWidth(QualType(T, 0)), BT->isUnsignedInteger());
   }


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,17 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10161,7 +10161,13 @@
   return IntRange(EIT->getNumBits(), EIT->isUnsigned());
 
 const BuiltinType *BT = cast(T);
-assert(BT->isInteger());
+if (!BT->isInteger()) {
+  // This can happen in a conditional expression with a throw statement
+  // C++11 [expr.cond]p2
+  //   If either the second or the third operand has type (cv) void, ...
+  assert(BT->isVoidType());
+  IntRange(1, true /*NonNegative*/);
+}
 
 return IntRange(C.getIntWidth(QualType(T, 0)), BT->isUnsignedInteger());
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: aaron.ballman, dblaikie, rsmith.
Mordante added a project: clang.
Herald added a subscriber: dexonsmith.
Mordante requested review of this revision.

When casting an enumerate with a fixed bool type the casting should use an 
IntegralToBoolean instead of an IntegralCast.

Fixes PR47055: Incorrect codegen for enum with bool underlying type


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85612

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/AST/ast-dump-enum-bool.cpp

Index: clang/test/AST/ast-dump-enum-bool.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-enum-bool.cpp
@@ -0,0 +1,1291 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump=json %s | FileCheck %s
+
+namespace dr2338 { // dr2338: yes
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+} // namespace D
+}
+
+// NOTE: CHECK lines have been autogenerated by gen_ast_dump_json_test.py
+
+
+// CHECK:  "kind": "TranslationUnitDecl",
+// CHECK-NEXT:  "loc": {},
+// CHECK-NEXT:  "range": {
+// CHECK-NEXT:   "begin": {},
+// CHECK-NEXT:   "end": {}
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "TypedefDecl",
+// CHECK-NEXT:"loc": {},
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {},
+// CHECK-NEXT: "end": {}
+// CHECK-NEXT:},
+// CHECK-NEXT:"isImplicit": true,
+// CHECK-NEXT:"name": "__int128_t",
+// CHECK-NEXT:"type": {
+// CHECK-NEXT: "qualType": "__int128"
+// CHECK-NEXT:},
+// CHECK-NEXT:"inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT:  "id": "0x{{.*}}",
+// CHECK-NEXT:  "kind": "BuiltinType",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "__int128"
+// CHECK-NEXT:  }
+// CHECK-NEXT: }
+// CHECK-NEXT:]
+// CHECK-NEXT:   },
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "TypedefDecl",
+// CHECK-NEXT:"loc": {},
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {},
+// CHECK-NEXT: "end": {}
+// CHECK-NEXT:},
+// CHECK-NEXT:"isImplicit": true,
+// CHECK-NEXT:"name": "__uint128_t",
+// CHECK-NEXT:"type": {
+// CHECK-NEXT: "qualType": "unsigned __int128"
+// CHECK-NEXT:},
+// CHECK-NEXT:"inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT:  "id": "0x{{.*}}",
+// CHECK-NEXT:  "kind": "BuiltinType",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "unsigned __int128"
+// CHECK-NEXT:  }
+// CHECK-NEXT: }
+// CHECK-NEXT:]
+// CHECK-NEXT:   },
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "TypedefDecl",
+// CHECK-NEXT:"loc": {},
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {},
+// CHECK-NEXT: "end": {}
+// CHECK-NEXT:},
+// CHECK-NEXT:"isImplicit": true,
+// CHECK-NEXT:"name": "__NSConstantString",
+// CHECK-NEXT:"type": {
+// CHECK-NEXT: "qualType": "__NSConstantString_tag"
+// CHECK-NEXT:},
+// CHECK-NEXT:"inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT:  "id": "0x{{.*}}",
+// CHECK-NEXT:  "kind": "RecordType",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "__NSConstantString_tag"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "decl": {
+// CHECK-NEXT:   "id": "0x{{.*}}",
+// CHECK-NEXT:   "kind": "CXXRecordDecl",
+// CHECK-NEXT:   "name": "__NSConstantString_tag"
+// CHECK-NEXT:  }
+// CHECK-NEXT: }
+// CHECK-NEXT:]
+// CHECK-NEXT:   },
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "TypedefDecl",
+// CHECK-NEXT:"loc": {},
+// CHECK-NEXT:"range": {
+// CHECK-NEXT: "begin": {},
+// CHECK-NEXT: "end": {}
+// CHECK-NEXT:},
+// CHECK-NEXT:"isImplicit": true,
+// CHECK-NEXT:"name": "__builtin_ms_va_list",
+// CHECK-NEXT:"type": {
+// CHECK-NEXT: "qualType": "char *"
+// CHECK-NEXT:},
+// CHECK-NEXT:"inner": [
+// CHECK-NEXT: {
+// CHECK-NEXT:  "id": "0x{{.*}}",
+// CHECK-NEXT:  "kind": "PointerType",
+// CHECK-NEXT:  "type": {
+// CHECK-NEXT:   "qualType": "char *"
+// CHECK-NEXT:  },
+// CHECK-NEXT:  "inner": [
+// CHECK-NEXT:   {
+// CHECK-NEXT:"id": "0x{{.*}}",
+// CHECK-NEXT:"kind": "BuiltinType",
+// CHECK-NEXT:"type": {
+// CHECK-NEXT: "qualType": "char"
+// CHECK-NEXT:}
+// CHECK-NEXT:   }
+// CHECK-NEXT:  ]
+// CHECK-NEXT: }
+// CHECK-NEXT:]
+// CHECK-NEXT:   },
+// CHECK-NEXT: 

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 3 inline comments as done.
Mordante added inline comments.



Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump=json 
%s | FileCheck %s
+

rsmith wrote:
> aaron.ballman wrote:
> > riccibruno wrote:
> > > Why a (pretty unreadable) json test? There is also 
> > > `make-ast-dump-check.sh` to auto-generate an ast dump test.
> > I'd also like to see some CodeGen tests for various fixed underlying types 
> > that ensures the conversion to the underlying type happens before the 
> > conversion to the enumeration.
> > 
> > Also, at least one test showing this behaves properly with a C-style cast 
> > instead of a named cast.
> In addition to CodeGen tests, we should also test that constant evaluation 
> does the right thing.
@riccibruno Thanks for the hint! I wasn't aware of this tool. I've modified the 
AST test.

@aaron.ballman I've added a C-style AST test and CodeGen tests. However I'm not 
sure what you expect to see in the CodeGen tests. As I expected the CodeGen 
doesn't know about the enum, but it just uses the underlying type of the enum. 
Please let me know if you want to see additional tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85612

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

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 284478.
Mordante added a comment.

Addresses the review comments.


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

https://reviews.llvm.org/D85612

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/AST/ast-dump-enum-bool.cpp
  clang/test/CodeGen/enum-bool.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -594,3 +594,16 @@
 }
 
 } // namespace special_ctor
+
+namespace dr2338 {
+namespace B {
+enum E : bool { Zero, One };
+consteval E c(int x) { return (E)x; }
+static_assert(static_cast(c(2)) == 1);
+} // namespace B
+namespace D {
+enum class E : bool { Zero, One };
+consteval E c(int x) { return (E)x; }
+static_assert(static_cast(c(2)) == 1);
+} // namespace D
+} // namespace dr2338
Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,100 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381A1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   ret i32 %0
+// CHECK-NEXT: }
+
+E b(int x) { return (E)x; }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381A1bEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   ret i32 %0
+// CHECK-NEXT: }
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define zeroext i1 @_ZN6dr23381B1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   %tobool = icmp ne i32 %0, 0
+// CHECK-NEXT:   ret i1 %tobool
+// CHECK-NEXT: }
+
+E b(int x) { return (E)x; }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define zeroext i1 @_ZN6dr23381B1bEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   %tobool = icmp ne i32 %0, 0
+// CHECK-NEXT:   ret i1 %tobool
+// CHECK-NEXT: }
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381C1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   ret i32 %0
+// CHECK-NEXT: }
+
+E b(int x) { return (E)x; }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381C1bEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   ret i32 %0
+// CHECK-NEXT: }
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define zeroext i1 @_ZN6dr23381D1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   %tobool = icmp ne i32 %0, 0
+// CHECK-NEXT:   ret i1 %tobool
+// CHECK-NEXT: }
+
+E b(int x) { return (E)x; }
+
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define zeroext i1 @_ZN6dr23381D1bEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   %tobool = icmp ne i32 %0, 0
+// CHECK-NEXT:   ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/AST/ast-dump-enum-bool.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-enum-bool.cpp
@@ -0,0 +1,131 @@
+// RUN: %clang_cc1 -std=c++20 -triple x86

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

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

I added `void g()` since that's valid code which also caused an assertion 
failure. So the issue isn't in the error recovery but in determining the 
required IntRange. It seems the code doesn't take 
http://eel.is/c++draft/expr.cond#2.1 into account.




Comment at: clang/lib/Sema/SemaChecking.cpp:10164
 const BuiltinType *BT = cast(T);
-assert(BT->isInteger());
+if (!BT->isInteger()) {
+  // This can happen in a conditional expression with a throw statement

rsmith wrote:
> Can we handle this in code that's specific to conditional expressions 
> instead? Presumably somewhere higher up in the call graph, some code is 
> assuming that it can recurse from a conditional expression to its 
> subexpressions, and that assumption is wrong.
I can take  a look at it if you want. However I feel this fix is better. If the 
conditional doesn't throw it can properly evaluate the required IntRange. If it 
throws the range doesn't matter, therefore I didn't want to increment the 
required range.
Do you agree?
Should I add more comment to clarify the design decission?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601

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

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

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



Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-unknown-unknown -fsyntax-only 
-ast-dump %s | FileCheck %s
+

rsmith wrote:
> I don't think we really need a dedicated AST test for this. Such tests create 
> a maintenance burden, and they don't really capture what we care about here: 
> that all non-zero values are correctly converted to the `true` value of the 
> enumeration type.
I'll remove the test.



Comment at: clang/test/CodeGen/enum-bool.cpp:7-14
+// CHECK:  ; Function Attrs: noinline nounwind optnone
+// CHECK-NEXT: define i32 @_ZN6dr23381A1aEi(i32 %x) #0 {
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   %x.addr = alloca i32, align 4
+// CHECK-NEXT:   store i32 %x, i32* %x.addr, align 4
+// CHECK-NEXT:   %0 = load i32, i32* %x.addr, align 4
+// CHECK-NEXT:   ret i32 %0

rsmith wrote:
> Some general guidance for writing IR testcases:
> 
>  - Don't test things that aren't relevant to the test case; doing so will 
> make the test brittle as IR generation changes. In particular, don't test the 
> function attribute comments, the `#0` introducing function metadata, 
> instruction names, alignments.
>  - Use `CHECK-LABEL` for each function definition to improve matching 
> semantics and diagnostics on mismatches. (The `CHECK-LABEL`s are checked 
> first, then the input is sliced up into pieces between them, and those pieces 
> are checked independently.)
>  - Don't use `CHECK-NEXT` unless it's relevant to your test that no other 
> instructions appear in between.
Thanks for the information, I'll reduce the test case.



Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:597-609
+
+namespace dr2338 {
+namespace B {
+enum E : bool { Zero, One };
+consteval E c(int x) { return (E)x; }
+static_assert(static_cast(c(2)) == 1);
+} // namespace B

rsmith wrote:
> This DR test should go in `test/CXX/drs/dr23xx.cpp`, along with a suitable 
> "magic comment" `// dr2338: 12` to indicate this DR is implemented in Clang 
> 12 onwards. (We have tooling that generates the 
> `clang.llvm.org/cxx_dr_status.html` page from those magic comments.)
> 
> I don't think this has anything to do with `consteval`; a more-reduced test 
> should work just as well (eg, `static_assert((int)(E)2 == 1, "");`) and would 
> allow us to test this in C++11 onwards, not only in C++20.
I thought the magic comment could be done in any test file. It'll move the test 
and add the comment.


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

https://reviews.llvm.org/D85612

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

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 285685.
Mordante added a comment.

Addresses review comments.


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

https://reviews.llvm.org/D85612

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/test/CodeGen/enum-bool.cpp


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 
+#if __cplusplus >= 201103L
+namespace dr2338 { // dr2338: 12
+namespace B {
+enum E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace B
+namespace D {
+enum class E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace D
+} // namespace dr2338
+#endif
+
 namespace dr2346 { // dr2346: 11
   void test() {
 const int i2 = 0;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -1243,7 +1243,13 @@
   return TC_Failed;
 }
 if (SrcType->isIntegralOrEnumerationType()) {
-  Kind = CK_IntegralCast;
+  // [expr.static.cast]p10 If the enumeration type has a fixed underlying
+  // type, the value is first converted to that type by integral conversion
+  const EnumType *Enum = DestType->getAs();
+  Kind = Enum->getDecl()->isFixed() &&
+ Enum->getDecl()->getIntegerType()->isBooleanType()
+ ? CK_IntegralToBoolean
+ : CK_IntegralCast;
   return TC_Success;
 } else if (SrcType->isRealFloatingType())   {
   Kind = CK_FloatingToIntegral;


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_c

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 3 inline comments as done.
Mordante added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:10317-10320
 IntRange L =
 GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
 IntRange R =
 GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);

rsmith wrote:
> It seems to me that the bug is here. `GetExprRange` is documented as 
> "Pseudo-evaluate the given **integer** expression", but the true and false 
> expressions here might not be integer expressions -- specifically, one of 
> them could be of type `void` if it's a //throw-expression//. In that case, we 
> should only call `GetExprRange` on the other expression. The expression range 
> of the `void`-typed expression is irrelevant in this case, because that 
> expression never returns.
Thanks for the hint I'll look into this solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601

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

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 285704.
Mordante marked an inline comment as done.
Mordante added a comment.

Addresses review comments.


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

https://reviews.llvm.org/D85601

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/conditional-expr.cpp


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10308,10 +10308,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10308,10 +10308,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante planned changes to this revision.
Mordante marked 14 inline comments as done.
Mordante added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

aaron.ballman wrote:
> Hmm, I'm on the fence about specifying `201803` for these attributes. Given 
> that this is only the start of supporting the attribute, do we want to claim 
> it already matches the standard's behavior? Or do we just want to return `1` 
> to signify that we understand this attribute but we don't yet fully support 
> it in common cases (such as on labels in switch statements, etc)?
> 
> As another question, should we consider adding a C2x spelling 
> `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality to C?
I was also somewhat on the fence, for now I'll change it to specify `1`.

I'll have a look at the C2x changes. Just curious do you know whether there's a 
proposal to add this to C2x?



Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It's not allowed to annotate a statement with both ``likely`` and
+``unlikely``.  It's not recommended to annotate both branches of an ``if``
+statement with an attribute.

Quuxplusone wrote:
> aaron.ballman wrote:
> > Why? I would expect this to be reasonable code:
> > ```
> > if (foo) [[likely]] {
> >   ...
> > } else if (bar) [[unlikely]] {
> >   ...
> > } else if (baz) [[likely]] {
> >   ...
> > } else {
> >   ...
> > }
> > ```
> > Especially because I would expect this to be reasonable code:
> > ```
> > switch (value) {
> > [[likely]] case 0: ... break;
> > [[unlikely]] case 1: ... break;
> > [[likely]] case 2: ... break;
> > [[unlikely]] default: ... break;
> > }
> > ```
> > As motivating examples, consider a code generator that knows whether a 
> > particular branch is likely or not and it writes out the attribute on all 
> > branches. Or, something like this:
> > ```
> > float rnd_value = get_super_random_number_between_zero_and_one();
> > if (rnd_value < .1) [[unlikely]] {
> > } else if (rnd_value > .9) [[unlikely]] {
> > } else [[likely]] {
> > }
> > ```
> Right, annotating both/multiple branches of a control statement should be 
> perfectly fine. Even `if (x) [[likely]] { } else [[likely]] { }` should be 
> perfectly okay as far as the code generator is concerned (and we should have 
> a test for that); it's silly, but there's no reason to warn against it in the 
> compiler docs.
> 
> Aaron, notice that `if (x) [[likely]] { } else if (y) [[likely]] { }` is not 
> actually annotating "both" branches of any single `if`-statement. There 
> you're annotating the true branch of an if (without annotating the else 
> branch), and then annotating the true branch of another if (which doesn't 
> have an else branch).
> 
> Mordante, in these docs, please document the "interesting" behavior of the 
> standard attribute on labels — annotating a label is different from 
> annotating the labeled statement itself. In particular,
> 
> [[likely]] case 1: case 2: foo(); break;
> case 3: [[likely]] case 4: foo(); break;
> case 5: case 6: [[likely]] foo(); break;
> 
> indicates that the likely cases are 1, 4, 5, and 6. (1 and 4 because their 
> labels are annotated; 5 and 6 because their control flow passes through an 
> annotated statement. Case 3's control flow passes through an annotated label, 
> but that doesn't matter to the standard attribute.)
Aaron, the current `CodeGen` code for the `switch` statement allows all 
branches to have a weight, for the `if` there are only two weights allowed. As 
Arthur explained the chained `if`s are different statements, so this will work 
properly.

Arthur, I agree we should add the documentation about the `switch`, but I'd 
rather do that when the attributes are implemented for the `switch`. For now I 
think it would make sense to add some documentation about the fact that the 
attribute can be placed inside the `CompoundStmt`.



Comment at: clang/include/clang/Basic/AttrDocs.td:1702
+
+At the moment the attribute is only implemented for an ``if`` statement.
+

aaron.ballman wrote:
> We should document whether we silently accept the attribute in other 
> locations and do nothing with it (I'm not keen on this approach because it 
> surprises users) or that we loudly diagnose the attribute in situations that 
> we don't support it in (I think this is a more user-friendly approach, but 
> may require a separate warning flag).
I'll update the documentation. I intend to continue to work on implementing 
these attributes so I feel adding a temporary diagnostic is not worth the 
effort.



Comment at: clang/include/clang/Basic/AttrDocs.td:1708
+
+  // compile with -Wimplicit-fallthrough
+  if (b) [[likely]] {

aaron.ballman wrote:
> Why does `-Wimplicit-fallthrough` matter?
I

[PATCH] D85601: Fixes an assertion when IntRange processes a throw expression

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG827ba67e3833: [Sema] Validate calls to GetExprRange. 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85601

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/conditional-expr.cpp


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,10 +10368,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 


Index: clang/test/SemaCXX/conditional-expr.cpp
===
--- clang/test/SemaCXX/conditional-expr.cpp
+++ clang/test/SemaCXX/conditional-expr.cpp
@@ -409,3 +409,20 @@
 D d = b ? D{B()} : D{C()};
   }
 }
+
+namespace PR46484 {
+// expected-error@+4{{expected ':'}}
+// expected-note@+3{{to match this '?'}}
+// expected-warning@+2{{variable 'b' is uninitialized}}
+// expected-error@+1 2 {{expected ';' after top level declarator}}
+int a long b = a = b ? throw 0 1
+
+void g() {
+  extern int a;
+  extern long b;
+  long c = a = b ? throw 0 : 1;
+  long d = a = b ? 1 : throw 0;
+  // expected-error@+1 {{assigning to 'int' from incompatible type 'void'}}
+  long e = a = b ? throw 0 : throw 1;
+}
+} // namespace PR46484
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,10 +10368,16 @@
   MaxWidth, InConstantContext);
 
 // Otherwise, conservatively merge.
-IntRange L =
-GetExprRange(C, CO->getTrueExpr(), MaxWidth, InConstantContext);
-IntRange R =
-GetExprRange(C, CO->getFalseExpr(), MaxWidth, InConstantContext);
+// GetExprRange requires an integer expression, but a throw expression
+// results in a void type.
+Expr *E = CO->getTrueExpr();
+IntRange L = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
+E = CO->getFalseExpr();
+IntRange R = E->getType()->isVoidType()
+ ? IntRange{0, true}
+ : GetExprRange(C, E, MaxWidth, InConstantContext);
 return IntRange::join(L, R);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

[Mark de Wever via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfef260712407: [Sema] Use the proper cast for a fixed bool 
enum. (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85612

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/CXX/drs/dr23xx.cpp
  clang/test/CodeGen/enum-bool.cpp


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | 
FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===
--- clang/test/CXX/drs/dr23xx.cpp
+++ clang/test/CXX/drs/dr23xx.cpp
@@ -4,6 +4,19 @@
 // RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 // RUN: %clang_cc1 -std=c++2a %s -verify -fexceptions -fcxx-exceptions 
-pedantic-errors 2>&1 | FileCheck %s
 
+#if __cplusplus >= 201103L
+namespace dr2338 { // dr2338: 12
+namespace B {
+enum E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace B
+namespace D {
+enum class E : bool { Zero, One };
+static_assert((int)(E)2 == 1, "");
+} // namespace D
+} // namespace dr2338
+#endif
+
 namespace dr2346 { // dr2346: 11
   void test() {
 const int i2 = 0;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -1243,7 +1243,13 @@
   return TC_Failed;
 }
 if (SrcType->isIntegralOrEnumerationType()) {
-  Kind = CK_IntegralCast;
+  // [expr.static.cast]p10 If the enumeration type has a fixed underlying
+  // type, the value is first converted to that type by integral conversion
+  const EnumType *Enum = DestType->getAs();
+  Kind = Enum->getDecl()->isFixed() &&
+ Enum->getDecl()->getIntegerType()->isBooleanType()
+ ? CK_IntegralToBoolean
+ : CK_IntegralCast;
   return TC_Success;
 } else if (SrcType->isRealFloatingType())   {
   Kind = CK_FloatingToIntegral;


Index: clang/test/CodeGen/enum-bool.cpp
===
--- /dev/null
+++ clang/test/CodeGen/enum-bool.cpp
@@ -0,0 +1,49 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
+
+namespace dr2338 {
+namespace A {
+enum E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381A1bEi
+// CHECK: ret i32 %0
+
+} // namespace A
+namespace B {
+enum E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381B1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace B
+namespace C {
+enum class E { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1aEi
+// CHECK: ret i32 %0
+
+E b(int x) { return (E)x; }
+// CHECK-LABEL: define i32 @_ZN6dr23381C1bEi
+// CHECK: ret i32 %0
+
+} // namespace C
+namespace D {
+enum class E : bool { Zero, One };
+E a(int x) { return static_cast(x); }
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1aEi
+// CHECK: ret i1 %tobool
+
+E b(int x) { return (E)x; }
+
+// CHECK-LABEL: define zeroext i1 @_ZN6dr23381D1bEi
+// CHECK: ret i1 %tobool
+
+} // namespace D
+} // namespace dr2338
Index: clang/test/CXX/drs/dr23xx.cpp
===

[PATCH] D85091: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in IfStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 14 inline comments as done.
Mordante added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

aaron.ballman wrote:
> Mordante wrote:
> > aaron.ballman wrote:
> > > Hmm, I'm on the fence about specifying `201803` for these attributes. 
> > > Given that this is only the start of supporting the attribute, do we want 
> > > to claim it already matches the standard's behavior? Or do we just want 
> > > to return `1` to signify that we understand this attribute but we don't 
> > > yet fully support it in common cases (such as on labels in switch 
> > > statements, etc)?
> > > 
> > > As another question, should we consider adding a C2x spelling 
> > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality 
> > > to C?
> > I was also somewhat on the fence, for now I'll change it to specify `1`.
> > 
> > I'll have a look at the C2x changes. Just curious do you know whether 
> > there's a proposal to add this to C2x?
> > I'll have a look at the C2x changes. Just curious do you know whether 
> > there's a proposal to add this to C2x?
> 
> There isn't one currently because there is no implementation experience with 
> the attributes in C. This is a way to get that implementation experience so 
> it's easier to propose the feature to WG14.
> I was also somewhat on the fence, for now I'll change it to specify `1`.

There seems to be an issue using the `1` so I used `2` instead. (Didn't want to 
look closely at the issue.)




Comment at: clang/include/clang/Basic/Attr.td:1288
+def Likely : StmtAttr {
+  let Spellings = [CXX11<"", "likely", 201803>];
+  let Documentation = [LikelihoodDocs];

Mordante wrote:
> aaron.ballman wrote:
> > Mordante wrote:
> > > aaron.ballman wrote:
> > > > Hmm, I'm on the fence about specifying `201803` for these attributes. 
> > > > Given that this is only the start of supporting the attribute, do we 
> > > > want to claim it already matches the standard's behavior? Or do we just 
> > > > want to return `1` to signify that we understand this attribute but we 
> > > > don't yet fully support it in common cases (such as on labels in switch 
> > > > statements, etc)?
> > > > 
> > > > As another question, should we consider adding a C2x spelling 
> > > > `[[clang::likely]]` and `[[clang::unlikely]]` to add this functionality 
> > > > to C?
> > > I was also somewhat on the fence, for now I'll change it to specify `1`.
> > > 
> > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > there's a proposal to add this to C2x?
> > > I'll have a look at the C2x changes. Just curious do you know whether 
> > > there's a proposal to add this to C2x?
> > 
> > There isn't one currently because there is no implementation experience 
> > with the attributes in C. This is a way to get that implementation 
> > experience so it's easier to propose the feature to WG14.
> > I was also somewhat on the fence, for now I'll change it to specify `1`.
> 
> There seems to be an issue using the `1` so I used `2` instead. (Didn't want 
> to look closely at the issue.)
> 
> > I'll have a look at the C2x changes. Just curious do you know whether 
> > there's a proposal to add this to C2x?
> 
> There isn't one currently because there is no implementation experience with 
> the attributes in C. This is a way to get that implementation experience so 
> it's easier to propose the feature to WG14.

Nice to know. It seems the C2x wasn't at straightforward as I hoped, so I 
didn't implement it yet. I intend to look at it later. I first want the initial 
part done to see whether this is the right approach.



Comment at: clang/include/clang/Basic/AttrDocs.td:1697
+It's not allowed to annotate a statement with both ``likely`` and
+``unlikely``.  It's not recommended to annotate both branches of an ``if``
+statement with an attribute.

aaron.ballman wrote:
> Mordante wrote:
> > Quuxplusone wrote:
> > > aaron.ballman wrote:
> > > > Why? I would expect this to be reasonable code:
> > > > ```
> > > > if (foo) [[likely]] {
> > > >   ...
> > > > } else if (bar) [[unlikely]] {
> > > >   ...
> > > > } else if (baz) [[likely]] {
> > > >   ...
> > > > } else {
> > > >   ...
> > > > }
> > > > ```
> > > > Especially because I would expect this to be reasonable code:
> > > > ```
> > > > switch (value) {
> > > > [[likely]] case 0: ... break;
> > > > [[unlikely]] case 1: ... break;
> > > > [[likely]] case 2: ... break;
> > > > [[unlikely]] default: ... break;
> > > > }
> > > > ```
> > > > As motivating examples, consider a code generator that knows whether a 
> > > > particular branch is likely or not and it writes out the attribute on 
> > > > all branches. Or, something like this:
> > > > ```
> > > > float rnd_value = get_super_random_number_between_zero_and_one(

[PATCH] D131479: Handle explicitly defaulted consteval special members.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

This change breaks the libc++ test 
`libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp`.
 
(https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)

Looking at the description of this patch it should only affect `consteval` 
functions, but it seems to affect `constexpr` functions too. Can you have a 
look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

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

[PATCH] D131479: Handle explicitly defaulted consteval special members.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D131479#3753533 , @aaron.ballman 
wrote:

> In D131479#3753462 , @Mordante 
> wrote:
>
>> This change breaks the libc++ test 
>> `libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp`.
>>  
>> (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)
>>
>> Looking at the description of this patch it should only affect `consteval` 
>> functions, but it seems to affect `constexpr` functions too. Can you have a 
>> look?
>
> Agreed that we should take a look, but it's interesting to note that 
> libstdc++ seems to have a different behavior than libc++ here: 
> https://godbolt.org/z/6cWhf6GYj (the last three compiler panes show Clang 14 
> libstd++, Clang 14 libc++, and Clang trunk libc++). How sure are you that the 
> libc++ test is actually correct, because (without checking the standard, so 
> take with a giant grain of salt) this seems like it might have fixed a bug in 
> libc++? The release note says this is expected to impact constexpr and 
> consteval default special member functions, so behavioral changes to 
> `constexpr` aren't unexpected (though I agree that the commit message didn't 
> mention `constexpr` so the changes may seem surprising).

Thanks for confirming it's indeed intended to affect `constexpr` too! It was a 
bit confusing since it also referred to C++14, which didn't have `consteval`. I 
wasn't entirely sure about the status of libc++, but since the patch gave of a 
mixed message I wanted make sure that `constexpr` was intended.

I've done some further digging and according to cppreference the paper P0602R4 
was a DR against C++20. This was proposed in the paper.
Looking at the changes in the addressing this paper I see no C++ version checks 
https://reviews.llvm.org/D32385
In a followup introduces tests using C++ version checks 
https://reviews.llvm.org/D54772
The failing test was introduced in 308624127fd6cc36558b6eee4d4ffa4e215a074e 
before the paper was voted in

So I think there's indeed a bug in libc++ which was hidden since Clang hadn't 
implemented some DRs. I will look at the libc++ side what needs to be done to 
fix the CI failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131479

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

[PATCH] D127695: WIP: clang: Implement Template Specialization Resugaring

[Mark de Wever via Phabricator via cfe-commits]

Mordante added subscribers: ldionne, Mordante.
Mordante added inline comments.



Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
+CFLAGS: "-fcrash-diagnostics-dir=crash_diagnostics_dir"
+CXXFLAGS: "-fcrash-diagnostics-dir=crash_diagnostics_dir"
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"

I actually think this change would be nice for libc++ in general. I've has 
several assertion failures in the bootstrapping CI. Especially with the 
symbolizer available the crash reports become a lot better.

I'm not convinced the way you used the `CXXFLAGS` in the CMakeLists.txt is the 
best way, maybe @ldionne has a better suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127695

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for working on this! It's really great to get the crash report as an 
artifact.




Comment at: libcxx/test/libcxx/crash.sh.cpp:15
+
+#pragma clang __debug parser_crash

The libc++ build seems to be green. I assume it was intended to be red so we 
can validate the artifact is available in the CI.



Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"
+CLANG_CRASH_DIAGNOSTICS_DIR: "crash_diagnostics"
 agents:

Are relative directories allowed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

Ideally it would be documented these artifacts are now available. However 
there's no good place in libc++ to document that. I'm working on such a 
document so I will take care of documenting these artifacts.

LGTM from libc++'s PoV.




Comment at: libcxx/utils/ci/buildkite-pipeline.yml:377
 LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"
+CLANG_CRASH_DIAGNOSTICS_DIR: "crash_diagnostics"
 agents:

mizvekov wrote:
> Mordante wrote:
> > Are relative directories allowed?
> Yes, it already worked with the flag.
> 
> You can check that it's working, even though the bootstrapping pipeline is 
> green, it still exports the artifacts.
Thanks I see.

Just a hint you can move this job earlier in this file for testing purposes 
and/or remove jobs you don't want to test. I use this every now and then when I 
have issues with a specific CI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133082

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

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: aaron.ballman, ldionne.
Herald added subscribers: mstorsjo, arphaman.
Herald added a project: All.
Mordante requested review of this revision.
Herald added projects: clang, libc++.
Herald added subscribers: libcxx-commits, cfe-commits.
Herald added a reviewer: libc++.

This documentation aims to make it cleare how the libc++ pre-commit CI
works. For libc++ developers and other LLVM projects whose changes can
affect libc++.

This was discusses with @aaron.ballman as a follow on some unclearities
for the Clang communitee how the libc++ pre-commit CI works.

Note some parts depend on patches under review as commented in the
documentation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133249

Files:
  clang/www/hacking.html
  libcxx/docs/Contributing.rst
  libcxx/docs/index.rst

Index: libcxx/docs/index.rst
===
--- libcxx/docs/index.rst
+++ libcxx/docs/index.rst
@@ -93,6 +93,7 @@
   Further, both projects are apparently abandoned: STLport 5.2.1 was
   released in Oct'08, and STDCXX 4.2.1 in May'08.
 
+.. _SupportedPlatforms:
 
 Platform and Compiler Support
 =
Index: libcxx/docs/Contributing.rst
===
--- libcxx/docs/Contributing.rst
+++ libcxx/docs/Contributing.rst
@@ -84,5 +84,158 @@
 Look for the failed build and select the ``artifacts`` tab. There, download the
 abilist for the platform, e.g.:
 
-* C++20 for the Linux platform.
-* MacOS C++20 for the Apple platform.
+* C++XX for the Linux platform (where XX is the latest C++ version).
+* MacOS X86_64 and MacOS arm64 for the Apple platform.
+
+
+.. comments The new part got longer than expected, would it make sense to move
+   it to its own page?
+
+
+Pre-commit CI
+=
+
+Introduction
+
+
+Unlike eost parts of the LLVM project, libc++ uses a pre-commit CI [#]_. This
+CI is hosted on `Buildkite `__ and
+the build results are visible in the review on Phabricator. Before committing a
+patch please make sure the CI is green.
+
+
+The CI tests libc++ for all :ref:`supported platforms `.
+The build is started for every diff uploaded. A complete CI run takes
+approximately one hour. To reduce the load:
+
+* The build is cancelled when a new diff for the same revision is uploaded.
+* The build is done in several stages and cancelled when a stage fails.
+
+Typically the libc++ jobs use a Ubuntu Docker image. This image contains
+recent `nightly builds `__ of all supported versions of
+Clang and the current version of the ``main`` branch. These versions of Clang
+are used to build libc++ and execute its tests.
+
+Unless specified otherwise the ``main`` version of Clang is used.
+
+Unless specified otherwise the tests are executed for the latest C++
+version; this is the version being "developed" by the C++ committee. CI runners
+using this language version will produce an artifact with a graphviz dot file
+containing the internal dependencies of the Standard headers in libc++.
+
+.. comments D133127 adds the artifact
+
+
+.. note:: Updating the Clang nightly builds in the Docker image is a manual
+   process and is done at an irregular interval. When you need to have the
+   latest nightly build to test recent Clang changes best ask at the
+   ``#libcxx`` channel on
+   `LLVM's Discord server `__.
+
+.. comments ``irregular`` is used on purpose. Committing to a schedule seems
+   unwanted; people might be unavailable to update the image, the nightly
+   builds fail and are not uploaded for multiple days (this is not uncommon).
+
+.. [#] There's `Dev meeting talk `__
+   explaining the benefits of libc++'s pre-commit CI.
+
+Builds
+--
+
+Below a short description of the most interesting CI builds [#]_:
+
+* ``Format`` runs ``clang-format`` and uploads its output as an artifact. At the
+  moment this build is a soft error and doesn't fail the build.
+* ``Generated output`` runs the ``libcxx-generate-files`` build target and
+  tests for non-ASCII characters in libcxx. Some files are excluded since they
+  use Unicode, mainly tests. The output of these commands are uploaded as
+  artifact.
+* ``Documentation`` builds the documentation. (This is done early in the build
+  process since it is cheap to run.)
+* ``C++XX`` these build steps test the various C++ versions, making sure all
+  C++ language versions work with the changes made.
+* ``Clang XX`` these build steps test whether the changes work with all
+  supported Clang versions.
+* ``Booststrapping build`` builds Clang using the revision of the patch and
+  uses that Clang version to build and test libc++. This validates the current
+  Clang and lib++ are compatible. When making changes in Clang that affect
+  libc++ t

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for the review comments. I'll address them in a little while. I want to 
wait for more review comments.




Comment at: clang/www/hacking.html:302
+
+  For most builds, the pre-commit CI uses a recent
+  https://apt.llvm.org/";>nightly build of Clang from LLVM's main

mstorsjo wrote:
> I'm wondering if there's a better synonym for "builds" here which would be 
> clearer? Initially I read this as if "most times a patch is run through the 
> CI", not "most of the configurations in the CI run". So maybe 
> "configurations"?
Agreed configurations seems more appropriate in this context.



Comment at: libcxx/docs/Contributing.rst:119
+
+Unless specified otherwise the ``main`` version of Clang is used.
+

mstorsjo wrote:
> I don't understand this paragraph - each CI run is run through the configured 
> set of supported Clang versions - not only `main`? Or does this talk about 
> specifics about manually running tests with the Docker image?
This is the version of Clang in the main branch. How about:
`Unless specified otherwise the nightly build of Clang from the ``main`` branch 
is used.`



Comment at: libcxx/docs/Contributing.rst:158
+* ``Clang XX`` these build steps test whether the changes work with all
+  supported Clang versions.
+* ``Booststrapping build`` builds Clang using the revision of the patch and

mstorsjo wrote:
> Side note - these tests only test one specific version of C++ with these 
> versions of Clang - we don't have fully coverage of all standard versions 
> with all supported versions of Clang. (Not really sure if that's worth 
> mentioning though. Also I do see this kinda hinted at at the end of the file.)
We might spell it our more explicitly. But maybe not here but where the hints 
are. There are a lot of possible permutations not tested. For example the 
modular build is only tested with C++2b.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 8 inline comments as done.
Mordante added a comment.

Thanks for all review comments! I'm a bit out of time so I will look at the 
other comments later.




Comment at: clang/www/hacking.html:295-296
+  directory will cause the update of the diff to start a CI run. This dummy
+  file will also add the libc++ group to the list of reviewers. The status of
+  the build will be available in Phabricator.
+

philnik wrote:
> aaron.ballman wrote:
> > I am guessing the dummy file should then be removed before landing the 
> > commit and we should document that?
> > 
> > (FWIW, adding a dummy file feels really unclean as a design -- it's 
> > mysterious behavior for new contributors, which the documentation helps 
> > with a bit, but also it's a risk for checking in files that are never 
> > intended to be in the tree. If there's a way to improve this somehow so we 
> > don't need to trick the precommit CI into running, that would be really 
> > nice and totally outside of the scope of this review.)
> It would be great if just the bootstrapping build could be run on all clang 
> reviews. That should show most problems with changes in clang. If there are 
> any problems in libc++, fixing them would result in a full CI run. Having 
> libc++ run against the patch would probably also be awesome as a smoke test 
> for any clang changes.
+1 to what @ldionne said.

One of the advantages would be that it's possible to test Clang against the 
libc++ code base using different language versions, by defining multiple test 
runs using different language versions. (Slightly related to 
https://discourse.llvm.org/t/lit-run-a-run-line-multiple-times-with-different-replacements/64932)
 @aaron.ballman is looking at extending the Clang pre-commit CI something worth 
looking into?





Comment at: clang/www/hacking.html:300
+  CI lacks the resources to build every Clang diff. A single run takes about
+  one hour. Secondly most changes of Clang won't affect libc++.
+

aaron.ballman wrote:
> philnik wrote:
> > Having a secondly without a firstly seems weird.
> +1 to the "secondly" being a bit weird, but also pushing back a bit on the 
> assertion that most changes in Clang won't affect libc++: we change the 
> output of Clang's diagnostics on an almost-daily basis.
> 
> I think it's honest to say: "It is difficult to determine which changes in 
> Clang will affect libc++ and running the precommit CI in all circumstances is 
> prohibitively resource intensive given how actively the project is updated." 
> or something along those lines.
I based my statement on the observations I made:
- I don't see a lot of Clang changes with a dummy file.
- Clang doesn't break libc++ often. (Still it would be good to get the number 
down).
Libc++ uses diagnostics, but mainly to validate diagnostics for ill-formed 
code. So that probably explains why not every diagnostic is an issue. But 
changing the diagnostic of something like a `static_assert` will be detected.

I've replaced the secondly sentence with `Unfortunately it is difficult to 
determine which changes in Clang will affect libc++.`

I want to keep the other two sentences since it explains why an opt-in is 
needed.



Comment at: clang/www/hacking.html:311-312
+  Unlike Clang, libc++ supports multiple versions of Clang. Therefore when a
+  patch changes the diagnostics it might be required to use a regex in the
+  "expected" tests to make it pass the CI.
+

philnik wrote:
> aaron.ballman wrote:
> > Should we document the expectation that breaking libc++ due to conforming 
> > changes in Clang (in terms of diagnostics and bug fixes, not so much in 
> > terms of introducing new language extensions) are generally the 
> > responsibility of libc++ maintainers to address, but Clang contributors 
> > should attempt to reduce disruptions as much as possible by collaborating 
> > with libc++ maintainers when this situation comes up?
> That's definitely a good idea. Maybe mention that clang contributors should 
> ping the #libc group to get the attention of libc++ contributors so we can 
> prepare a follow-up patch or, if the author is comfortable with it, also fix 
> libc++ in the same patch (and hopefully get fast approval). Most breakages 
> should be relatively simple to fix.
I like the idea to collaborate more.

I want to give this some thought and especially how to word it. I want to avoid 
that it's seen as a carte blanche to commit Clang changes which break libc++. 
This would mean HEAD is broken and that's a huge issue for downstream users 
like Google who tend to follow HEAD closely.

I would prefer to use commandeering and instead of two patches where one leave 
HEAD in a broken state. Even if it's short it would make bi-secting harder. 
Personally I really dislike the idea of having commits that knowingly leave a 
repository in a broken state. It also doesn't align with the LLVM d

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 459347.
Mordante marked 15 inline comments as done.
Mordante added a comment.

Addresses review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

Files:
  clang/www/hacking.html
  libcxx/docs/Contributing.rst
  libcxx/docs/index.rst

Index: libcxx/docs/index.rst
===
--- libcxx/docs/index.rst
+++ libcxx/docs/index.rst
@@ -93,6 +93,7 @@
   Further, both projects are apparently abandoned: STLport 5.2.1 was
   released in Oct'08, and STDCXX 4.2.1 in May'08.
 
+.. _SupportedPlatforms:
 
 Platform and Compiler Support
 =
Index: libcxx/docs/Contributing.rst
===
--- libcxx/docs/Contributing.rst
+++ libcxx/docs/Contributing.rst
@@ -84,5 +84,151 @@
 Look for the failed build and select the ``artifacts`` tab. There, download the
 abilist for the platform, e.g.:
 
-* C++20 for the Linux platform.
-* MacOS C++20 for the Apple platform.
+* C++.
+* MacOS X86_64 and MacOS arm64 for the Apple platform.
+
+
+.. comments The new part got longer than expected, would it make sense to move
+   it to its own page?
+
+
+Pre-commit CI
+=
+
+Introduction
+
+
+Unlike most parts of the LLVM project, libc++ uses a pre-commit CI [#]_. This
+CI is hosted on `Buildkite `__ and
+the build results are visible in the review on Phabricator. Before committing a
+patch please make sure the CI is green.
+
+
+The CI tests libc++ for all :ref:`supported platforms `.
+The build is started for every diff uploaded. A complete CI run takes
+approximately one hour. To reduce the load:
+
+* The build is cancelled when a new diff for the same revision is uploaded.
+* The build is done in several stages and cancelled when a stage fails.
+
+Typically, the libc++ jobs use a Ubuntu Docker image. This image contains
+recent `nightly builds `__ of all supported versions of
+Clang and the current version of the ``main`` branch. These versions of Clang
+are used to build libc++ and execute its tests.
+
+Unless specified otherwise, the configurations:
+
+* use a nightly build of the ``main`` branch of Clang,
+* execute the tests using the language C++. This is the version
+  "developed" by the C++ committee.
+
+.. note:: Updating the Clang nightly builds in the Docker image is a manual
+   process and is done at an irregular interval. When you need to have the
+   latest nightly build to test recent Clang changes best ask at the
+   ``#libcxx`` channel on
+   `LLVM's Discord server `__.
+
+.. comments ``irregular`` is used on purpose. Committing to a schedule seems
+   unwanted; people might be unavailable to update the image, the nightly
+   builds fail and are not uploaded for multiple days (this is not uncommon).
+
+.. [#] There's `LLVM Dev Meeting talk `__
+   explaining the benefits of libc++'s pre-commit CI.
+
+Builds
+--
+
+Below is a short description of the most interesting CI builds [#]_:
+
+* ``Format`` runs ``clang-format`` and uploads its output as an artifact. At the
+  moment this build is a soft error and doesn't fail the build.
+* ``Generated output`` runs the ``libcxx-generate-files`` build target and
+  tests for non-ASCII characters in libcxx. Some files are excluded since they
+  use Unicode, mainly tests. The output of these commands are uploaded as
+  artifact.
+* ``Documentation`` builds the documentation. (This is done early in the build
+  process since it is cheap to run.)
+* ``C++`` these build steps test the various C++ versions, making sure all
+  C++ language versions work with the changes made.
+* ``Clang `` these build steps test whether the changes work with all
+  supported Clang versions.
+* ``Booststrapping build`` builds Clang using the revision of the patch and
+  uses that Clang version to build and test libc++. This validates the current
+  Clang and lib++ are compatible. When making changes in Clang that affect
+  libc++ this is the build to validate all is well.
+
+  When a crash occurs in this build, the crash reproducer is available as an
+  artifact.
+
+* ``Modular build`` tests libc++ using Clang modules [#]_.
+* ``GCC `` tests libc++ with the latest stable GCC version. Only C++11
+  and the latest C++ version are tested.
+* ``Santitizers`` tests libc++ using the Clang sanitizers.
+* ``Parts disabled`` tests libc++ with certain libc++ features disabled.
+* ``Windows`` tests libc++ using MinGW and clang-cl.
+* ``Apple`` tests libc++ on MacOS.
+* ``ARM`` tests libc++ on various Linux ARM platforms.
+* ``AIX`` tests libc++ on AIX.
+
+.. [#] Not all all steps are listed: steps are added and removed when the need
+   arises.
+.. [#] Clang modules are not the sa

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: libcxx/docs/Contributing.rst:88
+* C++XX for the Linux platform (where XX is the latest C++ version).
+* MacOS X86_64 and MacOS arm64 for the Apple platform.
+

philnik wrote:
> 
They are synonyms, since use the underscore in the CI I prefer to use that name.



Comment at: libcxx/docs/Contributing.rst:119
+
+Unless specified otherwise the ``main`` version of Clang is used.
+

aaron.ballman wrote:
> mstorsjo wrote:
> > Mordante wrote:
> > > mstorsjo wrote:
> > > > I don't understand this paragraph - each CI run is run through the 
> > > > configured set of supported Clang versions - not only `main`? Or does 
> > > > this talk about specifics about manually running tests with the Docker 
> > > > image?
> > > This is the version of Clang in the main branch. How about:
> > > `Unless specified otherwise the nightly build of Clang from the ``main`` 
> > > branch is used.`
> > I still don't quite understand what this tries to say. "Unless specified 
> > otherwise" - where would I specify a different preference, when I upload a 
> > patch and it gets run through CI?
> > 
> > There's three different concepts involved here:
> > - The CI build matrix (buildkite-pipeline.yml) which specifies a bunch of 
> > different versions of tools and standards to run the tests on. Here you 
> > can't specify a different preference - all of them are run (unless you edit 
> > the file in your patch).
> > - The Docker images, where the default `clang` executable is a recent 
> > nightly build, but older releases are available as `clang-`
> > - The `run-buildbot` script, which just uses whatever compiler is set as 
> > default (via e.g. the `CXX` env var).
> > 
> > So, what about these three concepts is this paragraph trying to say - the 
> > second or the third point? This really needs to specify what it talks about 
> > - build matrix, docker images or run-buildbot script.
> > 
> > The same thing goes for the following paragraph.
> (mostly trying to get rid of the duplicate "Unless specified otherwise".)
I've reworded it, please have a look whether the new wording is clear.



Comment at: libcxx/docs/Contributing.rst:142
+
+Builds
+--

ldionne wrote:
> It feels like this whole section might be prone to becoming out of date. I'm 
> not sure how useful it is since the jobs are pretty self-explanatory. Perhaps 
> it is sufficient to link to `run-buildbot` and `buildkite-pipeline.yml`?
The documentation for these files is already available in this document, more 
at the end.

I tried to mainly describe the special parts and tried to keep it generic. I 
could prune a bit more, but I feel some explanation would be good since there 
are some "suprises" like, formatting allowed to fail, modular build has nothing 
to do with C++20 modules.



Comment at: libcxx/docs/Contributing.rst:231-233
+  export CC=clang-14
+  export CXX=clang++-14
+  run-buildbot generic-cxx17

philnik wrote:
> Complete nit, but I think `CC=clang-14 CXX=clang++-14 run-builtbot 
> generic-cxx17` as an example would be better to avoid polluting people's 
> environment if they're unfamiliar with a terminal.
Good point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Mark de Wever via Phabricator via cfe-commits]

Mordante updated this revision to Diff 459351.
Mordante added a comment.

Rebased to fix CI failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

Files:
  clang/www/hacking.html
  libcxx/docs/Contributing.rst
  libcxx/docs/index.rst

Index: libcxx/docs/index.rst
===
--- libcxx/docs/index.rst
+++ libcxx/docs/index.rst
@@ -93,6 +93,7 @@
   Further, both projects are apparently abandoned: STLport 5.2.1 was
   released in Oct'08, and STDCXX 4.2.1 in May'08.
 
+.. _SupportedPlatforms:
 
 Platform and Compiler Support
 =
Index: libcxx/docs/Contributing.rst
===
--- libcxx/docs/Contributing.rst
+++ libcxx/docs/Contributing.rst
@@ -84,5 +84,151 @@
 Look for the failed build and select the ``artifacts`` tab. There, download the
 abilist for the platform, e.g.:
 
-* C++20 for the Linux platform.
-* MacOS C++20 for the Apple platform.
+* C++.
+* MacOS X86_64 and MacOS arm64 for the Apple platform.
+
+
+.. comments The new part got longer than expected, would it make sense to move
+   it to its own page?
+
+
+Pre-commit CI
+=
+
+Introduction
+
+
+Unlike most parts of the LLVM project, libc++ uses a pre-commit CI [#]_. This
+CI is hosted on `Buildkite `__ and
+the build results are visible in the review on Phabricator. Before committing a
+patch please make sure the CI is green.
+
+
+The CI tests libc++ for all :ref:`supported platforms `.
+The build is started for every diff uploaded. A complete CI run takes
+approximately one hour. To reduce the load:
+
+* The build is cancelled when a new diff for the same revision is uploaded.
+* The build is done in several stages and cancelled when a stage fails.
+
+Typically, the libc++ jobs use a Ubuntu Docker image. This image contains
+recent `nightly builds `__ of all supported versions of
+Clang and the current version of the ``main`` branch. These versions of Clang
+are used to build libc++ and execute its tests.
+
+Unless specified otherwise, the configurations:
+
+* use a nightly build of the ``main`` branch of Clang,
+* execute the tests using the language C++. This is the version
+  "developed" by the C++ committee.
+
+.. note:: Updating the Clang nightly builds in the Docker image is a manual
+   process and is done at an irregular interval. When you need to have the
+   latest nightly build to test recent Clang changes best ask at the
+   ``#libcxx`` channel on
+   `LLVM's Discord server `__.
+
+.. comments ``irregular`` is used on purpose. Committing to a schedule seems
+   unwanted; people might be unavailable to update the image, the nightly
+   builds fail and are not uploaded for multiple days (this is not uncommon).
+
+.. [#] There's `LLVM Dev Meeting talk `__
+   explaining the benefits of libc++'s pre-commit CI.
+
+Builds
+--
+
+Below is a short description of the most interesting CI builds [#]_:
+
+* ``Format`` runs ``clang-format`` and uploads its output as an artifact. At the
+  moment this build is a soft error and doesn't fail the build.
+* ``Generated output`` runs the ``libcxx-generate-files`` build target and
+  tests for non-ASCII characters in libcxx. Some files are excluded since they
+  use Unicode, mainly tests. The output of these commands are uploaded as
+  artifact.
+* ``Documentation`` builds the documentation. (This is done early in the build
+  process since it is cheap to run.)
+* ``C++`` these build steps test the various C++ versions, making sure all
+  C++ language versions work with the changes made.
+* ``Clang `` these build steps test whether the changes work with all
+  supported Clang versions.
+* ``Booststrapping build`` builds Clang using the revision of the patch and
+  uses that Clang version to build and test libc++. This validates the current
+  Clang and lib++ are compatible. When making changes in Clang that affect
+  libc++ this is the build to validate all is well.
+
+  When a crash occurs in this build, the crash reproducer is available as an
+  artifact.
+
+* ``Modular build`` tests libc++ using Clang modules [#]_.
+* ``GCC `` tests libc++ with the latest stable GCC version. Only C++11
+  and the latest C++ version are tested.
+* ``Santitizers`` tests libc++ using the Clang sanitizers.
+* ``Parts disabled`` tests libc++ with certain libc++ features disabled.
+* ``Windows`` tests libc++ using MinGW and clang-cl.
+* ``Apple`` tests libc++ on MacOS.
+* ``ARM`` tests libc++ on various Linux ARM platforms.
+* ``AIX`` tests libc++ on AIX.
+
+.. [#] Not all all steps are listed: steps are added and removed when the need
+   arises.
+.. [#] Clang modules are not the same as C++20's modules.
+
+Infrastructure
+--

[PATCH] D128927: [libc++] Always build c++experimental.a

[Mark de Wever via Phabricator via cfe-commits]

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

Thanks for working on this! LGTM with some suggestions.




Comment at: libcxx/docs/ReleaseNotes.rst:226
+  experimental features provided by it. Note that vendors are encouraged to 
ship the
+  experimental library now that the compiler provides an ergonomic way to use 
it.

I agree this is a build system change, but I wonder how many users read this 
part and find the hidden gem.
Should we add a note at new features too? Maybe there we can mention format and 
ranges are available with this new switch.
(technically that actually belongs in the next patch, but I wouldn't mind to do 
it in this patch.)



Comment at: libcxx/docs/UsingLibcxx.rst:42
+default because they are neither API nor ABI stable. However, the Clang flag 
``-funstable``
+can be used to turn those features on.
 

I think it would be good to explain what experimental features are. The current 
wording makes it sound the features may not be part of an IS, WP, or TS.

How about something like
```
Libc++ provides implementations of some experimental features. Experimental 
features
are either a TS or a feature in the Standard whose implementation is 
incomplete. Those
features are disabled by default because they are neither API nor ABI stable. 
However,
the Clang flag ``-funstable`` can be used to turn those features on.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128927

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

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks @egorzhdan for working on this, I seem to have overlooked this item in 
the review queue.
SGMT, but it seems the libc++ CI hasn't been triggered with this change. Maybe 
change one file in libc++ to give the CI a spin?




Comment at: clang/docs/ClangCommandLineReference.rst:278
+
+This option enables various language and library features that are either 
experimental (also known as TSes), or have been standardized but are not stable 
yet. This option should not be used in production code, since neither ABI nor 
API stability are guaranteed.
+

I'm not happy with the word "should", I don't want to decide what users should 
or should not do. Several of these experimental features are perfectly usable. 
If you can rebuild your entire software stack ABI and API stability might be 
less of an issue. (The lack of polish of some features might be a reason not to 
use them.)



Comment at: clang/include/clang/Driver/Options.td:1167
+  "This option enables various language and library features that are 
either experimental (also known as TSes),"
+  " or have been standardized but are not stable yet. This option 
should not be used in production code, since "
+  "neither ABI nor API stability are guaranteed">,

Likewise regarding should.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121141

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

[PATCH] D128844: Improve the formatting of static_assert messages

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

In D128844#360 , @aaron.ballman 
wrote:

> So I *think* this LG, but am not certain what will happen when you land it. I 
> think the precommit CI failures are not failures that will happen when 
> testing against the just-built clang.

Most Linux runner use the apt.llvm.org nightly builds and are updated on an 
irregular interval. The Bootstrapping build will use the just-built Clang. So 
when the CI is green all should be fine.

I wanted to suggest a regex, but I see the latest patch already does that.

Thanks for improving the readability of the diagnostics! LGTM when the CI 
passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128844

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

[PATCH] D128927: [libc++] Always build c++experimental.a

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

I had a look at the changes and still LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128927

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

[PATCH] D122423: [Clang][doc] Fix __builtin_assume wording.

[Mark de Wever via Phabricator via cfe-commits]

Mordante created this revision.
Mordante added reviewers: serge-sans-paille, aaron.ballman.
Herald added a project: All.
Mordante requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

D117296  removed wording for 
__builtin_assume, D120205  restored the
wording, but the last sentence was only partly restored. This restores
the rest of the last sentence.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122423

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122423: [Clang][doc] Fix __builtin_assume wording.

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D122423#3406381 , @aaron.ballman 
wrote:

> LGTM, a second time. :-D Sorry for missing that the first time.

No problem, thanks for the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122423

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

[PATCH] D122423: [Clang][doc] Fix __builtin_assume wording.

[Mark de Wever via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc3b672a34cde: [Clang][doc] Fix __builtin_assume wording. 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122423

Files:
  clang/docs/LanguageExtensions.rst


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 


Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -2294,7 +2294,8 @@
 The boolean argument to this function is defined to be true. The optimizer may
 analyze the form of the expression provided as the argument and deduce from
 that information used to optimize the program. If the condition is violated
-during execution, the behavior is undefined. The argument itself is 
+during execution, the behavior is undefined. The argument itself is never
+evaluated, so any side effects of the expression will be discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131062: [docs] Add "C++20 Modules"

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks a lot for working on this!

I wonder whether it would be better to have some of the more technical 
information in a separate file and let this file mainly have an introduction to 
modules in general and how to get started using them in Clang.

I haven't tested the steps yet, but I intend to see whether the examples work 
for me.

I haven't validated whether the module explanation is correct; I'm sure there 
are other reviewers who have a deeper knowledge of modules.




Comment at: clang/docs/CPlusPlus20Modules.rst:11
+
+Modules have a lot of meanings. For the users of clang compiler, modules may
+refer to `Objective-C Modules`, `Clang C++ Modules` (or `Clang Header Modules`,

Clang is capitalized in its documentation.



Comment at: clang/docs/CPlusPlus20Modules.rst:18
+
+There is already a detailed document about clang modules Modules_, it
+should be helpful to read Modules_ if you want to know more about the general

Is `Modules_` intended to be a link?



Comment at: clang/docs/CPlusPlus20Modules.rst:20
+should be helpful to read Modules_ if you want to know more about the general
+idea of modules. But due to the C++20 modules having very different semantics, 
it
+might be more friendly for users who care about C++20 modules only to create a





Comment at: clang/docs/CPlusPlus20Modules.rst:22
+might be more friendly for users who care about C++20 modules only to create a
+new page.
+

IMO we don't need to justify why we add a new page. WDYT?



Comment at: clang/docs/CPlusPlus20Modules.rst:35
+tutorial. The document would only introduce concepts about the the
+structure and building of the project.
+

How about mentioning the state of modules in Clang? In your GitHub repository 
you mention some limitations. (A link to a status page could be an alternative.)



Comment at: clang/docs/CPlusPlus20Modules.rst:56
+
+A module consists of one or multiple module units. A module unit is a special
+translation unit. Every module unit should have a module declaration. The 
syntax





Comment at: clang/docs/CPlusPlus20Modules.rst:79
+A primary module interface unit is a module unit whose module declaration is
+`export module module_name;`. The `module_name` here denotes the name of the
+module. A module should have one and only primary module interface unit.

Did you render the document? I would have expected double backticks.



Comment at: clang/docs/CPlusPlus20Modules.rst:117
+
+Here is a hello world example to show how to use modules.
+

Maybe make this hello world example a bit simpler by not using partitions and 
have a separate example with partitions. This example gives the impression a 
simple "hello modular world" is a lot harder to achieve than a normal "hello 
world".



Comment at: clang/docs/CPlusPlus20Modules.rst:133
+  module;
+  #include 
+  #include 

Does `import ;` work? If not is that a Clang or libc++ limitation?



Comment at: clang/docs/CPlusPlus20Modules.rst:164
+  # Precompiling the module
+  clang++ -std=c++20 interface_part.cppm --precompile -o M-interface_part.pcm
+  clang++ -std=c++20 impl_part.cppm --precompile -fprebuilt-module-path=. -o 
M-impl_part.pcm

Maybe explain what all steps do.



Comment at: clang/docs/CPlusPlus20Modules.rst:178
+
+We would explain the options in the following sections.
+





Comment at: clang/docs/CPlusPlus20Modules.rst:183
+
+Currently, C++20 Modules is enabled automatically if the language standard is 
`-std=c++20`.
+Sadly, we can't enable C++20 Modules now with lower language standard versions 
like coroutines by `-fcoroutines-ts` due to some implementation problems.

I assume `-std=c++2b` works too. Maybe rephrase the wording like this.



Comment at: clang/docs/CPlusPlus20Modules.rst:184
+Currently, C++20 Modules is enabled automatically if the language standard is 
`-std=c++20`.
+Sadly, we can't enable C++20 Modules now with lower language standard versions 
like coroutines by `-fcoroutines-ts` due to some implementation problems.
+The `-fmodules-ts` option is deprecated and is planned to be removed.

I would remove this sentence, it's not too common to backport language features.



Comment at: clang/docs/CPlusPlus20Modules.rst:321
+
+If the user don't want to follow the consistency requirement due to some 
reasons (e.g., distributing module files),
+the user could try to use `-Xclang -fmodules-embed-all-files` when producing 
module files. For example:





Comment at: clang/docs/CPlusPlus20Modules.rst:539
+
+Since there is already one module maps in t

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D123182#3458712 , @royjacobson 
wrote:

> So, it seems like this broke one of `basic_arg_format`'s private 
> constructors.. Sorry for that.
>
> This is now ambiguous when calling `basic_format_arg(char)` because the 
> argument type is different and comparing constraints is not allowed. 
> @Mordante could you maybe take a look? I've reverted in the meantime.

Thanks for the ping!

I'll have a look, probably tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D123182: [Concepts] Fix overload resolution bug with constrained candidates

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I've a fix in D124103 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123182

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I didn't look at the code, but I have some hints how we can test libc++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D112921: [clang] Enable sized deallocation by default in C++14 onwards

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D112921#3463887 , @pcwang-thead 
wrote:

> In D112921#3462592 , @Mordante 
> wrote:
>
>> I didn't look at the code, but I have some hints how we can test libc++.
>
> Thanks! I ran tests with no error occurred on my local machine and I really 
> want to know how to test it!

I added similar information yesterday, but I somehow didn't properly submit it.




Comment at: 
libcxx/test/std/language.support/support.dynamic/new.delete/new.delete.array/sized_delete_array14.pass.cpp:17
+// XFAIL: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{9|10|11}}
 
 #include 

This `// XFAIL: clang-12, clang-13` is still needed but should be `// XFAIL: 
clang-13, clang-14`. (Since the LLVM 14 release we only support these two 
versions.)

For testing it's the easiest to remove line 147 of 
`libcxx/utils/ci/buildkite-pipeline.yml`
https://github.com/llvm/llvm-project/blob/afcc6baac52fcc91d1636f6803f5c230e7018016/libcxx/utils/ci/buildkite-pipeline.yml#L147

That way all builds run. The `Bootstrapping build` builds clang and uses that 
clang to test libc++. That way we can validate which builds fail and which 
succeed. Maybe some more builds need to be (temporary) disabled.

Once we verify that works we need to:
- undo the buildkite changes
- disable this test temporary (`UNSUPPORTED: clang-15`)
- create a followup patch to reenable the test
After the change has landed it will take some time before the change is 
propagated to the CI. Once it's propagated the followup patch can be landed. 
I'm willing to create the followup patch and land it at the proper time.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112921

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

[PATCH] D113319: [clang-format] Improve require and concept handling

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for working on this! I love to see better `requires` support in 
clang-format. I didn't do a real review, just some drive-by comments.




Comment at: clang/docs/ClangFormatStyleOptions.rst:3399
 
+**RequiresClausePositionForClasses** (``RequiresClausePositionStyle``) 
:versionbadge:`clang-format 14`
+  The position of the ``requires`` clause for class templates.

I assume that should `clang-format 15` now. Same for other new options.



Comment at: clang/lib/Format/Format.cpp:1212
   LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  // This is open for discussions! When will LLVM adapt C++20?
+  LLVMStyle.RequiresClausePositionForClasses = FormatStyle::RCPS_OwnLine;

Note the libc++ already uses C++20 code. We don't use clang-format officially, 
but I use it for new code. (Mainly the `std::format` related code.)


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

https://reviews.llvm.org/D113319

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

[PATCH] D113319: [clang-format] Improve require and concept handling

[Mark de Wever via Phabricator via cfe-commits]

Mordante added inline comments.



Comment at: clang/lib/Format/Format.cpp:1212
   LLVMStyle.ReferenceAlignment = FormatStyle::RAS_Pointer;
+  // This is open for discussions! When will LLVM adapt C++20?
+  LLVMStyle.RequiresClausePositionForClasses = FormatStyle::RCPS_OwnLine;

HazardyKnusperkeks wrote:
> Mordante wrote:
> > Note the libc++ already uses C++20 code. We don't use clang-format 
> > officially, but I use it for new code. (Mainly the `std::format` related 
> > code.)
> And is there a consensus about formatting requires clauses?
No there's not. But maybe it's interesting to discuss that after the patch 
lands to see how the different options look in real code.


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

https://reviews.llvm.org/D113319

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

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Sorry I missed your message. At the moment I don't intend to continue with this 
review. Do you want to commandeer this one or do you prefer me to abandon it?


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

https://reviews.llvm.org/D71966

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

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for these fixes. SGTM, but I want to see the CI pass.




Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:135
   FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true);
-  Out << "' call may invalidate the the result of the previous " << '\'';
+  Out << "' call may invalidate the result of the previous " << '\'';
   FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true);

I'm not familiar with the static analyzers, so I don't know whether this breaks 
a test.
I see the pre-commit CI couldn't apply your patch. Can you rebase this patch on 
main so the CI runs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124708

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

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

[Mark de Wever via Phabricator via cfe-commits]

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

I see some test time-outs, but they seem to happen quite often.
LGTM, but please wait a few days before landing this patch to give other 
reviewers time to have a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124708

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

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

[Mark de Wever via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG87a55137e2a2: Fix "the the" typo in documentation 
and user facing strings (authored by briantracy, committed by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124708

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/MatrixTypes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/docs/analyzer/checkers.rst
  clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
  clang/test/Analysis/cert/env34-c.c
  libcxx/include/__iterator/advance.h
  libcxx/src/filesystem/operations.cpp
  llvm/docs/AMDGPUUsage.rst
  llvm/docs/BugLifeCycle.rst
  llvm/docs/CodingStandards.rst
  llvm/docs/CommandGuide/llvm-profdata.rst
  llvm/include/llvm/Analysis/Loads.h
  llvm/include/llvm/ExecutionEngine/Orc/Core.h
  llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -995,7 +995,7 @@
   "zero-counter-threshold", cl::init(0.7), cl::Hidden,
   cl::desc("For the function which is cold in instr profile but hot in "
"sample profile, if the ratio of the number of zero counters "
-   "divided by the the total number of counters is above the "
+   "divided by the total number of counters is above the "
"threshold, the profile of the function will be regarded as "
"being harmful for performance and will be dropped."));
   cl::opt SupplMinSizeThreshold(
Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
===
--- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
+++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
@@ -196,7 +196,7 @@
 };
 
 /// Meta qualifiers for a value. Pair of whatever expression is used to qualify
-/// the the value, and Boolean of whether or not it's indirect.
+/// the value, and Boolean of whether or not it's indirect.
 class DbgValueProperties {
 public:
   DbgValueProperties(const DIExpression *DIExpr, bool Indirect)
Index: llvm/include/llvm/ExecutionEngine/Orc/Core.h
===
--- llvm/include/llvm/ExecutionEngine/Orc/Core.h
+++ llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -1331,7 +1331,7 @@
   lookupInitSymbols(ExecutionSession &ES,
 const DenseMap &InitSyms);
 
-  /// Performs an async lookup for the the given symbols in each of the given
+  /// Performs an async lookup for the given symbols in each of the given
   /// JITDylibs, calling the given handler once all lookups have completed.
   static void
   lookupInitSymbolsAsync(unique_function OnComplete,
Index: llvm/include/llvm/Analysis/Loads.h
===
--- llvm/include/llvm/Analysis/Loads.h
+++ llvm/include/llvm/Analysis/Loads.h
@@ -75,9 +75,9 @@
 /// within the specified loop) would access only dereferenceable memory, and
 /// be properly aligned on every iteration of the specified loop regardless of
 /// its placement within the loop. (i.e. does not require predication beyond
-/// that required by the the header itself and could be hoisted into the header
+/// that required by the header itself and could be hoisted into the header
 /// if desired.)  This is more powerful than the variants above when the
-/// address loaded from is analyzeable by SCEV.  
+/// address loaded from is analyzeable by SCEV.
 bool isDereferenceableAndAlignedInLoop(LoadInst *LI, Loop *L,
ScalarEvolution &SE,
DominatorTree &DT);
Index: llvm/docs/CommandGuide/llvm-profdata.rst
===
--- llvm/docs/CommandGuide/llvm-profdata.rst
+++ llvm/docs/CommandGuide/llvm-profdata.rst
@@ -170,7 +170,7 @@
 .. option:: --zero-counter-threshold=
 
  For the function which is cold in instr profile but hot in sample profile, if
- the ratio of the number of zero counters divided by the the total number of
+ the ratio of the number of zero counters divided by the total number of
  counters is above the threshold, the profile of the function will be regarded
  as being harmful for performance and will be dropped.
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -1312,7 +1312,7 @@
 ... use I ...
 
 Usage of ``std::for_each()``/``llvm::for_each()`` functions is discouraged,
-unless the the callable object already exists.
+unless the c

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

The libc++ build failures are due a broken libc++ HEAD. The current HEAD should 
work again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111283

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

[PATCH] D121141: [Clang] Add `-funstable` flag to enable unstable and experimental features: follow-up fixes

[Mark de Wever via Phabricator via cfe-commits]

Mordante accepted this revision.
Mordante added a comment.

LGTM, but please update the name of the flag in the title before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121141

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

[PATCH] D129048: Rewording the "static_assert" to static assertion

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

In D129048#3668905 , @aaron.ballman 
wrote:

> In D129048#3668846 , @ldionne wrote:
>
>> In D129048#3668594 , @philnik 
>> wrote:
>>
>>> Also, please wait for #libc  approval 
>>> next time.
>>
>> This, x1000.
>>
>> We go through the trouble of having excellent pre-commit testing and 
>> automatic review groups assigned to reviews, please don't bypass that.
>
> We certainly will keep this in mind for the future, thanks for pointing it 
> out! However, the precommit CI behavior confused multiple Clang contributors 
> (I also thought the bot was misconfigured because Clang tests are never run 
> against old Clang versions), the output did not clearly identify what was 
> wrong or how to address it, and the difficulties with testing this locally 
> are all things the libc++ maintainers should also keep in mind.

What can we do to make it easier for Clang contributors to understand the 
libc++ CI?
Maybe we can discuss it on Discord.




Comment at: 
libcxx/test/libcxx/atomics/atomics.types.operations/atomics.types.operations.req/atomic_fetch_add.verify.cpp:49
 volatile std::atomic fun;
-// expected-error-re@atomic:* {{static_assert failed due to requirement 
'!is_function::value'{{.*}}Pointer to function isn't allowed}}
+// expected-error-re@atomic:* {{static assertion failed due to requirement 
'!is_function::value'{{.*}}Pointer to function isn't allowed}}
 std::atomic_fetch_add(&fun, 0);

All libc++ tests need a regex accepting both the old and new diagnostic. Since 
we support older Clang versions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129048

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