date:20161229

[PATCH] D28174: [libcxx] [NFC] Rename _LIBCPP_TYPE_VIS_ONLY to _LIBCPP_TEMPLATE_VIS.

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

In https://reviews.llvm.org/D28174#632662, @smeenai wrote:

> As you're aware, I have plans to change `_LIBCPP_TYPE_VIS` to expand to 
> `visibility` instead of `type_visibility`.


I had forgotten. Thanks for the reminder.

> but I still think this rename makes sense.

Awesome, Thanks!


https://reviews.llvm.org/D28174



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

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread S. Gilles via Phabricator via cfe-commits

sgilles updated this revision to Diff 82720.
sgilles added a comment.

Address rsmith's comments, in particular: factor out testing zero initializers 
to a method of `InitListExpr`; use `ParentIList` instead of 
`StructuredSubobjectInitList`.

The warning is (still) not relaxed for C++ code.  I have no opinion on this 
beyond wanting to avoid regressions, but for lack of consensus I'll default to 
changing as little as possible.


https://reviews.llvm.org/D28148

Files:
  include/clang/AST/Expr.h
  lib/AST/Expr.cpp
  lib/Sema/SemaInit.cpp
  test/Sema/zero-initializer.c


Index: test/Sema/zero-initializer.c
===
--- /dev/null
+++ test/Sema/zero-initializer.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c99 -Wmissing-field-initializers -Wmissing-braces
+
+struct foo { int x; int y; };
+struct bar { struct foo a; struct foo b; };
+struct A { int a; }
+struct B { struct A a; }
+struct C { struct B b; }
+
+int main(void)
+{
+  struct foo f = { 0 }; // expected-no-diagnostics
+  struct foo g = { 9 }; // expected-warning {{missing field 'y' initializer}}
+  struct foo h = { 9, 9 }; // expected-no-diagnostics
+  struct bar i = { 0 }; // expected-no-diagnostics
+  struct bar j = { 0, 0 }; // expected-warning {{suggest braces around 
initialization of suboject}} expected-warning {{missing field 'b' initializer}}
+  struct bar k = { { 9, 9 }, { 9, 9 } }; // expected-no-diagnostics
+  struct bar l = { { 9, 9 }, { 0 } }; // expected-no-diagnostics
+  struct bar m = { { 0 }, { 0 } }; // expected-no-diagnostics
+  struct bar n = { { 0 }, { 9, 9 } }; // expected-no-diagnostics
+  struct bar o = { { 9, 9 }, { 0 } }; // expected-no-diagnostics
+  struct bar p = { { 9 }, { 9, 9 } }; // expected-warning {{missing field 'y' 
initializer}}
+  struct C q = { 0 }; // expected-no-diagnostics
+  struct C r = { 9 }; // expected-warning {{suggest braces around 
initialization of suboject}}
+
+  return 0;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -881,7 +881,8 @@
 }
 
 // Complain about missing braces.
