[PATCH] D43908: [RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 136471.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Added an error-by-default diagnostic (just like the existing 
warn_cxx_ms_struct) for this case, trigger it on all targets other than mingw 
(where the situation is quite likely to occur, and GCC handles it silently).

@compnerd, do you like this better?


https://reviews.llvm.org/D43908

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/AST/RecordLayoutBuilder.cpp
  test/CodeGen/ms_struct-long-double.c


Index: test/CodeGen/ms_struct-long-double.c
===
--- /dev/null
+++ test/CodeGen/ms_struct-long-double.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-windows-gnu 
-fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-linux -fdump-record-layouts 
-Wno-incompatible-ms-struct %s | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm-only -triple i686-linux 
-fdump-record-layouts %s 2>&1 | FileCheck %s -check-prefix=ERROR
+
+struct ldb_struct {
+  char c;
+  long double ldb;
+} __attribute__((__ms_struct__));
+
+struct ldb_struct a;
+
+// CHECK: 0 | struct ldb_struct
+// CHECK-NEXT:0 |   char c
+// CHECK-NEXT:4 |   long double ldb
+// CHECK-NEXT:  | [sizeof=16, align=4]
+
+// ERROR: error: ms_struct may not produce Microsoft-compatible layouts with 
fundamental data types with sizes that aren't a power of two
Index: lib/AST/RecordLayoutBuilder.cpp
===
--- lib/AST/RecordLayoutBuilder.cpp
+++ lib/AST/RecordLayoutBuilder.cpp
@@ -1752,10 +1752,32 @@
   QualType T = Context.getBaseElementType(D->getType());
   if (const BuiltinType *BTy = T->getAs()) {
 CharUnits TypeSize = Context.getTypeSizeInChars(BTy);
-assert(
-(llvm::isPowerOf2_64(TypeSize.getQuantity()) ||
- Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
-"Non PowerOf2 size outside of GNU mode");
+
+if (!llvm::isPowerOf2_64(TypeSize.getQuantity())) {
+  assert(
+  !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment() 
&&
+  "Non PowerOf2 size in MSVC mode");
+  // Base types with sizes that aren't a power of two don't work
+  // with the layout rules for MS structs. This isn't an issue in
+  // MSVC itself since there are no such base data types there.
+  // On e.g. x86_32 mingw and linux, long double is 12 bytes though.
+  // Any structs involving that data type obviously can't be ABI
+  // compatible with MSVC regardless of how it is laid out.
+
+  // Since ms_struct can be mass enabled (via a pragma or via the
+  // -mms-bitfields command line parameter), this can trigger for
+  // structs that don't actually need MSVC compatibility, so we
+  // need to be able to sidestep the ms_struct layout for these types.
+
+  // Since the combination of -mms-bitfields together with structs
+  // like max_align_t (which contains a long double) for mingw is
+  // quite comon (and GCC handles it silently), just handle it
+  // silently there. For other targets that have ms_struct enabled
+  // (most probably via a pragma or attribute), trigger a diagnostic
+  // that defaults to an error.
+  if (!Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+Diag(D->getLocation(), diag::warn_npot_ms_struct);
+}
 if (TypeSize > FieldAlign &&
 llvm::isPowerOf2_64(TypeSize.getQuantity()))
   FieldAlign = TypeSize;
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -759,6 +759,10 @@
   Warning<"ms_struct may not produce Microsoft-compatible layouts for classes "
   "with base classes or virtual functions">,
   DefaultError, InGroup;
+def warn_npot_ms_struct :
+  Warning<"ms_struct may not produce Microsoft-compatible layouts with 
fundamental "
+  "data types with sizes that aren't a power of two">,
+  DefaultError, InGroup;
 def err_section_conflict : Error<"%0 causes a section type conflict with %1">;
 def err_no_base_classes : Error<"invalid use of '__super', %0 has no base 
classes">;
 def err_invalid_super_scope : Error<"invalid use of '__super', "


Index: test/CodeGen/ms_struct-long-double.c
===
--- /dev/null
+++ test/CodeGen/ms_struct-long-double.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-windows-gnu -fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-linux -fdump-record-layouts -Wno-incompatible-ms-struct %s | FileCheck 

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

[András Leitereg via Phabricator via cfe-commits]

leanil created this revision.
leanil added reviewers: dcoughlin, xazax.hun, NoQ.
Herald added subscribers: a.sidorin, rnkovacs, szepet.
Herald added a reviewer: george.karpenkov.

This will handle those platforms that don't have 8-bit chars.
This is a follow up fix to review https://reviews.llvm.org/D41384, which has 
been committed since.


https://reviews.llvm.org/D43928

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp


Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -514,7 +514,7 @@
  *Source = CE->getArg(1)->IgnoreImpCasts();
   if (const auto *DeclRef = dyn_cast(Target))
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  uint64_t ArraySize = 
BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {
 if (ArraySize >= String->getLength() + 1)
   return;


Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -514,7 +514,7 @@
  *Source = CE->getArg(1)->IgnoreImpCasts();
   if (const auto *DeclRef = dyn_cast(Target))
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  uint64_t ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {
 if (ArraySize >= String->getLength() + 1)
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

[András Leitereg via Phabricator via cfe-commits]

leanil updated this revision to Diff 136474.
leanil added a comment.

`getQuantity()` returns a signed type


https://reviews.llvm.org/D43928

Files:
  lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp


Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -514,7 +514,7 @@
  *Source = CE->getArg(1)->IgnoreImpCasts();
   if (const auto *DeclRef = dyn_cast(Target))
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  auto ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {
 if (ArraySize >= String->getLength() + 1)
   return;


Index: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
===
--- lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
+++ lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp
@@ -514,7 +514,7 @@
  *Source = CE->getArg(1)->IgnoreImpCasts();
   if (const auto *DeclRef = dyn_cast(Target))
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  auto ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {
 if (ArraySize >= String->getLength() + 1)
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Frederic Tingaud via Phabricator via cfe-commits]

ftingaud added inline comments.



Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion",
+ getDefaultMinimumLanguageVersion())) 
{}

aaron.ballman wrote:
> Why is this is a user-facing option?
> 
> If it needs to be a user-facing option, you also need to implement an 
> override for `storeOptions()` as well.
As the rule was very customizable, I also made the c++ version customizable. 
But the option is only useful if a user has a custom make_unique that needs 
c++14 (uses std::make_unique internally) and multiple versions of C++ in their 
codeline. I agree this is a rather specific usecase. I can remove it and make 
the code simpler if it is your recommendation.



Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:25
+const std::string MakeUniqueCheck::getDefaultMinimumLanguageVersion() const {
+  return Options.get("MakeSmartPtrFunction", "").empty() ? "c++14" : "c++11";
+}

aaron.ballman wrote:
> What is this option? Why is this returning a string literal rather than 
> something less error-prone like an enumeration?
I made the MinimumLanguageVersion a string literal to make it customizable by 
the user. If we remove the customization, we can switch to an enum or a boolean.


https://reviews.llvm.org/D43766



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

[PATCH] D43731: [clang-format] Fix documentation for SpaceAfterCStyleCast option

[Krasimir Georgiev via Phabricator via cfe-commits]

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

This rst file is generated from include/clang/Format/Format.h using 
docs/tools/dump_format_style.py. Update it there and run the tool to regenerate 
the .rst file.


Repository:
  rC Clang

https://reviews.llvm.org/D43731



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

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/index/CanonicalIncludes.cpp:83
+  static const std::vector> SymbolMap = {
+  // Map symbols in  to their preferred includes.
+  {"std::basic_filebuf", ""},

Looks like the list only contains stream-related symbols, there are some 
symbols in `` not covered, is it intended? 

Maybe we keep the order of this map the same as the one listed in 
http://en.cppreference.com/w/cpp/header/iosfwd? That would make it easier to 
see the difference?



Comment at: unittests/clangd/SymbolCollectorTests.cpp:561
+  class no_map {};
+  class ios {};
+  class ostream {};

The STL implementation of `ios` is a typedef `typedef basic_ios  ios; `, 
I think we should make the test align with it?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869



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

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/CanonicalIncludes.cpp:83
+  static const std::vector> SymbolMap = {
+  // Map symbols in  to their preferred includes.
+  {"std::basic_filebuf", ""},

hokein wrote:
> Looks like the list only contains stream-related symbols, there are some 
> symbols in `` not covered, is it intended? 
> 
> Maybe we keep the order of this map the same as the one listed in 
> http://en.cppreference.com/w/cpp/header/iosfwd? That would make it easier to 
> see the difference?
Although `allocator` could be exposed by , it's not defined in 
, so we don't need to handle it specially.

I am sorting the symbols by their canonical headers, which I think would make 
it easier to check the header mapping.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:561
+  class no_map {};
+  class ios {};
+  class ostream {};

hokein wrote:
> The STL implementation of `ios` is a typedef `typedef basic_ios  ios; 
> `, I think we should make the test align with it?
The symbol mapping doesn't make a difference between symbol types, and symbol 
kind test is not in the scope of this patch,  so I think this wouldn't be 
necessary here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:

> I need a bit more context because I'm unfamiliar with `absl`. What is this 
> module's intended use?


As `absl` has been open-sourced (https://github.com/abseil/abseil-cpp), I think 
there will be more absl-related checks contributed from google or external 
contributors in the future, so it make sense to create a new module.

@niko, could you please refine the code style of this patch to follow the LLVM 
code standard 

 (especially the variable name, should be camel case "VariableName")?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

r326426 - [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via cfe-commits]

Author: typz
Date: Thu Mar  1 02:09:13 2018
New Revision: 326426

URL: http://llvm.org/viewvc/llvm-project?rev=326426&view=rev
Log:
[clang-format] Add SpaceBeforeColon option

Summary:
When disabled, this option allows removing the space before colon,
making it act more like the semi-colon. When enabled (default), the
current behavior is not affected.

This mostly affects C++11 loop, initializer list, inheritance list and
container literals:

  class Foo: Bar {}
  Foo::Foo(): a(a) {}
  for (auto i: myList) {}
  f({a: 1, b: 2, c: 3});

Reviewers: krasimir, djasper

Reviewed By: djasper

Subscribers: xvallspl, teemperor, karies, cfe-commits, klimek

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

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=326426&r1=326425&r2=326426&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Thu Mar  1 02:09:13 2018
@@ -1681,6 +1681,23 @@ the configuration (without a prefix: ``A
  int a = 5; vs. int a=5;
  a += 42a+=42;
 
+**SpaceBeforeCtorInitializerColon** (``bool``)
+  If ``false``, spaces will be removed before constructor initializer
+  colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ Foo::Foo() : a(a) {}   Foo::Foo(): a(a) {}
+
+**SpaceBeforeInheritanceColon** (``bool``)
+  If ``false``, spaces will be removed before inheritance colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ class Foo : Bar {} vs. class Foo: Bar {}
+
 **SpaceBeforeParens** (``SpaceBeforeParensOptions``)
   Defines in which cases to put a space before opening parentheses.
 
@@ -1725,6 +1742,15 @@ the configuration (without a prefix: ``A
 
 
 
+**SpaceBeforeRangeBasedForLoopColon** (``bool``)
+  If ``false``, spaces will be removed before range-based for loop
+  colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ for (auto v : values) {}   vs. for(auto v: values) {}
+
 **SpaceInEmptyParentheses** (``bool``)
   If ``true``, spaces may be inserted into ``()``.
 

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=326426&r1=326425&r2=326426&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Thu Mar  1 02:09:13 2018
@@ -1525,6 +1525,21 @@ struct FormatStyle {
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
+  /// \brief If ``false``, spaces will be removed before constructor 
initializer
+  /// colon.
+  /// \code
+  ///true:  false:
+  ///Foo::Foo() : a(a) {}   Foo::Foo(): a(a) {}
+  /// \endcode
+  bool SpaceBeforeCtorInitializerColon;
+
+  /// \brief If ``false``, spaces will be removed before inheritance colon.
+  /// \code
+  ///true:  false:
+  ///class Foo : Bar {} vs. class Foo: Bar {}
+  /// \endcode
+  bool SpaceBeforeInheritanceColon;
+
   /// \brief Different ways to put a space before opening parentheses.
   enum SpaceBeforeParensOptions {
 /// Never put a space before opening parentheses.
@@ -1563,6 +1578,14 @@ struct FormatStyle {
   /// \brief Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// \brief If ``false``, spaces will be removed before range-based for loop
+  /// colon.
+  /// \code
+  ///true:  false:
+  ///for (auto v : values) {}   vs. for(auto v: values) {}
+  /// \endcode
+  bool SpaceBeforeRangeBasedForLoopColon;
+
   /// \brief If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
@@ -1744,7 +1767,12 @@ struct FormatStyle {
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators 
&&
+   SpaceBeforeCtorInitializerColon ==
+   R.SpaceBeforeCtorInitializerColon &&
+   SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceBeforeRangeBasedForLoopColon ==
+   R.SpaceBeforeRangeBasedForLoopColon &&
  

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC326426: [clang-format] Add SpaceBeforeColon option (authored 
by Typz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D32525?vs=136305&id=136483#toc

Repository:
  rC Clang

https://reviews.llvm.org/D32525

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: docs/ClangFormatStyleOptions.rst
===
--- docs/ClangFormatStyleOptions.rst
+++ docs/ClangFormatStyleOptions.rst
@@ -1681,6 +1681,23 @@
  int a = 5; vs. int a=5;
  a += 42a+=42;
 
+**SpaceBeforeCtorInitializerColon** (``bool``)
+  If ``false``, spaces will be removed before constructor initializer
+  colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ Foo::Foo() : a(a) {}   Foo::Foo(): a(a) {}
+
+**SpaceBeforeInheritanceColon** (``bool``)
+  If ``false``, spaces will be removed before inheritance colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ class Foo : Bar {} vs. class Foo: Bar {}
+
 **SpaceBeforeParens** (``SpaceBeforeParensOptions``)
   Defines in which cases to put a space before opening parentheses.
 
@@ -1725,6 +1742,15 @@
 
 
 
+**SpaceBeforeRangeBasedForLoopColon** (``bool``)
+  If ``false``, spaces will be removed before range-based for loop
+  colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ for (auto v : values) {}   vs. for(auto v: values) {}
+
 **SpaceInEmptyParentheses** (``bool``)
   If ``true``, spaces may be inserted into ``()``.
 
Index: include/clang/Format/Format.h
===
--- include/clang/Format/Format.h
+++ include/clang/Format/Format.h
@@ -1525,6 +1525,21 @@
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
+  /// \brief If ``false``, spaces will be removed before constructor initializer
+  /// colon.
+  /// \code
+  ///true:  false:
+  ///Foo::Foo() : a(a) {}   Foo::Foo(): a(a) {}
+  /// \endcode
+  bool SpaceBeforeCtorInitializerColon;
+
+  /// \brief If ``false``, spaces will be removed before inheritance colon.
+  /// \code
+  ///true:  false:
+  ///class Foo : Bar {} vs. class Foo: Bar {}
+  /// \endcode
+  bool SpaceBeforeInheritanceColon;
+
   /// \brief Different ways to put a space before opening parentheses.
   enum SpaceBeforeParensOptions {
 /// Never put a space before opening parentheses.
@@ -1563,6 +1578,14 @@
   /// \brief Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// \brief If ``false``, spaces will be removed before range-based for loop
+  /// colon.
+  /// \code
+  ///true:  false:
+  ///for (auto v : values) {}   vs. for(auto v: values) {}
+  /// \endcode
+  bool SpaceBeforeRangeBasedForLoopColon;
+
   /// \brief If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
@@ -1744,7 +1767,12 @@
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
+   SpaceBeforeCtorInitializerColon ==
+   R.SpaceBeforeCtorInitializerColon &&
+   SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceBeforeRangeBasedForLoopColon ==
+   R.SpaceBeforeRangeBasedForLoopColon &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
SpacesInAngles == R.SpacesInAngles &&
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2575,8 +2575,15 @@
 return true;
   if (Right.is(tok::comma))
 return false;
-  if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
+  if (Right.is(TT_ObjCBlockLParen))
 return true;
+  if (Right.is(TT_CtorInitializerColon))
+return Style.SpaceBeforeCtorInitializerColon;
+  if (Right.is(TT_InheritanceColon) && !Style.SpaceBeforeInheritanceColon)
+return false;
+  if (Right.is(TT_RangeBasedForLoopColon) &&
+  !Style.SpaceBeforeRangeBasedForLoopColon)
+return false;
   if (Right.is(tok::colon)) {
 if (Line.First->isOneOf(tok::kw_case, tok::kw_default) ||
 !Right.getNextNonComment() || R

[PATCH] D32525: [clang-format] Add SpaceBeforeColon option

[Francois Ferrand via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326426: [clang-format] Add SpaceBeforeColon option (authored 
by Typz, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D32525

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/docs/ClangFormatStyleOptions.rst
===
--- cfe/trunk/docs/ClangFormatStyleOptions.rst
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst
@@ -1681,6 +1681,23 @@
  int a = 5; vs. int a=5;
  a += 42a+=42;
 
+**SpaceBeforeCtorInitializerColon** (``bool``)
+  If ``false``, spaces will be removed before constructor initializer
+  colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ Foo::Foo() : a(a) {}   Foo::Foo(): a(a) {}
+
+**SpaceBeforeInheritanceColon** (``bool``)
+  If ``false``, spaces will be removed before inheritance colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ class Foo : Bar {} vs. class Foo: Bar {}
+
 **SpaceBeforeParens** (``SpaceBeforeParensOptions``)
   Defines in which cases to put a space before opening parentheses.
 
@@ -1725,6 +1742,15 @@
 
 
 
+**SpaceBeforeRangeBasedForLoopColon** (``bool``)
+  If ``false``, spaces will be removed before range-based for loop
+  colon.
+
+  .. code-block:: c++
+
+ true:  false:
+ for (auto v : values) {}   vs. for(auto v: values) {}
+
 **SpaceInEmptyParentheses** (``bool``)
   If ``true``, spaces may be inserted into ``()``.
 
Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -1525,6 +1525,21 @@
   /// \endcode
   bool SpaceBeforeAssignmentOperators;
 
+  /// \brief If ``false``, spaces will be removed before constructor initializer
+  /// colon.
+  /// \code
+  ///true:  false:
+  ///Foo::Foo() : a(a) {}   Foo::Foo(): a(a) {}
+  /// \endcode
+  bool SpaceBeforeCtorInitializerColon;
+
+  /// \brief If ``false``, spaces will be removed before inheritance colon.
+  /// \code
+  ///true:  false:
+  ///class Foo : Bar {} vs. class Foo: Bar {}
+  /// \endcode
+  bool SpaceBeforeInheritanceColon;
+
   /// \brief Different ways to put a space before opening parentheses.
   enum SpaceBeforeParensOptions {
 /// Never put a space before opening parentheses.
@@ -1563,6 +1578,14 @@
   /// \brief Defines in which cases to put a space before opening parentheses.
   SpaceBeforeParensOptions SpaceBeforeParens;
 
+  /// \brief If ``false``, spaces will be removed before range-based for loop
+  /// colon.
+  /// \code
+  ///true:  false:
+  ///for (auto v : values) {}   vs. for(auto v: values) {}
+  /// \endcode
+  bool SpaceBeforeRangeBasedForLoopColon;
+
   /// \brief If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
@@ -1744,7 +1767,12 @@
SpaceAfterCStyleCast == R.SpaceAfterCStyleCast &&
SpaceAfterTemplateKeyword == R.SpaceAfterTemplateKeyword &&
SpaceBeforeAssignmentOperators == R.SpaceBeforeAssignmentOperators &&
+   SpaceBeforeCtorInitializerColon ==
+   R.SpaceBeforeCtorInitializerColon &&
+   SpaceBeforeInheritanceColon == R.SpaceBeforeInheritanceColon &&
SpaceBeforeParens == R.SpaceBeforeParens &&
+   SpaceBeforeRangeBasedForLoopColon ==
+   R.SpaceBeforeRangeBasedForLoopColon &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
SpacesInAngles == R.SpacesInAngles &&
Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2575,8 +2575,15 @@
 return true;
   if (Right.is(tok::comma))
 return false;
-  if (Right.isOneOf(TT_CtorInitializerColon, TT_ObjCBlockLParen))
+  if (Right.is(TT_ObjCBlockLParen))
 return true;
+  if (Right.is(TT_CtorInitializerColon))
+return Style.SpaceBeforeCtorInitializerColon;
+  if (Right.is(TT_InheritanceColon) && !Style.SpaceBeforeInheritanceColon)
+return false;
+  if (Right.is(TT_RangeBasedForLoopColon) &&
+  !Style.SpaceBeforeRangeBasedForLoopColon)
+return false;
   if (Right.is(tok::colon)) {
 if

[PATCH] D43108: Support for the mno-stack-arg-probe flag

[Amara Emerson via Phabricator via cfe-commits]

aemerson added a comment.

In https://reviews.llvm.org/D43108#1023300, @nruslan wrote:

> By default, stack probes are enabled (i.e., -mstack-arg-probe is the default 
> behavior) and have the size of 4K in x86.


This part what I wanted to clarify, `-mstack-probe-arg` is enabling stack 
probes if the ABI requires it only, not for other reasons like security.


Repository:
  rC Clang

https://reviews.llvm.org/D43108



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

r326432 - Revert "[analyzer] Support for naive cross translation unit analysis"

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Mar  1 04:43:39 2018
New Revision: 326432

URL: http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
Log:
Revert "[analyzer] Support for naive cross translation unit analysis"

Also revert "[analyzer] Fix a compiler warning"
This reverts commits r326323 and r326324.

Reason: the commits introduced a cyclic dependency in the build graph.
This happens to work with cmake, but breaks out internal integrate.

Removed:
cfe/trunk/test/Analysis/Inputs/ctu-chain.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/Inputs/externalFnMap.txt
cfe/trunk/test/Analysis/ctu-main.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CMakeLists.txt
cfe/trunk/test/Analysis/analyzer-config.cpp
cfe/trunk/tools/scan-build-py/README.md
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/arguments.py
cfe/trunk/tools/scan-build-py/libscanbuild/clang.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_clang.py

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=326432&r1=326431&r2=326432&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Thu Mar  1 
04:43:39 2018
@@ -308,16 +308,6 @@ private:
   /// \sa shouldDisplayNotesAsEvents
   Optional DisplayNotesAsEvents;
 
-  /// \sa getCTUDir
-  Optional CTUDir;
-
-  /// \sa getCTUIndexName
-  Optional CTUIndexName;
-
-  /// \sa naiveCTUEnabled
-  Optional NaiveCTU;
-
-
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -647,17 +637,6 @@ public:
   /// to false when unset.
   bool shouldDisplayNotesAsEvents();
 
-  /// Returns the directory containing the CTU related files.
-  StringRef getCTUDir();
-
-  /// Returns the name of the file containing the CTU index of functions.
-  StringRef getCTUIndexName();
-
-  /// Returns true when naive cross translation unit analysis is enabled.
-  /// This is an experimental feature to inline functions from another
-  /// translation units.
-  bool naiveCTUEnabled();
-
 public:
   AnalyzerOptions() :
 AnalysisStoreOpt(RegionStoreModel),

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=326432&r1=326431&r2=326432&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Thu 
Mar  1 04:43:39 2018
@@ -38,10 +38,6 @@ class CXXThisExpr;
 class MaterializeTemporaryExpr;
 class ObjCAtSynchronizedStmt;
 class ObjCForCollectionStmt;
-
-namespace cross_tu {
-class CrossTranslationUnitContext;
-}
   
 namespace ento {
 
@@ -78,8 +74,6 @@ public:
   };
 
 private:
-  cross_tu::CrossTranslationUnitContext &CTU;
-
   AnalysisManager &AMgr;
   
   AnalysisDeclContextManager &AnalysisDeclContexts;
@@ -121,9 +115,10 @@ private:
   InliningModes HowToInline;
 
 public:
-  ExprEngine(cross_tu::CrossTranslationUnitContext &CTU, AnalysisManager &mgr,
- bool gcEnabled, SetOfConstDecls *VisitedCalleesIn,
- FunctionSummariesTy *FS, InliningModes HowToInlineIn);
+  ExprEngine(AnalysisManager &mgr, bool gcEnabled,
+ SetOfConstDecls *VisitedCalleesIn,
+ FunctionSummariesTy *FS,
+ InliningModes HowToInlineIn);
 
   ~ExprEngine() override;
 
@@ -155,11 +150,6 @@ public:
 
   BugReporter& getBugReporter() { return BR; }
 
-  cross_tu::CrossTranslationUnitContext *
-  getCrossTranslationUnitContext() override {
-return &CTU;
-  }
-
   const NodeBuilderContext &getBuilderContext() {
 assert(currBldrCtx);
 return *currBldrCtx;

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEn

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Aleksei Sidorin via Phabricator via cfe-commits]

a.sidorin reopened this revision.
a.sidorin added a comment.

The changes were reverted: 
http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
Gabor, could you take a look?


Repository:
  rC Clang

https://reviews.llvm.org/D30691



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

[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

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

xazax.hun added a reviewer: ilya-biryukov.
xazax.hun added a subscriber: ilya-biryukov.
xazax.hun added a comment.

@ilya-biryukov

Could you please provide some more details where the cyclic dependency is? I 
cannot reproduce the problem and usually cmake fails when there is a cyclic 
dependency and you are building shared libraries.


Repository:
  rC Clang

https://reviews.llvm.org/D30691



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:1
+#include "StringFindStartswithCheck.h"
+

nit: We need a LICENSE comment at the top of the file.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:7
+
+using namespace clang::ast_matchers; // NOLINT: Too many names.
+

nit: no need for `NOLINT`.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:17-19
+  auto stringClassMatcher = anyOf(cxxRecordDecl(hasName("string")),
+  cxxRecordDecl(hasName("__versa_string")),
+  cxxRecordDecl(hasName("basic_string")));

aaron.ballman wrote:
> These should be using elaborated type specifiers to ensure we get the correct 
> class, not some class that happens to have the same name. Also, this list 
> should be configurable so that users can add their own entries to it.
+1, we could add a `StringLikeClasses` option.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:48
+ ->getImplicitObjectArgument();
+
+  // Get the source code blocks (as characters) for both the string object

nit: add `assert` to make sure these pointers are not nullptr.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:81
+  auto include_hint = include_inserter_->CreateIncludeInsertion(
+  source.getFileID(expr->getLocStart()), 
"third_party/absl/strings/match.h",
+  false);

The include path maybe different in different project.

I think we also need an option for the inserting header, and make it default to 
`absl/strings/match.h`.



Comment at: clang-tidy/absl/StringFindStartswithCheck.cpp:92
+  compiler.getSourceManager(), compiler.getLangOpts(),
+  clang::tidy::utils::IncludeSorter::IS_Google);
+  compiler.getPreprocessor().addPPCallbacks(

We need to make the style customizable by adding an `IncludeStyle` option (see 
for an example 
http://clang.llvm.org/extra/clang-tidy/checks/performance-unnecessary-value-param.html#cmdoption-arg-includestyle),
 instead of hard-coding.



Comment at: docs/clang-tidy/checks/absl-string-find-startswith.rst:17
+should be replaced with
+``if (absl::StartsWith(s, "Hello World")) { /* do something */ };``
+

Eugene.Zelenko wrote:
> Please use .. code-block: c++
nit: I would keep the `string s = "..."` here.



Comment at: test/clang-tidy/absl-string-find-startswith.cpp:1
+// RUN: %check_clang_tidy %s absl-string-find-startswith %t -- -- -I%S
+// -std=c++11

Since your test is isolated, `// RUN: %check_clang_tidy %s 
absl-string-find-startswith %t` should work.



Comment at: test/clang-tidy/absl-string-find-startswith.cpp:27
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use absl::StartsWith instead of
+  // find() == 0 [absl-string-find-startswith] CHECK_FIXES:
+  // {{^}}absl::StartsWith(s, "a");{{$}}

nit: we usually use `CHECK-FIXES` in a new line: `// CHECK-FIXES: 
absl::StartsWith(s, "a");`

The same below.



Comment at: test/clang-tidy/absl-string-find-startswith.cpp:30
+
+  s.find(s) == 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: Use absl::StartsWith instead of

Could you also add a test like `if (s.find(s) == 0) { ... }` to make sure the 
replacement range is correct.

Also maybe add `0 == s.find(s)`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43572: [Sema] Improve test coverage of narrowing conversion diagnostics

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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

Looks good to me. Thanks!


https://reviews.llvm.org/D43572



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

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

aaron.ballman wrote:
> massberg wrote:
> > aaron.ballman wrote:
> > > I think that this code should be generalized (same with the matchers) so 
> > > that you match on `hasAnyName()` for the function calls and use 
> > > `CallExpr::getCalleeDecl()` to get the declaration. e.g.,
> > > ```
> > > if (const auto *Call = Result.Nodes.getNodeAs("blech")) {
> > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > diag(Call->getLocStart(), Message) << Calleee;
> > > }
> > > ```
> > > This way you can add more named without having to add extra logic for the 
> > > diagnostics.
> > I generalized the code and the matcher.
> > When we use
> > ```
> > << cast(Callee);
> > ```
> > we get the template arguments in the name , e.g. `ptr_fun`, so I 
> > chose to use `getQualifiedNameAsString`.
> > If there is there a better way to get the function name without template 
> > arguments I appreciate any suggestions.
> > 
> > 
> Nope, in that case, your code is correct. However, we generally provide the 
> template arguments in diagnostics. I see @alexfh was asking for them to be 
> removed as not being useful, but I'm not certain I agree with his rationale. 
> Yes, all instances are deprecated and thus the template arguments don't 
> discern between good and bad cases, but showing the template arguments is 
> also consistent with the other diagnostics we emit. For instance, other 
> "deprecated" diagnostics also emit the template arguments. I'm not certain we 
> should be inconsistent with the way we produce diagnostics, but I'll defer to 
> Alex if he still feels strongly about leaving them off here.
Indeed, -Wdeprecated-declarations warnings print template arguments. Moreover, 
they seem to be issued only on instantiations, see https://godbolt.org/g/W563gw.

But I have a number of concerns with template arguments in the deprecation 
warnings:

1. The note attached to the warning lies. Consider a warning from the test 
above:
  ...
  :11:1: warning: 'B' is deprecated: bbb 
[-Wdeprecated-declarations]
  B e;
  ^
  :7:10: note: 'B' has been explicitly marked deprecated here
  struct [[deprecated("bbb")]] B {};

 But `B` hasn't been explicitly marked deprecated, only the template 
definition of `B` has been. Template arguments are important in the case of the 
explicit template specialization `A` in the same example, but not in cases 
where the template definition was marked deprecated, since template arguments 
only add noise and no useful information there.
2. Warnings can easily become unreadable: https://godbolt.org/g/AFdznH
3. Certain coding patterns can result in numerous deprecation warnings 
differing only in template arguments: https://godbolt.org/g/U2JCbG. Clang-tidy 
can deduplicate warnings, if they have identical text and location, but adding 
template arguments to the message will prevent deduplication. I've seen a case 
where thousands of deprecation warnings were generated for a single line in a 
widely used header.

So yes, I feel strongly about leaving off template arguments in case the whole 
template was marked deprecated. I think it would be the right thing to do for 
the -Wdeprecated-declarations diagnostic as well.


https://reviews.llvm.org/D42730



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

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

alexfh wrote:
> aaron.ballman wrote:
> > massberg wrote:
> > > aaron.ballman wrote:
> > > > I think that this code should be generalized (same with the matchers) 
> > > > so that you match on `hasAnyName()` for the function calls and use 
> > > > `CallExpr::getCalleeDecl()` to get the declaration. e.g.,
> > > > ```
> > > > if (const auto *Call = Result.Nodes.getNodeAs("blech")) {
> > > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > > diag(Call->getLocStart(), Message) << Calleee;
> > > > }
> > > > ```
> > > > This way you can add more named without having to add extra logic for 
> > > > the diagnostics.
> > > I generalized the code and the matcher.
> > > When we use
> > > ```
> > > << cast(Callee);
> > > ```
> > > we get the template arguments in the name , e.g. `ptr_fun`, so 
> > > I chose to use `getQualifiedNameAsString`.
> > > If there is there a better way to get the function name without template 
> > > arguments I appreciate any suggestions.
> > > 
> > > 
> > Nope, in that case, your code is correct. However, we generally provide the 
> > template arguments in diagnostics. I see @alexfh was asking for them to be 
> > removed as not being useful, but I'm not certain I agree with his 
> > rationale. Yes, all instances are deprecated and thus the template 
> > arguments don't discern between good and bad cases, but showing the 
> > template arguments is also consistent with the other diagnostics we emit. 
> > For instance, other "deprecated" diagnostics also emit the template 
> > arguments. I'm not certain we should be inconsistent with the way we 
> > produce diagnostics, but I'll defer to Alex if he still feels strongly 
> > about leaving them off here.
> Indeed, -Wdeprecated-declarations warnings print template arguments. 
> Moreover, they seem to be issued only on instantiations, see 
> https://godbolt.org/g/W563gw.
> 
> But I have a number of concerns with template arguments in the deprecation 
> warnings:
> 
> 1. The note attached to the warning lies. Consider a warning from the test 
> above:
>   ...
>   :11:1: warning: 'B' is deprecated: bbb 
> [-Wdeprecated-declarations]
>   B e;
>   ^
>   :7:10: note: 'B' has been explicitly marked deprecated here
>   struct [[deprecated("bbb")]] B {};
> 
>  But `B` hasn't been explicitly marked deprecated, only the template 
> definition of `B` has been. Template arguments are important in the case of 
> the explicit template specialization `A` in the same example, but not in 
> cases where the template definition was marked deprecated, since template 
> arguments only add noise and no useful information there.
> 2. Warnings can easily become unreadable: https://godbolt.org/g/AFdznH
> 3. Certain coding patterns can result in numerous deprecation warnings 
> differing only in template arguments: https://godbolt.org/g/U2JCbG. 
> Clang-tidy can deduplicate warnings, if they have identical text and 
> location, but adding template arguments to the message will prevent 
> deduplication. I've seen a case where thousands of deprecation warnings were 
> generated for a single line in a widely used header.
> 
> So yes, I feel strongly about leaving off template arguments in case the 
> whole template was marked deprecated. I think it would be the right thing to 
> do for the -Wdeprecated-declarations diagnostic as well.
s/leaving off/leaving out/


https://reviews.llvm.org/D42730



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

[PATCH] D43162: [Parser] (C++) Make -Wextra-semi slightly more useful

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 136503.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.

Reworked stuff via new `CXX98CompatExtraSemi` diag group.

As expected, passing `-Wno-c++98-compat-pedantic` disables the extra-semi diag, 
which exactly the opposite from what i'm trying to do here...
But now that the diag is moved to the new diag group, it can be re-enabled, 
which is at least something. But this looks kinda ugly.
Is this how it should be?


Repository:
  rC Clang

https://reviews.llvm.org/D43162

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticParseKinds.td
  test/Parser/cxx-extra-semi.cpp
  test/SemaCXX/extra-semi.cpp


Index: test/SemaCXX/extra-semi.cpp
===
--- /dev/null
+++ test/SemaCXX/extra-semi.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -verify -std=c++98 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++03 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++11 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++17 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++2a -Wextra-semi %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat-pedantic 
-Wc++98-compat-extra-semi %s
+
+// Last RUN line checks that c++98-compat-extra-semi can still be re-enabled.
+
+void F();
+
+void F(){}
+; // expected-warning {{extra ';' outside of a function is}}
+
+namespace ns {
+class C {
+  void F() const;
+};
+}
+; // expected-warning {{extra ';' outside of a function is}}
+
+void ns::C::F() const {}
+; // expected-warning {{extra ';' outside of a function is}}
Index: test/Parser/cxx-extra-semi.cpp
===
--- test/Parser/cxx-extra-semi.cpp
+++ test/Parser/cxx-extra-semi.cpp
@@ -38,4 +38,7 @@
 #if __cplusplus < 201103L
 // expected-warning@-3{{extra ';' outside of a function is a C++11 extension}}
 // expected-warning@-3{{extra ';' outside of a function is a C++11 extension}}
+#elif !defined(PEDANTIC)
+// expected-warning@-6{{extra ';' outside of a function is incompatible with 
C++98}}
+// expected-warning@-6{{extra ';' outside of a function is incompatible with 
C++98}}
 #endif
Index: include/clang/Basic/DiagnosticParseKinds.td
===
--- include/clang/Basic/DiagnosticParseKinds.td
+++ include/clang/Basic/DiagnosticParseKinds.td
@@ -39,7 +39,7 @@
   InGroup>;
 def warn_cxx98_compat_top_level_semi : Warning<
   "extra ';' outside of a function is incompatible with C++98">,
-  InGroup, DefaultIgnore;
+  InGroup, DefaultIgnore;
 def ext_extra_semi : Extension<
   "extra ';' %select{"
   "outside of a function|"
Index: include/clang/Basic/DiagnosticGroups.td
===
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -151,8 +151,10 @@
 def GNUEmptyInitializer : DiagGroup<"gnu-empty-initializer">;
 def GNUEmptyStruct : DiagGroup<"gnu-empty-struct">;
 def ExtraTokens : DiagGroup<"extra-tokens">;
+def CXX98CompatExtraSemi : DiagGroup<"c++98-compat-extra-semi">;
 def CXX11ExtraSemi : DiagGroup<"c++11-extra-semi">;
-def ExtraSemi : DiagGroup<"extra-semi", [CXX11ExtraSemi]>;
+def ExtraSemi : DiagGroup<"extra-semi", [CXX98CompatExtraSemi,
+ CXX11ExtraSemi]>;
 
 def GNUFlexibleArrayInitializer : DiagGroup<"gnu-flexible-array-initializer">;
 def GNUFlexibleArrayUnionMember : DiagGroup<"gnu-flexible-array-union-member">;
@@ -196,6 +198,7 @@
 def CXX98CompatPedantic : DiagGroup<"c++98-compat-pedantic",
 [CXX98Compat,
  CXX98CompatBindToTemporaryCopy,
+ CXX98CompatExtraSemi,
  CXXPre14CompatPedantic,
  CXXPre17CompatPedantic,
  CXXPre2aCompatPedantic]>;


Index: test/SemaCXX/extra-semi.cpp
===
--- /dev/null
+++ test/SemaCXX/extra-semi.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -verify -std=c++98 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++03 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++11 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++17 -Wextra-semi %s
+// RUN: %clang_cc1 -verify -std=c++2a -Wextra-semi %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat %s
+// RUN: %clang_cc1 -verify -Weverything -Wno-c++98-compat-pedantic -Wc++98-compat-extra-semi %s
+
+// Last RUN line checks that c++98-compat-extra-semi can still be re-enabled.
+
+void F();
+
+void F(){}
+; // expected-warning {{extra ';' outside of a function is}}
+
+namespace ns {
+class C {
+  void F() const;
+};
+}
+; // expected-warning {{extra ';' outside of a function is}}
+
+void ns::C::F() const {}
+; // 

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

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

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

LGTM!


https://reviews.llvm.org/D43928



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

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Ilya Biryukov via cfe-commits]

I replied to a commit in the wrong thread (https://reviews.llvm.org/rL326323),
sorry.
Here are the important bits:

This change introduced the following cyclic dependency in the build system:
StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
broke our internal integrate (we use a different buildsystem, which breaks
on cyclic deps) and it's really messy to workaround it since CrossTU
depends on both Index and Frontend.
Moving the code that uses CrossTU from StaticAnalyzerCore to
StaticAnalyzerFrontend should probably fix it, but I don't have enough
context to come up with a fix.



On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
cfe-commits  wrote:

> a.sidorin reopened this revision.
> a.sidorin added a comment.
>
> The changes were reverted:
> http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
> Gabor, could you take a look?
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D30691
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


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

r326434 - UsersManual: beef up the clang-cl text a little

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Mar  1 06:00:19 2018
New Revision: 326434

URL: http://llvm.org/viewvc/llvm-project?rev=326434&view=rev
Log:
UsersManual: beef up the clang-cl text a little

Modified:
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=326434&r1=326433&r2=326434&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Thu Mar  1 06:00:19 2018
@@ -2684,10 +2684,20 @@ compatibility with the Visual C++ compil
 To enable clang-cl to find system headers, libraries, and the linker when run
 from the command-line, it should be executed inside a Visual Studio Native 
Tools
 Command Prompt or a regular Command Prompt where the environment has been set
-up using e.g. `vcvars32.bat 
`_.
+up using e.g. `vcvarsall.bat 
`_.
+
+clang-cl can also be used from inside Visual Studio by selecting the LLVM
+Platform Toolset. The toolset is installed by the LLVM installer, which can be
+downloaded from the `LLVM release `_ or
+`snapshot build `_ web pages. To use the toolset,
+select a project in Solution Explorer, open its Property Page (Alt+F7), and in
+the "General" section of "Configuration Properties" change "Platform Toolset"
+to e.g. LLVM-vs2014.
+
+To use the toolset with MSBuild directly, invoke it with e.g.
+``/p:PlatformToolset=LLVM-vs2014``. This allows trying out the clang-cl
+toolchain without modifying your project files.
 
-clang-cl can also be used from inside Visual Studio by using an LLVM Platform
-Toolset.
 
 Command-Line Options
 


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

Re: r326434 - UsersManual: beef up the clang-cl text a little

[Nico Weber via cfe-commits]

Should we also mention /p:CLToolExe for just swapping out the compiler
without using a full toolset?

Also, should we include some notes on cmake usage? I.e. -G "Visual Studio
14 2015" -T "LLVM-vs2014" for the MSVS generator,
and -DCMAKE_C_COMPILER=/path/to/llvm/bin/clang-cl.exe
-DCMAKE_CXX_COMPILER=/path/to/llvm/bin/clang-cl.exe for the ninja
generator? (Don't know if -T works with ninja and/or setting
CMAKE_C(XX)_COMPILER works for msvc.)

I haven't tested either of these recently, but they all worked at some
point and all were useful at some point.

On Thu, Mar 1, 2018 at 9:00 AM, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: hans
> Date: Thu Mar  1 06:00:19 2018
> New Revision: 326434
>
> URL: http://llvm.org/viewvc/llvm-project?rev=326434&view=rev
> Log:
> UsersManual: beef up the clang-cl text a little
>
> Modified:
> cfe/trunk/docs/UsersManual.rst
>
> Modified: cfe/trunk/docs/UsersManual.rst
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/
> UsersManual.rst?rev=326434&r1=326433&r2=326434&view=diff
> 
> ==
> --- cfe/trunk/docs/UsersManual.rst (original)
> +++ cfe/trunk/docs/UsersManual.rst Thu Mar  1 06:00:19 2018
> @@ -2684,10 +2684,20 @@ compatibility with the Visual C++ compil
>  To enable clang-cl to find system headers, libraries, and the linker when
> run
>  from the command-line, it should be executed inside a Visual Studio
> Native Tools
>  Command Prompt or a regular Command Prompt where the environment has been
> set
> -up using e.g. `vcvars32.bat  us/library/f2ccy3wt.aspx>`_.
> +up using e.g. `vcvarsall.bat  us/library/f2ccy3wt.aspx>`_.
> +
> +clang-cl can also be used from inside Visual Studio by selecting the LLVM
> +Platform Toolset. The toolset is installed by the LLVM installer, which
> can be
> +downloaded from the `LLVM release `_
> or
> +`snapshot build `_ web pages. To use the
> toolset,
> +select a project in Solution Explorer, open its Property Page (Alt+F7),
> and in
> +the "General" section of "Configuration Properties" change "Platform
> Toolset"
> +to e.g. LLVM-vs2014.
> +
> +To use the toolset with MSBuild directly, invoke it with e.g.
> +``/p:PlatformToolset=LLVM-vs2014``. This allows trying out the clang-cl
> +toolchain without modifying your project files.
>
> -clang-cl can also be used from inside Visual Studio by using an LLVM
> Platform
> -Toolset.
>
>  Command-Line Options
>  
>
>
> ___
> 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

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clangd/index/CanonicalIncludes.h:47
+  /// Sets the canonical include for any symbol with \p QualifiedName.
+  /// Header mappings are ignored if \p QualifiedName matches any symbol in the
+  /// symbol mapping.

Nit: "Symbol mappings take precedence over header mappings"?



Comment at: clangd/index/CanonicalIncludes.h:52
+
   /// \return \p Header itself if there is no mapping for it; otherwise, return
   /// a canonical header name.

Nit: rephrase as "returns the canonical include for \p Symbol, which is 
declared in \p Header"?



Comment at: clangd/index/CanonicalIncludes.h:54
   /// a canonical header name.
-  llvm::StringRef mapHeader(llvm::StringRef Header) const;
+  /// An optional qualified symbol name can be provided to check against the
+  /// symbol mapping.

Why can't we always provide this?



Comment at: unittests/clangd/SymbolCollectorTests.cpp:561
+  class no_map {};
+  class ios {};
+  class ostream {};

ioeric wrote:
> hokein wrote:
> > The STL implementation of `ios` is a typedef `typedef basic_ios  ios; 
> > `, I think we should make the test align with it?
> The symbol mapping doesn't make a difference between symbol types, and symbol 
> kind test is not in the scope of this patch,  so I think this wouldn't be 
> necessary here.
I'd be +1 on matching the real header a bit more so it's more obvious how this 
relates to the standard library. As long as it's not too hard!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869



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

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

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

2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):

I replied to a commit in the wrong thread (https://reviews.llvm.org/rL326323),
sorry.
Here are the important bits:

This change introduced the following cyclic dependency in the build system:
StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.


I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CMakeLists.txt

Do I miss something here?

Thanks in advance,
Gábor


I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
broke our internal integrate (we use a different buildsystem, which breaks
on cyclic deps) and it's really messy to workaround it since CrossTU
depends on both Index and Frontend.
Moving the code that uses CrossTU from StaticAnalyzerCore to
StaticAnalyzerFrontend should probably fix it, but I don't have enough
context to come up with a fix.



On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
cfe-commits  wrote:

> a.sidorin reopened this revision.
> a.sidorin added a comment.
>
> The changes were reverted: http://llvm.org/viewvc/llvm-
> project?rev=326432&view=rev
> Gabor, could you take a look?
>
>
> Repository:
>   rC Clang
>
> https://reviews.llvm.org/D30691
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>


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

[PATCH] D43817: [x86] wbnoinvd intrinsic

[Gabor Buella via Phabricator via cfe-commits]

GBuella updated this revision to Diff 136510.
GBuella edited the summary of this revision.
GBuella added a comment.

Added Ice Lake Server architecture.
Removed wbinvd related code.


https://reviews.llvm.org/D43817

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/BuiltinsX86.def
  include/clang/Basic/X86Target.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/Headers/CMakeLists.txt
  lib/Headers/cpuid.h
  lib/Headers/wbnoinvdintrin.h
  lib/Headers/x86intrin.h
  test/CodeGen/builtin-wbnoinvd.c
  test/CodeGen/builtins-x86.c
  test/Driver/x86-march.c
  test/Driver/x86-target-features.c
  test/Frontend/x86-target-cpu.c
  test/Preprocessor/predefined-arch-macros.c

Index: test/Preprocessor/predefined-arch-macros.c
===
--- test/Preprocessor/predefined-arch-macros.c
+++ test/Preprocessor/predefined-arch-macros.c
@@ -1110,7 +1110,64 @@
 // CHECK_ICL_M32: #define __i386__ 1
 // CHECK_ICL_M32: #define __tune_corei7__ 1
 // CHECK_ICL_M32: #define i386 1
-//
+
+// RUN: %clang -march=icelakex -m32 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICX_M32
+// CHECK_ICX_M32: #define __AES__ 1
+// CHECK_ICX_M32: #define __AVX2__ 1
+// CHECK_ICX_M32: #define __AVX512BITALG__ 1
+// CHECK_ICX_M32: #define __AVX512BW__ 1
+// CHECK_ICX_M32: #define __AVX512CD__ 1
+// CHECK_ICX_M32: #define __AVX512DQ__ 1
+// CHECK_ICX_M32: #define __AVX512F__ 1
+// CHECK_ICX_M32: #define __AVX512IFMA__ 1
+// CHECK_ICX_M32: #define __AVX512VBMI2__ 1
+// CHECK_ICX_M32: #define __AVX512VBMI__ 1
+// CHECK_ICX_M32: #define __AVX512VL__ 1
+// CHECK_ICX_M32: #define __AVX512VNNI__ 1
+// CHECK_ICX_M32: #define __AVX512VPOPCNTDQ__ 1
+// CHECK_ICX_M32: #define __AVX__ 1
+// CHECK_ICX_M32: #define __BMI2__ 1
+// CHECK_ICX_M32: #define __BMI__ 1
+// CHECK_ICX_M32: #define __CLFLUSHOPT__ 1
+// CHECK_ICX_M32: #define __CLWB__ 1
+// CHECK_ICX_M32: #define __F16C__ 1
+// CHECK_ICX_M32: #define __FMA__ 1
+// CHECK_ICX_M32: #define __GFNI__ 1
+// CHECK_ICX_M32: #define __LZCNT__ 1
+// CHECK_ICX_M32: #define __MMX__ 1
+// CHECK_ICX_M32: #define __MPX__ 1
+// CHECK_ICX_M32: #define __PCLMUL__ 1
+// CHECK_ICX_M32: #define __PKU__ 1
+// CHECK_ICX_M32: #define __POPCNT__ 1
+// CHECK_ICX_M32: #define __PRFCHW__ 1
+// CHECK_ICX_M32: #define __RDPID__ 1
+// CHECK_ICX_M32: #define __RDRND__ 1
+// CHECK_ICX_M32: #define __RDSEED__ 1
+// CHECK_ICX_M32: #define __RTM__ 1
+// CHECK_ICX_M32: #define __SGX__ 1
+// CHECK_ICX_M32: #define __SHA__ 1
+// CHECK_ICX_M32: #define __SSE2__ 1
+// CHECK_ICX_M32: #define __SSE3__ 1
+// CHECK_ICX_M32: #define __SSE4_1__ 1
+// CHECK_ICX_M32: #define __SSE4_2__ 1
+// CHECK_ICX_M32: #define __SSE__ 1
+// CHECK_ICX_M32: #define __SSSE3__ 1
+// CHECK_ICX_M32: #define __VAES__ 1
+// CHECK_ICX_M32: #define __VPCLMULQDQ__ 1
+// CHECK_ICX_M32: #define __WBNOINVD__ 1
+// CHECK_ICX_M32: #define __XSAVEC__ 1
+// CHECK_ICX_M32: #define __XSAVEOPT__ 1
+// CHECK_ICX_M32: #define __XSAVES__ 1
+// CHECK_ICX_M32: #define __XSAVE__ 1
+// CHECK_ICX_M32: #define __corei7 1
+// CHECK_ICX_M32: #define __corei7__ 1
+// CHECK_ICX_M32: #define __i386 1
+// CHECK_ICX_M32: #define __i386__ 1
+// CHECK_ICX_M32: #define __tune_corei7__ 1
+// CHECK_ICX_M32: #define i386 1
+
 // RUN: %clang -march=icelake -m64 -E -dM %s -o - 2>&1 \
 // RUN: -target i386-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICL_M64
@@ -1168,6 +1225,64 @@
 // CHECK_ICL_M64: #define __x86_64 1
 // CHECK_ICL_M64: #define __x86_64__ 1
 
+// RUN: %clang -march=icelakex -m64 -E -dM %s -o - 2>&1 \
+// RUN: -target i386-unknown-linux \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_ICX_M64
+// CHECK_ICX_M64: #define __AES__ 1
+// CHECK_ICX_M64: #define __AVX2__ 1
+// CHECK_ICX_M64: #define __AVX512BITALG__ 1
+// CHECK_ICX_M64: #define __AVX512BW__ 1
+// CHECK_ICX_M64: #define __AVX512CD__ 1
+// CHECK_ICX_M64: #define __AVX512DQ__ 1
+// CHECK_ICX_M64: #define __AVX512F__ 1
+// CHECK_ICX_M64: #define __AVX512IFMA__ 1
+// CHECK_ICX_M64: #define __AVX512VBMI2__ 1
+// CHECK_ICX_M64: #define __AVX512VBMI__ 1
+// CHECK_ICX_M64: #define __AVX512VL__ 1
+// CHECK_ICX_M64: #define __AVX512VNNI__ 1
+// CHECK_ICX_M64: #define __AVX512VPOPCNTDQ__ 1
+// CHECK_ICX_M64: #define __AVX__ 1
+// CHECK_ICX_M64: #define __BMI2__ 1
+// CHECK_ICX_M64: #define __BMI__ 1
+// CHECK_ICX_M64: #define __CLFLUSHOPT__ 1
+// CHECK_ICX_M64: #define __CLWB__ 1
+// CHECK_ICX_M64: #define __F16C__ 1
+// CHECK_ICX_M64: #define __FMA__ 1
+// CHECK_ICX_M64: #define __GFNI__ 1
+// CHECK_ICX_M64: #define __LZCNT__ 1
+// CHECK_ICX_M64: #define __MMX__ 1
+// CHECK_ICX_M64: #define __MPX__ 1
+// CHECK_ICX_M64: #define __PCLMUL__ 1
+// CHECK_ICX_M64: #define __PKU__ 1
+// CHECK_ICX_M64: #define __POPCNT__ 1
+// CHECK_ICX_M64: #define __PRFCHW__ 1
+// CHECK_ICX_M64: #define __RDPID__ 1

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

In https://reviews.llvm.org/D43847#1023452, @hokein wrote:

> In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
>
> > I need a bit more context because I'm unfamiliar with `absl`. What is this 
> > module's intended use?
>
>
> As `absl` has been open-sourced (https://github.com/abseil/abseil-cpp), I 
> think there will be more absl-related checks contributed from google or 
> external contributors in the future, so it make sense to create a new module.
>
> @niko, could you please refine the code style of this patch to follow the 
> LLVM code standard 
> 
>  (especially the variable name, should be camel case "VariableName")?


May be //abseil// is better name for module?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

By the word, may be similar check for std::string::rfind() and 
std::string::ends_with() (does abseil have analog) should be added too?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Ilya Biryukov via cfe-commits]

You're right. We have this extra dependency in our internal build files for
some reason and I missed that.
It's totally my fault.

Should I resubmit the patch that I reverted or you would rather do it
yourself?



On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth  wrote:

>
>
> 2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):
>
> I replied to a commit in the wrong thread (
> https://reviews.llvm.org/rL326323), sorry.
> Here are the important bits:
>
> This change introduced the following cyclic dependency in the build
> system: StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
>
>
> I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
> https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CMakeLists.txt
>
> Do I miss something here?
>
> Thanks in advance,
> Gábor
>
>
> I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
> broke our internal integrate (we use a different buildsystem, which breaks
> on cyclic deps) and it's really messy to workaround it since CrossTU
> depends on both Index and Frontend.
> Moving the code that uses CrossTU from StaticAnalyzerCore to
> StaticAnalyzerFrontend should probably fix it, but I don't have enough
> context to come up with a fix.
>
>
>
> On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
> cfe-commits  wrote:
>
>> a.sidorin reopened this revision.
>> a.sidorin added a comment.
>>
>> The changes were reverted:
>> http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
>> Gabor, could you take a look?
>>
>>
>> Repository:
>>   rC Clang
>>
>> https://reviews.llvm.org/D30691
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
>
>

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

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

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

I am away from my workstation so I would really appreciate if you could
recommit.

Thanks in advance,
Gábor

2018. márc. 1. 15:28 ezt írta ("Ilya Biryukov" ):

> You're right. We have this extra dependency in our internal build files
> for some reason and I missed that.
> It's totally my fault.
>
> Should I resubmit the patch that I reverted or you would rather do it
> yourself?
>
>
>
> On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth  wrote:
>
>>
>>
>> 2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):
>>
>> I replied to a commit in the wrong thread (https://reviews.llvm.org/
>> rL326323), sorry.
>> Here are the important bits:
>>
>> This change introduced the following cyclic dependency in the build
>> system: StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
>>
>>
>> I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
>> https://github.com/llvm-mirror/clang/blob/master/lib/
>> Frontend/CMakeLists.txt
>>
>> Do I miss something here?
>>
>> Thanks in advance,
>> Gábor
>>
>>
>> I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
>> broke our internal integrate (we use a different buildsystem, which breaks
>> on cyclic deps) and it's really messy to workaround it since CrossTU
>> depends on both Index and Frontend.
>> Moving the code that uses CrossTU from StaticAnalyzerCore to
>> StaticAnalyzerFrontend should probably fix it, but I don't have enough
>> context to come up with a fix.
>>
>>
>>
>> On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
>> cfe-commits  wrote:
>>
>>> a.sidorin reopened this revision.
>>> a.sidorin added a comment.
>>>
>>> The changes were reverted: http://llvm.org/viewvc/llvm-
>>> project?rev=326432&view=rev
>>> Gabor, could you take a look?
>>>
>>>
>>> Repository:
>>>   rC Clang
>>>
>>> https://reviews.llvm.org/D30691
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>>
>>
>
> --
> Regards,
> Ilya Biryukov
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Benjamin Kramer via cfe-commits]

Frontend depends on StaticAnalyzerCore by
including "clang/StaticAnalyzer/Core/Analyses.def". That's a clear layering
violation, but cmake doesn't model header dependencies. Maybe Analyses.def
should move into its own library.


On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:

>
>
> 2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):
>
> I replied to a commit in the wrong thread (
> https://reviews.llvm.org/rL326323), sorry.
> Here are the important bits:
>
> This change introduced the following cyclic dependency in the build
> system: StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
>
>
> I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
> https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CMakeLists.txt
>
> Do I miss something here?
>
> Thanks in advance,
> Gábor
>
>
> I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
> broke our internal integrate (we use a different buildsystem, which breaks
> on cyclic deps) and it's really messy to workaround it since CrossTU
> depends on both Index and Frontend.
> Moving the code that uses CrossTU from StaticAnalyzerCore to
> StaticAnalyzerFrontend should probably fix it, but I don't have enough
> context to come up with a fix.
>
>
>
> On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
> cfe-commits  wrote:
>
>> a.sidorin reopened this revision.
>> a.sidorin added a comment.
>>
>> The changes were reverted:
>> http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
>> Gabor, could you take a look?
>>
>>
>> Repository:
>>   rC Clang
>>
>> https://reviews.llvm.org/D30691
>>
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
>
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326438 - UsersManual: improve the clang-cl text some more

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Mar  1 06:48:19 2018
New Revision: 326438

URL: http://llvm.org/viewvc/llvm-project?rev=326438&view=rev
Log:
UsersManual: improve the clang-cl text some more

Modified:
cfe/trunk/docs/UsersManual.rst

Modified: cfe/trunk/docs/UsersManual.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=326438&r1=326437&r2=326438&view=diff
==
--- cfe/trunk/docs/UsersManual.rst (original)
+++ cfe/trunk/docs/UsersManual.rst Thu Mar  1 06:48:19 2018
@@ -2698,6 +2698,23 @@ To use the toolset with MSBuild directly
 ``/p:PlatformToolset=LLVM-vs2014``. This allows trying out the clang-cl
 toolchain without modifying your project files.
 
+It's also possible to point MSBuild at clang-cl without changing toolset by
+passing ``/p:CLToolPath=c:\llvm\bin /p:CLToolExe=clang-cl.exe``.
+
+When using CMake and the Visual Studio generators, the toolset can be set with 
the ``-T`` flag:
+
+  ::
+
+cmake -G"Visual Studio 15 2017" -T LLVM-vs2014 ..
+
+When using CMake with the Ninja generator, set the ``CMAKE_C_COMPILER`` and
+``CMAKE_CXX_COMPILER`` variables to clang-cl:
+
+  ::
+
+cmake -GNinja -DCMAKE_C_COMPILER="c:/Program Files 
(x86)/LLVM/bin/clang-cl.exe"
+-DCMAKE_CXX_COMPILER="c:/Program Files (x86)/LLVM/bin/clang-cl.exe" ..
+
 
 Command-Line Options
 


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

Re: r326434 - UsersManual: beef up the clang-cl text a little

[Hans Wennborg via cfe-commits]

Thanks! Added in r326438.

On Thu, Mar 1, 2018 at 3:05 PM, Nico Weber via cfe-commits
 wrote:
> Should we also mention /p:CLToolExe for just swapping out the compiler
> without using a full toolset?
>
> Also, should we include some notes on cmake usage? I.e. -G "Visual Studio 14
> 2015" -T "LLVM-vs2014" for the MSVS generator, and
> -DCMAKE_C_COMPILER=/path/to/llvm/bin/clang-cl.exe
> -DCMAKE_CXX_COMPILER=/path/to/llvm/bin/clang-cl.exe for the ninja generator?
> (Don't know if -T works with ninja and/or setting CMAKE_C(XX)_COMPILER works
> for msvc.)
>
> I haven't tested either of these recently, but they all worked at some point
> and all were useful at some point.
>
> On Thu, Mar 1, 2018 at 9:00 AM, Hans Wennborg via cfe-commits
>  wrote:
>>
>> Author: hans
>> Date: Thu Mar  1 06:00:19 2018
>> New Revision: 326434
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=326434&view=rev
>> Log:
>> UsersManual: beef up the clang-cl text a little
>>
>> Modified:
>> cfe/trunk/docs/UsersManual.rst
>>
>> Modified: cfe/trunk/docs/UsersManual.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/UsersManual.rst?rev=326434&r1=326433&r2=326434&view=diff
>>
>> ==
>> --- cfe/trunk/docs/UsersManual.rst (original)
>> +++ cfe/trunk/docs/UsersManual.rst Thu Mar  1 06:00:19 2018
>> @@ -2684,10 +2684,20 @@ compatibility with the Visual C++ compil
>>  To enable clang-cl to find system headers, libraries, and the linker when
>> run
>>  from the command-line, it should be executed inside a Visual Studio
>> Native Tools
>>  Command Prompt or a regular Command Prompt where the environment has been
>> set
>> -up using e.g. `vcvars32.bat
>> `_.
>> +up using e.g. `vcvarsall.bat
>> `_.
>> +
>> +clang-cl can also be used from inside Visual Studio by selecting the LLVM
>> +Platform Toolset. The toolset is installed by the LLVM installer, which
>> can be
>> +downloaded from the `LLVM release
>> `_ or
>> +`snapshot build `_ web pages. To use the
>> toolset,
>> +select a project in Solution Explorer, open its Property Page (Alt+F7),
>> and in
>> +the "General" section of "Configuration Properties" change "Platform
>> Toolset"
>> +to e.g. LLVM-vs2014.
>> +
>> +To use the toolset with MSBuild directly, invoke it with e.g.
>> +``/p:PlatformToolset=LLVM-vs2014``. This allows trying out the clang-cl
>> +toolchain without modifying your project files.
>>
>> -clang-cl can also be used from inside Visual Studio by using an LLVM
>> Platform
>> -Toolset.
>>
>>  Command-Line Options
>>  
>>
>>
>> ___
>> 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
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r326439 - Resubmit [analyzer] Support for naive cross translation unit analysis

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Mar  1 06:54:16 2018
New Revision: 326439

URL: http://llvm.org/viewvc/llvm-project?rev=326439&view=rev
Log:
Resubmit [analyzer] Support for naive cross translation unit analysis

Originally submitted as r326323 and r326324.
Reverted in r326432.

Reverting the commit was a mistake.
The breakage was due to invalid build files in our internal buildsystem,
CMakeLists did not have any cyclic dependencies.

Added:
cfe/trunk/test/Analysis/Inputs/ctu-chain.cpp
cfe/trunk/test/Analysis/Inputs/ctu-other.cpp
cfe/trunk/test/Analysis/Inputs/externalFnMap.txt
cfe/trunk/test/Analysis/ctu-main.cpp
Modified:
cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Core/CallEvent.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngine.cpp
cfe/trunk/lib/StaticAnalyzer/Core/PathDiagnostic.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/CMakeLists.txt
cfe/trunk/test/Analysis/analyzer-config.cpp
cfe/trunk/tools/scan-build-py/README.md
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/arguments.py
cfe/trunk/tools/scan-build-py/libscanbuild/clang.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_clang.py

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h?rev=326439&r1=326438&r2=326439&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h (original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h Thu Mar  1 
06:54:16 2018
@@ -308,6 +308,16 @@ private:
   /// \sa shouldDisplayNotesAsEvents
   Optional DisplayNotesAsEvents;
 
+  /// \sa getCTUDir
+  Optional CTUDir;
+
+  /// \sa getCTUIndexName
+  Optional CTUIndexName;
+
+  /// \sa naiveCTUEnabled
+  Optional NaiveCTU;
+
+
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
   /// Options for checkers can be specified via 'analyzer-config' command-line
@@ -637,6 +647,17 @@ public:
   /// to false when unset.
   bool shouldDisplayNotesAsEvents();
 
+  /// Returns the directory containing the CTU related files.
+  StringRef getCTUDir();
+
+  /// Returns the name of the file containing the CTU index of functions.
+  StringRef getCTUIndexName();
+
+  /// Returns true when naive cross translation unit analysis is enabled.
+  /// This is an experimental feature to inline functions from another
+  /// translation units.
+  bool naiveCTUEnabled();
+
 public:
   AnalyzerOptions() :
 AnalysisStoreOpt(RegionStoreModel),

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=326439&r1=326438&r2=326439&view=diff
==
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
(original)
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h Thu 
Mar  1 06:54:16 2018
@@ -38,6 +38,10 @@ class CXXThisExpr;
 class MaterializeTemporaryExpr;
 class ObjCAtSynchronizedStmt;
 class ObjCForCollectionStmt;
+
+namespace cross_tu {
+class CrossTranslationUnitContext;
+}
   
 namespace ento {
 
@@ -74,6 +78,8 @@ public:
   };
 
 private:
+  cross_tu::CrossTranslationUnitContext &CTU;
+
   AnalysisManager &AMgr;
   
   AnalysisDeclContextManager &AnalysisDeclContexts;
@@ -115,10 +121,9 @@ private:
   InliningModes HowToInline;
 
 public:
-  ExprEngine(AnalysisManager &mgr, bool gcEnabled,
- SetOfConstDecls *VisitedCalleesIn,
- FunctionSummariesTy *FS,
- InliningModes HowToInlineIn);
+  ExprEngine(cross_tu::CrossTranslationUnitContext &CTU, AnalysisManager &mgr,
+ bool gcEnabled, SetOfConstDecls *VisitedCalleesIn,
+ FunctionSummariesTy *FS, InliningModes HowToInlineIn);
 
   ~ExprEngine() override;
 
@@ -150,6 +155,11 @@ public:
 
   BugReporter& getBugReporter() { return BR; }
 
+  cross_tu::CrossTranslationUnitContext *
+  getCrossTranslationUnitContext() override {
+return &CTU;
+  }
+
   const NodeBuilderContext &getBuilderContext() {
 assert(currBldrCtx);
 return *currBldrCtx;

Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h

Re: [PATCH] D30691: [analyzer] Support for naive cross translational unit analysis

[Ilya Biryukov via cfe-commits]

Resubmitted as r326439. Sorry for all the trouble.

We need to hack around the Analyses.def being required by Frontend, but it
would nice to remove this dependency upstream.


On Thu, Mar 1, 2018 at 3:34 PM Benjamin Kramer  wrote:

> Frontend depends on StaticAnalyzerCore by
> including "clang/StaticAnalyzer/Core/Analyses.def". That's a clear layering
> violation, but cmake doesn't model header dependencies. Maybe Analyses.def
> should move into its own library.
>
>
> On Thu, Mar 1, 2018 at 3:07 PM Gábor Horváth via llvm-commits <
> llvm-comm...@lists.llvm.org> wrote:
>
>>
>>
>> 2018. márc. 1. 14:58 ezt írta ("Ilya Biryukov" ):
>>
>> I replied to a commit in the wrong thread (
>> https://reviews.llvm.org/rL326323), sorry.
>> Here are the important bits:
>>
>> This change introduced the following cyclic dependency in the build
>> system: StaticAnalyzerCore -> CrossTU -> Frontend -> StaticAnalyzerCore.
>>
>>
>> I do not see the Frontend -> StaticAnalyzerCore dependency upstream. See:
>> https://github.com/llvm-mirror/clang/blob/master/lib/Frontend/CMakeLists.txt
>>
>> Do I miss something here?
>>
>> Thanks in advance,
>> Gábor
>>
>>
>> I'm sorry, but I had to revert the commit in r326432. Cyclic dependency
>> broke our internal integrate (we use a different buildsystem, which breaks
>> on cyclic deps) and it's really messy to workaround it since CrossTU
>> depends on both Index and Frontend.
>> Moving the code that uses CrossTU from StaticAnalyzerCore to
>> StaticAnalyzerFrontend should probably fix it, but I don't have enough
>> context to come up with a fix.
>>
>>
>>
>> On Thu, Mar 1, 2018 at 2:01 PM Aleksei Sidorin via Phabricator via
>> cfe-commits  wrote:
>>
>>> a.sidorin reopened this revision.
>>> a.sidorin added a comment.
>>>
>>> The changes were reverted:
>>> http://llvm.org/viewvc/llvm-project?rev=326432&view=rev
>>> Gabor, could you take a look?
>>>
>>>
>>> Repository:
>>>   rC Clang
>>>
>>> https://reviews.llvm.org/D30691
>>>
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>>
>> ___
>> llvm-commits mailing list
>> llvm-comm...@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>

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

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

In https://reviews.llvm.org/D42684#1022219, @Typz wrote:

> Indeed, seems to apply to classes as well. Maybe I was mislead by my testing, 
> where I did not get the case (possibly because we use 
> `ConstructorInitializerAllOnOneLineOrOnePerLine=true`, so the continuation 
> indenter only sees "short" class declarations unless breaking the template is 
> required because the name is too long).


Not sure how that style flag is related to class declarations, but ok ;).

> So just to be clear, are you saying the whole approach is not the right one, 
> or simply that the "names" of each modes are not?
>  For the name, maybe something like may be better:
> 
> - `Never`
> - `MultiLineDeclaration`, or maybe even `MultiLine`
> - `Always`

Ah, yeah, maybe it's just the name. "AlwaysBreak: Never" and "AlwaysBreak: 
Always" seem not very intuitive to interpret, though. How about just: No, Yes, 
MultiLine?


Repository:
  rC Clang

https://reviews.llvm.org/D42684



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

[PATCH] D42787: clang-format: do not add extra indent when wrapping last parameter

[Daniel Jasper via Phabricator via cfe-commits]

djasper added a comment.

Are you sure that you are even addressing an important case? I have done some 
research on our codebase and this is something that happens incredibly rarely. 
The reason is that you have to have a very specific combination of line length, 
where the last parameter does not fit on one line if indented back to align 
with the open paren while it does fit on multiple lines if indented right of 
the comma. In all of LLVM/Clang, there seem to be only about 50 cases. How 
certain are you that people actually care?


Repository:
  rC Clang

https://reviews.llvm.org/D42787



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

[PATCH] D41102: Setup clang-doc frontend framework

[Julie Hockett via Phabricator via cfe-commits]

juliehockett updated this revision to Diff 136520.
juliehockett marked 14 inline comments as done.
juliehockett added a comment.

Fixing comments and adding tests


https://reviews.llvm.org/D41102

Files:
  CMakeLists.txt
  clang-doc/BitcodeWriter.cpp
  clang-doc/BitcodeWriter.h
  clang-doc/CMakeLists.txt
  clang-doc/ClangDoc.h
  clang-doc/Mapper.cpp
  clang-doc/Mapper.h
  clang-doc/Representation.h
  clang-doc/Serialize.cpp
  clang-doc/Serialize.h
  clang-doc/tool/CMakeLists.txt
  clang-doc/tool/ClangDocMain.cpp
  docs/clang-doc.rst
  test/CMakeLists.txt
  test/clang-doc/mapper-class-in-class.cpp
  test/clang-doc/mapper-class-in-function.cpp
  test/clang-doc/mapper-class.cpp
  test/clang-doc/mapper-enum.cpp
  test/clang-doc/mapper-function.cpp
  test/clang-doc/mapper-method.cpp
  test/clang-doc/mapper-namespace.cpp
  test/clang-doc/mapper-struct.cpp
  test/clang-doc/mapper-union.cpp

Index: test/clang-doc/mapper-union.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-union.cpp
@@ -0,0 +1,28 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@u...@d.bc --dump | FileCheck %s
+
+union D { int X; int Y; };
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@U@D'
+  // CHECK:  blob data = 'D'
+  // CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'D::X'
+// CHECK: 
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'D::Y'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-struct.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-struct.cpp
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@c.bc --dump | FileCheck %s
+
+struct C { int i; };
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@S@C'
+  // CHECK:  blob data = 'C'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'int'
+// CHECK:  blob data = 'C::i'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-namespace.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-namespace.cpp
@@ -0,0 +1,51 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@n...@a.bc --dump | FileCheck %s
+
+/// \brief Brief description.
+///
+/// Extended description that
+/// continues onto the next line.
+namespace A {}
+// CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'c:@N@A'
+  // CHECK:  blob data = 'A'
+  // CHECK: 
+// CHECK:  blob data = 'FullComment'
+// CHECK: 
+  // CHECK:  blob data = 'ParagraphComment'
+  // CHECK: 
+// CHECK:  blob data = 'TextComment'
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'BlockCommandComment'
+  // CHECK:  blob data = 'brief'
+  // CHECK: 
+// CHECK:  blob data = 'ParagraphComment'
+// CHECK: 
+  // CHECK:  blob data = 'TextComment'
+  // CHECK:  blob data = ' Brief description.'
+// CHECK: 
+  // CHECK: 
+// CHECK: 
+// CHECK: 
+  // CHECK:  blob data = 'ParagraphComment'
+  // CHECK: 
+// CHECK:  blob data = 'TextComment'
+// CHECK:  blob data = ' Extended description that'
+  // CHECK: 
+  // CHECK: 
+// CHECK:  blob data = 'TextComment'
+// CHECK:  blob data = ' continues onto the next line.'
+  // CHECK: 
+// CHECK: 
+  // CHECK: 
+// CHECK: 
Index: test/clang-doc/mapper-method.cpp
===
--- /dev/null
+++ test/clang-doc/mapper-method.cpp
@@ -0,0 +1,45 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: echo "" > %t/compile_flags.txt
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-doc --dump --omit-filenames -doxygen -p %t %t/test.cpp -output=%t/docs
+// RUN: llvm-bcanalyzer %t/docs/c:@S@G@F@Method#I#.bc --dump | FileCheck %s --check-prefix CHECK-G-F
+// RUN: llvm-bcanalyzer %t/docs/c:@s...@g.bc --dump | FileCheck %s --check-prefix CHECK-G
+
+class G {
+public: 
+	int Method(int param) { return param; }
+};
+
+// CHECK-G: 
+// CHECK-G: 
+  // CHECK-G: 
+// CHECK-G: 
+// CHECK-G: 
+  // CHECK-G:  blob data = 'c:@S@G'
+  // CHECK-G:  blob data = 'G'
+  // CHECK-G: 
+  // CHECK-G: 
+// CHECK-G: 
+
+
+// CHECK-G-F: 
+// CHECK-G-F

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Hi Aaron.  It occurs to me now that this patch has grown rather large and, in 
some places, a little subtle.  Would it help the review if I were to break it 
up into a patch series that introduces ParamIdx to each attribute, one at a 
time?  I'm not trying to rush you, but I hate for the review to be painful for 
you if it doesn't have to be.


https://reviews.llvm.org/D43248



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

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In https://reviews.llvm.org/D43847#1023642, @Eugene.Zelenko wrote:

> May be //abseil// is better name for module?


I'd also go for "abseil". I'll try to get abseil folks to review this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43660: [OpenMP] Add OpenMP data sharing infrastructure using global memory

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 136528.

Repository:
  rC Clang

https://reviews.llvm.org/D43660

Files:
  lib/CodeGen/CGDecl.cpp
  lib/CodeGen/CGOpenMPRuntime.cpp
  lib/CodeGen/CGOpenMPRuntime.h
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  lib/CodeGen/CGStmtOpenMP.cpp
  lib/CodeGen/CodeGenFunction.cpp
  test/OpenMP/nvptx_data_sharing.cpp
  test/OpenMP/nvptx_parallel_codegen.cpp

Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -64,254 +64,243 @@
 
   // CHECK-NOT: define {{.*}}void {{@__omp_offloading_.+template.+l17}}_worker()
 
+// CHECK-LABEL: define {{.*}}void {{@__omp_offloading_.+template.+l26}}_worker()
+// CHECK-DAG: [[OMP_EXEC_STATUS:%.+]] = alloca i8,
+// CHECK-DAG: [[OMP_WORK_FN:%.+]] = alloca i8*,
+// CHECK: store i8* null, i8** [[OMP_WORK_FN]],
+// CHECK: store i8 0, i8* [[OMP_EXEC_STATUS]],
+// CHECK: br label {{%?}}[[AWAIT_WORK:.+]]
+//
+// CHECK: [[AWAIT_WORK]]
+// CHECK: call void @llvm.nvvm.barrier0()
+// CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]]
+// CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
+// store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
+// CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
+// CHECK: [[SHOULD_EXIT:%.+]] = icmp eq i8* [[WORK]], null
+// CHECK: br i1 [[SHOULD_EXIT]], label {{%?}}[[EXIT:.+]], label {{%?}}[[SEL_WORKERS:.+]]
+//
+// CHECK: [[SEL_WORKERS]]
+// CHECK: [[ST:%.+]] = load i8, i8* [[OMP_EXEC_STATUS]]
+// CHECK: [[IS_ACTIVE:%.+]] = icmp ne i8 [[ST]], 0
+// CHECK: br i1 [[IS_ACTIVE]], label {{%?}}[[EXEC_PARALLEL:.+]], label {{%?}}[[BAR_PARALLEL:.+]]
+//
+// CHECK: [[EXEC_PARALLEL]]
+// CHECK: [[WF1:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
+// CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i16, i32)* [[PARALLEL_FN1:@.+]]_wrapper to i8*)
+// CHECK: br i1 [[WM1]], label {{%?}}[[EXEC_PFN1:.+]], label {{%?}}[[CHECK_NEXT1:.+]]
+//
+// CHECK: [[EXEC_PFN1]]
+// CHECK: call void [[PARALLEL_FN1]]_wrapper(
+// CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
+//
+// CHECK: [[CHECK_NEXT1]]
+// CHECK: [[WF2:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
+// CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i16, i32)* [[PARALLEL_FN2:@.+]]_wrapper to i8*)
+// CHECK: br i1 [[WM2]], label {{%?}}[[EXEC_PFN2:.+]], label {{%?}}[[CHECK_NEXT2:.+]]
+//
+// CHECK: [[EXEC_PFN2]]
+// CHECK: call void [[PARALLEL_FN2]]_wrapper(
+// CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
+//
+// CHECK: [[CHECK_NEXT2]]
+// CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
+//
+// CHECK: [[TERM_PARALLEL]]
+// CHECK: call void @__kmpc_kernel_end_parallel()
+// CHECK: br label {{%?}}[[BAR_PARALLEL]]
+//
+// CHECK: [[BAR_PARALLEL]]
+// CHECK: call void @llvm.nvvm.barrier0()
+// CHECK: br label {{%?}}[[AWAIT_WORK]]
+//
+// CHECK: [[EXIT]]
+// CHECK: ret void
 
+// CHECK: define {{.*}}void [[T6:@__omp_offloading_.+template.+l26]](i[[SZ:32|64]]
+// Create local storage for each capture.
+// CHECK:  [[LOCAL_A:%.+]] = alloca i[[SZ]],
+// CHECK-DAG:  store i[[SZ]] [[ARG_A:%.+]], i[[SZ]]* [[LOCAL_A]]
+// Store captures in the context.
+// CHECK-64-DAG:[[REF_A:%.+]] = bitcast i[[SZ]]* [[LOCAL_A]] to i32*
+//
+// CHECK-DAG: [[TID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
+// CHECK-DAG: [[NTH:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
+// CHECK-DAG: [[WS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
+// CHECK-DAG: [[TH_LIMIT:%.+]] = sub i32 [[NTH]], [[WS]]
+// CHECK: [[IS_WORKER:%.+]] = icmp ult i32 [[TID]], [[TH_LIMIT]]
+// CHECK: br i1 [[IS_WORKER]], label {{%?}}[[WORKER:.+]], label {{%?}}[[CHECK_MASTER:.+]]
+//
+// CHECK: [[WORKER]]
+// CHECK: {{call|invoke}} void [[T6]]_worker()
+// CHECK: br label {{%?}}[[EXIT:.+]]
+//
+// CHECK: [[CHECK_MASTER]]
+// CHECK-DAG: [[CMTID:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.tid.x()
+// CHECK-DAG: [[CMNTH:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
+// CHECK-DAG: [[CMWS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
+// CHECK: [[IS_MASTER:%.+]] = icmp eq i32 [[CMTID]],
+// CHECK: br i1 [[IS_MASTER]], label {{%?}}[[MASTER:.+]], label {{%?}}[[EXIT]]
+//
+// CHECK: [[MASTER]]
+// CHECK-DAG: [[MNTH:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.ntid.x()
+// CHECK-DAG: [[MWS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
+// CHECK: [[MTMP1:%.+]] = sub i32 [[MNTH]], [[MWS]]
+// CHECK: call void @__kmpc_kernel_init(i32 [[MTMP1]]
+// CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32)* [[PARALLEL_FN1]]_wrapper to i8*),
+// CHECK: call void @llvm.nvvm.barrier0()
+// CHECK: call void @llvm.nvvm.barrier0()
+// CHECK: call void @__kmpc_serialized_parallel(
+// CHECK: {{call|invoke}} void [[PARALLEL_FN3:@.+]](
+// CHECK: call void @__kmpc_end_serialized_parallel(
+// CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32)* [[PARALLEL_FN2]]_wrapper to i8*),
+// CHECK: call void @

[PATCH] D43660: [OpenMP] Add OpenMP data sharing infrastructure using global memory

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

https://reviews.llvm.org/D43660



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

[PATCH] D43500: [clang-tidy]: modernize-use-default-member-init: Remove trailing comma and colon.

[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: unittests/clang-tidy/ClangTidyTest.h:145
+
+  if (Options.FormatStyle) {
+llvm::Expected Style = format::getStyle(

I wonder whether it's better to use lit for the tests that require formatting 
than to expand clang-tidy unit test utilities with runCheckAndFormatOnCode? 
What was the reason to use unit tests in this patch as opposed to lit tests?


https://reviews.llvm.org/D43500



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

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion",
+ getDefaultMinimumLanguageVersion())) 
{}

ftingaud wrote:
> aaron.ballman wrote:
> > Why is this is a user-facing option?
> > 
> > If it needs to be a user-facing option, you also need to implement an 
> > override for `storeOptions()` as well.
> As the rule was very customizable, I also made the c++ version customizable. 
> But the option is only useful if a user has a custom make_unique that needs 
> c++14 (uses std::make_unique internally) and multiple versions of C++ in 
> their codeline. I agree this is a rather specific usecase. I can remove it 
> and make the code simpler if it is your recommendation.
> if a user has a custom make_unique that needs c++14

The only reason to have a custom make_unique that I know is to workaround the 
absence of std::make_unique in c++11. So this looks like a not very useful case 
to support.


https://reviews.llvm.org/D43766



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

[PATCH] D43817: [x86] wbnoinvd intrinsic

[Craig Topper via Phabricator via cfe-commits]

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

LGTM


https://reviews.llvm.org/D43817



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

[PATCH] D43162: [Parser] (C++) Make -Wextra-semi slightly more useful

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

**If** i'm reading `git blame` correctly, the `-Wnewline-eof` diag, which i 
used as a base to model the previous version of this diff on,
was added in https://reviews.llvm.org/rL189110 / 
https://github.com/llvm-mirror/clang/commit/7865b8e4324378e06f59adb4d60bec26a7d3d584
 by @jordan_rose.
Since the @rsmith's review note suggests the approach is incorrect, i've added 
the author of that commit as a subscriber, just in case.


Repository:
  rC Clang

https://reviews.llvm.org/D43162



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

[PATCH] D43108: Support for the mno-stack-arg-probe flag

[Ruslan Nikolaev via Phabricator via cfe-commits]

nruslan added a comment.

@aemerson : yes, it is just part of MS ABI


Repository:
  rC Clang

https://reviews.llvm.org/D43108



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

[PATCH] D43928: [analyzer] Correctly measure array size in security.insecureAPI.strcpy

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:517
 if (const auto *Array = dyn_cast(DeclRef->getType())) {
-  uint64_t ArraySize = BR.getContext().getTypeSize(Array) / 8;
+  auto ArraySize = BR.getContext().getTypeSizeInChars(Array).getQuantity();
   if (const auto *String = dyn_cast(Source)) {

I suggest not using `auto` here, because it makes it harder to understand 
integer promotions (potential overflows or sign extensions) in the comparison.


https://reviews.llvm.org/D43928



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

[PATCH] D43953: clangformat-vs: Fix plugin not correctly loading in some cases

[François-Xavier Roure via Phabricator via cfe-commits]

fxroure-ubisoft created this revision.
fxroure-ubisoft added a reviewer: clang.
fxroure-ubisoft created this object with edit policy "Only User: 
fxroure-ubisoft (François-Xavier Roure)".
Herald added a subscriber: cfe-commits.

When installing the clang-format visual studio plugin 

 sometimes it won't be loaded correctly (some users seem to have the same issue 
)
 and we must do

  devenv.exe /updateconfiguration

in order to fix the installation (normally visual studio should automatically 
execute this command whenever you install a plugin).
This issue seems to come from the fact that the plugin manifest does not list a 
VsPackage as an asset.
After adding it to assets list (code change in this review) installation of the 
plugin was working and when uninstalling the plugin visual correctly ask to 
restart it (that was not the case before).


Repository:
  rC Clang

https://reviews.llvm.org/D43953

Files:
  tools/clang-format-vs/source.extension.vsixmanifest.in


Index: tools/clang-format-vs/source.extension.vsixmanifest.in
===
--- tools/clang-format-vs/source.extension.vsixmanifest.in
+++ tools/clang-format-vs/source.extension.vsixmanifest.in
@@ -16,4 +16,7 @@
   
 
   
+  
+
+  
 


Index: tools/clang-format-vs/source.extension.vsixmanifest.in
===
--- tools/clang-format-vs/source.extension.vsixmanifest.in
+++ tools/clang-format-vs/source.extension.vsixmanifest.in
@@ -16,4 +16,7 @@
   
 
   
+  
+
+  
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r326452 - [clangd] Forward all environment variables along with CLANGD_TRACE to clangd.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Mar  1 09:42:27 2018
New Revision: 326452

URL: http://llvm.org/viewvc/llvm-project?rev=326452&view=rev
Log:
[clangd] Forward all environment variables along with CLANGD_TRACE to clangd.

Modified:
clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts

Modified: clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts?rev=326452&r1=326451&r2=326452&view=diff
==
--- clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts 
(original)
+++ clang-tools-extra/trunk/clangd/clients/clangd-vscode/src/extension.ts Thu 
Mar  1 09:42:27 2018
@@ -23,8 +23,10 @@ export function activate(context: vscode
 args: getConfig('arguments')
 };
 const traceFile = getConfig('trace');
-if (traceFile != null)
-clangd.options = {env: {CLANGD_TRACE: traceFile}};
+if (traceFile != '') {
+  const trace = {CLANGD_TRACE : traceFile};
+  clangd.options = {env : {...process.env, ...trace}};
+}
 const serverOptions: vscodelc.ServerOptions = clangd;
 
 const filePattern: string = '**/*.{' +


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

[PATCH] D43281: [AMDGPU] fixes for lds f32 builtins

[Daniil Fukalov via Phabricator via cfe-commits]

dfukalov added a comment.

The problem is that if set addrspace "2" in description string, 
CanT.getAddressSpace() returns target addrspace value "11" (shifted in the 
enum) and compares it with input LangAS addrspace ("2", "opencl_local" in our 
case).
So I cannot set a number a description string that will be equal to LangAS 
addrspace "opencl_local".

Moreover, this change is preparation to move to custom processing of these 
builtins. Then I'm going to remove link (GCCBuiltin in IntrinsicsAMDGPU.td) 
from the llvm intrinsics definitions. Then I'll be able to switch on custom 
processing in cfe.


https://reviews.llvm.org/D43281



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

[PATCH] D43015: clang-format: Introduce BreakInheritanceList option

[Francois Ferrand via Phabricator via cfe-commits]

Typz added a comment.

ping?


Repository:
  rC Clang

https://reviews.llvm.org/D43015



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

[PATCH] D42730: [clang-tidy]] Add check for use of types/classes/functions from header which are deprecated and removed in C++17

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/DeprecatedFunctionalCheck.cpp:48-54
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("ptr_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::ptr_fun'";
+  } else if (const auto *const Call =
+ Result.Nodes.getNodeAs("mem_fun_call")) {
+diag(Call->getLocStart(), Message) << "'std::mem_fun'";
+  }

alexfh wrote:
> alexfh wrote:
> > aaron.ballman wrote:
> > > massberg wrote:
> > > > aaron.ballman wrote:
> > > > > I think that this code should be generalized (same with the matchers) 
> > > > > so that you match on `hasAnyName()` for the function calls and use 
> > > > > `CallExpr::getCalleeDecl()` to get the declaration. e.g.,
> > > > > ```
> > > > > if (const auto *Call = Result.Nodes.getNodeAs("blech")) {
> > > > >   if (const Decl *Callee = Call->getCalleeDecl())
> > > > > diag(Call->getLocStart(), Message) << Calleee;
> > > > > }
> > > > > ```
> > > > > This way you can add more named without having to add extra logic for 
> > > > > the diagnostics.
> > > > I generalized the code and the matcher.
> > > > When we use
> > > > ```
> > > > << cast(Callee);
> > > > ```
> > > > we get the template arguments in the name , e.g. `ptr_fun`, 
> > > > so I chose to use `getQualifiedNameAsString`.
> > > > If there is there a better way to get the function name without 
> > > > template arguments I appreciate any suggestions.
> > > > 
> > > > 
> > > Nope, in that case, your code is correct. However, we generally provide 
> > > the template arguments in diagnostics. I see @alexfh was asking for them 
> > > to be removed as not being useful, but I'm not certain I agree with his 
> > > rationale. Yes, all instances are deprecated and thus the template 
> > > arguments don't discern between good and bad cases, but showing the 
> > > template arguments is also consistent with the other diagnostics we emit. 
> > > For instance, other "deprecated" diagnostics also emit the template 
> > > arguments. I'm not certain we should be inconsistent with the way we 
> > > produce diagnostics, but I'll defer to Alex if he still feels strongly 
> > > about leaving them off here.
> > Indeed, -Wdeprecated-declarations warnings print template arguments. 
> > Moreover, they seem to be issued only on instantiations, see 
> > https://godbolt.org/g/W563gw.
> > 
> > But I have a number of concerns with template arguments in the deprecation 
> > warnings:
> > 
> > 1. The note attached to the warning lies. Consider a warning from the test 
> > above:
> >   ...
> >   :11:1: warning: 'B' is deprecated: bbb 
> > [-Wdeprecated-declarations]
> >   B e;
> >   ^
> >   :7:10: note: 'B' has been explicitly marked deprecated here
> >   struct [[deprecated("bbb")]] B {};
> > 
> >  But `B` hasn't been explicitly marked deprecated, only the template 
> > definition of `B` has been. Template arguments are important in the case of 
> > the explicit template specialization `A` in the same example, but not 
> > in cases where the template definition was marked deprecated, since 
> > template arguments only add noise and no useful information there.
> > 2. Warnings can easily become unreadable: https://godbolt.org/g/AFdznH
> > 3. Certain coding patterns can result in numerous deprecation warnings 
> > differing only in template arguments: https://godbolt.org/g/U2JCbG. 
> > Clang-tidy can deduplicate warnings, if they have identical text and 
> > location, but adding template arguments to the message will prevent 
> > deduplication. I've seen a case where thousands of deprecation warnings 
> > were generated for a single line in a widely used header.
> > 
> > So yes, I feel strongly about leaving off template arguments in case the 
> > whole template was marked deprecated. I think it would be the right thing 
> > to do for the -Wdeprecated-declarations diagnostic as well.
> s/leaving off/leaving out/
> The note attached to the warning lies.

No it doesn't? The attribute is inherited from the primary template unless your 
explicit specialization *removes* the attribute. https://godbolt.org/g/ZuXZds

> Warnings can easily become unreadable

This is true of all template diagnostics and isn't specific to clang-tidy's 
treatment of them.

> I've seen a case where thousands of deprecation warnings were generated for a 
> single line in a widely used header.

This sounds more worrying, but at the same time, your link behaving by design 
and doing what I would want it to do. The presence of the deprecated primary 
template isn't what gets diagnosed, it's the *uses* of the deprecated entity. 
This is called out explicitly in [dcl.attr.deprecated]p4.

> So yes, I feel strongly about leaving off template arguments in case the 
> whole template was marked deprecated. I think it would be the right thing to 
> do for the -Wdeprecated-declarations diagnostic as well.

I would be strongly opposed to that change 

[PATCH] D42684: clang-format: Allow optimizer to break template declaration.

[Francois Ferrand via Phabricator via cfe-commits]

Typz updated this revision to Diff 136557.
Typz added a comment.

Change options values to No/MultiLine/Yes.


Repository:
  rC Clang

https://reviews.llvm.org/D42684

Files:
  include/clang/Format/Format.h
  lib/Format/ContinuationIndenter.cpp
  lib/Format/Format.cpp
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -5445,7 +5445,7 @@
"const typename  aaa);");
 
   FormatStyle AlwaysBreak = getLLVMStyle();
-  AlwaysBreak.AlwaysBreakTemplateDeclarations = true;
+  AlwaysBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
   verifyFormat("template \nclass C {};", AlwaysBreak);
   verifyFormat("template \nvoid f();", AlwaysBreak);
   verifyFormat("template \nvoid f() {}", AlwaysBreak);
@@ -5463,6 +5463,32 @@
"public:\n"
"  E *f();\n"
"};");
+
+  FormatStyle NeverBreak = getLLVMStyle();
+  NeverBreak.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_No;
+  verifyFormat("template  class C {};", NeverBreak);
+  verifyFormat("template  void f();", NeverBreak);
+  verifyFormat("template  void f() {}", NeverBreak);
+  verifyFormat("template \nvoid foo(aa ) {}",
+   NeverBreak);
+  verifyFormat("void aaa(\n"
+   "ccc);",
+   NeverBreak);
+  verifyFormat("template  class Fooo,\n"
+   "  template  class Baaar>\n"
+   "struct C {};",
+   NeverBreak);
+  verifyFormat("template  // T can be A, B or C.\n"
+   "struct C {};",
+   NeverBreak);
+  verifyFormat("template  class A {\n"
+   "public:\n"
+   "  E *f();\n"
+   "};", NeverBreak);
+  NeverBreak.PenaltyBreakTemplateDeclaration = 100;
+  verifyFormat("template  void\nfoo(aa ) {}",
+   NeverBreak);
 }
 
 TEST_F(FormatTest, WrapsTemplateParameters) {
@@ -10352,7 +10378,6 @@
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortIfStatementsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
-  CHECK_PARSE_BOOL(AlwaysBreakTemplateDeclarations);
   CHECK_PARSE_BOOL(BinPackArguments);
   CHECK_PARSE_BOOL(BinPackParameters);
   CHECK_PARSE_BOOL(BreakAfterJavaFieldAnnotations);
@@ -10418,6 +10443,8 @@
   PenaltyBreakAssignment, 1234u);
   CHECK_PARSE("PenaltyBreakBeforeFirstCallParameter: 1234",
   PenaltyBreakBeforeFirstCallParameter, 1234u);
+  CHECK_PARSE("PenaltyBreakTemplateDeclaration: 1234",
+  PenaltyBreakTemplateDeclaration, 1234u);
   CHECK_PARSE("PenaltyExcessCharacter: 1234", PenaltyExcessCharacter, 1234u);
   CHECK_PARSE("PenaltyReturnTypeOnItsOwnLine: 1234",
   PenaltyReturnTypeOnItsOwnLine, 1234u);
@@ -10572,6 +10599,18 @@
   AlwaysBreakAfterReturnType,
   FormatStyle::RTBS_TopLevelDefinitions);
 
+  Style.AlwaysBreakTemplateDeclarations = FormatStyle::BTDS_Yes;
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: No", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_No);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: MultiLine", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_MultiLine);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: Yes", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_Yes);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: false", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_MultiLine);
+  CHECK_PARSE("AlwaysBreakTemplateDeclarations: true", AlwaysBreakTemplateDeclarations,
+  FormatStyle::BTDS_Yes);
+
   Style.AlwaysBreakAfterDefinitionReturnType = FormatStyle::DRTBS_All;
   CHECK_PARSE("AlwaysBreakAfterDefinitionReturnType: None",
   AlwaysBreakAfterDefinitionReturnType, FormatStyle::DRTBS_None);
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2244,6 +2244,8 @@
   return 2;
 return 1;
   }
+  if (Left.ClosesTemplateDeclaration)
+  return Style.PenaltyBreakTemplateDeclaration;
   if (Left.is(TT_ConditionalExpr))
 return prec::Conditional;
   prec::Level Level = Left.getPrecedence();
@@ -2753,7 +2755,7 @@
   if (Right.Previous->ClosesTemplateDeclaration &&
   Right.Previous->MatchingParen &&
   Right.Previous->MatchingParen->NestingLevel == 0 &&
-  Style.AlwaysBreakTemplateDeclarations)
+  Style.AlwaysBreakTemplateDeclarations == FormatStyle::BTDS_Yes)
 return true;
   if (Right.is(TT_CtorInitializerComma) &&
   Style.BreakConstructorInitializers == F

[PATCH] D43800: [ASTMatchers] Allow file-based narrowing matches to work with NestedNameSpecifierLocs.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Please add some test coverage for the NNS case.


Repository:
  rC Clang

https://reviews.llvm.org/D43800



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

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion",
+ getDefaultMinimumLanguageVersion())) 
{}

alexfh wrote:
> ftingaud wrote:
> > aaron.ballman wrote:
> > > Why is this is a user-facing option?
> > > 
> > > If it needs to be a user-facing option, you also need to implement an 
> > > override for `storeOptions()` as well.
> > As the rule was very customizable, I also made the c++ version 
> > customizable. But the option is only useful if a user has a custom 
> > make_unique that needs c++14 (uses std::make_unique internally) and 
> > multiple versions of C++ in their codeline. I agree this is a rather 
> > specific usecase. I can remove it and make the code simpler if it is your 
> > recommendation.
> > if a user has a custom make_unique that needs c++14
> 
> The only reason to have a custom make_unique that I know is to workaround the 
> absence of std::make_unique in c++11. So this looks like a not very useful 
> case to support.
Agreed, I'd drop it.


https://reviews.llvm.org/D43766



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

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:21
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  MinimumLanguageVersion(Options.get("MakeUniqueLanguageVersion",
+ getDefaultMinimumLanguageVersion())) 
{}

aaron.ballman wrote:
> alexfh wrote:
> > ftingaud wrote:
> > > aaron.ballman wrote:
> > > > Why is this is a user-facing option?
> > > > 
> > > > If it needs to be a user-facing option, you also need to implement an 
> > > > override for `storeOptions()` as well.
> > > As the rule was very customizable, I also made the c++ version 
> > > customizable. But the option is only useful if a user has a custom 
> > > make_unique that needs c++14 (uses std::make_unique internally) and 
> > > multiple versions of C++ in their codeline. I agree this is a rather 
> > > specific usecase. I can remove it and make the code simpler if it is your 
> > > recommendation.
> > > if a user has a custom make_unique that needs c++14
> > 
> > The only reason to have a custom make_unique that I know is to workaround 
> > the absence of std::make_unique in c++11. So this looks like a not very 
> > useful case to support.
> Agreed, I'd drop it.
From the peanut gallery: IIUC, yes, this is not a useful case to support.
If the user has a custom `my::make_unique`, then it is *by definition* usable 
in C++11. Now, it might still be implemented as a call to `std::make_unique` in 
C++14... but it will not *require* C++14.
The user's `my::make_unique` will likely require C++11 (not just C++03), but I 
personally don't think clang-tidy ought to cater to corner cases that involve 
C++03. I'd just assume that an instruction to "use `my::make_unique`" is 
appropriate for any translation unit that's being tidied with a custom 
make_unique.



Comment at: test/clang-tidy/modernize-make-unique-cxx14.cpp:10
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead
+  // CHECK-FIXES: auto my_ptr = std::make_unique(1);
+  return 0;

IIUC above, you allow the user to pass in the name of a custom 
`my::make_unique` function. Could you add a test case for that?


https://reviews.llvm.org/D43766



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

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 136561.
ioeric marked 2 inline comments as done.
ioeric added a comment.

- Merge with origin/master
- Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869

Files:
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -547,6 +547,32 @@
 }
 #endif
 
+TEST_F(SymbolCollectorTest, STLiosfwd) {
+  CollectorOpts.CollectIncludePath = true;
+  CanonicalIncludes Includes;
+  addSystemHeadersMapping(&Includes);
+  CollectorOpts.Includes = &Includes;
+  // Symbols from  should be mapped individually.
+  TestHeaderName = testPath("iosfwd");
+  TestFileName = testPath("iosfwd.cpp");
+  std::string Header = R"(
+namespace std {
+  class no_map {};
+  class ios {};
+  class ostream {};
+  class filebuf {};
+} // namespace std
+  )";
+  runSymbolCollector(Header, /*Main=*/"");
+  EXPECT_THAT(Symbols,
+  UnorderedElementsAre(
+  QName("std"),
+  AllOf(QName("std::no_map"), IncludeHeader("")),
+  AllOf(QName("std::ios"), IncludeHeader("")),
+  AllOf(QName("std::ostream"), IncludeHeader("")),
+  AllOf(QName("std::filebuf"), IncludeHeader("";
+}
+
 TEST_F(SymbolCollectorTest, IWYUPragma) {
   CollectorOpts.CollectIncludePath = true;
   CanonicalIncludes Includes;
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -154,13 +154,13 @@
 /// FIXME: we should handle .inc files whose symbols are expected be exported by
 /// their containing headers.
 llvm::Optional
-getIncludeHeader(const SourceManager &SM, SourceLocation Loc,
- const SymbolCollector::Options &Opts) {
+getIncludeHeader(llvm::StringRef QName, const SourceManager &SM,
+ SourceLocation Loc, const SymbolCollector::Options &Opts) {
   llvm::StringRef FilePath = SM.getFilename(Loc);
   if (FilePath.empty())
 return llvm::None;
   if (Opts.Includes) {
-llvm::StringRef Mapped = Opts.Includes->mapHeader(FilePath);
+llvm::StringRef Mapped = Opts.Includes->mapHeader(FilePath, QName);
 if (Mapped != FilePath)
   return (Mapped.startswith("<") || Mapped.startswith("\""))
  ? Mapped.str()
@@ -316,8 +316,8 @@
   if (Opts.CollectIncludePath && shouldCollectIncludePath(S.SymInfo.Kind)) {
 // Use the expansion location to get the #include header since this is
 // where the symbol is exposed.
-if (auto Header =
-getIncludeHeader(SM, SM.getExpansionLoc(ND.getLocation()), Opts))
+if (auto Header = getIncludeHeader(
+QName, SM, SM.getExpansionLoc(ND.getLocation()), Opts))
   Include = std::move(*Header);
   }
   S.CompletionFilterText = FilterText;
Index: clangd/index/CanonicalIncludes.h
===
--- clangd/index/CanonicalIncludes.h
+++ clangd/index/CanonicalIncludes.h
@@ -43,9 +43,17 @@
   /// Maps all files matching \p RE to \p CanonicalPath
   void addRegexMapping(llvm::StringRef RE, llvm::StringRef CanonicalPath);
 
+  /// Sets the canonical include for any symbol with \p QualifiedName.
+  /// Symbol mappings take precedence over header mappings.
+  void addSymbolMapping(llvm::StringRef QualifiedName,
+llvm::StringRef CanonicalPath);
+
   /// \return \p Header itself if there is no mapping for it; otherwise, return
   /// a canonical header name.
-  llvm::StringRef mapHeader(llvm::StringRef Header) const;
+  /// \p QualifiedName of a symbol declared in \p Header can be provided to
+  /// check against the symbol mapping.
+  llvm::StringRef mapHeader(llvm::StringRef Header,
+llvm::StringRef QualifiedName = "") const;
 
 private:
   // A map from header patterns to header names. This needs to be mutable so
@@ -55,6 +63,8 @@
   // arbitrary regexes.
   mutable std::vector>
   RegexHeaderMappingTable;
+  // A map from fully qualified symbol names to header names.
+  llvm::StringMap SymbolMapping;
   // Guards Regex matching as it's not thread-safe.
   mutable std::mutex RegexMutex;
 };
@@ -68,8 +78,9 @@
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 
-/// Adds mapping for system headers. Approximately, the following system headers
-/// are handled:
+/// Adds mapping for system headers and some special symbols (e.g. STL symbols
+/// in  need to be mapped individually). Approximately, the following
+/// system headers are handled:
 ///   - C++ standard library e.g. bits/basic_string.h$ -> 
 ///   - Posix librar

[PATCH] D43847: [clang-tidy] Add check: replace string::find(...) == 0 with absl::StartsWith

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D43847#1023452, @hokein wrote:

> In https://reviews.llvm.org/D43847#1021967, @aaron.ballman wrote:
>
> > I need a bit more context because I'm unfamiliar with `absl`. What is this 
> > module's intended use?
>
>
> As `absl` has been open-sourced (https://github.com/abseil/abseil-cpp), I 
> think there will be more absl-related checks contributed from google or 
> external contributors in the future, so it make sense to create a new module.


Ah, so this is for abseil, good to know. Yeah, I think we should have a new 
module for it but perhaps with the full name instead of this shortened version?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43847



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

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/CanonicalIncludes.h:54
   /// a canonical header name.
-  llvm::StringRef mapHeader(llvm::StringRef Header) const;
+  /// An optional qualified symbol name can be provided to check against the
+  /// symbol mapping.

sammccall wrote:
> Why can't we always provide this?
I think it would be better to keep supporting the header-only mapping use case.



Comment at: unittests/clangd/SymbolCollectorTests.cpp:561
+  class no_map {};
+  class ios {};
+  class ostream {};

sammccall wrote:
> ioeric wrote:
> > hokein wrote:
> > > The STL implementation of `ios` is a typedef `typedef basic_ios  
> > > ios; `, I think we should make the test align with it?
> > The symbol mapping doesn't make a difference between symbol types, and 
> > symbol kind test is not in the scope of this patch,  so I think this 
> > wouldn't be necessary here.
> I'd be +1 on matching the real header a bit more so it's more obvious how 
> this relates to the standard library. As long as it's not too hard!
I'd prefer to keep the test case minimum so that it's clear that we are only 
matching on names.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869



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

[clang-tools-extra] r326456 - [clangd] Support include canonicalization in symbol leve.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Mar  1 10:06:40 2018
New Revision: 326456

URL: http://llvm.org/viewvc/llvm-project?rev=326456&view=rev
Log:
[clangd] Support include canonicalization in symbol leve.

Summary:
Symbols with different canonical includes might be defined in the same header
(e.g. symbols defined in STL ). This patch adds support for mapping from
qualified symbol names to canonical headers and special mapping for symbols in 


Reviewers: sammccall, hokein

Reviewed By: sammccall

Subscribers: klimek, ilya-biryukov, jkorous-apple, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=326456&r1=326455&r2=326456&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Thu Mar  1 
10:06:40 2018
@@ -27,7 +27,19 @@ void CanonicalIncludes::addRegexMapping(
   this->RegexHeaderMappingTable.emplace_back(llvm::Regex(RE), CanonicalPath);
 }
 
-llvm::StringRef CanonicalIncludes::mapHeader(llvm::StringRef Header) const {
+void CanonicalIncludes::addSymbolMapping(llvm::StringRef QualifiedName,
+ llvm::StringRef CanonicalPath) {
+  this->SymbolMapping[QualifiedName] = CanonicalPath;
+}
+
+llvm::StringRef
+CanonicalIncludes::mapHeader(llvm::StringRef Header,
+ llvm::StringRef QualifiedName) const {
+  if (!QualifiedName.empty()) {
+auto SE = SymbolMapping.find(QualifiedName);
+if (SE != SymbolMapping.end())
+  return SE->second;
+  }
   std::lock_guard Lock(RegexMutex);
   for (auto &Entry : RegexHeaderMappingTable) {
 #ifndef NDEBUG
@@ -67,6 +79,53 @@ collectIWYUHeaderMaps(CanonicalIncludes
 }
 
 void addSystemHeadersMapping(CanonicalIncludes *Includes) {
+  static const std::vector> SymbolMap = {
+  // Map symbols in  to their preferred includes.
+  {"std::basic_filebuf", ""},
+  {"std::basic_fstream", ""},
+  {"std::basic_ifstream", ""},
+  {"std::basic_ofstream", ""},
+  {"std::filebuf", ""},
+  {"std::fstream", ""},
+  {"std::ifstream", ""},
+  {"std::ofstream", ""},
+  {"std::wfilebuf", ""},
+  {"std::wfstream", ""},
+  {"std::wifstream", ""},
+  {"std::wofstream", ""},
+  {"std::basic_ios", ""},
+  {"std::ios", ""},
+  {"std::wios", ""},
+  {"std::basic_iostream", ""},
+  {"std::iostream", ""},
+  {"std::wiostream", ""},
+  {"std::basic_istream", ""},
+  {"std::istream", ""},
+  {"std::wistream", ""},
+  {"std::istreambuf_iterator", ""},
+  {"std::ostreambuf_iterator", ""},
+  {"std::basic_ostream", ""},
+  {"std::ostream", ""},
+  {"std::wostream", ""},
+  {"std::basic_istringstream", ""},
+  {"std::basic_ostringstream", ""},
+  {"std::basic_stringbuf", ""},
+  {"std::basic_stringstream", ""},
+  {"std::istringstream", ""},
+  {"std::ostringstream", ""},
+  {"std::stringbuf", ""},
+  {"std::stringstream", ""},
+  {"std::wistringstream", ""},
+  {"std::wostringstream", ""},
+  {"std::wstringbuf", ""},
+  {"std::wstringstream", ""},
+  {"std::basic_streambuf", ""},
+  {"std::streambuf", ""},
+  {"std::wstreambuf", ""},
+  };
+  for (const auto &Pair : SymbolMap)
+Includes->addSymbolMapping(Pair.first, Pair.second);
+
   static const std::vector>
   SystemHeaderMap = {
   {"include/__stddef_max_align_t.h$", ""},

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h?rev=326456&r1=326455&r2=326456&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h Thu Mar  1 
10:06:40 2018
@@ -43,9 +43,17 @@ public:
   /// Maps all files matching \p RE to \p CanonicalPath
   void addRegexMapping(llvm::StringRef RE, llvm::StringRef CanonicalPath);
 
+  /// Sets the canonical include for any symbol with \p QualifiedName.
+  /// Symbol mappings take precedence over header mappings.
+  void addSymbolMapping(llvm::StringRef QualifiedName,
+llvm::StringRef CanonicalPath);
+
   /// \return \p Header itself if there is no mapping for it; otherwise, return
   /// a canonical header name.
-  llvm::StringRef mapHeader(llvm::StringRef Header) const;
+  /// \p QualifiedNa

[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE326456: [clangd] Support include canonicalization in 
symbol leve. (authored by ioeric, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43869?vs=136561&id=136565#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869

Files:
  clangd/index/CanonicalIncludes.cpp
  clangd/index/CanonicalIncludes.h
  clangd/index/SymbolCollector.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: clangd/index/CanonicalIncludes.h
===
--- clangd/index/CanonicalIncludes.h
+++ clangd/index/CanonicalIncludes.h
@@ -43,9 +43,17 @@
   /// Maps all files matching \p RE to \p CanonicalPath
   void addRegexMapping(llvm::StringRef RE, llvm::StringRef CanonicalPath);
 
+  /// Sets the canonical include for any symbol with \p QualifiedName.
+  /// Symbol mappings take precedence over header mappings.
+  void addSymbolMapping(llvm::StringRef QualifiedName,
+llvm::StringRef CanonicalPath);
+
   /// \return \p Header itself if there is no mapping for it; otherwise, return
   /// a canonical header name.
-  llvm::StringRef mapHeader(llvm::StringRef Header) const;
+  /// \p QualifiedName of a symbol declared in \p Header can be provided to
+  /// check against the symbol mapping.
+  llvm::StringRef mapHeader(llvm::StringRef Header,
+llvm::StringRef QualifiedName = "") const;
 
 private:
   // A map from header patterns to header names. This needs to be mutable so
@@ -55,6 +63,8 @@
   // arbitrary regexes.
   mutable std::vector>
   RegexHeaderMappingTable;
+  // A map from fully qualified symbol names to header names.
+  llvm::StringMap SymbolMapping;
   // Guards Regex matching as it's not thread-safe.
   mutable std::mutex RegexMutex;
 };
@@ -68,8 +78,9 @@
 std::unique_ptr
 collectIWYUHeaderMaps(CanonicalIncludes *Includes);
 
-/// Adds mapping for system headers. Approximately, the following system headers
-/// are handled:
+/// Adds mapping for system headers and some special symbols (e.g. STL symbols
+/// in  need to be mapped individually). Approximately, the following
+/// system headers are handled:
 ///   - C++ standard library e.g. bits/basic_string.h$ -> 
 ///   - Posix library e.g. bits/pthreadtypes.h$ -> 
 ///   - Compiler extensions, e.g. include/avx512bwintrin.h$ -> 
Index: clangd/index/CanonicalIncludes.cpp
===
--- clangd/index/CanonicalIncludes.cpp
+++ clangd/index/CanonicalIncludes.cpp
@@ -27,7 +27,19 @@
   this->RegexHeaderMappingTable.emplace_back(llvm::Regex(RE), CanonicalPath);
 }
 
-llvm::StringRef CanonicalIncludes::mapHeader(llvm::StringRef Header) const {
+void CanonicalIncludes::addSymbolMapping(llvm::StringRef QualifiedName,
+ llvm::StringRef CanonicalPath) {
+  this->SymbolMapping[QualifiedName] = CanonicalPath;
+}
+
+llvm::StringRef
+CanonicalIncludes::mapHeader(llvm::StringRef Header,
+ llvm::StringRef QualifiedName) const {
+  if (!QualifiedName.empty()) {
+auto SE = SymbolMapping.find(QualifiedName);
+if (SE != SymbolMapping.end())
+  return SE->second;
+  }
   std::lock_guard Lock(RegexMutex);
   for (auto &Entry : RegexHeaderMappingTable) {
 #ifndef NDEBUG
@@ -67,6 +79,53 @@
 }
 
 void addSystemHeadersMapping(CanonicalIncludes *Includes) {
+  static const std::vector> SymbolMap = {
+  // Map symbols in  to their preferred includes.
+  {"std::basic_filebuf", ""},
+  {"std::basic_fstream", ""},
+  {"std::basic_ifstream", ""},
+  {"std::basic_ofstream", ""},
+  {"std::filebuf", ""},
+  {"std::fstream", ""},
+  {"std::ifstream", ""},
+  {"std::ofstream", ""},
+  {"std::wfilebuf", ""},
+  {"std::wfstream", ""},
+  {"std::wifstream", ""},
+  {"std::wofstream", ""},
+  {"std::basic_ios", ""},
+  {"std::ios", ""},
+  {"std::wios", ""},
+  {"std::basic_iostream", ""},
+  {"std::iostream", ""},
+  {"std::wiostream", ""},
+  {"std::basic_istream", ""},
+  {"std::istream", ""},
+  {"std::wistream", ""},
+  {"std::istreambuf_iterator", ""},
+  {"std::ostreambuf_iterator", ""},
+  {"std::basic_ostream", ""},
+  {"std::ostream", ""},
+  {"std::wostream", ""},
+  {"std::basic_istringstream", ""},
+  {"std::basic_ostringstream", ""},
+  {"std::basic_stringbuf", ""},
+  {"std::basic_stringstream", ""},
+  {"std::istringstream", ""},
+  {"std::ostringstream", ""},
+  {"std::stringbuf", ""},
+  {"std::stringstream", ""},
+  {"std::wistringstream", ""},
+  {"std::wostringstream", ""},
+  {"std::wstringbuf", ""},
+  {"std::wstringstream", ""},
+  {"std::basic_streambuf", ""},
+  {"std::streambuf", ""},
+  {"std::wstreambuf", ""},
+  };
+  for (const auto &Pair : SymbolM

[PATCH] D43745: Fix cppcoreguidelines-pro-bounds-pointer-arithmetic not working for functions with auto return specifier.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Some minor formatting nits, but otherwise LGTM. You should run the patch 
through clang-format: 
https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting




Comment at: clang-tidy/cppcoreguidelines/ProBoundsPointerArithmeticCheck.cpp:29
 hasOperatorName("+="), hasOperatorName("-=")),
-  hasType(pointerType()),
+  anyOf(hasType(pointerType()), 
hasType(autoType(hasDeducedType(pointerType(),
   unless(hasLHS(ignoringImpCasts(declRefExpr(to(isImplicit()))

This extends past the 80-col limit.



Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:12
 
+int* pointer() {
+  return nullptr;

Should be `int *` instead of `int*` (elsewhere as well).



Comment at: 
test/clang-tidy/cppcoreguidelines-pro-bounds-pointer-arithmetic.cpp:26
+  auto x = (int*) nullptr;
+  p = x + 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:29: warning: do not use pointer arithmetic

Can you remove the whitespace alignment from the new tests?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43745



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

[PATCH] D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set

[Darby Payne via Phabricator via cfe-commits]

dpayne created this revision.
dpayne added a reviewer: djasper.
Herald added subscribers: cfe-commits, klimek.

When SpacesInParentheses is set to true clang-format does not add a space 
before a global namespace variable. For example this is the output of 
clang-format for a somewhat contrived exampled.

  #include 
  
  void print_val( std::ostream &s ) { s << "Hello world" << std::endl; }
  
  int main( void ) {
print_val(::std::cout );
return 0;
  }

The .clang-format looked like

  Language:Cpp
  SpacesInParentheses: true


Repository:
  rC Clang

https://reviews.llvm.org/D43957

Files:
  lib/Format/TokenAnnotator.cpp


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2631,7 +2631,8 @@
 Style.Standard == FormatStyle::LS_Cpp03) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
   tok::kw___super, TT_TemplateCloser,
-  TT_TemplateOpener));
+  TT_TemplateOpener)) ||
+   (Left.is(tok ::l_paren) && Style.SpacesInParentheses);
   if ((Left.is(TT_TemplateOpener)) != (Right.is(TT_TemplateCloser)))
 return Style.SpacesInAngles;
   // Space before TT_StructuredBindingLSquare.


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -2631,7 +2631,8 @@
 Style.Standard == FormatStyle::LS_Cpp03) ||
!(Left.isOneOf(tok::l_paren, tok::r_paren, tok::l_square,
   tok::kw___super, TT_TemplateCloser,
-  TT_TemplateOpener));
+  TT_TemplateOpener)) ||
+   (Left.is(tok ::l_paren) && Style.SpacesInParentheses);
   if ((Left.is(TT_TemplateOpener)) != (Right.is(TT_TemplateCloser)))
 return Style.SpacesInAngles;
   // Space before TT_StructuredBindingLSquare.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43625: [OpenMP] Remove implicit data sharing code gen that aims to use device shared memory

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

gtbercea updated this revision to Diff 136570.
gtbercea added a comment.

Add Source location.


Repository:
  rC Clang

https://reviews.llvm.org/D43625

Files:
  lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
  lib/CodeGen/CGOpenMPRuntimeNVPTX.h
  test/OpenMP/nvptx_data_sharing.cpp
  test/OpenMP/nvptx_parallel_codegen.cpp
  test/OpenMP/nvptx_target_teams_codegen.cpp

Index: test/OpenMP/nvptx_target_teams_codegen.cpp
===
--- test/OpenMP/nvptx_target_teams_codegen.cpp
+++ test/OpenMP/nvptx_target_teams_codegen.cpp
@@ -60,7 +60,7 @@
   //
   // CHECK: [[AWAIT_WORK]]
   // CHECK: call void @llvm.nvvm.barrier0()
-  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i8*** %shared_args, i16 1)
+  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i16 1)
   // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
   // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
   // CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
@@ -146,7 +146,7 @@
   //
   // CHECK: [[AWAIT_WORK]]
   // CHECK: call void @llvm.nvvm.barrier0()
-  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i8*** %shared_args, i16 1)
+  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]], i16 1)
   // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
   // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
   // CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
Index: test/OpenMP/nvptx_parallel_codegen.cpp
===
--- test/OpenMP/nvptx_parallel_codegen.cpp
+++ test/OpenMP/nvptx_parallel_codegen.cpp
@@ -78,7 +78,7 @@
   //
   // CHECK: [[AWAIT_WORK]]
   // CHECK: call void @llvm.nvvm.barrier0()
-  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]],
+  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]]
   // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
   // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
   // CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
@@ -92,20 +92,20 @@
   //
   // CHECK: [[EXEC_PARALLEL]]
   // CHECK: [[WF1:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
-  // CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1:@.+]]_wrapper to i8*)
+  // CHECK: [[WM1:%.+]] = icmp eq i8* [[WF1]], bitcast (void (i32*, i32*)* [[PARALLEL_FN1:@.+]] to i8*)
   // CHECK: br i1 [[WM1]], label {{%?}}[[EXEC_PFN1:.+]], label {{%?}}[[CHECK_NEXT1:.+]]
   //
   // CHECK: [[EXEC_PFN1]]
-  // CHECK: call void [[PARALLEL_FN1]]_wrapper(
+  // CHECK: call void [[PARALLEL_FN1]](
   // CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
   //
   // CHECK: [[CHECK_NEXT1]]
   // CHECK: [[WF2:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
-  // CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2:@.+]]_wrapper to i8*)
+  // CHECK: [[WM2:%.+]] = icmp eq i8* [[WF2]], bitcast (void (i32*, i32*)* [[PARALLEL_FN2:@.+]] to i8*)
   // CHECK: br i1 [[WM2]], label {{%?}}[[EXEC_PFN2:.+]], label {{%?}}[[CHECK_NEXT2:.+]]
   //
   // CHECK: [[EXEC_PFN2]]
-  // CHECK: call void [[PARALLEL_FN2]]_wrapper(
+  // CHECK: call void [[PARALLEL_FN2]](
   // CHECK: br label {{%?}}[[TERM_PARALLEL:.+]]
   //
   // CHECK: [[CHECK_NEXT2]]
@@ -152,13 +152,13 @@
   // CHECK-DAG: [[MWS:%.+]] = call i32 @llvm.nvvm.read.ptx.sreg.warpsize()
   // CHECK: [[MTMP1:%.+]] = sub i32 [[MNTH]], [[MWS]]
   // CHECK: call void @__kmpc_kernel_init(i32 [[MTMP1]]
-  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN1]]_wrapper to i8*),
+  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i32*, i32*)* [[PARALLEL_FN1]] to i8*),
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @__kmpc_serialized_parallel(
   // CHECK: {{call|invoke}} void [[PARALLEL_FN3:@.+]](
   // CHECK: call void @__kmpc_end_serialized_parallel(
-  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i16, i32, i8**)* [[PARALLEL_FN2]]_wrapper to i8*),
+  // CHECK: call void @__kmpc_kernel_prepare_parallel(i8* bitcast (void (i32*, i32*)* [[PARALLEL_FN2]] to i8*),
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK: call void @llvm.nvvm.barrier0()
   // CHECK-64-DAG: load i32, i32* [[REF_A]]
@@ -203,7 +203,7 @@
   //
   // CHECK: [[AWAIT_WORK]]
   // CHECK: call void @llvm.nvvm.barrier0()
-  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]],
+  // CHECK: [[KPR:%.+]] = call i1 @__kmpc_kernel_parallel(i8** [[OMP_WORK_FN]]
   // CHECK: [[KPRB:%.+]] = zext i1 [[KPR]] to i8
   // store i8 [[KPRB]], i8* [[OMP_EXEC_STATUS]], align 1
   // CHECK: [[WORK:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
@@ -217,11 +217,11 @@
   //
   // CHECK: [[EXEC_PARALLEL]]
   // CHECK: [[WF:%.+]] = load i8*, i8** [[OMP_WORK_FN]],
-  // CHECK: [[WM:%.+]] = icmp eq i8* [[WF]],

[PATCH] D43625: [OpenMP] Remove implicit data sharing code gen that aims to use device shared memory

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rC Clang

https://reviews.llvm.org/D43625



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

[PATCH] D43162: [Parser] (C++) Make -Wextra-semi slightly more useful

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added a comment.

It's been a long time since that commit, but I think the difference is that 
`-pedantic` / `Extension`-type diagnostics are magic and 
`-Wc++98-compat-pedantic` is not. I like Richard's way better and if it could 
be applied to -Wnewline-eof later as well then that sounds great.


Repository:
  rC Clang

https://reviews.llvm.org/D43162



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

[clang-tools-extra] r326458 - [clangd] Make symbol name a required parameter for CanonicalIncludes::mapHeader

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Mar  1 10:30:48 2018
New Revision: 326458

URL: http://llvm.org/viewvc/llvm-project?rev=326458&view=rev
Log:
[clangd] Make symbol name a required parameter for CanonicalIncludes::mapHeader

Modified:
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp?rev=326458&r1=326457&r2=326458&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.cpp Thu Mar  1 
10:30:48 2018
@@ -35,11 +35,9 @@ void CanonicalIncludes::addSymbolMapping
 llvm::StringRef
 CanonicalIncludes::mapHeader(llvm::StringRef Header,
  llvm::StringRef QualifiedName) const {
-  if (!QualifiedName.empty()) {
-auto SE = SymbolMapping.find(QualifiedName);
-if (SE != SymbolMapping.end())
-  return SE->second;
-  }
+  auto SE = SymbolMapping.find(QualifiedName);
+  if (SE != SymbolMapping.end())
+return SE->second;
   std::lock_guard Lock(RegexMutex);
   for (auto &Entry : RegexHeaderMappingTable) {
 #ifndef NDEBUG

Modified: clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h?rev=326458&r1=326457&r2=326458&view=diff
==
--- clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h (original)
+++ clang-tools-extra/trunk/clangd/index/CanonicalIncludes.h Thu Mar  1 
10:30:48 2018
@@ -48,12 +48,10 @@ public:
   void addSymbolMapping(llvm::StringRef QualifiedName,
 llvm::StringRef CanonicalPath);
 
-  /// \return \p Header itself if there is no mapping for it; otherwise, return
-  /// a canonical header name.
-  /// \p QualifiedName of a symbol declared in \p Header can be provided to
-  /// check against the symbol mapping.
+  /// Returns the canonical include for symbol with \p QualifiedName, which is
+  /// declared in \p Header
   llvm::StringRef mapHeader(llvm::StringRef Header,
-llvm::StringRef QualifiedName = "") const;
+llvm::StringRef QualifiedName) const;
 
 private:
   // A map from header patterns to header names. This needs to be mutable so


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

[PATCH] D40737: [clang-tidy] Resubmit hicpp-multiway-paths-covered without breaking test

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

After long inactivity (sorry!) i had a chance to look at it again:

  switch(i) {
  case 0:;
  case 1:;
  case 2:;
  ...
  }

does *NOT* lead to the stack overflow. This is most likely an issue in the AST:
https://godbolt.org/g/vZw2BD

Empty case labels do nest, an empty statement prevents this. The nesting leads 
most likely to the deep recursion. I will file a bug for it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40737



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

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Frederic Tingaud via Phabricator via cfe-commits]

ftingaud updated this revision to Diff 136575.
ftingaud added a comment.

Remove customizable c++ version that added no real value.


https://reviews.llvm.org/D43766

Files:
  clang-tidy/modernize/MakeSmartPtrCheck.cpp
  clang-tidy/modernize/MakeSmartPtrCheck.h
  clang-tidy/modernize/MakeUniqueCheck.cpp
  clang-tidy/modernize/MakeUniqueCheck.h
  test/clang-tidy/modernize-make-unique-cxx11.cpp
  test/clang-tidy/modernize-make-unique-cxx14.cpp
  test/clang-tidy/modernize-make-unique-macros.cpp
  test/clang-tidy/modernize-make-unique.cpp

Index: test/clang-tidy/modernize-make-unique.cpp
===
--- test/clang-tidy/modernize-make-unique.cpp
+++ test/clang-tidy/modernize-make-unique.cpp
@@ -1,4 +1,4 @@
-// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++11 \
+// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++14 \
 // RUN:   -I%S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
Index: test/clang-tidy/modernize-make-unique-macros.cpp
===
--- test/clang-tidy/modernize-make-unique-macros.cpp
+++ test/clang-tidy/modernize-make-unique-macros.cpp
@@ -1,6 +1,6 @@
 // RUN: %check_clang_tidy %s modernize-make-unique %t -- \
 // RUN:   -config="{CheckOptions: [{key: modernize-make-unique.IgnoreMacros, value: 0}]}" \
-// RUN:   -- -std=c++11  -I%S/Inputs/modernize-smart-ptr
+// RUN:   -- -std=c++14  -I%S/Inputs/modernize-smart-ptr
 
 #include "unique_ptr.h"
 
Index: test/clang-tidy/modernize-make-unique-cxx14.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-make-unique-cxx14.cpp
@@ -0,0 +1,12 @@
+// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++14 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
+
+#include "unique_ptr.h"
+// CHECK-FIXES: #include 
+
+int main() {
+  auto my_ptr = std::unique_ptr(new int(1));
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead
+  // CHECK-FIXES: auto my_ptr = std::make_unique(1);
+  return 0;
+}
Index: test/clang-tidy/modernize-make-unique-cxx11.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-make-unique-cxx11.cpp
@@ -0,0 +1,11 @@
+// RUN: %check_clang_tidy %s modernize-make-unique %t -- -- -std=c++11 \
+// RUN:   -I%S/Inputs/modernize-smart-ptr
+
+#include "unique_ptr.h"
+// CHECK-FIXES: #include "unique_ptr.h"
+
+int main() {
+  auto my_ptr = std::unique_ptr(new int(1));
+  // CHECK-FIXES: auto my_ptr = std::unique_ptr(new int(1));
+  return 0;
+}
Index: clang-tidy/modernize/MakeUniqueCheck.h
===
--- clang-tidy/modernize/MakeUniqueCheck.h
+++ clang-tidy/modernize/MakeUniqueCheck.h
@@ -31,6 +31,11 @@
 
 protected:
   SmartPtrTypeMatcher getSmartPointerTypeMatcher() const override;
+
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+
+private:
+  const bool RequireCPlusPlus14;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/MakeUniqueCheck.cpp
===
--- clang-tidy/modernize/MakeUniqueCheck.cpp
+++ clang-tidy/modernize/MakeUniqueCheck.cpp
@@ -17,7 +17,8 @@
 
 MakeUniqueCheck::MakeUniqueCheck(StringRef Name,
  clang::tidy::ClangTidyContext *Context)
-: MakeSmartPtrCheck(Name, Context, "std::make_unique") {}
+: MakeSmartPtrCheck(Name, Context, "std::make_unique"),
+  RequireCPlusPlus14(Options.get("MakeSmartPtrFunction", "").empty()) {}
 
 MakeUniqueCheck::SmartPtrTypeMatcher
 MakeUniqueCheck::getSmartPointerTypeMatcher() const {
@@ -36,6 +37,13 @@
 equalsBoundNode(PointerType;
 }
 
+bool MakeUniqueCheck::isLanguageVersionSupported(
+const LangOptions &LangOpts) const {
+  if (RequireCPlusPlus14)
+return LangOpts.CPlusPlus14;
+  return LangOpts.CPlusPlus11;
+}
+
 } // namespace modernize
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/modernize/MakeSmartPtrCheck.h
===
--- clang-tidy/modernize/MakeSmartPtrCheck.h
+++ clang-tidy/modernize/MakeSmartPtrCheck.h
@@ -40,6 +40,9 @@
   /// in this class.
   virtual SmartPtrTypeMatcher getSmartPointerTypeMatcher() const = 0;
 
+  /// Returns whether the C++ version is compatible with current check.
+  virtual bool isLanguageVersionSupported(const LangOptions &LangOpts) const;
+
   static const char PointerType[];
   static const char ConstructorCall[];
   static const char ResetCall[];
Index: clang-tidy/modernize/MakeSmartPtrCheck.cpp
===
--- clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -61,16 +61,21 @@
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);
 }
 
+bool

[PATCH] D43766: [clang-tidy][modernize-make-unique] Checks c++14 flag before using std::make_unique

[Frederic Tingaud via Phabricator via cfe-commits]

ftingaud added inline comments.



Comment at: test/clang-tidy/modernize-make-unique-cxx14.cpp:10
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: use std::make_unique instead
+  // CHECK-FIXES: auto my_ptr = std::make_unique(1);
+  return 0;

Quuxplusone wrote:
> IIUC above, you allow the user to pass in the name of a custom 
> `my::make_unique` function. Could you add a test case for that?
It is tested by modernize-make-unique-header.cpp and the patch doesn't change 
the feature.


https://reviews.llvm.org/D43766



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

r326461 - [analyzer] Enable cfg-temporary-dtors by default.

[Artem Dergachev via cfe-commits]

Author: dergachev
Date: Thu Mar  1 10:53:13 2018
New Revision: 326461

URL: http://llvm.org/viewvc/llvm-project?rev=326461&view=rev
Log:
[analyzer] Enable cfg-temporary-dtors by default.

Don't enable c++-temp-dtor-inlining by default yet, due to this reference
counting pointe problem.

Otherwise the new mode seems stable and allows us to incrementally fix C++
problems in much less hacky ways.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
cfe/trunk/test/Analysis/analyzer-config.c
cfe/trunk/test/Analysis/analyzer-config.cpp
cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
cfe/trunk/test/Analysis/lifetime-extension.cpp
cfe/trunk/test/Analysis/temporaries.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp?rev=326461&r1=326460&r2=326461&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp Thu Mar  1 10:53:13 
2018
@@ -193,7 +193,7 @@ bool AnalyzerOptions::getBooleanOption(O
 bool AnalyzerOptions::includeTemporaryDtorsInCFG() {
   return getBooleanOption(IncludeTemporaryDtorsInCFG,
   "cfg-temporary-dtors",
-  /* Default = */ false);
+  /* Default = */ true);
 }
 
 bool AnalyzerOptions::includeImplicitDtorsInCFG() {
@@ -251,7 +251,7 @@ bool AnalyzerOptions::mayInlineCXXShared
 bool AnalyzerOptions::mayInlineCXXTemporaryDtors() {
   return getBooleanOption(InlineCXXTemporaryDtors,
   "c++-temp-dtor-inlining",
-  /*Default=*/true);
+  /*Default=*/false);
 }
 
 bool AnalyzerOptions::mayInlineObjCMethod() {

Modified: cfe/trunk/test/Analysis/analyzer-config.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.c?rev=326461&r1=326460&r2=326461&view=diff
==
--- cfe/trunk/test/Analysis/analyzer-config.c (original)
+++ cfe/trunk/test/Analysis/analyzer-config.c Thu Mar  1 10:53:13 2018
@@ -16,7 +16,7 @@ void foo() {
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
 // CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000

Modified: cfe/trunk/test/Analysis/analyzer-config.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/analyzer-config.cpp?rev=326461&r1=326460&r2=326461&view=diff
==
--- cfe/trunk/test/Analysis/analyzer-config.cpp (original)
+++ cfe/trunk/test/Analysis/analyzer-config.cpp Thu Mar  1 10:53:13 2018
@@ -13,7 +13,7 @@ void foo() {
 class Foo {
 public:
   ~Foo() {}
-  void baz();
+  void baz() { Foo(); }
void bar() { const Foo &f = Foo(); }
void foo() { bar(); }
 };
@@ -23,13 +23,14 @@ public:
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
+// CHECK-NEXT: c++-temp-dtor-inlining = false
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
 // CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
@@ -48,4 +49,4 @@ public:
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 28
+// CHECK-NEXT: num-entries = 29

Modified: cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp?rev=326461&r1=326460&r2=326461&view=diff
==
--- cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp (original)
+++ cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp Thu Mar  1 
10:53:13 2018
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config 
cfg-temporary-dtors=true -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config 
cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyze

[PATCH] D43804: [analyzer] Enable cfg-temporary-dtors by default?

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326461: [analyzer] Enable cfg-temporary-dtors by default. 
(authored by dergachev, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43804?vs=136428&id=136578#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43804

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
  cfe/trunk/test/Analysis/analyzer-config.c
  cfe/trunk/test/Analysis/analyzer-config.cpp
  cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
  cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
  cfe/trunk/test/Analysis/lifetime-extension.cpp
  cfe/trunk/test/Analysis/temporaries.cpp

Index: cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
===
--- cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
+++ cfe/trunk/test/Analysis/lifetime-cfg-output.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-rich-constructors=false -analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
+// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -analyze -analyzer-checker=debug.DumpCFG -analyzer-config cfg-lifetime=true,cfg-temporary-dtors=false,cfg-rich-constructors=false -analyzer-config cfg-implicit-dtors=false %s > %t 2>&1
 // RUN: FileCheck --input-file=%t %s
 
 extern bool UV;
Index: cfe/trunk/test/Analysis/analyzer-config.cpp
===
--- cfe/trunk/test/Analysis/analyzer-config.cpp
+++ cfe/trunk/test/Analysis/analyzer-config.cpp
@@ -13,7 +13,7 @@
 class Foo {
 public:
   ~Foo() {}
-  void baz();
+  void baz() { Foo(); }
 	void bar() { const Foo &f = Foo(); }
 	void foo() { bar(); }
 };
@@ -23,13 +23,14 @@
 // CHECK-NEXT: c++-inlining = destructors
 // CHECK-NEXT: c++-shared_ptr-inlining = false
 // CHECK-NEXT: c++-stdlib-inlining = true
+// CHECK-NEXT: c++-temp-dtor-inlining = false
 // CHECK-NEXT: c++-template-inlining = true
 // CHECK-NEXT: cfg-conditional-static-initializers = true
 // CHECK-NEXT: cfg-implicit-dtors = true
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
 // CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
@@ -48,4 +49,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 28
+// CHECK-NEXT: num-entries = 29
Index: cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
===
--- cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
+++ cfe/trunk/test/Analysis/inlining/temp-dtors-path-notes.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -analyzer-output=text -verify %s
 
 namespace test_simple_temporary {
 class C {
Index: cfe/trunk/test/Analysis/lifetime-extension.cpp
===
--- cfe/trunk/test/Analysis/lifetime-extension.cpp
+++ cfe/trunk/test/Analysis/lifetime-extension.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -verify %s
-// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true -DTEMPORARIES -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=false -verify %s
+// RUN: %clang_analyze_cc1 -Wno-unused -std=c++11 -analyzer-checker=core,debug.ExprInspection -analyzer-config cfg-temporary-dtors=true,c++-temp-dtor-inlining=true -DTEMPORARIES -verify %s
 
 void clang_analyzer_eval(bool);
 
Index: cfe/trunk/test/Analysis/analyzer-config.c
===
--- cfe/trunk/test/Analysis/analyzer-config.c
+++ cfe/trunk/test/Analysis/analyzer-config.c
@@ -16,7 +16,7 @@
 // CHECK-NEXT: cfg-lifetime = false
 // CHECK-NEXT: cfg-loopexit = false
 // CHECK-NEXT: cfg-rich-constructors = true
-// CHECK-NEXT: cfg-temporary-dtors = false
+// CHECK-NEXT: cfg-temporary-dtors = true
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
Index: cfe/trunk/test/Analysis/temporaries.cpp
===
--- cfe/trunk/test/Analysis/temporaries.cpp
+++ cfe/trunk/test

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/Attr.h:206
+
+  void cmpable(const ParamIdx &I) const {
+assert(isValid() && I.isValid() &&

The name here can be improved. How about `checkInvariants()`? Might as well 
make this inline while you're at it.



Comment at: include/clang/AST/Attr.h:236
+  /// parameter.
+  ParamIdx(unsigned Idx, bool HasThis)
+  : Idx(Idx), HasThis(HasThis), IsValid(true) {}

Is this constructor used anywhere? I didn't see it being used, but I could have 
missed something. If it's not used, go ahead and remove it.



Comment at: include/clang/AST/Attr.h:267
+assert(isValid() && "ParamIdx must be valid");
+return Idx - 1 - HasThis;
+  }

Please assert that `Idx` won't wrap before doing the return.



Comment at: include/clang/AST/Attr.h:276
+assert(isValid() && "ParamIdx must be valid");
+return Idx - 1;
+  }

Likewise here.



Comment at: include/clang/AST/Attr.h:281-284
+  bool operator<(const ParamIdx &I) const { cmpable(I); return Idx < I.Idx; }
+  bool operator>(const ParamIdx &I) const { cmpable(I); return Idx > I.Idx; }
+  bool operator<=(const ParamIdx &I) const { cmpable(I); return Idx <= I.Idx; }
+  bool operator>=(const ParamIdx &I) const { cmpable(I); return Idx >= I.Idx; }

Are all of these operations required for the class?



Comment at: include/clang/Basic/Attr.td:172-174
+  // Whether the C++ implicit this parameter is allowed.  Users that construct
+  // attributes from the source code use this information when validating
+  // parameter indices.

I still find the `AllowThis` parts to be hard to follow, so I want to be sure I 
understand your design properly. Everything that uses this new argument type 
sets `AllowsThis` to 0. As written, it sounds like setting that to 0 means that 
the parameter index cannot be used on a C++ instance method, and I know that's 
not the correct interpretation. Under what circumstances would this be set to 1 
instead?

Looking at the existing code, the only circumstances under which 
`checkFunctionOrMethodParameterIndex()` was being passed `true` for "allow 
this" was for XRayLogArgs, which doesn't even use `ParamIdx`. Perhaps this 
member isn't necessary any longer?



Comment at: lib/Sema/SemaChecking.cpp:2622
 
-  for (unsigned Val : NonNull->args()) {
-if (Val >= Args.size())
+  for (ParamIdx Idx : NonNull->args()) {
+if (Idx.getASTIndex() >= Args.size())

`const ParamIdx &`



Comment at: lib/Sema/SemaChecking.cpp:10083
 
-  for (unsigned ArgNo : NonNull->args()) {
-if (ArgNo == ParamNo) {
+  for (ParamIdx ArgNo : NonNull->args()) {
+if (ArgNo.getASTIndex() == ParamNo) {

`const ParamIdx &`



Comment at: lib/Sema/SemaChecking.cpp:12244
   }
-  const Expr *TypeTagExpr = ExprArgs[Attr->getTypeTagIdx()];
+  const Expr *TypeTagExpr = ExprArgs[Attr->typeTagIdx().getASTIndex()];
   bool FoundWrongKind;

Hoist the AST index so you don't have to call for it twice. (Same applies 
elsewhere.)



Comment at: lib/Sema/SemaDeclAttr.cpp:785-786
-  const ParmVarDecl *Param = FD->getParamDecl(Idx);
-  if (AllowDependentType && Param->getType()->isDependentType())
-return true;
   if (!Param->getType()->isIntegerType() && !Param->getType()->isCharType()) {

Good catch about this not being needed any longer!



Comment at: lib/Sema/SemaDeclAttr.cpp:1650-1651
   OwnershipAttr(AL.getLoc(), S.Context, nullptr, nullptr, 0,
-AL.getAttributeSpellingListIndex()).getOwnKind();
+AL.getAttributeSpellingListIndex())
+  .getOwnKind();
 

This change looks to be unrelated to the patch?



Comment at: lib/Sema/SemaDeclAttr.cpp:4549-4551
+if (ArgumentIdx.getASTIndex() >= getFunctionOrMethodNumParams(D) ||
+!getFunctionOrMethodParamType(D, ArgumentIdx.getASTIndex())
+ ->isPointerType())

Is this formatting produced by clang-format?



Comment at: lib/StaticAnalyzer/Checkers/NonNullParamChecker.cpp:61
 }
-for (unsigned Val : NonNull->args()) {
-  if (Val >= NumArgs)
+for (ParamIdx Idx : NonNull->args()) {
+  if (Idx.getASTIndex() >= NumArgs)

`const ParamIdx &`


https://reviews.llvm.org/D43248



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

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D43248#1023720, @jdenny wrote:

> Hi Aaron.  It occurs to me now that this patch has grown rather large and, in 
> some places, a little subtle.  Would it help the review if I were to break it 
> up into a patch series that introduces ParamIdx to each attribute, one at a 
> time?  I'm not trying to rush you, but I hate for the review to be painful 
> for you if it doesn't have to be.


No need to do that -- this review just takes a bit more time for me to 
complete, but it's reasonably well-factored. Thank you, though!


https://reviews.llvm.org/D43248



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

[PATCH] D43681: [WebAssembly] Add exception handling option

[Derek Schuff via Phabricator via cfe-commits]

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

This flag turns on CPU features (i.e. there's one for each new wasm feature 
proposal that has to be feature-detected), so I think this makes sense to be 
consistent with all the others. I could imagine enabling exceptions in the 
frontend but having some kind of emulation in the backend (like we do today for 
emscripten). More generally the semantics `-fno-exceptions` unfortunately 
doesn't exactly match the kind of behavior people typically want because it 
doesn't allow you to compile code that has try/catch/throw at all. In PNaCl and 
emscripten the default behavior is instead to compile that code but to lower it 
away and turn throw into abort. Also IIRC even with `-fno-exceptions` the code 
still has to allow exceptions to propagate which means you end up with invokes 
everywhere instead of calls, so you are still paying most of the costs of 
enabling EH. So that bit is a little bit complex and we'll probably want to 
think a bit more carefully about what options we want to have. But a machine 
flag for enabling the CPU feature makes sense to start.


Repository:
  rC Clang

https://reviews.llvm.org/D43681



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

r326469 - Driver: hoist `-fno-rtti-data` to a driver flag

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Thu Mar  1 11:13:43 2018
New Revision: 326469

URL: http://llvm.org/viewvc/llvm-project?rev=326469&view=rev
Log:
Driver: hoist `-fno-rtti-data` to a driver flag

This is needed for building with the GNU driver (`clang++`) when
targeting Windows and using msvcprt.  This flag is the equivalent of
`/GR-`.

Added:
cfe/trunk/test/Driver/fno-rtti-data.cpp
Modified:
cfe/trunk/include/clang/Driver/CC1Options.td
cfe/trunk/include/clang/Driver/Options.td

Modified: cfe/trunk/include/clang/Driver/CC1Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CC1Options.td?rev=326469&r1=326468&r2=326469&view=diff
==
--- cfe/trunk/include/clang/Driver/CC1Options.td (original)
+++ cfe/trunk/include/clang/Driver/CC1Options.td Thu Mar  1 11:13:43 2018
@@ -707,8 +707,6 @@ def fobjc_subscripting_legacy_runtime :
   HelpText<"Allow Objective-C array and dictionary subscripting in legacy 
runtime">;
 def vtordisp_mode_EQ : Joined<["-"], "vtordisp-mode=">,
   HelpText<"Control vtordisp placement on win32 targets">;
-def fno_rtti_data : Flag<["-"], "fno-rtti-data">,
-  HelpText<"Control emission of RTTI data">;
 def fnative_half_type: Flag<["-"], "fnative-half-type">,
   HelpText<"Use the native half type for __fp16 instead of promoting to 
float">;
 def fnative_half_arguments_and_returns : Flag<["-"], 
"fnative-half-arguments-and-returns">,

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=326469&r1=326468&r2=326469&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Mar  1 11:13:43 2018
@@ -1315,6 +1315,8 @@ def fno_operator_names : Flag<["-"], "fn
 def fno_pascal_strings : Flag<["-"], "fno-pascal-strings">, Group;
 def fno_rtti : Flag<["-"], "fno-rtti">, Group, Flags<[CC1Option]>,
   HelpText<"Disable generation of rtti information">;
+def fno_rtti_data : Flag<["-"], "fno-rtti-data">, Group, 
Flags<[CC1Option]>,
+  HelpText<"Control emission of RTTI data">;
 def fno_short_enums : Flag<["-"], "fno-short-enums">, Group;
 def fno_show_column : Flag<["-"], "fno-show-column">, Group, 
Flags<[CC1Option]>,
   HelpText<"Do not include column number on diagnostics">;

Added: cfe/trunk/test/Driver/fno-rtti-data.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fno-rtti-data.cpp?rev=326469&view=auto
==
--- cfe/trunk/test/Driver/fno-rtti-data.cpp (added)
+++ cfe/trunk/test/Driver/fno-rtti-data.cpp Thu Mar  1 11:13:43 2018
@@ -0,0 +1,2 @@
+// RUN: %clang -### -fno-rtti-data %s 2>&1 | FileCheck %s
+// CHECK: -fno-rtti-data


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

[PATCH] D43908: [RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd accepted this revision.
compnerd added a comment.

Awesome, thanks, this makes me feel much more comfortable.


https://reviews.llvm.org/D43908



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

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

Ping. It would be helpful to know how reasonable is the idea to handle more 
expression types in `Sema::CheckForIntOverflow`.


https://reviews.llvm.org/D42938



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

[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 136580.
ahatanak marked 3 inline comments as done.
ahatanak added a comment.

Address review comments.


https://reviews.llvm.org/D43841

Files:
  include/clang/AST/Decl.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/disable-tail-call-escaping-block.m
  test/Driver/fno-escaping-block-tail-calls.c

Index: test/Driver/fno-escaping-block-tail-calls.c
===
--- /dev/null
+++ test/Driver/fno-escaping-block-tail-calls.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s -fescaping-block-tail-calls -fno-escaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-DISABLE < %t %s
+// CHECK-DISABLE: "-fno-escaping-block-tail-calls"
+
+// RUN: %clang -### %s -fno-escaping-block-tail-calls -fescaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO-DISABLE < %t %s
+// CHECK-NO-DISABLE-NOT: "-fno-escaping-block-tail-calls"
Index: test/CodeGenObjC/disable-tail-call-escaping-block.m
===
--- /dev/null
+++ test/CodeGenObjC/disable-tail-call-escaping-block.m
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fblocks -fno-escaping-block-tail-calls -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @test(
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE0:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE1:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE2:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE3:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE4:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE5:.*]] to i8*)
+
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE0]]({{.*}}) #[[DISABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE1]]({{.*}}) #[[ENABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE2]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE3]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE4]]({{.*}}) #[[ENABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE5]]({{.*}}) #[[DISABLEATTR]] {
+
+// CHECK: attributes #[[ENABLEATTR]] = {{{.*}}"disable-tail-calls"="false"{{.*}}}
+// CHECK: attributes #[[DISABLEATTR]] = {{{.*}}"disable-tail-calls"="true"{{.*}}}
+
+typedef void (^BlockTy)(void);
+
+void callee0(__attribute__((noescape)) BlockTy);
+void callee1(BlockTy);
+
+__attribute__((objc_root_class))
+@interface C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p;
+-(void)m1:(BlockTy)p;
+@end
+
+@implementation C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p {}
+-(void)m1:(BlockTy)p {}
+@end
+
+void test(id a, C0 *c0) {
+  BlockTy b0 = ^{ (void)a; }; // disable tail-call optimization.
+  callee0(b0);
+  callee0(^{ (void)a; }); // enable tail-call optimization.
+  callee1(^{ (void)a; }); // disable tail-call optimization.
+
+  BlockTy b1 = ^{ (void)a; }; // disable tail-call optimization.
+  [c0 m0:b1];
+  [c0 m0:^{ (void)a; }]; // enable tail-call optimization.
+  [c0 m1:^{ (void)a; }]; // disable tail-call optimization.
+}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1613,6 +1613,11 @@
 ParmVarDecl *param = Method->parameters()[i];
 assert(argExpr && "CheckMessageArgumentTypes(): missing expression");
 
+if (param->hasAttr())
+  if (auto *BE = dyn_cast(
+  argExpr->IgnoreParenNoopCasts(Context)))
+BE->getBlockDecl()->setDoesNotEscape();
+
 // Strip the unbridged-cast placeholder expression off unless it's
 // a consumed argument.
 if (argExpr->hasPlaceholderType(BuiltinType::ARCUnbridgedCast) &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4841,6 +4841,10 @@
(!Param || !Param->hasAttr()))
 CFAudited = true;
 
+  if (Param && Param->hasAttr())
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+
   InitializedEntity Entity =
   Param ? InitializedEntity::InitializeParameter(Context, Param,
  ProtoArgType)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -640,6 +640,8 @@
   Opts.DisableFree = Args.hasArg(OPT_disable_free);
   Opts.DiscardValueNames = Args.hasArg(OPT_discard_value_name

[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: include/clang/Driver/Options.td:1419
+def fno_disable_tail_calls_escaping_blocks : Flag<["-"], 
"fno-disable-tail-calls-escaping-blocks">, Group, Flags<[CC1Option]>;
+def fdisable_tail_calls_escaping_blocks : Flag<["-"], 
"fdisable-tail-calls-escaping-blocks">, Group, Flags<[CC1Option]>;
 def force__cpusubtype__ALL : Flag<["-"], "force_cpusubtype_ALL">;

rjmccall wrote:
> rjmccall wrote:
> > These are pretty unidiomatic option names.  I would suggest one of these:
> >   - [fixed]-fescaping-block-tail-calls[/fixed] (the default) and 
> > [fixed]-fno-escaping-block-tail-calls[/fixed]
> >   - [fixed]-enable-escaping-block-tail-calls[/fixed] (the default) and 
> > [fixed]-disable-escaping-block-tail-calls[/fixed]
> Wow, this is not even close to Phabricator markup, I don't know what I was 
> thinking.
I chose the first option 
"-fno-escaping-block-tail-calls/-fescaping-block-tail-calls".


https://reviews.llvm.org/D43841



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

[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:4846
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+

Can this be based on the noescape parameter bit on the function type?


https://reviews.llvm.org/D43841



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

[PATCH] D43773: Implement the container bits of P0805R1

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists updated this revision to Diff 136582.
mclow.lists added a comment.

Add the `tuple` bits. Regularize the equality comparisons of the containers; 
i.e, don't try to be clever - let the compiler be clever.


https://reviews.llvm.org/D43773

Files:
  include/array
  include/deque
  include/forward_list
  include/list
  include/tuple
  include/vector
  test/std/containers/sequences/array/compare.fail.cpp
  test/std/containers/sequences/array/compare.pass.cpp
  test/std/containers/sequences/deque/compare.fail.cpp
  test/std/containers/sequences/deque/compare.pass.cpp
  test/std/containers/sequences/forwardlist/compare.fail.cpp
  test/std/containers/sequences/forwardlist/compare.pass.cpp
  test/std/containers/sequences/list/compare.fail.cpp
  test/std/containers/sequences/list/compare.pass.cpp
  test/std/containers/sequences/vector/compare.fail.cpp
  test/std/containers/sequences/vector/compare.pass.cpp
  test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
  test/std/utilities/tuple/tuple.tuple/tuple.rel/lt.pass.cpp

Index: test/std/utilities/tuple/tuple.tuple/tuple.rel/lt.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.rel/lt.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.rel/lt.pass.cpp
@@ -35,6 +35,16 @@
 
 #include "test_macros.h"
 
+
+template 
+void compare (const T1 &t1, const T2 &t2)
+{
+assert((t1  < t2) == ( isLess));
+assert((t1  > t2) == (!isLess && !isEqual));
+assert((t1 <= t2) == ( isLess ||  isEqual));
+assert((t1 >= t2) == (!isLess ||  isEqual));
+}
+
 int main()
 {
 {
@@ -208,5 +218,57 @@
 static_assert(!(t1 >  t2), "");
 static_assert(!(t1 >= t2), "");
 }
+{ // P0805
+typedef std::tuple<> T0;
+typedef std::tuple T1;
+typedef std::tuple T2;
+constexpr T0 t0;
+constexpr T1 t1(1, 2, 3);
+constexpr T2 t2(1, 2);
+constexpr T1 t3(0, 1, 2); // common tail
+
+// less, equal
+compare(t0, t0);
+static_assert(!(t0  < t0), "");
+static_assert(!(t0  > t0), "");
+static_assert( (t0 <= t0), "");
+static_assert( (t0 >= t0), "");
+
+compare(t0, t1);
+static_assert( (t0  < t1), "");
+static_assert(!(t0  > t1), "");
+static_assert( (t0 <= t1), "");
+static_assert(!(t0 >= t1), "");
+
+compare(t1, t0);
+static_assert(!(t1  < t0), "");
+static_assert( (t1  > t0), "");
+static_assert(!(t1 <= t0), "");
+static_assert( (t1 >= t0), "");
+
+compare(t1, t2);
+static_assert(!(t1  < t2), "");
+static_assert( (t1  > t2), "");
+static_assert(!(t1 <= t2), "");
+static_assert( (t1 >= t2), "");
+
+compare(t2, t1);
+static_assert( (t2  < t1), "");
+static_assert(!(t2  > t1), "");
+static_assert( (t2 <= t1), "");
+static_assert(!(t2 >= t1), "");
+
+compare(t2, t3);
+static_assert(!(t2  < t3), "");
+static_assert( (t2  > t3), "");
+static_assert(!(t2 <= t3), "");
+static_assert( (t2 >= t3), "");
+
+compare(t3, t2);
+static_assert( (t3  < t2), "");
+static_assert(!(t3  > t2), "");
+static_assert( (t3 <= t2), "");
+static_assert(!(t3 >= t2), "");
+}
 #endif
 }
Index: test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.rel/eq.pass.cpp
@@ -154,5 +154,20 @@
 static_assert(!(t1 == t2), "");
 static_assert(t1 != t2, "");
 }
+{ // P0805
+typedef std::tuple T1;
+typedef std::tuple T2;
+constexpr T1 t1(1, 2, 3);
+constexpr T2 t2(1.1, 3);
+constexpr T2 t3(1, 2);
+static_assert(!(t1 == t2), "");
+static_assert(!(t2 == t1), "");
+static_assert(!(t1 == t3), "");
+static_assert(!(t3 == t1), "");
+static_assert(  t1 != t2,  "");
+static_assert(  t2 != t1,  "");
+static_assert(  t1 != t3,  "");
+static_assert(  t3 != t1,  "");
+}
 #endif
 }
Index: test/std/containers/sequences/vector/compare.pass.cpp
===
--- test/std/containers/sequences/vector/compare.pass.cpp
+++ test/std/containers/sequences/vector/compare.pass.cpp
@@ -0,0 +1,90 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+
+// template  bool operator==(const vector& x, const vector& y);
+// template  bool 

[PATCH] D43961: OpenBSD Driver basic sanitiser support

[David CARLIER via Phabricator via cfe-commits]

devnexen created this revision.
devnexen added reviewers: krytarowski, vitalybuka, kettenis.
devnexen created this object with visibility "All Users".
Herald added a subscriber: cfe-commits.

Basic support of Sanitiser to follow-up ubsan support in compiler-rt.
Needs to use lld instead of base ld to be fully workable.


Repository:
  rC Clang

https://reviews.llvm.org/D43961

Files:
  lib/Driver/ToolChains/OpenBSD.cpp
  lib/Driver/ToolChains/OpenBSD.h


Index: lib/Driver/ToolChains/OpenBSD.h
===
--- lib/Driver/ToolChains/OpenBSD.h
+++ lib/Driver/ToolChains/OpenBSD.h
@@ -64,6 +64,8 @@
   }
   unsigned GetDefaultDwarfVersion() const override { return 2; }
 
+  SanitizerMask getSupportedSanitizers() const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -13,6 +13,7 @@
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
 
 using namespace clang::driver;
@@ -97,6 +98,8 @@
const InputInfoList &Inputs,
const ArgList &Args,
const char *LinkingOutput) const {
+  const toolchains::OpenBSD &ToolChain =
+  static_cast(getToolChain());
   const Driver &D = getToolChain().getDriver();
   ArgStringList CmdArgs;
 
@@ -170,11 +173,13 @@
 Triple.replace(0, 6, "amd64");
   CmdArgs.push_back(
   Args.MakeArgString("-L/usr/lib/gcc-lib/" + Triple + "/4.2.1"));
+  CmdArgs.push_back(Args.MakeArgString("-L/usr/lib"));
 
   Args.AddAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
 options::OPT_e, options::OPT_s, options::OPT_t,
 options::OPT_Z_Flag, options::OPT_r});
 
+  bool NeedsSanitizerDeps = addSanitizerRuntimes(ToolChain, Args, CmdArgs);
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs, JA);
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
@@ -186,7 +191,10 @@
   else
 CmdArgs.push_back("-lm");
 }
-
+if (NeedsSanitizerDeps) {
+  CmdArgs.push_back(ToolChain.getCompilerRTArgString(Args, "builtins", 
false));
+  linkSanitizerRuntimeDeps(ToolChain, CmdArgs);
+}
 // FIXME: For some reason GCC passes -lgcc before adding
 // the default system libraries. Just mimic this for now.
 CmdArgs.push_back("-lgcc");
@@ -221,6 +229,19 @@
   C.addCommand(llvm::make_unique(JA, *this, Exec, CmdArgs, Inputs));
 }
 
+SanitizerMask OpenBSD::getSupportedSanitizers() const {
+  const bool IsX86 = getTriple().getArch() == llvm::Triple::x86;
+  const bool IsX86_64 = getTriple().getArch() == llvm::Triple::x86_64;
+
+  // For future use, only UBsan at the moment
+  SanitizerMask Res = ToolChain::getSupportedSanitizers();
+
+  if (IsX86 || IsX86_64)
+Res |= SanitizerKind::Vptr;
+
+  return Res;
+}
+
 /// OpenBSD - OpenBSD tool chain which can call as(1) and ld(1) directly.
 
 OpenBSD::OpenBSD(const Driver &D, const llvm::Triple &Triple,


Index: lib/Driver/ToolChains/OpenBSD.h
===
--- lib/Driver/ToolChains/OpenBSD.h
+++ lib/Driver/ToolChains/OpenBSD.h
@@ -64,6 +64,8 @@
   }
   unsigned GetDefaultDwarfVersion() const override { return 2; }
 
+  SanitizerMask getSupportedSanitizers() const override;
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
Index: lib/Driver/ToolChains/OpenBSD.cpp
===
--- lib/Driver/ToolChains/OpenBSD.cpp
+++ lib/Driver/ToolChains/OpenBSD.cpp
@@ -13,6 +13,7 @@
 #include "CommonArgs.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
 
 using namespace clang::driver;
@@ -97,6 +98,8 @@
const InputInfoList &Inputs,
const ArgList &Args,
const char *LinkingOutput) const {
+  const toolchains::OpenBSD &ToolChain =
+  static_cast(getToolChain());
   const Driver &D = getToolChain().getDriver();
   ArgStringList CmdArgs;
 
@@ -170,11 +173,13 @@
 Triple.replace(0, 6, "amd64");
   CmdArgs.push_back(
   Args.MakeArgString("-L/usr/lib/gcc-lib/" + Triple + "/4.2.1"));
+  CmdArgs.push_back(Args.MakeArgString("-L/usr/lib"));
 
   Args.AddAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
 options::OPT_e, options::OPT_s, options::OPT_t,
 options::OPT_Z_Flag, options::OPT_r});
 
+  bool NeedsSanitizerDeps = addSanitizerR

[PATCH] D41102: Setup clang-doc frontend framework

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thank you for working on this!
Some more nitpicking.

//Please// consider adding even more tests (ideally, all this code should have 
100% test coverage)




Comment at: clang-doc/BitcodeWriter.cpp:139
+  {COMMENT_NAME, {"Name", &StringAbbrev}},
+  {COMMENT_POSITION, {"Position", &IntAbbrev}},
+  {COMMENT_DIRECTION, {"Direction", &StringAbbrev}},

This change is not covered by tests.
(I've actually found out that the hard way, by trying to find why it didn't 
trigger any asssertions, oh well)



Comment at: clang-doc/BitcodeWriter.cpp:325
+  emitHeader();
+  Stream.EnterBlockInfoBlock();
+

I think it would be cleaner to move it (at least the enterblock, it might make 
sense to leave the header at the very top) after the static variable



Comment at: clang-doc/BitcodeWriter.cpp:363
+
+  for (const auto &Block : TheBlocks) {
+assert(Block.second.size() < (1U << BitCodeConstants::SubblockIDSize));

I.e.
```
...
, FUNCTION_IS_METHOD}}};

  Stream.EnterBlockInfoBlock();
  for (const auto &Block : TheBlocks) {
assert(Block.second.size() < (1U << BitCodeConstants::SubblockIDSize));
emitBlockInfo(Block.first, Block.second);
  }
  Stream.ExitBlock();

  emitVersion();
}
```



Comment at: clang-doc/BitcodeWriter.h:19
+
+#include 
+#include 

Please sort includes, clang-tidy complains.



Comment at: clang-doc/BitcodeWriter.h:32
+// BitCodeConstants, though they can be added without breaking it.
+static const unsigned VERSION_NUMBER = 1;
+

```
/build/clang-tools-extra/clang-doc/BitcodeWriter.h:32:23: warning: invalid case 
style for variable 'VERSION_NUMBER' [readability-identifier-naming]
static const unsigned VERSION_NUMBER = 1;
  ^~
  VersionNumber

```



Comment at: clang-doc/BitcodeWriter.h:163
+ public:
+  ClangDocBitcodeWriter(llvm::BitstreamWriter &Stream,
+bool OmitFilenames = false)

The simplest solution would be
```
#ifndef NDEBUG // Don't want explicit dtor unless needed
~ClangDocBitcodeWriter() {
  // Check that the static size is large-enough.
  assert(Record.capacity() == BitCodeConstants::RecordSize);
}
#endif
```



Comment at: clang-doc/BitcodeWriter.h:228
+  // Static size is the maximum length of the block/record names we're pushing
+  // to this + 1. Longest is currently `MemberTypeBlock` at 15 chars.
+  SmallVector Record;

So you want to be really definitive with this. I wanted to avoid that, 
actually..
Then i'm afraid one more assert is needed, to make sure this is *actually* true.

I'm not seeing any way to make `SmallVector` completely static,
so you could either add one more wrapper around it (rather ugly),
or check the final size in the `ClangDocBitcodeWriter` destructor (will not 
pinpoint when the size has 'overflowed')



Comment at: clang-doc/BitcodeWriter.h:246
+void ClangDocBitcodeWriter::writeBitstream(const T &I, bool WriteBlockInfo) {
+  if (WriteBlockInfo) emitBlockInfoBlock();
+  StreamSubBlockGuard Block(Stream, MapFromInfoToBlockId::ID);

Does it *ever* make sense to output `BlockInfoBlock` anywhere else other than 
once at the very beginning?
I'd think you should drop the boolean param, and unconditinally call the 
`emitBlockInfoBlock();` from `ClangDocBitcodeWriter::ClangDocBitcodeWriter()` 
ctor.



Comment at: clang-doc/BitcodeWriter.h:248
+  StreamSubBlockGuard Block(Stream, MapFromInfoToBlockId::ID);
+  emitBlock(I);
+}

The naming choices confuse me.
There is `writeBitstream()` and `emitBlock()`, which is called from 
`writeBitstream()` to write the actual contents of the block.

Why one is `write` and another is `emit`?
To match the `BitstreamWriter` naming choices? (which uses `Emit` prefix)?
To avoid the confusion of which one outputs the actual content, and which one 
outputs the whole block?

I think it should be:
*
```
- void emitBlock(const NamespaceInfo &I);
+ void emitBlockContent(const NamespaceInfo &I);
```
*
```
- void ClangDocBitcodeWriter::writeBitstream(const T &I, bool WriteBlockInfo);
+ void ClangDocBitcodeWriter::emitBlock(const T &I, bool EmitBlockInfo);
```

This way, *i think* their names would clearner-er state what they do, and won't 
be weirdly different.
What do you think?



Comment at: clang-doc/Representation.h:18
+
+#include 
+#include "clang/AST/Type.h"

Please sort includes, clang-tidy complains.



Comment at: clang-doc/Serialize.cpp:88
+  CurrentCI.Name = getCommandName(C->getCommandID());
+  for (unsigned i = 0, e = C->getNumArgs(); i < e; ++i)
+CurrentCI.Args.push_back(C->getArgText(i));

```
/build/clang-tools-extra/clang-doc/Serialize.cpp:

r326476 - [RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Thu Mar  1 12:22:57 2018
New Revision: 326476

URL: http://llvm.org/viewvc/llvm-project?rev=326476&view=rev
Log:
[RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

Make types with sizes that aren't a power of two an error (that can
be disabled) in structs with ms_struct layout, except on mingw where
the situation is quite likely to occur and GCC handles it silently.

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

Added:
cfe/trunk/test/CodeGen/ms_struct-long-double.c
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/RecordLayoutBuilder.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=326476&r1=326475&r2=326476&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar  1 12:22:57 
2018
@@ -759,6 +759,10 @@ def warn_cxx_ms_struct :
   Warning<"ms_struct may not produce Microsoft-compatible layouts for classes "
   "with base classes or virtual functions">,
   DefaultError, InGroup;
+def warn_npot_ms_struct :
+  Warning<"ms_struct may not produce Microsoft-compatible layouts with 
fundamental "
+  "data types with sizes that aren't a power of two">,
+  DefaultError, InGroup;
 def err_section_conflict : Error<"%0 causes a section type conflict with %1">;
 def err_no_base_classes : Error<"invalid use of '__super', %0 has no base 
classes">;
 def err_invalid_super_scope : Error<"invalid use of '__super', "

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=326476&r1=326475&r2=326476&view=diff
==
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Thu Mar  1 12:22:57 2018
@@ -1752,10 +1752,32 @@ void ItaniumRecordLayoutBuilder::LayoutF
   QualType T = Context.getBaseElementType(D->getType());
   if (const BuiltinType *BTy = T->getAs()) {
 CharUnits TypeSize = Context.getTypeSizeInChars(BTy);
-assert(
-(llvm::isPowerOf2_64(TypeSize.getQuantity()) ||
- Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
-"Non PowerOf2 size outside of GNU mode");
+
+if (!llvm::isPowerOf2_64(TypeSize.getQuantity())) {
+  assert(
+  !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment() 
&&
+  "Non PowerOf2 size in MSVC mode");
+  // Base types with sizes that aren't a power of two don't work
+  // with the layout rules for MS structs. This isn't an issue in
+  // MSVC itself since there are no such base data types there.
+  // On e.g. x86_32 mingw and linux, long double is 12 bytes though.
+  // Any structs involving that data type obviously can't be ABI
+  // compatible with MSVC regardless of how it is laid out.
+
+  // Since ms_struct can be mass enabled (via a pragma or via the
+  // -mms-bitfields command line parameter), this can trigger for
+  // structs that don't actually need MSVC compatibility, so we
+  // need to be able to sidestep the ms_struct layout for these types.
+
+  // Since the combination of -mms-bitfields together with structs
+  // like max_align_t (which contains a long double) for mingw is
+  // quite comon (and GCC handles it silently), just handle it
+  // silently there. For other targets that have ms_struct enabled
+  // (most probably via a pragma or attribute), trigger a diagnostic
+  // that defaults to an error.
+  if (!Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+Diag(D->getLocation(), diag::warn_npot_ms_struct);
+}
 if (TypeSize > FieldAlign &&
 llvm::isPowerOf2_64(TypeSize.getQuantity()))
   FieldAlign = TypeSize;

Added: cfe/trunk/test/CodeGen/ms_struct-long-double.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms_struct-long-double.c?rev=326476&view=auto
==
--- cfe/trunk/test/CodeGen/ms_struct-long-double.c (added)
+++ cfe/trunk/test/CodeGen/ms_struct-long-double.c Thu Mar  1 12:22:57 2018
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-windows-gnu 
-fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-linux -fdump-record-layouts 
-Wno-incompatible-ms-struct %s | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm-only -triple i686-linux 
-fdump-record-layouts %s 2>&1 | FileCheck %s -check-prefix=ERROR
+
+struct ldb_struct {

[PATCH] D43908: [RecordLayout] Only assert that fundamental type sizes are power of two on MSVC

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL326476: [RecordLayout] Only assert that fundamental type 
sizes are power of two on MSVC (authored by mstorsjo, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D43908?vs=136471&id=136583#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D43908

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
  cfe/trunk/test/CodeGen/ms_struct-long-double.c


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -759,6 +759,10 @@
   Warning<"ms_struct may not produce Microsoft-compatible layouts for classes "
   "with base classes or virtual functions">,
   DefaultError, InGroup;
+def warn_npot_ms_struct :
+  Warning<"ms_struct may not produce Microsoft-compatible layouts with 
fundamental "
+  "data types with sizes that aren't a power of two">,
+  DefaultError, InGroup;
 def err_section_conflict : Error<"%0 causes a section type conflict with %1">;
 def err_no_base_classes : Error<"invalid use of '__super', %0 has no base 
classes">;
 def err_invalid_super_scope : Error<"invalid use of '__super', "
Index: cfe/trunk/test/CodeGen/ms_struct-long-double.c
===
--- cfe/trunk/test/CodeGen/ms_struct-long-double.c
+++ cfe/trunk/test/CodeGen/ms_struct-long-double.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-windows-gnu 
-fdump-record-layouts %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-linux -fdump-record-layouts 
-Wno-incompatible-ms-struct %s | FileCheck %s
+// RUN: not %clang_cc1 -emit-llvm-only -triple i686-linux 
-fdump-record-layouts %s 2>&1 | FileCheck %s -check-prefix=ERROR
+
+struct ldb_struct {
+  char c;
+  long double ldb;
+} __attribute__((__ms_struct__));
+
+struct ldb_struct a;
+
+// CHECK: 0 | struct ldb_struct
+// CHECK-NEXT:0 |   char c
+// CHECK-NEXT:4 |   long double ldb
+// CHECK-NEXT:  | [sizeof=16, align=4]
+
+// ERROR: error: ms_struct may not produce Microsoft-compatible layouts with 
fundamental data types with sizes that aren't a power of two
Index: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
===
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
@@ -1752,10 +1752,32 @@
   QualType T = Context.getBaseElementType(D->getType());
   if (const BuiltinType *BTy = T->getAs()) {
 CharUnits TypeSize = Context.getTypeSizeInChars(BTy);
-assert(
-(llvm::isPowerOf2_64(TypeSize.getQuantity()) ||
- Context.getTargetInfo().getTriple().isWindowsGNUEnvironment()) &&
-"Non PowerOf2 size outside of GNU mode");
+
+if (!llvm::isPowerOf2_64(TypeSize.getQuantity())) {
+  assert(
+  !Context.getTargetInfo().getTriple().isWindowsMSVCEnvironment() 
&&
+  "Non PowerOf2 size in MSVC mode");
+  // Base types with sizes that aren't a power of two don't work
+  // with the layout rules for MS structs. This isn't an issue in
+  // MSVC itself since there are no such base data types there.
+  // On e.g. x86_32 mingw and linux, long double is 12 bytes though.
+  // Any structs involving that data type obviously can't be ABI
+  // compatible with MSVC regardless of how it is laid out.
+
+  // Since ms_struct can be mass enabled (via a pragma or via the
+  // -mms-bitfields command line parameter), this can trigger for
+  // structs that don't actually need MSVC compatibility, so we
+  // need to be able to sidestep the ms_struct layout for these types.
+
+  // Since the combination of -mms-bitfields together with structs
+  // like max_align_t (which contains a long double) for mingw is
+  // quite comon (and GCC handles it silently), just handle it
+  // silently there. For other targets that have ms_struct enabled
+  // (most probably via a pragma or attribute), trigger a diagnostic
+  // that defaults to an error.
+  if (!Context.getTargetInfo().getTriple().isWindowsGNUEnvironment())
+Diag(D->getLocation(), diag::warn_npot_ms_struct);
+}
 if (TypeSize > FieldAlign &&
 llvm::isPowerOf2_64(TypeSize.getQuantity()))
   FieldAlign = TypeSize;


Index: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
@@ -759,6 +759,10 @@
  

[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaExpr.cpp:4846
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+

rjmccall wrote:
> Can this be based on the noescape parameter bit on the function type?
Oh that's right. It should be able to set the DoesNotEscape bit of a block when 
it's passed to an indirect function call. I added an IRGen test case that 
checks tail-call is enabled in such cases.



https://reviews.llvm.org/D43841



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

[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 136584.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Check the function prototype's noescape bit.


https://reviews.llvm.org/D43841

Files:
  include/clang/AST/Decl.h
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/CodeGen/CGCall.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/disable-tail-call-escaping-block.m
  test/Driver/fno-escaping-block-tail-calls.c

Index: test/Driver/fno-escaping-block-tail-calls.c
===
--- /dev/null
+++ test/Driver/fno-escaping-block-tail-calls.c
@@ -0,0 +1,7 @@
+// RUN: %clang -### %s -fescaping-block-tail-calls -fno-escaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-DISABLE < %t %s
+// CHECK-DISABLE: "-fno-escaping-block-tail-calls"
+
+// RUN: %clang -### %s -fno-escaping-block-tail-calls -fescaping-block-tail-calls 2> %t
+// RUN: FileCheck --check-prefix=CHECK-NO-DISABLE < %t %s
+// CHECK-NO-DISABLE-NOT: "-fno-escaping-block-tail-calls"
Index: test/CodeGenObjC/disable-tail-call-escaping-block.m
===
--- /dev/null
+++ test/CodeGenObjC/disable-tail-call-escaping-block.m
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fblocks -fno-escaping-block-tail-calls -emit-llvm -o - %s | FileCheck %s
+
+// CHECK-LABEL: define void @test(
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE0:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE1:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE2:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE3:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE4:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE5:.*]] to i8*)
+// CHECK: store i8* bitcast (void (i8*)* @[[TEST_BLOCK_INVOKE6:.*]] to i8*)
+
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE0]]({{.*}}) #[[DISABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE1]]({{.*}}) #[[ENABLEATTR:.*]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE2]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE3]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE4]]({{.*}}) #[[ENABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE5]]({{.*}}) #[[DISABLEATTR]] {
+// CHECK: define internal void @[[TEST_BLOCK_INVOKE6]]({{.*}}) #[[ENABLEATTR]] {
+
+// CHECK: attributes #[[ENABLEATTR]] = {{{.*}}"disable-tail-calls"="false"{{.*}}}
+// CHECK: attributes #[[DISABLEATTR]] = {{{.*}}"disable-tail-calls"="true"{{.*}}}
+
+typedef void (^BlockTy)(void);
+typedef void (*NoEscapeFnTy)(__attribute__((noescape)) BlockTy);
+
+void callee0(__attribute__((noescape)) BlockTy);
+void callee1(BlockTy);
+
+__attribute__((objc_root_class))
+@interface C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p;
+-(void)m1:(BlockTy)p;
+@end
+
+@implementation C0
+-(void)m0:(__attribute__((noescape)) BlockTy)p {}
+-(void)m1:(BlockTy)p {}
+@end
+
+NoEscapeFnTy noescapefunc;
+
+void test(id a, C0 *c0) {
+  BlockTy b0 = ^{ (void)a; }; // disable tail-call optimization.
+  callee0(b0);
+  callee0(^{ (void)a; }); // enable tail-call optimization.
+  callee1(^{ (void)a; }); // disable tail-call optimization.
+
+  BlockTy b1 = ^{ (void)a; }; // disable tail-call optimization.
+  [c0 m0:b1];
+  [c0 m0:^{ (void)a; }]; // enable tail-call optimization.
+  [c0 m1:^{ (void)a; }]; // disable tail-call optimization.
+
+  noescapefunc(^{ (void)a; }); // enable tail-call optimization.
+}
Index: lib/Sema/SemaExprObjC.cpp
===
--- lib/Sema/SemaExprObjC.cpp
+++ lib/Sema/SemaExprObjC.cpp
@@ -1613,6 +1613,11 @@
 ParmVarDecl *param = Method->parameters()[i];
 assert(argExpr && "CheckMessageArgumentTypes(): missing expression");
 
+if (param->hasAttr())
+  if (auto *BE = dyn_cast(
+  argExpr->IgnoreParenNoopCasts(Context)))
+BE->getBlockDecl()->setDoesNotEscape();
+
 // Strip the unbridged-cast placeholder expression off unless it's
 // a consumed argument.
 if (argExpr->hasPlaceholderType(BuiltinType::ARCUnbridgedCast) &&
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -4841,6 +4841,10 @@
(!Param || !Param->hasAttr()))
 CFAudited = true;
 
+  if (Proto->getExtParameterInfo(i).isNoEscape())
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+
   InitializedEntity Entity =
   Param ? InitializedEntity::InitializeParameter(Context, Param,
  

[PATCH] D43841: Add an option to disable tail-call optimization for escaping blocks

[John McCall via Phabricator via cfe-commits]

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

Alright, this looks good to me.




Comment at: lib/Sema/SemaExpr.cpp:4846
+if (auto *BE = dyn_cast(Arg->IgnoreParenNoopCasts(Context)))
+  BE->getBlockDecl()->setDoesNotEscape();
+

ahatanak wrote:
> rjmccall wrote:
> > Can this be based on the noescape parameter bit on the function type?
> Oh that's right. It should be able to set the DoesNotEscape bit of a block 
> when it's passed to an indirect function call. I added an IRGen test case 
> that checks tail-call is enabled in such cases.
> 
Thanks!


https://reviews.llvm.org/D43841



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

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In https://reviews.llvm.org/D43248#1024183, @aaron.ballman wrote:

> In https://reviews.llvm.org/D43248#1023720, @jdenny wrote:
>
> > Hi Aaron.  It occurs to me now that this patch has grown rather large and, 
> > in some places, a little subtle.  Would it help the review if I were to 
> > break it up into a patch series that introduces ParamIdx to each attribute, 
> > one at a time?  I'm not trying to rush you, but I hate for the review to be 
> > painful for you if it doesn't have to be.
>
>
> No need to do that -- this review just takes a bit more time for me to 
> complete, but it's reasonably well-factored. Thank you, though!


Sure!  Thanks for the review.




Comment at: include/clang/AST/Attr.h:206
+
+  void cmpable(const ParamIdx &I) const {
+assert(isValid() && I.isValid() &&

aaron.ballman wrote:
> The name here can be improved. How about `checkInvariants()`? Might as well 
> make this inline while you're at it.
Sure, I can change the name.

It's inside the class, so specifying inline is against the LLVM coding 
standards, right?



Comment at: include/clang/AST/Attr.h:236
+  /// parameter.
+  ParamIdx(unsigned Idx, bool HasThis)
+  : Idx(Idx), HasThis(HasThis), IsValid(true) {}

aaron.ballman wrote:
> Is this constructor used anywhere? I didn't see it being used, but I could 
> have missed something. If it's not used, go ahead and remove it.
It's used by the deserialization code generated by ClangAttrEmitter.cpp.



Comment at: include/clang/AST/Attr.h:281-284
+  bool operator<(const ParamIdx &I) const { cmpable(I); return Idx < I.Idx; }
+  bool operator>(const ParamIdx &I) const { cmpable(I); return Idx > I.Idx; }
+  bool operator<=(const ParamIdx &I) const { cmpable(I); return Idx <= I.Idx; }
+  bool operator>=(const ParamIdx &I) const { cmpable(I); return Idx >= I.Idx; }

aaron.ballman wrote:
> Are all of these operations required for the class?
operator== and operator< are needed for sorting and finding.  It seems strange 
to me not to finish the set.



Comment at: include/clang/Basic/Attr.td:172-174
+  // Whether the C++ implicit this parameter is allowed.  Users that construct
+  // attributes from the source code use this information when validating
+  // parameter indices.

aaron.ballman wrote:
> I still find the `AllowThis` parts to be hard to follow, so I want to be sure 
> I understand your design properly. Everything that uses this new argument 
> type sets `AllowsThis` to 0. As written, it sounds like setting that to 0 
> means that the parameter index cannot be used on a C++ instance method, and I 
> know that's not the correct interpretation. Under what circumstances would 
> this be set to 1 instead?
> 
> Looking at the existing code, the only circumstances under which 
> `checkFunctionOrMethodParameterIndex()` was being passed `true` for "allow 
> this" was for XRayLogArgs, which doesn't even use `ParamIdx`. Perhaps this 
> member isn't necessary any longer?
> I still find the AllowThis parts to be hard to follow, so I want to be sure I 
> understand your design properly. Everything that uses this new argument type 
> sets AllowsThis to 0. As written, it sounds like setting that to 0 means that 
> the parameter index cannot be used on a C++ instance method, and I know 
> that's not the correct interpretation.

Right. AllowsThis=0 means it is an error for the attribute in the source code 
to specify the C++ implicit this parameter (index 1).

> Under what circumstances would this be set to 1 instead?
>
> Looking at the existing code, the only circumstances under which 
> checkFunctionOrMethodParameterIndex() was being passed true for "allow this" 
> was for XRayLogArgs, which doesn't even use ParamIdx. Perhaps this member 
> isn't necessary any longer?

Right.  I also noticed this issue, but I probably should have mentioned that in 
a comment in the source instead of just rewording the last paragraph of the 
patch summary.  Sorry.

I thought about removing AllowsThis, but I hesitated because I had already 
implemented it by the time I noticed this issue and because I assumed there 
must be some reason why attributes for C++ have index 1 mean the this 
parameter, so there might be some attribute that could eventually take 
advantage of AllowsThis=1.  Moreover, it makes the related argument to 
checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp clearer.

I don't feel strongly either way, so I'm happy to remove it or keep it.



Comment at: lib/Sema/SemaDeclAttr.cpp:1650-1651
   OwnershipAttr(AL.getLoc(), S.Context, nullptr, nullptr, 0,
-AL.getAttributeSpellingListIndex()).getOwnKind();
+AL.getAttributeSpellingListIndex())
+  .getOwnKind();
 

aaron.ballman wrote:
> This change looks to be unrelated to the patch?
Sorry, I think clang-f

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: include/clang/AST/Attr.h:206
+
+  void cmpable(const ParamIdx &I) const {
+assert(isValid() && I.isValid() &&

jdenny wrote:
> aaron.ballman wrote:
> > The name here can be improved. How about `checkInvariants()`? Might as well 
> > make this inline while you're at it.
> Sure, I can change the name.
> 
> It's inside the class, so specifying inline is against the LLVM coding 
> standards, right?
Derp, you're correct, it's already implicitly inline. Ignore that part of the 
suggestion.



Comment at: include/clang/AST/Attr.h:236
+  /// parameter.
+  ParamIdx(unsigned Idx, bool HasThis)
+  : Idx(Idx), HasThis(HasThis), IsValid(true) {}

jdenny wrote:
> aaron.ballman wrote:
> > Is this constructor used anywhere? I didn't see it being used, but I could 
> > have missed something. If it's not used, go ahead and remove it.
> It's used by the deserialization code generated by ClangAttrEmitter.cpp.
That'd explain why I hadn't seen it.



Comment at: include/clang/AST/Attr.h:281-284
+  bool operator<(const ParamIdx &I) const { cmpable(I); return Idx < I.Idx; }
+  bool operator>(const ParamIdx &I) const { cmpable(I); return Idx > I.Idx; }
+  bool operator<=(const ParamIdx &I) const { cmpable(I); return Idx <= I.Idx; }
+  bool operator>=(const ParamIdx &I) const { cmpable(I); return Idx >= I.Idx; }

jdenny wrote:
> aaron.ballman wrote:
> > Are all of these operations required for the class?
> operator== and operator< are needed for sorting and finding.  It seems 
> strange to me not to finish the set.
I don't think it's actively harmful to do so, but I also don't think it's 
really needed either. Your call.



Comment at: include/clang/Basic/Attr.td:172-174
+  // Whether the C++ implicit this parameter is allowed.  Users that construct
+  // attributes from the source code use this information when validating
+  // parameter indices.

jdenny wrote:
> aaron.ballman wrote:
> > I still find the `AllowThis` parts to be hard to follow, so I want to be 
> > sure I understand your design properly. Everything that uses this new 
> > argument type sets `AllowsThis` to 0. As written, it sounds like setting 
> > that to 0 means that the parameter index cannot be used on a C++ instance 
> > method, and I know that's not the correct interpretation. Under what 
> > circumstances would this be set to 1 instead?
> > 
> > Looking at the existing code, the only circumstances under which 
> > `checkFunctionOrMethodParameterIndex()` was being passed `true` for "allow 
> > this" was for XRayLogArgs, which doesn't even use `ParamIdx`. Perhaps this 
> > member isn't necessary any longer?
> > I still find the AllowThis parts to be hard to follow, so I want to be sure 
> > I understand your design properly. Everything that uses this new argument 
> > type sets AllowsThis to 0. As written, it sounds like setting that to 0 
> > means that the parameter index cannot be used on a C++ instance method, and 
> > I know that's not the correct interpretation.
> 
> Right. AllowsThis=0 means it is an error for the attribute in the source code 
> to specify the C++ implicit this parameter (index 1).
> 
> > Under what circumstances would this be set to 1 instead?
> >
> > Looking at the existing code, the only circumstances under which 
> > checkFunctionOrMethodParameterIndex() was being passed true for "allow 
> > this" was for XRayLogArgs, which doesn't even use ParamIdx. Perhaps this 
> > member isn't necessary any longer?
> 
> Right.  I also noticed this issue, but I probably should have mentioned that 
> in a comment in the source instead of just rewording the last paragraph of 
> the patch summary.  Sorry.
> 
> I thought about removing AllowsThis, but I hesitated because I had already 
> implemented it by the time I noticed this issue and because I assumed there 
> must be some reason why attributes for C++ have index 1 mean the this 
> parameter, so there might be some attribute that could eventually take 
> advantage of AllowsThis=1.  Moreover, it makes the related argument to 
> checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp clearer.
> 
> I don't feel strongly either way, so I'm happy to remove it or keep it.
> Right. AllowsThis=0 means it is an error for the attribute in the source code 
> to specify the C++ implicit this parameter (index 1).

Then if we keep this functionality, I think a better identifier would be 
something like `CanIndexImplicitThis` and the comments could be updated to more 
clearly state what is allowed/disallowed. Then the other uses of "allow this" 
can be updated to use similar terminology for clarity.

> so there might be some attribute that could eventually take advantage of 
> AllowsThis=1. Moreover, it makes the related argument to 
> checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp clearer.

My gut feeling is that th

[PATCH] D43965: [CodeGen] Force the backend to follow clang's EmulatedTLS flag

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: chh, jyknight.

Since LLVM r326341, this is needed for the backend to actually respect the 
EmulatedTLS flag that is set, otherwise it just uses the target default flag 
instead.


Repository:
  rC Clang

https://reviews.llvm.org/D43965

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -432,6 +432,7 @@
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
+  Options.ExplicitEmulatedTLS = true;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -432,6 +432,7 @@
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
+  Options.ExplicitEmulatedTLS = true;
   Options.EmulatedTLS = CodeGenOpts.EmulatedTLS;
   Options.DebuggerTuning = CodeGenOpts.getDebuggerTuning();
   Options.EmitStackSizeSection = CodeGenOpts.StackSizeSection;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] r326485 - Added P0805 to the list of ready bits

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu Mar  1 13:16:07 2018
New Revision: 326485

URL: http://llvm.org/viewvc/llvm-project?rev=326485&view=rev
Log:
Added P0805 to the list of ready bits

Modified:
libcxx/trunk/www/upcoming_meeting.html

Modified: libcxx/trunk/www/upcoming_meeting.html
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/www/upcoming_meeting.html?rev=326485&r1=326484&r2=326485&view=diff
==
--- libcxx/trunk/www/upcoming_meeting.html (original)
+++ libcxx/trunk/www/upcoming_meeting.html Thu Mar  1 13:16:07 2018
@@ -48,7 +48,8 @@
 
   Paper Status
   
-   Paper #GroupPaper 
NameMeetingStatusFirst released version
+   Paper #Paper 
NameMeetingStatus
+   https://wg21.link/P0805R1";>P0805R1Comparing 
ContainersJacksonvillePatch Ready: https://reviews.llvm.org/D43773";>D43773
 
@@ -150,7 +151,7 @@
  2936 - Eric - don't we do this already?
 
 
-Last Updated: 7-Feb-2018
+Last Updated: 1-Mar-2018
 
 
 


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

[PATCH] D43965: [CodeGen] Force the backend to follow clang's EmulatedTLS flag

[Chih-Hung Hsieh via Phabricator via cfe-commits]

chh commandeered this revision.
chh edited reviewers, added: mstorsjo; removed: chh.
chh added a comment.

I will upload a different fix soon.
We should set ExplicitEmulatedTLS only when -f[no-]emulated-tls flag is found 
at command line.
Any front-end should only pass the flag and let backend decide the default 
based on target.


Repository:
  rC Clang

https://reviews.llvm.org/D43965



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

[PATCH] D43248: [Attr] Fix parameter indexing for attributes

[Joel E. Denny via Phabricator via cfe-commits]

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



Comment at: include/clang/Basic/Attr.td:172-174
+  // Whether the C++ implicit this parameter is allowed.  Users that construct
+  // attributes from the source code use this information when validating
+  // parameter indices.

aaron.ballman wrote:
> jdenny wrote:
> > aaron.ballman wrote:
> > > I still find the `AllowThis` parts to be hard to follow, so I want to be 
> > > sure I understand your design properly. Everything that uses this new 
> > > argument type sets `AllowsThis` to 0. As written, it sounds like setting 
> > > that to 0 means that the parameter index cannot be used on a C++ instance 
> > > method, and I know that's not the correct interpretation. Under what 
> > > circumstances would this be set to 1 instead?
> > > 
> > > Looking at the existing code, the only circumstances under which 
> > > `checkFunctionOrMethodParameterIndex()` was being passed `true` for 
> > > "allow this" was for XRayLogArgs, which doesn't even use `ParamIdx`. 
> > > Perhaps this member isn't necessary any longer?
> > > I still find the AllowThis parts to be hard to follow, so I want to be 
> > > sure I understand your design properly. Everything that uses this new 
> > > argument type sets AllowsThis to 0. As written, it sounds like setting 
> > > that to 0 means that the parameter index cannot be used on a C++ instance 
> > > method, and I know that's not the correct interpretation.
> > 
> > Right. AllowsThis=0 means it is an error for the attribute in the source 
> > code to specify the C++ implicit this parameter (index 1).
> > 
> > > Under what circumstances would this be set to 1 instead?
> > >
> > > Looking at the existing code, the only circumstances under which 
> > > checkFunctionOrMethodParameterIndex() was being passed true for "allow 
> > > this" was for XRayLogArgs, which doesn't even use ParamIdx. Perhaps this 
> > > member isn't necessary any longer?
> > 
> > Right.  I also noticed this issue, but I probably should have mentioned 
> > that in a comment in the source instead of just rewording the last 
> > paragraph of the patch summary.  Sorry.
> > 
> > I thought about removing AllowsThis, but I hesitated because I had already 
> > implemented it by the time I noticed this issue and because I assumed there 
> > must be some reason why attributes for C++ have index 1 mean the this 
> > parameter, so there might be some attribute that could eventually take 
> > advantage of AllowsThis=1.  Moreover, it makes the related argument to 
> > checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp clearer.
> > 
> > I don't feel strongly either way, so I'm happy to remove it or keep it.
> > Right. AllowsThis=0 means it is an error for the attribute in the source 
> > code to specify the C++ implicit this parameter (index 1).
> 
> Then if we keep this functionality, I think a better identifier would be 
> something like `CanIndexImplicitThis` and the comments could be updated to 
> more clearly state what is allowed/disallowed. Then the other uses of "allow 
> this" can be updated to use similar terminology for clarity.
> 
> > so there might be some attribute that could eventually take advantage of 
> > AllowsThis=1. Moreover, it makes the related argument to 
> > checkFunctionOrMethodParameterIndex in SemaDeclAttr.cpp clearer.
> 
> My gut feeling is that this functionality isn't going to be needed all that 
> frequently. If we get a second case where we need it, then I'd say it might 
> make more sense to add it.
> 
> Attributes that use positional arguments should hopefully be the exception, 
> not the rule, because those indexes are horribly fragile.
> 
> What do you think?
> My gut feeling is that this functionality isn't going to be needed all that 
> frequently. If we get a second case where we need it, then I'd say it might 
> make more sense to add it.
> 
> Attributes that use positional arguments should hopefully be the exception, 
> not the rule, because those indexes are horribly fragile.
> 
> What do you think?

I'm guessing you have more experience with attributes in general, so let's go 
with your gut.  :-)



Comment at: lib/Sema/SemaDeclAttr.cpp:4549-4551
+if (ArgumentIdx.getASTIndex() >= getFunctionOrMethodNumParams(D) ||
+!getFunctionOrMethodParamType(D, ArgumentIdx.getASTIndex())
+ ->isPointerType())

aaron.ballman wrote:
> jdenny wrote:
> > aaron.ballman wrote:
> > > Is this formatting produced by clang-format?
> > Yes.
> How funky. :-)
Agreed.


https://reviews.llvm.org/D43248



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

[PATCH] D43967: Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: a.sidorin, xazax.hun, szepet.
Herald added subscribers: cfe-commits, dkrupp, rnkovacs.

[ASTImporter] Add a helper test Fixture, so we can add tests which may
check internal attributes of AST nodes. Also it makes possible to import from
several "From" contexts.


Repository:
  rC Clang

https://reviews.llvm.org/D43967

Files:
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/DeclMatcher.h

Index: unittests/AST/DeclMatcher.h
===
--- /dev/null
+++ unittests/AST/DeclMatcher.h
@@ -0,0 +1,68 @@
+//===- unittest/AST/DeclMatcher.h - AST unit test support ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+#define LLVM_CLANG_UNITTESTS_AST_DECLMATCHER_H
+
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+namespace clang {
+namespace ast_matchers {
+
+enum class DeclMatcherKind { First, Last };
+
+// Matcher class to retrieve the first/last matched node under a given AST.
+template 
+class DeclMatcher : public MatchFinder::MatchCallback {
+  NodeType *Node = nullptr;
+  void run(const MatchFinder::MatchResult &Result) override {
+if ((MatcherKind == DeclMatcherKind::First && Node == nullptr) ||
+MatcherKind == DeclMatcherKind::Last) {
+  Node = const_cast(Result.Nodes.getNodeAs(""));
+}
+  }
+public:
+  // Returns the first/last matched node under the tree rooted in `D`.
+  template 
+  NodeType *match(const Decl *D, const MatcherType &AMatcher) {
+MatchFinder Finder;
+Finder.addMatcher(AMatcher.bind(""), this);
+Finder.matchAST(D->getASTContext());
+assert(Node);
+return Node;
+  }
+};
+template 
+using LastDeclMatcher = DeclMatcher;
+template 
+using FirstDeclMatcher = DeclMatcher;
+
+template 
+class DeclCounter : public MatchFinder::MatchCallback {
+  unsigned count = 0;
+  void run(const MatchFinder::MatchResult &Result) override {
+  if(Result.Nodes.getNodeAs("")) {
+++count;
+  }
+  }
+public:
+  // Returns the number of matched nodes under the tree rooted in `D`.
+  template 
+  unsigned match(const Decl *D, const MatcherType &AMatcher) {
+MatchFinder Finder;
+Finder.addMatcher(AMatcher.bind(""), this);
+Finder.matchAST(D->getASTContext());
+return count;
+  }
+};
+
+} // end namespace ast_matchers
+} // end namespace clang
+
+#endif
Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -17,6 +17,8 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Tooling/Tooling.h"
+
+#include "DeclMatcher.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -29,7 +31,7 @@
   return Lang == Lang_CXX || Lang == Lang_CXX11;
 }
 
-static RunOptions getRunOptionsForLanguage(Language Lang) {
+static ArgVector getBasicRunOptionsForLanguage(Language Lang) {
   ArgVector BasicArgs;
   // Test with basic arguments.
   switch (Lang) {
@@ -49,6 +51,11 @@
   case Lang_OBJCXX:
 llvm_unreachable("Not implemented yet!");
   }
+  return BasicArgs;
+}
+
+static RunOptions getRunOptionsForLanguage(Language Lang) {
+  ArgVector BasicArgs = getBasicRunOptionsForLanguage(Lang);
 
   // For C++, test with "-fdelayed-template-parsing" enabled to handle MSVC
   // default behaviour.
@@ -133,6 +140,146 @@
  Verifier, AMatcher));
 }
 
+class Fixture : public ::testing::Test {
+
+  const char *const InputFileName = "input.cc";
+  const char *const OutputFileName = "output.cc";
+
+  //Buffers for the contexts, must live in test scope
+  std::string ToCode;
+
+  struct TU {
+std::string Code;
+std::string FileName;
+std::unique_ptr Unit;
+TranslationUnitDecl *TUDecl = nullptr;
+TU(StringRef Code, StringRef FileName, ArgVector Args)
+: Code(Code), FileName(FileName),
+  Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
+ this->FileName)),
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {}
+  };
+  // We may have several From context and related translation units.
+  std::list FromTUs;
+
+  // Creates a virtual file and assigns that to the To context.  If the file
+  // already exists then the file will not be created again as a duplicate.
+  void createVirtualFile(StringRef FileName, const std::string &Code) {
+assert(ToAST);
+ASTContext &ToCtx = ToAST->getASTContext();
+vfs::OverlayFileSystem *OFS = static_cast(
+ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+vfs::InMemoryFileSystem *MFS =
+st

[PATCH] D43967: Add test helper Fixture

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

We add several test cases here, some of them are disabled.
We plan to pass the disabled tests in different patches.


Repository:
  rC Clang

https://reviews.llvm.org/D43967



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

[PATCH] D43965: [CodeGen] Force the backend to follow clang's EmulatedTLS flag

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Ok, thanks. Yes, this should probably only set it if some of those flags are 
set here.

FWIW, clang also has got a list of targets where emulated TLS is enabled by 
default - could this be omitted somehow, now that LLVM can handle that on its 
own?


Repository:
  rC Clang

https://reviews.llvm.org/D43965



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