[PATCH] D60661: Revert "Revert "[clang-format] Keep protobuf "package" statement on one line""

2019-04-13 Thread Donald Chai via Phabricator via cfe-commits 
dchai created this revision.
dchai added a reviewer: krasimir.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Top-level "package" and "import" statements should generally be kept on
one line, for all languages.



This reverts commit rL356912 .
The regression from rL356835  was fixed via 
rC358275 .


Repository:
  rC Clang

https://reviews.llvm.org/D60661

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestProto.cpp


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -397,6 +397,16 @@
"};");
 }
 
+TEST_F(FormatTestProto, DoesntWrapPackageStatements) {
+  verifyFormat(
+  "package"
+  " some.really.long.package.that.exceeds.the.column.limit;");
+}
+
+TEST_F(FormatTestProto, TrailingCommentAfterPackage) {
+  verifyFormat("package foo.pkg;  // comment\n");
+}
+
 TEST_F(FormatTestProto, FormatsService) {
   verifyFormat("service SearchService {\n"
"  rpc Search(SearchRequest) returns (SearchResponse) {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1119,10 +1119,10 @@
   return LT_ImportStatement;
 }
 
-// In .proto files, top-level options are very similar to import statements
-// and should not be line-wrapped.
+// In .proto files, top-level options and package statements are very
+// similar to import statements and should not be line-wrapped.
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
-CurrentToken->is(Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_option, Keywords.kw_package)) {
   next();
   if (CurrentToken && CurrentToken->is(tok::identifier)) {
 while (CurrentToken)


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -397,6 +397,16 @@
"};");
 }
 
+TEST_F(FormatTestProto, DoesntWrapPackageStatements) {
+  verifyFormat(
+  "package"
+  " some.really.long.package.that.exceeds.the.column.limit;");
+}
+
+TEST_F(FormatTestProto, TrailingCommentAfterPackage) {
+  verifyFormat("package foo.pkg;  // comment\n");
+}
+
 TEST_F(FormatTestProto, FormatsService) {
   verifyFormat("service SearchService {\n"
"  rpc Search(SearchRequest) returns (SearchResponse) {\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1119,10 +1119,10 @@
   return LT_ImportStatement;
 }
 
-// In .proto files, top-level options are very similar to import statements
-// and should not be line-wrapped.
+// In .proto files, top-level options and package statements are very
+// similar to import statements and should not be line-wrapped.
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
-CurrentToken->is(Keywords.kw_option)) {
+CurrentToken->isOneOf(Keywords.kw_option, Keywords.kw_package)) {
   next();
   if (CurrentToken && CurrentToken->is(tok::identifier)) {
 while (CurrentToken)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

2019-04-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits 
Quuxplusone added a comment.

As you're making tests for ADL corner cases, you might also consider testing 
the interactions between ADL and defaulted function parameters, e.g. 
https://godbolt.org/z/vHnyFl
It looks like everyone (except MSVC) already gets that stuff right (or at least 
portable-between-the-big-three). I bet the behavior naturally falls out of some 
other rules; you might say "there's no way that could possibly break, so we 
don't need to test it," and I'd accept that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60570



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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-13 Thread Phabricator via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL358337: [CommandLineParser] Add DefaultOption flag (authored 
by dhinton, committed by ).
Herald added a subscriber: kristina.

Changed prior to commit:
  https://reviews.llvm.org/D59746?vs=194899=195020#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59746

Files:
  cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
  llvm/trunk/docs/CommandLine.rst
  llvm/trunk/include/llvm/Support/CommandLine.h
  llvm/trunk/lib/Support/CommandLine.cpp
  llvm/trunk/test/Support/check-default-options.txt
  llvm/trunk/tools/llvm-opt-report/OptReport.cpp
  llvm/trunk/unittests/Support/CommandLineTest.cpp

Index: llvm/trunk/docs/CommandLine.rst
===
--- llvm/trunk/docs/CommandLine.rst
+++ llvm/trunk/docs/CommandLine.rst
@@ -128,6 +128,7 @@
   USAGE: compiler [options]
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -194,6 +195,7 @@
   USAGE: compiler [options] 
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -1251,6 +1253,14 @@
   with ``cl::CommaSeparated``, this modifier only makes sense with a `cl::list`_
   option.
 
+.. _cl::DefaultOption:
+
+* The **cl::DefaultOption** modifier is used to specify that the option is a
+  default that can be overridden by application specific parsers. For example,
+  the ``-help`` alias, ``-h``, is registered this way, so it can be overridden
+  by applications that need to use the ``-h`` option for another purpose,
+  either as a regular option or an alias for another option.
+
 .. _response files:
 
 Response files
Index: llvm/trunk/include/llvm/Support/CommandLine.h
===
--- llvm/trunk/include/llvm/Support/CommandLine.h
+++ llvm/trunk/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -270,7 +273,7 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.
 
@@ -306,6 +309,7 @@
   bool hasArgStr() const { return !ArgStr.empty(); }
   bool isPositional() const { return getFormattingFlag() == cl::Positional; }
   bool isSink() const { return getMiscFlags() & cl::Sink; }
+  bool isDefaultOption() const { return getMiscFlags() & cl::DefaultOption; }
 
   bool isConsumeAfter() const {
 return getNumOccurrencesFlag() == cl::ConsumeAfter;
@@ -382,7 +386,7 @@
   }
 
   inline int getNumOccurrences() const { return NumOccurrences; }
-  inline void reset() { NumOccurrences = 0; }
+  void reset();
 };
 
 //===--===//
@@ -1732,7 +1736,10 @@
   error("cl::alias must have argument name specified!");
 if (!AliasFor)
   error("cl::alias must have an cl::aliasopt(option) specified!");
+if (!Subs.empty())
+  error("cl::alias must not have cl::sub(), aliased option's cl::sub() will be used!");
 Subs = AliasFor->Subs;
+Category = AliasFor->Category;
 addArgument();
   }
 