-if (T->isArrayType() || T->isRecordType()) {
+if ((T->isArrayType() || T->isRecordType()) &&
+!ParentIList->isIdiomaticZeroInitializer(SemaRef.getLangOpts())) {
   SemaRef.Diag(StructuredSubobjectInitList->getLocStart(),
diag::warn_missing_braces)
   << StructuredSubobjectInitList->getSourceRange()
@@ -1827,7 +1828,9 @@
   // worthwhile to skip over the rest of the initializer, though.
   RecordDecl *RD = DeclType->getAs()->getDecl();
   RecordDecl::field_iterator FieldEnd = RD->field_end();
-  bool CheckForMissingFields = true;
+  bool CheckForMissingFields =
+!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
+
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
 
Index: lib/AST/Expr.cpp
===
--- lib/AST/Expr.cpp
+++ lib/AST/Expr.cpp
@@ -1883,6 +1883,21 @@
  getInit(0)->getType().getCanonicalType();
 }
 
+bool InitListExpr::isIdiomaticZeroInitializer(const LangOptions &LangOpts) 
const {
+  assert(!getSyntacticForm() && "only test syntactic form as zero 
initializer");
+
+  if (LangOpts.CPlusPlus || getNumInits() != 1) {
+return false;
+  }
+
+  const IntegerLiteral *lit = dyn_cast(getInit(0));
+  if (!lit) {
+return false;
+  }
+
+  return lit->getValue() == 0;
+}
+
 SourceLocation InitListExpr::getLocStart() const {
   if (InitListExpr *SyntacticForm = getSyntacticForm())
 return SyntacticForm->getLocStart();
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -3899,6 +3899,10 @@
   /// initializer)?
   bool isTransparent() const;
 
+  /// Is this the zero initializer {0} in a language which considers it
+  /// idiomatic?
+  bool isIdiomaticZeroInitializer(const LangOptions &LangOpts) const;
+
   SourceLocation getLBraceLoc() const { return LBraceLoc; }
   void setLBraceLoc(SourceLocation Loc) { LBraceLoc = Loc; }
   SourceLocation getRBraceLoc() const { return RBraceLoc; }


Index: test/Sema/zero-initializer.c
===
--- /dev/null
+++ test/Sema/zero-initializer.c
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -std=c99 -Wmissing-field-initializers -Wmissing-braces
+
+struct foo { int x; int y; };
+struct bar { struct foo a; struct foo b; };
+struct A { int a; }
+struct B { struct A a; }
+struct C { struct B b; }
+
+int main(void)
+{
+  struct foo f = { 0 }; // expected-no-diagnostics
+  struct foo g = { 9 }; // expected-warning {{missing field 'y' initializer}}
+  struct foo h = { 9, 9 }; // expected-no-diagnostics
+  struct bar i = { 0 }; // expected-no-diagnostics
+  struct bar j = { 0, 0 }; // expected-warning {{suggest bra

[PATCH] D28174: [libcxx] [NFC] Rename _LIBCPP_TYPE_VIS_ONLY to _LIBCPP_TEMPLATE_VIS.

2016-12-29 Thread Shoaib Meenai via Phabricator via cfe-commits

smeenai added a comment.

As you're aware, I have plans to change `_LIBCPP_TYPE_VIS` to expand to 
`visibility` instead of `type_visibility`, but I still think this rename makes 
sense.


https://reviews.llvm.org/D28174



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

[PATCH] D27430: [libc++] Annotate template methods with visibility

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

Please put these attributes to the first declaration instead of the definition.




Comment at: include/thread:392
 template 
+_LIBCPP_HIDDEN
 thread::thread(_Fp __f)

We really should hide this using `inline _LIBCPP_INLINE_VISIBILITY` because 
it's a special C++03 symbol, so we don't even want a hidden definition omitted 
ideally.


https://reviews.llvm.org/D27430



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

[PATCH] D26949: [libc++abi] Clean up visibility

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added inline comments.



Comment at: src/abort_message.cpp:25
 
-#pragma GCC visibility push(hidden)
-

Is this really redundant? There is an `#include ` after 
it. Is this not going to affect those symbols?



Comment at: src/cxa_handlers.hpp:23
 
-__attribute__((visibility("hidden"), noreturn))
+_LIBCXXABI_HIDDEN __attribute__((noreturn))
 void

`_LIBCXXABI_HIDDEN LIBCXXABI_NORETURN`?



Comment at: src/cxa_handlers.hpp:27
 
-__attribute__((visibility("hidden"), noreturn))
+_LIBCXXABI_HIDDEN __attribute__((noreturn))
 void

`_LIBCXXABI_HIDDEN LIBCXXABI_NORETURN`?



Comment at: src/cxa_new_delete.cpp:34
 */
-__attribute__((__weak__, __visibility__("default")))
+__attribute__((__weak__))
 void *

Can we abstract this away to a `_LIBCXXABI_NEW_DELETE_VIS` macro?


https://reviews.llvm.org/D26949



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

[PATCH] D26830: [libcxx] Add string_view literals

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

Alright. I committed the Clang changes. Just re-building now so I can test this.




Comment at: include/string_view:780
+} // namespace literals
+#endif
+

`// _LIBCPP_STD_VER > 14` comment please.



Comment at: 
test/std/strings/string.view/string.view.literals/literal1.pass.cpp:16
+
+int main()
+{

What's the point of this test that `literal.pass.cpp` doesn't cover?



Comment at: 
test/std/strings/string.view/string.view.literals/literal2.pass.cpp:16
+
+int main()
+{

What's the point of this test that `literal.pass.cpp` doesn't cover?


https://reviews.llvm.org/D26830



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

[PATCH] D26829: [clang] Allow lexer to handle string_view literals

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF closed this revision.
EricWF added a comment.

Committed on behalf of @AntonBikineev in r290744.


https://reviews.llvm.org/D26829



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

r290744 - Allow lexer to handle string_view literals. Patch from Anton Bikineev.

2016-12-29 Thread Eric Fiselier via cfe-commits

Author: ericwf
Date: Thu Dec 29 22:51:10 2016
New Revision: 290744

URL: http://llvm.org/viewvc/llvm-project?rev=290744&view=rev
Log:
Allow lexer to handle string_view literals. Patch from Anton Bikineev.

This implements the compiler side of p0403r0. This patch was reviewed as
https://reviews.llvm.org/D26829.

Added:
cfe/trunk/test/SemaCXX/cxx1z-user-defined-literals.cpp
Modified:
cfe/trunk/include/clang/Lex/LiteralSupport.h
cfe/trunk/lib/Lex/Lexer.cpp
cfe/trunk/lib/Lex/LiteralSupport.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp

Modified: cfe/trunk/include/clang/Lex/LiteralSupport.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/LiteralSupport.h?rev=290744&r1=290743&r2=290744&view=diff
==
--- cfe/trunk/include/clang/Lex/LiteralSupport.h (original)
+++ cfe/trunk/include/clang/Lex/LiteralSupport.h Thu Dec 29 22:51:10 2016
@@ -259,6 +259,8 @@ public:
 return UDSuffixOffset;
   }
 
+  static bool isValidUDSuffix(const LangOptions &LangOpts, StringRef Suffix);
+
 private:
   void init(ArrayRef StringToks);
   bool CopyStringFragment(const Token &Tok, const char *TokBegin,

Modified: cfe/trunk/lib/Lex/Lexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Lexer.cpp?rev=290744&r1=290743&r2=290744&view=diff
==
--- cfe/trunk/lib/Lex/Lexer.cpp (original)
+++ cfe/trunk/lib/Lex/Lexer.cpp Thu Dec 29 22:51:10 2016
@@ -1713,9 +1713,9 @@ const char *Lexer::LexUDSuffix(Token &Re
  getLangOpts());
 if (!isIdentifierBody(Next)) {
   // End of suffix. Check whether this is on the whitelist.
-  IsUDSuffix = (Chars == 1 && Buffer[0] == 's') ||
-   NumericLiteralParser::isValidUDSuffix(
-   getLangOpts(), StringRef(Buffer, Chars));
+  const StringRef CompleteSuffix(Buffer, Chars);
+  IsUDSuffix = StringLiteralParser::isValidUDSuffix(getLangOpts(),
+CompleteSuffix);
   break;
 }
 

Modified: cfe/trunk/lib/Lex/LiteralSupport.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/LiteralSupport.cpp?rev=290744&r1=290743&r2=290744&view=diff
==
--- cfe/trunk/lib/Lex/LiteralSupport.cpp (original)
+++ cfe/trunk/lib/Lex/LiteralSupport.cpp Thu Dec 29 22:51:10 2016
@@ -1708,3 +1708,12 @@ unsigned StringLiteralParser::getOffsetO
 
   return SpellingPtr-SpellingStart;
 }
+
+/// Determine whether a suffix is a valid ud-suffix. We avoid treating reserved
+/// suffixes as ud-suffixes, because the diagnostic experience is better if we
+/// treat it as an invalid suffix.
+bool StringLiteralParser::isValidUDSuffix(const LangOptions &LangOpts,
+  StringRef Suffix) {
+  return NumericLiteralParser::isValidUDSuffix(LangOpts, Suffix) ||
+ Suffix == "sv";
+}

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=290744&r1=290743&r2=290744&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Thu Dec 29 22:51:10 2016
@@ -12913,7 +12913,7 @@ bool Sema::CheckLiteralOperatorDeclarati
 //   Literal suffix identifiers that do not start with an underscore
 //   are reserved for future standardization.
 Diag(FnDecl->getLocation(), diag::warn_user_literal_reserved)
-  << NumericLiteralParser::isValidUDSuffix(getLangOpts(), LiteralName);
+  << StringLiteralParser::isValidUDSuffix(getLangOpts(), LiteralName);
   }
 
   return false;

Added: cfe/trunk/test/SemaCXX/cxx1z-user-defined-literals.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1z-user-defined-literals.cpp?rev=290744&view=auto
==
--- cfe/trunk/test/SemaCXX/cxx1z-user-defined-literals.cpp (added)
+++ cfe/trunk/test/SemaCXX/cxx1z-user-defined-literals.cpp Thu Dec 29 22:51:10 
2016
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -std=c++1z %s -include %s -verify
+
+#ifndef INCLUDED
+#define INCLUDED
+
+#pragma clang system_header
+namespace std {
+  using size_t = decltype(sizeof(0));
+
+  struct string_view {};
+  string_view operator""sv(const char*, size_t);
+}
+
+#else
+
+using namespace std;
+string_view s = "foo"sv;
+const char* p = "bar"sv; // expected-error {{no viable conversion}}
+char error = 'x'sv; // expected-error {{invalid suffix}} expected-error 
{{expected ';'}}
+
+#endif


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

[PATCH] D28172: [libcxx] Remove unexpected handlers in C++17

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF created this revision.
EricWF added a reviewer: mclow.lists.
EricWF added subscribers: cfe-commits, mclow.lists.

This patch implements P0003R5 
 which 
removes exception specifications from C++17.

The only changes to the library are removing `set_unexpected`, 
`get_unexpected`, `unexpected`, and `unexpected_handler`. These functions can 
be re-enabled in C++17 using 
`_LIBCPP_ENABLE_CXX17_REMOVED_UNEXPECTED_FUNCTIONS`.

@mclow.lists what do you think about removing stuff is this way?


https://reviews.llvm.org/D28172

Files:
  docs/UsingLibcxx.rst
  include/exception
  test/libcxx/depr/exception.unexpected/get_unexpected.pass.cpp
  test/libcxx/depr/exception.unexpected/set_unexpected.pass.cpp
  test/libcxx/depr/exception.unexpected/unexpected.pass.cpp
  test/libcxx/depr/exception.unexpected/unexpected_disabled_cpp17.fail.cpp
  test/std/depr/exception.unexpected/set.unexpected/get_unexpected.pass.cpp
  test/std/depr/exception.unexpected/set.unexpected/set_unexpected.pass.cpp
  
test/std/depr/exception.unexpected/unexpected.handler/unexpected_handler.pass.cpp
  test/std/depr/exception.unexpected/unexpected/unexpected.pass.cpp
  www/cxx1z_status.html

Index: www/cxx1z_status.html
===
--- www/cxx1z_status.html
+++ www/cxx1z_status.html
@@ -122,7 +122,7 @@
 	http://wg21.link/p0393r3";>p0393r3LWGMaking Variant Greater EqualOuluComplete4.0
 	http://wg21.link/P0394r4";>P0394r4LWGHotel Parallelifornia: terminate() for Parallel Algorithms Exception HandlingOulu
   	
-	http://wg21.link/P0003R5";>P0003R5LWGRemoving Deprecated Exception Specifications from C++17Issaquah
+	http://wg21.link/P0003R5";>P0003R5LWGRemoving Deprecated Exception Specifications from C++17IssaquahComplete4.0
 	http://wg21.link/P0067R5";>P0067R5LWGElementary string conversions, revision 5Issaquah
 	http://wg21.link/P0403R1";>P0403R1LWGLiteral suffixes for basic_string_viewIssaquah
 	http://wg21.link/P0414R2";>P0414R2LWGMerging shared_ptr changes from Library Fundamentals to C++17Issaquah
Index: test/std/depr/exception.unexpected/unexpected/unexpected.pass.cpp
===
--- test/std/depr/exception.unexpected/unexpected/unexpected.pass.cpp
+++ test/std/depr/exception.unexpected/unexpected/unexpected.pass.cpp
@@ -7,6 +7,8 @@
 //
 //===--===//
 
+// REQUIRES-ANY: c++98, c++03, c++11, c++14
+
 // test unexpected
 
 #include 
Index: test/std/depr/exception.unexpected/unexpected.handler/unexpected_handler.pass.cpp
===
--- test/std/depr/exception.unexpected/unexpected.handler/unexpected_handler.pass.cpp
+++ test/std/depr/exception.unexpected/unexpected.handler/unexpected_handler.pass.cpp
@@ -7,6 +7,8 @@
 //
 //===--===//
 
+// REQUIRES-ANY: c++98, c++03, c++11, c++14
+
 // test unexpected_handler
 
 #include 
Index: test/std/depr/exception.unexpected/set.unexpected/set_unexpected.pass.cpp
===
--- test/std/depr/exception.unexpected/set.unexpected/set_unexpected.pass.cpp
+++ test/std/depr/exception.unexpected/set.unexpected/set_unexpected.pass.cpp
@@ -7,6 +7,8 @@
 //
 //===--===//
 
+// REQUIRES-ANY: c++98, c++03, c++11, c++14
+
 // test set_unexpected
 
 #include 
Index: test/std/depr/exception.unexpected/set.unexpected/get_unexpected.pass.cpp
===
--- test/std/depr/exception.unexpected/set.unexpected/get_unexpected.pass.cpp
+++ test/std/depr/exception.unexpected/set.unexpected/get_unexpected.pass.cpp
@@ -7,6 +7,8 @@
 //
 //===--===//
 
+// REQUIRES-ANY: c++98, c++03, c++11, c++14
+
 // test get_unexpected
 
 #include 
Index: test/libcxx/depr/exception.unexpected/unexpected_disabled_cpp17.fail.cpp
===
--- /dev/null
+++ test/libcxx/depr/exception.unexpected/unexpected_disabled_cpp17.fail.cpp
@@ -0,0 +1,23 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+
+// test unexpected
+
+#include 
+
+void f() {}
+
+int main() {
+  using T = std::unexpected_handler; // expected-error {{no type named 'unexpected_handler' in namespace 'std'}}
+  std::unexpected(); // expected-error {{no member name

r290743 - Remove bogus assertion and add testcase that triggers it.

2016-12-29 Thread Richard Smith via cfe-commits

Author: rsmith
Date: Thu Dec 29 22:32:02 2016
New Revision: 290743

URL: http://llvm.org/viewvc/llvm-project?rev=290743&view=rev
Log:
Remove bogus assertion and add testcase that triggers it.

Modified:
cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp?rev=290743&r1=290742&r2=290743&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateDeduction.cpp Thu Dec 29 22:32:02 2016
@@ -380,8 +380,6 @@ static Sema::TemplateDeductionResult Ded
 Sema &S, TemplateParameterList *TemplateParams,
 NonTypeTemplateParmDecl *NTTP, Expr *Value, TemplateDeductionInfo &Info,
 SmallVectorImpl &Deduced) {
-  assert((Value->isTypeDependent() || Value->isValueDependent()) &&
- "Expression template argument must be type- or value-dependent.");
   return DeduceNonTypeTemplateArgument(S, TemplateParams, NTTP,
DeducedTemplateArgument(Value),
Value->getType(), Info, Deduced);
@@ -4363,6 +4361,10 @@ static bool isAtLeastAsSpecializedAs(Sem
 if (Deduced[ArgIdx].isNull())
   break;
 
+  // FIXME: We fail to implement [temp.deduct.type]p1 along this path. We need
+  // to substitute the deduced arguments back into the template and check that
+  // we get the right type.
+
   if (ArgIdx == NumArgs) {
 // All template arguments were deduced. FT1 is at least as specialized
 // as FT2.

Modified: cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp?rev=290743&r1=290742&r2=290743&view=diff
==
--- cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp (original)
+++ cfe/trunk/test/SemaTemplate/temp_arg_nontype.cpp Thu Dec 29 22:32:02 2016
@@ -434,3 +434,20 @@ namespace dependent_nested_partial_speci
   };
   E::F e1; // expected-note {{instantiation of}}
 }
+
+namespace nondependent_default_arg_ordering {
+  int n, m;
+  template struct X {};
+  template void f(X); // expected-note {{candidate}}
+  template void f(X); // expected-note {{candidate}}
+  template void f(X); // expected-note 2{{candidate}}
+  template class T, typename A, int *B> void f(T); // expected-note 2{{candidate}}
+  void g() {
+// FIXME: The first and second function templates above should be
+// considered more specialized than the last two, but during partial
+// ordering we fail to check that we actually deduced template arguments
+// that make the deduced A identical to A.
+X x; f(x); // expected-error {{ambiguous}}
+X y; f(y); // expected-error {{ambiguous}}
+  }
+}


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

[libcxx] r290742 - Update "Making Optional Greater Equal Again" paper status

2016-12-29 Thread Eric Fiselier via cfe-commits

Author: ericwf
Date: Thu Dec 29 22:16:30 2016
New Revision: 290742

URL: http://llvm.org/viewvc/llvm-project?rev=290742&view=rev
Log:
Update "Making Optional Greater Equal Again" paper status

Modified:
libcxx/trunk/www/cxx1z_status.html

Modified: libcxx/trunk/www/cxx1z_status.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/cxx1z_status.html?rev=290742&r1=290741&r2=290742&view=diff
==
--- libcxx/trunk/www/cxx1z_status.html (original)
+++ libcxx/trunk/www/cxx1z_status.html Thu Dec 29 22:16:30 2016
@@ -112,7 +112,7 @@
http://wg21.link/p0258r2";>p0258r2LWGhas_unique_object_representationsOulu
http://wg21.link/p0295r0";>p0295r0LWGAdopt Selected 
Library Fundamentals V2 Components for 
C++17OuluComplete4.0
http://wg21.link/p0302r1";>p0302r1LWGRemoving 
Allocator Support in 
std::functionOuluComplete4.0
-   http://wg21.link/p0307r2";>p0307r2LWGMaking Optional 
Greater Equal AgainOulu
+   http://wg21.link/p0307r2";>p0307r2LWGMaking Optional 
Greater Equal AgainOuluComplete4.0
http://wg21.link/p0336r1";>p0336r1LWGBetter Names 
for Parallel Execution Policies in 
C++17Oulu
http://wg21.link/p0337r0";>p0337r0LWGDelete 
operator= for 
polymorphic_allocatorOuluComplete3.9
http://wg21.link/p0346r1";>p0346r1LWGA 
 Nomenclature 
TweakOuluComplete3.9


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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Mehdi AMINI via Phabricator via cfe-commits

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

LGTM.


https://reviews.llvm.org/D28153



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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexshap added inline comments.



Comment at: utils/perf-training/CMakeLists.txt:61
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+

alexshap wrote:
> mehdi_amini wrote:
> > I'm missing something: the code in the main CMakeLists seems to allows to 
> > have an empty value for the CLANG_ORDER_FILE?
> if i understand correctly: CMakeLists.txt: lines 424 - 433:
> if(CLANG_ORDER_FILE STREQUAL "")
> unset(CLANG_ORDER_FILE CACHE)
> unset(CLANG_ORDER_FILE)
>   endif()
> 
> set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
> 
> but the file (CLANG_ORDER_FILE) itself can be empty, not the variable. If 
> CLANG_ORDER_FILE is empty that command line won't be correct
> 
i've added this check to catch potential issues here earlier.


https://reviews.llvm.org/D28153



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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexshap removed rL LLVM as the repository for this revision.
alexshap updated this revision to Diff 82715.
alexshap added a comment.

upd


https://reviews.llvm.org/D28153

Files:
  CMakeLists.txt
  utils/perf-training/CMakeLists.txt


Index: utils/perf-training/CMakeLists.txt
===
--- utils/perf-training/CMakeLists.txt
+++ utils/perf-training/CMakeLists.txt
@@ -40,7 +40,7 @@
 endif()
 
 find_program(DTRACE dtrace)
-if(DTRACE)
+if(APPLE AND DTRACE)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/order-files.lit.site.cfg.in
 ${CMAKE_CURRENT_BINARY_DIR}/order-files/lit.site.cfg
@@ -56,6 +56,10 @@
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py 
clean ${CMAKE_CURRENT_BINARY_DIR} dtrace
 COMMENT "Clearing old dtrace data")
 
+  if(NOT CLANG_ORDER_FILE)
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+
   add_custom_target(generate-order-file
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py 
gen-order-file --binary $ --output ${CLANG_ORDER_FILE} 
${CMAKE_CURRENT_BINARY_DIR}
 COMMENT "Generating order file"
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -421,6 +421,29 @@
 endif()
 add_subdirectory(examples)
 
+if(APPLE)
+  # this line is needed as a cleanup to ensure that any CMakeCaches with the 
old
+  # default value get updated to the new default.
+  if(CLANG_ORDER_FILE STREQUAL "")
+unset(CLANG_ORDER_FILE CACHE)
+unset(CLANG_ORDER_FILE)
+  endif()
+
+
+  set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
+"Order file to use when compiling clang in order to improve startup time 
(Darwin Only - requires ld64).")
+
+  if(NOT EXISTS ${CLANG_ORDER_FILE})
+string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)
+if(PATH_START EQUAL 0)
+  file(WRITE ${CLANG_ORDER_FILE} "\n")
+else()
+  message(FATAL_ERROR "Specified order file '${CLANG_ORDER_FILE}' does not 
exist.")
+endif()
+  endif()
+endif()
+
+
 if( CLANG_INCLUDE_TESTS )
   if(EXISTS 
${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include/gtest/gtest.h)
 add_subdirectory(unittests)
@@ -455,29 +478,6 @@
   add_subdirectory(docs)
 endif()
 
-
-if(APPLE)
-  # this line is needed as a cleanup to ensure that any CMakeCaches with the 
old
-  # default value get updated to the new default.
-  if(CLANG_ORDER_FILE STREQUAL "")
-unset(CLANG_ORDER_FILE CACHE)
-unset(CLANG_ORDER_FILE)
-  endif()
-
-
-  set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
-"Order file to use when compiling clang in order to improve startup time 
(Darwin Only - requires ld64).")
-
-  if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE})
-string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)
-if(PATH_START EQUAL 0)
-  file(WRITE ${CLANG_ORDER_FILE} "\n")
-else()
-  message(FATAL_ERROR "Specified order file '${CLANG_ORDER_FILE}' does not 
exist.")
-endif()
-  endif()
-endif()
-
 add_subdirectory(cmake/modules)
 
 if(CLANG_STAGE)


Index: utils/perf-training/CMakeLists.txt
===
--- utils/perf-training/CMakeLists.txt
+++ utils/perf-training/CMakeLists.txt
@@ -40,7 +40,7 @@
 endif()
 
 find_program(DTRACE dtrace)
-if(DTRACE)
+if(APPLE AND DTRACE)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/order-files.lit.site.cfg.in
 ${CMAKE_CURRENT_BINARY_DIR}/order-files/lit.site.cfg
@@ -56,6 +56,10 @@
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py clean ${CMAKE_CURRENT_BINARY_DIR} dtrace
 COMMENT "Clearing old dtrace data")
 
+  if(NOT CLANG_ORDER_FILE)
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+
   add_custom_target(generate-order-file
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py gen-order-file --binary $ --output ${CLANG_ORDER_FILE} ${CMAKE_CURRENT_BINARY_DIR}
 COMMENT "Generating order file"
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -421,6 +421,29 @@
 endif()
 add_subdirectory(examples)
 
+if(APPLE)
+  # this line is needed as a cleanup to ensure that any CMakeCaches with the old
+  # default value get updated to the new default.
+  if(CLANG_ORDER_FILE STREQUAL "")
+unset(CLANG_ORDER_FILE CACHE)
+unset(CLANG_ORDER_FILE)
+  endif()
+
+
+  set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
+"Order file to use when compiling clang in order to improve startup time (Darwin Only - requires ld64).")
+
+  if(NOT EXISTS ${CLANG_ORDER_FILE})
+string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)
+if(PATH_START EQUAL 0)
+  file(WRITE ${CLANG_ORDER_FILE} "\n")
+

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added a comment.

I mean the two checks being out-of-sync is weird, so I rather have them 
reconciled.


Repository:
  rL LLVM

https://reviews.llvm.org/D28153



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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexshap added inline comments.



Comment at: CMakeLists.txt:436
+
+  if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE})
+string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)

mehdi_amini wrote:
> So why `if(CLANG_ORDER_FILE ` here? 
i don't know why originally it was this way, 
if my understanding is correct - 
you are right - if(CLANG_ORDER_FILE  is not necessary here -
this condition should be replaced just with if(NOT EXISTS ${CLANG_ORDER_FILE})
so yea, i can update this as well.
In the first version of this patch i moved this entire block "as is" without 
changes.
(this resolved the issue with the empty CLANG_ORDER_FILE in 
utils/perf-training/CMakeLists.txt).


Repository:
  rL LLVM

https://reviews.llvm.org/D28153



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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added inline comments.



Comment at: CMakeLists.txt:436
+
+  if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE})
+string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)

So why `if(CLANG_ORDER_FILE ` here? 


Repository:
  rL LLVM

https://reviews.llvm.org/D28153



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

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread S. Gilles via Phabricator via cfe-commits

sgilles added a comment.

Thank you for the comments, rsmith.  I'm addressing them now, and I'll make 
sure to add your examples to the test case.  I don't think `isSyntactic()` 
exists, so I'm using `!getSyntactic()` instead, which should have the desired 
effect.

In https://reviews.llvm.org/D28148#632484, @fhahn wrote:

