[PATCH] D26057: [coroutines] Add DependentCoawaitExpr and fix re-building CoroutineBodyStmt.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 88319.
EricWF added a comment.

- More cleanup
- Rename `BuildCoawaitExpr` -> `BuildResolvedCoawaitExpr` and 
`BuildDependentCoawaitExpr` -> `BuildUnresolvedCoawaitExpr`.


https://reviews.llvm.org/D26057

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/AST/StmtCXX.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/ScopeInfo.cpp
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/SemaCXX/coroutines.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -231,6 +231,7 @@
   case Stmt::TypeTraitExprClass:
   case Stmt::CoroutineBodyStmtClass:
   case Stmt::CoawaitExprClass:
+  case Stmt::DependentCoawaitExprClass:
   case Stmt::CoreturnStmtClass:
   case Stmt::CoyieldExprClass:
   case Stmt::CXXBindTemporaryExprClass:
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -59,25 +59,25 @@
 template 
 struct std::experimental::coroutine_traits {};
 
-int no_promise_type() {
-  co_await a; // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
+int no_promise_type() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
+  co_await a;
 }
 
 template <>
 struct std::experimental::coroutine_traits { typedef int promise_type; };
-double bad_promise_type(double) {
-  co_await a; // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits::promise_type' (aka 'int') is not a class}}
+double bad_promise_type(double) { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits::promise_type' (aka 'int') is not a class}}
+  co_await a;
 }
 
 template <>
 struct std::experimental::coroutine_traits {
   struct promise_type {};
 };
-double bad_promise_type_2(int) {
+double bad_promise_type_2(int) { // expected-error {{no member named 'initial_suspend'}}
   co_yield 0; // expected-error {{no member named 'yield_value' in 'std::experimental::coroutine_traits::promise_type'}}
 }
 
-struct promise; // expected-note 2{{forward declaration}}
+struct promise; // expected-note {{forward declaration}}
 struct promise_void;
 struct void_tag {};
 template 
@@ -94,9 +94,7 @@
 }
 
 // FIXME: This diagnostic is terrible.
-void undefined_promise() { // expected-error {{variable has incomplete type 'promise_type'}}
-  // FIXME: This diagnostic doesn't make any sense.
-  // expected-error@-2 {{incomplete definition of type 'promise'}}
+void undefined_promise() { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits::promise_type' (aka 'promise') is an incomplete type}}
   co_await a;
 }
 
@@ -216,6 +214,13 @@
 }
 
 struct outer {};
+struct await_arg_1 {};
+struct await_arg_2 {};
+
+namespace adl_ns {
+struct coawait_arg_type {};
+awaitable operator co_await(coawait_arg_type);
+}
 
 namespace dependent_operator_co_await_lookup {
   template void await_template(T t) {
@@ -238,6 +243,94 @@
   };
   template void await_template(outer); // expected-note {{instantiation}}
   template void await_template_2(outer);
+
+  struct transform_awaitable {};
+  struct transformed {};
+
+  struct transform_promise {
+typedef transform_awaitable await_arg;
+coro get_return_object();
+transformed initial_suspend();
+::adl_ns::coawait_arg_type final_suspend();
+transformed await_transform(transform_awaitable);
+  };
+  template 
+  struct basic_promise {
+typedef AwaitArg await_arg;
+coro get_return_object();
+awaitable initial_suspend();
+awaitable final_suspend();
+  };
+
+  awaitable operator co_await(await_arg_1);
+
+  template 
+  coro await_template_3(U t) {
+co_await t;
+  }
+
+  template coro> await_template_3>(await_arg_1);
+
+  template 
+  struct dependent_member {
+coro mem_fn() const {
+  co_await typename T::await_arg{}; // expected-error {{call to function 'operator co_await'}}}
+}
+template 
+coro dep_mem_fn(U t) {
+  co_await t;
+}
+  };
+
+  template <>
+  struct dependent_member {
+// FIXME this diagnostic is terrible
+coro mem_fn() const { // expected-error {{no member named 'await_ready

[PATCH] D26057: [coroutines] Add DependentCoawaitExpr and fix re-building CoroutineBodyStmt.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF updated this revision to Diff 88325.
EricWF added a comment.

Add more tests.


https://reviews.llvm.org/D26057

Files:
  include/clang/AST/ExprCXX.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/AST/Stmt.h
  include/clang/AST/StmtCXX.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/StmtNodes.td
  include/clang/Sema/ScopeInfo.h
  include/clang/Sema/Sema.h
  lib/AST/Expr.cpp
  lib/AST/ExprClassification.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/StmtPrinter.cpp
  lib/AST/StmtProfile.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/ScopeInfo.cpp
  lib/Sema/SemaCoroutine.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaExceptionSpec.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReaderStmt.cpp
  lib/Serialization/ASTWriterStmt.cpp
  lib/StaticAnalyzer/Core/ExprEngine.cpp
  test/SemaCXX/coroutines.cpp
  tools/libclang/CXCursor.cpp

Index: tools/libclang/CXCursor.cpp
===
--- tools/libclang/CXCursor.cpp
+++ tools/libclang/CXCursor.cpp
@@ -231,6 +231,7 @@
   case Stmt::TypeTraitExprClass:
   case Stmt::CoroutineBodyStmtClass:
   case Stmt::CoawaitExprClass:
+  case Stmt::DependentCoawaitExprClass:
   case Stmt::CoreturnStmtClass:
   case Stmt::CoyieldExprClass:
   case Stmt::CXXBindTemporaryExprClass:
Index: test/SemaCXX/coroutines.cpp
===
--- test/SemaCXX/coroutines.cpp
+++ test/SemaCXX/coroutines.cpp
@@ -59,25 +59,25 @@
 template 
 struct std::experimental::coroutine_traits {};
 
-int no_promise_type() {
-  co_await a; // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
+int no_promise_type() { // expected-error {{this function cannot be a coroutine: 'std::experimental::coroutine_traits' has no member named 'promise_type'}}
+  co_await a;
 }
 
 template <>
 struct std::experimental::coroutine_traits { typedef int promise_type; };
-double bad_promise_type(double) {
-  co_await a; // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits::promise_type' (aka 'int') is not a class}}
+double bad_promise_type(double) { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits::promise_type' (aka 'int') is not a class}}
+  co_await a;
 }
 
 template <>
 struct std::experimental::coroutine_traits {
   struct promise_type {};
 };
-double bad_promise_type_2(int) {
+double bad_promise_type_2(int) { // expected-error {{no member named 'initial_suspend'}}
   co_yield 0; // expected-error {{no member named 'yield_value' in 'std::experimental::coroutine_traits::promise_type'}}
 }
 
-struct promise; // expected-note 2{{forward declaration}}
+struct promise; // expected-note {{forward declaration}}
 struct promise_void;
 struct void_tag {};
 template 
@@ -94,9 +94,7 @@
 }
 
 // FIXME: This diagnostic is terrible.
-void undefined_promise() { // expected-error {{variable has incomplete type 'promise_type'}}
-  // FIXME: This diagnostic doesn't make any sense.
-  // expected-error@-2 {{incomplete definition of type 'promise'}}
+void undefined_promise() { // expected-error {{this function cannot be a coroutine: 'experimental::coroutine_traits::promise_type' (aka 'promise') is an incomplete type}}
   co_await a;
 }
 
@@ -216,6 +214,13 @@
 }
 
 struct outer {};
+struct await_arg_1 {};
+struct await_arg_2 {};
+
+namespace adl_ns {
+struct coawait_arg_type {};
+awaitable operator co_await(coawait_arg_type);
+}
 
 namespace dependent_operator_co_await_lookup {
   template void await_template(T t) {
@@ -238,6 +243,94 @@
   };
   template void await_template(outer); // expected-note {{instantiation}}
   template void await_template_2(outer);
+
+  struct transform_awaitable {};
+  struct transformed {};
+
+  struct transform_promise {
+typedef transform_awaitable await_arg;
+coro get_return_object();
+transformed initial_suspend();
+::adl_ns::coawait_arg_type final_suspend();
+transformed await_transform(transform_awaitable);
+  };
+  template 
+  struct basic_promise {
+typedef AwaitArg await_arg;
+coro get_return_object();
+awaitable initial_suspend();
+awaitable final_suspend();
+  };
+
+  awaitable operator co_await(await_arg_1);
+
+  template 
+  coro await_template_3(U t) {
+co_await t;
+  }
+
+  template coro> await_template_3>(await_arg_1);
+
+  template 
+  struct dependent_member {
+coro mem_fn() const {
+  co_await typename T::await_arg{}; // expected-error {{call to function 'operator co_await'}}}
+}
+template 
+coro dep_mem_fn(U t) {
+  co_await t;
+}
+  };
+
+  template <>
+  struct dependent_member {
+// FIXME this diagnostic is terrible
+coro mem_fn() const { // expected-error {{no member named 'await_ready' in 'dependent_operator_co_await_lookup::transformed'}}
+  // expected-note@-1 {{call to 'initial_suspend' implicitly 

[PATCH] D29928: [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
Herald added a subscriber: JDevlieghere.

Users might get confused easily when they see the check's message on
full template function speciliations.

Add a note to the output message, which mentions these kind of function
specializations are treated as regular functions.


https://reviews.llvm.org/D29928

Files:
  clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  test/clang-tidy/misc-definitions-in-headers.hpp


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -29,6 +29,7 @@
 template <>
 int CA::f3() {
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: note: this is a full function template 
specilization
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -93,6 +94,7 @@
 template <>
 int f3() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-2]]:5: note: this is a full function template 
specilization
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -127,6 +127,12 @@
  "function definitions in header files can lead to ODR violations")
 << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
+// Output notes for full function template specializations.
+if (FD->getTemplateSpecializationKind() != TSK_Undeclared)
+  diag(FD->getLocation(), "this is a full function template specilization "
+  "which behaves as a regular function, so the ODR 
"
+  "still applies",
+   DiagnosticIDs::Note);
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -29,6 +29,7 @@
 template <>
 int CA::f3() {
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a header file;
+// CHECK-MESSAGES: :[[@LINE-2]]:9: note: this is a full function template specilization
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -93,6 +94,7 @@
 template <>
 int f3() {
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a header file;
+// CHECK-MESSAGES: :[[@LINE-2]]:5: note: this is a full function template specilization
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -127,6 +127,12 @@
  "function definitions in header files can lead to ODR violations")
 << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
+// Output notes for full function template specializations.
+if (FD->getTemplateSpecializationKind() != TSK_Undeclared)
+  diag(FD->getLocation(), "this is a full function template specilization "
+  "which behaves as a regular function, so the ODR "
+  "still applies",
+   DiagnosticIDs::Note);
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29910: [OpenMP] Specialize default schedule on a worksharing loop on the NVPTX device.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

General comment: do you really need to define default schedule kind in Sema? 
Could you do it during codegen phase?




Comment at: include/clang/Basic/OpenMPKinds.h:23-25
+class Sema;
+class OMPClause;
+

No way, these classes should not be used here



Comment at: include/clang/Basic/OpenMPKinds.h:250-256
+/// Get the default schedule type for any loop-based OpenMP directive,
+/// specialized for a particular target.  This is used to guide codegen
+/// if a) no 'schedule' clause is specified, or b) a 'schedule' type of
+/// 'auto' is specified by the user.
+OpenMPDefaultScheduleKind
+getDefaultSchedule(Sema &SemaRef, OpenMPDirectiveKind Kind,
+   llvm::ArrayRef Clauses);

You can't use Sema and OMPClause in Basic, it is not allowed



Comment at: lib/CodeGen/CGStmtOpenMP.cpp:2190
   const bool IVSigned = 
IVExpr->getType()->hasSignedIntegerRepresentation();
-  // OpenMP 4.5, 2.7.1 Loop Construct, Description.
-  // If the static schedule kind is specified or if the ordered clause is
-  // specified, and if no monotonic modifier is specified, the effect will
-  // be as if the monotonic modifier was specified.
-  if (RT.isStaticNonchunked(ScheduleKind.Schedule,
-/* Chunked */ Chunk != nullptr) &&
-  !Ordered) {
+  if (S.getDefaultSchedule() == OMPDSK_static_chunkone) {
+// For NVPTX and other GPU targets high performance is often achieved

What's the difference between this code and next else branch?


https://reviews.llvm.org/D29910



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

[PATCH] D19586: Misleading Indentation check

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 88330.
xazax.hun marked 7 inline comments as done.
xazax.hun added a comment.

- Added a note to make it easier to understand the diagnostics.
- Reworded the error message about dangling else.
- Fixed other review comments.


https://reviews.llvm.org/D19586

Files:
  clang-tidy/readability/CMakeLists.txt
  clang-tidy/readability/MisleadingIndentationCheck.cpp
  clang-tidy/readability/MisleadingIndentationCheck.h
  clang-tidy/readability/ReadabilityTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/readability-misleading-indentation.rst
  test/clang-tidy/readability-misleading-indentation.cpp

Index: test/clang-tidy/readability-misleading-indentation.cpp
===
--- /dev/null
+++ test/clang-tidy/readability-misleading-indentation.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s readability-misleading-indentation %t
+
+void foo1();
+void foo2();
+
+#define BLOCK \
+  if (cond1)  \
+foo1();   \
+foo2();
+
+int main()
+{
+  bool cond1 = true;
+  bool cond2 = true;
+
+  if (cond1)
+if (cond2)
+  foo1();
+  else
+foo2();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+
+  if (cond1) {
+if (cond2)
+  foo1();
+  }
+  else
+foo2();
+
+  if (cond1)
+if (cond2)
+  foo1();
+else
+  foo2();
+
+  if (cond2)
+foo1();
+foo2();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: misleading indentation: statement is indented too deeply [readability-misleading-indentation]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: did you mean this line to be inside this 'if'
+foo2(); // No redundant warning.
+
+  if (cond1)
+  {
+foo1();
+  }
+foo2();
+
+  if (cond1)
+foo1();
+  foo2();
+
+  if (cond2)
+if (cond1) foo1(); else foo2();
+
+  if (cond1) {
+  } else {
+  }
+
+  if (cond1) {
+  }
+  else {
+  }
+
+  if (cond1)
+  {
+  }
+  else
+  {
+  }
+
+  if (cond1)
+{
+}
+  else
+{
+}
+
+  BLOCK
+}
Index: docs/clang-tidy/checks/readability-misleading-indentation.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/readability-misleading-indentation.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - readability-misleading-indentation
+
+readability-misleading-indentation
+==
+
+Correct indentation helps to understand code. Mismatch of the syntactical
+structure and the indentation of the code may hide serious problems.
+Missing braces can also make it significantly harder to read the code,
+therefore it is important to use braces. 
+
+The way to avoid dangling else is to always check that an ``else`` belongs
+to the ``if`` that begins in the same column.
+
+You can omit braces when your inner part of e.g. an ``if`` statement has only
+one statement in it. Although in that case you should begin the next statement
+in the same column with the ``if``.
+
+Examples:
+
+.. code-block:: c++
+
+  // Dangling else:
+  if (cond1)
+if (cond2)
+  foo1();
+  else
+foo2();  // Wrong indentation: else belongs to if(cond2) statement.
+
+  // Missing braces:
+  if (cond1)
+foo1();
+foo2();  // Not guarded by if(cond1).
+
+Limitations
+===
+
+Note that this check only works as expected when the tabs or spaces are used
+consistently and not mixed.
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -139,6 +139,7 @@
readability-identifier-naming
readability-implicit-bool-cast
readability-inconsistent-declaration-parameter-name
+   readability-misleading-indentation
readability-misplaced-array-index
readability-named-parameter
readability-non-const-parameter
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,11 @@
 Improvements to clang-tidy
 --
 
+- New `readability-misleading-indentation
+  `_ check
+
+  Finds misleading indentation where braces should be introduced or the code should be reformatted.
+
 - New `safety-no-assembler
   `_ check
 
Index: clang-tidy/readability/ReadabilityTidyModule.cpp
===
--- clang-tidy/readability/ReadabilityTidyModule.cpp
+++ clang-tidy/readability/ReadabilityTidyModule.cpp
@@ -20,6 +20,7 @@
 #include "IdentifierNamingCheck.h"
 #include "ImplicitBoolCastCheck.h"
 #include "InconsistentDeclarationParameterNameCheck.h"
+#include "MisleadingIndentationCheck.h"
 #include "MisplacedArrayIn

[PATCH] D19586: Misleading Indentation check

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Thank you for the review!




Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:79
+  Finder->addMatcher(
+  compoundStmt(anyOf(has(ifStmt()), has(forStmt()), has(whileStmt(
+  .bind("compound"),

alexfh wrote:
> `has(anyOf(ifStmt(), forStmt(), whileStmt()))` would read better.
I agree but unfortunately that does not compile. 



Comment at: test/clang-tidy/readability-misleading-indentation.cpp:21
+foo2();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: potential dangling 'else' 
[readability-misleading-indentation]
+

danielmarjamaki wrote:
> I am skeptic about this warning message.
> 
> Why does it say "potential". I would say that in this test case the 
> indentation _is_ "dangling".
> 
> The message is not very clear to me. I personally don't intuitively 
> understand what is wrong without looking at the code.
> 
> I don't know what it should say. Maybe:
> ```
> different indentation for 'if' and 'else'
> ```
> 
Thank you, good idea!


https://reviews.llvm.org/D19586



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

[PATCH] D29928: [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[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/misc/DefinitionsInHeadersCheck.cpp:131-135
+if (FD->getTemplateSpecializationKind() != TSK_Undeclared)
+  diag(FD->getLocation(), "this is a full function template specilization "
+  "which behaves as a regular function, so the ODR 
"
+  "still applies",
+   DiagnosticIDs::Note);

Notes are useful for pointing at related but different locations (e.g. point to 
the declaration of an entity when the diagnostic is issued at a reference to 
the entity).

Here I would suggest just issuing a different message (e.g. `diag(..., 
"%select{function|full function template specialization}0 %1 defined in a 
header file ") << (FD->getTemplateSpecializationKind() != TSK_Undeclared) 
<< FD << ...;`) or if it seems more helpful, add a note pointing to the 
template being specialized.


https://reviews.llvm.org/D29928



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

[PATCH] D23421: [Clang-tidy] CERT-MSC53-CPP (checker for std namespace modification)

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun updated this revision to Diff 88333.
xazax.hun marked 9 inline comments as done.
xazax.hun added a comment.

- Updated to latest trunk.
- The cert rule was renamed, the patch is updated accordingly.
- Fixes as per review comments.


https://reviews.llvm.org/D23421

Files:
  clang-tidy/cert/CERTTidyModule.cpp
  clang-tidy/cert/CMakeLists.txt
  clang-tidy/cert/DontModifyStdNamespaceCheck.cpp
  clang-tidy/cert/DontModifyStdNamespaceCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cert-dcl58-cpp.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/cert-dcl58-cpp.cpp

Index: test/clang-tidy/cert-dcl58-cpp.cpp
===
--- /dev/null
+++ test/clang-tidy/cert-dcl58-cpp.cpp
@@ -0,0 +1,37 @@
+// RUN: %check_clang_tidy %s cert-dcl58-cpp %t -- -- -std=c++1z
+
+namespace A {
+  namespace B {
+int b;
+  }
+}
+
+namespace A {
+  namespace B {
+int c;
+  }
+}
+
+namespace posix {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: modification of 'posix' namespace can result in undefined behavior [cert-dcl58-cpp]
+  namespace vmi {
+  }
+}
+
+namespace std {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: modification of 'std' namespace can
+  int stdInt;
+}
+
+namespace foobar {
+  namespace std {
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: modification of 'std' namespace can 
+  }
+}
+
+namespace posix::a {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: modification of 'posix' namespace 
+}
+
+using namespace std;
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -8,6 +8,7 @@
cert-dcl03-c (redirects to misc-static-assert) 
cert-dcl50-cpp
cert-dcl54-cpp (redirects to misc-new-delete-overloads) 
+   cert-dcl58-cpp
cert-dcl59-cpp (redirects to google-build-namespaces) 
cert-env33-c
cert-err09-cpp (redirects to misc-throw-by-value-catch-by-reference) 
Index: docs/clang-tidy/checks/cert-dcl58-cpp.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cert-dcl58-cpp.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - cert-dcl58-cpp
+
+cert-dcl58-cpp
+==
+
+Modification of the ``std`` or ``posix`` namespace can result in undefined
+behavior.
+This check warns for such modifications.
+
+Examples:
+
+.. code-block:: c++
+
+  namespace std {
+int x; // May cause undefined behavior.
+  }
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -57,6 +57,11 @@
 Improvements to clang-tidy
 --
 
+- New `cert-dcl58-cpp
+  `_ check
+
+  Finds modification of the ``std`` or ``posix`` namespace.
+
 - New `safety-no-assembler
   `_ check
 
Index: clang-tidy/cert/DontModifyStdNamespaceCheck.h
===
--- /dev/null
+++ clang-tidy/cert/DontModifyStdNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- DontModifyStdNamespaceCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_DONT_MODIFY_STD_NAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_DONT_MODIFY_STD_NAMESPACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace cert {
+
+/// Modification of the std or posix namespace can result in undefined behavior.
+/// This check warns for such modifications.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/cert-msc53-cpp.html
+class DontModifyStdNamespaceCheck : public ClangTidyCheck {
+public:
+  DontModifyStdNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace cert
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CERT_DONT_MODIFY_STD_NAMESPACE_H
Index: clang-tidy/cert/DontModifyStdNamespaceCheck.cpp
===
--- /dev/null
+++ clang-tidy/cert/DontModifyStdNamespaceCheck.cpp
@@ -0,0 +1,41 @@
+//===--- DontModifyStdNamespaceCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT fo

[PATCH] D19586: Misleading Indentation check

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG




Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:79
+  Finder->addMatcher(
+  compoundStmt(anyOf(has(ifStmt()), has(forStmt()), has(whileStmt(
+  .bind("compound"),

xazax.hun wrote:
> alexfh wrote:
> > `has(anyOf(ifStmt(), forStmt(), whileStmt()))` would read better.
> I agree but unfortunately that does not compile. 
Yep, with matchers you never know what will compile ;)

Maybe `has(stmt(anyOf(ifStmt(), forStmt(), whileStmt(`? If this also 
doesn't work, then it's fine as is.


https://reviews.llvm.org/D19586



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

[PATCH] D19586: Misleading Indentation check

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Gábor, thank you for picking up this patch and finishing it!


https://reviews.llvm.org/D19586



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

[clang-tools-extra] r295041 - [clang-tidy] Add readability-misleading-indentation check.

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Tue Feb 14 04:03:27 2017
New Revision: 295041

URL: http://llvm.org/viewvc/llvm-project?rev=295041&view=rev
Log:
[clang-tidy] Add readability-misleading-indentation check.

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

Added:

clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.h

clang-tools-extra/trunk/docs/clang-tidy/checks/readability-misleading-indentation.rst

clang-tools-extra/trunk/test/clang-tidy/readability-misleading-indentation.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt?rev=295041&r1=295040&r2=295041&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt Tue Feb 14 
04:03:27 2017
@@ -11,6 +11,7 @@ add_clang_library(clangTidyReadabilityMo
   IdentifierNamingCheck.cpp
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
+  MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp

Added: 
clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.cpp?rev=295041&view=auto
==
--- 
clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.cpp 
(added)
+++ 
clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.cpp 
Tue Feb 14 04:03:27 2017
@@ -0,0 +1,103 @@
+//===--- MisleadingIndentationCheck.cpp - 
clang-tidy---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "MisleadingIndentationCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+void MisleadingIndentationCheck::danglingElseCheck(const SourceManager &SM,
+   const IfStmt *If) {
+  SourceLocation IfLoc = If->getIfLoc();
+  SourceLocation ElseLoc = If->getElseLoc();
+
+  if (IfLoc.isMacroID() || ElseLoc.isMacroID())
+return;
+
+  if (SM.getExpansionLineNumber(If->getThen()->getLocEnd()) ==
+  SM.getExpansionLineNumber(ElseLoc))
+return;
+
+  if (SM.getExpansionColumnNumber(IfLoc) !=
+  SM.getExpansionColumnNumber(ElseLoc))
+diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
+}
+
+void MisleadingIndentationCheck::missingBracesCheck(const SourceManager &SM,
+const CompoundStmt *CStmt) 
{
+  const static StringRef StmtNames[] = {"if", "for", "while"};
+  for (unsigned int i = 0; i < CStmt->size() - 1; i++) {
+const Stmt *CurrentStmt = CStmt->body_begin()[i];
+const Stmt *Inner = nullptr;
+int StmtKind = 0;
+
+if (const auto *CurrentIf = dyn_cast(CurrentStmt)) {
+  StmtKind = 0;
+  Inner =
+  CurrentIf->getElse() ? CurrentIf->getElse() : CurrentIf->getThen();
+} else if (const auto *CurrentFor = dyn_cast(CurrentStmt)) {
+  StmtKind = 1;
+  Inner = CurrentFor->getBody();
+} else if (const auto *CurrentWhile = dyn_cast(CurrentStmt)) {
+  StmtKind = 2;
+  Inner = CurrentWhile->getBody();
+} else {
+  continue;
+}
+
+if (isa(Inner))
+  continue;
+
+SourceLocation InnerLoc = Inner->getLocStart();
+SourceLocation OuterLoc = CurrentStmt->getLocStart();
+
+if (SM.getExpansionLineNumber(InnerLoc) ==
+SM.getExpansionLineNumber(OuterLoc))
+  continue;
+
+const Stmt *NextStmt = CStmt->body_begin()[i + 1];
+SourceLocation NextLoc = NextStmt->getLocStart();
+
+if (InnerLoc.isMacroID() || OuterLoc.isMacroID() || NextLoc.isMacroID())
+  continue;
+
+if (SM.getExpansionColumnNumber(InnerLoc) ==
+SM.getExpansionColumnNumber(NextLoc)) {
+  diag(NextLoc, "misleading indentation: statement is indented too 
deeply");
+  diag(OuterLoc, "did you mean this line to be inside this '%0'",
+   DiagnosticIDs::Note)
+  << StmtNames[StmtKind];
+}
+  }
+}
+
+vo

[PATCH] D19586: Misleading Indentation check

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL295041: [clang-tidy] Add readability-misleading-indentation 
check. (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D19586?vs=88330&id=88334#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D19586

Files:
  clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.h
  clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/readability-misleading-indentation.rst
  clang-tools-extra/trunk/test/clang-tidy/readability-misleading-indentation.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/readability-misleading-indentation.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/readability-misleading-indentation.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/readability-misleading-indentation.cpp
@@ -0,0 +1,80 @@
+// RUN: %check_clang_tidy %s readability-misleading-indentation %t
+
+void foo1();
+void foo2();
+
+#define BLOCK \
+  if (cond1)  \
+foo1();   \
+foo2();
+
+int main()
+{
+  bool cond1 = true;
+  bool cond2 = true;
+
+  if (cond1)
+if (cond2)
+  foo1();
+  else
+foo2();
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: different indentation for 'if' and corresponding 'else' [readability-misleading-indentation]
+
+  if (cond1) {
+if (cond2)
+  foo1();
+  }
+  else
+foo2();
+
+  if (cond1)
+if (cond2)
+  foo1();
+else
+  foo2();
+
+  if (cond2)
+foo1();
+foo2();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: misleading indentation: statement is indented too deeply [readability-misleading-indentation]
+// CHECK-MESSAGES: :[[@LINE-4]]:3: note: did you mean this line to be inside this 'if'
+foo2(); // No redundant warning.
+
+  if (cond1)
+  {
+foo1();
+  }
+foo2();
+
+  if (cond1)
+foo1();
+  foo2();
+
+  if (cond2)
+if (cond1) foo1(); else foo2();
+
+  if (cond1) {
+  } else {
+  }
+
+  if (cond1) {
+  }
+  else {
+  }
+
+  if (cond1)
+  {
+  }
+  else
+  {
+  }
+
+  if (cond1)
+{
+}
+  else
+{
+}
+
+  BLOCK
+}
Index: clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/readability/CMakeLists.txt
@@ -11,6 +11,7 @@
   IdentifierNamingCheck.cpp
   ImplicitBoolCastCheck.cpp
   InconsistentDeclarationParameterNameCheck.cpp
+  MisleadingIndentationCheck.cpp
   MisplacedArrayIndexCheck.cpp
   NamedParameterCheck.cpp
   NamespaceCommentCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.h
+++ clang-tools-extra/trunk/clang-tidy/readability/MisleadingIndentationCheck.h
@@ -0,0 +1,41 @@
+//===--- MisleadingIndentationCheck.h - clang-tidy---*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Checks the code for dangling else, and possible misleading indentations due
+/// to missing braces. Note that this check only works as expected when the tabs
+/// or spaces are used consistently and not mixed.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/readability-misleading-indentation.html
+class MisleadingIndentationCheck : public ClangTidyCheck {
+public:
+  MisleadingIndentationCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  void danglingElseCheck(const SourceManager &SM, const IfStmt *If);
+  void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_MISLEADING_INDENTATION_H
Index: clang-tools-extra/trunk/clang-tidy/readability/ReadabilityTidyModule.c

[PATCH] D19586: Misleading Indentation check

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang-tidy/readability/MisleadingIndentationCheck.cpp:79
+  Finder->addMatcher(
+  compoundStmt(anyOf(has(ifStmt()), has(forStmt()), has(whileStmt(
+  .bind("compound"),

alexfh wrote:
> xazax.hun wrote:
> > alexfh wrote:
> > > `has(anyOf(ifStmt(), forStmt(), whileStmt()))` would read better.
> > I agree but unfortunately that does not compile. 
> Yep, with matchers you never know what will compile ;)
> 
> Maybe `has(stmt(anyOf(ifStmt(), forStmt(), whileStmt(`? If this also 
> doesn't work, then it's fine as is.
That did the trick, thanks :)


Repository:
  rL LLVM

https://reviews.llvm.org/D19586



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

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

A few more nits.




Comment at: clang-tidy/modernize/ReturnBracedInitListCheck.cpp:66
+  auto Diag = diag(Loc, "to avoid repeating the return type from the "
+"declaration, use a braced initializer list instead");
+

nit: Use a semicolon before "use": `; use`.



Comment at: docs/clang-tidy/checks/modernize-return-braced-init-list.rst:8
+initializer list. This way the return type is not needlessly duplicated in the
+return type and the return statement.
+

"return type is not ... duplicated in the return type" doesn't read well. 
"return type is not ... duplicated in the function definition" maybe?



Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:71
+  return Foo(b);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return 
type from the declaration, use a braced initializer list instead 
[modernize-return-braced-init-list]
+  // CHECK-FIXES: return {b};

Please truncate the static repeated parts of the CHECK lines around the 80th 
column (e.g. after "type" or after "declaration;", if the latter still fits 
into 80 columns). The first CHECK line should be complete though.



Comment at: test/clang-tidy/modernize-return-braced-init-list.cpp:72
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return 
type from the declaration, use a braced initializer list instead 
[modernize-return-braced-init-list]
+  // CHECK-FIXES: return {b};
+}

This pattern is not reliable, since there are multiple return statements 
involving `b` in this file. The only way check patterns are bound to the line 
is using the content (for this reason `CHECK-MESSAGES` contains 
`:[[@LINE-x]]:...`).

In case of `CHECK-FIXES` the easiest way to ensure patterns match only what is 
intended, is to make their content unique. Here I'd give all variables unique 
names (`b1`, `b2`, ..., for example). If there are multiple identical lines 
that contain no identifiers, they can be made unique by adding comments (and 
matching them).


Repository:
  rL LLVM

https://reviews.llvm.org/D28768



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

[PATCH] D29928: [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 88336.
hokein added a comment.

Address review comment.


https://reviews.llvm.org/D29928

Files:
  clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  test/clang-tidy/misc-definitions-in-headers.hpp


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -28,7 +28,7 @@
 
 template <>
 int CA::f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: full function template 
specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -92,7 +92,7 @@
 
 template <>
 int f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: full function template 
specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -122,10 +122,12 @@
   }
 }
 
+bool is_full_spec = FD->getTemplateSpecializationKind() != TSK_Undeclared;
 diag(FD->getLocation(),
- "function %0 defined in a header file; "
- "function definitions in header files can lead to ODR violations")
-<< FD << FixItHint::CreateInsertion(
+ "%select{function|full function template specialization}0 %1 defined "
+ "in a header file; function definitions in header files can lead to "
+ "ODR violations")
+<< is_full_spec << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -28,7 +28,7 @@
 
 template <>
 int CA::f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: full function template specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -92,7 +92,7 @@
 
 template <>
 int f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: full function template specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -122,10 +122,12 @@
   }
 }
 
+bool is_full_spec = FD->getTemplateSpecializationKind() != TSK_Undeclared;
 diag(FD->getLocation(),
- "function %0 defined in a header file; "
- "function definitions in header files can lead to ODR violations")
-<< FD << FixItHint::CreateInsertion(
+ "%select{function|full function template specialization}0 %1 defined "
+ "in a header file; function definitions in header files can lead to "
+ "ODR violations")
+<< is_full_spec << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r295043 - [scan-build-py] use subprocess wrapper to execute build

[Laszlo Nagy via cfe-commits]

Author: rizsotto
Date: Tue Feb 14 04:30:50 2017
New Revision: 295043

URL: http://llvm.org/viewvc/llvm-project?rev=295043&view=rev
Log:
[scan-build-py] use subprocess wrapper to execute build

Modified:
cfe/trunk/tools/scan-build-py/libscanbuild/__init__.py
cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
cfe/trunk/tools/scan-build-py/libscanbuild/intercept.py

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/__init__.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/__init__.py?rev=295043&r1=295042&r2=295043&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/__init__.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/__init__.py Tue Feb 14 04:30:50 
2017
@@ -39,6 +39,19 @@ def tempdir():
 return os.getenv('TMPDIR', os.getenv('TEMP', os.getenv('TMP', '/tmp')))
 
 
+def run_build(command, *args, **kwargs):
+""" Run and report build command execution
+
+:param command: array of tokens
+:return: exit code of the process
+"""
+environment = kwargs.get('env', os.environ)
+logging.debug('run build %s, in environment: %s', command, environment)
+exit_code = subprocess.call(command, *args, **kwargs)
+logging.debug('build finished with exit code: %d', exit_code)
+return exit_code
+
+
 def run_command(command, cwd=None):
 """ Run a given command and report the execution.
 

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py?rev=295043&r1=295042&r2=295043&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py Tue Feb 14 04:30:50 
2017
@@ -20,7 +20,8 @@ import argparse
 import logging
 import subprocess
 import multiprocessing
-from libscanbuild import initialize_logging, tempdir, command_entry_point
+from libscanbuild import initialize_logging, tempdir, command_entry_point, \
+run_build
 from libscanbuild.runner import run
 from libscanbuild.intercept import capture
 from libscanbuild.report import report_directory, document
@@ -70,9 +71,7 @@ def analyze_build_main(bin_dir, from_bui
 # run the build command with compiler wrappers which
 # execute the analyzer too. (interposition)
 environment = setup_environment(args, target_dir, bin_dir)
-logging.debug('run build in environment: %s', environment)
-exit_code = subprocess.call(args.build, env=environment)
-logging.debug('build finished with exit code: %d', exit_code)
+exit_code = run_build(args.build, env=environment)
 # cover report generation and bug counting
 number_of_bugs = document(args, target_dir, False)
 # set exit status as it was requested

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/intercept.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/intercept.py?rev=295043&r1=295042&r2=295043&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/intercept.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/intercept.py Tue Feb 14 04:30:50 
2017
@@ -31,7 +31,7 @@ import argparse
 import logging
 import subprocess
 from libear import build_libear, TemporaryDirectory
-from libscanbuild import command_entry_point, run_command
+from libscanbuild import command_entry_point, run_build, run_command
 from libscanbuild import duplicate_check, tempdir, initialize_logging
 from libscanbuild.compilation import split_command
 from libscanbuild.shell import encode, decode
@@ -95,9 +95,7 @@ def capture(args, bin_dir):
 with TemporaryDirectory(prefix='intercept-', dir=tempdir()) as tmp_dir:
 # run the build command
 environment = setup_environment(args, tmp_dir, bin_dir)
-logging.debug('run build in environment: %s', environment)
-exit_code = subprocess.call(args.build, env=environment)
-logging.info('build finished with exit code: %d', exit_code)
+exit_code = run_build(args.build, env=environment)
 # read the intercepted exec calls
 exec_traces = itertools.chain.from_iterable(
 parse_exec_trace(os.path.join(tmp_dir, filename))


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

[PATCH] D29928: [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[Haojian Wu via Phabricator via cfe-commits]

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



Comment at: clang-tidy/misc/DefinitionsInHeadersCheck.cpp:131-135
+if (FD->getTemplateSpecializationKind() != TSK_Undeclared)
+  diag(FD->getLocation(), "this is a full function template specilization "
+  "which behaves as a regular function, so the ODR 
"
+  "still applies",
+   DiagnosticIDs::Note);

alexfh wrote:
> Notes are useful for pointing at related but different locations (e.g. point 
> to the declaration of an entity when the diagnostic is issued at a reference 
> to the entity).
> 
> Here I would suggest just issuing a different message (e.g. `diag(..., 
> "%select{function|full function template specialization}0 %1 defined in a 
> header file ") << (FD->getTemplateSpecializationKind() != TSK_Undeclared) 
> << FD << ...;`) or if it seems more helpful, add a note pointing to the 
> template being specialized.
SG. Didn't know `%select` before, thanks for pointing it out.


https://reviews.llvm.org/D29928



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

r295044 - [clang-format] Remove dead code in FormatTestComments, NFC

[Krasimir Georgiev via cfe-commits]

Author: krasimir
Date: Tue Feb 14 04:35:42 2017
New Revision: 295044

URL: http://llvm.org/viewvc/llvm-project?rev=295044&view=rev
Log:
[clang-format] Remove dead code in FormatTestComments, NFC

Modified:
cfe/trunk/unittests/Format/FormatTestComments.cpp

Modified: cfe/trunk/unittests/Format/FormatTestComments.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestComments.cpp?rev=295044&r1=295043&r2=295044&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestComments.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestComments.cpp Tue Feb 14 04:35:42 2017
@@ -20,7 +20,6 @@
 #define DEBUG_TYPE "format-test"
 
 using clang::tooling::ReplacementTest;
-using clang::tooling::toReplacements;
 
 namespace clang {
 namespace format {
@@ -67,12 +66,6 @@ protected:
 EXPECT_EQ(Code.str(), format(test::messUp(Code), Style));
   }
 
-  void verifyIncompleteFormat(llvm::StringRef Code,
-  const FormatStyle &Style = getLLVMStyle()) {
-EXPECT_EQ(Code.str(),
-  format(test::messUp(Code), Style, IC_ExpectIncomplete));
-  }
-
   void verifyGoogleFormat(llvm::StringRef Code) {
 verifyFormat(Code, getGoogleStyle());
   }


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

r295045 - [scan-build-py] move function report_directory from report module to analyze module

[Laszlo Nagy via cfe-commits]

Author: rizsotto
Date: Tue Feb 14 04:43:38 2017
New Revision: 295045

URL: http://llvm.org/viewvc/llvm-project?rev=295045&view=rev
Log:
[scan-build-py] move function report_directory from report module to analyze 
module

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

Modified:
cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
cfe/trunk/tools/scan-build-py/libscanbuild/report.py
cfe/trunk/tools/scan-build-py/tests/unit/test_analyze.py
cfe/trunk/tools/scan-build-py/tests/unit/test_report.py

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py?rev=295045&r1=295044&r2=295045&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/analyze.py Tue Feb 14 04:43:38 
2017
@@ -18,13 +18,16 @@ import os.path
 import json
 import argparse
 import logging
+import tempfile
 import subprocess
 import multiprocessing
+import contextlib
+import datetime
 from libscanbuild import initialize_logging, tempdir, command_entry_point, \
 run_build
 from libscanbuild.runner import run
 from libscanbuild.intercept import capture
-from libscanbuild.report import report_directory, document
+from libscanbuild.report import document
 from libscanbuild.clang import get_checkers
 from libscanbuild.compilation import split_command
 
@@ -190,6 +193,39 @@ def analyze_build_wrapper(cplusplus):
 return result
 
 
+@contextlib.contextmanager
+def report_directory(hint, keep):
+""" Responsible for the report directory.
+
+hint -- could specify the parent directory of the output directory.
+keep -- a boolean value to keep or delete the empty report directory. """
+
+stamp_format = 'scan-build-%Y-%m-%d-%H-%M-%S-%f-'
+stamp = datetime.datetime.now().strftime(stamp_format)
+parent_dir = os.path.abspath(hint)
+if not os.path.exists(parent_dir):
+os.makedirs(parent_dir)
+name = tempfile.mkdtemp(prefix=stamp, dir=parent_dir)
+
+logging.info('Report directory created: %s', name)
+
+try:
+yield name
+finally:
+if os.listdir(name):
+msg = "Run 'scan-view %s' to examine bug reports."
+keep = True
+else:
+if keep:
+msg = "Report directory '%s' contains no report, but kept."
+else:
+msg = "Removing directory '%s' because it contains no report."
+logging.warning(msg, name)
+
+if not keep:
+os.rmdir(name)
+
+
 def analyzer_params(args):
 """ A group of command line arguments can mapped to command
 line arguments of the analyzer. This method generates those. """

Modified: cfe/trunk/tools/scan-build-py/libscanbuild/report.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/libscanbuild/report.py?rev=295045&r1=295044&r2=295045&view=diff
==
--- cfe/trunk/tools/scan-build-py/libscanbuild/report.py (original)
+++ cfe/trunk/tools/scan-build-py/libscanbuild/report.py Tue Feb 14 04:43:38 
2017
@@ -13,54 +13,15 @@ import os
 import os.path
 import sys
 import shutil
-import time
-import tempfile
 import itertools
 import plistlib
 import glob
 import json
 import logging
-import contextlib
-import datetime
 from libscanbuild import duplicate_check
 from libscanbuild.clang import get_version
 
-__all__ = ['report_directory', 'document']
-
-
-@contextlib.contextmanager
-def report_directory(hint, keep):
-""" Responsible for the report directory.
-
-hint -- could specify the parent directory of the output directory.
-keep -- a boolean value to keep or delete the empty report directory. """
-
-stamp_format = 'scan-build-%Y-%m-%d-%H-%M-%S-%f-'
-stamp = datetime.datetime.now().strftime(stamp_format)
-
-parentdir = os.path.abspath(hint)
-if not os.path.exists(parentdir):
-os.makedirs(parentdir)
-
-name = tempfile.mkdtemp(prefix=stamp, dir=parentdir)
-
-logging.info('Report directory created: %s', name)
-
-try:
-yield name
-finally:
-if os.listdir(name):
-msg = "Run 'scan-view %s' to examine bug reports."
-keep = True
-else:
-if keep:
-msg = "Report directory '%s' contans no report, but kept."
-else:
-msg = "Removing directory '%s' because it contains no report."
-logging.warning(msg, name)
-
-if not keep:
-os.rmdir(name)
+__all__ = ['document']
 
 
 def document(args, output_dir, use_cdb):

Modified: cfe/trunk/tools/scan-build-py/tests/unit/test_analyze.py
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/scan-build-py/tests/unit/test_analyze.py?rev=295045&r1=295044&r2=295045&view=diff
===

[PATCH] D29930: Add `__is_direct_constructible` trait for checking safe reference initialization.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

The STL types `std::pair` and `std::tuple` can both store reference types. 
However their constructors cannot adequately check if the initialization of 
reference types is safe.  For example:

  std::tuple const&> t = 42;
  // The stored reference is already dangling.

This patch attempts to implement a traits that acts the same as 
`__is_constructible(type, argtypes...)` except when `type` is a reference. In 
which case the trait disallows initialization from a materialized temporary.  
For example:

  static_assert(__is_constructible(int const&, long));
  static_assert(!__is_direct_constructible(int const&, long));

I don't think `__is_direct_constructible` is a good name, but I don't have any 
better ideas. All bikeshedding welcome.


https://reviews.llvm.org/D29930

Files:
  docs/LanguageExtensions.rst
  include/clang/Basic/TokenKinds.def
  include/clang/Basic/TypeTraits.h
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExpr.cpp
  lib/Sema/SemaExprCXX.cpp
  test/SemaCXX/type-traits.cpp

Index: test/SemaCXX/type-traits.cpp
===
--- test/SemaCXX/type-traits.cpp
+++ test/SemaCXX/type-traits.cpp
@@ -2048,6 +2048,8 @@
 
   // PR25513
   { int arr[F(__is_constructible(int(int)))]; }
+
+  { int arr[T(__is_constructible(int const&, long))]; }
 }
 
 // Instantiation of __is_trivially_constructible
@@ -2078,6 +2080,29 @@
   { int arr[F((is_trivially_constructible::value))]; } // PR19178
 }
 
+struct MultiInit {
+  template 
+  MultiInit(Args&&...) {}
+};
+
+void direct_constructible_checks() {
+  { int arr[T((__is_direct_constructible(int&, int&)))]; }
+  { int arr[F((__is_direct_constructible(int&, int&&)))]; }
+
+  { int arr[T((__is_direct_constructible(int const&, int&)))]; }
+  { int arr[T((__is_direct_constructible(int const&, int const&)))]; }
+  { int arr[T((__is_direct_constructible(int const&, int&&)))]; }
+
+  { int arr[F((__is_direct_constructible(int&, long &)))]; }
+  { int arr[F((__is_direct_constructible(int &&, long &)))]; }
+  { int arr[F((__is_direct_constructible(MultiInit&&, long, int, void*)))]; }
+  { int arr[F((__is_direct_constructible(MultiInit&&, MultiInit const&)))]; }
+
+  // Test that it works like __is_constructible on object types.
+  { int arr[T((__is_direct_constructible(int, long)))]; }
+  { int arr[T((__is_direct_constructible(MultiInit, long, int, void*)))]; }
+}
+
 void array_rank() {
   int t01[T(__array_rank(IntAr) == 1)];
   int t02[T(__array_rank(ConstIntArAr) == 2)];
Index: lib/Sema/SemaExprCXX.cpp
===
--- lib/Sema/SemaExprCXX.cpp
+++ lib/Sema/SemaExprCXX.cpp
@@ -4570,6 +4570,7 @@
 
   switch (Kind) {
   case clang::TT_IsConstructible:
+  case clang::TT_IsDirectConstructible:
   case clang::TT_IsNothrowConstructible:
   case clang::TT_IsTriviallyConstructible: {
 // C++11 [meta.unary.prop]:
@@ -4644,6 +4645,13 @@
 if (Kind == clang::TT_IsConstructible)
   return true;
 
+if (Kind == clang::TT_IsDirectConstructible) {
+  if (!T->isReferenceType())
+return true;
+
+  return Init.isDirectReferenceBinding();
+}
+
 if (Kind == clang::TT_IsNothrowConstructible)
   return S.canThrow(Result.get()) == CT_Cannot;
 
Index: lib/Parse/ParseExpr.cpp
===
--- lib/Parse/ParseExpr.cpp
+++ lib/Parse/ParseExpr.cpp
@@ -809,6 +809,7 @@
   REVERTIBLE_TYPE_TRAIT(__is_constructible);
   REVERTIBLE_TYPE_TRAIT(__is_convertible);
   REVERTIBLE_TYPE_TRAIT(__is_convertible_to);
+  REVERTIBLE_TYPE_TRAIT(__is_direct_constructible);
   REVERTIBLE_TYPE_TRAIT(__is_destructible);
   REVERTIBLE_TYPE_TRAIT(__is_empty);
   REVERTIBLE_TYPE_TRAIT(__is_enum);
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -1394,6 +1394,7 @@
   tok::kw___is_constructible,
   tok::kw___is_convertible,
   tok::kw___is_convertible_to,
+  tok::kw___is_direct_constructible,
   tok::kw___is_destructible,
   tok::kw___is_empty,
   tok::kw___is_enum,
Index: include/clang/Basic/TypeTraits.h
===
--- include/clang/Basic/TypeTraits.h
+++ include/clang/Basic/TypeTraits.h
@@ -79,6 +79,7 @@
 BTT_IsTriviallyAssignable,
 BTT_Last = BTT_IsTriviallyAssignable,
 TT_IsConstructible,
+TT_IsDirectConstructible,
 TT_IsNothrowConstructible,
 TT_IsTriviallyConstructible
   };
Index: include/clang/Basic/TokenKinds.def
===
--- include/clang/Basic/TokenKinds.def
+++ include/clang/Basic/TokenKinds.def
@@ -415,6 +415,7 @@
 TYPE_TRAIT_2(__is_nothrow_assignable, IsNothrow

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added inline comments.



Comment at: clangd/ASTManager.cpp:148
+  std::string Error;
+  I = tooling::CompilationDatabase::autoDetectFromSource(Uri, Error);
+  return I.get();

Do you have an idea about a proper error handling if Error?



Comment at: clangd/ASTManager.cpp:162
+Commands = CDB->getCompileCommands(Uri);
+// chdir. This is thread hostile.
+if (!Commands.empty())

Then maybe we need a big disclaimer comment at the declaration of 
`createASTUnitForFile`?



Comment at: clangd/ClangDMain.cpp:32
+  auto *AST = new ASTManager(Out, Store);
+  Store.addListener(std::unique_ptr(AST));
   JSONRPCDispatcher Dispatcher(llvm::make_unique(Out));

Why not directly `Store.addListener(llvm::make_unique(Out, 
Store));`?



Comment at: clangd/DocumentStore.h:39
+for (const auto &Listener : Listeners)
+  Listener->onDocumentAdd(Uri, *this);
+  }

Is only the main thread supposed to addDocument-s? Otherwise the listener 
notifications might need to be guarded.


https://reviews.llvm.org/D29886



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

[PATCH] D28445: [Analyzer] Extend taint propagation and checking

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin added a comment.

Sorry for the delay, I have commented inline.
For me, this patch looks like a strict improvement. I think, after some clean 
up this can be accepted.




Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:442
+
+const RecordDecl *RD = RT->getDecl()->getDefinition();
+for (const auto *I : RD->fields()) {

vlad.tsyrklevich wrote:
> a.sidorin wrote:
> > NoQ wrote:
> > > We need to be careful in the case when we don't have the definition in 
> > > the current translation unit. In this case we may still have derived 
> > > symbols by casting the pointer into some blindly guessed type, which may 
> > > be primitive or having well-defined primitive fields.
> > > 
> > > By the way, in D26837 i'm suspecting that there are other errors of this 
> > > kind in the checker, eg. when a function returns a void pointer, we put 
> > > taint on symbols of type "void", which is weird.
> > > 
> > > Adding Alexey who may recall something on this topic.
> > I will check the correctness of this code sample because I have some doubts 
> > about it.
> > The problem of casts is hard because of our approach to put taints on 0th 
> > elements. We lose casts and may get some strange void symbols. However, I 
> > guess this patch isn't related to this problem directly.
> Not sure which form of correctness you're interested in here but I'll bring 
> up one issue I'm aware of: currently this will create a new SymbolDerived for 
> an LCV sub-region, but it won't be correctly matched against in `isTainted()` 
> because subsequent references to the same region will have a different 
> SymbolDerived. This is the FIXME I mentioned below in `taint-generic.c` I 
> have some idea on how to fix this but I think it will probably require more 
> back and forth, hence why I didn't include it in this change. As it stands 
> now, the sub-region tainting could be removed without changing the 
> functionality of the current patch.
Checking a default binding symbol here works because we're in PostStmt of the 
call that creates this default binding. I think this machinery deserves a 
comment, it was not evident for me initially.
However, I don't like to create a SymbolDerived manually. The first idea is to 
just use getSVal(MemRegion*) which will return a SymbolDerived if it is. The 
second is to create... SymbolMetadata that will carry a taint (if the value is 
not a symbol, for example). Not sure if second idea is sane enough, I need some 
comments on it. Artem, Anna?
Also, instead of referring to the base region, we may need to walk parents 
one-by-one.


https://reviews.llvm.org/D28445



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

[PATCH] D29899: [clang-tidy] Add support for NOLINTNEXTLINE.

[Alexander Kornienko via Phabricator via cfe-commits]

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

Awesome! LG with one nit.




Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:300
+  // Now skip any newlines.
+  // FIXME: We want to skip over exactly one line, not an arbitrary number.
+  while (P != BufBegin && (*P == '\r' || *P == '\n'))

Is it difficult to address right away?

I also suppose (https://en.wikipedia.org/wiki/Newline#Representations) that we 
only care about CR+LF or LF line endings, so just looking at LFs seems more 
than enough (and will make code slightly simpler). This is the approach we took 
in many places in clang-format.


https://reviews.llvm.org/D29899



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

[PATCH] D29928: [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. Thank you!


https://reviews.llvm.org/D29928



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Mateusz Mikuła via Phabricator via cfe-commits]

mati865 added a comment.

@yaron.keren, @rnk I would like to see OpenMP support by MinGW Clang but using 
GCC "internal" headers is just wrong.

> For example, the most popular (37089 download this week) 
> https://sourceforge.net/projects/mingw-w64 distribuion for Windows had 
> elected to place the includes omp.h, quadmath.h, openacc.h at 
> c:\mingw32\lib\gcc\i686-w64-mingw32\5.4.0\include\

It wasn't mingw-w64 decision but rather GCC. Linux examples:

- Arch Linux: `/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.1/include/`
- Ubuntu 14.04: `/usr/lib/gcc/x86_64-linux-gnu/4.8/include`
- Ubuntu 17.04: `/usr/lib/gcc/x86_64-linux-gnu/6/include`

None of those directories are searched by Clang on Linux host.

> Removing the include dir will make clang less useful for mingw users using 
> these includes, since they will not be found anymore.

I highly doubt it:

Summary:

1. quadmath.h:
  - Linux:not working, quadmath. not found
  - MinGW with GCC headers:  not working, __float128 isn't supported
  - MinGW without GCC headers: not working, quadmath. not found and __float128 
isn't supported
2. omp.h:
  - Linux:working via 
http://openmp.llvm.org/
  - MinGW with GCC headers:  not working, compilation error
  - MinGW without GCC headers: not working, http://openmp.llvm.org/ is not 
buildable
3. openacc.h:
  - Linux:not working, openacc.h not found
  - MinGW with GCC headers:  not working, not tested
  - MinGW without GCC headers: not working, openacc.h not found

Details:
**quadmath.h** is useless without __float128

Clang with enabled __float128 via patch:

  $ clang++ float128_ex.cc -std=gnu++11
  
  $ ./a
  Segmentation fault

Clang with unpatched __float128:

  $ clang++ float128_ex.cc -std=gnu++11
  
  
D:\projekty\msys2\clang\msys64\mingw64\lib\gcc\x86_64-w64-mingw32\6.3.0\include\quadmath.h:43:26:
 error: __float128 is not supported on this target
  extern __float128 acosq (__float128) __quadmath_throw;

GCC:

  $ g++ float128_ex.cc -lquadmath -std=gnu++11
  
  $ ./a
  r=0
  exp_d=255250

**omp.h** crashes, example 
https://computing.llnl.gov/tutorials/openMP/samples/C/omp_hello.c :

  $ g++ omp.cc -fopenmp
  
  $ ./a
  Hello World from thread = 0
  Hello World from thread = 1
  Number of threads = 2
  
  $ clang++ omp.cc -fopenmp
  D:\projekty\msys2\clang\msys64\tmp\omp-87d437.o:(.text+0x35): undefined 
reference to `__imp___kmpc_fork_call'
  clang++.exe: error: linker command failed with exit code 1 (use -v to see 
invocation)

**openacc.h** isn't available even on Linux (openacc.h not found) and I cannot 
test it with MinGW


https://reviews.llvm.org/D29464



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

[clang-tools-extra] r295048 - [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Tue Feb 14 06:39:22 2017
New Revision: 295048

URL: http://llvm.org/viewvc/llvm-project?rev=295048&view=rev
Log:
[clang-tidy] Improve diagnostic message for misc-definitions-in-header.

Summary:
Users might get confused easily when they see the check's message on
full template function speciliations.

Add a note to the output message, which mentions these kind of function
specializations are treated as regular functions.

Reviewers: alexfh

Reviewed By: alexfh

Subscribers: JDevlieghere, cfe-commits

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

Modified:
clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp

Modified: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp?rev=295048&r1=295047&r2=295048&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp Tue 
Feb 14 06:39:22 2017
@@ -122,10 +122,12 @@ void DefinitionsInHeadersCheck::check(co
   }
 }
 
+bool is_full_spec = FD->getTemplateSpecializationKind() != TSK_Undeclared;
 diag(FD->getLocation(),
- "function %0 defined in a header file; "
- "function definitions in header files can lead to ODR violations")
-<< FD << FixItHint::CreateInsertion(
+ "%select{function|full function template specialization}0 %1 defined "
+ "in a header file; function definitions in header files can lead to "
+ "ODR violations")
+<< is_full_spec << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.

Modified: 
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp?rev=295048&r1=295047&r2=295048&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp Tue 
Feb 14 06:39:22 2017
@@ -28,7 +28,7 @@ void CA::f2() { }
 
 template <>
 int CA::f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: full function template 
specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -92,7 +92,7 @@ T f3() {
 
 template <>
 int f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: full function template 
specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;


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

[PATCH] D29928: [clang-tidy] Improve diagnostic message for misc-definitions-in-header.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rL295048: [clang-tidy] Improve diagnostic message for 
misc-definitions-in-header. (authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D29928?vs=88336&id=88350#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29928

Files:
  clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp


Index: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -28,7 +28,7 @@
 
 template <>
 int CA::f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: full function template 
specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -92,7 +92,7 @@
 
 template <>
 int f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a 
header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: full function template 
specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;
Index: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -122,10 +122,12 @@
   }
 }
 
+bool is_full_spec = FD->getTemplateSpecializationKind() != TSK_Undeclared;
 diag(FD->getLocation(),
- "function %0 defined in a header file; "
- "function definitions in header files can lead to ODR violations")
-<< FD << FixItHint::CreateInsertion(
+ "%select{function|full function template specialization}0 %1 defined "
+ "in a header file; function definitions in header files can lead to "
+ "ODR violations")
+<< is_full_spec << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.


Index: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
@@ -28,7 +28,7 @@
 
 template <>
 int CA::f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: full function template specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int CA::f3() {
   int a = 1;
   return a;
@@ -92,7 +92,7 @@
 
 template <>
 int f3() {
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f3' defined in a header file;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: full function template specialization 'f3' defined in a header file;
 // CHECK-FIXES: inline int f3() {
   int a = 1;
   return a;
Index: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -122,10 +122,12 @@
   }
 }
 
+bool is_full_spec = FD->getTemplateSpecializationKind() != TSK_Undeclared;
 diag(FD->getLocation(),
- "function %0 defined in a header file; "
- "function definitions in header files can lead to ODR violations")
-<< FD << FixItHint::CreateInsertion(
+ "%select{function|full function template specialization}0 %1 defined "
+ "in a header file; function definitions in header files can lead to "
+ "ODR violations")
+<< is_full_spec << FD << FixItHint::CreateInsertion(
  FD->getReturnTypeSourceRange().getBegin(), "inline ");
   } else if (const auto *VD = dyn_cast(ND)) {
 // Static data members of a class template are allowed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r295049 - [clang-tidy] Add support for NOLINTNEXTLINE.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Feb 14 06:47:56 2017
New Revision: 295049

URL: http://llvm.org/viewvc/llvm-project?rev=295049&view=rev
Log:
[clang-tidy] Add support for NOLINTNEXTLINE.

Reviewers: alexfh

Subscribers: JDevlieghere, cfe-commits

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

Added:
clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp?rev=295049&r1=295048&r2=295049&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp Tue Feb 
14 06:47:56 2017
@@ -272,6 +272,7 @@ static bool LineIsMarkedWithNOLINT(Sourc
   if (Invalid)
 return false;
 
+  // Check if there's a NOLINT on this line.
   const char *P = CharacterData;
   while (*P != '\0' && *P != '\r' && *P != '\n')
 ++P;
@@ -279,6 +280,34 @@ static bool LineIsMarkedWithNOLINT(Sourc
   // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
   if (RestOfLine.find("NOLINT") != StringRef::npos)
 return true;
+
+  // Check if there's a NOLINTNEXTLINE on the previous line.
+  const char *BufBegin =
+  SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), 
&Invalid);
+  if (Invalid || P == BufBegin)
+return false;
+
+  // Scan backwards over the current line.
+  P = CharacterData;
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  // If we reached the begin of the file there is no line before it.
+  if (P == BufBegin)
+return false;
+
+  // Skip over the newline.
+  --P;
+  const char *LineEnd = P;
+
+  // Now we're on the previous line. Skip to the beginning of it.
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  RestOfLine = StringRef(P, LineEnd - P + 1);
+  if (RestOfLine.find("NOLINTNEXTLINE") != StringRef::npos)
+return true;
+
   return false;
 }
 

Added: clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp?rev=295049&view=auto
==
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp Tue Feb 14 
06:47:56 2017
@@ -0,0 +1,33 @@
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(we-dont-care-about-categories-yet)
+class C { C(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must 
be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT)
+
+// RUN: %check_clang_tidy %s google-explicit-constructor %t --


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

[PATCH] D29899: [clang-tidy] Add support for NOLINTNEXTLINE.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL295049: [clang-tidy] Add support for NOLINTNEXTLINE. 
(authored by d0k).

Changed prior to commit:
  https://reviews.llvm.org/D29899?vs=88352&id=88353#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D29899

Files:
  clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp


Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -272,13 +272,42 @@
   if (Invalid)
 return false;
 
+  // Check if there's a NOLINT on this line.
   const char *P = CharacterData;
   while (*P != '\0' && *P != '\r' && *P != '\n')
 ++P;
   StringRef RestOfLine(CharacterData, P - CharacterData + 1);
   // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
   if (RestOfLine.find("NOLINT") != StringRef::npos)
 return true;
+
+  // Check if there's a NOLINTNEXTLINE on the previous line.
+  const char *BufBegin =
+  SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), 
&Invalid);
+  if (Invalid || P == BufBegin)
+return false;
+
+  // Scan backwards over the current line.
+  P = CharacterData;
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  // If we reached the begin of the file there is no line before it.
+  if (P == BufBegin)
+return false;
+
+  // Skip over the newline.
+  --P;
+  const char *LineEnd = P;
+
+  // Now we're on the previous line. Skip to the beginning of it.
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  RestOfLine = StringRef(P, LineEnd - P + 1);
+  if (RestOfLine.find("NOLINTNEXTLINE") != StringRef::npos)
+return true;
+
   return false;
 }
 
Index: clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
@@ -0,0 +1,33 @@
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(we-dont-care-about-categories-yet)
+class C { C(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must 
be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT)
+
+// RUN: %check_clang_tidy %s google-explicit-constructor %t --


Index: clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -272,13 +272,42 @@
   if (Invalid)
 return false;
 
+  // Check if there's a NOLINT on this line.
   const char *P = CharacterData;
   while (*P != '\0' && *P != '\r' && *P != '\n')
 ++P;
   StringRef RestOfLine(CharacterData, P - CharacterData + 1);
   // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
   if (RestOfLine.find("NOLINT") != StringRef::npos)
 return true;
+
+  // Check if there's a NOLINTNEXTLINE on the previous line.
+  const char *BufBegin =
+  SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), &Invalid);
+  if (Invalid || P == BufBegin)
+return false;
+
+  // Scan backwards over the current line.
+  P = CharacterData;
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  // If we reached the begin of the file there is no line before it.
+  if (P == BufBegin)
+return false;
+
+  // Skip over the newline.
+  --P;
+  const char *LineEnd = P;
+
+  // Now we're on the previous line. Skip to the beginning of it.
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  RestOfLine = StringRef(P, LineEnd - P + 1);
+  if (RestOfLine.find("NOLINTNEXTLINE") != StringRef::npos)
+return true;
+
   return false;
 }
 
Index: clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/nolintnextline.cpp
@@ -0,0 +1,33 @@
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+

[PATCH] D29899: [clang-tidy] Add support for NOLINTNEXTLINE.

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer updated this revision to Diff 88352.
bkramer added a comment.

- Simplify code by not worrying about \r
- Don't allow blank lines between NOLINTNEXTLINE and the warning


https://reviews.llvm.org/D29899

Files:
  clang-tidy/ClangTidyDiagnosticConsumer.cpp
  test/clang-tidy/nolintnextline.cpp


Index: test/clang-tidy/nolintnextline.cpp
===
--- /dev/null
+++ test/clang-tidy/nolintnextline.cpp
@@ -0,0 +1,33 @@
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(we-dont-care-about-categories-yet)
+class C { C(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must 
be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must 
be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT)
+
+// RUN: %check_clang_tidy %s google-explicit-constructor %t --
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -272,13 +272,42 @@
   if (Invalid)
 return false;
 
+  // Check if there's a NOLINT on this line.
   const char *P = CharacterData;
   while (*P != '\0' && *P != '\r' && *P != '\n')
 ++P;
   StringRef RestOfLine(CharacterData, P - CharacterData + 1);
   // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
   if (RestOfLine.find("NOLINT") != StringRef::npos)
 return true;
+
+  // Check if there's a NOLINTNEXTLINE on the previous line.
+  const char *BufBegin =
+  SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), 
&Invalid);
+  if (Invalid || P == BufBegin)
+return false;
+
+  // Scan backwards over the current line.
+  P = CharacterData;
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  // If we reached the begin of the file there is no line before it.
+  if (P == BufBegin)
+return false;
+
+  // Skip over the newline.
+  --P;
+  const char *LineEnd = P;
+
+  // Now we're on the previous line. Skip to the beginning of it.
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  RestOfLine = StringRef(P, LineEnd - P + 1);
+  if (RestOfLine.find("NOLINTNEXTLINE") != StringRef::npos)
+return true;
+
   return false;
 }
 


Index: test/clang-tidy/nolintnextline.cpp
===
--- /dev/null
+++ test/clang-tidy/nolintnextline.cpp
@@ -0,0 +1,33 @@
+class A { A(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+class B { B(int i); };
+
+// NOLINTNEXTLINE(we-dont-care-about-categories-yet)
+class C { C(int i); };
+
+
+// NOLINTNEXTLINE
+
+class D { D(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+// NOLINTNEXTLINE
+//
+class E { E(int i); };
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: single-argument constructors must be marked explicit
+
+#define MACRO(X) class X { X(int i); };
+MACRO(F)
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: single-argument constructors must be marked explicit
+// NOLINTNEXTLINE
+MACRO(G)
+
+#define MACRO_NOARG class H { H(int i); };
+// NOLINTNEXTLINE
+MACRO_NOARG
+
+// CHECK-MESSAGES: Suppressed 4 warnings (4 NOLINT)
+
+// RUN: %check_clang_tidy %s google-explicit-constructor %t --
Index: clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -272,13 +272,42 @@
   if (Invalid)
 return false;
 
+  // Check if there's a NOLINT on this line.
   const char *P = CharacterData;
   while (*P != '\0' && *P != '\r' && *P != '\n')
 ++P;
   StringRef RestOfLine(CharacterData, P - CharacterData + 1);
   // FIXME: Handle /\bNOLINT\b(\([^)]*\))?/ as cpplint.py does.
   if (RestOfLine.find("NOLINT") != StringRef::npos)
 return true;
+
+  // Check if there's a NOLINTNEXTLINE on the previous line.
+  const char *BufBegin =
+  SM.getCharacterData(SM.getLocForStartOfFile(SM.getFileID(Loc)), &Invalid);
+  if (Invalid || P == BufBegin)
+return false;
+
+  // Scan backwards over the current line.
+  P = CharacterData;
+  while (P != BufBegin && *P != '\n')
+--P;
+
+  // If we reached the begin of the file there is no line before it.
+  if (P == BufBegin)
+return false;
+
+  // Skip over 

[PATCH] D29910: [OpenMP] Specialize default schedule on a worksharing loop on the NVPTX device.

[Arpith Jacob via Phabricator via cfe-commits]

arpith-jacob added a comment.

Hi Alexey,

Thank you for your review.  The main difference in the specialized codegen (if 
vs. else part in CGStmtOpenMP.cpp).

If-part: emitForStaticInit uses the Chunk parameter (else has it set to null).
If-part: does not use EmitIgnoredExpr()

I can combine if- and else- part with appropriate guards over the above two 
statements if you like.

> General comment: do you really need to define default schedule kind in Sema? 
> Could you do it during codegen phase?

Yes, we need to define default schedule in Sema because it has to set getCond() 
and getInc() in CheckOpenMPLoop(), which is in Sema.  For example, the Inc 
expression is calculated as follows based on the default schedule:

  // Loop increment (IV = IV + 1) or (IV = IV + ST) if (static,1) scheduling.
  ExprResult Inc =
DefaultScheduleKind == OMPDSK_static_chunkone
? SemaRef.BuildBinOp(CurScope, IncLoc, BO_Add, IV.get(), ST.get())
: SemaRef.BuildBinOp(CurScope, IncLoc, BO_Add, IV.get(),
 SemaRef.ActOnIntegerConstant(IncLoc, 1).get());



> You can't use Sema and OMPClause in Basic, it is not allowed

Ok, I can move getDefaultSchedule() from OpenMPKinds.cpp and make it a static 
function in SemaOpenMP.cpp.  Then, CheckOpenMPLoop() can directly call this 
static function.  Does this sound okay?

Thanks.


https://reviews.llvm.org/D29910



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

Re: [PATCH] Improved plugin/tool support by expanding an existing attribute

[Marcwell Helpdesk via cfe-commits]

The intention of the patch is to extend the functionality of annotations while utilizing existing infrastructure in the AST and IR as much as possible. Annotations are indeed a general purpose solution, sufficient for many use cases, and a complement, not mutual exclusive to pluggable attributes. And don’t forget the realm of AST tools. The compiler should not perform any content checking of annotations but leave that to tool implementations if it is used for other purposes than merely tags. For a more complete support of annotations the AST and IR needs further work but this patch is a step on the way.Having pluggable attributes would be cool but it would face even greater problems than you describe. Unless pluggable attributes involves writing pluggable AST and IR modules the AST and IR must both support some form of generic containers that could handle any type of attribute with any number of parameters. In either case, it would most likely involve substantial architectural changes to many parts.I have revised the patch by adding a second, optional parameter that makes it possible to group annotations, reducing the risk for collisions and since it is optional it won’t break any existing code. A non-zero group string is prepended to the value string when forwarded to IR. The C++11 attribute has been moved to the clang namespace as requested and unit tests and documentation are updated to reflect the changes.Cheers,ChrisModified  include/clang/Basic/Attr.td  include/clang/Basic/AttrDocs.td  lib/CodeGen/CodeGenFunction.cpp  lib/CodeGen/CodeGenModule.cpp  lib/CodeGen/CodeGenModule.h  lib/Sema/SemaDeclAttr.cpp  lib/Sema/SemaStmtAttr.cpp  test/Parser/access-spec-attrs.cpp  test/Sema/annotate.cAdded  test/CodeGenCXX/annotations-field.cpp  test/CodeGenCXX/annotations-global.cpp  test/CodeGenCXX/annotations-loc.cpp  test/CodeGenCXX/annotations-var.cpp  test/SemaCXX/attr-annotate.cpp

attr-annotate-rev2.patch
Description: Binary data
On 9 feb 2017, at 15:07, Aaron Ballman  wrote:On Sat, Feb 4, 2017 at 8:26 AM, Marcwell Helpdesk via cfe-commits wrote:Many plugins/tools could benefit from having a generic way for communicating control directives directly from the source code to itself (via the AST) when acting, for example, as source code transformers, generators, collectors and the like. Attributes are a suitable way of doing this but most available attributes have predefined functionality and modifying the compiler for every plugin/tool is obviously not an option. There is however one undocumented but existing attribute that could be used for a generic solution if it was slightly modified and expanded - the annotate attribute.The current functionality of the annotate attribute encompass annotating declarations with arbitrary strings using the GNU spelling. The attached patch expands on this functionality by adding annotation of statements, C++11 spelling and documentation. With the patch applied most of the code could be annotated for the use by any Clang plugin or tool. For a more detailed description of the updated attribute, see patch for "AttrDocs.td".I definitely agree with the premise for this work -- having a genericway for Clang to pass attributed information down to LLVM IR isdesirable. However, I'm not convinced that the annotate attribute isthe correct approach over something like pluggable attributes. Myprimary concerns stem from the fact that the annotate attribute has nosafe guards for feature collisions (you have to pick a unique string,or else you collide with someone else's string, but there's nothingthat forces you to do this), has no mechanisms for accepting argumentsin a consistent fashion, and provides no way to check the semantics ofthe attribute. It's basically a minimum viable option for loweringattributed information down to LLVM IR, which is fine for things thataren't in the user's face, but isn't a good general-purpose solution.I do not have a problem with adding a C++11 spelling to the annotateattribute for the cases we already support, and I definitely think weshould document the attribute. But I don't think it makes sense to addit as a statement attribute (and the current patch also leaves outtype attributes).Some quick thoughts on the patch:Index: include/clang/Basic/Attr.td===--- include/clang/Basic/Attr.td (revision 293612)+++ include/clang/Basic/Attr.td (working copy)@@ -462,9 +462,10 @@}def Annotate : InheritableParamAttr {-  let Spellings = [GNU<"annotate">];+  let Spellings = [GNU<"annotate">,+   CXX11<"", "annotate", 201612>];This should be in the clang namespace and should not be given a thirdargument (it's not a standards-based attribute).  let Args = [StringArgument<"Annotation">];-  let Documentation = [Undocumented];+  let Documentation = [AnnotateDocs];}def ARMInterrupt : InheritableAttr, TargetSpecificAttr {Index: lib/Sema/SemaStmtAttr.cpp

[PATCH] D29910: [OpenMP] Specialize default schedule on a worksharing loop on the NVPTX device.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In https://reviews.llvm.org/D29910#676186, @arpith-jacob wrote:

> Hi Alexey,
>
> Thank you for your review.  The main difference in the specialized codegen 
> (if vs. else part in CGStmtOpenMP.cpp).
>
> If-part: emitForStaticInit uses the Chunk parameter (else has it set to null).
>  If-part: does not use EmitIgnoredExpr()
>
> I can combine if- and else- part with appropriate guards over the above two 
> statements if you like.


Ok, please try to do it.

> 
> 
>> General comment: do you really need to define default schedule kind in Sema? 
>> Could you do it during codegen phase?
> 
> Yes, we need to define default schedule in Sema because it has to set 
> getCond() and getInc() in CheckOpenMPLoop(), which is in Sema.  For example, 
> the Inc expression is calculated as follows based on the default schedule:
> 
>   // Loop increment (IV = IV + 1) or (IV = IV + ST) if (static,1) scheduling.
>   ExprResult Inc =
> DefaultScheduleKind == OMPDSK_static_chunkone
> ? SemaRef.BuildBinOp(CurScope, IncLoc, BO_Add, IV.get(), ST.get())
> : SemaRef.BuildBinOp(CurScope, IncLoc, BO_Add, IV.get(),
>  SemaRef.ActOnIntegerConstant(IncLoc, 1).get());

The maybe it is better to define a variable with loop increment and define this 
variable during codegen with 1 or stride (for GPU)? I don't like the idea of 
adding some kind of default scheduling, that is not defined in standard in 
Sema, preferer to define it during codegen and somewhere in NVPTX specific 
runtime support class.

>> You can't use Sema and OMPClause in Basic, it is not allowed
> 
> Ok, I can move getDefaultSchedule() from OpenMPKinds.cpp and make it a static 
> function in SemaOpenMP.cpp.  Then, CheckOpenMPLoop() can directly call this 
> static function.  Does this sound okay?
> 
> Thanks.




https://reviews.llvm.org/D29910



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren added a comment.

If something is broken now we don't break it even more.
__float128 remain be fixed to be compatible, not only some poor developer would 
have to fix the missing headers bug one day, he will have to re-fix limits.h 
the right way and undo this "fix".
There is a problems with limits.h, fix limits.h.  Don't make all headers that 
happens to be in the same directory as limits.h disappear.


https://reviews.llvm.org/D29464



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Mateusz Mikuła via Phabricator via cfe-commits]

mati865 added a comment.

Final decision is left for @rnk and @asl but I'm still against using GCC 
"internal" headers in the way they aren't supposed to be used. And have MinGW 
Clang behaving like Linux Clang.

> developer would have to fix the missing headers bug one day

Solving general Clang issue with win32 only fix doesn't seem right.

> he will have to re-fix limits.h the right way and undo this "fix".

It will be better to have proper fix than this hackish workaround in limits.h

> There is a problems with limits.h, fix limits.h. Don't make all headers that 
> happens to be in the same directory as limits.h disappear.

There are more issues hidden unless someone tries to compile specific code 
(ia32intrin.h for an example, I no longer have example however).


https://reviews.llvm.org/D29464



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

[PATCH] D29755: Cache FileID when translating diagnostics in PCH files

[Erik Verbruggen via Phabricator via cfe-commits]

erikjv updated this revision to Diff 88366.

https://reviews.llvm.org/D29755

Files:
  lib/Frontend/ASTUnit.cpp


Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -2539,14 +2539,19 @@
 
   SmallVector Result;
   Result.reserve(Diags.size());
+  const FileEntry *PreviousFE = nullptr;
+  FileID FID;
   for (const StandaloneDiagnostic &SD : Diags) {
 // Rebuild the StoredDiagnostic.
 if (SD.Filename.empty())
   continue;
 const FileEntry *FE = FileMgr.getFile(SD.Filename);
 if (!FE)
   continue;
-FileID FID = SrcMgr.translateFile(FE);
+if (FE != PreviousFE) {
+  FID = SrcMgr.translateFile(FE);
+  PreviousFE = FE;
+}
 SourceLocation FileLoc = SrcMgr.getLocForStartOfFile(FID);
 if (FileLoc.isInvalid())
   continue;


Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -2539,14 +2539,19 @@
 
   SmallVector Result;
   Result.reserve(Diags.size());
+  const FileEntry *PreviousFE = nullptr;
+  FileID FID;
   for (const StandaloneDiagnostic &SD : Diags) {
 // Rebuild the StoredDiagnostic.
 if (SD.Filename.empty())
   continue;
 const FileEntry *FE = FileMgr.getFile(SD.Filename);
 if (!FE)
   continue;
-FileID FID = SrcMgr.translateFile(FE);
+if (FE != PreviousFE) {
+  FID = SrcMgr.translateFile(FE);
+  PreviousFE = FE;
+}
 SourceLocation FileLoc = SrcMgr.getLocForStartOfFile(FID);
 if (FileLoc.isInvalid())
   continue;
___
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

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Basic/FileManager.cpp:167-168
 DirNameStr = DirName.str() + '.';
+llvm::sys::path::native(DirNameStr);
+DirName = DirNameStr;
+  } else {

I'd add a canonicalizePath function that:
-> on unix just returns the path
-> on windows, does the ::native dance and returns that path

Then we can call that when we access the cache or write to the cache instead of 
sprinkling #ifdefs around.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1716-1717
 Loc = Loc.getSpellingLoc();
-if (SM.getFilename(Loc).endswith("sys/queue.h")) {
+   StringRef FN = SM.getFilename(Loc);
+if (FN.endswith("sys/queue.h") || FN.endswith("sys\\queue.h")) {
   BR.markInvalid(getTag(), nullptr);

I'd do changes like this in a separate patch.



Comment at: test/CodeGen/debug-prefix-map.c:21
 // CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: "/var/empty{{[/\\]}}{{.*}}"
-// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"/var/empty{{[/\\]}}Inputs/stdio.h"
+// CHECK-NO-MAIN-FILE-NAME: !DIFile(filename: 
"/var/empty{{/|\\5C}}Inputs{{/|\\5C}}stdio.h"
 // CHECK-NO-MAIN-FILE-NAME-NOT: !DIFile(filename:

That is super unfortunate.
If we want tests to be able to run on windows, I think we might just want to 
give clang a flag to always show paths in unix format. That also should be 
something we could just make the file manager do (I think whatever the 
filemanager stores will be what is visible to the user).


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] D29755: Cache FileID when translating diagnostics in PCH files

[Benjamin Kramer via Phabricator via cfe-commits]

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

lg


https://reviews.llvm.org/D29755



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

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: clangd/ASTManager.cpp:69
+  // our one-element queue is empty.
+  if (!RequestIsPending && !Done)
+ClangRequestCV.wait(Lock);

arphaman wrote:
> This is just a suggestion based on personal opinion: `RequestIsPending` and 
> `Done` can be merged into one enum value that uses an enum like this:
> 
> ```
> enum ASTManagerState {
>   Waiting, ReceivedRequest, Done
> }
> ```
> 
> Then this condition could be rewritten as `if (state == Waiting)`.
I'm not so sure. Once we have a queue (iff we ever get a queue), the "request 
available" is an attribute of the queue, while the "we need to exit" is an 
attribute of the system.



Comment at: clangd/ASTManager.cpp:70
+  if (!RequestIsPending && !Done)
+ClangRequestCV.wait(Lock);
+

arphaman wrote:
>  I believe `std::condition_variable` may also be unblocked spuriously when 
> `wait` is called without a predicate, which would lead to an empty `File` 
> down below.
Yep, +1, the general pattern is to put while loops around.
It might actually be simpler to write that with a queue from the start (note 
that this would make the request queueing side somewhat less simple):
  ... Lock(RequestLock);
  while (Requests.empty() && !Done) {
ClangRequestCV.wait(Lock);
  }
  if (Done) return;
  File = Request.pop_front();
  ClangRequestCV.notify_all();
  ...

  the while loop  can be written somewhat simpler with the pred version:
  ClangRequestCV.wait(Lock, [&] { return !Request.empty() || Done; });




Comment at: clangd/ASTManager.cpp:116
+  Diagnostics.pop_back(); // Drop trailing comma.
+this->Output.writeMessage(
+
R"({"jsonrpc":"2.0","method":"textDocument/publishDiagnostics","params":{"uri":")"
 +

One question is whether we want to couple this to JSON, or have structured 
output that we serialize.
(I'm happy with doing this now, and refining later - doesn't seem super hard to 
switch)



Comment at: clangd/ASTManager.h:34
+
+  void onDocumentAdd(StringRef Uri, const DocumentStore &Docs) override;
+  // FIXME: Implement onDocumentRemove

At a first glance, it's weird that we have a global document store, but get a 
document store for each notification. This at least needs documentation.



Comment at: clangd/ASTManager.h:67
+  /// Setting Done to true will make the worker thread terminate.
+  std::atomic Done;
+};

arphaman wrote:
> It looks like `Done` is always accessed in a scope where `RequestLock` is 
> locked, so `atomic` doesn't seem needed here.
Yea, after only having read this header, it looks like we might want to pull 
out a Request as an abstraction.



Comment at: clangd/ClangDMain.cpp:32
+  auto *AST = new ASTManager(Out, Store);
+  Store.addListener(std::unique_ptr(AST));
   JSONRPCDispatcher Dispatcher(llvm::make_unique(Out));

krasimir wrote:
> Why not directly `Store.addListener(llvm::make_unique(Out, 
> Store));`?
Why's the ASTManager not on the stack?



Comment at: clangd/DocumentStore.h:39
+for (const auto &Listener : Listeners)
+  Listener->onDocumentAdd(Uri, *this);
+  }

krasimir wrote:
> Is only the main thread supposed to addDocument-s? Otherwise the listener 
> notifications might need to be guarded.
It would always be the responsibility of the callee to guard.



Comment at: clangd/DocumentStore.h:42
   /// Delete a document from the store.
-  void removeDocument(StringRef Uri) { Docs.erase(Uri); }
+  void removeDocument(StringRef Uri) { Docs.erase(Uri);
+for (const auto &Listener : Listeners)

krasimir wrote:
> cierpuchaw wrote:
> > cierpuchaw wrote:
> > > Shouldn't this be called under a lock_guard?
> > I should've reworded my comment before submitting it. By 'this' I'm 
> > referring to the 'Docs.erase()' part, not the whole function.
> > 
> Even under a guard, it may potentially still be unsafe since workers may 
> operate on erased/modified StringRef-s.
That depends on the guarantees made, and when we create copies :)


https://reviews.llvm.org/D29886



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

[PATCH] D29858: [clang-tidy] Catch trivially true statements like a != 1 || a != 3

[Etienne Bergeron via Phabricator via cfe-commits]

etienneb added inline comments.



Comment at: clang-tidy/misc/RedundantExpressionCheck.cpp:244
+  // x != 5 || x != 10
+  if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE)
+return true;

etienneb wrote:
> The good news is that this code will be catch by this check!
> ```
>   if (OpcodeLHS == BO_NE || OpcodeLHS == BO_NE)   <<-- redundant expression
> ```
> Should be:
> ```
>   if (OpcodeLHS == BO_NE || OpcodeRHS == BO_NE) 
> ```
> 
> 
> 
btw, it should be:

```
  if (OpcodeLHS == BO_NE && OpcodeRHS == BO_NE)
```


https://reviews.llvm.org/D29858



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

[PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

[Michael via Phabricator via cfe-commits]

redm123 created this revision.

Printing typedefs or type aliases using clang_getTypeSpelling() is missing the 
namespace they are defined in. This is in contrast to other types that always 
yield the full typename including namespaces.


https://reviews.llvm.org/D29944

Files:
  lib/AST/TypePrinter.cpp
  test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/Index/annotate-nested-name-specifier.cpp
  test/Index/file-refs.cpp
  test/Index/print-type.cpp
  test/Index/recursive-cxx-member-calls.cpp
  test/Layout/ms-x86-basic-layout.cpp
  test/Misc/diag-template-diffing.cpp
  test/SemaCXX/attr-noreturn.cpp
  test/SemaCXX/calling-conv-compat.cpp
  test/SemaCXX/coroutines.cpp
  test/SemaCXX/cxx0x-initializer-aggregates.cpp
  test/SemaCXX/cxx1y-contextual-conversion-tweaks.cpp
  test/SemaCXX/enum-scoped.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/pseudo-destructors.cpp
  test/SemaObjCXX/arc-templates.mm
  test/SemaTemplate/member-access-ambig.cpp
  test/SemaTemplate/typename-specifier.cpp

Index: test/SemaTemplate/typename-specifier.cpp
===
--- test/SemaTemplate/typename-specifier.cpp
+++ test/SemaTemplate/typename-specifier.cpp
@@ -102,7 +102,7 @@
 
 template struct E {
   typedef typename T::foo foo;
-  typedef typename foo::bar bar;  // expected-error {{type 'foo' (aka 'double') cannot be used prior to '::' because it has no members}}
+  typedef typename foo::bar bar;  // expected-error {{type 'E::foo' (aka 'double') cannot be used prior to '::' because it has no members}}
 };
 
 struct F {
Index: test/SemaTemplate/member-access-ambig.cpp
===
--- test/SemaTemplate/member-access-ambig.cpp
+++ test/SemaTemplate/member-access-ambig.cpp
@@ -48,7 +48,7 @@
   typedef int (A::*P);
   template struct S : T {
 void f() {
-  P(&T::X) // expected-error {{cannot cast from type 'int *' to member pointer type 'P'}}
+  P(&T::X) // expected-error {{cannot cast from type 'int *' to member pointer type 'AddrOfMember::P'}}
   == &A::X;
 }
   };
Index: test/SemaObjCXX/arc-templates.mm
===
--- test/SemaObjCXX/arc-templates.mm
+++ test/SemaObjCXX/arc-templates.mm
@@ -102,8 +102,8 @@
 template
 struct make_weak_fail {
   typedef T T_type;
-  typedef __weak T_type type; // expected-error{{the type 'T_type' (aka '__weak id') is already explicitly ownership-qualified}} \
-  // expected-error{{the type 'T_type' (aka '__strong id') is already explicitly ownership-qualified}}
+  typedef __weak T_type type; // expected-error{{the type 'make_weak_fail<__weak id>::T_type' (aka '__weak id') is already explicitly ownership-qualified}} \
+  // expected-error{{the type 'make_weak_fail::T_type' (aka '__strong id') is already explicitly ownership-qualified}}
 };
 
 int check_make_weak_fail0[is_same::type, __weak id>::value? 1 : -1]; // expected-note{{in instantiation of template class 'make_weak_fail<__weak id>' requested here}}
Index: test/SemaCXX/pseudo-destructors.cpp
===
--- test/SemaCXX/pseudo-destructors.cpp
+++ test/SemaCXX/pseudo-destructors.cpp
@@ -109,6 +109,6 @@
 
 void test2(Foo d) {
   d.~Foo(); // This is ok
-  d.~Derived(); // expected-error {{member reference type 'Foo' (aka 'dotPointerAccess::Derived *') is a pointer; did you mean to use '->'}}
+  d.~Derived(); // expected-error {{member reference type 'dotPointerAccess::Foo' (aka 'dotPointerAccess::Derived *') is a pointer; did you mean to use '->'}}
 }
 }
Index: test/SemaCXX/nested-name-spec.cpp
===
--- test/SemaCXX/nested-name-spec.cpp
+++ test/SemaCXX/nested-name-spec.cpp
@@ -310,7 +310,7 @@
 }
 
 namespace TypedefNamespace { typedef int F; };
-TypedefNamespace::F::NonexistentName BadNNSWithCXXScopeSpec; // expected-error {{'F' (aka 'int') is not a class, namespace, or enumeration}}
+TypedefNamespace::F::NonexistentName BadNNSWithCXXScopeSpec; // expected-error {{'TypedefNamespace::F' (aka 'int') is not a class, namespace, or enumeration}}
 
 namespace PR18587 {
 
@@ -449,7 +449,7 @@
 class B {
   typedef C D; // expected-error{{unknown type name 'C'}}
   A::D::F;
-  // expected-error@-1{{'D' (aka 'int') is not a class, namespace, or enumeration}}
+  // expected-error@-1{{'PR30619::A::B::D' (aka 'int') is not a class, namespace, or enumeration}}
 };
 }
 }
Index: test/SemaCXX/enum-scoped.cpp
===
--- test/SemaCXX/enum-scoped.cpp
+++ test/SemaCXX/enum-scoped.cpp
@@ -307,5 +307,5 @@
   typedef E E2;
   E2 f1() { return E::a; }
 
-  bool f() { return !f1(); } // expected-error {{invalid argument type 'E2' (aka 'test11::E') to unary expression}}
+  bool f() { return !f1(); } // expected-error {{invalid arg

Re: [PATCH] D29944: libclang: Print namespaces for typedefs and type aliases

[Michael via cfe-commits]

As an explanation for this patch my original mail to cfe-dev:

I'm using libclang to parse header files and generate code from them. I 
found that clang_getTypeSpelling() usually includes the namespace(s) a 
type was declared in. However with the exception being typedefs (and 
same for "using A = B"). Not sure if this is a bug or intended behavior, 
but it seems at least inconsistent. I also couldn't really find a good 
workaround for this. I'd have to manually figure out all typedefs (not 
just pure typedefs, they could also be nested template arguments or 
whatever) and then their originating namespaces. This is a bit 
cumbersome and not really straight forward. Getting the canonical type 
is also not really a solution as I would like to keep the typedefs.


Minimal example:

namespace foo {
class Bar {
};
typedef Bar BarDef;
}

clang_getTypeSpelling on "Bar" (kind "Record") gives: "foo::Bar"
clang_getTypeSpelling on "BarDef" (kind "Typedef") gives: "BarDef" (<== 
missing "foo::")


Thanks

Michael

On 02/14/2017 05:11 PM, Michael via Phabricator wrote:

redm123 created this revision.

Printing typedefs or type aliases using clang_getTypeSpelling() is missing the 
namespace they are defined in. This is in contrast to other types that always 
yield the full typename including namespaces.


https://reviews.llvm.org/D29944

Files:
  lib/AST/TypePrinter.cpp
  test/CXX/basic/basic.lookup/basic.lookup.qual/class.qual/p2.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/Index/annotate-nested-name-specifier.cpp
  test/Index/file-refs.cpp
  test/Index/print-type.cpp
  test/Index/recursive-cxx-member-calls.cpp
  test/Layout/ms-x86-basic-layout.cpp
  test/Misc/diag-template-diffing.cpp
  test/SemaCXX/attr-noreturn.cpp
  test/SemaCXX/calling-conv-compat.cpp
  test/SemaCXX/coroutines.cpp
  test/SemaCXX/cxx0x-initializer-aggregates.cpp
  test/SemaCXX/cxx1y-contextual-conversion-tweaks.cpp
  test/SemaCXX/enum-scoped.cpp
  test/SemaCXX/nested-name-spec.cpp
  test/SemaCXX/pseudo-destructors.cpp
  test/SemaObjCXX/arc-templates.mm
  test/SemaTemplate/member-access-ambig.cpp
  test/SemaTemplate/typename-specifier.cpp



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

Re: r294855 - docs: update docs for objc_storeStrong behaviour

[Hans Wennborg via cfe-commits]

Sure, r295076.

Thanks,
Hans

On Sat, Feb 11, 2017 at 12:29 PM, Saleem Abdulrasool
 wrote:
> Hi Hans,
>
> Would you mind grabbing this for the 4.0 release as well?  It's merely
> correcting the documentation, so should have no impact on the toolchain
> itself.
>
> On Sat, Feb 11, 2017 at 9:24 AM, Saleem Abdulrasool via cfe-commits
>  wrote:
>>
>> Author: compnerd
>> Date: Sat Feb 11 11:24:09 2017
>> New Revision: 294855
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=294855&view=rev
>> Log:
>> docs: update docs for objc_storeStrong behaviour
>>
>> objc_storeStrong does not return a value.
>>
>> Modified:
>> cfe/trunk/docs/AutomaticReferenceCounting.rst
>> cfe/trunk/lib/CodeGen/CodeGenModule.h
>>
>> Modified: cfe/trunk/docs/AutomaticReferenceCounting.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/AutomaticReferenceCounting.rst?rev=294855&r1=294854&r2=294855&view=diff
>>
>> ==
>> --- cfe/trunk/docs/AutomaticReferenceCounting.rst (original)
>> +++ cfe/trunk/docs/AutomaticReferenceCounting.rst Sat Feb 11 11:24:09 2017
>> @@ -2258,16 +2258,13 @@ non-block type [*]_.  Equivalent to the
>>
>>  .. code-block:: objc
>>
>> -  id objc_storeStrong(id *object, id value) {
>> -value = [value retain];
>> +  void objc_storeStrong(id *object, id value) {
>>  id oldValue = *object;
>> +value = [value retain];
>>  *object = value;
>>  [oldValue release];
>> -return value;
>>}
>>
>> -Always returns ``value``.
>> -
>>  .. [*] This does not imply that a ``__strong`` object of block type is an
>> invalid argument to this function. Rather it implies that an
>> ``objc_retain``
>> and not an ``objc_retainBlock`` operation will be emitted if the
>> argument is
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=294855&r1=294854&r2=294855&view=diff
>>
>> ==
>> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Sat Feb 11 11:24:09 2017
>> @@ -166,7 +166,7 @@ struct ObjCEntrypoints {
>>/// void objc_release(id);
>>llvm::Constant *objc_release;
>>
>> -  /// id objc_storeStrong(id*, id);
>> +  /// void objc_storeStrong(id*, id);
>>llvm::Constant *objc_storeStrong;
>>
>>/// id objc_storeWeak(id*, id);
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
>
> --
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29830: [OpenCL][Doc] Relase 4.0 notes for OpenCL

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans added a comment.

Thanks for writing these!

Go ahead and commit to the branch, or let me know if you'd like me to do it.


https://reviews.llvm.org/D29830



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

So far as I can tell, Clang on Linux supports none of the headers under 
discussion here, and that's been OK for some time. None of the headers in 
question appear to actually work today, so I think we should remove them from 
the search path.

Regarding OpenMP, we should do whatever Intel is doing to make OpenMP work in 
Clang, which doesn't rely on omp.h from libgcc. quadmath.h seems like something 
that should be part of compiler-rt if we want to support it. If users want to 
use quadmath functions from libgcc, then I think it's fair for them to have to 
do extra work to pull that header out so that they don't accidentally pull in 
conflicting GCC headers like immintrin.h and limits.h.

@yaron.keren does that seem reasonable?


https://reviews.llvm.org/D29464



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

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

[Will Dietz via Phabricator via cfe-commits]

dtzWill added a comment.

In https://reviews.llvm.org/D29369#673064, @vsk wrote:

> In https://reviews.llvm.org/D29369#672166, @dtzWill wrote:
>
> > After some thought, can we discuss why this is a good idea?
>
>
> The goal is to lower ubsan's compile-time + instrumentation overhead at -O0, 
> since this reduces the friction of debugging a ubsan-instrumented project.


Apologies for the delay, thank you for the explanation.

> 
> 
>> This increases the cyclomatic complexity of code that already is difficult 
>> to reason about, and seems like it's both brittle and out-of-place in 
>> CGExprScalar.
> 
> Are there cleanups or ways to reorganize the code that would make this sort 
> of change less complex / brittle? I'm open to taking that on.

None that I see immediately (heh, otherwise I'd be working on them myself...) 
but the code paths for trapping/non-trapping are particularly what I meant 
re:complexity, and while I suppose the AST or its interface is probably 
unlikely to change much (?) I'm concerned about these checks silently removing 
checks they shouldn't in the future.

(Who would notice if this happened?)

> 
> 
>> It really seems it would be better to let InstCombine or some other 
>> analysis/transform deal with proving checks redundant instead of attempting 
>> to do so on-the-fly during CodeGen.
> 
> -O1/-O2 do get rid of a lot of checks, but they also degrade the debugging 
> experience, so it's not really a solution for this use case.

Understood, that makes sense.

Would running InstCombine (and only InstCombine):

1. Fail to remove any checks elided by this change?
2. Have a negative impact on debugging experience? For this I'm mostly asking 
for a guess, I don't know how to exactly quantify this easily.

(3) Have an undesirable impact on compilation time or other negative 
consequence?)

> 
> 
>> Can you better motivate why this is worth these costs, or explain your use 
>> case a bit more?
> 
> I have some numbers from LNT. I did a pre-patch and post-patch run at -O0 + 
> -fsanitize=signed-integer-overflow,unsigned-integer-overflow. There were 
> 4,672 object files produced in each run. This patch brings the average object 
> size down from 36,472.0 to 36,378.3 bytes (a 0.26% improvement), and the 
> average number of overflow checks per object down from 66.8 to 66.2 (a 0.81% 
> improvement).

Wonderful, thank you for producing and sharing these numbers.  Those 
improvements don't convince me, but if you're saying this is important to you 
and your use-case/users I'm happy to go with that.

> I don't have reliable compile-time numbers, but not emitting IR really seems 
> like a straightforward improvement over emitting/analyzing/removing it.

Hard to say.  Separation of concerns is important too, but of course there's 
trade-offs everywhere :).  I'd suspect this doesn't change compile-time much 
either way.

> So, those are the benefits. IMO getting close to 1% better at reducing 
> instrumentation overhead is worth the complexity cost here.

Couldn't say, but that sounds reasonable to me and I don't mean to stand in the 
way of progress!

Can you answer my questions about InstCombine above? Thanks!


https://reviews.llvm.org/D29369



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

r295083 - Remove unnecessary std::string construction

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Feb 14 12:38:40 2017
New Revision: 295083

URL: http://llvm.org/viewvc/llvm-project?rev=295083&view=rev
Log:
Remove unnecessary std::string construction

Modified:
cfe/trunk/lib/Lex/PPDirectives.cpp

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=295083&r1=295082&r2=295083&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Tue Feb 14 12:38:40 2017
@@ -1983,14 +1983,13 @@ void Preprocessor::HandleIncludeDirectiv
   Path.size() <= Filename.size() ? Filename[Path.size()-1] :
 (isAngled ? '>' : '"'));
   }
-  auto Replacement = Path.str().str();
   // For user files and known standard headers, by default we issue a 
diagnostic.
   // For other system headers, we don't. They can be controlled separately.
   auto DiagId = (FileCharacter == SrcMgr::C_User || 
warnByDefaultOnWrongCase(Name)) ?
   diag::pp_nonportable_path : diag::pp_nonportable_system_path;
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, DiagId) << Replacement <<
-FixItHint::CreateReplacement(Range, Replacement);
+  Diag(FilenameTok, DiagId) << Path <<
+FixItHint::CreateReplacement(Range, Path);
 }
   }
 


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

r295082 - Fix some warnings in intrin.h

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Feb 14 12:38:19 2017
New Revision: 295082

URL: http://llvm.org/viewvc/llvm-project?rev=295082&view=rev
Log:
Fix some warnings in intrin.h

Modified:
cfe/trunk/lib/Headers/intrin.h

Modified: cfe/trunk/lib/Headers/intrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/intrin.h?rev=295082&r1=295081&r2=295082&view=diff
==
--- cfe/trunk/lib/Headers/intrin.h (original)
+++ cfe/trunk/lib/Headers/intrin.h Tue Feb 14 12:38:19 2017
@@ -897,19 +897,21 @@ __readfsqword(unsigned long __offset) {
 #ifdef __x86_64__
 static __inline__ unsigned char __DEFAULT_FN_ATTRS
 __readgsbyte(unsigned long __offset) {
-  return *__ptr_to_addr_space(256, unsigned char, __offset);
+  return *__ptr_to_addr_space(256, unsigned char, (unsigned long 
long)__offset);
 }
 static __inline__ unsigned short __DEFAULT_FN_ATTRS
 __readgsword(unsigned long __offset) {
-  return *__ptr_to_addr_space(256, unsigned short, __offset);
+  return *__ptr_to_addr_space(256, unsigned short,
+  (unsigned long long)__offset);
 }
 static __inline__ unsigned long __DEFAULT_FN_ATTRS
 __readgsdword(unsigned long __offset) {
-  return *__ptr_to_addr_space(256, unsigned long, __offset);
+  return *__ptr_to_addr_space(256, unsigned long, (unsigned long 
long)__offset);
 }
 static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS
 __readgsqword(unsigned long __offset) {
-  return *__ptr_to_addr_space(256, unsigned __int64, __offset);
+  return *__ptr_to_addr_space(256, unsigned __int64,
+  (unsigned long long)__offset);
 }
 #endif
 #undef __ptr_to_addr_space


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

[PATCH] D29692: [clang-tidy] add check modernize-use-const-instead-of-define

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin added a comment.

Thanks for the feedback. As I'm new to this I cannot say whether checking only 
the file in question fits better with clang-tidy’s policy or not.
Also, I’m not sure about ODR. Of course, it’s a possibility, but isn’t the 
programmer responsible for that? This will be more of an issue as soon as this 
check provides a Fix-It solution.

As for the other part, I've checked some guidelines (without any order or 
selection process)
MISRA C++: Don’t use `#define`, use `const` variables; Also don’t do math on 
enums
CppCoreGuidelines: Don’t use `#define`, use `constexpr` variables 
SEI CERT C++: No mention of `#define` as far as I can tell
JSF AV C++: Don’t use `#define`, use `const` variables
HIC++: Don’t use `#define`, use `const` objects (reference to JSF AV C++ and 
MISRA C++)

So basically they're all the same. The only question is `const` vs `constexpr`


https://reviews.llvm.org/D29692



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

r295085 - [profiling] Update test case to deal with name variable change (NFC)

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Tue Feb 14 12:49:03 2017
New Revision: 295085

URL: http://llvm.org/viewvc/llvm-project?rev=295085&view=rev
Log:
[profiling] Update test case to deal with name variable change (NFC)

The 'profn' name variables shouldn't show up after we run the instrprof
pass, see https://reviews.llvm.org/D29921 for more details.

Modified:
cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp

Modified: cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp?rev=295085&r1=295084&r2=295085&view=diff
==
--- cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp (original)
+++ cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp Tue Feb 14 12:49:03 
2017
@@ -13,15 +13,6 @@ struct B : A {
   virtual ~B();
 };
 
-// Base dtor
-// CHECK: @__profn__ZN1BD2Ev = private constant [9 x i8] c"_ZN1BD2Ev"
-
-// Complete dtor must not be instrumented
-// CHECK-NOT: @__profn__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
-
-// Deleting dtor must not be instrumented
-// CHECK-NOT: @__profn__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
-
 // Base dtor counters and profile data
 // CHECK: @__profc__ZN1BD2Ev = private global [1 x i64] zeroinitializer
 // CHECK: @__profd__ZN1BD2Ev =


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

[PATCH] D29951: Load lazily the template specialization in multi-module setups.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.

Currently, we load all template specialization if we have more than one module 
attached and we touch anything around the template definition.

This patch registers the template specializations as a lazily-loadable 
entities. This reduces the amount of deserializations by 1%.


Repository:
  rL LLVM

https://reviews.llvm.org/D29951

Files:
  lib/Serialization/ASTReaderDecl.cpp


Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -3788,9 +3788,31 @@
   break;
 }
 
-case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
-  // It will be added to the template's specializations set when loaded.
-  (void)Record.readDecl();
+case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+  // It will be added to the template's lazy specialization set when 
loaded.
+  // FIXME: reduce the copy paste.
+  SmallVector SpecID;
+  if (auto *CTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = CTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+  } else if (auto *FTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = FTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+  } else if (auto *VTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = VTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+  } else // TypeAliasTemplateDecl
+assert(0 && "TypeAliasTemplateDecl doesn't have specs!");
+}
   break;
 
 case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {


Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -3788,9 +3788,31 @@
   break;
 }
 
-case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION:
-  // It will be added to the template's specializations set when loaded.
-  (void)Record.readDecl();
+case UPD_CXX_ADDED_TEMPLATE_SPECIALIZATION: {
+  // It will be added to the template's lazy specialization set when loaded.
+  // FIXME: reduce the copy paste.
+  SmallVector SpecID;
+  if (auto *CTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = CTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+  } else if (auto *FTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = FTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+  } else if (auto *VTD = dyn_cast(D)) {
+SpecID.push_back(ReadDeclID());
+auto *CommonPtr = VTD->getCommonPtr();
+CommonPtr->LazySpecializations = newDeclIDList(
+Reader.getContext(), CommonPtr->LazySpecializations, SpecID);
+
+  } else // TypeAliasTemplateDecl
+assert(0 && "TypeAliasTemplateDecl doesn't have specs!");
+}
   break;
 
 case UPD_CXX_ADDED_ANONYMOUS_NAMESPACE: {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29908: Disallow returning a __block variable via a move

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Posting John's review comment, which was sent to the review I abandoned:

"Hmm.  The behavior I think we actually want here is that we should move out of 
the __block variable but not perform NRVO, essentially like we would if the 
variable were a parameter.

The block byref copy helper should always be doing a move if it can, regardless 
of whether the function is returned.  That's independent of this, though."


https://reviews.llvm.org/D29908



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

Re: [libcxx] r294696 - Fully qualify (preprend ::) calls to math functions from libc

[Hans Wennborg via cfe-commits]

I'm guessing this isn't addressing a new issue in 4.0, so perhaps we
should take the safe alternative and not merge this one.

Thanks,
Hans

On Thu, Feb 9, 2017 at 11:35 PM, Eric Fiselier  wrote:
> I'm a bit paranoid one of the names we just qualified will end up being
> defined as a macro. Conceivably  could have tolerated macros for
> names it uses but doesn't overload (ex, modff), however  would have
> always blown up.
>
> /Eric
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Yaron Keren via Phabricator via cfe-commits]

yaron.keren added a comment.

OK.


https://reviews.llvm.org/D29464



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

Re: r294800 - Don't let EvaluationModes dictate whether an invalid base is OK

[Hans Wennborg via cfe-commits]

Merged in r295087.

On Fri, Feb 10, 2017 at 5:00 PM, Hans Wennborg  wrote:
> Sgtm. Go ahead and merge when the bots have chewed on it for a bit,
> otherwise I'll do it next week.
>
> Thanks,
> Hans
>
> On Fri, Feb 10, 2017 at 3:06 PM, George Burgess IV
>  wrote:
>> Hi Hans!
>>
>> This fixes PR31843, which is a release blocker. Once the bots seem happy
>> with it, can we merge this into the 4.0 branch, please?
>>
>> (Richard okayed this when he LGTM'ed the patch)
>>
>> Thanks,
>> George
>>
>> On Fri, Feb 10, 2017 at 2:52 PM, George Burgess IV via cfe-commits
>>  wrote:
>>>
>>> Author: gbiv
>>> Date: Fri Feb 10 16:52:29 2017
>>> New Revision: 294800
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=294800&view=rev
>>> Log:
>>> Don't let EvaluationModes dictate whether an invalid base is OK
>>>
>>> What we want to actually control this behavior is something more local
>>> than an EvalutationMode. Please see the linked revision for more
>>> discussion on why/etc.
>>>
>>> This fixes PR31843.
>>>
>>> Differential Revision: https://reviews.llvm.org/D29469
>>>
>>> Modified:
>>> cfe/trunk/lib/AST/ExprConstant.cpp
>>> cfe/trunk/test/CodeGen/object-size.c
>>> cfe/trunk/test/Sema/builtin-object-size.c
>>>
>>> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=294800&r1=294799&r2=294800&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>>> +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Feb 10 16:52:29 2017
>>> @@ -616,10 +616,12 @@ namespace {
>>>/// gets a chance to look at it.
>>>EM_PotentialConstantExpressionUnevaluated,
>>>
>>> -  /// Evaluate as a constant expression. Continue evaluating if
>>> either:
>>> -  /// - We find a MemberExpr with a base that can't be evaluated.
>>> -  /// - We find a variable initialized with a call to a function that
>>> has
>>> -  ///   the alloc_size attribute on it.
>>> +  /// Evaluate as a constant expression. In certain scenarios, if:
>>> +  /// - we find a MemberExpr with a base that can't be evaluated, or
>>> +  /// - we find a variable initialized with a call to a function that
>>> has
>>> +  ///   the alloc_size attribute on it
>>> +  /// then we may consider evaluation to have succeeded.
>>> +  ///
>>>/// In either case, the LValue returned shall have an invalid base;
>>> in the
>>>/// former, the base will be the invalid MemberExpr, in the latter,
>>> the
>>>/// base will be either the alloc_size CallExpr or a CastExpr
>>> wrapping
>>> @@ -902,10 +904,6 @@ namespace {
>>>return KeepGoing;
>>>  }
>>>
>>> -bool allowInvalidBaseExpr() const {
>>> -  return EvalMode == EM_OffsetFold;
>>> -}
>>> -
>>>  class ArrayInitLoopIndex {
>>>EvalInfo &Info;
>>>uint64_t OuterIndex;
>>> @@ -1416,8 +1414,10 @@ static bool Evaluate(APValue &Result, Ev
>>>  static bool EvaluateInPlace(APValue &Result, EvalInfo &Info,
>>>  const LValue &This, const Expr *E,
>>>  bool AllowNonLiteralTypes = false);
>>> -static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo
>>> &Info);
>>> -static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>>> &Info);
>>> +static bool EvaluateLValue(const Expr *E, LValue &Result, EvalInfo &Info,
>>> +   bool InvalidBaseOK = false);
>>> +static bool EvaluatePointer(const Expr *E, LValue &Result, EvalInfo
>>> &Info,
>>> +bool InvalidBaseOK = false);
>>>  static bool EvaluateMemberPointer(const Expr *E, MemberPtr &Result,
>>>EvalInfo &Info);
>>>  static bool EvaluateTemporary(const Expr *E, LValue &Result, EvalInfo
>>> &Info);
>>> @@ -4835,6 +4835,7 @@ class LValueExprEvaluatorBase
>>>: public ExprEvaluatorBase {
>>>  protected:
>>>LValue &Result;
>>> +  bool InvalidBaseOK;
>>>typedef LValueExprEvaluatorBase LValueExprEvaluatorBaseTy;
>>>typedef ExprEvaluatorBase ExprEvaluatorBaseTy;
>>>
>>> @@ -4843,9 +4844,14 @@ protected:
>>>  return true;
>>>}
>>>
>>> +  bool evaluatePointer(const Expr *E, LValue &Result) {
>>> +return EvaluatePointer(E, Result, this->Info, InvalidBaseOK);
>>> +  }
>>> +
>>>  public:
>>> -  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result) :
>>> -ExprEvaluatorBaseTy(Info), Result(Result) {}
>>> +  LValueExprEvaluatorBase(EvalInfo &Info, LValue &Result, bool
>>> InvalidBaseOK)
>>> +  : ExprEvaluatorBaseTy(Info), Result(Result),
>>> +InvalidBaseOK(InvalidBaseOK) {}
>>>
>>>bool Success(const APValue &V, const Expr *E) {
>>>  Result.setFrom(this->Info.Ctx, V);
>>> @@ -4857,7 +4863,7 @@ public:
>>>  QualType BaseTy;
>>>  bool EvalOK;
>>>  if (E->isArrow()) {
>>> -  EvalOK = EvaluatePointer(E->getBase(), Result, 

[PATCH] D29908: Disallow returning a __block variable via a move

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D29908#676589, @ahatanak wrote:

> Posting John's review comment, which was sent to the review I abandoned:
>
> "Hmm.  The behavior I think we actually want here is that we should move out 
> of the __block variable but not perform NRVO, essentially like we would if 
> the variable were a parameter.


I think that's what's happening. Because of the changes made in r274291, NRVO 
isn't performed, but the move constructor instead of the copy constructor is 
called to return the shared_ptr. The shared_ptr in the heap is moved to 
returned shared_ptr (%agg.result), so when the block function passed to 
dispatch_async is executed, it segfaults because the object "ret" used to point 
to has been destructed.

If that's the case, should we conclude that clang is behaving as expected?

> The block byref copy helper should always be doing a move if it can, 
> regardless of whether the function is returned.  That's independent of this, 
> though."


https://reviews.llvm.org/D29908



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

[PATCH] D29879: [OpenMP] Teams reduction on the NVPTX device.

[Arpith Jacob via Phabricator via cfe-commits]

arpith-jacob updated this revision to Diff 88407.
arpith-jacob added a comment.

Use SizeTy instead of assuming 64 bits!


https://reviews.llvm.org/D29879

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  test/OpenMP/nvptx_teams_reduction_codegen.cpp

Index: test/OpenMP/nvptx_teams_reduction_codegen.cpp
===
--- /dev/null
+++ test/OpenMP/nvptx_teams_reduction_codegen.cpp
@@ -0,0 +1,1143 @@
+// Test target codegen - host bc file has to be created first.
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32
+// RUN: %clang_cc1 -verify -fopenmp -fexceptions -fcxx-exceptions -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+// Check for the data transfer medium in shared memory to transfer the reduction list to the first warp.
+// CHECK-DAG: [[TRANSFER_STORAGE:@.+]] = common addrspace([[SHARED_ADDRSPACE:[0-9]+]]) global [32 x i64]
+
+// Check that the execution mode of all 3 target regions is set to Generic Mode.
+// CHECK-DAG: {{@__omp_offloading_.+l27}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l33}}_exec_mode = weak constant i8 1
+// CHECK-DAG: {{@__omp_offloading_.+l40}}_exec_mode = weak constant i8 1
+
+template
+tx ftemplate(int n) {
+  int a;
+  short b;
+  tx c;
+  float d;
+  double e;
+
+  #pragma omp target
+  #pragma omp teams reduction(+: e)
+  {
+e += 5;
+  }
+
+  #pragma omp target
+  #pragma omp teams reduction(^: c) reduction(*: d)
+  {
+c ^= 2;
+d *= 33;
+  }
+
+  #pragma omp target
+  #pragma omp teams reduction(|: a) reduction(max: b)
+  {
+a |= 1;
+b = 99 > b ? 99 : b;
+  }
+
+  return a+b+c+d+e;
+}
+
+int bar(int n){
+  int a = 0;
+
+  a += ftemplate(n);
+
+  return a;
+}
+
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l27}}_worker()
+
+  // CHECK: define {{.*}}void [[T1:@__omp_offloading_.+template.+l27]](
+  //
+  // CHECK: {{call|invoke}} void [[T1]]_worker()
+  //
+  // CHECK: call void @__kmpc_kernel_init(
+  //
+  // CHECK: store double {{[0\.e\+]+}}, double* [[E:%.+]], align
+  // CHECK: [[EV:%.+]] = load double, double* [[E]], align
+  // CHECK: [[ADD:%.+]] = fadd double [[EV]], 5
+  // CHECK: store double [[ADD]], double* [[E]], align
+  // CHECK: [[PTR1:%.+]] = getelementptr inbounds [[RLT:.+]], [1 x i8*]* [[RL:%.+]], i[[SZ:32|64]] 0, i{{32|64}} 0
+  // CHECK: [[E_CAST:%.+]] = bitcast double* [[E]] to i8*
+  // CHECK: store i8* [[E_CAST]], i8** [[PTR1]], align
+  // CHECK: [[ARG_RL:%.+]] = bitcast [[RLT]]* [[RL]] to i8*
+  // CHECK: [[RET:%.+]] = call i32 @__kmpc_nvptx_teams_reduce_nowait(i32 {{.+}}, i32 1, i[[SZ]] {{4|8}}, i8* [[ARG_RL]], void (i8*, i16, i16, i16)* [[SHUFFLE_REDUCE_FN:@.+]], void (i8*, i32)* [[WARP_COPY_FN:@.+]], void (i8*, i8*, i32, i32)* [[SCRATCH_COPY_FN:@.+]], void (i8*, i8*, i32, i32, i32)* [[LOAD_REDUCE_FN:@.+]])
+  // CHECK: [[COND:%.+]] = icmp eq i32 [[RET]], 1
+  // CHECK: br i1 [[COND]], label {{%?}}[[IFLABEL:.+]], label {{%?}}[[EXIT:.+]]
+  //
+  // CHECK: [[IFLABEL]]
+  // CHECK: [[E_INV:%.+]] = load double, double* [[E_IN:%.+]], align
+  // CHECK: [[EV:%.+]] = load double, double* [[E]], align
+  // CHECK: [[ADD:%.+]] = fadd double [[E_INV]], [[EV]]
+  // CHECK: store double [[ADD]], double* [[E_IN]], align
+  // CHECK: call void @__kmpc_nvptx_end_reduce_nowait(
+  // CHECK: br label %[[EXIT]]
+  //
+  // CHECK: [[EXIT]]
+  // CHECK: call void @__kmpc_kernel_deinit()
+
+  //
+  // Reduction function
+  // CHECK: define internal void [[REDUCTION_FUNC:@.+]](i8*, i8*)
+  // CHECK: [[VAR_RHS_REF:%.+]] = getelementptr inbounds [[RLT]], [[RLT]]* [[RED_LIST_RHS:%.+]], i[[SZ]] 0, i[[SZ]] 0
+  // CHECK: [[VAR_RHS_VOID:%.+]] = load i8*, i8** [[VAR_RHS_REF]],
+  // CHECK: [[VAR_RHS:%.+]] = bitcast i8* [[VAR_RHS_VOID]] to double*
+  //
+  // CHECK: [[VAR_LHS_REF:%.+]] = getelementptr inbounds [[RLT]], [[RLT]]* [[RED_LIST_LHS:%.+]], i[[SZ]] 0, i[[SZ]] 0
+  // CHECK: [[VAR_LHS_VOID:%.+]] = load i8*, i8** [[VAR_LHS_REF]],
+  // CHECK: [[VAR_LHS:%.+]] = bi

[PATCH] D29879: [OpenMP] Teams reduction on the NVPTX device.

[Arpith Jacob via Phabricator via cfe-commits]

arpith-jacob marked 8 inline comments as done.
arpith-jacob added a comment.

Alexey, thank you for your review.  I have used SizeTy instead of assuming 
64-bits.




Comment at: lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp:725
+/*isVarArg=*/false);
+llvm::Type *InterWarpCopyTypeParams[] = {CGM.VoidPtrTy, CGM.Int32Ty};
+auto *InterWarpCopyFnTy =

ABataev wrote:
> You should use `CGM.IntTy` instead of `CGM.Int32Ty` if 
> `(*kmp_InterWarpCopyFctPtr)` really uses `int` type as the second type.
Yes, this should be Int32Ty.  I've modified the function signature in the 
comment to use int32_t instead of int.


https://reviews.llvm.org/D29879



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

[PATCH] D28768: [clang-tidy] Add check 'modernize-return-braced-init-list'

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere updated this revision to Diff 88409.
JDevlieghere marked 4 inline comments as done.
JDevlieghere added a comment.

- Updated the diff with comments from @alexfh

Thanks for mentioning the check fixes pattern, it's quite crucial but I never 
really thought about that!


Repository:
  rL LLVM

https://reviews.llvm.org/D28768

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/ReturnBracedInitListCheck.cpp
  clang-tidy/modernize/ReturnBracedInitListCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-return-braced-init-list.rst
  test/clang-tidy/modernize-return-braced-init-list.cpp

Index: test/clang-tidy/modernize-return-braced-init-list.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-return-braced-init-list.cpp
@@ -0,0 +1,199 @@
+// RUN: %check_clang_tidy %s modernize-return-braced-init-list %t -- --
+// -std=c++14
+
+namespace std {
+typedef decltype(sizeof(int)) size_t;
+
+// libc++'s implementation
+template 
+class initializer_list {
+  const _E *__begin_;
+  size_t __size_;
+
+  initializer_list(const _E *__b, size_t __s)
+  : __begin_(__b),
+__size_(__s) {}
+
+public:
+  typedef _E value_type;
+  typedef const _E &reference;
+  typedef const _E &const_reference;
+  typedef size_t size_type;
+
+  typedef const _E *iterator;
+  typedef const _E *const_iterator;
+
+  initializer_list() : __begin_(nullptr), __size_(0) {}
+
+  size_t size() const { return __size_; }
+  const _E *begin() const { return __begin_; }
+  const _E *end() const { return __begin_ + __size_; }
+};
+
+template 
+class vector {
+public:
+  vector(T) {}
+  vector(std::initializer_list) {}
+};
+}
+
+class Bar {};
+
+Bar b0;
+
+class Foo {
+public:
+  Foo(Bar) {}
+  explicit Foo(Bar, unsigned int) {}
+  Foo(unsigned int) {}
+};
+
+class Baz {
+public:
+  Foo m() {
+Bar bm;
+return Foo(bm);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: to avoid repeating the return type from the declaration; use a braced initializer list instead [modernize-return-braced-init-list]
+// CHECK-FIXES: return {bm};
+  }
+};
+
+class Quux : public Foo {
+public:
+  Quux(Bar bar) : Foo(bar) {}
+  Quux(unsigned, unsigned, unsigned k = 0) : Foo(k) {}
+};
+
+Foo f() {
+  Bar b1;
+  return Foo(b1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {b1};
+}
+
+Foo f2() {
+  Bar b2;
+  return {b2};
+}
+
+auto f3() {
+  Bar b3;
+  return Foo(b3);
+}
+
+#define A(b) Foo(b)
+
+Foo f4() {
+  Bar b4;
+  return A(b4);
+}
+
+Foo f5() {
+  Bar b5;
+  return Quux(b5);
+}
+
+Foo f6() {
+  Bar b6;
+  return Foo(b6, 1);
+}
+
+std::vector f7() {
+  int i7 = 1;
+  return std::vector(i7);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+}
+
+Bar f8() {
+  return {};
+}
+
+Bar f9() {
+  return Bar();
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+}
+
+Bar f10() {
+  return Bar{};
+}
+
+Foo f11(Bar b11) {
+  return Foo(b11);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {b11};
+}
+
+Foo f12() {
+  return f11(Bar());
+}
+
+Foo f13() {
+  return Foo(Bar()); // 13
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {Bar()}; // 13
+}
+
+Foo f14() {
+  // FIXME: Type narrowing should not occur!
+  return Foo(-1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {-1};
+}
+
+Foo f15() {
+  return Foo(f10());
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {f10()};
+}
+
+Quux f16() {
+  return Quux(1, 2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {1, 2};
+}
+
+Quux f17() {
+  return Quux(1, 2, 3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: to avoid repeating the return type
+  // CHECK-FIXES: return {1, 2, 3};
+}
+
+template 
+T f19() {
+  return T();
+}
+
+Bar i1 = f19();
+Baz i2 = f19();
+
+template 
+Foo f20(T t) {
+  return Foo(t);
+}
+
+Foo i3 = f20(b0);
+
+template 
+class BazT {
+public:
+  T m() {
+Bar b;
+return T(b);
+  }
+
+  Foo m2(T t) {
+return Foo(t);
+  }
+};
+
+BazT bazFoo;
+Foo i4 = bazFoo.m();
+Foo i5 = bazFoo.m2(b0);
+
+BazT bazQuux;
+Foo i6 = bazQuux.m();
+Foo i7 = bazQuux.m2(b0);
+
+auto v1 = []() { return std::vector({1, 2}); }();
+auto v2 = []() -> std::vector { return std::vector({1, 2}); }();
Index: docs/clang-tidy/checks/modernize-return-braced-init-list.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-return-braced-init-list.rst
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - modernize-return-braced-init-list
+
+modernize-return-bra

r295094 - Finish a comment + remove trailing whitespace. NFC

[George Burgess IV via cfe-commits]

Author: gbiv
Date: Tue Feb 14 13:34:33 2017
New Revision: 295094

URL: http://llvm.org/viewvc/llvm-project?rev=295094&view=rev
Log:
Finish a comment + remove trailing whitespace. NFC

Modified:
cfe/trunk/include/clang/AST/DeclBase.h

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=295094&r1=295093&r2=295094&view=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Tue Feb 14 13:34:33 2017
@@ -654,20 +654,19 @@ public:
   /// a precompiled header or module) rather than having been parsed.
   bool isFromASTFile() const { return FromASTFile; }
 
-  /// \brief Retrieve the global declaration ID associated with this 
-  /// declaration, which specifies where in the 
-  unsigned getGlobalID() const { 
+  /// \brief Retrieve the global declaration ID associated with this
+  /// declaration, which specifies where this Decl was loaded from.
+  unsigned getGlobalID() const {
 if (isFromASTFile())
   return *((const unsigned*)this - 1);
 return 0;
   }
-  
+
   /// \brief Retrieve the global ID of the module that owns this particular
   /// declaration.
   unsigned getOwningModuleID() const {
 if (isFromASTFile())
   return *((const unsigned*)this - 2);
-
 return 0;
   }
 


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

[PATCH] D29957: [clang-tidy] Ignore instantiated functions and static data members of classes in misc-definitions-in-headers.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
Herald added a subscriber: JDevlieghere.

https://reviews.llvm.org/D29957

Files:
  clang-tidy/misc/DefinitionsInHeadersCheck.cpp
  test/clang-tidy/misc-definitions-in-headers.hpp


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -71,6 +71,12 @@
 template 
 int CB::a = 2; // OK: static data member definition of a class template.
 
+template class CB; // OK: explicitly instantiated static data member of a 
class template.
+inline int callCB() {
+  CB cb; // OK: implicitly instantiated static data member of a class 
template.
+  return cb.a;
+}
+
 template 
 T tf() { // OK: template function definition.
   T a;
@@ -107,6 +113,12 @@
 
 int f8() = delete; // OK: the function being marked delete is not callable.
 
+template 
+int f9(T t) { return 1; }
+
+inline void callF9() { f9(1); } // OK: implicitly instantiated function.
+template int f9(double); // OK: explicitly instantiated function.
+
 int a = 1;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'a' defined in a header 
file; variable definitions in header files can lead to ODR violations 
[misc-definitions-in-headers]
 CA a1;
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -104,8 +104,8 @@
 // Function templates are allowed.
 if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
   return;
-// Function template full specialization is prohibited in header file.
-if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+// Ignore instantiated functions.
+if (FD->isTemplateInstantiation())
   return;
 // Member function of a class template and member function of a nested 
class
 // in a class template are allowed.
@@ -131,7 +131,8 @@
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
   return;
-if (VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+// Ignore instantiated static data members of classes.
+if (isTemplateInstantiation(VD->getTemplateSpecializationKind()))
   return;
 // Ignore variable definition within function scope.
 if (VD->hasLocalStorage() || VD->isStaticLocal())


Index: test/clang-tidy/misc-definitions-in-headers.hpp
===
--- test/clang-tidy/misc-definitions-in-headers.hpp
+++ test/clang-tidy/misc-definitions-in-headers.hpp
@@ -71,6 +71,12 @@
 template 
 int CB::a = 2; // OK: static data member definition of a class template.
 
+template class CB; // OK: explicitly instantiated static data member of a class template.
+inline int callCB() {
+  CB cb; // OK: implicitly instantiated static data member of a class template.
+  return cb.a;
+}
+
 template 
 T tf() { // OK: template function definition.
   T a;
@@ -107,6 +113,12 @@
 
 int f8() = delete; // OK: the function being marked delete is not callable.
 
+template 
+int f9(T t) { return 1; }
+
+inline void callF9() { f9(1); } // OK: implicitly instantiated function.
+template int f9(double); // OK: explicitly instantiated function.
+
 int a = 1;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'a' defined in a header file; variable definitions in header files can lead to ODR violations [misc-definitions-in-headers]
 CA a1;
Index: clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===
--- clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -104,8 +104,8 @@
 // Function templates are allowed.
 if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
   return;
-// Function template full specialization is prohibited in header file.
-if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+// Ignore instantiated functions.
+if (FD->isTemplateInstantiation())
   return;
 // Member function of a class template and member function of a nested class
 // in a class template are allowed.
@@ -131,7 +131,8 @@
 // Static data members of a class template are allowed.
 if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
   return;
-if (VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+// Ignore instantiated static data members of classes.
+if (isTemplateInstantiation(VD->getTemplateSpecializationKind()))
   return;
 // Ignore variable definition within function scope.
 if (VD->hasLocalStorage() || VD->isStaticLocal())
___
cfe-commits mailing list
cfe-commits@lists.llv

r295100 - Revert "[profiling] Update test case to deal with name variable change (NFC)"

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Tue Feb 14 14:03:53 2017
New Revision: 295100

URL: http://llvm.org/viewvc/llvm-project?rev=295100&view=rev
Log:
Revert "[profiling] Update test case to deal with name variable change (NFC)"

This reverts commit r295085, because the corresponding llvm change was
reverted.

Modified:
cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp

Modified: cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp?rev=295100&r1=295099&r2=295100&view=diff
==
--- cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp (original)
+++ cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp Tue Feb 14 14:03:53 
2017
@@ -13,6 +13,15 @@ struct B : A {
   virtual ~B();
 };
 
+// Base dtor
+// CHECK: @__profn__ZN1BD2Ev = private constant [9 x i8] c"_ZN1BD2Ev"
+
+// Complete dtor must not be instrumented
+// CHECK-NOT: @__profn__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
+
+// Deleting dtor must not be instrumented
+// CHECK-NOT: @__profn__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
+
 // Base dtor counters and profile data
 // CHECK: @__profc__ZN1BD2Ev = private global [1 x i64] zeroinitializer
 // CHECK: @__profd__ZN1BD2Ev =


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

r295101 - [profiling] Update test cases to deal with name variable change (NFC)

[Vedant Kumar via cfe-commits]

Author: vedantk
Date: Tue Feb 14 14:03:56 2017
New Revision: 295101

URL: http://llvm.org/viewvc/llvm-project?rev=295101&view=rev
Log:
[profiling] Update test cases to deal with name variable change (NFC)

This is a re-try of r295085: fix up some test cases that assume that
profile name variables are preserved by the instrprof pass.

This catches one additional case in test/CoverageMapping/unused_names.c.

Modified:
cfe/trunk/test/CoverageMapping/unused_names.c
cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp

Modified: cfe/trunk/test/CoverageMapping/unused_names.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CoverageMapping/unused_names.c?rev=295101&r1=295100&r2=295101&view=diff
==
--- cfe/trunk/test/CoverageMapping/unused_names.c (original)
+++ cfe/trunk/test/CoverageMapping/unused_names.c Tue Feb 14 14:03:56 2017
@@ -2,14 +2,15 @@
 // RUN: FileCheck -input-file %t %s
 // RUN: FileCheck -check-prefix=SYSHEADER -input-file %t %s
 
-// Since foo is never emitted, there should not be a profile name for it.
-
-// CHECK-DAG: @__profn_bar = {{.*}} [3 x i8] c"bar"
-// CHECK-DAG: @__profn_baz = {{.*}} [3 x i8] c"baz"
-// CHECK-DAG: @__profn_unused_names.c_qux = {{.*}} [18 x i8] 
c"unused_names.c:qux"
+// CHECK-DAG: @__profc_bar
 // CHECK-DAG: @__llvm_prf_nm = private constant {{.*}}, section 
"{{.*}}__llvm_prf_names"
 
-// SYSHEADER-NOT: @__profn_foo =
+// These are never instantiated, so we shouldn't get counters for them.
+//
+// CHECK-NOT: @__profc_baz
+// CHECK-NOT: @__profc_unused_names.c_qux
+
+// SYSHEADER-NOT: @__profc_foo =
 
 
 #ifdef IS_SYSHEADER

Modified: cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp?rev=295101&r1=295100&r2=295101&view=diff
==
--- cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp (original)
+++ cfe/trunk/test/Profile/cxx-virtual-destructor-calls.cpp Tue Feb 14 14:03:56 
2017
@@ -13,15 +13,6 @@ struct B : A {
   virtual ~B();
 };
 
-// Base dtor
-// CHECK: @__profn__ZN1BD2Ev = private constant [9 x i8] c"_ZN1BD2Ev"
-
-// Complete dtor must not be instrumented
-// CHECK-NOT: @__profn__ZN1BD1Ev = private constant [9 x i8] c"_ZN1BD1Ev"
-
-// Deleting dtor must not be instrumented
-// CHECK-NOT: @__profn__ZN1BD0Ev = private constant [9 x i8] c"_ZN1BD0Ev"
-
 // Base dtor counters and profile data
 // CHECK: @__profc__ZN1BD2Ev = private global [1 x i64] zeroinitializer
 // CHECK: @__profd__ZN1BD2Ev =


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

[PATCH] D23421: [Clang-tidy] CERT-MSC53-CPP (checker for std namespace modification)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.cpp:34
+
+  diag(N->getLocation(),
+   "modification of '%0' namespace can result in undefined behavior")

I think this will still trigger false positives because there are times when it 
is permissible to modify the std namespace outside of a system header. The 
important bit of the CERT requirement is "Do not add declarations or 
definitions to the standard namespaces std or posix, or to a namespace 
contained therein, except for a template specialization that depends on a 
user-defined type that meets the standard library requirements for the original 
template." So the part that's missing from this check is excluding template 
specializations that depend on user-defined types. For instance, the following 
should be valid:
```
namespace std {
template <>
struct is_error_code_enum : std::true_type {};
}
```
(This comes from Error.h in LLVM.)



Comment at: clang-tidy/cert/DontModifyStdNamespaceCheck.cpp:36
+   "modification of '%0' namespace can result in undefined behavior")
+  << N->getName();
+}

No need to call `getName()` -- you can pass `N` directly. When you correct 
this, also remove the explicit quotes from the diagnostic (they're added 
automatically by the diagnostics engine).



Comment at: docs/clang-tidy/checks/cert-dcl58-cpp.rst:8
+behavior.
+This check warns for such modifications.
+

You should add a link to the CERT guideline this addresses. See 
docs/clang-tidy/checks/cert-dcl50-cpp.rst as an example.


https://reviews.llvm.org/D23421



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

[PATCH] D29748: [cxx1z-constexpr-lambda] Implement captures - thus completing implementation of constexpr lambdas.

[Richard Smith via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/AST/ExprConstant.cpp:428-429
 
+llvm::DenseMap LambdaCaptureFields;
+FieldDecl *LambdaThisCaptureField;
+

I'm a little concerned that adding this to every `CallStackFrame` may have a 
nontrivial impact on the overall stack usage of deeply-recursing constexpr 
evaluataions. (I'd also like to cache this map rather than recomputing it 
repeatedly.) But let's try this and see how it goes; we can look into caching 
the map as a later change.



Comment at: lib/AST/ExprConstant.cpp:4194
+MD->getParent()->getCaptureFields(Frame.LambdaCaptureFields,
+  Frame.LambdaThisCaptureField);
   }

Indent.



Comment at: lib/AST/ExprConstant.cpp:5061
+  // ... then update it to refer to the field of the closure object
+  // that represents the capture.
+  if (!HandleLValueMember(Info, E, Result, FD))

```constexpr bool b = [&]{ return &n; }() == &n; // should be accepted```

... is more what I was thinking.



Comment at: lib/AST/ExprConstant.cpp:5067-5071
+  APValue RVal;
+  if (!handleLValueToRValueConversion(Info, E, FD->getType(), Result,
+  RVal))
+return false;
+  Result.setFrom(Info.Ctx, RVal);

Too much indentation here.


https://reviews.llvm.org/D29748



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

[PATCH] D29930: Add `__is_direct_constructible` trait for checking safe reference initialization.

[David Majnemer via Phabricator via cfe-commits]

majnemer added inline comments.



Comment at: include/clang/Basic/TokenKinds.def:418
 TYPE_TRAIT_N(__is_nothrow_constructible, IsNothrowConstructible, KEYCXX)
+TYPE_TRAIT_N(__is_direct_constructible, IsDirectConstructible, KEYCXX)
 

Should this trait be grouped here?


https://reviews.llvm.org/D29930



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

[PATCH] D29910: [OpenMP] Specialize default schedule on a worksharing loop on the NVPTX device.

[Arpith Jacob via Phabricator via cfe-commits]

arpith-jacob updated this revision to Diff 88423.
arpith-jacob added a comment.

Hi Alexey,

Thank you for reviewing this patch.

> I don't like the idea of adding some kind of default scheduling, that is not 
> defined in standard in Sema

Actually, "default scheduling" is defined in the OpenMP spec.  It is called 
"def-sched-var" and controls the scheduling of loops.  It's value is 
implementation (compiler) defined.  So why not allow the target device to 
choose this value in the compiler?

  http://www.openmp.org/wp-content/uploads/openmp-4.5.pdf
  
  Section 2.3.1: ICV Descriptions, pg 46:
  def-sched-var - controls the implementation defined default scheduling of 
loop regions. There is one copy of this ICV per device.
  
  Section 2.3.2: ICV Initialization, pg 47:
  Table 2.1:
  def-sched-var   No environment variable  Initial value is implementation 
defined
  
  Section 2.7.1.1: Determining the Schedule of a Worksharing Loop
  When execution encounters a loop directive, the schedule clause (if any) on 
the directive, and the run-sched-var and def-sched-var ICVs are used to 
determine how loop iterations are assigned to threads. See Section 2.3 on page 
36 for details of how the values of the ICVs are determined. If the loop 
directive does not have a schedule clause then the current value of the 
def-sched-var ICV determines the schedule.

I've reworked the patch to handle the default scheduling in Sema and removed 
the function from OpenMPKind.cpp.  Please let me know if this looks good.

I can rewrite the patch as you suggested, involving NVPTX specific RT, but I 
think the code will look quite ugly.


https://reviews.llvm.org/D29910

Files:
  include/clang/AST/StmtOpenMP.h
  include/clang/Basic/OpenMPKinds.h
  lib/AST/StmtOpenMP.cpp
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  test/OpenMP/nvptx_coalesced_scheduling_codegen.cpp

Index: test/OpenMP/nvptx_coalesced_scheduling_codegen.cpp
===
--- /dev/null
+++ test/OpenMP/nvptx_coalesced_scheduling_codegen.cpp
@@ -0,0 +1,322 @@
+// Test target codegen - host bc file has to be created first.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -fexceptions -fcxx-exceptions -x c++ -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-32
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+// Check that the execution mode of the target regions on the gpu is set to the right mode.
+// CHECK-DAG: {{@__omp_offloading_.+l19}}_exec_mode = weak constant i8 0
+
+template
+tx ftemplate() {
+  tx a[100];
+  tx b[10][10];
+
+  #pragma omp target parallel
+  {
+#pragma omp for
+for (int i = 0; i < 99; i++) {
+  a[i] = 1;
+}
+
+#pragma omp for schedule(auto)
+for (int i = 0; i < 98; i++) {
+  a[i] = 2;
+}
+
+#pragma omp for schedule(static,1)
+for (int i = 0; i < 97; i++) {
+  a[i] = 3;
+}
+
+#pragma omp for schedule(static,2)
+for (int i = 0; i < 96; i++) {
+  a[i] = 1;
+}
+
+#pragma omp for schedule(static)
+for (int i = 0; i < 95; i++) {
+  a[i] = 1;
+}
+
+#pragma omp for schedule(auto) ordered
+for (int i = 0; i < 94; i++) {
+  a[i] = 1;
+}
+
+#pragma omp for schedule(runtime)
+for (int i = 0; i < 93; i++) {
+  a[i] = 1;
+}
+
+#pragma omp for schedule(dynamic)
+for (int i = 0; i < 92; i++) {
+  a[i] = 1;
+}
+
+#pragma omp for schedule(guided)
+for (int i = 0; i < 91; i++) {
+  a[i] = 1;
+}
+  }
+
+  return a[0] + b[9][9];
+}
+
+int bar(){
+  int a = 0;
+
+  a += ftemplate();
+
+  return a;
+}
+
+  // CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l19}}(
+  // CHECK: call void @__kmpc_spmd_kernel_init(
+  // CHECK: br label {{%?}}[[EXEC:.+]]
+  //
+  // CHECK: [[EXEC]]
+  // CHECK: {{call|invoke}} void [[OP1:@.+]](i32*
+  // CHECK: br label {{%?}}[[DONE:.+]]
+  //
+  // CHECK: [[DONE]]
+  // CHECK: call 

[PATCH] D29908: Disallow returning a __block variable via a move

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D29908#676601, @ahatanak wrote:

> In https://reviews.llvm.org/D29908#676589, @ahatanak wrote:
>
> > Posting John's review comment, which was sent to the review I abandoned:
> >
> > "Hmm.  The behavior I think we actually want here is that we should move 
> > out of the __block variable but not perform NRVO, essentially like we would 
> > if the variable were a parameter.
>
>
> I think that's what's happening. Because of the changes made in r274291, NRVO 
> isn't performed, but the move constructor instead of the copy constructor is 
> called to return the shared_ptr. The shared_ptr in the heap is moved to 
> returned shared_ptr (%agg.result), so when the block function passed to 
> dispatch_async is executed, it segfaults because the object "ret" used to 
> point to has been destructed.
>
> If that's the case, should we conclude that clang is behaving as expected?
>
> > The block byref copy helper should always be doing a move if it can, 
> > regardless of whether the function is returned.  That's independent of 
> > this, though."


Oh, I see.  Just to clarify: it doesn't matter that the object "ret" used to 
point to has been destructed, what matters is that "ret" now holds a null 
reference because it's been moved out of.

I retract my comment; I agree there's a bug here.  We should not implicitly 
move out of __block variables during a return because we cannot assume that the 
variable will no longer be used.  Please update the comment to correctly 
identify this as the reasoning; it has nothing to do with whether __block 
variables technically *can* support NRVO.


https://reviews.llvm.org/D29908



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

[PATCH] D29369: [ubsan] Omit superflous overflow checks for promoted arithmetic (PR20193)

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

In https://reviews.llvm.org/D29369#676521, @dtzWill wrote:

> In https://reviews.llvm.org/D29369#673064, @vsk wrote:
>
> > In https://reviews.llvm.org/D29369#672166, @dtzWill wrote:
> >
> > > After some thought, can we discuss why this is a good idea?
> >
> >
> > The goal is to lower ubsan's compile-time + instrumentation overhead at 
> > -O0, since this reduces the friction of debugging a ubsan-instrumented 
> > project.
>
>
> Apologies for the delay, thank you for the explanation.


Np, thanks for taking a look!

>>> This increases the cyclomatic complexity of code that already is difficult 
>>> to reason about, and seems like it's both brittle and out-of-place in 
>>> CGExprScalar.
>> 
>> Are there cleanups or ways to reorganize the code that would make this sort 
>> of change less complex / brittle? I'm open to taking that on.
> 
> None that I see immediately (heh, otherwise I'd be working on them myself...) 
> but the code paths for trapping/non-trapping are particularly what I meant 
> re:complexity, and while I suppose the AST or its interface is probably 
> unlikely to change much (?) I'm concerned about these checks silently 
> removing checks they shouldn't in the future.
> 
> (Who would notice if this happened?)

I don't have a good answer for this. I've tried to make sure we don't introduce 
any false negatives with this patch by covering all the cases I can think of, 
but it's possible we could have missed something. There are enough people using 
this feature that I think we'd be alerted to + fix false negatives.

>>> It really seems it would be better to let InstCombine or some other 
>>> analysis/transform deal with proving checks redundant instead of attempting 
>>> to do so on-the-fly during CodeGen.
>> 
>> -O1/-O2 do get rid of a lot of checks, but they also degrade the debugging 
>> experience, so it's not really a solution for this use case.
> 
> Understood, that makes sense.
> 
> Would running InstCombine (and only InstCombine):
> 
> 1. Fail to remove any checks elided by this change?

No, instcombine gets all of these.

> 2. Have a negative impact on debugging experience? For this I'm mostly asking 
> for a guess, I don't know how to exactly quantify this easily.

Probably, but I'm not 100% sure. Instcombine can touch a fair amount of debug 
info.

> (3) Have an undesirable impact on compilation time or other negative 
> consequence?)

Instcombine is one of the slower llvm passes, IIRC. At any rate, the idea of 
modifying the -O0 pipeline when ubsan is enabled just to turn on instcombine 
doesn't seem palatable..

>>> Can you better motivate why this is worth these costs, or explain your use 
>>> case a bit more?
>> 
>> I have some numbers from LNT. I did a pre-patch and post-patch run at -O0 + 
>> -fsanitize=signed-integer-overflow,unsigned-integer-overflow. There were 
>> 4,672 object files produced in each run. This patch brings the average 
>> object size down from 36,472.0 to 36,378.3 bytes (a 0.26% improvement), and 
>> the average number of overflow checks per object down from 66.8 to 66.2 (a 
>> 0.81% improvement).
> 
> Wonderful, thank you for producing and sharing these numbers.  Those 
> improvements don't convince me, but if you're saying this is important to you 
> and your use-case/users I'm happy to go with that.

Yeah, on average, the patch isn't a huge improvement. What makes it worthwhile 
(imo) is that the risk is also very low, and that it can pay to emit less IR 
(for the one person out there that wants to add a million shorts together).

Some context: we have a project that adds ~17,000 integers together in 
straight-line code (someone must have auto-generated the C code that does this 
><...). The amount of add/sub overflow checks generated there brings clang to 
its knees at -O0, -Os, etc. We had to kill the build of this project. I get 
that it's not a representative example, but this is the kind of behavior I 
really don't want unsuspecting users to hit.

>> I don't have reliable compile-time numbers, but not emitting IR really seems 
>> like a straightforward improvement over emitting/analyzing/removing it.
> 
> Hard to say.  Separation of concerns is important too, but of course there's 
> trade-offs everywhere :).  I'd suspect this doesn't change compile-time much 
> either way.
> 
>> So, those are the benefits. IMO getting close to 1% better at reducing 
>> instrumentation overhead is worth the complexity cost here.
> 
> Couldn't say, but that sounds reasonable to me and I don't mean to stand in 
> the way of progress!
> 
> Can you answer my questions about InstCombine above? Thanks!


https://reviews.llvm.org/D29369



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

r295107 - MS inline asm: Filter MXCSR out of the inferred clobber list

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Tue Feb 14 15:38:17 2017
New Revision: 295107

URL: http://llvm.org/viewvc/llvm-project?rev=295107&view=rev
Log:
MS inline asm: Filter MXCSR out of the inferred clobber list

Since r295004, LLVM has started modelling this new register, but we
don't have GCC constraint inline asm spellings for it yet.

Modified:
cfe/trunk/lib/Parse/ParseStmtAsm.cpp
cfe/trunk/test/CodeGen/ms-inline-asm.c

Modified: cfe/trunk/lib/Parse/ParseStmtAsm.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmtAsm.cpp?rev=295107&r1=295106&r2=295107&view=diff
==
--- cfe/trunk/lib/Parse/ParseStmtAsm.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmtAsm.cpp Tue Feb 14 15:38:17 2017
@@ -621,10 +621,11 @@ StmtResult Parser::ParseMicrosoftAsmStat
MII.get(), IP.get(), Callback))
 return StmtError();
 
-  // Filter out "fpsw".  Clang doesn't accept it, and it always lists flags and
-  // fpsr as clobbers.
-  auto End = std::remove(Clobbers.begin(), Clobbers.end(), "fpsw");
-  Clobbers.erase(End, Clobbers.end());
+  // Filter out "fpsw" and "mxcsr". They aren't valid GCC asm clobber
+  // constraints. Clang always adds fpsr to the clobber list anyway.
+  llvm::erase_if(Clobbers, [](const std::string &C) {
+return C == "fpsw" || C == "mxcsr";
+  });
 
   // Build the vector of clobber StringRefs.
   ClobberRefs.insert(ClobberRefs.end(), Clobbers.begin(), Clobbers.end());

Modified: cfe/trunk/test/CodeGen/ms-inline-asm.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-inline-asm.c?rev=295107&r1=295106&r2=295107&view=diff
==
--- cfe/trunk/test/CodeGen/ms-inline-asm.c (original)
+++ cfe/trunk/test/CodeGen/ms-inline-asm.c Tue Feb 14 15:38:17 2017
@@ -649,6 +649,14 @@ void label6(){
   // CHECK: call void asm sideeffect inteldialect "jmp 
{{.*}}__MSASMLABEL_.${:uid}__label\0A\09{{.*}}__MSASMLABEL_.${:uid}__label:", 
"~{dirflag},~{fpsr},~{flags}"()
 }
 
+// Don't include mxcsr in the clobber list.
+void mxcsr() {
+  char buf[4096];
+  __asm fxrstor buf
+}
+// CHECK-LABEL: define void @mxcsr
+// CHECK: call void asm sideeffect inteldialect "fxrstor byte ptr $0", 
"=*m,~{dirflag},~{fpsr},~{flags}"
+
 typedef union _LARGE_INTEGER {
   struct {
 unsigned int LowPart;


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

[PATCH] D29908: Disallow returning a __block variable via a move

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 88430.
ahatanak added a comment.

Update comment.


https://reviews.llvm.org/D29908

Files:
  lib/Sema/SemaStmt.cpp
  test/SemaObjCXX/blocks.mm


Index: test/SemaObjCXX/blocks.mm
===
--- test/SemaObjCXX/blocks.mm
+++ test/SemaObjCXX/blocks.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class 
-std=c++11 %s
 @protocol NSObject;
 
 void bar(id(^)(void));
@@ -144,3 +144,17 @@
 
   template void f(X);
 }
+
+namespace MoveBlockVariable {
+struct B0 {
+};
+
+struct B1 { // expected-note 2 {{candidate constructor (the implicit}}
+  B1(B0&&); // expected-note {{candidate constructor not viable}}
+};
+
+B1 test_move() {
+  __block B0 b;
+  return b; // expected-error {{no viable conversion from returned value of 
type 'MoveBlockVariable::B0' to function return type 'MoveBlockVariable::B1'}}
+}
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2743,15 +2743,17 @@
   // ...automatic...
   if (!VD->hasLocalStorage()) return false;
 
+  // Return false if VD is a __block variable. We don't want to implicitly move
+  // out of a __block variable during a return because we cannot assume the
+  // variable will no longer be used.
+  if (VD->hasAttr()) return false;
+
   if (AllowParamOrMoveConstructible)
 return true;
 
   // ...non-volatile...
   if (VD->getType().isVolatileQualified()) return false;
 
-  // __block variables can't be allocated in a way that permits NRVO.
-  if (VD->hasAttr()) return false;
-
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VD->getType()->isDependentType() && VD->hasAttr() &&


Index: test/SemaObjCXX/blocks.mm
===
--- test/SemaObjCXX/blocks.mm
+++ test/SemaObjCXX/blocks.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class -std=c++11 %s
 @protocol NSObject;
 
 void bar(id(^)(void));
@@ -144,3 +144,17 @@
 
   template void f(X);
 }
+
+namespace MoveBlockVariable {
+struct B0 {
+};
+
+struct B1 { // expected-note 2 {{candidate constructor (the implicit}}
+  B1(B0&&); // expected-note {{candidate constructor not viable}}
+};
+
+B1 test_move() {
+  __block B0 b;
+  return b; // expected-error {{no viable conversion from returned value of type 'MoveBlockVariable::B0' to function return type 'MoveBlockVariable::B1'}}
+}
+}
Index: lib/Sema/SemaStmt.cpp
===
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -2743,15 +2743,17 @@
   // ...automatic...
   if (!VD->hasLocalStorage()) return false;
 
+  // Return false if VD is a __block variable. We don't want to implicitly move
+  // out of a __block variable during a return because we cannot assume the
+  // variable will no longer be used.
+  if (VD->hasAttr()) return false;
+
   if (AllowParamOrMoveConstructible)
 return true;
 
   // ...non-volatile...
   if (VD->getType().isVolatileQualified()) return false;
 
-  // __block variables can't be allocated in a way that permits NRVO.
-  if (VD->hasAttr()) return false;
-
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VD->getType()->isDependentType() && VD->hasAttr() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29908: Disallow returning a __block variable via a move

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

In https://reviews.llvm.org/D29908#676788, @rjmccall wrote:

> Oh, I see.  Just to clarify: it doesn't matter that the object "ret" used to 
> point to has been destructed, what matters is that "ret" now holds a null 
> reference because it's been moved out of.


Yes, that's right.

> I retract my comment; I agree there's a bug here.  We should not implicitly 
> move out of __block variables during a return because we cannot assume that 
> the variable will no longer be used.  Please update the comment to correctly 
> identify this as the reasoning; it has nothing to do with whether __block 
> variables technically *can* support NRVO.

OK, thanks. I was wondering whether there is something we should or can do in 
IRGen to make it work, but it looks like we have to just disable moving out of 
block variables.

I'll commit this patch later today.


https://reviews.llvm.org/D29908



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Anton Korobeynikov via Phabricator via cfe-commits]

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

We shouldn't add resource dirs to the path. After all, these headers could use, 
for example, gcc-only extensions.


https://reviews.llvm.org/D29464



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

[PATCH] D29930: Add `__is_direct_constructible` trait for checking safe reference initialization.

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

I don't like this name; it sounds too much like you're asking whether a certain 
direct-initialization is possible, which is what `__is_constructible` does. I 
also don't like the idea of combining an "is this type direct-initializable 
from this list of arguments" check with a reference-specific side-check; it 
seems better to expose the underlying check itself and allow the user to figure 
out how they want to combine the checks.

Perhaps we could introduce a `__reference_binds_to_temporary(T, U)` trait, 
where `T` is required to be a reference type, that determines whether a 
reference of type `T` bound to an expression of type `U` would bind to a 
materialized temporary object (or subobject thereof)?


https://reviews.llvm.org/D29930



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

[PATCH] D29930: Add `__is_direct_constructible` trait for checking safe reference initialization.

[Eric Fiselier via Phabricator via cfe-commits]

EricWF added a comment.

In https://reviews.llvm.org/D29930#676858, @rsmith wrote:

> I don't like this name; it sounds too much like you're asking whether a 
> certain direct-initialization is possible, which is what `__is_constructible` 
> does. I also don't like the idea of combining an "is this type 
> direct-initializable from this list of arguments" check with a 
> reference-specific side-check; it seems better to expose the underlying check 
> itself and allow the user to figure out how they want to combine the checks.
>
> Perhaps we could introduce a `__reference_binds_to_temporary(T, U)` trait, 
> where `T` is required to be a reference type, that determines whether a 
> reference of type `T` bound to an expression of type `U` would bind to a 
> materialized temporary object (or subobject thereof)?


That all sounds great to me. Thanks for the comments @rsmith. I'll update it 
tomorrow with the changes.


https://reviews.llvm.org/D29930



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

[PATCH] D29692: [clang-tidy] add check modernize-use-const-instead-of-define

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin marked 5 inline comments as done.
AlexanderLanin added inline comments.



Comment at: clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp:86
+else if (isAnyParenthesis(Tok) || isAnyNumericLiteral(Tok) ||
+ (!hasPrefix && isAnyCharLiteral(Tok))) {
+  // prefix shall not be accepted anymore after any number

aaron.ballman wrote:
> What about string literals? e.g., `#define NAME "foobar"`
Excluded (for now) due to string concatenation done by preprocessor when you 
write NAME1 NAME2 in your code. I've added a section into 
modernize-use-const-instead-of-define.rst to explain this.


https://reviews.llvm.org/D29692



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

[PATCH] D29967: Get class property setter selector from property decl if exists

[David Herzka via Phabricator via cfe-commits]

herzka created this revision.

Before this fix, trying to set a class property using dot syntax would always 
use the constructed name (setX:), which might not match the real selector if 
the setter is specified via the `setter` property attribute. Now, the setter 
selector in the declaration has priority over the constructed name, which is 
consistent with instance properties.


https://reviews.llvm.org/D29967

Files:
  lib/Sema/SemaExprObjC.cpp


Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2000,10 +2000,16 @@
   }
 
   // Look for the matching setter, in case it is needed.
-  Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
-   &propertyName);
+  Selector SetterSel;
+  if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+  &propertyName, ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+SetterSel = PD->getSetterName();
+  } else {
+SetterSel =
+  SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ &propertyName);
+  }
 
   ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
   if (!Setter) {


Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -2000,10 +2000,16 @@
   }
 
   // Look for the matching setter, in case it is needed.
-  Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
-   &propertyName);
+  Selector SetterSel;
+  if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+  &propertyName, ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+SetterSel = PD->getSetterName();
+  } else {
+SetterSel =
+  SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ &propertyName);
+  }
 
   ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
   if (!Setter) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29967: Get class property setter selector from property decl if exists

[David Herzka via Phabricator via cfe-commits]

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

This issue applies to getters too. I'll make this fix broader.

I think the real fix for both is to construct the ObjCPropertyRefExpr using the 
ObjCPropertyDecl so that it's considered an explicit property. Currently, it 
still gets considered implicit, which is not true. I don't know what 
implications changing that may have, though. This fix seems safer.


https://reviews.llvm.org/D29967



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

[PATCH] D29692: [clang-tidy] add check modernize-use-const-instead-of-define

[Alexander Lanin via Phabricator via cfe-commits]

AlexanderLanin updated this revision to Diff 88447.
AlexanderLanin marked an inline comment as done.
AlexanderLanin added a comment.

Applied review comments and added test cases regarding parenthesis, floats, 
doubles, wide chars etc


https://reviews.llvm.org/D29692

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseConstInsteadOfDefineCheck.cpp
  clang-tidy/modernize/UseConstInsteadOfDefineCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst
  test/clang-tidy/modernize-use-const-instead-of-define.cpp

Index: test/clang-tidy/modernize-use-const-instead-of-define.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-const-instead-of-define.cpp
@@ -0,0 +1,67 @@
+// RUN: %check_clang_tidy %s modernize-use-const-instead-of-define %t
+
+// Although there might be cases where the - sign is actually used those
+// should be rare enough. e.g. int a = 5 BAD1;
+#define BAD1  -1
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD2  2
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD3(A)   0
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD4(X)   (7)
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD5(X)   +4
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD6 'x'
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD7 0xff
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD8 L'c'
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD9 U'c'
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD101U
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD111.5
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD121.4f
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD131.3L
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD14((-3))
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+#define BAD15-(-3)
+// CHECK-MESSAGES: :[[@LINE-1]]:{{.*}} [modernize-use-const-instead-of-define]
+
+#define GOOD1 -
+#define GOOD2 (1+2)
+#define GOOD3(A)  #A
+#define GOOD4(A,B)A+B
+#define GOOD5(T)  ((T*)0)
+#define GOOD6(type)   (type(123))
+#define GOOD7(car, ...)   car
+#define GOOD8 a[5]
+#define GOOD9(x)  ;x;
+#define GOOD10;-2;
+#define GOOD11=2
+#define GOOD12+'a'
+#define GOOD13-'z'
+#define GOOD14!3
+#define GOOD15~-1
+#define GOOD16"str"
+#define GOOD17L"str"
+#define GOOD18U"str"
+#define GOOD19()
+#define GOOD20++
+#define GOOD21++i
+#define GOOD22k--
+#define GOOD23m
+#define GOOD245-
+#define GOOD25((3)
+#define GOOD26(3))
+#define GOOD27)2(
+
+#define NOT_DETECTED_YET_1(x)  ((unsigned char)(0xff))
+#define NOT_DETECTED_YET_2 (int)7
+#define NOT_DETECTED_YET_3 int(-5)
+#define NOT_DETECTED_YET_4 (int)(0)
Index: docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/modernize-use-const-instead-of-define.rst
@@ -0,0 +1,57 @@
+.. title:: clang-tidy - modernize-use-const-instead-of-define
+
+modernize-use-const-instead-of-define
+=
+
+C++ ``const`` variables should be preferred over ``#define`` directives as
+``#define`` does not obey type checking and scope rules.
+
+Example
+---
+
+.. code-block:: c++
+
+  // calc.h
+  namespace RealCalculation {
+#define PI 3.14159
+  }
+
+  // quick.h
+  namespace QuickCalculation {
+#define PI 1
+  }
+
+  // calc.cpp
+  #include "calc.h"
+  #include "quick.h"
+
+  double calc(const double r) {
+return 2*PI*r*r;
+  }
+
+Strings
+---
+
+Strings are excluded from this rule as they might be used for string
+concatenations. Example:
+
+.. code-block:: c++
+
+  #define A "A"
+  #define B "B"
+  char AB[3] = A B;
+
+Code guidelines
+---
+
+While many C++ code guidelines like to prohibit macros completely with the
+exception of include guards, that's a rather

[PATCH] D29967: Get class property selectors from property decl if it exists

[David Herzka via Phabricator via cfe-commits]

herzka updated this revision to Diff 88448.
herzka retitled this revision from "Get class property setter selector from 
property decl if exists" to "Get class property selectors from property decl if 
it exists".
herzka edited the summary of this revision.

https://reviews.llvm.org/D29967

Files:
  lib/Sema/SemaExprObjC.cpp


Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
 }
   }
 
+  Selector GetterSel;
+  Selector SetterSel;
+  if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+  &propertyName, ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+  } else {
+GetterSel = PP.getSelectorTable().getNullarySelector(&propertyName);
+SetterSel =
+  SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ &propertyName);
+  }
+
   // Search for a declared property first.
-  Selector Sel = PP.getSelectorTable().getNullarySelector(&propertyName);
-  ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+  ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
 
   // If this reference is in an @implementation, check for 'private' methods.
   if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
 
   if (Getter) {
 // FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
   }
 
   // Look for the matching setter, in case it is needed.
-  Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
-   &propertyName);
-
   ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
   if (!Setter) {
 // If this reference is in an @implementation, also check for 'private'


Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1984,13 +1984,26 @@
 }
   }
 
+  Selector GetterSel;
+  Selector SetterSel;
+  if (ObjCPropertyDecl *PD = IFace->FindPropertyDeclaration(
+  &propertyName, ObjCPropertyQueryKind::OBJC_PR_query_class)) {
+GetterSel = PD->getGetterName();
+SetterSel = PD->getSetterName();
+  } else {
+GetterSel = PP.getSelectorTable().getNullarySelector(&propertyName);
+SetterSel =
+  SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
+ PP.getSelectorTable(),
+ &propertyName);
+  }
+
   // Search for a declared property first.
-  Selector Sel = PP.getSelectorTable().getNullarySelector(&propertyName);
-  ObjCMethodDecl *Getter = IFace->lookupClassMethod(Sel);
+  ObjCMethodDecl *Getter = IFace->lookupClassMethod(GetterSel);
 
   // If this reference is in an @implementation, check for 'private' methods.
   if (!Getter)
-Getter = IFace->lookupPrivateClassMethod(Sel);
+Getter = IFace->lookupPrivateClassMethod(GetterSel);
 
   if (Getter) {
 // FIXME: refactor/share with ActOnMemberReference().
@@ -2000,11 +2013,6 @@
   }
 
   // Look for the matching setter, in case it is needed.
-  Selector SetterSel =
-SelectorTable::constructSetterSelector(PP.getIdentifierTable(),
-PP.getSelectorTable(),
-   &propertyName);
-
   ObjCMethodDecl *Setter = IFace->lookupClassMethod(SetterSel);
   if (!Setter) {
 // If this reference is in an @implementation, also check for 'private'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r295113 - [Driver] Report available language standards on user error

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 14 16:44:20 2017
New Revision: 295113

URL: http://llvm.org/viewvc/llvm-project?rev=295113&view=rev
Log:
[Driver] Report available language standards on user error

In case user did not provide valid standard name for -std option, available
values (with short description) will be reported.

Patch by Paweł Żukowski!

Added:
cfe/trunk/test/Driver/unknown-std.c
cfe/trunk/test/Driver/unknown-std.cl
cfe/trunk/test/Driver/unknown-std.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Frontend/LangStandard.h
cfe/trunk/include/clang/Frontend/LangStandards.def
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=295113&r1=295112&r2=295113&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Tue Feb 14 16:44:20 
2017
@@ -230,6 +230,7 @@ def note_drv_t_option_is_global : Note<
   "The last /TC or /TP option takes precedence over earlier instances">;
 def note_drv_address_sanitizer_debug_runtime : Note<
   "AddressSanitizer doesn't support linking with debug runtime libraries yet">;
+def note_drv_use_standard : Note<"use '%0' for '%1' standard">;
 
 def err_analyzer_config_no_value : Error<
   "analyzer-config option '%0' has a key but no value">;

Modified: cfe/trunk/include/clang/Frontend/LangStandard.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/LangStandard.h?rev=295113&r1=295112&r2=295113&view=diff
==
--- cfe/trunk/include/clang/Frontend/LangStandard.h (original)
+++ cfe/trunk/include/clang/Frontend/LangStandard.h Tue Feb 14 16:44:20 2017
@@ -29,7 +29,8 @@ enum LangFeatures {
   Digraphs = (1 << 8),
   GNUMode = (1 << 9),
   HexFloat = (1 << 10),
-  ImplicitInt = (1 << 11)
+  ImplicitInt = (1 << 11),
+  OpenCL = (1 << 12)
 };
 
 }
@@ -91,6 +92,9 @@ public:
   /// hasImplicitInt - Language allows variables to be typed as int implicitly.
   bool hasImplicitInt() const { return Flags & frontend::ImplicitInt; }
 
+  /// isOpenCL - Language is a OpenCL variant.
+  bool isOpenCL() const { return Flags & frontend::OpenCL; }
+
   static const LangStandard &getLangStandardForKind(Kind K);
   static const LangStandard *getLangStandardForName(StringRef Name);
 };

Modified: cfe/trunk/include/clang/Frontend/LangStandards.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/LangStandards.def?rev=295113&r1=295112&r2=295113&view=diff
==
--- cfe/trunk/include/clang/Frontend/LangStandards.def (original)
+++ cfe/trunk/include/clang/Frontend/LangStandards.def Tue Feb 14 16:44:20 2017
@@ -142,16 +142,16 @@ LANGSTANDARD(gnucxx1z, "gnu++1z",
 // OpenCL
 LANGSTANDARD(opencl, "cl",
  "OpenCL 1.0",
- LineComment | C99 | Digraphs | HexFloat)
+ LineComment | C99 | Digraphs | HexFloat | OpenCL)
 LANGSTANDARD(opencl11, "cl1.1",
  "OpenCL 1.1",
- LineComment | C99 | Digraphs | HexFloat)
+ LineComment | C99 | Digraphs | HexFloat | OpenCL)
 LANGSTANDARD(opencl12, "cl1.2",
  "OpenCL 1.2",
- LineComment | C99 | Digraphs | HexFloat)
+ LineComment | C99 | Digraphs | HexFloat | OpenCL)
 LANGSTANDARD(opencl20, "cl2.0",
  "OpenCL 2.0",
- LineComment | C99 | Digraphs | HexFloat)
+ LineComment | C99 | Digraphs | HexFloat | OpenCL)
 
 LANGSTANDARD_ALIAS(opencl, "CL")
 LANGSTANDARD_ALIAS(opencl11, "CL1.1")

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=295113&r1=295112&r2=295113&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 14 16:44:20 2017
@@ -1535,13 +1535,6 @@ static void ParseHeaderSearchArgs(Header
 Opts.AddVFSOverlayFile(A->getValue());
 }
 
-static bool isOpenCL(LangStandard::Kind LangStd) {
-  return LangStd == LangStandard::lang_opencl ||
- LangStd == LangStandard::lang_opencl11 ||
- LangStd == LangStandard::lang_opencl12 ||
- LangStd == LangStandard::lang_opencl20;
-}
-
 void CompilerInvocation::setLangDefaults(LangOptions &Opts, InputKind IK,
  const llvm::Triple &T,
  PreprocessorOptions &PPOpts,
@@ -1612,7 +1605,7 @@ void CompilerInvocation::setLangDefaults
   Opts.ImplicitInt = Std.hasImplicitInt();
 
   // Set Ope

[PATCH] D29724: [Driver] Report available language standards on user error

[Richard Smith via Phabricator via cfe-commits]

rsmith added a comment.

Committed as r295113.


https://reviews.llvm.org/D29724



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

r295114 - Improve diagnostic reporting when using __declspec without enabling __declspec as a keyword.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Feb 14 16:47:20 2017
New Revision: 295114

URL: http://llvm.org/viewvc/llvm-project?rev=295114&view=rev
Log:
Improve diagnostic reporting when using __declspec without enabling __declspec 
as a keyword.

Fixes PR31936.

Added:
cfe/trunk/test/Parser/declspec-recovery.c
cfe/trunk/test/Parser/declspec-supported.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/lib/Parse/ParseDecl.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=295114&r1=295113&r2=295114&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Tue Feb 14 16:47:20 
2017
@@ -176,6 +176,9 @@ def warn_gcc_attribute_location : Warnin
 def warn_attribute_no_decl : Warning<
   "attribute %0 ignored, because it is not attached to a declaration">, 
   InGroup;
+def err_ms_attributes_not_enabled : Error<
+  "'__declspec' attributes are not enabled; use '-fdeclspec' or "
+  "'-fms-extensions' to enable support for __declspec attributes">;
 def err_expected_method_body : Error<"expected method body">;
 def err_declspec_after_virtspec : Error<
   "'%0' qualifier may not appear after the virtual specifier '%1'">;

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=295114&r1=295113&r2=295114&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Feb 14 16:47:20 2017
@@ -2966,6 +2966,31 @@ void Parser::ParseDeclarationSpecifiers(
   if (DS.hasTypeSpecifier())
 goto DoneWithDeclSpec;
 
+  // If the token is an identifier named "__declspec" and Microsoft
+  // extensions are not enabled, it is likely that there will be cascading
+  // parse errors if this really is a __declspec attribute. Attempt to
+  // recognize that scenario and recover gracefully.
+  if (!getLangOpts().DeclSpecKeyword && Tok.is(tok::identifier) &&
+  Tok.getIdentifierInfo()->getName().equals("__declspec")) {
+Diag(Loc, diag::err_ms_attributes_not_enabled);
+
+// The next token should be an open paren. If it is, eat the entire
+// attribute declaration and continue.
+if (NextToken().is(tok::l_paren)) {
+  // Consume the __declspec identifier.
+  SourceLocation Loc = ConsumeToken();
+
+  // Eat the parens and everything between them.
+  BalancedDelimiterTracker T(*this, tok::l_paren);
+  if (T.consumeOpen()) {
+assert(false && "Not a left paren?");
+return;
+  }
+  T.skipToEnd();
+  continue;
+}
+  }
+
   // In C++, check to see if this is a scope specifier like foo::bar::, if
   // so handle it as such.  This is important for ctor parsing.
   if (getLangOpts().CPlusPlus) {

Added: cfe/trunk/test/Parser/declspec-recovery.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/declspec-recovery.c?rev=295114&view=auto
==
--- cfe/trunk/test/Parser/declspec-recovery.c (added)
+++ cfe/trunk/test/Parser/declspec-recovery.c Tue Feb 14 16:47:20 2017
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -verify %s
+
+__declspec(naked) void f(void) {} // expected-error{{'__declspec' attributes 
are not enabled; use '-fdeclspec' or '-fms-extensions' to enable support for 
__declspec attributes}}
+
+struct S {
+  __declspec(property(get=Getter, put=Setter)) int X; // 
expected-error{{'__declspec' attributes are not enabled; use '-fdeclspec' or 
'-fms-extensions' to enable support for __declspec attributes}}
+  int Y;
+};

Added: cfe/trunk/test/Parser/declspec-supported.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/declspec-supported.c?rev=295114&view=auto
==
--- cfe/trunk/test/Parser/declspec-supported.c (added)
+++ cfe/trunk/test/Parser/declspec-supported.c Tue Feb 14 16:47:20 2017
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -fms-extensions 
-verify %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -fdeclspec -verify %s
+// expected-no-diagnostics
+
+__declspec(naked) void f(void) {}
+
+struct S {
+  __declspec(property(get=Getter, put=Setter)) int X;
+  int Y;
+};


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

[PATCH] D29868: Recover more gracefully when __declspec is not supported as a keyword

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

Commit in r295114.


https://reviews.llvm.org/D29868



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

[PATCH] D29967: Get class property selectors from property decl if it exists

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Can you please post the patch with full context 
(http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)?


https://reviews.llvm.org/D29967



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

[PATCH] D29464: [MinGWToolChain] Don't use GCC headers on Win32

[Mateusz Mikuła via Phabricator via cfe-commits]

mati865 added a comment.

If there are no further objections it can be committed by somebody.


https://reviews.llvm.org/D29464



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

r295118 - Do not implicitly instantiate the definition of a class template specialization

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 14 17:27:44 2017
New Revision: 295118

URL: http://llvm.org/viewvc/llvm-project?rev=295118&view=rev
Log:
Do not implicitly instantiate the definition of a class template specialization
that has been explicitly specialized!

We assume in various places that we can tell the template specialization kind
of a class type by looking at the declaration produced by TagType::getDecl.
That was previously not quite true: for an explicit specialization, we could
have first seen a template-id denoting the specialization (with a use that does
not trigger an implicit instantiation of the defintiion) and then seen the
first explicit specialization declaration. TagType::getDecl would previously
return an arbitrary declaration when called on a not-yet-defined class; it
now consistently returns the most recent declaration in that case.

Modified:
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp

Modified: cfe/trunk/lib/AST/Type.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Type.cpp?rev=295118&r1=295117&r2=295118&view=diff
==
--- cfe/trunk/lib/AST/Type.cpp (original)
+++ cfe/trunk/lib/AST/Type.cpp Tue Feb 14 17:27:44 2017
@@ -3023,8 +3023,10 @@ static TagDecl *getInterestingTagDecl(Ta
 if (I->isCompleteDefinition() || I->isBeingDefined())
   return I;
   }
-  // If there's no definition (not even in progress), return what we have.
-  return decl;
+  // If there's no definition (not even in progress), return the most recent
+  // declaration. This is important for template specializations, in order to
+  // pick the declaration with the most complete TemplateSpecializationKind.
+  return decl->getMostRecentDecl();
 }
 
 TagDecl *TagType::getDecl() const {

Modified: cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp?rev=295118&r1=295117&r2=295118&view=diff
==
--- cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp (original)
+++ cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp Tue Feb 14 
17:27:44 2017
@@ -57,3 +57,14 @@ template struct Helper {
 template void Helper::func<2>() {} // expected-error {{cannot 
specialize a member}} \
   // expected-error {{no 
function template matches}}
 }
+
+namespace b35070233 {
+  template  struct Cls {
+static void f() {}
+  };
+
+  void g(Cls);
+
+  template<> struct Cls; // expected-note {{forward declaration}}
+  template<> void Cls::f(); // expected-error {{incomplete type}}
+}


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

r295122 - Stop asserting when a meaningless -std= flag is passed for a non-compilation

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 14 17:41:38 2017
New Revision: 295122

URL: http://llvm.org/viewvc/llvm-project?rev=295122&view=rev
Log:
Stop asserting when a meaningless -std= flag is passed for a non-compilation
input kind; go back to silently ignoring the flag.

Added:
cfe/trunk/test/Driver/unknown-std.S
Modified:
cfe/trunk/lib/Frontend/CompilerInvocation.cpp

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=295122&r1=295121&r2=295122&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Tue Feb 14 17:41:38 2017
@@ -1702,8 +1702,8 @@ static bool IsInputCompatibleWithStandar
   return true;
 break;
   default:
-llvm_unreachable("Cannot decide whether language standard and "
-"input file kind are compatible!");
+// For other inputs, accept (and ignore) all -std= values.
+return true;
   }
   return false;
 }

Added: cfe/trunk/test/Driver/unknown-std.S
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/unknown-std.S?rev=295122&view=auto
==
--- cfe/trunk/test/Driver/unknown-std.S (added)
+++ cfe/trunk/test/Driver/unknown-std.S Tue Feb 14 17:41:38 2017
@@ -0,0 +1,2 @@
+// RUN: %clang -std=c++11 %s -E -o /dev/null 2>&1 | FileCheck %s --allow-empty
+// CHECK-NOT: error


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

r295123 - [VLA] Handle VLA size expression in a full-expression context.

[Tim Shen via cfe-commits]

Author: timshen
Date: Tue Feb 14 17:46:37 2017
New Revision: 295123

URL: http://llvm.org/viewvc/llvm-project?rev=295123&view=rev
Log:
[VLA] Handle VLA size expression in a full-expression context.

Summary: Previously the cleanups (e.g. dtor calls) are inserted into the
outer scope (e.g. function body scope), instead of it's own scope. After
the fix, the cleanups are inserted right after getting the size value.

This fixes pr30306.

Reviewers: rsmith

Subscribers: cfe-commits

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

Added:
cfe/trunk/test/Sema/pr30306.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/lib/Sema/TreeTransform.h

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=295123&r1=295122&r2=295123&view=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Tue Feb 14 17:46:37 2017
@@ -3864,6 +3864,8 @@ void Sema::InstantiateFunctionDefinition
 if (Body.isInvalid())
   Function->setInvalidDecl();
 
+// FIXME: finishing the function body while in an expression evaluation
+// context seems wrong. Investigate more.
 ActOnFinishFunctionBody(Function, Body.get(),
 /*IsInstantiation=*/true);
 

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=295123&r1=295122&r2=295123&view=diff
==
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Tue Feb 14 17:46:37 2017
@@ -4603,8 +4603,15 @@ TreeTransform::TransformVariabl
   if (ElementType.isNull())
 return QualType();
 
-  ExprResult SizeResult
-= getDerived().TransformExpr(T->getSizeExpr());
+  ExprResult SizeResult;
+  {
+EnterExpressionEvaluationContext Context(SemaRef,
+ Sema::PotentiallyEvaluated);
+SizeResult = getDerived().TransformExpr(T->getSizeExpr());
+  }
+  if (SizeResult.isInvalid())
+return QualType();
+  SizeResult = SemaRef.ActOnFinishFullExpr(SizeResult.get());
   if (SizeResult.isInvalid())
 return QualType();
 

Added: cfe/trunk/test/Sema/pr30306.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pr30306.cpp?rev=295123&view=auto
==
--- cfe/trunk/test/Sema/pr30306.cpp (added)
+++ cfe/trunk/test/Sema/pr30306.cpp Tue Feb 14 17:46:37 2017
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -emit-llvm < %s | FileCheck %s
+
+struct A { A(int); ~A(); };
+int f(const A &);
+// CHECK: call void @_ZN1AC1Ei
+// CHECK-NEXT: call i32 @_Z1fRK1A
+// CHECK-NEXT: call void @_ZN1AD1Ev
+// CHECK: call void @_ZN1AC1Ei
+// CHECK-NEXT: call i32 @_Z1fRK1A
+// CHECK-NEXT: call void @_ZN1AD1Ev
+template void g() {
+  int a[f(3)];
+  int b[f(3)];
+}
+int main() { g(); }


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

[PATCH] D24333: [VLA] Handle VLA size expression in a full-expression context.

[Tim Shen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL295123: [VLA] Handle VLA size expression in a 
full-expression context. (authored by timshen).

Changed prior to commit:
  https://reviews.llvm.org/D24333?vs=86377&id=88464#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D24333

Files:
  cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/test/Sema/pr30306.cpp


Index: cfe/trunk/test/Sema/pr30306.cpp
===
--- cfe/trunk/test/Sema/pr30306.cpp
+++ cfe/trunk/test/Sema/pr30306.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -emit-llvm < %s | FileCheck %s
+
+struct A { A(int); ~A(); };
+int f(const A &);
+// CHECK: call void @_ZN1AC1Ei
+// CHECK-NEXT: call i32 @_Z1fRK1A
+// CHECK-NEXT: call void @_ZN1AD1Ev
+// CHECK: call void @_ZN1AC1Ei
+// CHECK-NEXT: call i32 @_Z1fRK1A
+// CHECK-NEXT: call void @_ZN1AD1Ev
+template void g() {
+  int a[f(3)];
+  int b[f(3)];
+}
+int main() { g(); }
Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3864,6 +3864,8 @@
 if (Body.isInvalid())
   Function->setInvalidDecl();
 
+// FIXME: finishing the function body while in an expression evaluation
+// context seems wrong. Investigate more.
 ActOnFinishFunctionBody(Function, Body.get(),
 /*IsInstantiation=*/true);
 
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -4603,8 +4603,15 @@
   if (ElementType.isNull())
 return QualType();
 
-  ExprResult SizeResult
-= getDerived().TransformExpr(T->getSizeExpr());
+  ExprResult SizeResult;
+  {
+EnterExpressionEvaluationContext Context(SemaRef,
+ Sema::PotentiallyEvaluated);
+SizeResult = getDerived().TransformExpr(T->getSizeExpr());
+  }
+  if (SizeResult.isInvalid())
+return QualType();
+  SizeResult = SemaRef.ActOnFinishFullExpr(SizeResult.get());
   if (SizeResult.isInvalid())
 return QualType();
 


Index: cfe/trunk/test/Sema/pr30306.cpp
===
--- cfe/trunk/test/Sema/pr30306.cpp
+++ cfe/trunk/test/Sema/pr30306.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -x c++ -emit-llvm < %s | FileCheck %s
+
+struct A { A(int); ~A(); };
+int f(const A &);
+// CHECK: call void @_ZN1AC1Ei
+// CHECK-NEXT: call i32 @_Z1fRK1A
+// CHECK-NEXT: call void @_ZN1AD1Ev
+// CHECK: call void @_ZN1AC1Ei
+// CHECK-NEXT: call i32 @_Z1fRK1A
+// CHECK-NEXT: call void @_ZN1AD1Ev
+template void g() {
+  int a[f(3)];
+  int b[f(3)];
+}
+int main() { g(); }
Index: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3864,6 +3864,8 @@
 if (Body.isInvalid())
   Function->setInvalidDecl();
 
+// FIXME: finishing the function body while in an expression evaluation
+// context seems wrong. Investigate more.
 ActOnFinishFunctionBody(Function, Body.get(),
 /*IsInstantiation=*/true);
 
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -4603,8 +4603,15 @@
   if (ElementType.isNull())
 return QualType();
 
-  ExprResult SizeResult
-= getDerived().TransformExpr(T->getSizeExpr());
+  ExprResult SizeResult;
+  {
+EnterExpressionEvaluationContext Context(SemaRef,
+ Sema::PotentiallyEvaluated);
+SizeResult = getDerived().TransformExpr(T->getSizeExpr());
+  }
+  if (SizeResult.isInvalid())
+return QualType();
+  SizeResult = SemaRef.ActOnFinishFullExpr(SizeResult.get());
   if (SizeResult.isInvalid())
 return QualType();
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D29972: Make Lit tests C++11 compatible - accessible destructors

[Charles Li via Phabricator via cfe-commits]

tigerleapgorge created this revision.
Herald added a subscriber: mehdi_amini.

I am continuing to make Lit tests C++11 compatible.
This patch contains 2 tests previously in https://reviews.llvm.org/D20710.

In both tests, I have made the base class destructors “protected” so they are 
accessible to derived classes.
In C++11, an inaccessible destructor is considered implicitly deleted and this 
causes the following type of compilation errors.

  f.h:82:13: error: deleted function '~C3' cannot override a non-deleted 
function
  C3:
  ^
  f.h:61:15: note: overridden virtual function is here
virtual ~
^
  f.h:83:15: note: destructor of 'C3' is implicitly deleted because base class 
'(anonymous namespace)::AAA' has an inaccessible destructor
AAA {
^


https://reviews.llvm.org/D29972

Files:
  test/CodeGenCXX/debug-info-use-after-free.cpp
  test/CodeGenCXX/dynamic-cast-hint.cpp


Index: test/CodeGenCXX/dynamic-cast-hint.cpp
===
--- test/CodeGenCXX/dynamic-cast-hint.cpp
+++ test/CodeGenCXX/dynamic-cast-hint.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin12 -emit-llvm -o - %s | 
FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin12 -emit-llvm -std=c++98 -o - %s 
| FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin12 -emit-llvm -std=c++11 -o - %s 
| FileCheck %s
 
-class A { virtual ~A() {} };
-class B { virtual ~B() {} };
+class A { protected: virtual ~A() {} };
+class B { protected: virtual ~B() {} };
 
 class C : A { char x; };
 class D : public A { short y; };
Index: test/CodeGenCXX/debug-info-use-after-free.cpp
===
--- test/CodeGenCXX/debug-info-use-after-free.cpp
+++ test/CodeGenCXX/debug-info-use-after-free.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple 
-emit-llvm-only %s
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple 
-emit-llvm-only -std=c++98 %s
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple 
-emit-llvm-only -std=c++11 %s
 // Check that we don't crash.
 // PR12305, PR12315
 
@@ -233,6 +235,7 @@
 namespace {
 class
 AAA {
+protected:
   virtual ~
   AAA () {
   }};


Index: test/CodeGenCXX/dynamic-cast-hint.cpp
===
--- test/CodeGenCXX/dynamic-cast-hint.cpp
+++ test/CodeGenCXX/dynamic-cast-hint.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin12 -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin12 -emit-llvm -std=c++98 -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin12 -emit-llvm -std=c++11 -o - %s | FileCheck %s
 
-class A { virtual ~A() {} };
-class B { virtual ~B() {} };
+class A { protected: virtual ~A() {} };
+class B { protected: virtual ~B() {} };
 
 class C : A { char x; };
 class D : public A { short y; };
Index: test/CodeGenCXX/debug-info-use-after-free.cpp
===
--- test/CodeGenCXX/debug-info-use-after-free.cpp
+++ test/CodeGenCXX/debug-info-use-after-free.cpp
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple -emit-llvm-only %s
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple -emit-llvm-only -std=c++98 %s
+// RUN: %clang_cc1 -debug-info-kind=limited -triple %itanium_abi_triple -emit-llvm-only -std=c++11 %s
 // Check that we don't crash.
 // PR12305, PR12315
 
@@ -233,6 +235,7 @@
 namespace {
 class
 AAA {
+protected:
   virtual ~
   AAA () {
   }};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r295125 - Remove unused variable. No functional change.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Tue Feb 14 17:56:55 2017
New Revision: 295125

URL: http://llvm.org/viewvc/llvm-project?rev=295125&view=rev
Log:
Remove unused variable.  No functional change.

Modified:
cfe/trunk/lib/Parse/ParseDecl.cpp

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=295125&r1=295124&r2=295125&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Feb 14 17:56:55 2017
@@ -2978,7 +2978,7 @@ void Parser::ParseDeclarationSpecifiers(
 // attribute declaration and continue.
 if (NextToken().is(tok::l_paren)) {
   // Consume the __declspec identifier.
-  SourceLocation Loc = ConsumeToken();
+  ConsumeToken();
 
   // Eat the parens and everything between them.
   BalancedDelimiterTracker T(*this, tok::l_paren);


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

r295127 - [VLA] Fix the test failure on msvc by specifying the triple.

[Tim Shen via cfe-commits]

Author: timshen
Date: Tue Feb 14 18:01:12 2017
New Revision: 295127

URL: http://llvm.org/viewvc/llvm-project?rev=295127&view=rev
Log:
[VLA] Fix the test failure on msvc by specifying the triple.

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

Modified:
cfe/trunk/test/Sema/pr30306.cpp

Modified: cfe/trunk/test/Sema/pr30306.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/pr30306.cpp?rev=295127&r1=295126&r2=295127&view=diff
==
--- cfe/trunk/test/Sema/pr30306.cpp (original)
+++ cfe/trunk/test/Sema/pr30306.cpp Tue Feb 14 18:01:12 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -x c++ -emit-llvm < %s | FileCheck %s
+// RUN: %clang_cc1 -x c++ -triple x86_64-pc-linux-gnu -emit-llvm < %s | 
FileCheck %s
 
 struct A { A(int); ~A(); };
 int f(const A &);


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

[PATCH] D13330: Implement __attribute__((unique_instantiation))

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In https://reviews.llvm.org/D13330#582607, @rsmith wrote:

> I think this attribute is poorly named. Explicit instantiation definitions 
> are *already* required to be globally unique; see [temp.spec]/5.1:
>
> "For a given template and a given set of template-arguments, an explicit 
> instantiation definition shall appear at most once in a program"


So what prevents from emitting these as "strong" symbols without any attribute? 
We should have the guarantee that we have only one such copy in the program.


Repository:
  rL LLVM

https://reviews.llvm.org/D13330



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

r295139 - Refactor GCC lib directory detection to make it easier to add lib directories

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 14 19:13:54 2017
New Revision: 295139

URL: http://llvm.org/viewvc/llvm-project?rev=295139&view=rev
Log:
Refactor GCC lib directory detection to make it easier to add lib directories
that are only checked for some targets.

Modified:
cfe/trunk/lib/Driver/ToolChains.cpp

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=295139&r1=295138&r2=295139&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Tue Feb 14 19:13:54 2017
@@ -2732,45 +2732,57 @@ void Generic_GCC::GCCInstallationDetecto
 const llvm::Triple &TargetTriple, const ArgList &Args,
 const std::string &LibDir, StringRef CandidateTriple,
 bool NeedsBiarchSuffix) {
-  llvm::Triple::ArchType TargetArch = TargetTriple.getArch();
-  // There are various different suffixes involving the triple we
-  // check for. We also record what is necessary to walk from each back
-  // up to the lib directory. Specifically, the number of "up" steps
-  // in the second half of each row is 1 + the number of path separators
-  // in the first half.
-  const std::string LibAndInstallSuffixes[][2] = {
-  {"/gcc/" + CandidateTriple.str(), "/../../.."},
-
-  // Debian puts cross-compilers in gcc-cross
-  {"/gcc-cross/" + CandidateTriple.str(), "/../../.."},
-
-  {"/" + CandidateTriple.str() + "/gcc/" + CandidateTriple.str(),
-   "/../../../.."},
-
-  // The Freescale PPC SDK has the gcc libraries in
-  // /usr/lib//x.y.z so have a look there as well.
-  {"/" + CandidateTriple.str(), "/../.."},
-
-  // Ubuntu has a strange mis-matched pair of triples that this happens to
-  // match.
-  // FIXME: It may be worthwhile to generalize this and look for a second
-  // triple.
-  {"/i386-linux-gnu/gcc/" + CandidateTriple.str(), "/../../../.."}};
-
   if (TargetTriple.getOS() == llvm::Triple::Solaris) {
 scanLibDirForGCCTripleSolaris(TargetTriple, Args, LibDir, CandidateTriple,
   NeedsBiarchSuffix);
 return;
   }
 
-  // Only look at the final, weird Ubuntu suffix for i386-linux-gnu.
-  const unsigned NumLibSuffixes = (llvm::array_lengthof(LibAndInstallSuffixes) 
-
-   (TargetArch != llvm::Triple::x86));
-  for (unsigned i = 0; i < NumLibSuffixes; ++i) {
-StringRef LibSuffix = LibAndInstallSuffixes[i][0];
+  llvm::Triple::ArchType TargetArch = TargetTriple.getArch();
+  // Locations relative to the system lib directory where GCC's triple-specific
+  // directories might reside.
+  struct GCCLibSuffix {
+// Path from system lib directory to GCC triple-specific directory.
+std::string LibSuffix;
+// Path from GCC triple-specific directory back to system lib directory.
+// This is one '..' component per component in LibSuffix.
+StringRef ReversePath;
+// Whether this library suffix is relevant for the triple.
+bool Active;
+  } Suffixes[] = {
+// This is the normal place.
+{"gcc/" + CandidateTriple.str(), "../..", true},
+
+// Debian puts cross-compilers in gcc-cross.
+{"gcc-cross/" + CandidateTriple.str(), "../..", true},
+
+// The Freescale PPC SDK has the gcc libraries in
+// /usr/lib//x.y.z so have a look there as well.
+// FIXME: Only do this on Freescale triples, since some systems put a *lot*
+// of files in that location, not just GCC installation data.
+{CandidateTriple.str(), "..", true},
+
+// Natively multiarch systems sometimes put the GCC triple-specific
+// directory within their multiarch lib directory, resulting in the
+// triple appearing twice.
+{CandidateTriple.str() + "/gcc/" + CandidateTriple.str(), "../../..", 
true},
+
+// Deal with cases (on Ubuntu) where the system architecture could be i386
+// but the GCC target architecture could be (say) i686.
+// FIXME: It may be worthwhile to generalize this and look for a second
+// triple.
+{"i386-linux-gnu/gcc/" + CandidateTriple.str(), "../../..",
+  TargetArch == llvm::Triple::x86}
+  };
+
+  for (auto &Suffix : Suffixes) {
+if (!Suffix.Active)
+  continue;
+
+StringRef LibSuffix = Suffix.LibSuffix;
 std::error_code EC;
 for (vfs::directory_iterator
- LI = D.getVFS().dir_begin(LibDir + LibSuffix, EC),
+ LI = D.getVFS().dir_begin(LibDir + "/" + LibSuffix, EC),
  LE;
  !EC && LI != LE; LI = LI.increment(EC)) {
   StringRef VersionText = llvm::sys::path::filename(LI->getName());
@@ -2792,9 +2804,8 @@ void Generic_GCC::GCCInstallationDetecto
   // FIXME: We hack together the directory name here instead of
   // using LI to ensure stable path separators across Windows and
   // Linux.
-  GCCInstallPath =
-  LibDir + LibAndInstallSuffixes[i][0] + "/" + Ver

r295140 - Don't look for GCC versions in /usr/lib/ except when is a

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Feb 14 19:16:48 2017
New Revision: 295140

URL: http://llvm.org/viewvc/llvm-project?rev=295140&view=rev
Log:
Don't look for GCC versions in /usr/lib/ except when  is a
freescale triple.

On multiarch systems, this previously caused us to stat every file in
/usr/lib/ (typically several thousand files). This change halves
the runtime of a clang invocation on an empty file on my system.

Modified:
cfe/trunk/lib/Driver/ToolChains.cpp

Modified: cfe/trunk/lib/Driver/ToolChains.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains.cpp?rev=295140&r1=295139&r2=295140&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains.cpp Tue Feb 14 19:16:48 2017
@@ -2757,10 +2757,11 @@ void Generic_GCC::GCCInstallationDetecto
 {"gcc-cross/" + CandidateTriple.str(), "../..", true},
 
 // The Freescale PPC SDK has the gcc libraries in
-// /usr/lib//x.y.z so have a look there as well.
-// FIXME: Only do this on Freescale triples, since some systems put a *lot*
-// of files in that location, not just GCC installation data.
-{CandidateTriple.str(), "..", true},
+// /usr/lib//x.y.z so have a look there as well. Only do
+// this on Freescale triples, though, since some systems put a *lot* of
+// files in that location, not just GCC installation data.
+{CandidateTriple.str(), "..",
+  TargetTriple.getVendor() == llvm::Triple::Freescale},
 
 // Natively multiarch systems sometimes put the GCC triple-specific
 // directory within their multiarch lib directory, resulting in the


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

Re: [cfe-commits] r164177 - in /cfe/trunk: lib/Driver/ test/Driver/ test/Driver/Inputs/freescale_ppc_tree/ test/Driver/Inputs/freescale_ppc_tree/lib/ test/Driver/Inputs/freescale_ppc_tree/usr/ test/Dr

[Richard Smith via cfe-commits]

On 9 February 2017 at 16:15, Hal Finkel via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On 02/09/2017 04:58 PM, Chandler Carruth wrote:
>
> On Thu, Feb 9, 2017 at 2:46 PM Tobias von Koch 
> wrote:
>
>> On Wed, Feb 8, 2017 at 7:21 PM, Chandler Carruth 
>> wrote:
>>
>>
>> +// The Freescale PPC SDK has the gcc libraries in
>> +// /usr/lib//x.y.z so have a look there as well.
>> +"/" + CandidateTriple.str(),
>>
>>
>> So, this is really bad it turns out.
>>
>> We use this directory to walk every installed GCC version. But because
>> this is just a normal lib directory on many systems (ever Debian and Ubuntu
>> system for example) this goes very badly. It goes even more badly because
>> of the (questionable) design of LLVM's directory iterator:
>>
>>
>> Wow, this is pretty bad, but it really sounds like the iterator should be
>> fixed rather than trying to hack around it.
>>
>
> I mean, we should.
>
> But even then, walking the entire directory seems bad too... See below.
>
>
> Agreed. FWIW, it looks like LLVM's directory iterators stat lazily
> (although doing an equality comparison will cause them to stat). Is going
> through Clang's VFS layer causing the eager stating somehow?
>
>
>
>> Doesn't this happen for the other directories as well (which, admittedly,
>> will have less entries)?
>>
>
> The *only* entries in the other directories are the actual installed GCC
> toolchains though, so walking them makes a lot of sense. The tricky thing
> is that this isn't a gcc-specific directory.
>
> I suspect the fix should be to not use this base path as part of the walk
> to discover GCC toolchains, and instead to hard code the specific toolchain
> patterns on this specific platform.
>
> Or we could do the walk, but only when actually on the NXP/Freescale Power
> platform where this is necessary to find GCC installations.
>
>
> Given that we don't have a platform on which to test right now, I think
> that this second option sounds best. Only add those directories to the
> search path when -fsl- is in the triple (or something like that).
>

Done in r295139.


>  -Hal
>
>
> Both of those would seem reasonable. Fixing the directory iterator would
> be icing on the cake IMO. =D
>
>
> --
> Hal Finkel
> Lead, Compiler Technology and Programming Languages
> Leadership Computing Facility
> Argonne National Laboratory
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r291955 - PR31606: Generalize our tentative DR resolution for inheriting copy/move

[Akira Hatanaka via cfe-commits]

Hi Richard,

It looks like this commit causes an assertion failure when the following code 
is compiled:

$ cat test1.cpp
template
struct S3 {
};

template
struct S2 {
  S2(S3 &&);
};

template
struct S1 : S2 {
  using S2::S2;
  S1();
};

template
struct S0 {
  S0();
  S0(S0&&) = default;
  S1 m1;
};

void foo1() {
  S0 s0;
}

$ clang++  -std=c++1z test1.cpp -c -o /dev/null

Assertion failed: (Loc.isValid() && "point of instantiation must be valid!"),

What’s the right way to fix this?

The patch I have now just checks whether PointOfInstantiation.isValid() returns 
true before calling Spec->setPointOfInstantiation at 
SemaTemplateInstantiate.cpp:1938.

> On Jan 13, 2017, at 12:46 PM, Richard Smith via cfe-commits 
>  wrote:
> 
> Author: rsmith
> Date: Fri Jan 13 14:46:54 2017
> New Revision: 291955
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=291955&view=rev
> Log:
> PR31606: Generalize our tentative DR resolution for inheriting copy/move
> constructors to better match the pre-P0136R1 behavior.
> 
> Modified:
>cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>cfe/trunk/lib/Sema/SemaOverload.cpp
>cfe/trunk/test/CXX/dcl.dcl/basic.namespace/namespace.udecl/p15.cpp
>cfe/trunk/test/CXX/dcl.decl/dcl.init/dcl.init.aggr/p1.cpp
>cfe/trunk/test/CXX/drs/dr16xx.cpp
>cfe/trunk/test/CXX/drs/dr19xx.cpp
>cfe/trunk/test/CXX/special/class.inhctor/p1.cpp
>cfe/trunk/test/CXX/special/class.inhctor/p3.cpp
>cfe/trunk/test/CXX/special/class.inhctor/p7.cpp
>cfe/trunk/test/SemaCXX/cxx11-inheriting-ctors.cpp
>cfe/trunk/test/SemaTemplate/cxx1z-using-declaration.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=291955&r1=291954&r2=291955&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Jan 13 14:46:54 
> 2017
> @@ -3344,8 +3344,8 @@ def note_ovl_candidate : Note<"candidate
> def note_ovl_candidate_inherited_constructor : Note<
> "constructor from base class %0 inherited here">;
> def note_ovl_candidate_inherited_constructor_slice : Note<
> -"constructor inherited from base class cannot be used to initialize from 
> "
> -"an argument of the derived class type">;
> +"candidate %select{constructor|template}0 ignored: "
> +"inherited constructor cannot be used to %select{copy|move}1 object">;
> def note_ovl_candidate_illegal_constructor : Note<
> "candidate %select{constructor|template}0 ignored: "
> "instantiation %select{takes|would take}0 its own class type by value">;
> 
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=291955&r1=291954&r2=291955&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Fri Jan 13 14:46:54 2017
> @@ -5944,6 +5944,28 @@ Sema::AddOverloadCandidate(FunctionDecl
>   Candidate.FailureKind = ovl_fail_illegal_constructor;
>   return;
> }
> +
> +// C++ [over.match.funcs]p8: (proposed DR resolution)
> +//   A constructor inherited from class type C that has a first parameter
> +//   of type "reference to P" (including such a constructor instantiated
> +//   from a template) is excluded from the set of candidate functions 
> when
> +//   constructing an object of type cv D if the argument list has exactly
> +//   one argument and D is reference-related to P and P is 
> reference-related
> +//   to C.
> +auto *Shadow = dyn_cast(FoundDecl.getDecl());
> +if (Shadow && Args.size() == 1 && Constructor->getNumParams() >= 1 &&
> +Constructor->getParamDecl(0)->getType()->isReferenceType()) {
> +  QualType P = Constructor->getParamDecl(0)->getType()->getPointeeType();
> +  QualType C = Context.getRecordType(Constructor->getParent());
> +  QualType D = Context.getRecordType(Shadow->getParent());
> +  SourceLocation Loc = Args.front()->getExprLoc();
> +  if ((Context.hasSameUnqualifiedType(P, C) || IsDerivedFrom(Loc, P, C)) 
> &&
> +  (Context.hasSameUnqualifiedType(D, P) || IsDerivedFrom(Loc, D, 
> P))) {
> +Candidate.Viable = false;
> +Candidate.FailureKind = ovl_fail_inhctor_slice;
> +return;
> +  }
> +}
>   }
> 
>   unsigned NumParams = Proto->getNumParams();
> @@ -6016,31 +6038,6 @@ Sema::AddOverloadCandidate(FunctionDecl
> }
>   }
> 
> -  // C++ [over.best.ics]p4+: (proposed DR resolution)
> -  //   If the target is the first parameter of an inherited constructor when
> -  //   constructing an object of type C with an argument list that has 
> exactly
> -  //   one expression, an implicit conversion sequence cannot 

Re: r293199 - Turn on -Wblock-capture-autoreleasing by default.

[Akira Hatanaka via cfe-commits]

Yes, you are correct: clang implicitly marks indirect parameters with 
__autoreleasing.

The whole purpose of Wblock-capture-autoreleasing (which was committed in 
r285031) is to inform the users that clang has implicitly marked an 
out-parameter as __autoreleasing. clang doesn’t warn if the user explicitly 
marks the parameter __autoreleasing.

> On Feb 13, 2017, at 8:09 AM, Nico Weber  wrote:
> 
> If I'm understanding you correctly, you're saying that clang implicitly marks 
> the indirect NSError** parameter with __autoreleasing, so there's no bug 
> here. Is that correct? If so, should Wblock-capture-autoreleasing just not 
> fire on parameters that are already implicitly marked as __autoreleasing?
> 
> On Mon, Feb 13, 2017 at 1:54 AM, Akira Hatanaka  > wrote:
> Hi Nico,
> 
> The documentation might confuse people now that the warning is on by default, 
> but it’s correct as clang still qualifies indirect parameters with 
> __autoreleasing. We had to add this warning and turn it on by default because 
> a lot of people capture indirect parameters in their code not realizing the 
> object assigned might get released by an autorelease pool.
> 
> Perhaps we can change the warning message or the documentation to make it 
> clear that the parameter has been implicitly marked __autoreleasing 
> regardless.
> 
>> On Feb 11, 2017, at 11:54 AM, Nico Weber > > wrote:
>> 
>> Hi Akira,
>> 
>> this fires in internal chrome/ios code like so:
>> 
>> somefile.m: error: block captures an autoreleasing out-parameter, which may 
>> result in use-after-free bugs [-Werror,-Wblock-capture-autoreleasing]
>>   if (error) {
>>   ^
>> somefile.m: note: declare the parameter __autoreleasing explicitly to 
>> suppress this warning
>> - (NSDictionary *)fooWithError:(NSError **)error {
>>   ^
>>   __autoreleasing
>> somefile.m: note: declare the parameter __strong or capture a __block 
>> __strong variable to keep values alive across autorelease pools
>> 
>> 
>> A colleague points out that 
>> http://clang.llvm.org/docs/AutomaticReferenceCounting.html#indirect-parameters
>>  
>> 
>>  suggests NSError ** should "be implicitly qualified with __autoreleasing." 
>> Is that a bug in the implementation of the warning, is 
>> AutomaticReferenceCounting.html incorrect, or are we just misunderstanding 
>> that document?
>> 
>> Thanks,
>> Nico
>> 
>> On Thu, Jan 26, 2017 at 1:51 PM, Akira Hatanaka via cfe-commits 
>> mailto:cfe-commits@lists.llvm.org>> wrote:
>> Author: ahatanak
>> Date: Thu Jan 26 12:51:10 2017
>> New Revision: 293199
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=293199&view=rev 
>> 
>> Log:
>> Turn on -Wblock-capture-autoreleasing by default.
>> 
>> Turning on the warning by default helps the users as it's a common
>> mistake to capture out-parameters in a block without ensuring the object
>> assigned doesn't get released.
>> 
>> rdar://problem/30200058 <>
>> 
>> Modified:
>> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> cfe/trunk/test/SemaObjC/arc.m
>> 
>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=293199&r1=293198&r2=293199&view=diff
>>  
>> 
>> ==
>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jan 26 12:51:10 
>> 2017
>> @@ -5185,7 +5185,7 @@ def err_arc_inconsistent_property_owners
>>  def warn_block_capture_autoreleasing : Warning<
>>"block captures an autoreleasing out-parameter, which may result in "
>>"use-after-free bugs">,
>> -  InGroup, DefaultIgnore;
>> +  InGroup;
>>  def note_declare_parameter_autoreleasing : Note<
>>"declare the parameter __autoreleasing explicitly to suppress this 
>> warning">;
>>  def note_declare_parameter_strong : Note<
>> 
>> Modified: cfe/trunk/test/SemaObjC/arc.m
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc.m?rev=293199&r1=293198&r2=293199&view=diff
>>  
>> 
>> ==
>> --- cfe/trunk/test/SemaObjC/arc.m (original)
>> +++ cfe/trunk/test/SemaObjC/arc.m Thu Jan 26 12:51:10 2017
>> @@ -1,4 +1,4 @@
>> -// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak 
>> -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-r

[PATCH] D29599: Clang Changes for alloc_align

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

My question probably wasn't clear, but I wasn't sure how template functions in 
general (not just member functions) should be handled.

I see a warning when the following function is compiled:

  template
  T  __attribute__((alloc_align(1))) foo0(int a) {
typedef typename std::remove_pointer::type T2;
return new T2();
  }
  
  void foo1() {
foo0(1);
  }

clang complains that the attribute only applies to functions returning pointers 
or references, but foo0 does return a pointer. Is that intended?


https://reviews.llvm.org/D29599



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