Index: llvm/trunk/test/Support/check-default-options.txt
===
--- llvm/trunk/test/Support/check-default-options.txt
+++ llvm/trunk/test/Support/check-default-options.txt
@@ -0,0 +1,18 @@
+# RUN: llvm-objdump -help-hidden %t | FileCheck --check-prefix=CHECK-OBJDUMP %s
+# RUN: llvm-readobj -help-hidden %t | FileCheck --check-prefix=CHECK-READOBJ %s
+# RUN: llvm-tblgen -help-hidden %t | FileCheck --check-prefix=CHECK-TBLGEN %s
+# RUN: llvm-opt-report -help-hidden %t | FileCheck --check-prefix=CHECK-OPT-RPT %s
+# RUN: llvm-dwarfdump -help-hidden %t | FileCheck --check-prefix=CHECK-DWARF %s
+# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
+
+
+# CHECK-OBJDUMP: -h  - Alias for --section-headers
+# CHECK-READOBJ: -h  - Alias for --file-headers
+# CHECK-TBLGEN:  -h  - Alias for -help
+# CHECK-OPT-RPT: -h  - Alias for -help
+# CHECK-DWARF:   -h  - Alias for -help
+
+# llvm-dwarfdump declares `-h` option and prints special help in that case,
+# which is weird, but makes for

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-13 Thread Don Hinton via Phabricator via cfe-commits 
hintonda added a comment.

In D59746#1465445 , @klimek wrote:

> lg


Thanks again for the help and the suggestion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

r358337 - [CommandLineParser] Add DefaultOption flag

2019-04-13 Thread Don Hinton via cfe-commits 
Author: dhinton
Date: Sat Apr 13 09:55:28 2019
New Revision: 358337