> Ops, it looks like I accidentally switched the diff back to the initial 
> version and I have no idea how to switch it back :(
>
> I am very sorry about that, would you mind uploading the latest version again?

No problem - I'll upload a new version to address rsmith's comments in a bit.

In https://reviews.llvm.org/D28148#632541, @Quuxplusone wrote:

> (IOW, it looks as if this patch proposes to fix the noisy diagnostic for 
> every language //except for// C-sometimes-compiled-as-C++, and as a sometime 
> writer of C-sometimes-compiled-as-C++ myself, that irks me. Apologies if I've 
> misconstrued the patch.)

My intention was only to fix the noisy diagnostic only for C compiled as C, 
because that's my particular use case and I didn't want to accidentally remove 
valuable warnings for other use cases.  Extending this to the case of 
C-sometimes-compiled-as-C++ should be as easy as dropping the 
`getLangOpts().CPlusPlus`.  But is this warning useful for C++-compiled-as-C++? 
 Perhaps it would be better to wait until explicit recognition of C-as-C++ to 
do that.

https://reviews.llvm.org/D28148

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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexshap added inline comments.



Comment at: utils/perf-training/CMakeLists.txt:61
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+

mehdi_amini wrote:
> I'm missing something: the code in the main CMakeLists seems to allows to 
> have an empty value for the CLANG_ORDER_FILE?
if i understand correctly: CMakeLists.txt: lines 424 - 433:
if(CLANG_ORDER_FILE STREQUAL "")
unset(CLANG_ORDER_FILE CACHE)
unset(CLANG_ORDER_FILE)
  endif()

set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH

but the file (CLANG_ORDER_FILE) itself can be empty, not the variable. If 
CLANG_ORDER_FILE is empty that command line won't be correct



Repository:
  rL LLVM

https://reviews.llvm.org/D28153



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

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread John McCall via Phabricator via cfe-commits

rjmccall added a comment.

In https://reviews.llvm.org/D28148#632541, @Quuxplusone wrote:

> Re the language option: In https://reviews.llvm.org/D27163, @rsmith wrote:
>
> > Perhaps some catch-all "I want defined behavior for things that C defines 
> > even though I'm compiling in C++" flag would make sense here?
>
> It didn't happen then, but I'd like to re-raise the idea. There's probably 
> plenty of code (e.g. Apple kernel drivers, judging from @rjmccall's comments 
> on https://reviews.llvm.org/D27163) that's written in the common subset of C 
> and C++ but compiled as C++. It's unfortunate that that workflow keeps 
> getting "broken" by aggressive diagnostics (in this case) and aggressive 
> optimizations (in that case). I'd like if Clang provided a way for the user 
> to explicitly opt-in and say "Some or all of this code is C compiled as C++; 
> please respect that."
>
> (IOW, it looks as if this patch proposes to fix the noisy diagnostic for 
> every language //except for// C-sometimes-compiled-as-C++, and as a sometime 
> writer of C-sometimes-compiled-as-C++ myself, that irks me. Apologies if I've 
> misconstrued the patch.)


C++, for all its problems, does fix some things about C.  I don't think it 
makes all that much sense to support a general "this is C being compiled as 
C++" mode, especially for diagnostics where (generally) compiling things as C++ 
ends up applying a more sensible language rule than C provides.  We tend to 
have a lot more diagnostics that are basically pointing out flaws in C that C++ 
fixes than the other way around.


https://reviews.llvm.org/D28148



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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Mehdi AMINI via Phabricator via cfe-commits

mehdi_amini added inline comments.



Comment at: utils/perf-training/CMakeLists.txt:61
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+

I'm missing something: the code in the main CMakeLists seems to allows to have 
an empty value for the CLANG_ORDER_FILE?


Repository:
  rL LLVM

https://reviews.llvm.org/D28153



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

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-12-29 Thread Saleem Abdulrasool via Phabricator via cfe-commits

compnerd added a comment.

This would need an upgrade path, since the output name can actually be 
different from the `AchTypeName` (e.g. i386 vs i686).


https://reviews.llvm.org/D26796



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

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-29 Thread Antonio Maiorano via Phabricator via cfe-commits

amaiorano updated this revision to Diff 82708.
amaiorano added a comment.

Updated with the changes @ioeric suggested.


https://reviews.llvm.org/D28081

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Tooling/Refactoring.cpp
  tools/clang-format/ClangFormat.cpp
  unittests/Format/FormatTest.cpp
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -69,15 +69,15 @@
 };
 
 TEST_F(FormatTestObjC, DetectsObjCInHeaders) {
-  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  Style = *getStyle("LLVM", "a.h", "none", "@interface\n"
   "- (id)init;");
   EXPECT_EQ(FormatStyle::LK_ObjC, Style.Language);
-  Style = getStyle("LLVM", "a.h", "none", "@interface\n"
+  Style = *getStyle("LLVM", "a.h", "none", "@interface\n"
   "+ (id)init;");
   EXPECT_EQ(FormatStyle::LK_ObjC, Style.Language);
 
   // No recognizable ObjC.
-  Style = getStyle("LLVM", "a.h", "none", "void f() {}");
+  Style = *getStyle("LLVM", "a.h", "none", "void f() {}");
   EXPECT_EQ(FormatStyle::LK_Cpp, Style.Language);
 }
 
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -10945,22 +10945,55 @@
   ASSERT_TRUE(
   FS.addFile("/a/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;")));
   auto Style1 = getStyle("file", "/a/.clang-format", "Google", "", &FS);
-  ASSERT_EQ(Style1, getLLVMStyle());
+  ASSERT_TRUE((bool)Style1);
+  ASSERT_EQ(*Style1, getLLVMStyle());
 
   // Test 2: fallback to default.
   ASSERT_TRUE(
   FS.addFile("/b/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;")));
   auto Style2 = getStyle("file", "/b/test.cpp", "Mozilla", "", &FS);