URL: http://llvm.org/viewvc/llvm-project?rev=358337=rev
Log:
[CommandLineParser] Add DefaultOption flag

Summary: Add DefaultOption flag to CommandLineParser which provides a
default option or alias, but allows users to override it for some
other purpose as needed.

Also, add `-h` as a default alias to `-help`, which can be seamlessly
overridden by applications like llvm-objdump and llvm-readobj which
use `-h` as an alias for other options.

Reviewers: alexfh, klimek

Reviewed By: klimek

Subscribers: MaskRay, mehdi_amini, inglorion, dexonsmith, hiraditya, 
llvm-commits, jhenderson, arphaman, cfe-commits

Tags: #clang, #llvm

Differential Revision: https://reviews.llvm.org/D59746

Modified:
cfe/trunk/lib/Tooling/CommonOptionsParser.cpp

Modified: cfe/trunk/lib/Tooling/CommonOptionsParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/CommonOptionsParser.cpp?rev=358337=358336=358337=diff
==
--- cfe/trunk/lib/Tooling/CommonOptionsParser.cpp (original)
+++ cfe/trunk/lib/Tooling/CommonOptionsParser.cpp Sat Apr 13 09:55:28 2019
@@ -83,8 +83,6 @@ std::vector ArgumentsAdj
 llvm::Error CommonOptionsParser::init(
 int , const char **argv, cl::OptionCategory ,
 llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
-  static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden,
-cl::sub(*cl::AllSubCommands));
 
   static cl::opt BuildPath("p", cl::desc("Build path"),
 cl::Optional, cl::cat(Category),


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

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

2019-04-13 Thread Manuel Klimek via Phabricator via cfe-commits 
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.

lg


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D59870: [clang-tidy] Add MagnitudeBitsUpperLimit option to bugprone-too-small-loop-variable

2019-04-13 Thread Tamás Zolnai via Phabricator via cfe-commits 
ztamas added a comment.

In the meantime, I've got the commit access, so I'll give it a try to push this 
myself.


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

https://reviews.llvm.org/D59870



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

[clang-tools-extra] r358333 - [clang-tidy] Use back-tick here

2019-04-13 Thread Tamas Zolnai via cfe-commits 
Author: ztamas
Date: Sat Apr 13 07:31:54 2019
New Revision: 358333

URL: http://llvm.org/viewvc/llvm-project?rev=358333=rev
Log:
[clang-tidy] Use back-tick here

Modified:

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst

Modified: 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst?rev=358333=358332=358333=diff
==
--- 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
 (original)
+++ 
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-throw-by-value-catch-by-reference.rst
 Sat Apr 13 07:31:54 2019
@@ -3,8 +3,8 @@
 misc-throw-by-value-catch-by-reference
 ==
 
-"cert-err09-cpp" redirects here as an alias for this check.
-"cert-err61-cpp" redirects here as an alias for this check.
+`cert-err09-cpp` redirects here as an alias for this check.
+`cert-err61-cpp` redirects here as an alias for this check.
 
 Finds violations of the rule "Throw by value, catch by reference" presented for
 example in "C++ Coding Standards" by H. Sutter and A. Alexandrescu.


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

[PATCH] D60570: [Sema] Add more tests for the behavior of argument-dependent name lookup

2019-04-13 Thread Arthur O'Dwyer via Phabricator via cfe-commits 
Quuxplusone added inline comments.



Comment at: 
test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:77
+
+  // associated class: itself, lambda
+  namespace X5 {

Do you also have a test somewhere to verify that the parameter and return types 
of a lambda's `operator()` do not contribute to the associated types of the 
lambda type itself? That is,
```
// https://godbolt.org/z/g_oMOA
namespace N {
struct A {};
template constexpr int f(T) { return 1; }
}

constexpr int f(N::A (*)()) { return 2; }
constexpr int f(void (*)(N::A)) { return 3; }

void test() {
constexpr auto lambda = []() -> N::A { return {}; };
static_assert(f(lambda) == 2);

constexpr auto lambda2 = [](N::A) {};
static_assert(f(lambda2) == 3);
}
```
Clang does handle this correctly; I'm just asking for it to be tested, if it's 
not already. (I might have overlooked an existing test.)



Comment at: 
test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:144
+
+  // template template argument
+  namespace X3 {

I think for completeness there should be a "negative test" for non-type 
template arguments:
```
  namespace X4 {
template  struct C {};
namespace N {
  struct Z {
  enum E { E0 };
  void X4_f(C);
  };
  enum E { E0 };
  void X4_g(C);
}
  }
  void test4() {
X4::C c1;
X4::C c2;
X4_f(c1); // expected-error{{undeclared identifier 'X4_f'}}
X4_g(c2); // expected-error{{undeclared identifier 'X4_g'}}
  }
```
In C++2a, user-defined NTTPs will become possible, so we'll want another test 
for something like
```
  // https://godbolt.org/z/MfWG8C
  namespace X4 {
template struct C {};
namespace N {
  struct Z {
int i;
constexpr Z(int i): i(i) {}
auto operator<=>(const Z&) const = default;
  };
  void X4_f(C);
}
  }
  void test4() {
X4::C c1;
X4_f(c1); // expected-error{{undeclared identifier 'X4_f'}}
  }
```



Comment at: 
test/CXX/basic/basic.lookup/basic.lookup.argdep/p2-associated-namespaces-classes.cpp:304
+static_assert(f(g3) == 4, "");// FIXME: Also well-formed from the 
union rule.
+  // expected-error@-1 {{use of 
undeclared}}
+  }