-  ASSERT_EQ(Style2, getMozillaStyle());
+  ASSERT_TRUE((bool)Style2);
+  ASSERT_EQ(*Style2, getMozillaStyle());
 
   // Test 3: format file in parent directory.
   ASSERT_TRUE(
   FS.addFile("/c/.clang-format", 0,
  llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: Google")));
   ASSERT_TRUE(FS.addFile("/c/sub/sub/sub/test.cpp", 0,
  llvm::MemoryBuffer::getMemBuffer("int i;")));
   auto Style3 = getStyle("file", "/c/sub/sub/sub/test.cpp", "LLVM", "", &FS);
-  ASSERT_EQ(Style3, getGoogleStyle());
+  ASSERT_TRUE((bool)Style3);
+  ASSERT_EQ(*Style3, getGoogleStyle());
+
+  // Test 4: error on invalid fallback style
+  auto Style4 = getStyle("file", "a.h", "KungFu", "", &FS);
+  ASSERT_FALSE((bool)Style4);
+  auto ErrorMsg4 = llvm::toString(Style4.takeError());
+  ASSERT_GT(ErrorMsg4.length(), 0);
+
+  // Test 5: error on invalid yaml on command line
+  auto Style5 = getStyle("{invalid_key=invalid_value}", "a.h", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style5);
+  auto ErrorMsg5 = llvm::toString(Style5.takeError());
+  ASSERT_GT(ErrorMsg5.length(), 0);
+
+  // Test 6: error on invalid style
+  auto Style6 = getStyle("KungFu", "a.h", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style6);
+  auto ErrorMsg6 = llvm::toString(Style6.takeError());
+  ASSERT_GT(ErrorMsg6.length(), 0);
+
+  // Test 7: found config file, error on parsing it
+  ASSERT_TRUE(
+  FS.addFile("/d/.clang-format", 0,
+ llvm::MemoryBuffer::getMemBuffer("BasedOnStyle: LLVM\n"
+  "InvalidKey: InvalidValue")));
+  ASSERT_TRUE(
+  FS.addFile("/d/test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int i;")));
+  auto Style7 = getStyle("file", "/d/.clang-format", "LLVM", "", &FS);
+  ASSERT_FALSE((bool)Style7);
+  auto ErrorMsg7 = llvm::toString(Style7.takeError());
+  ASSERT_GT(ErrorMsg7.length(), 0);
 }
 
 TEST_F(ReplacementTest, FormatCodeAfterReplacements) {
Index: tools/clang-format/ClangFormat.cpp
===
--- tools/clang-format/ClangFormat.cpp
+++ tools/clang-format/ClangFormat.cpp
@@ -249,8 +249,14 @@
   if (fillRanges(Code.get(), Ranges))
 return true;
   StringRef AssumedFileName = (FileName == "-") ? AssumeFileName : FileName;
-  FormatStyle FormatStyle =
+
+  llvm::Expected FormatStyleOrError =
   getStyle(Style, AssumedFileName, FallbackStyle, Code->getBuffer());
+  if (!FormatStyleOrError) {
+llvm::errs() << llvm::toString(FormatStyleOrError.takeError()) << "\n";
+return true;
+  }
+  FormatStyle FormatStyle = *FormatStyleOrError;
   if (SortIncludes.getNumOccurrences() != 0)
 FormatStyle.SortIncludes = SortIncludes;
   unsigned CursorPosition = Cursor;
@@ -334,10 +340,16 @@
 cl::PrintHelpMessage();
 
   if (DumpConfig) {
-std::string Config =
-clang::format::configurationAsText(clang::format::getStyle(
+llvm::Expected FormatStyleOrError =
+clang::format::getStyle(
 Style, FileNames.

[PATCH] D21675: New ODR checker for modules

2016-12-29 Thread Richard Trieu via Phabricator via cfe-commits

rtrieu updated this revision to Diff 82702.
rtrieu added a comment.
Herald added a subscriber: mgorny.

This is a redesign of the ODR checker which was discovered to have several 
shortcomings when run over test cases.  The old version mainly used depth-first 
processing of AST nodes to gather the information to be hashed.  In some 
instances, a recursive loop developed preventing the termination of the hashing 
function.

This version of the ODR checker will queue Type and Decl pointers into a 
vector.  When the hashing function needs to refer to the pointers, it will use 
the index of the pointers' location in the vector instead.  After any in 
progress hashing is finished, unprocessed pointers in the vector will be 
processed.  Other AST nodes, such as Stmt's and TemplateArgument's are 
processed when received.

The design change also necessitated creation of an ODRHash class to manage the 
queued nodes.  This is in ODRHash.{cpp,h} and most of the hashing logic is also 
moved into those files.  Only the hashing for Stmt is left in StmtProfile.h 
since it shares code with Stmt::Profile.


https://reviews.llvm.org/D21675

Files:
  include/clang/AST/DeclCXX.h
  include/clang/AST/ODRHash.h
  include/clang/AST/Stmt.h
  include/clang/Basic/DiagnosticSerializationKinds.td
  lib/AST/CMakeLists.txt
  lib/AST/DeclCXX.cpp
  lib/AST/ODRHash.cpp
  lib/AST/StmtProfile.cpp
  lib/Sema/SemaDecl.cpp
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTReaderDecl.cpp
  lib/Serialization/ASTWriter.cpp
  test/Modules/merge-using-decls.cpp
  test/Modules/odr_hash.cpp

Index: test/Modules/odr_hash.cpp
===
--- test/Modules/odr_hash.cpp
+++ test/Modules/odr_hash.cpp
@@ -0,0 +1,1077 @@
+// Clear and create directories
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: mkdir %t/cache
+// RUN: mkdir %t/Inputs
+
+// Build first header file
+// RUN: echo "#define FIRST" >> %t/Inputs/first.h
+// RUN: cat %s   >> %t/Inputs/first.h
+
+// Build second header file
+// RUN: echo "#define SECOND" >> %t/Inputs/second.h
+// RUN: cat %s>> %t/Inputs/second.h
+
+// Build module map file
+// RUN: echo "module first {"   >> %t/Inputs/module.map
+// RUN: echo "header \"first.h\""   >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+// RUN: echo "module second {"  >> %t/Inputs/module.map
+// RUN: echo "header \"second.h\""  >> %t/Inputs/module.map
+// RUN: echo "}">> %t/Inputs/module.map
+
+// Run test
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -x c++ -I%t/Inputs -verify %s -std=c++11
+
+#if !defined(FIRST) && !defined(SECOND)
+#include "first.h"
+#include "second.h"
+#endif
+
+#if defined(FIRST)
+struct S1 {
+  public:
+};
+#elif defined(SECOND)
+struct S1 {
+  private:
+};
+#else
+S1 s1;
+// expected-error@first.h:* {{'S1' has different definitions in different modules; first difference is definition in module 'first' found public access specifier}}
+// expected-note@second.h:* {{but in 'second' found private access specifier}}
+#endif
+
+#if defined(FIRST)
+struct S2Friend2 {};
+struct S2 {
+  friend S2Friend2;
+};
+#elif defined(SECOND)
+struct S2Friend1 {};
+struct S2 {
+  friend S2Friend1;
+};
+#else
+S2 s2;
+// expected-error@first.h:* {{'S2' has different definitions in different modules; first difference is definition in module 'first' found friend 'S2Friend2'}}
+// expected-note@second.h:* {{but in 'second' found other friend 'S2Friend1'}}
+#endif
+
+#if defined(FIRST)
+template
+struct S3Template {};
+struct S3 {
+  friend S3Template;
+};
+#elif defined(SECOND)
+template
+struct S3Template {};
+struct S3 {
+  friend S3Template;
+};
+#else
+S3 s3;
+// expected-error@first.h:* {{'S3' has different definitions in different modules; first difference is definition in module 'first' found friend 'S3Template'}}
+// expected-note@second.h:* {{but in 'second' found other friend 'S3Template'}}
+#endif
+
+#if defined(FIRST)
+struct S4 {
+  static_assert(1 == 1, "First");
+};
+#elif defined(SECOND)
+struct S4 {
+  static_assert(1 == 1, "Second");
+};
+#else
+S4 s4;
+// expected-error@first.h:* {{'S4' has different definitions in different modules; first difference is definition in module 'first' found static assert with message}}
+// expected-note@second.h:* {{but in 'second' found static assert with different message}}
+#endif
+
+#if defined(FIRST)
+struct S5 {
+  static_assert(1 == 1, "Message");
+};
+#elif defined(SECOND)
+struct S5 {
+  static_assert(2 == 2, "Message");
+};
+#else
+S5 s5;
+// expected-error@first.h:* {{'S5' has different definitions in different modules; first difference is definition in module 'first' found static assert with condition}}
+// expected-note@second.h:* {{but in 'second' found static assert with different condition}}
+#endif
+
+#if defined(FIRST)
+struct S6 {
+  int First();
+};
+#elif defined(SECOND)

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread Arthur O'Dwyer via Phabricator via cfe-commits

Quuxplusone added subscribers: rjmccall, Quuxplusone.
Quuxplusone added a comment.

Re the language option: In https://reviews.llvm.org/D27163, @rsmith wrote:

> Perhaps some catch-all "I want defined behavior for things that C defines 
> even though I'm compiling in C++" flag would make sense here?

It didn't happen then, but I'd like to re-raise the idea. There's probably 
plenty of code (e.g. Apple kernel drivers, judging from @rjmccall's comments on 
https://reviews.llvm.org/D27163) that's written in the common subset of C and 
C++ but compiled as C++. It's unfortunate that that workflow keeps getting 
"broken" by aggressive diagnostics (in this case) and aggressive optimizations 
(in that case). I'd like if Clang provided a way for the user to explicitly 
opt-in and say "Some or all of this code is C compiled as C++; please respect 
that."

(IOW, it looks as if this patch proposes to fix the noisy diagnostic for every 
language //except for// C-sometimes-compiled-as-C++, and as a sometime writer 
of C-sometimes-compiled-as-C++ myself, that irks me. Apologies if I've 
misconstrued the patch.)

https://reviews.llvm.org/D28148

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

[PATCH] Add missing Decl::Kind for -print-decl-contexts

2016-12-29 Thread Fangrui Song via cfe-commits

---
 lib/Frontend/ASTConsumers.cpp | 37 -
 1 file changed, 32 insertions(+), 5 deletions(-)

diff --git a/lib/Frontend/ASTConsumers.cpp b/lib/Frontend/ASTConsumers.cpp
index bd2ee06d16..987b8ca398 100644
--- a/lib/Frontend/ASTConsumers.cpp
+++ b/lib/Frontend/ASTConsumers.cpp
@@ -405,6 +405,22 @@ void DeclContextPrinter::PrintDeclContext(const 
DeclContext* DC,
   PrintDeclContext(DC, Indentation+2);
   break;
 }
+case Decl::AccessSpec: {
+  Out << "\n";
+  break;
+}
+case Decl::ClassTemplate: {
+  Out << " " << *cast(I) << '\n';
+  break;
+}
+case Decl::ClassTemplateSpecialization: {
+  Out << " " << 
*cast(I) << '\n';
+  break;
+}
+case Decl::ClassTemplatePartialSpecialization: {
+  Out << " " << 
*cast(I) << '\n';
+  break;
+}
 case Decl::IndirectField: {
   IndirectFieldDecl* IFD = cast(I);
   Out << " " << *IFD << '\n';
@@ -420,6 +436,10 @@ void DeclContextPrinter::PrintDeclContext(const 
DeclContext* DC,
   Out << " " << *FD << '\n';
   break;
 }
+case Decl::Friend: {
+  Out << "\n";
+  break;
+}
 case Decl::Typedef:
 case Decl::TypeAlias: {
   TypedefNameDecl* TD = cast(I);
@@ -460,6 +480,14 @@ void DeclContextPrinter::PrintDeclContext(const 
DeclContext* DC,
   Out << "\n";
   break;
 }
+case Decl::Using: {
+  Out << " " << *cast(I) << '\n';
+  break;
+}
+case Decl::UsingShadow: {
+  Out << " " << *cast(I) << '\n';
+  break;
+}
 case Decl::UsingDirective: {
   Out << "\n";
   break;
@@ -469,15 +497,14 @@ void DeclContextPrinter::PrintDeclContext(const 
DeclContext* DC,
   Out << " " << *NAD << '\n';
   break;
 }
-case Decl::ClassTemplate: {
-  ClassTemplateDecl *CTD = cast(I);
-  Out << " " << *CTD << '\n';
-  break;
-}
 case Decl::OMPThreadPrivate: {
   Out << " " << '"' << I << "\"\n";
   break;
 }
+case Decl::VarTemplate: {
+  Out << " " << *cast(I) << '\n';
+  break;
+}
 default:
   Out << "DeclKind: " << DK << '"' << I << "\"\n";
   llvm_unreachable("decl unhandled");
-- 
2.11.0

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

[PATCH] D28153: [clang] Fix clean build of generate-order-file

2016-12-29 Thread Alexander Shaposhnikov via Phabricator via cfe-commits

alexshap updated this revision to Diff 82703.
alexshap added a comment.

minor changes


Repository:
  rL LLVM

https://reviews.llvm.org/D28153

Files:
  CMakeLists.txt
  utils/perf-training/CMakeLists.txt


Index: utils/perf-training/CMakeLists.txt
===
--- utils/perf-training/CMakeLists.txt
+++ utils/perf-training/CMakeLists.txt
@@ -40,7 +40,7 @@
 endif()
 
 find_program(DTRACE dtrace)
-if(DTRACE)
+if(APPLE AND DTRACE)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/order-files.lit.site.cfg.in
 ${CMAKE_CURRENT_BINARY_DIR}/order-files/lit.site.cfg
@@ -56,6 +56,10 @@
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py 
clean ${CMAKE_CURRENT_BINARY_DIR} dtrace
 COMMENT "Clearing old dtrace data")
 
+  if(NOT CLANG_ORDER_FILE)
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+
   add_custom_target(generate-order-file
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py 
gen-order-file --binary $ --output ${CLANG_ORDER_FILE} 
${CMAKE_CURRENT_BINARY_DIR}
 COMMENT "Generating order file"
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -421,6 +421,29 @@
 endif()
 add_subdirectory(examples)
 
+if(APPLE)
+  # this line is needed as a cleanup to ensure that any CMakeCaches with the 
old
+  # default value get updated to the new default.
+  if(CLANG_ORDER_FILE STREQUAL "")
+unset(CLANG_ORDER_FILE CACHE)
+unset(CLANG_ORDER_FILE)
+  endif()
+
+
+  set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
+"Order file to use when compiling clang in order to improve startup time 
(Darwin Only - requires ld64).")
+
+  if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE})
+string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)
+if(PATH_START EQUAL 0)
+  file(WRITE ${CLANG_ORDER_FILE} "\n")
+else()
+  message(FATAL_ERROR "Specified order file '${CLANG_ORDER_FILE}' does not 
exist.")
+endif()
+  endif()
+endif()
+
+
 if( CLANG_INCLUDE_TESTS )
   if(EXISTS 
${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include/gtest/gtest.h)
 add_subdirectory(unittests)
@@ -455,29 +478,6 @@
   add_subdirectory(docs)
 endif()
 
-
-if(APPLE)
-  # this line is needed as a cleanup to ensure that any CMakeCaches with the 
old
-  # default value get updated to the new default.
-  if(CLANG_ORDER_FILE STREQUAL "")
-unset(CLANG_ORDER_FILE CACHE)
-unset(CLANG_ORDER_FILE)
-  endif()
-
-
-  set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
-"Order file to use when compiling clang in order to improve startup time 
(Darwin Only - requires ld64).")
-
-  if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE})
-string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)
-if(PATH_START EQUAL 0)
-  file(WRITE ${CLANG_ORDER_FILE} "\n")
-else()
-  message(FATAL_ERROR "Specified order file '${CLANG_ORDER_FILE}' does not 
exist.")
-endif()
-  endif()
-endif()
-
 add_subdirectory(cmake/modules)
 
 if(CLANG_STAGE)


Index: utils/perf-training/CMakeLists.txt
===
--- utils/perf-training/CMakeLists.txt
+++ utils/perf-training/CMakeLists.txt
@@ -40,7 +40,7 @@
 endif()
 
 find_program(DTRACE dtrace)
-if(DTRACE)
+if(APPLE AND DTRACE)
   configure_lit_site_cfg(
 ${CMAKE_CURRENT_SOURCE_DIR}/order-files.lit.site.cfg.in
 ${CMAKE_CURRENT_BINARY_DIR}/order-files/lit.site.cfg
@@ -56,6 +56,10 @@
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py clean ${CMAKE_CURRENT_BINARY_DIR} dtrace
 COMMENT "Clearing old dtrace data")
 
+  if(NOT CLANG_ORDER_FILE)
+message(FATAL_ERROR "Output clang order file is not set")
+  endif()
+
   add_custom_target(generate-order-file
 COMMAND ${PYTHON_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/perf-helper.py gen-order-file --binary $ --output ${CLANG_ORDER_FILE} ${CMAKE_CURRENT_BINARY_DIR}
 COMMENT "Generating order file"
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -421,6 +421,29 @@
 endif()
 add_subdirectory(examples)
 
+if(APPLE)
+  # this line is needed as a cleanup to ensure that any CMakeCaches with the old
+  # default value get updated to the new default.
+  if(CLANG_ORDER_FILE STREQUAL "")
+unset(CLANG_ORDER_FILE CACHE)
+unset(CLANG_ORDER_FILE)
+  endif()
+
+
+  set(CLANG_ORDER_FILE ${CMAKE_CURRENT_BINARY_DIR}/clang.order CACHE FILEPATH
+"Order file to use when compiling clang in order to improve startup time (Darwin Only - requires ld64).")
+
+  if(CLANG_ORDER_FILE AND NOT EXISTS ${CLANG_ORDER_FILE})
+string(FIND "${CLANG_ORDER_FILE}" "${CMAKE_CURRENT_BINARY_DIR}" PATH_START)
+if(PATH_START EQUAL 0)
+  file(WRITE ${CLANG_ORDER_F

[PATCH] D28131: [libcxx] Fix PR31402: map::__find_equal_key has undefined behavior.

2016-12-29 Thread Vedant Kumar via Phabricator via cfe-commits

vsk added a comment.

LGTM. I'm not sure what to do for a test. Have you tried checking the IR for 
the affected object file in tablegen at '-O2 -fsanitize=undefined'? If there's 
an unconditional call to the ubsan handler, maybe the tablegen source could be 
used to find a small reproducer?


https://reviews.llvm.org/D28131



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

[PATCH] D28166: Properly merge K&R functions that have attributes

2016-12-29 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:7464-7470
+const Type *NonAttributedFTy = R.getTypePtr();
+while (const auto *AttrTy = NonAttributedFTy->getAs()) {
+  NonAttributedFTy = AttrTy->getModifiedType().getTypePtr();
+}
 bool HasPrototype =
   (D.isFunctionDeclarator() && D.getFunctionTypeInfo().hasPrototype) ||
+  (!isa(NonAttributedFTy) && R->isFunctionProtoType());

rsmith wrote:
> Rather than hardcoding the forms of type sugar that can appear here, can we 
> just use `R.getTypePtr()->getAs()`? I expect we can also have 
> `ParenType`s wrapping the `FunctionNoProtoType` (`int (f)();`).
Good point about `ParenType`, but no, you cannot use 
`R->getAs()`, because that strips off all sugar, including 
typedefs. However, I share your concern about hardcoding the forms of sugar. 
Perhaps we should add `getAsAdjusted()` like you were suggesting for 
`TypeLoc` to `Type` as well?


https://reviews.llvm.org/D28166



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

[libcxx] r290727 - Fix build using the buildit script

2016-12-29 Thread Eric Fiselier via cfe-commits

Author: ericwf
Date: Thu Dec 29 16:42:45 2016
New Revision: 290727

URL: http://llvm.org/viewvc/llvm-project?rev=290727&view=rev
Log:
Fix build using the buildit script

Modified:
libcxx/trunk/lib/buildit

Modified: libcxx/trunk/lib/buildit
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/lib/buildit?rev=290727&r1=290726&r2=290727&view=diff
==
--- libcxx/trunk/lib/buildit (original)
+++ libcxx/trunk/lib/buildit Thu Dec 29 16:42:45 2016
@@ -41,7 +41,7 @@ then
 fi
 
 EXTRA_FLAGS="-nostdinc++ -std=${CXX_LANG} -fstrict-aliasing -Wall -Wextra 
-Wshadow -Wconversion \
- -Wstrict-aliasing=2 -Wstrict-overflow=4 "
+ -Wstrict-aliasing=2 -Wstrict-overflow=4 
-D_LIBCPP_BUILDING_LIBRARY "
 
 case $TRIPLE in
   *-apple-*)


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

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread Florian Hahn via Phabricator via cfe-commits

fhahn added a comment.

Ops, it looks like I accidentally switched the diff back to the initial version 
and I have no idea how to switch it back :(

I am very sorry about that, would you mind uploading the latest version again?


https://reviews.llvm.org/D28148



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

[PATCH] D28145: [OpenMP] Basic support for a parallel directive in a target region on an NVPTX device.

2016-12-29 Thread Arpith Jacob via Phabricator via cfe-commits

arpith-jacob added inline comments.

Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:114
   /// \brief Get the name of the capture helper.
-  StringRef getHelperName() const override { return ".omp_outlined."; }
+  StringRef getHelperName() const override { return "__omp_outlined__"; }

ABataev wrote:
> arpith-jacob wrote:
> > On the nvptx device, it is illegal for an identifier to contain a dot ('.') 
> > so I've modified it here.  If there is a better way to do this, please let 
> > me know.
> Could you just override this function in CGOpenMPRuntimeNVPTX?
Alexey, thank you for your review of this patch.

To override getHelperName() in CGOpenMPRuntime.cpp, I will have to move the two 
classes  CGOpenMPRegionInfo and CGOpenMPOutlinedRegionInfo from the anonymous 
namespace to the header file in CGOpenMPRuntime.h (under protected mode).  I 
would prefer to do this since I will need to use these classes for implementing 
codegen of other directives in the future.

Is that okay with you?

https://reviews.llvm.org/D28145

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

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread Florian Hahn via Phabricator via cfe-commits

fhahn updated this revision to Diff 82626.

https://reviews.llvm.org/D28148

Files:
  include/clang/Basic/LangOptions.def
  include/clang/Frontend/LangStandard.h
  include/clang/Frontend/LangStandards.def
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaInit.cpp
  test/Sema/zero-initializer.c

Index: test/Sema/zero-initializer.c
===
--- /dev/null
+++ test/Sema/zero-initializer.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -std=c99 -Wmissing-field-initializers -Wmissing-braces
+
+struct foo {
+/* */
+int x;
+int y;
+};
+
+struct bar {
+/* */
+struct foo a;
+struct foo b;
+};
+
+int main(void)
+{
+struct foo f = { 0 }; // expected-no-diagnostics
+struct foo g = { 9 }; // expected-warning {{missing field 'y' initializer}}
+struct foo h = { 9, 9 }; // expected-no-diagnostics
+struct bar i = { 0 }; // expected-no-diagnostics
+struct bar j = { 0, 0 }; // expected-warning {{suggest braces around initialization of suboject}} expected-warning {{missing field 'b' initializer}}
+struct bar k = { { 9, 9 }, { 9, 9 } }; // expected-no-diagnostics
+struct bar l = { { 9, 9 }, { 0 } }; // expected-no-diagnostics
+struct bar m = { { 0 }, { 0 } }; // expected-no-diagnostics
+struct bar n = { { 0 }, { 9, 9 } }; // expected-no-diagnostics
+struct bar o = { { 9, 9 }, { 0 } }; // expected-no-diagnostics
+struct bar p = { { 9 }, { 9, 9 } }; // expected-warning {{missing field 'y' initializer}}
+
+return f.x + g.x + h.x + i.a.x + j.a.x + k.a.x + l.a.x + m.a.x + n.a.x +
+   o.a.x + p.a.x;
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -880,8 +880,19 @@
   StructuredSubobjectInitList->setRBraceLoc(EndLoc);
 }
 
+bool MissingBracesOkay = false;
+
+if (SemaRef.getLangOpts().ZeroInitializer &&
+StructuredSubobjectInitList->getNumInits() == 1) {
+  if (const IntegerLiteral *lit = dyn_cast(StructuredSubobjectInitList->getInit(0))) {
+if (lit->getValue() == 0) {
+  MissingBracesOkay = true;
+}
+  }
+}
+
 // Complain about missing braces.
-if (T->isArrayType() || T->isRecordType()) {
+if (!MissingBracesOkay && (T->isArrayType() || T->isRecordType())) {
   SemaRef.Diag(StructuredSubobjectInitList->getLocStart(),
diag::warn_missing_braces)
   << StructuredSubobjectInitList->getSourceRange()
@@ -1828,6 +1839,17 @@
   RecordDecl *RD = DeclType->getAs()->getDecl();
   RecordDecl::field_iterator FieldEnd = RD->field_end();
   bool CheckForMissingFields = true;
+
+  // Check if this is C's zero initializer { 0 }
+  if (SemaRef.getLangOpts().ZeroInitializer &&
+  IList->getNumInits() == 1) {
+if (const IntegerLiteral *lit = dyn_cast(IList->getInit(0))) {
+  if (lit->getValue() == 0) {
+CheckForMissingFields = false;
+  }
+}
+  }
+
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
 
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1599,6 +1599,7 @@
   Opts.GNUInline = Std.isC89();
   Opts.HexFloats = Std.hasHexFloats();
   Opts.ImplicitInt = Std.hasImplicitInt();
+  Opts.ZeroInitializer = Std.hasZeroInitializer();
 
   // Set OpenCL Version.
   Opts.OpenCL = isOpenCL(LangStd) || IK == IK_OpenCL;
Index: include/clang/Frontend/LangStandards.def
===
--- include/clang/Frontend/LangStandards.def
+++ include/clang/Frontend/LangStandards.def
@@ -30,66 +30,66 @@
 // C89-ish modes.
 LANGSTANDARD(c89, "c89",
  "ISO C 1990",
- C89 | ImplicitInt)
+ C89 | ImplicitInt | ZeroInitializer)
 LANGSTANDARD(c90, "c90",
  "ISO C 1990",
- C89 | ImplicitInt)
+ C89 | ImplicitInt | ZeroInitializer)
 LANGSTANDARD(iso9899_1990, "iso9899:1990",
  "ISO C 1990",
- C89 | ImplicitInt)
+ C89 | ImplicitInt | ZeroInitializer)
 
 LANGSTANDARD(c94, "iso9899:199409",
  "ISO C 1990 with amendment 1",
- C89 | Digraphs | ImplicitInt)
+ C89 | Digraphs | ImplicitInt | ZeroInitializer)
 
 LANGSTANDARD(gnu89, "gnu89",
  "ISO C 1990 with GNU extensions",
- LineComment | C89 | Digraphs | GNUMode | ImplicitInt)
+ LineComment | C89 | Digraphs | GNUMode | ImplicitInt | ZeroInitializer)
 LANGSTANDARD(gnu90, "gnu90",
  "ISO C 1990 with GNU extensions",
- LineComment | C89 | Digraphs | GNUMode | ImplicitInt)
+ LineComment | C89 | Digraphs | GNUMode | ImplicitInt | ZeroInitializer)
 
 // C99-ish modes
 L

[PATCH] D16135: Macro Debug Info support in Clang

2016-12-29 Thread Amjad Aboud via Phabricator via cfe-commits

aaboud updated this revision to Diff 82696.
aaboud added a comment.
Herald added subscribers: mgorny, nemanjai.

Improved code based on Richard's comments.
Removed the change to ASTConsumer, now it does not need to know anything about 
PP consumer.
However, I still needed to change the CodeGenerator ASTConsumer to return the 
CGDebugInfo class.

Richard, please review the new patch.
This time I made sure to add the "MacroPPCallbacks.*" new files.


https://reviews.llvm.org/D16135

Files:
  include/clang/CodeGen/ModuleBuilder.h
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CMakeLists.txt
  lib/CodeGen/CodeGenAction.cpp
  lib/CodeGen/MacroPPCallbacks.cpp
  lib/CodeGen/MacroPPCallbacks.h
  lib/CodeGen/ModuleBuilder.cpp
  test/CodeGen/debug-info-global-constant.c

Index: lib/CodeGen/ModuleBuilder.cpp
===
--- lib/CodeGen/ModuleBuilder.cpp
+++ lib/CodeGen/ModuleBuilder.cpp
@@ -65,6 +65,7 @@
 
   private:
 SmallVector DeferredInlineMethodDefinitions;
+CGDebugInfo *DebugInfoRef;
 
   public:
 CodeGeneratorImpl(DiagnosticsEngine &diags, llvm::StringRef ModuleName,
@@ -74,7 +75,8 @@
   CoverageSourceInfo *CoverageInfo = nullptr)
 : Diags(diags), Ctx(nullptr), HeaderSearchOpts(HSO),
   PreprocessorOpts(PPO), CodeGenOpts(CGO), HandlingTopLevelDecls(0),
-  CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)) {
+  CoverageInfo(CoverageInfo), M(new llvm::Module(ModuleName, C)),
+  DebugInfoRef(nullptr) {
   C.setDiscardValueNames(CGO.DiscardValueNames);
 }
 
@@ -92,6 +94,10 @@
   return M.get();
 }
 
+CGDebugInfo *&getModuleDebugInfoRef() {
+  return DebugInfoRef;
+}
+
 llvm::Module *ReleaseModule() {
   return M.release();
 }
@@ -124,6 +130,9 @@
PreprocessorOpts, CodeGenOpts,
*M, Diags, CoverageInfo));
 
+  // Initialize the place holder for the CGDebugInfo.
+  DebugInfoRef = Builder->getModuleDebugInfo();
+
   for (auto &&Lib : CodeGenOpts.DependentLibraries)
 Builder->AddDependentLib(Lib);
   for (auto &&Opt : CodeGenOpts.LinkerOptions)
@@ -299,6 +308,10 @@
   return static_cast(this)->ReleaseModule();
 }
 
+CGDebugInfo *&CodeGenerator::getModuleDebugInfoRef() {
+  return static_cast(this)->getModuleDebugInfoRef();
+}
+
 const Decl *CodeGenerator::GetDeclForMangledName(llvm::StringRef name) {
   return static_cast(this)->GetDeclForMangledName(name);
 }
Index: lib/CodeGen/MacroPPCallbacks.h
===
--- lib/CodeGen/MacroPPCallbacks.h
+++ lib/CodeGen/MacroPPCallbacks.h
@@ -0,0 +1,110 @@
+//===--- MacroPPCallbacks.h -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines implementation for the macro preprocessors callbacks.
+//
+//===--===//
+
+#include "clang/Lex/PPCallbacks.h"
+
+namespace llvm {
+class DIMacroFile;
+class DIMacroNode;
+}
+namespace clang {
+class Preprocessor;
+class MacroInfo;
+
+namespace CodeGen {
+class CGDebugInfo;
+}
+
+class MacroPPCallbacks : public PPCallbacks {
+  /// A reference pointer to debug info code generator.
+  CodeGen::CGDebugInfo *&DebugInfo;
+
+  /// Preprocessor.
+  Preprocessor &PP;
+
+  /// Location of recent included file, used for line number.
+  SourceLocation LastHashLoc;
+
+  /// Location of main file, used for file info.
+  SourceLocation MainFileLoc;
+
+  /// Counts current number of command line included files, which was entered
+  /// and was not exited yet.
+  int CommandIncludeFiles;
+
+  enum FileScopeStatus {
+NoScope,  // Scope is not initialized yet.
+InitializedScope, // Main file scope is initialized but not set yet.
+BuiltinScope, //  file scope.
+CommandLineScope, //  file scope.
+CommandLineScopeIncludes, // Included file, from  file, scope.
+MainFileScope // Main file scope.
+  };
+  FileScopeStatus Status;
+
+  /// Parent contains all entered files that were not exited yet according to
+  /// the inclusion order.
+  llvm::SmallVector Scopes;
+
+  /// Get current DIMacroFile scope.
+  /// \return current DIMacroFile scope or nullptr if there is no such scope.
+  llvm::DIMacroFile *getCurrentScope();
+
+  /// Get current line location or invalid location.
+  /// \param Loc current line location.
+  /// \return current line location \p `Loc`, or invalid location if it's in a
+  /// skipped file scope.
+  SourceLocation getCorrectLocatio

[PATCH] D28166: Properly merge K&R functions that have attributes

2016-12-29 Thread Richard Smith via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:7464-7470
+const Type *NonAttributedFTy = R.getTypePtr();
+while (const auto *AttrTy = NonAttributedFTy->getAs()) {
+  NonAttributedFTy = AttrTy->getModifiedType().getTypePtr();
+}
 bool HasPrototype =
   (D.isFunctionDeclarator() && D.getFunctionTypeInfo().hasPrototype) ||
+  (!isa(NonAttributedFTy) && R->isFunctionProtoType());

Rather than hardcoding the forms of type sugar that can appear here, can we 
just use `R.getTypePtr()->getAs()`? I expect we can also have 
`ParenType`s wrapping the `FunctionNoProtoType` (`int (f)();`).



Comment at: lib/Sema/SemaDecl.cpp:11958-11962
+// The type location may be attributed; strip the attributes to get to
+// the function type location.
+while (auto ATL = TL.getAs()) {
+  TL = ATL.getModifiedLoc();
+}

Again, I don't like having this knowledge about what kinds of type sugar can 
appear in a function declaration hardcoded here. Please put this somewhere more 
central.

A quick look finds that `FunctionDecl::getReturnTypeSourceRange()` gets this 
wrong in the opposite direction: it skips parens but not attributes. Maybe we 
should have a `TypeLoc::getAsAdjusted` or similar, that walks over type 
sugar nodes that represent some kind of type adjustment from a type that was 
written as a T to another type that is still canonically a T (`ParenType`, 
`AttributedType`, `ElaboratedType`).


https://reviews.llvm.org/D28166



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

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-12-29 Thread Evgeniy Stepanov via Phabricator via cfe-commits

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

LGTM


https://reviews.llvm.org/D26796



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

[PATCH] D28166: Properly merge K&R functions that have attributes

2016-12-29 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman created this revision.
aaron.ballman added a reviewer: rsmith.
aaron.ballman added a subscriber: cfe-commits.

When determining whether a function was written with a prototype, we would look 
at the type to see if it was something other than a FunctionType while the 
canonical type was a function with a prototype. We need to ignore attributed 
function types in this check, as they should not generate an incompatible 
function type in C. Fixing this introduced a secondary problem where we were 
assuming that K&R functions could not be attributed types when reporting 
old-style function definitions that are not preceded by a prototype.


https://reviews.llvm.org/D28166

Files:
  lib/Sema/SemaDecl.cpp
  test/Sema/knr-def-call.c
  test/Sema/warn-strict-prototypes.c


Index: test/Sema/warn-strict-prototypes.c
===
--- test/Sema/warn-strict-prototypes.c
+++ test/Sema/warn-strict-prototypes.c
@@ -60,3 +60,8 @@
 // K&R function definition with previous prototype declared is not diagnosed.
 void foo11(int p, int p2);
 void foo11(p, p2) int p; int p2; {}
+
+// PR31020
+void __attribute__((cdecl)) foo12(d) // expected-warning {{this old-style 
function definition is not preceded by a prototype}}
+  short d;
+{}
Index: test/Sema/knr-def-call.c
===
--- test/Sema/knr-def-call.c
+++ test/Sema/knr-def-call.c
@@ -39,3 +39,9 @@
   proto(42.1); // expected-warning{{implicit conversion from 'double' to 'int' 
changes value from 42.1 to 42}}
   (&proto)(42.1); // expected-warning{{implicit conversion from 'double' to 
'int' changes value from 42.1 to 42}}
 }
+
+// PR31020
+void func(short d) __attribute__((cdecl)); // expected-note{{previous 
declaration is here}}
+void __attribute__((cdecl)) func(d)
+  short d; // expected-warning{{promoted type 'int' of K&R function parameter 
is not compatible with the parameter type 'short' declared in a previous 
prototype}}
+{}
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -7458,11 +7458,16 @@
 // Determine whether the function was written with a
 // prototype. This true when:
 //   - there is a prototype in the declarator, or
-//   - the type R of the function is some kind of typedef or other 
reference
-// to a type name (which eventually refers to a function type).
+//   - the type R of the function is some kind of typedef or other non-
+// attributed reference to a type name (which eventually refers to a
+// function type).
+const Type *NonAttributedFTy = R.getTypePtr();
+while (const auto *AttrTy = NonAttributedFTy->getAs()) {
+  NonAttributedFTy = AttrTy->getModifiedType().getTypePtr();
+}
 bool HasPrototype =
   (D.isFunctionDeclarator() && D.getFunctionTypeInfo().hasPrototype) ||
-  (!isa(R.getTypePtr()) && R->isFunctionProtoType());
+  (!isa(NonAttributedFTy) && R->isFunctionProtoType());
 
 NewFD = FunctionDecl::Create(SemaRef.Context, DC,
  D.getLocStart(), NameInfo, R,
@@ -11950,6 +11955,11 @@
   !LangOpts.CPlusPlus) {
 TypeSourceInfo *TI = FD->getTypeSourceInfo();
 TypeLoc TL = TI->getTypeLoc();
+// The type location may be attributed; strip the attributes to get to
+// the function type location.
+while (auto ATL = TL.getAs()) {
+  TL = ATL.getModifiedLoc();
+}
 FunctionTypeLoc FTL = TL.castAs();
 Diag(FTL.getLParenLoc(), diag::warn_strict_prototypes) << 1;
   }


Index: test/Sema/warn-strict-prototypes.c
===
--- test/Sema/warn-strict-prototypes.c
+++ test/Sema/warn-strict-prototypes.c
@@ -60,3 +60,8 @@
 // K&R function definition with previous prototype declared is not diagnosed.
 void foo11(int p, int p2);
 void foo11(p, p2) int p; int p2; {}
+
+// PR31020
+void __attribute__((cdecl)) foo12(d) // expected-warning {{this old-style function definition is not preceded by a prototype}}
+  short d;
+{}
Index: test/Sema/knr-def-call.c
===
--- test/Sema/knr-def-call.c
+++ test/Sema/knr-def-call.c
@@ -39,3 +39,9 @@
   proto(42.1); // expected-warning{{implicit conversion from 'double' to 'int' changes value from 42.1 to 42}}
   (&proto)(42.1); // expected-warning{{implicit conversion from 'double' to 'int' changes value from 42.1 to 42}}
 }
+
+// PR31020
+void func(short d) __attribute__((cdecl)); // expected-note{{previous declaration is here}}
+void __attribute__((cdecl)) func(d)
+  short d; // expected-warning{{promoted type 'int' of K&R function parameter is not compatible with the parameter type 'short' declared in a previous prototype}}
+{}
Index: lib/Sema/SemaDecl.cpp
=

[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer

2016-12-29 Thread Richard Smith via Phabricator via cfe-commits

rsmith added inline comments.



Comment at: lib/Sema/SemaInit.cpp:884
+bool MissingBracesOkay = false;
+
+if (!SemaRef.getLangOpts().CPlusPlus &&

Remove empty line.



Comment at: lib/Sema/SemaInit.cpp:885-892
+if (!SemaRef.getLangOpts().CPlusPlus &&
+StructuredSubobjectInitList->getNumInits() == 1) {
+  if (const IntegerLiteral *lit = 
dyn_cast(StructuredSubobjectInitList->getInit(0))) {
+if (lit->getValue() == 0) {
+  MissingBracesOkay = true;
+}
+  }

I think it would make more sense to check `ParentIList` here instead of 
`StructuredSubobjectInitList` -- we want to check whether the list the user 
wrote in the source code was `{0}`, not the list after semantic checking. This 
would matter for a case like:

```
struct A { int n; };
struct B { struct A a; };
struct C { struct B b; };
C c = {0};
```

`ParentIList` will be `{0}` at every level here, but it looks like 
`StructuredSubobjectInitList` will be `{{0}}` when checking the initialization 
of `c.b`, so you won't suppress the warning.

It would also matter for a case like

```
struct A { short p; };
struct B { A a; };
B b = {0};
```

where the list after semantic checking will have an implicit conversion wrapped 
around the initializer.



Comment at: lib/Sema/SemaInit.cpp:1843-1851
+  // Check if this is C's zero initializer { 0 }
+  if (!SemaRef.getLangOpts().CPlusPlus &&
+  IList->getNumInits() == 1) {
+if (const IntegerLiteral *lit = 
dyn_cast(IList->getInit(0))) {
+  if (lit->getValue() == 0) {
+CheckForMissingFields = false;
+  }

Please factor this check out into something like 
`InitListExpr::isIdiomaticZeroInitializer()`. It would make sense for that 
function to also assert `isSyntactic()`.


https://reviews.llvm.org/D28148



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

[PATCH] D28165: Change clang-format's Chromium JavaScript defaults

2016-12-29 Thread Dan Beam via Phabricator via cfe-commits

danbeam created this revision.
danbeam added a reviewer: thakis.
danbeam added a subscriber: cfe-commits.
Herald added a subscriber: klimek.

Chromium is starting to use clang-format on more JavaScript.

In doing this, we discovered that our defaults were not doing a good job 
differentiating between JS and C++.

This change moves some defaults to only apply to C++.


https://reviews.llvm.org/D28165

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -639,13 +639,17 @@
 ChromiumStyle.ContinuationIndentWidth = 8;
 ChromiumStyle.IndentWidth = 4;
   } else {
+ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
+ChromiumStyle.AllowShortLoopsOnASingleLine = false;
+  }
+
+  if (Language == FormatStyle::LK_Cpp) {
 ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
 ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
-ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
-ChromiumStyle.AllowShortLoopsOnASingleLine = false;
 ChromiumStyle.BinPackParameters = false;
 ChromiumStyle.DerivePointerAlignment = false;
   }
+
   ChromiumStyle.SortIncludes = false;
   return ChromiumStyle;
 }


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -639,13 +639,17 @@
 ChromiumStyle.ContinuationIndentWidth = 8;
 ChromiumStyle.IndentWidth = 4;
   } else {
+ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
+ChromiumStyle.AllowShortLoopsOnASingleLine = false;
+  }
+
+  if (Language == FormatStyle::LK_Cpp) {
 ChromiumStyle.AllowAllParametersOfDeclarationOnNextLine = false;
 ChromiumStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
-ChromiumStyle.AllowShortIfStatementsOnASingleLine = false;
-ChromiumStyle.AllowShortLoopsOnASingleLine = false;
 ChromiumStyle.BinPackParameters = false;
 ChromiumStyle.DerivePointerAlignment = false;
   }
+
   ChromiumStyle.SortIncludes = false;
   return ChromiumStyle;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r290721 - Fix PR31489 - std::function self-swap segfaults

2016-12-29 Thread Eric Fiselier via cfe-commits

Author: ericwf
Date: Thu Dec 29 14:03:55 2016
New Revision: 290721

URL: http://llvm.org/viewvc/llvm-project?rev=290721&view=rev
Log:
Fix PR31489 - std::function self-swap segfaults

Modified:
libcxx/trunk/include/__functional_03
libcxx/trunk/include/functional

libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp

libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.mod/swap.pass.cpp

Modified: libcxx/trunk/include/__functional_03
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/__functional_03?rev=290721&r1=290720&r2=290721&view=diff
==
--- libcxx/trunk/include/__functional_03 (original)
+++ libcxx/trunk/include/__functional_03 Thu Dec 29 14:03:55 2016
@@ -642,6 +642,8 @@ template
 void
 function<_Rp()>::swap(function& __f)
 {
+if (_VSTD::addressof(__f) == this)
+  return;
 if (__f_ == (__base*)&__buf_ && __f.__f_ == (__base*)&__f.__buf_)
 {
 typename aligned_storage::type __tempbuf;
@@ -916,6 +918,8 @@ template
 void
 function<_Rp(_A0)>::swap(function& __f)
 {
+if (_VSTD::addressof(__f) == this)
+  return;
 if (__f_ == (__base*)&__buf_ && __f.__f_ == (__base*)&__f.__buf_)
 {
 typename aligned_storage::type __tempbuf;
@@ -1190,6 +1194,8 @@ template::swap(function& __f)
 {
+if (_VSTD::addressof(__f) == this)
+  return;
 if (__f_ == (__base*)&__buf_ && __f.__f_ == (__base*)&__f.__buf_)
 {
 typename aligned_storage::type __tempbuf;
@@ -1464,6 +1470,8 @@ template::swap(function& __f)
 {
+if (_VSTD::addressof(__f) == this)
+  return;
 if (__f_ == (__base*)&__buf_ && __f.__f_ == (__base*)&__f.__buf_)
 {
 typename aligned_storage::type __tempbuf;

Modified: libcxx/trunk/include/functional
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/functional?rev=290721&r1=290720&r2=290721&view=diff
==
--- libcxx/trunk/include/functional (original)
+++ libcxx/trunk/include/functional Thu Dec 29 14:03:55 2016
@@ -1870,6 +1870,8 @@ template
 void
 function<_Rp(_ArgTypes...)>::swap(function& __f) _NOEXCEPT
 {
+if (_VSTD::addressof(__f) == this)
+  return;
 if ((void *)__f_ == &__buf_ && (void *)__f.__f_ == &__f.__buf_)
 {
 typename aligned_storage::type __tempbuf;

Modified: 
libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp?rev=290721&r1=290720&r2=290721&view=diff
==
--- 
libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
 (original)
+++ 
libcxx/trunk/test/std/utilities/function.objects/func.wrap/func.wrap.func/func.wrap.func.con/copy_assign.pass.cpp
 Thu Dec 29 14:03:55 2016
@@ -19,92 +19,120 @@
 #include "test_macros.h"
 #include "count_new.hpp"
 
-class A
-{
-int data_[10];
+class A {
+  int data_[10];
+
 public:
-static int count;
+  static int count;
 
-A()
-{
-++count;
-for (int i = 0; i < 10; ++i)
-data_[i] = i;
-}
-
-A(const A&) {++count;}
-
-~A() {--count;}
-
-int operator()(int i) const
-{
-for (int j = 0; j < 10; ++j)
-i += data_[j];
-return i;
-}
+  A() {
+++count;
+for (int i = 0; i < 10; ++i)
+  data_[i] = i;
+  }
+
+  A(const A &) { ++count; }
+
+  ~A() { --count; }
+
+  int operator()(int i) const {
+for (int j = 0; j < 10; ++j)
+  i += data_[j];
+return i;
+  }
 };
 
 int A::count = 0;
 
-int g(int) {return 0;}
-
-int main()
-{
-assert(globalMemCounter.checkOutstandingNewEq(0));
-{
+int g0() { return 0; }
+int g(int) { return 0; }
+int g2(int, int) { return 2; }
+int g3(int, int, int) { return 3; }
+
+int main() {
+  assert(globalMemCounter.checkOutstandingNewEq(0));
+  {
 std::function f = A();
 assert(A::count == 1);
 assert(globalMemCounter.checkOutstandingNewEq(1));
 assert(f.target());
-assert(f.target() == 0);
+assert(f.target() == 0);
 std::function f2;
 f2 = f;
 assert(A::count == 2);
 assert(globalMemCounter.checkOutstandingNewEq(2));
 assert(f2.target());
-assert(f2.target() == 0);
-}
-assert(A::count == 0);
-assert(globalMemCounter.checkOutstandingNewEq(0));
-{
+assert(f2.target() == 0);
+  }
+  assert(A::count == 0);
+  assert(globalMemCounter.checkOutstandingNewEq(0));
+  {
 std::function f = g;
 assert(globalMemCounter.checkOutstandingNewEq(0));
-assert(f.target());
+assert(f.target());
 assert(f.target() == 0);
 std::fun

[PATCH] D28162: [ADT] Delete RefCountedBaseVPTR.

2016-12-29 Thread Justin Lebar via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
jlebar marked 2 inline comments as done.
Closed by commit rL290717: [ADT] Delete RefCountedBaseVPTR. (authored by 
jlebar).

Changed prior to commit:
  https://reviews.llvm.org/D28162?vs=82681&id=82685#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D28162

Files:
  cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
  cfe/trunk/include/clang/Basic/LLVM.h
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h
  llvm/trunk/lib/Support/CMakeLists.txt
  llvm/trunk/lib/Support/IntrusiveRefCntPtr.cpp
  llvm/trunk/unittests/ADT/IntrusiveRefCntPtrTest.cpp

Index: llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h
===
--- llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/trunk/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -9,9 +9,9 @@
 //
 // This file defines IntrusiveRefCntPtr, a template class that
 // implements a "smart" pointer for objects that maintain their own
-// internal reference count, and RefCountedBase/RefCountedBaseVPTR, two
-// generic base classes for objects that wish to have their lifetimes
-// managed using reference counting.
+// internal reference count, and RefCountedBase, a generic base class
+// for objects that wish to have their lifetimes managed using reference
+// counting.
 //
 // IntrusiveRefCntPtr is similar to Boost's intrusive_ptr with added
 // LLVM-style casting.
@@ -52,36 +52,6 @@
 }
   };
 
-//===--===//
-/// RefCountedBaseVPTR - A class that has the same function as
-///  RefCountedBase, but with a virtual destructor. Should be used
-///  instead of RefCountedBase for classes that already have virtual
-///  methods to enforce dynamic allocation via 'new'. Classes that
-///  inherit from RefCountedBaseVPTR can't be allocated on stack -
-///  attempting to do this will produce a compile error.
-//===--===//
-  class RefCountedBaseVPTR {
-mutable unsigned ref_cnt = 0;
-
-virtual void anchor();
-
-  protected:
-RefCountedBaseVPTR() = default;
-RefCountedBaseVPTR(const RefCountedBaseVPTR &) : ref_cnt(0) {}
-
-virtual ~RefCountedBaseVPTR() = default;
-
-void Retain() const { ++ref_cnt; }
-void Release() const {
-  assert (ref_cnt > 0 && "Reference count is already zero.");
-  if (--ref_cnt == 0) delete this;
-}
-
-template 
-friend struct IntrusiveRefCntPtrInfo;
-  };
-
-
   template  struct IntrusiveRefCntPtrInfo {
 static void retain(T *obj) { obj->Retain(); }
 static void release(T *obj) { obj->Release(); }
@@ -124,10 +94,9 @@
 ///  wrapping NULL pointers.
 ///
 /// Reference counting is implemented via calls to
-///  Obj->Retain()/Obj->Release(). Release() is required to destroy
-///  the object when the reference count reaches zero. Inheriting from
-///  RefCountedBase/RefCountedBaseVPTR takes care of this
-///  automatically.
+///  Obj->Retain()/Obj->Release(). Release() is required to destroy the
+///  object when the reference count reaches zero. Inheriting from
+///  RefCountedBase takes care of this automatically.
 //===--===//
   template 
   class IntrusiveRefCntPtr {
Index: llvm/trunk/lib/Support/CMakeLists.txt
===
--- llvm/trunk/lib/Support/CMakeLists.txt
+++ llvm/trunk/lib/Support/CMakeLists.txt
@@ -61,7 +61,6 @@
   Hashing.cpp
   IntEqClasses.cpp
   IntervalMap.cpp
-  IntrusiveRefCntPtr.cpp
   JamCRC.cpp
   LEB128.cpp
   LineIterator.cpp
Index: llvm/trunk/lib/Support/IntrusiveRefCntPtr.cpp
===
--- llvm/trunk/lib/Support/IntrusiveRefCntPtr.cpp
+++ llvm/trunk/lib/Support/IntrusiveRefCntPtr.cpp
@@ -1,14 +0,0 @@
-//== IntrusiveRefCntPtr.cpp - Smart Refcounting Pointer --*- C++ -*-==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
-
-using namespace llvm;
-
-void RefCountedBaseVPTR::anchor() { }
Index: llvm/trunk/unittests/ADT/IntrusiveRefCntPtrTest.cpp
===
--- llvm/trunk/unittests/ADT/IntrusiveRefCntPtrTest.cpp
+++ llvm/trunk/unittests/ADT/IntrusiveRefCntPtrTest.cpp
@@ -10,30 +10,29 @@
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "gtest/gtest.h"
 
-namespace {
-struct VirtualRefCounted : public llvm::RefCountedBaseVPTR {
-  virtual void f() {}
-};
-}
-
 namespace llvm {
 
-// Run this test with valgrind to detect memory leaks.
-T

r290717 - [ADT] Delete RefCountedBaseVPTR.

2016-12-29 Thread Justin Lebar via cfe-commits

Author: jlebar
Date: Thu Dec 29 13:59:26 2016
New Revision: 290717

URL: http://llvm.org/viewvc/llvm-project?rev=290717&view=rev
Log:
[ADT] Delete RefCountedBaseVPTR.

Summary:
This class is unnecessary.

Its comment indicated that it was a compile error to allocate an
instance of a class that inherits from RefCountedBaseVPTR on the stack.
This may have been true at one point, but it's not today.

Moreover you really do not want to allocate *any* refcounted object on
the stack, vptrs or not, so if we did have a way to prevent these
objects from being stack-allocated, we'd want to apply it to regular
RefCountedBase too, obviating the need for a separate RefCountedBaseVPTR
class.

It seems that the main way RefCountedBaseVPTR provides safety is by
making its subclass's destructor virtual.  This may have been helpful at
one point, but these days clang will emit an error if you define a class
with virtual functions that inherits from RefCountedBase but doesn't
have a virtual destructor.

Reviewers: compnerd, dblaikie

Subscribers: cfe-commits, klimek, llvm-commits, mgorny

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

Modified:
cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
cfe/trunk/include/clang/Basic/LLVM.h
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h

Modified: cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h?rev=290717&r1=290716&r2=290717&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/Dynamic/VariantValue.h Thu Dec 29 
13:59:26 2016
@@ -119,9 +119,9 @@ class VariantMatcher {
   /// \brief Payload interface to be specialized by each matcher type.
   ///
   /// It follows a similar interface as VariantMatcher itself.
-  class Payload : public RefCountedBaseVPTR {
+  class Payload : public RefCountedBase {
   public:
-~Payload() override;
+virtual ~Payload();
 virtual llvm::Optional getSingleMatcher() const = 0;
 virtual std::string getTypeAsString() const = 0;
 virtual llvm::Optional

Modified: cfe/trunk/include/clang/Basic/LLVM.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LLVM.h?rev=290717&r1=290716&r2=290717&view=diff
==
--- cfe/trunk/include/clang/Basic/LLVM.h (original)
+++ cfe/trunk/include/clang/Basic/LLVM.h Thu Dec 29 13:59:26 2016
@@ -43,7 +43,6 @@ namespace llvm {
   template  class IntrusiveRefCntPtr;
   template  struct IntrusiveRefCntPtrInfo;
   template  class RefCountedBase;
-  class RefCountedBaseVPTR;
 
   class raw_ostream;
   class raw_pwrite_stream;
@@ -76,7 +75,6 @@ namespace clang {
   using llvm::IntrusiveRefCntPtr;
   using llvm::IntrusiveRefCntPtrInfo;
   using llvm::RefCountedBase;
-  using llvm::RefCountedBaseVPTR;
 
   using llvm::raw_ostream;
   using llvm::raw_pwrite_stream;

Modified: 
cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h?rev=290717&r1=290716&r2=290717&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h 
Thu Dec 29 13:59:26 2016
@@ -334,7 +334,7 @@ public:
 // Path "pieces" for path-sensitive diagnostics.
 
//===--===//
 
-class PathDiagnosticPiece : public RefCountedBaseVPTR {
+class PathDiagnosticPiece : public RefCountedBase {
 public:
   enum Kind { ControlFlow, Event, Macro, Call, Note };
   enum DisplayHint { Above, Below };
@@ -366,7 +366,7 @@ protected:
   PathDiagnosticPiece(Kind k, DisplayHint hint = Below);
 
 public:
-  ~PathDiagnosticPiece() override;
+  virtual ~PathDiagnosticPiece();
 
   StringRef getString() const { return str; }
 


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

[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386

2016-12-29 Thread Evgeniy Stepanov via Phabricator via cfe-commits

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

LGTM


https://reviews.llvm.org/D26764



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

[PATCH] D28162: [ADT] Delete RefCountedBaseVPTR.

2016-12-29 Thread Justin Lebar via Phabricator via cfe-commits

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



Comment at: llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp:14-37
+struct VirtualRefCounted : public llvm::RefCountedBase {
+  VirtualRefCounted() { ++NumInstances; }
+  VirtualRefCounted(const VirtualRefCounted &) { ++NumInstances; }
+  virtual ~VirtualRefCounted() { --NumInstances; }
   virtual void f() {}
+
+  static int NumInstances;

dblaikie wrote:
> I'm not sure we even need test coverage for this anymore.
> 
> If IntrusiveRefCntPtr calls any dtor (& we should have some coverage of that, 
> to be sure) then it'd be basically impossible for it to call the dtor in a 
> way that wouldn't be compatible with that dtor dispatching if it was virtual. 
> (ie: this test sort of boils down to testing virtual dispatch of virtual 
> dtors - a feature of the compiler, not of this library)
> 
> But up to you.
sgtm.  I got rid of the virtual test and moved the explicit checking for 
destructor calls into the non-virtual test below.  Will land the patch with 
that change.


https://reviews.llvm.org/D28162



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

[PATCH] D28162: [ADT] Delete RefCountedBaseVPTR.

2016-12-29 Thread David Blaikie via Phabricator via cfe-commits

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

Looks good - thanks for the cleanup!




Comment at: llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp:14-37
+struct VirtualRefCounted : public llvm::RefCountedBase {
+  VirtualRefCounted() { ++NumInstances; }
+  VirtualRefCounted(const VirtualRefCounted &) { ++NumInstances; }
+  virtual ~VirtualRefCounted() { --NumInstances; }
   virtual void f() {}
+
+  static int NumInstances;

I'm not sure we even need test coverage for this anymore.

If IntrusiveRefCntPtr calls any dtor (& we should have some coverage of that, 
to be sure) then it'd be basically impossible for it to call the dtor in a way 
that wouldn't be compatible with that dtor dispatching if it was virtual. (ie: 
this test sort of boils down to testing virtual dispatch of virtual dtors - a 
feature of the compiler, not of this library)

But up to you.


https://reviews.llvm.org/D28162



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

[PATCH] D28162: [ADT] Delete RefCountedBaseVPTR.

2016-12-29 Thread Justin Lebar via Phabricator via cfe-commits

jlebar created this revision.
jlebar added reviewers: compnerd, dblaikie.
jlebar added subscribers: llvm-commits, cfe-commits.
Herald added subscribers: mgorny, klimek.

This class is unnecessary.

Its comment indicated that it was a compile error to allocate an
instance of a class that inherits from RefCountedBaseVPTR on the stack.
This may have been true at one point, but it's not today.

Moreover you really do not want to allocate *any* refcounted object on
the stack, vptrs or not, so if we did have a way to prevent these
objects from being stack-allocated, we'd want to apply it to regular
RefCountedBase too, obviating the need for a separate RefCountedBaseVPTR
class.

It seems that the main way RefCountedBaseVPTR provides safety is by
making its subclass's destructor virtual.  This may have been helpful at
one point, but these days clang will emit an error if you define a class
with virtual functions that inherits from RefCountedBase but doesn't
have a virtual destructor.


https://reviews.llvm.org/D28162

Files:
  clang/include/clang/ASTMatchers/Dynamic/VariantValue.h
  clang/include/clang/Basic/LLVM.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/PathDiagnostic.h
  llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/IntrusiveRefCntPtr.cpp
  llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp

Index: llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp
===
--- llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp
+++ llvm/unittests/ADT/IntrusiveRefCntPtrTest.cpp
@@ -11,19 +11,29 @@
 #include "gtest/gtest.h"
 
 namespace {
-struct VirtualRefCounted : public llvm::RefCountedBaseVPTR {
+struct VirtualRefCounted : public llvm::RefCountedBase {
+  VirtualRefCounted() { ++NumInstances; }
+  VirtualRefCounted(const VirtualRefCounted &) { ++NumInstances; }
+  virtual ~VirtualRefCounted() { --NumInstances; }
   virtual void f() {}
+
+  static int NumInstances;
 };
-}
+
+int VirtualRefCounted::NumInstances = 0;
+} // anonymous namespace
 
 namespace llvm {
 
-// Run this test with valgrind to detect memory leaks.
-TEST(IntrusiveRefCntPtr, RefCountedBaseVPTRCopyDoesNotLeak) {
-  VirtualRefCounted *V1 = new VirtualRefCounted;
-  IntrusiveRefCntPtr R1 = V1;
-  VirtualRefCounted *V2 = new VirtualRefCounted(*V1);
-  IntrusiveRefCntPtr R2 = V2;
+TEST(IntrusiveRefCntPtr, VirtualRefCountedCopyDoesNotLeak) {
+  {
+VirtualRefCounted *V1 = new VirtualRefCounted;
+IntrusiveRefCntPtr R1 = V1;
+VirtualRefCounted *V2 = new VirtualRefCounted(*V1);
+IntrusiveRefCntPtr R2 = V2;
+EXPECT_EQ(2, VirtualRefCounted::NumInstances);
+  }
+  EXPECT_EQ(0, VirtualRefCounted::NumInstances);
 }
 
 struct SimpleRefCounted : public RefCountedBase {};
Index: llvm/lib/Support/IntrusiveRefCntPtr.cpp
===
--- llvm/lib/Support/IntrusiveRefCntPtr.cpp
+++ /dev/null
@@ -1,14 +0,0 @@
-//== IntrusiveRefCntPtr.cpp - Smart Refcounting Pointer --*- C++ -*-==//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#include "llvm/ADT/IntrusiveRefCntPtr.h"
-
-using namespace llvm;
-
-void RefCountedBaseVPTR::anchor() { }
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -61,7 +61,6 @@
   Hashing.cpp
   IntEqClasses.cpp
   IntervalMap.cpp
-  IntrusiveRefCntPtr.cpp
   JamCRC.cpp
   LEB128.cpp
   LineIterator.cpp
Index: llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
===
--- llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
+++ llvm/include/llvm/ADT/IntrusiveRefCntPtr.h
@@ -9,9 +9,9 @@
 //
 // This file defines IntrusiveRefCntPtr, a template class that
 // implements a "smart" pointer for objects that maintain their own
-// internal reference count, and RefCountedBase/RefCountedBaseVPTR, two
-// generic base classes for objects that wish to have their lifetimes
-// managed using reference counting.
+// internal reference count, and RefCountedBase, a generic base class
+// for objects that wish to have their lifetimes managed using reference
+// counting.
 //
 // IntrusiveRefCntPtr is similar to Boost's intrusive_ptr with added
 // LLVM-style casting.
@@ -52,36 +52,6 @@
 }
   };
 
-//===--===//
-/// RefCountedBaseVPTR - A class that has the same function as
-///  RefCountedBase, but with a virtual destructor. Should be used
-///  instead of RefCountedBase for classes that already have virtual
-///  methods to enforce dynamic allocation via 'new'. Classes that
-///  inherit from RefCountedBaseVPTR can't be allocated on

[PATCH] D27575: [libcxxabi] Introduce an externally threaded libc++abi variant (take-2)

2016-12-29 Thread Eric Fiselier via Phabricator via cfe-commits

EricWF added a comment.

Why don't we commit this as is and we can deal with the mutex later?


https://reviews.llvm.org/D27575



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

[PATCH] D28160: [OpenMP] Sema and parsing for 'target teams distribute parallel for' pragma

2016-12-29 Thread Alexey Bataev via Phabricator via cfe-commits

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

LG


https://reviews.llvm.org/D28160



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

[PATCH] D26764: [compiler-rt] [cmake] Remove i686 target that is duplicate to i386

2016-12-29 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

Ping. @eugenis, do you consider it good to go then? Or do you want me to change 
something specifically?


https://reviews.llvm.org/D26764



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

[PATCH] D26796: [Driver] Use arch type to find compiler-rt libraries (on Linux)

2016-12-29 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

Second ping.


https://reviews.llvm.org/D26796



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

[PATCH] D21298: [Clang-tidy] delete null check

2016-12-29 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added a comment.

In https://reviews.llvm.org/D21298#632265, @alexfh wrote:

> In https://reviews.llvm.org/D21298#632235, @aaron.ballman wrote:
>
> > In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote:
> >
> > > Small ping, is this ready to be committed?
> >
> >
> > If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We 
> > can deal with any last minute comments post-commit.
>
>
> Yup, that's totally fine, especially when there was a thorough pre-commit 
> code review.


Clarification: it's totally fine to submit without waiting for me longer than a 
grace period of a day or so, once all comments are addressed and other 
reviewers have LGTM'd the patch.

However, here specifically I have a few more comments ;)


https://reviews.llvm.org/D21298



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

[PATCH] D21298: [Clang-tidy] delete null check

2016-12-29 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh added a comment.

In https://reviews.llvm.org/D21298#632235, @aaron.ballman wrote:

> In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote:
>
> > Small ping, is this ready to be committed?
>
>
> If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We can 
> deal with any last minute comments post-commit.

Yup, that's totally fine, especially when there was a thorough pre-commit code 
review.

https://reviews.llvm.org/D21298

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

[PATCH] D21298: [Clang-tidy] delete null check

2016-12-29 Thread Alexander Kornienko via Phabricator via cfe-commits

alexfh requested changes to this revision.
alexfh added inline comments.
This revision now requires changes to proceed.



Comment at: clang-tidy/readability/DeleteNullPointerCheck.cpp:29
+  const auto BinaryPointerCheckCondition = binaryOperator(
+  allOf(hasEitherOperand(castExpr(hasCastKind(CK_NullToPointer))),
+hasEitherOperand(ignoringImpCasts(declRefExpr();

Since `binaryOperator` (and most if not all other node matchers) is 
`VariadicDynCastAllOfMatcher`, `allOf` is redundant here (similar to Piotr's 
comment below).

Same for `compoundStmt` below.



Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:6
+
+Checks the 'if' statements where a pointer's existence is checked and then 
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.

Enclose `if` in double backquotes instead of single quotes.



Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:7
+Checks the 'if' statements where a pointer's existence is checked and then 
deletes the pointer.
+The check is unnecessary as deleting a nullpointer has no effect.
+

Either `null pointer` or `nullptr` (enclosed in double backquotes).



Comment at: docs/clang-tidy/checks/readability-delete-null-pointer.rst:10
+.. code:: c++
+  int *p;
+  if (p)

Insert an empty line above and check that Sphinx can build this.



Comment at: test/clang-tidy/readability-delete-null-pointer.cpp:7
+  int *p = 0;
+  // CHECK-FIXES: // comment that should not be deleted
+  if (p) {

This check (also combined with the ones below) is useless. It will work for the 
unchanged file as well: it will skip the `if (p)` line and find the comment, 
the next CHECK-FIXES will find the `delete p;` line and the CHECK-FIXES-NOT 
will find no lines matching `if (p)` between them.

I'd use something like this:

  // comment1
  if (p) { // comment 2
delete p;
  } // comment 3
  // CHECK-FIXES: {{^  }}// comment1
  // CHECK-FIXES-NEXT: {{^  }} // comment2
  // CHECK-FIXES-NEXT: {{^  }}  delete p;
  // CHECK-FIXES-NEXT: {{^  }} // comment3

Same for patterns below.


https://reviews.llvm.org/D21298



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

[PATCH] D21298: [Clang-tidy] delete null check

2016-12-29 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In https://reviews.llvm.org/D21298#632154, @xazax.hun wrote:

> Small ping, is this ready to be committed?


If @alexfh doesn't sign off by tomorrow, I think it's fine to commit. We can 
deal with any last minute comments post-commit.


https://reviews.llvm.org/D21298



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

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

2016-12-29 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added inline comments.



Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+  // a tag declaration (e.g. struct, class etc.):
+  // class A { } Object1, Object2;  <-- won't be matched
+  Finder->addMatcher(

firolino wrote:
> firolino wrote:
> > firolino wrote:
> > > firolino wrote:
> > > > aaron.ballman wrote:
> > > > > firolino wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > Why do we not want to match this?
> > > > > > If we decide, whether we transform 
> > > > > > ```
> > > > > > class A { 
> > > > > > } Object1, Object2;
> > > > > > ``` 
> > > > > > to
> > > > > > ```
> > > > > > class A { 
> > > > > > } Object1, 
> > > > > > Object2;
> > > > > > ``` 
> > > > > > or
> > > > > > ```
> > > > > > class A { 
> > > > > > } 
> > > > > > Object1, 
> > > > > > Object2;
> > > > > > ``` 
> > > > > > I might consider adding support for it. Moreover, this kind of 
> > > > > > definition is usually seen globally and I don't know how to handle 
> > > > > > globals yet. See 
> > > > > > http://lists.llvm.org/pipermail/cfe-dev/2015-November/046262.html
> > > > > I think this should be handled. It can be handled in either of the 
> > > > > forms you show, or by saying:
> > > > > ```
> > > > > A Object1;
> > > > > A Object2;
> > > > > ```
> > > > > If all of these turn out to be a problem, we can still diagnose 
> > > > > without providing a fixit.
> > > > > 
> > > > > As for globals in general, they can be handled in a separate patch 
> > > > > once we figure out the declaration grouping.
> > > > OK. I will try to split the object definition from the class 
> > > > definition, as you have suggested. Thus, I can kick out the 
> > > > tagDecl-matcher as well. If there is no easy way to do this, it will be 
> > > > reported anyway but without a fixit.
> > > > 
> > > > Note for me: Update documentation!
> > > What about
> > > ```
> > > struct S {
> > > } S1;
> > > ```
> > > I would like to report this too, since two names are being declared here. 
> > > `S` and `S1`. What do you think?
> > ```
> > struct {
> > } nn1, nn2;
> > ```
> > Shall we ignore anonymous definitions?
> To be more precise: Warn and provide a fixit for `struct S {} S1`. Only warn 
> for `struct {} nn1, nn2`.
> What about
> 
> ```
> struct S {
> } S1;
> ```
> I would like to report this too, since two names are being declared here. S 
> and S1. What do you think?

I don't think that this should be diagnosed. For one, this is a relatively 
common pattern (arguably more common than `struct S { } S1, S2;`), but also, it 
names two very distinct entities (a type and a variable).

> ```
> struct {
> } nn1, nn2;
>```
> Shall we ignore anonymous definitions?

Yes, we basically have to.



https://reviews.llvm.org/D27621



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

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

2016-12-29 Thread Firat Kasmis via Phabricator via cfe-commits

firolino added inline comments.



Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+  // a tag declaration (e.g. struct, class etc.):
+  // class A { } Object1, Object2;  <-- won't be matched
+  Finder->addMatcher(

firolino wrote:
> firolino wrote:
> > firolino wrote:
> > > aaron.ballman wrote:
> > > > firolino wrote:
> > > > > aaron.ballman wrote:
> > > > > > Why do we not want to match this?
> > > > > If we decide, whether we transform 
> > > > > ```
> > > > > class A { 
> > > > > } Object1, Object2;
> > > > > ``` 
> > > > > to
> > > > > ```
> > > > > class A { 
> > > > > } Object1, 
> > > > > Object2;
> > > > > ``` 
> > > > > or
> > > > > ```
> > > > > class A { 
> > > > > } 
> > > > > Object1, 
> > > > > Object2;
> > > > > ``` 
> > > > > I might consider adding support for it. Moreover, this kind of 
> > > > > definition is usually seen globally and I don't know how to handle 
> > > > > globals yet. See 
> > > > > http://lists.llvm.org/pipermail/cfe-dev/2015-November/046262.html
> > > > I think this should be handled. It can be handled in either of the 
> > > > forms you show, or by saying:
> > > > ```
> > > > A Object1;
> > > > A Object2;
> > > > ```
> > > > If all of these turn out to be a problem, we can still diagnose without 
> > > > providing a fixit.
> > > > 
> > > > As for globals in general, they can be handled in a separate patch once 
> > > > we figure out the declaration grouping.
> > > OK. I will try to split the object definition from the class definition, 
> > > as you have suggested. Thus, I can kick out the tagDecl-matcher as well. 
> > > If there is no easy way to do this, it will be reported anyway but 
> > > without a fixit.
> > > 
> > > Note for me: Update documentation!
> > What about
> > ```
> > struct S {
> > } S1;
> > ```
> > I would like to report this too, since two names are being declared here. 
> > `S` and `S1`. What do you think?
> ```
> struct {
> } nn1, nn2;
> ```
> Shall we ignore anonymous definitions?
To be more precise: Warn and provide a fixit for `struct S {} S1`. Only warn 
for `struct {} nn1, nn2`.


https://reviews.llvm.org/D27621



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

[PATCH] D27575: [libcxxabi] Introduce an externally threaded libc++abi variant (take-2)

2016-12-29 Thread Asiri Rathnayake via Phabricator via cfe-commits

rmaprath added inline comments.



Comment at: src/fallback_malloc.cpp:37
 class mutexor {
 public:

EricWF wrote:
> rmaprath wrote:
> > EricWF wrote:
> > > Can't we replace this class with `std::mutex` directly?
> > Again, I should've included more context to the patch :(
> > 
> > The `mutexor` here is supposed to degrade to a nop when threading is 
> > disabled. I think we cannot use `std::mutex` without threading support.
> > 
> > Will update the patch with more context.
> We cannot use `std::mutex` without threading support but I would still rather 
> use it.
> 
> I would `typedef std::lock_guard mutexor` when threading is 
> enabled and otherwise I would just `#ifdef` all usages away.
> 
> Also please add `_LIBCPP_SAFE_STATIC` to the heap_mutex declaration.
Hmmm, using `std::mutex` here breaks the shared library builds of `libcxxabi`. 
The problem is that methods like `std::mutex::lock()` and `~std::mutex()` are 
only declared in the `` header, with the implementation going into the 
dylib.

I was just about to update the patch because it worked for all my other 
configurations, but those configurations are either static builds or ones that 
suppress the `-Wl, -z defs` linker option passed to the shared library builds 
by default (this is an inherited option).

Perhaps it's time to get rid of `-Wl, -z defs` for all the `libcxxabi` 
configurations? That makes sense if `libcxxabi` is inherently dependent on 
`libcxx`.


https://reviews.llvm.org/D27575



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

[PATCH] D27621: [clang-tidy] check to find declarations declaring more than one name

2016-12-29 Thread Firat Kasmis via Phabricator via cfe-commits

firolino added inline comments.



Comment at: clang-tidy/readability/OneNamePerDeclarationCheck.cpp:34
+  // a tag declaration (e.g. struct, class etc.):
+  // class A { } Object1, Object2;  <-- won't be matched
+  Finder->addMatcher(

firolino wrote:
> firolino wrote:
> > aaron.ballman wrote:
> > > firolino wrote:
> > > > aaron.ballman wrote:
> > > > > Why do we not want to match this?
> > > > If we decide, whether we transform 
> > > > ```
> > > > class A { 
> > > > } Object1, Object2;
> > > > ``` 
> > > > to
> > > > ```
> > > > class A { 
> > > > } Object1, 
> > > > Object2;
> > > > ``` 
> > > > or
> > > > ```
> > > > class A { 
> > > > } 
> > > > Object1, 
> > > > Object2;
> > > > ``` 
> > > > I might consider adding support for it. Moreover, this kind of 
> > > > definition is usually seen globally and I don't know how to handle 
> > > > globals yet. See 
> > > > http://lists.llvm.org/pipermail/cfe-dev/2015-November/046262.html
> > > I think this should be handled. It can be handled in either of the forms 
> > > you show, or by saying:
> > > ```
> > > A Object1;
> > > A Object2;
> > > ```
> > > If all of these turn out to be a problem, we can still diagnose without 
> > > providing a fixit.
> > > 
> > > As for globals in general, they can be handled in a separate patch once 
> > > we figure out the declaration grouping.
> > OK. I will try to split the object definition from the class definition, as 
> > you have suggested. Thus, I can kick out the tagDecl-matcher as well. If 
> > there is no easy way to do this, it will be reported anyway but without a 
> > fixit.
> > 
> > Note for me: Update documentation!
> What about
> ```
> struct S {
> } S1;
> ```
> I would like to report this too, since two names are being declared here. `S` 
> and `S1`. What do you think?
```
struct {
} nn1, nn2;
```
Shall we ignore anonymous definitions?


https://reviews.llvm.org/D27621



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

[PATCH] D27810: Normalize all filenames before searching FileManager caches

2016-12-29 Thread Yaron Keren via Phabricator via cfe-commits

yaron.keren added inline comments.



Comment at: lib/Basic/FileManager.cpp:218
+#ifdef LLVM_ON_WIN32
+  SmallString<128> NormalizedPath(Filename.str());
+  llvm::sys::path::native(NormalizedPath);

I'd use a larger SmallString<256>, with large projects 128 bytes are frequently 
not enough.


https://reviews.llvm.org/D27810



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

[PATCH] D27810: Normalize all filenames before searching FileManager caches

2016-12-29 Thread Bernd Schöler via Phabricator via cfe-commits

besc added a comment.

I found a crash when applying this patch to 3.9.1 final (source archives from 
http://llvm.org/releases/download.html#3.9.1) and compiling LLVM/Clang with 
MinGW-w64 GCC 6.2.0 or MSVC 2015. Clang-format sometimes crashes with input 
from stdin, output to stdout and a command line like this:

  clang-format.exe -style=file -assume-filename=C:/path/to/file.hpp

I’m not familiar with the code base, so these are just my observations from a 
user’s point of view. From what I found the problem could be linked to the 
first slash/backslash in an absolute path:

Crash

- C:/path/to/file.hpp
- C:/path\to\file.hpp
- /path/to/file.hpp
- /path\to\file.hpp

Works

- Backslashes only always work.
- C:\path/to/file.hpp
- \path/to/file.hpp
- path/to/file.hpp
- path/to\file.hpp

Header file (.hpp) or source file (.cpp) does not make a difference.

A crash produces this error message:

  Wrote crash dump file 
"C:\Users\besc\AppData\Local\Temp\clang-format.exe-e00fa3.dmp"
  0x00459504 (0x 0x 0x 
0x0022E300)
  0x0047A45F (0x0022E3F0 0x0015 0x0022E5AC 
0x0404)
  0x0047174F (0x0022EE20 0x0022ED30 0x 
0x0094846C)
  0x00403744 (0x00A77F30 0x77CC0A7A 0x00857120 
0x0022FDB0)
  0x006E9CBF (0x0002 0x004D 0x0085ABD0 
0x)
  0x004013E8 (0x 0x 0x 
0x)
  0x0040151B (0x 0x 0x 
0x)
  0x77B859CD (0x 0x 0x 
0x), BaseThreadInitThunk() + 0xD bytes(s)
  0x77CBA561 (0x 0x 0x 
0x), RtlUserThreadStart() + 0x21 bytes(s)

A corresponding dump file is attached: F2878050: clang-format.exe-e00fa3.dmp 


I also tested a vanilla 3.9.1 final, built with the same compilers: works fine 
with all the above slash combinations.


https://reviews.llvm.org/D27810



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

[PATCH] D23934: Add a -ffixed-date-time= flag that sets the initial value of DATE, TIME, TIMESTAMP

2016-12-29 Thread Hal Finkel via Phabricator via cfe-commits

hfinkel added a comment.

In https://reviews.llvm.org/D23934#632216, @emaste wrote:

> Please also accept `SOURCE_DATE_EPOCH` set in the environment -- see 
> https://reproducible-builds.org/specs/source-date-epoch/

It looks like there's reasonable adoption of this: 
https://wiki.debian.org/ReproducibleBuilds/TimestampsProposal#Reading_the_variable
 and gcc >= 7 will support it.

> Also although I'm generally leery of options auto-detecting the argument 
> format, I think we should be able to pass an epoch timestamp to 
> -ffixed-date-time.

I'd also find this convenient. The auto-detection should be unambiguous (if it 
is an integer, then it is an epoch).

https://reviews.llvm.org/D23934

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

[PATCH] D23934: Add a -ffixed-date-time= flag that sets the initial value of DATE, TIME, TIMESTAMP

2016-12-29 Thread Ed Maste via Phabricator via cfe-commits

emaste added a comment.

Please also accept `SOURCE_DATE_EPOCH` set in the environment -- see 
https://reproducible-builds.org/specs/source-date-epoch/

Also although I'm generally leery of options auto-detecting the argument 
format, I think we should be able to pass an epoch timestamp to 
-ffixed-date-time.


https://reviews.llvm.org/D23934



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

RE: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

2016-12-29 Thread Yatsina, Marina via cfe-commits

Adding Ziv, the author of this patch.

From: Chandler Carruth [mailto:chandl...@google.com]
Sent: Thursday, December 29, 2016 11:57
To: Yatsina, Marina ; cfe-commits@lists.llvm.org
Subject: Re: r290539 - [inline-asm]No error for conflict between inputs\outputs 
and clobber list

On Mon, Dec 26, 2016 at 4:34 AM Marina Yatsina via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: myatsina
Date: Mon Dec 26 06:23:42 2016
New Revision: 290539

URL: http://llvm.org/viewvc/llvm-project?rev=290539&view=rev
Log:
[inline-asm]No error for conflict between inputs\outputs and clobber list

According to extended asm syntax, a case where the clobber list includes a 
variable from the inputs or outputs should be an error - conflict.
for example:

const long double a = 0.0;
int main()
{

char b;
double t1 = a;
__asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");

return 0;
}

This should conflict with the output - t1 which is st, and st which is st 
aswell.
The patch fixes it.

Commit on behald of Ziv Izhar.

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/lib/Basic/TargetInfo.cpp
cfe/trunk/lib/Basic/Targets.cpp
cfe/trunk/lib/Headers/intrin.h
cfe/trunk/lib/Sema/SemaStmtAsm.cpp
cfe/trunk/test/Sema/asm.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290539&r1=290538&r2=290539&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 26 06:23:42 
2016
@@ -7069,6 +7069,10 @@ let CategoryName = "Inline Assembly Issu
 "constraint '%0' is already present here">;
 }

+  def error_inoutput_conflict_with_clobber : Error<
+"asm-specifier for input or output variable conflicts with asm"
+" clobber list">;

Clang generally works to avoid this kind of error message. All this does is say 
"there was a problem of this kind" without identifying any of the specifics. 
And for this error in particular I think this is of the utmost importance. 
Developers are not going to understand what went wrong here.

I would suggest at a minimum to enhance this to explain:

1) What operands and clobbers conflict, preferably with source ranges 
underlining them.

2) Why they conflict (for example the fact that "D" means the di register 
group, of which "%rdi" is a member)

Beyond that, I wonder if you could add a note suggesting to remove the clobber 
if the input (or output) operand is sufficient.

You can make this note explain carefully the case where something would need to 
be added to the inputs or outputs instead, but I think it at least makes sense 
to clarify that this will be a common fix and what to look out for that might 
make it an incorrect fix.
-
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D21298: [Clang-tidy] delete null check

2016-12-29 Thread Gábor Horváth via Phabricator via cfe-commits

xazax.hun added a comment.

Small ping, is this ready to be committed?


https://reviews.llvm.org/D21298



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

Re: r290539 - [inline-asm]No error for conflict between inputs\outputs and clobber list

2016-12-29 Thread Chandler Carruth via cfe-commits

On Mon, Dec 26, 2016 at 4:34 AM Marina Yatsina via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: myatsina
> Date: Mon Dec 26 06:23:42 2016
> New Revision: 290539
>
> URL: http://llvm.org/viewvc/llvm-project?rev=290539&view=rev
> Log:
> [inline-asm]No error for conflict between inputs\outputs and clobber list
>
> According to extended asm syntax, a case where the clobber list includes a
> variable from the inputs or outputs should be an error - conflict.
> for example:
>
> const long double a = 0.0;
> int main()
> {
>
> char b;
> double t1 = a;
> __asm__ ("fucompp": "=a" (b) : "u" (t1), "t" (t1) : "cc", "st", "st(1)");
>
> return 0;
> }
>
> This should conflict with the output - t1 which is st, and st which is st
> aswell.
> The patch fixes it.

> Commit on behald of Ziv Izhar.
>
> Differential Revision: https://reviews.llvm.org/D15075
>
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/include/clang/Basic/TargetInfo.h
> cfe/trunk/lib/Basic/TargetInfo.cpp
> cfe/trunk/lib/Basic/Targets.cpp
> cfe/trunk/lib/Headers/intrin.h
> cfe/trunk/lib/Sema/SemaStmtAsm.cpp
> cfe/trunk/test/Sema/asm.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=290539&r1=290538&r2=290539&view=diff
>
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 26
> 06:23:42 2016
> @@ -7069,6 +7069,10 @@ let CategoryName = "Inline Assembly Issu
>  "constraint '%0' is already present here">;
>  }
>
> +  def error_inoutput_conflict_with_clobber : Error<
> +"asm-specifier for input or output variable conflicts with asm"
> +" clobber list">;
>

Clang generally works to avoid this kind of error message. All this does is
say "there was a problem of this kind" without identifying any of the
specifics. And for this error in particular I think this is of the utmost
importance. Developers are not going to understand what went wrong here.

I would suggest at a minimum to enhance this to explain:

1) What operands and clobbers conflict, preferably with source ranges
underlining them.

2) Why they conflict (for example the fact that "D" means the di register
group, of which "%rdi" is a member)

Beyond that, I wonder if you could add a note suggesting to remove the
clobber if the input (or output) operand is sufficient.

You can make this note explain carefully the case where something would
need to be added to the inputs or outputs instead, but I think it at least
makes sense to clarify that this will be a common fix and what to look out
for that might make it an incorrect fix.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28081: Make GetStyle return Expected instead of FormatStyle

2016-12-29 Thread Eric Liu via Phabricator via cfe-commits

ioeric added inline comments.



Comment at: lib/Format/Format.cpp:1984
+// If so, can't return this error here...
+return make_string_error("Configuration file(s) do(es) not support " +
+ getLanguageName(Language) + ": " +

amaiorano wrote:
> amaiorano wrote:
> > See the TODO comment above (which will be removed obviously). Is it an 
> > error if we find no suitable config for the input language? If that 
> > happens, should the fallback style be returned?
> @ioeric Do you have any thoughts on my question here? Say the user specified 
> "-file" and a fallback style, and we find files but they are not suitable 
> (for a different language), do we use the fallback style, since it's as if we 
> found no config file. If so, then we wouldn't consider this an error, and 
> therefore would not print nor return the message that we see here 
> ("Configuration file(s) do(es) not support...").
> 
> The fact that the original code output to errs() here leads me to believe 
> that this should be considered an error condition, in which case I should 
> keep my change - that is, return an error here and _not_ return the fallback 
> style.
I don't have strong opinion here, but I am inclined to return an error.


https://reviews.llvm.org/D28081



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

[PATCH] D27850: [libcxx] add missing constexpr to optional::value_or

2016-12-29 Thread S. B. Tam via Phabricator via cfe-commits

cpplearner updated this revision to Diff 82653.
cpplearner added a comment.

backed out the changes to ``


https://reviews.llvm.org/D27850

Files:
  include/optional
  
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp


Index: 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ 
test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -10,7 +10,7 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// template  T optional::value_or(U&& v) &&;
+// template  constexpr T optional::value_or(U&& v) &&;
 
 #include 
 #include 
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }
Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");


Index: test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
===
--- test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
+++ test/std/utilities/optional/optional.object/optional.object.observe/value_or.pass.cpp
@@ -10,7 +10,7 @@
 // UNSUPPORTED: c++98, c++03, c++11, c++14
 // 
 
-// template  T optional::value_or(U&& v) &&;
+// template  constexpr T optional::value_or(U&& v) &&;
 
 #include 
 #include 
@@ -26,22 +26,22 @@
 {
 int i_;
 
-Y(int i) : i_(i) {}
+constexpr Y(int i) : i_(i) {}
 };
 
 struct X
 {
 int i_;
 
-X(int i) : i_(i) {}
-X(X&& x) : i_(x.i_) {x.i_ = 0;}
-X(const Y& y) : i_(y.i_) {}
-X(Y&& y) : i_(y.i_+1) {}
+constexpr X(int i) : i_(i) {}
+constexpr X(X&& x) : i_(x.i_) {x.i_ = 0;}
+constexpr X(const Y& y) : i_(y.i_) {}
+constexpr X(Y&& y) : i_(y.i_+1) {}
 friend constexpr bool operator==(const X& x, const X& y)
 {return x.i_ == y.i_;}
 };
 
-int main()
+constexpr int test()
 {
 {
 optional opt(in_place, 2);
@@ -65,4 +65,10 @@
 assert(std::move(opt).value_or(Y(3)) == 4);
 assert(!opt);
 }
+return 0;
+}
+
+int main()
+{
+static_assert(test() == 0);
 }
Index: include/optional
===
--- include/optional
+++ include/optional
@@ -893,7 +893,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-value_type value_or(_Up&& __v) &&
+constexpr value_type value_or(_Up&& __v) &&
 {
 static_assert(is_move_constructible_v,
   "optional::value_or: T must be move constructible");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

67 matches

Mail list logo