riccibruno wrote:
> Quuxplusone wrote:
> > I see how `g3` matches the example in CWG997
> > http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#997
> > However, I don't see how CWG997's resolution actually affected this example 
> > in the slightest. The wording inserted for CWG997 was, "Additionally, if 
> > the aforementioned set of overloaded functions is named with a template-id, 
> > its associated classes and namespaces are those of its type 
> > template-arguments and its template template-arguments." That makes e.g.
> > 
> > f(g3)
> > 
> > consider `N::f`, because `N::S` is a "type template-argument" of the 
> > template-id `g3` which names the set of overloaded functions.  But it 
> > doesn't do anything at all to `f(g3)` because `g3` is not a template-id and 
> > doesn't have any template-arguments.
> > 
> > This piece of ADL is implemented only by GCC (not EDG, Clang, or MSVC), and 
> > personally I would very much like to keep it that way. We know there's no 
> > real-world code that expects or relies on CWG997 — because such code would 
> > never work in practice except on GCC. Let's keep it that way!  As soon as 
> > you implement a crazy arcane rule, such that code _could_ portably rely on 
> > it, code _will start_ relying on it... and then we'll never be able to 
> > simplify the language.
> > Case in point: the piece of ADL described in this blog post --
> > https://quuxplusone.github.io/blog/2019/04/09/adl-insanity-round-2/
> > As soon as the above-described arcane ADL rule was implemented in GCC and 
> > Clang, Boost.Hana started relying on it; and now the rule is "locked in" to 
> > the paper standard because there's real-world code relying on it.
> > Personally I'd like to _keep_ real-world code from relying on CWG997, until 
> > someone figures out what CWG was thinking when they added it.
> I think that the relevant part of CWG 997 is the removal of the restriction 
> on non-dependent parameter types. Sure, `g3` is not a `template-id`, but it 
> refers to an overload set which contains the second `g3`, and one of the 
> parameter of this second `g3` is `N::Q`.
> 
> I don't think this is a surprising rule. It matches the general intuition 
> that for function types ADL is done based on the function parameter types and 
> return type. Not having this rule introduces a difference between function 
> templates and functions in overload sets. Consider 
> https://godbolt.org/z/UXHqm2 :
> ```
> namespace N {
> struct S1 {};
> template  struct S2 {};
> 
> void f(void (*g)());
> }
> 
> void g1();  // #1
> void

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

2019-04-13 Thread Bruno Ricci via Phabricator via cfe-commits 
riccibruno added a comment.

In D60561#1465373 , @Tyker wrote:

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


Good to know, thanks for doing the measurement !


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

https://reviews.llvm.org/D60561



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

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

2019-04-13 Thread Saar Raz via Phabricator via cfe-commits 
saar.raz updated this revision to Diff 195010.
saar.raz added a comment.
Herald added a reviewer: martong.
Herald added a reviewer: shafik.

Fixed bug in normalization substitution into CSEs
Rebase onto trunk


Repository:
  rC Clang

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

https://reviews.llvm.org/D41910

Files:
  include/clang/AST/DeclTemplate.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/AST/ASTImporter.cpp
  lib/AST/DeclTemplate.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriterDecl.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.decl/func-template-decl.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.normal/p1.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp
  test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp

Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 4
+bool a = false; // expected-note{{template is declared here}}
+
+template requires sizeof(T) >= 4 && sizeof(T) <= 10
+bool a = true; // expected-error{{variable template partial specialization is not more specialized than the primary template}}
+
+template
+concept C1 = sizeof(T) >= 4;
+
+template requires C1
+bool b = false;
+
+template requires C1 && sizeof(T) <= 10
+bool b = true;
+
+template
+concept C2 = sizeof(T) > 1 && sizeof(T) <= 8;
+
+template
+bool c = false;
+
+template requires C1
+bool c = true;
+
+template
+bool d = false;
+
+template
+bool d = true; // expected-error{{variable template partial specialization does not specialize any template argument; to define the primary template, remove the template argument list}}
+
+template requires C1
+bool e = false;
+
+template
+bool e = true; // expected-error{{variable template partial specialization does not specialize any template argument; to define the primary template, remove the template argument list}}
+
+template
+constexpr int f = 1;
+
+template requires C1 && C2
+constexpr int f = 2;
+
+template requires C1 || C2
+constexpr int f = 3;
+
+static_assert(f == 2);
+static_assert(f == 3);
+static_assert(f == 1);
+
+
+
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 4
+bool a() { return false; } // expected-note {{candidate function [with T = unsigned int]}}
+
+template requires sizeof(T) >= 4 && sizeof(T) <= 10
+bool a() { return true; } // expected-note {{candidate function [with T = unsigned int]}}
+
+bool av = a(); // expected-error {{call to 'a' is ambiguous}}
+
+template
+concept C1 = sizeof(T) >= 4;
+
+template requires C1
+constexpr bool b() { return false; }
+
+template requires C1 && sizeof(T) <= 10
+constexpr bool b() { return true; }
+
+static_assert(b());
+static_assert(!b());
+
+template
+concept C2 = sizeof(T) > 1 && sizeof(T) <= 8;
+
+template
+bool c() { return false; }
+
+template requires C1
+bool c() { return true; }
+
+template requires C1
+constexpr bool d() { return false; }
+
+template
+constexpr bool d() { return true; }
+
+static_assert(!d());
+
+template
+constexpr int e() { return 1; }
+
+template requires C1 && C2
+constexpr int e() { return 2; }
+
+template requires C1 || C2
+constexpr int e() { return 3; }
+
+static_assert(e() == 2);
+static_assert(e() == 3);
+static_assert(e() == 1);
+
+template
+concept BiggerThan = sizeof(T) > sizeof(U);
+
+template
+concept BiggerThanInt = BiggerThan;
+
+template requires BiggerThan
+void f() { }
+// expected-note@-1 {{candidate function [with T = long long, U = int]}}
+
+template requires BiggerThanInt
+void f() { }
+// expected-note@-1 {{candidate function [with T = long long, U = int]}}
+
+static_assert(sizeof(f()));
+// expected-error@-1 {{call to 'f' is ambiguous}}
+
+template
+concept C3 = true;
+
+template
+concept C4 = true && C3;
+
+template requires C3
+int g() { }
+
+template requires C4
+int g() { }
+
+static_assert(sizeof(g()));
\ No newline at end of file
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp
===
---

[PATCH] D41569: [Concepts] Constraint enforcement and diagnostics

2019-04-13 Thread Saar Raz via Phabricator via cfe-commits 
saar.raz updated this revision to Diff 195009.
saar.raz added a comment.

Rebase onto trunk


Repository:
  rC Clang

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

https://reviews.llvm.org/D41569

Files:
  include/clang/AST/ExprCXX.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  include/clang/Sema/SemaConcept.h
  include/clang/Sema/TemplateDeduction.h
  lib/AST/ExprCXX.cpp
  lib/Sema/SemaConcept.cpp
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaTemplate.cpp
  lib/Sema/SemaTemplateDeduction.cpp
  lib/Sema/SemaTemplateInstantiate.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  
test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp

Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/partial-specializations.cpp
@@ -0,0 +1,67 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+namespace class_templates
+{
+  template requires sizeof(T) >= 4 // expected-note {{because 'sizeof(char) >= 4' (1 >= 4) evaluated to false}}
+  struct is_same { static constexpr bool value = false; };
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  struct is_same { static constexpr bool value = true; };
+
+  static_assert(!is_same::value);
+  static_assert(!is_same::value);
+  static_assert(is_same::value);
+  static_assert(is_same::value); // expected-error {{constraints not satisfied for class template 'is_same' [with T = char, U = char]}}
+
+  template
+  struct A { using type = typename T::type; }; // expected-error{{type 'int *' cannot be used prior to '::' because it has no members}}
+
+  template
+  struct B {};
+
+  template requires A::type // expected-note{{in instantiation of template class 'class_templates::A' requested here}}
+   // expected-note@-1{{while substituting template arguments into constraint expression here}}
+  struct B {};
+
+  template requires T{} // expected-error{{atomic constraint must be of type 'bool' (found 'int')}}
+  struct B {};
+
+  static_assert((B{}, true)); // expected-note{{while checking constraint satisfaction for class template partial specialization 'B' required here}}
+  // expected-note@-1{{while checking constraint satisfaction for class template partial specialization 'B' required here}}
+  // expected-note@-2{{during template argument deduction for class template partial specialization 'B' [with T = int *]}}
+  // expected-note@-3{{during template argument deduction for class template partial specialization 'B' [with T = int]}}
+  // expected-note@-4 2{{in instantiation of template class 'class_templates::B' requested here}}
+}
+
+namespace variable_templates
+{
+  template requires sizeof(T) >= 4
+  constexpr bool is_same_v = false;
+
+  template requires sizeof(T*) >= 4 && sizeof(T) >= 4
+  constexpr bool is_same_v = true;
+
+  static_assert(!is_same_v);
+  static_assert(!is_same_v);
+  static_assert(is_same_v);
+
+  template
+  struct A { using type = typename T::type; }; // expected-error{{type 'int *' cannot be used prior to '::' because it has no members}}
+
+  template
+  constexpr bool v1 = false;
+
+  template requires A::type // expected-note{{in instantiation of template class 'variable_templates::A' requested here}}
+   // expected-note@-1{{while substituting template arguments into constraint expression here}}
+  constexpr bool v1 = true;
+
+  template requires T{} // expected-error{{atomic constraint must be of type 'bool' (found 'int')}}
+  constexpr bool v1 = true;
+
+  static_assert(v1); // expected-note{{while checking constraint satisfaction for variable template partial specialization 'v1' required here}}
+  // expected-note@-1{{while checking constraint satisfaction for variable template partial specialization 'v1' required here}}
+  // expected-note@-2{{during template argument deduction for variable template partial specialization 'v1' [with T = int *]}}
+  // expected-note@-3{{during template argument deduction for variable template partial specialization 'v1' [with T = int]}}
+  // expected-error@-4{{static_assert failed due to requirement 'v1'}}
+
+}
\ No newline at end of file
Index: test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
===
--- /dev/null
+++ test/CXX/concepts-ts/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
@@ -0,0 +1,92 @@
+// RUN: %clang_cc1

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

2019-04-13 Thread Gauthier via Phabricator via cfe-commits 
Tyker added a comment.

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


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

https://reviews.llvm.org/D60561



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-04-13 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added inline comments.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:29-30
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char  : Src) {

lebedev.ri wrote:
> `SmallString<32>` ?
> Also, it is safe to `OS.reserve(Src.size())`
Also, you probably want to add `raw_svector_ostream` ontop, and `<<` into it.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

2019-04-13 Thread Roman Lebedev via Phabricator via cfe-commits 
lebedev.ri added a comment.

Some post-commit review (please submit a new review, don't replace this diff)
As usual, the incorrect license headers keep slipping through.




Comment at: llvm/trunk/include/llvm/Support/TimeProfiler.h:5-6
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

OOOPS, wrong license.



Comment at: llvm/trunk/include/llvm/Support/TimeProfiler.h:53
+/// is not initialized, the overhead is a single branch.
+struct TimeTraceScope {
+  TimeTraceScope(StringRef Name, StringRef Detail) {

Did you mean to explicitly prohibit all the default constructors / `operator=`?



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:5-6
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

Wrong license.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:29-30
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char  : Src) {

`SmallString<32>` ?
Also, it is safe to `OS.reserve(Src.size())`



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:59-60
+  DurationType Duration;
+  std::string Name;
+  std::string Detail;
+};

`SmallString<32>`?



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:70-71
+
+  void begin(std::string Name, llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail()};
+Stack.push_back(std::move(E));

Why not either take `StringRef` arg, or at least `std::move()` it when creating 
`Entry`?



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:89
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry ) {
+  return Val.Name == E.Name;

llvm::find_if(llvm::reverse(), ) 



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:99
+
+  void Write(std::unique_ptr ) {
+assert(Stack.empty() &&

Why pass `std::unique_ptr` ?
Just `raw_pwrite_stream `



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:122
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),

Elide `{}`



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:123
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration , const NameAndDuration ) {

llvm::sort <- this is a correctness issue.



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:144-145
+
+  std::vector Stack;
+  std::vector Entries;
+  std::unordered_map TotalPerName;

Would it make sense to make these `SmallVector`?



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:146-147
+  std::vector Entries;
+  std::unordered_map TotalPerName;
+  std::unordered_map CountPerName;
+  time_point StartTime;

1. Eww, `std::unordered_map`, that will have *horrible* perf.
2. Eww, map with key = string. Use `llvm::StringMap`



Comment at: llvm/trunk/lib/Support/TimeProfiler.cpp:162
+
+void timeTraceProfilerWrite(std::unique_ptr ) {
+  assert(TimeTraceProfilerInstance != nullptr &&

Just `raw_pwrite_stream `?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58675



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

Re: r357877 - [clang-format] Fix bug https://bugs.llvm.org/show_bug.cgi?id=41413

2019-04-13 Thread Owen Pan via cfe-commits 
Hi Paul,

Thank you for the information! I will remember to do that from now on.

Regards,

Owen

On Fri, Apr 12, 2019 at 11:34 AM  wrote:

> Hi Owen,
>
> FYI, putting a URL in the headline of the commit message takes up
> space and doesn't really describe the fix to a casual reader. The
> subject line of your Phabricator review looks like it would have
> been perfectly fine to use for the commit as well.
>
> Citing the bug in the body of the commit message is enough to let
> people track down the original report, although even there we usually
> abbreviate it to 'PR' (so PR41413 in this example).
>
> Thanks!
> --paulr
>
> > -Original Message-
> > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf
> Of
> > Owen Pan via cfe-commits
> > Sent: Sunday, April 07, 2019 5:06 PM
> > To: cfe-commits@lists.llvm.org
> > Subject: r357877 - [clang-format] Fix bug
> > https://bugs.llvm.org/show_bug.cgi?id=41413
> >
> > Author: owenpan
> > Date: Sun Apr  7 14:05:52 2019
> > New Revision: 357877
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=357877=rev
> > Log:
> > [clang-format] Fix bug https://bugs.llvm.org/show_bug.cgi?id=41413
> >
> > Differential Revision: https://reviews.llvm.org/D60374
> >
> > Modified:
> > cfe/trunk/lib/Format/ContinuationIndenter.cpp
> > cfe/trunk/unittests/Format/FormatTest.cpp
> >
> > Modified: cfe/trunk/lib/Format/ContinuationIndenter.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/lib/Format/ContinuationIndenter.cpp?rev=357877=357876
> > =357877=diff
> >
> ==
> > 
> > --- cfe/trunk/lib/Format/ContinuationIndenter.cpp (original)
> > +++ cfe/trunk/lib/Format/ContinuationIndenter.cpp Sun Apr  7 14:05:52
> 2019
> > @@ -945,18 +945,24 @@ unsigned ContinuationIndenter::getNewLin
> >return State.Stack[State.Stack.size() - 2].LastSpace;
> >  return State.FirstIndent;
> >}
> > -  // Indent a closing parenthesis at the previous level if followed by a
> > semi or
> > -  // opening brace. This allows indentations such as:
> > +  // Indent a closing parenthesis at the previous level if followed by a
> > semi,
> > +  // const, or opening brace. This allows indentations such as:
> >// foo(
> >//   a,
> >// );
> > +  // int Foo::getter(
> > +  // //
> > +  // ) const {
> > +  //   return foo;
> > +  // }
> >// function foo(
> >//   a,
> >// ) {
> >//   code(); //
> >// }
> >if (Current.is(tok::r_paren) && State.Stack.size() > 1 &&
> > -  (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace)))
> > +  (!Current.Next ||
> > +   Current.Next->isOneOf(tok::semi, tok::kw_const, tok::l_brace)))
> >  return State.Stack[State.Stack.size() - 2].LastSpace;
> >if (NextNonComment->is(TT_TemplateString) && NextNonComment-
> > >closesScope())
> >  return State.Stack[State.Stack.size() - 2].LastSpace;
> >
> > Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> > URL: http://llvm.org/viewvc/llvm-
> >
> project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=357877=357876=
> > 357877=diff
> >
> ==
> > 
> > --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> > +++ cfe/trunk/unittests/Format/FormatTest.cpp Sun Apr  7 14:05:52 2019
> > @@ -12822,6 +12822,24 @@ TEST_F(FormatTest, ConfigurableContinuat
> >  format("int i = longFunction(arg);", SixIndent));
> >  }
> >
> > +TEST_F(FormatTest, WrappedClosingParenthesisIndent) {
> > +  FormatStyle Style = getLLVMStyle();
> > +  verifyFormat(
> > +  "int Foo::getter(\n"
> > +  "//\n"
> > +  ") const {\n"
> > +  "  return foo;\n"
> > +  "}",
> > +  Style);
> > +  verifyFormat(
> > +  "void Foo::setter(\n"
> > +  "//\n"
> > +  ") {\n"
> > +  "  foo = 1;\n"
> > +  "}",
> > +  Style);
> > +}
> > +
> >  TEST_F(FormatTest, SpacesInAngles) {
> >FormatStyle Spaces = getLLVMStyle();
> >Spaces.SpacesInAngles = true;
> >
> >
> > ___
> > cfe-commits mailing list
> > cfe-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits