[PATCH] D65854: [diagtool] Use `operator<<(Colors)` to print out colored output.

[Rui Ueyama via Phabricator via cfe-commits]

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



Comment at: clang/tools/diagtool/TreeView.cpp:167
+  if (!hasColors(out))
+out.enable_colors(false);
+

MaskRay wrote:
> `out.enable_colors(out.has_colors());`
> 
> It looks the function `hasColors` is overengineered. `out` in these files can 
> only be `llvm::outs()`. It doesn't have to check if `llvm::errs()` is 
> connected to a terminal.
Done. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65854



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

[PATCH] D65925: Add SpaceInEmptyBlock option for WebKit

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: sammccall, MyDeveloperDay, djasper, klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

See PR40840


Repository:
  rC Clang

https://reviews.llvm.org/D65925

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

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -3683,6 +3683,11 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
+  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
+  FormatStyle Style = getLLVMStyle();
+  Style.AllowShortBlocksOnASingleLine = true;
+  Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
 TEST_F(FormatTest, FormatBeginBlockEndMacros) {
@@ -11756,6 +11761,7 @@
   CHECK_PARSE_BOOL(SpacesInParentheses);
   CHECK_PARSE_BOOL(SpacesInSquareBrackets);
   CHECK_PARSE_BOOL(SpacesInAngles);
+  CHECK_PARSE_BOOL(SpaceInEmptyBlock);
   CHECK_PARSE_BOOL(SpaceInEmptyParentheses);
   CHECK_PARSE_BOOL(SpacesInContainerLiterals);
   CHECK_PARSE_BOOL(SpacesInCStyleCastParentheses);
Index: clang/lib/Format/UnwrappedLineFormatter.cpp
===
--- clang/lib/Format/UnwrappedLineFormatter.cpp
+++ clang/lib/Format/UnwrappedLineFormatter.cpp
@@ -551,7 +551,7 @@
   (Tok->getNextNonComment() == nullptr ||
Tok->getNextNonComment()->is(tok::semi))) {
 // We merge empty blocks even if the line exceeds the column limit.
-Tok->SpacesRequiredBefore = 0;
+Tok->SpacesRequiredBefore = Style.SpaceInEmptyBlock ? 1 : 0;
 Tok->CanBreakBefore = true;
 return 1;
   } else if (Limit != 0 && !Line.startsWithNamespace() &&
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -494,6 +494,7 @@
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
+IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock);
 IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses);
 IO.mapOptional("SpacesBeforeTrailingComments",
Style.SpacesBeforeTrailingComments);
@@ -734,6 +735,7 @@
   LLVMStyle.ReflowComments = true;
   LLVMStyle.SpacesInParentheses = false;
   LLVMStyle.SpacesInSquareBrackets = false;
+  LLVMStyle.SpaceInEmptyBlock = false;
   LLVMStyle.SpaceInEmptyParentheses = false;
   LLVMStyle.SpacesInContainerLiterals = true;
   LLVMStyle.SpacesInCStyleCastParentheses = false;
@@ -979,6 +981,7 @@
   Style.ObjCSpaceAfterProperty = true;
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.SpaceBeforeCpp11BracedList = true;
+  Style.SpaceInEmptyBlock = true;
   return Style;
 }
 
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1799,6 +1799,14 @@
   /// \endcode
   bool SpaceBeforeRangeBasedForLoopColon;
 
+  /// If ``true``, spaces may be inserted into ``{}``.
+  /// \code
+  ///true:false:
+  ///void f() { }   vs.   void f() {}
+  ///while (true) { } while (true) {}
+  /// \endcode
+  bool SpaceInEmptyBlock;
+
   /// If ``true``, spaces may be inserted into ``()``.
   /// \code
   ///true:false:
@@ -1995,6 +2003,7 @@
SpaceBeforeParens == R.SpaceBeforeParens &&
SpaceBeforeRangeBasedForLoopColon ==
R.SpaceBeforeRangeBasedForLoopColon &&
+   SpaceInEmptyBlock == R.SpaceInEmptyBlock &&
SpaceInEmptyParentheses == R.SpaceInEmptyParentheses &&
SpacesBeforeTrailingComments == R.SpacesBeforeTrailingComments &&
SpacesInAngles == R.SpacesInAngles &&
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -192,20 +192,6 @@
 
 
 
-**AlignConsecutiveMacros** (``bool``)
-  If ``true``, aligns consecutive C/C++ preprocessor macros.
-
-  This will align the C/C++ preprocessor macros of consecutive lines. This
-  will result in formattings like
-
-  .. code-block:: c++
-
-#define SHORT_NAME   42
-#define LONGER_NAME  0x007f
-#define EVEN_LONGER_NAME (2)
-#define foo(x)   (x * x)
-#define bar(y, z)(y + z)
-
 *

[PATCH] D65917: [clang-tidy] Added check for the Google style guide's category method naming rule.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:16
+namespace {
+const char *kCustomCategoryMethodIdentifier = "ThisIsACategoryMethod";
+} // anonymous namespace

Please use static. See LLVM Coding Guidelines.



Comment at: 
clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp:58
+  std::string method_name = method_declaration->getNameAsString();
+  auto owning_objc_class_interface = method_declaration->getClassInterface();
+  if (!owning_objc_class_interface) {

Please don't use auto, unless type presents in same statement or in case of 
iterators.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:74
+  Warns when Objective-C category method names are not properly prefixed (e.g.
+  gmo_methodName) unless the category is extending a class with a 
(configurable)
+  whitelisted prefix.

Please enclose gmo_methodName in double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:6
+
+Finds method declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.

Please make first statement same as in Release Notes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:16
+This checker ensures that all methods in categories have some sort of prefix
+(e.g. gmo_). It excludes categories on classes whose names have a whitelisted
+three-letter prefix.

Please enclose gmo_ in double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:19
+
+You should set the clang option WhitelistedPrefixes to a semicolon-delimited
+lits of class prefixes within your project if you want to be able to create

See other checks documentation for proper option section style.

clang -> :program:`clang-tidy` and enclose WhitelistedPrefixes in single 
back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:24
+For example, the following code sample is a properly prefixed method on a
+non-owned class (NSObject):
+

Please enclose NSObject in double back-ticks.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst:31
+
+If you whitelist the QED three-letter prefix, the following code sample
+is also allowed:

Please enclose QED in double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65917



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

r368251 - [Driver] Delete XFAIL: windows-msvc after D65880/r368245

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Aug  7 21:56:21 2019
New Revision: 368251

URL: http://llvm.org/viewvc/llvm-project?rev=368251&view=rev
Log:
[Driver] Delete XFAIL: windows-msvc after D65880/r368245

`-target %itanium_abi_triple` fixed the problem.

Modified:
cfe/trunk/test/Driver/linker-opts.c

Modified: cfe/trunk/test/Driver/linker-opts.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linker-opts.c?rev=368251&r1=368250&r2=368251&view=diff
==
--- cfe/trunk/test/Driver/linker-opts.c (original)
+++ cfe/trunk/test/Driver/linker-opts.c Wed Aug  7 21:56:21 2019
@@ -7,7 +7,6 @@
 // CHECK: "-la"
 
 // GCC driver is used as linker on cygming. It should be aware of LIBRARY_PATH.
-// XFAIL: windows-msvc
 // REQUIRES: clang-driver
 // REQUIRES: native
 


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

[PATCH] D65915: [clang-doc] Protect Index with mutex during reducing and generation stage

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D65915



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

[PATCH] D65880: [Driver] Move LIBRARY_PATH before user inputs

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@hfinkel Thanks!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D65880



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

[PATCH] D65880: [Driver] Move LIBRARY_PATH before user inputs

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368245: [Driver] Move LIBRARY_PATH before user inputs 
(authored by MaskRay, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65880

Files:
  cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
  cfe/trunk/test/Driver/linker-opts.c


Index: cfe/trunk/test/Driver/linker-opts.c
===
--- cfe/trunk/test/Driver/linker-opts.c
+++ cfe/trunk/test/Driver/linker-opts.c
@@ -1,8 +1,10 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
 //
-// RUN: env LIBRARY_PATH=%t/test1 %clang -x c %s -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target %itanium_abi_triple %s -la 
-### 2>&1 | FileCheck %s
 // CHECK: "-L{{.*}}/test1"
+// CHECK: "{{[^"]+}}.o"
+// CHECK: "-la"
 
 // GCC driver is used as linker on cygming. It should be aware of LIBRARY_PATH.
 // XFAIL: windows-msvc
@@ -10,8 +12,8 @@
 // REQUIRES: native
 
 // Make sure that LIBRARY_PATH works for both i386 and x86_64 on Darwin.
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -### 
2>&1 | FileCheck %s
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin  %s -### 
2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -la 
-### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin %s -la -### 
2>&1 | FileCheck %s
 //
 // Make sure that we don't warn on unused compiler arguments.
 // RUN: %clang -Xclang -I. -x c %s -c -o %t/tmp.o
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -145,6 +145,11 @@
   // (constructed via -Xarch_).
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
+  // LIBRARY_PATH are included before user inputs and only supported on native
+  // toolchains.
+  if (!TC.isCrossCompiling())
+addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+
   for (const auto &II : Inputs) {
 // If the current tool chain refers to an OpenMP or HIP offloading host, we
 // should ignore inputs that refer to OpenMP or HIP offloading devices -
@@ -182,12 +187,6 @@
   A.renderAsInput(Args, CmdArgs);
 }
   }
-
-  // LIBRARY_PATH - included following the user specified library paths.
-  //and only supported on native toolchains.
-  if (!TC.isCrossCompiling()) {
-addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
-  }
 }
 
 void tools::AddTargetFeature(const ArgList &Args,


Index: cfe/trunk/test/Driver/linker-opts.c
===
--- cfe/trunk/test/Driver/linker-opts.c
+++ cfe/trunk/test/Driver/linker-opts.c
@@ -1,8 +1,10 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
 //
-// RUN: env LIBRARY_PATH=%t/test1 %clang -x c %s -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target %itanium_abi_triple %s -la -### 2>&1 | FileCheck %s
 // CHECK: "-L{{.*}}/test1"
+// CHECK: "{{[^"]+}}.o"
+// CHECK: "-la"
 
 // GCC driver is used as linker on cygming. It should be aware of LIBRARY_PATH.
 // XFAIL: windows-msvc
@@ -10,8 +12,8 @@
 // REQUIRES: native
 
 // Make sure that LIBRARY_PATH works for both i386 and x86_64 on Darwin.
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -### 2>&1 | FileCheck %s
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin  %s -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -la -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin %s -la -### 2>&1 | FileCheck %s
 //
 // Make sure that we don't warn on unused compiler arguments.
 // RUN: %clang -Xclang -I. -x c %s -c -o %t/tmp.o
Index: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
@@ -145,6 +145,11 @@
   // (constructed via -Xarch_).
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
+  // LIBRARY_PATH are included before user inputs and only supported on native
+  // toolchains.
+  if (!TC.isCrossCompiling())
+addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+
   for (const auto &II : Inputs) {
 // If the current tool chain refers to an OpenMP or HIP offloading host, we
 // should ignore inputs that refer to OpenMP or HIP offloading devices -
@@ -182,12 +187,6 @@
   A.renderAsInput(Args, CmdArgs);
 }
   }
-
-  // LIBRARY_PATH - included following the user specified library paths.
-  //and only supported on native toolchains.
-  if (!TC.isCrossComp

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 214053.
stephanemoore added a comment.

Add tests for `isDirectlyDerivedFrom`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -573,6 +573,111 @@
   EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
 }
 
+TEST(DeclarationMatcher, ObjCClassIsDerived) {
+  DeclarationMatcher IsDerivedFromX = objcInterfaceDecl(isDerivedFrom("X"));
+  EXPECT_TRUE(
+  matchesObjC("@interface X @end @interface Y : X @end", IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y<__covariant ObjectType> : X @end",
+  IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class X;", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end @compatibility_alias Y X;",
+ IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end typedef X Y;", IsDerivedFromX));
+
+  DeclarationMatcher IsDirectlyDerivedFromX =
+  objcInterfaceDecl(isDirectlyDerivedFrom("X"));
+  EXPECT_TRUE(
+  matchesObjC("@interface X @end @interface Y : X @end", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y<__covariant ObjectType> : X @end",
+  IsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  IsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end",
+  IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class X;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end @compatibility_alias Y X;",
+ IsDirectlyDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end typedef X Y;",
+ IsDirectlyDerivedFromX));
+
+  DeclarationMatcher IsAX = objcInterfaceDecl(isSameOrDerivedFrom("X"));
+  EXPECT_TRUE(matchesObjC("@interface X @end @interface Y : X @end", IsAX));
+  EXPECT_TRUE(matchesObjC("@interface X @end", IsAX));
+  EXPECT_TRUE(matchesObjC("@class X;", IsAX));
+  EXPECT_TRUE(notMatchesObjC("@interface Y @end", IsAX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsAX));
+
+  DeclarationMatcher ZIsDerivedFromX =
+  objcInterfaceDecl(hasName("Z"), isDerivedFrom("X"));
+  DeclarationMatcher ZIsDirectlyDerivedFromX =
+  objcInterfaceDecl(hasName("Z"), isDirectlyDerivedFrom("X"));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y : X @end @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC("@interface X @end @interface Y : X @end typedef Y "
+  "V; typedef V W; @interface Z : W @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end", ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end",
+  ZIsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end typedef A X; typedef A Y; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end typedef A X; typedef A Y; @interface Z : Y @end",
+  ZIsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  ZIsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface Y @end @compatibility_alias X Y; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface Y @end @compatibility_alias X Y; @interface Z : Y @end",
+  ZIsDirectlyDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end @compatibility_alias X A; @compatibility_alias Y A;"
+  "@interfac

r368245 - [Driver] Move LIBRARY_PATH before user inputs

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Aug  7 18:55:27 2019
New Revision: 368245

URL: http://llvm.org/viewvc/llvm-project?rev=368245&view=rev
Log:
[Driver] Move LIBRARY_PATH before user inputs

Fixes PR16786

Currently, library paths specified by LIBRARY_PATH are placed after inputs: 
`inputs LIBRARY_PATH stdlib`
In gcc, the order is: `LIBRARY_PATH inputs stdlib` if not cross compiling.
(On Darwin targets, isCrossCompiling() always returns false.)

This patch changes the behavior to match gcc.

Reviewed By: hfinkel

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

Modified:
cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
cfe/trunk/test/Driver/linker-opts.c

Modified: cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp?rev=368245&r1=368244&r2=368245&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/CommonArgs.cpp Wed Aug  7 18:55:27 2019
@@ -145,6 +145,11 @@ void tools::AddLinkerInputs(const ToolCh
   // (constructed via -Xarch_).
   Args.AddAllArgValues(CmdArgs, options::OPT_Zlinker_input);
 
+  // LIBRARY_PATH are included before user inputs and only supported on native
+  // toolchains.
+  if (!TC.isCrossCompiling())
+addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
+
   for (const auto &II : Inputs) {
 // If the current tool chain refers to an OpenMP or HIP offloading host, we
 // should ignore inputs that refer to OpenMP or HIP offloading devices -
@@ -182,12 +187,6 @@ void tools::AddLinkerInputs(const ToolCh
   A.renderAsInput(Args, CmdArgs);
 }
   }
-
-  // LIBRARY_PATH - included following the user specified library paths.
-  //and only supported on native toolchains.
-  if (!TC.isCrossCompiling()) {
-addDirectoryList(Args, CmdArgs, "-L", "LIBRARY_PATH");
-  }
 }
 
 void tools::AddTargetFeature(const ArgList &Args,

Modified: cfe/trunk/test/Driver/linker-opts.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linker-opts.c?rev=368245&r1=368244&r2=368245&view=diff
==
--- cfe/trunk/test/Driver/linker-opts.c (original)
+++ cfe/trunk/test/Driver/linker-opts.c Wed Aug  7 18:55:27 2019
@@ -1,8 +1,10 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
 //
-// RUN: env LIBRARY_PATH=%t/test1 %clang -x c %s -### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target %itanium_abi_triple %s -la 
-### 2>&1 | FileCheck %s
 // CHECK: "-L{{.*}}/test1"
+// CHECK: "{{[^"]+}}.o"
+// CHECK: "-la"
 
 // GCC driver is used as linker on cygming. It should be aware of LIBRARY_PATH.
 // XFAIL: windows-msvc
@@ -10,8 +12,8 @@
 // REQUIRES: native
 
 // Make sure that LIBRARY_PATH works for both i386 and x86_64 on Darwin.
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -### 
2>&1 | FileCheck %s
-// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin  %s -### 
2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target x86_64-apple-darwin %s -la 
-### 2>&1 | FileCheck %s
+// RUN: env LIBRARY_PATH=%t/test1 %clang -target i386-apple-darwin %s -la -### 
2>&1 | FileCheck %s
 //
 // Make sure that we don't warn on unused compiler arguments.
 // RUN: %clang -Xclang -I. -x c %s -c -o %t/tmp.o


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

r368244 - Inline diagnostic text into .td file. NFC.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Wed Aug  7 18:45:31 2019
New Revision: 368244

URL: http://llvm.org/viewvc/llvm-project?rev=368244&view=rev
Log:
Inline diagnostic text into .td file.  NFC.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaExpr.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=368244&r1=368243&r2=368244&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Aug  7 18:45:31 
2019
@@ -8150,7 +8150,8 @@ def warn_unsupported_lifetime_extension
 // should result in a warning, since these always evaluate to a constant.
 // Array comparisons have similar warnings
 def warn_comparison_always : Warning<
-  "%select{self-|array }0comparison always evaluates to %select{a 
constant|%2}1">,
+  "%select{self-|array }0comparison always evaluates to "
+  "%select{a constant|true|false|'std::strong_ordering::equal'}1">,
   InGroup;
 def warn_comparison_bitwise_always : Warning<
   "bitwise comparison always evaluates to %select{false|true}0">,

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=368244&r1=368243&r2=368244&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Aug  7 18:45:31 2019
@@ -10174,45 +10174,55 @@ static void diagnoseTautologicalComparis
   // result.
   ValueDecl *DL = getCompareDecl(LHSStripped);
   ValueDecl *DR = getCompareDecl(RHSStripped);
+
+  // Used for indexing into %select in warn_comparison_always
+  enum {
+AlwaysConstant,
+AlwaysTrue,
+AlwaysFalse,
+AlwaysEqual, // std::strong_ordering::equal from operator<=>
+  };
   if (DL && DR && declaresSameEntity(DL, DR)) {
-StringRef Result;
+unsigned Result;
 switch (Opc) {
 case BO_EQ: case BO_LE: case BO_GE:
-  Result = "true";
+  Result = AlwaysTrue;
   break;
 case BO_NE: case BO_LT: case BO_GT:
-  Result = "false";
+  Result = AlwaysFalse;
   break;
 case BO_Cmp:
-  Result = "'std::strong_ordering::equal'";
+  Result = AlwaysEqual;
   break;
 default:
+  Result = AlwaysConstant;
   break;
 }
 S.DiagRuntimeBehavior(Loc, nullptr,
   S.PDiag(diag::warn_comparison_always)
-  << 0 /*self-comparison*/ << !Result.empty()
+  << 0 /*self-comparison*/
   << Result);
   } else if (DL && DR &&
  DL->getType()->isArrayType() && DR->getType()->isArrayType() &&
  !DL->isWeak() && !DR->isWeak()) {
 // What is it always going to evaluate to?
-StringRef Result;
+unsigned Result;
 switch(Opc) {
 case BO_EQ: // e.g. array1 == array2
-  Result = "false";
+  Result = AlwaysFalse;
   break;
 case BO_NE: // e.g. array1 != array2
-  Result = "true";
+  Result = AlwaysTrue;
   break;
 default: // e.g. array1 <= array2
   // The best we can say is 'a constant'
+  Result = AlwaysConstant;
   break;
 }
 S.DiagRuntimeBehavior(Loc, nullptr,
   S.PDiag(diag::warn_comparison_always)
   << 1 /*array comparison*/
-  << !Result.empty() << Result);
+  << Result);
   }
 
   if (isa(LHSStripped))


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

[PATCH] D65918: [clang-doc] Generate an HTML index file

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added reviewers: juliehockett, jakehehrlich.
DiegoAstiazaran added a project: clang-tools-extra.
Herald added a subscriber: arphaman.

clang-doc now generates a file that contains only an index to all the infos 
that can be used as the landing page for the generated website.


https://reviews.llvm.org/D65918

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp


Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -693,6 +693,24 @@
   return true;
 }
 
+static bool GenIndex(const ClangDocContext &CDCtx) {
+  std::error_code FileErr, OK;
+  llvm::SmallString<128> IndexPath;
+  llvm::sys::path::native(CDCtx.OutDirectory, IndexPath);
+  llvm::sys::path::append(IndexPath, "index.html");
+  llvm::raw_fd_ostream IndexOS(IndexPath, FileErr, llvm::sys::fs::F_None);
+  if (FileErr != OK) {
+llvm::errs() << "Error creating main index: " << FileErr.message() << "\n";
+return false;
+  }
+  HTMLFile F;
+  std::vector> BasicNodes =
+  genCommonFileNodes("Index", "", CDCtx);
+  AppendVector(std::move(BasicNodes), F.Children);
+  F.Render(IndexOS);
+  return true;
+}
+
 static bool CopyFile(StringRef FilePath, StringRef OutDirectory) {
   llvm::SmallString<128> PathWrite;
   llvm::sys::path::native(OutDirectory, PathWrite);
@@ -711,7 +729,7 @@
 }
 
 bool HTMLGenerator::createResources(ClangDocContext &CDCtx) {
-  if (!SerializeIndex(CDCtx))
+  if (!SerializeIndex(CDCtx) || !GenIndex(CDCtx))
 return false;
   for (const auto &FilePath : CDCtx.UserStylesheets)
 if (!CopyFile(FilePath, CDCtx.OutDirectory))


Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -693,6 +693,24 @@
   return true;
 }
 
+static bool GenIndex(const ClangDocContext &CDCtx) {
+  std::error_code FileErr, OK;
+  llvm::SmallString<128> IndexPath;
+  llvm::sys::path::native(CDCtx.OutDirectory, IndexPath);
+  llvm::sys::path::append(IndexPath, "index.html");
+  llvm::raw_fd_ostream IndexOS(IndexPath, FileErr, llvm::sys::fs::F_None);
+  if (FileErr != OK) {
+llvm::errs() << "Error creating main index: " << FileErr.message() << "\n";
+return false;
+  }
+  HTMLFile F;
+  std::vector> BasicNodes =
+  genCommonFileNodes("Index", "", CDCtx);
+  AppendVector(std::move(BasicNodes), F.Children);
+  F.Render(IndexOS);
+  return true;
+}
+
 static bool CopyFile(StringRef FilePath, StringRef OutDirectory) {
   llvm::SmallString<128> PathWrite;
   llvm::sys::path::native(OutDirectory, PathWrite);
@@ -711,7 +729,7 @@
 }
 
 bool HTMLGenerator::createResources(ClangDocContext &CDCtx) {
-  if (!SerializeIndex(CDCtx))
+  if (!SerializeIndex(CDCtx) || !GenIndex(CDCtx))
 return false;
   for (const auto &FilePath : CDCtx.UserStylesheets)
 if (!CopyFile(FilePath, CDCtx.OutDirectory))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65917: Added clang-tidy module for the Google style guide's category method naming rule.

[David Gatwood via Phabricator via cfe-commits]

dgatwood created this revision.
dgatwood added a reviewer: stephanemoore.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65917

Files:
  clang-tools-extra/clang-tidy/google/CMakeLists.txt
  clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
  clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.cpp
  clang-tools-extra/clang-tidy/google/RequireCategoryMethodPrefixesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m

Index: clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/google-objc-require-category-method-prefixes.m
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s google-objc-require-category-method-prefixes %t -config="{CheckOptions: [{key: google-objc-require-category-method-prefixes.WhitelistedPrefixes, value: GMO}]}"
+
+@class NSString;
+
+@interface NSURL
++ (nullable instancetype)URLWithString:(NSString *)URLString;
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+@interface NSURL (CustomExtension)
+
+- (void)unprefixedMethod;
+- (void)unprefixed;
+- (void)justprefixed_;
+
+@end
+
+// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method unprefixedMethod is not properly prefixed. [google-objc-require-category-method-prefixes]
+// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method unprefixed is not properly prefixed. [google-objc-require-category-method-prefixes]
+// CHECK-MESSAGES: :[[@LINE-6]]:1: warning: the category method justprefixed_ is not properly prefixed. [google-objc-require-category-method-prefixes]
+
+@interface NSURL (CustomExtension2)
+- (void)gmo_prefixedMethod;
+@end
+
+@interface GMOURL
++ (nullable instancetype)URLWithString:(NSString *)URLString;
++ (instancetype)alloc;
+- (instancetype)init;
+@end
+
+@interface GMOURL (CustomExtension3)
+- (void)unprefixedMethodInWhitelistedClass;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -229,6 +229,7 @@
google-objc-avoid-throwing-exception
google-objc-function-naming
google-objc-global-variable-declaration
+   google-objc-require-category-method-prefixes
google-readability-avoid-underscore-in-googletest-name
google-readability-braces-around-statements (redirects to readability-braces-around-statements) 
google-readability-casting
Index: clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google-objc-require-category-method-prefixes.rst
@@ -0,0 +1,39 @@
+.. title:: clang-tidy - google-objc-require-category-method-prefixes
+
+google-objc-require-category-method-prefixes
+===
+
+Finds method declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The Google Objective-C style guide requires
+`prefixes for methods http://go/objc-style#Category_Names`_ in categories on
+classes that you don't control (for example, categories on Apple or third-party
+framework classes or classes created by other teams) to prevent name collisions
+when those frameworks are updated.
+
+This checker ensures that all methods in categories have some sort of prefix
+(e.g. gmo_). It excludes categories on classes whose names have a whitelisted
+three-letter prefix.
+
+You should set the clang option WhitelistedPrefixes to a semicolon-delimited
+lits of class prefixes within your project if you want to be able to create
+unprefixed category methods on classes whose names begin with those prefixes.
+
+For example, the following code sample is a properly prefixed method on a
+non-owned class (NSObject):
+
+.. code-block:: objc
+  @interface NSObject (QEDMyCategory)
+  - (BOOL)qed_myCustomMethod;
+  @end
+
+If you whitelist the QED three-letter prefix, the following code sample
+is also allowed:
+
+.. code-block:: objc
+
+  @interface QEDMyClass (MyCategory)
+  - (BOOL)myCustomMethod;
+  @end
+
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,13 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`google-objc-require-category-method-prefixes
+  ` check.
+
+  Warns when Objective-C category method names are not prope

[PATCH] D65493: Modernize atomic detection and usage

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added a comment.

This looks like a good simplification, but I think `call_once` could be 
simplified more.




Comment at: llvm/cmake/modules/CheckAtomic.cmake:46
+  if (NOT HAVE_ATOMICS_WITH_LIB)
+   message(FATAL_ERROR "Host compiler must support atomic!")
   endif()

Indentation?



Comment at: llvm/cmake/modules/CheckAtomic.cmake:75
 
-## TODO: This define is only used for the legacy atomic operations in
-## llvm's Atomic.h, which should be replaced.  Other code simply
-## assumes C++11  works.
-CHECK_CXX_SOURCE_COMPILES("
-#ifdef _MSC_VER
-#include 
-#endif
-int main() {
-#ifdef _MSC_VER
-volatile LONG val = 1;
-MemoryBarrier();
-InterlockedCompareExchange(&val, 0, 1);
-InterlockedIncrement(&val);
-InterlockedDecrement(&val);
-#else
-volatile unsigned long val = 1;
-__sync_synchronize();
-__sync_val_compare_and_swap(&val, 1, 0);
-__sync_add_and_fetch(&val, 1);
-__sync_sub_and_fetch(&val, 1);
-#endif
-return 0;
-  }
-" LLVM_HAS_ATOMICS)
-
 if( NOT LLVM_HAS_ATOMICS )
   message(STATUS "Warning: LLVM will be built thread-unsafe because atomic 
builtins are missing")

Does anything else set `LLVM_HAS_ATOMICS`?



Comment at: llvm/include/llvm/Support/Threading.h:111
+  std::atomic_thread_fence(std::memory_order_seq_cst);
   TsanIgnoreWritesBegin();
   TsanHappensBefore(&flag.status);

Not sure we still need the tsan hooks when using `std::atomic`.



Comment at: llvm/include/llvm/Support/Threading.h:118
+  InitStatus tmp = flag.status;
+  std::atomic_thread_fence(std::memory_order_seq_cst);
   while (tmp != Done) {

Do we actually need thread fences here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65493



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Jian Cai via Phabricator via cfe-commits]

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

In D65019#1619511 , @efriedma wrote:

> This seems better.
>
> I'm not sure I follow why this needs special handling in 
> SelectionDAGBuilder::visitIntrinsicCall, as opposed to just using 
> ISD::INTRINSIC_VOID like other similar target-specific intrinsics.  (You can 
> custom-lower INTRINSIC_VOID in ARMTargetLowering::LowerOperation, if that 
> makes it easier.)


Thanks for the suggestion, and I agree the code would look cleaner this way.  
But I have some questions on implementation details, and please bear with me if 
they seem naive since I am new to backend. So I have been trying to reuse the 
code of  ARMTargetLowering::LowerCall to build a SelectionDAG call node for the 
new intrinsic, which essentially is a function call with a push instruction 
before. If we handle the intrinsic at ARMTargetLowering::LowerOperation until 
legalize DAG stage, then I am not sure if we can still do that, as some of the 
information needed is gone by this stage. I did see code handling ARM intrinsic 
on ARMTargetLowering::LowerOperation, but they didn't seem to need to generate 
function calls later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Stephane Moore via Phabricator via cfe-commits]

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

Whoops; forgot to add test cases for `isDirectlyDerivedFrom` 🤦 Will do that 
shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D65915: [clang-doc] Protect Index with mutex during reducing and generation stage

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran created this revision.
DiegoAstiazaran added a reviewer: juliehockett.
DiegoAstiazaran added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, ilya-biryukov.

Idx in ClangDocContext instance was being modified by multiple threads causing 
a seg fault.
A mutex is added to avoid this.


https://reviews.llvm.org/D65915

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -36,6 +36,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Mutex.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
@@ -246,6 +247,7 @@
   llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
   std::atomic Error;
   Error = false;
+  llvm::sys::Mutex IndexMutex;
   // ExecutorConcurrency is a flag exposed by AllTUsExecution.h
   llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
  : ExecutorConcurrency);
@@ -289,8 +291,10 @@
 return;
   }
 
+  IndexMutex.lock();
   // Add a reference to this Info in the Index
   clang::doc::Generator::addInfoToIndex(CDCtx.Idx, I);
+  IndexMutex.unlock();
 
   if (auto Err = G->get()->generateDocForInfo(I, InfoOS, CDCtx))
 llvm::errs() << toString(std::move(Err)) << "\n";


Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -36,6 +36,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Mutex.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
@@ -246,6 +247,7 @@
   llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
   std::atomic Error;
   Error = false;
+  llvm::sys::Mutex IndexMutex;
   // ExecutorConcurrency is a flag exposed by AllTUsExecution.h
   llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
  : ExecutorConcurrency);
@@ -289,8 +291,10 @@
 return;
   }
 
+  IndexMutex.lock();
   // Add a reference to this Info in the Index
   clang::doc::Generator::addInfoToIndex(CDCtx.Idx, I);
+  IndexMutex.unlock();
 
   if (auto Err = G->get()->generateDocForInfo(I, InfoOS, CDCtx))
 llvm::errs() << toString(std::move(Err)) << "\n";
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Andy Kaylor via Phabricator via cfe-commits]

andrew.w.kaylor added a comment.

I'm not entirely caught up on this review. I've only read the most recent 
comments, but I think I've got enough context to comment on the metadata 
arguments.

Based only on the fp-model command line options, the front end should only ever 
use "round.dynamic" (for strict mode) or "round.tonearest" (where full fenv 
access isn't permitted but we want to enable strict exception semantics). There 
are some pragmas, which I believe are in some draft of a standards document but 
not yet approved, which can declare any of the other rounding modes, but I 
don't know that we have plans to implement those yet. I'm also hoping that at 
some point we'll have a pass that finds calls to fesetround() and changes the 
metadata argument when it can prove what the rounding mode will be.

For the fp exception argument, the only values that can be implied by the 
-fp-model option are "fpexcept.strict" and "fpexcept.ignore". In icc, we have a 
separate option that can prevent speculation (equivalent to 
"fpexcept.maytrap"). I think gcc's, -ftrapping-math has a similar function 
(though the default may be reversed). I don't think we've talked about how (or 
if) clang should ever get into the "fpexcept.maytrap" state.

So for now, I think both arguments only need to support one of two states, 
depending on the -fp-model arguments.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D65912: Add new check for math constants

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

May be this check belongs to readability module?




Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:13
+#include "../../../clang/unittests/AST/Language.h"
+
+#include 

Unnecessary empty line.



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:27
+
+constexpr char StdMathHeader[] = "math";
+constexpr char StdCmathHeader[] = "cmath";

Functions and constants should be static. See LLVM Coding Guidelines.



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:70
+void MathConstantsCheck::registerMatchers(MatchFinder *Finder) {
+  // FIXME: Add matchers.
+  Finder->addMatcher(floatLiteral().bind(FloatType), this);

Is it needed?



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:84
+  const auto *MatchedDecl = Result.Nodes.getNodeAs(FloatType);
+  const auto MatchedFloatConstant = MatchedDecl->getValue().convertToDouble();
+

Please don't use auto when type is not spelled in same statement or in case of 
iterators. Same in other places.



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp:85
+  const auto MatchedFloatConstant = MatchedDecl->getValue().convertToDouble();
+
+  const auto Language = getLangOpts();

Unnecessary empty line.



Comment at: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h:13
+#include "../ClangTidyCheck.h"
+
+#include "../utils/IncludeInserter.h"

Please remove empty lines between headers.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:91
+
+  FIXME: add release notes.
+

Please add one sentence description. Should be same as first sentence of 
documentation.



Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst:6
+
+FIXME: Describe what patterns does the check detect and why. Give examples.

Please add documentation.


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

https://reviews.llvm.org/D65912



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

[PATCH] D65912: Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK updated this revision to Diff 214043.

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

https://reviews.llvm.org/D65912

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
  clang-tools-extra/test/clang-tidy/misc-math-constants.cpp

Index: clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s misc-math-constants %t
+
+// FIXME: Add something that triggers the check here.
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [misc-math-constants]
+
+// FIXME: Verify the applied fix.
+//   * Make the CHECK patterns specific enough and try to make verified lines
+// unique to avoid incorrect matches.
+//   * Use {{}} for regular expressions.
+// CHECK-FIXES: {{^}}void awesome_f();{{$}}
+
+// FIXME: Add something that doesn't trigger the check here.
+void foo()
+{
+double pi = 3.14;
+double p_2 = 1.57;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - misc-math-constants
+
+misc-math-constants
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -276,6 +276,7 @@
llvm-prefer-isa-or-dyn-cast-in-conditionals
llvm-twine-local
misc-definitions-in-headers
+   misc-math-constants
misc-misplaced-const
misc-new-delete-overloads
misc-non-copyable-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -85,6 +85,11 @@
 
 The improvements are...
 
+- New :doc:`misc-math-constants
+  ` check.
+
+  FIXME: add release notes.
+
 Improvements to clang-include-fixer
 ---
 
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
+#include "MathConstantsCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NonCopyableObjects.h"
@@ -32,6 +33,8 @@
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
 CheckFactories.registerCheck(
 "misc-definitions-in-headers");
+CheckFactories.registerCheck(
+"misc-math-constants");
 CheckFactories.registerCheck("misc-misplaced-const");
 CheckFactories.registerCheck(
 "misc-new-delete-overloads");
Index: clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
@@ -0,0 +1,59 @@
+//===--- MathConstantsCheck.h - clang-tidy --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MATHCONSTANTSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MATHCONSTANTSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+#include "../utils/IncludeInserter.h"
+
+#include "llvm/ADT/Optional.h"
+
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-math-constants.html
+class MathConstantsCheck : public ClangTidyCheck {
+public:
+  MathConstantsCheck(StringRef Name, ClangTidyContext *Context);
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void registerPPCallbacks(const

[PATCH] D65913: Fix segfault caused by the "libstdc++ eager exception spec hack"

[Brad Moody via Phabricator via cfe-commits]

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

Segfault occurs when the hack is applied to a function decl containing
an unparsed default argument. Parser::HandleMemberFunctionDeclDelays
was assuming that if a decl contains an unparsed default arg then it
must also have an unparsed exception spec. This causes a
read-from-wrong-union-member on FunctionTypeInfo::ExceptionSpecTokens,
triggering a segfault later on when the pointer is used.


Repository:
  rC Clang

https://reviews.llvm.org/D65913

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp


Index: clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
===
--- clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
+++ clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
@@ -9,6 +9,7 @@
 
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions 
-fcxx-exceptions -DCLASS=array
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions 
-fcxx-exceptions -DCLASS=array -DPR28423
+// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions 
-fcxx-exceptions -DCLASS=array -DDEFAULT_ARG
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions 
-fcxx-exceptions -DCLASS=pair
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions 
-fcxx-exceptions -DCLASS=priority_queue
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions 
-fcxx-exceptions -DCLASS=stack
@@ -48,7 +49,11 @@
 #endif
 A member;
 #ifndef MSVC
+#ifndef DEFAULT_ARG
 void swap(CLASS &other) noexcept(noexcept(swap(member, other.member)));
+#else
+void swap(CLASS &other, int = 0) noexcept(noexcept(swap(member, 
other.member)));
+#endif
 #endif
   };
 
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2180,8 +2180,10 @@
 LateMethod->TemplateScope = getCurScope()->isTemplateParamScope();
 
 // Stash the exception-specification tokens in the late-pased method.
-LateMethod->ExceptionSpecTokens = FTI.ExceptionSpecTokens;
-FTI.ExceptionSpecTokens = nullptr;
+if (FTI.getExceptionSpecType() == EST_Unparsed) {
+  LateMethod->ExceptionSpecTokens = FTI.ExceptionSpecTokens;
+  FTI.ExceptionSpecTokens = nullptr;
+}
 
 // Push tokens for each parameter.  Those that do not have
 // defaults will be NULL.


Index: clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
===
--- clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
+++ clang/test/SemaCXX/libstdcxx_pair_swap_hack.cpp
@@ -9,6 +9,7 @@
 
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array -DPR28423
+// RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=array -DDEFAULT_ARG
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=pair
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=priority_queue
 // RUN: %clang_cc1 -fsyntax-only %s -std=c++11 -verify -fexceptions -fcxx-exceptions -DCLASS=stack
@@ -48,7 +49,11 @@
 #endif
 A member;
 #ifndef MSVC
+#ifndef DEFAULT_ARG
 void swap(CLASS &other) noexcept(noexcept(swap(member, other.member)));
+#else
+void swap(CLASS &other, int = 0) noexcept(noexcept(swap(member, other.member)));
+#endif
 #endif
   };
 
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2180,8 +2180,10 @@
 LateMethod->TemplateScope = getCurScope()->isTemplateParamScope();
 
 // Stash the exception-specification tokens in the late-pased method.
-LateMethod->ExceptionSpecTokens = FTI.ExceptionSpecTokens;
-FTI.ExceptionSpecTokens = nullptr;
+if (FTI.getExceptionSpecType() == EST_Unparsed) {
+  LateMethod->ExceptionSpecTokens = FTI.ExceptionSpecTokens;
+  FTI.ExceptionSpecTokens = nullptr;
+}
 
 // Push tokens for each parameter.  Those that do not have
 // defaults will be NULL.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 214041.
Charusso added a comment.

- May it is better to also check for `getAsCXXRecordDecl()` for obtaining a 
class.


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

https://reviews.llvm.org/D65889

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value.cpp

Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ clang/test/Analysis/cast-value.cpp
@@ -20,14 +20,26 @@
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+
+template 
+const X *dyn_cast_or_null(const Y &Value);
 } // namespace llvm
 
-using namespace llvm;
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
 
-class Shape {};
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +103,44 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalNonNullParamNonNullReturnReference(const Shape &S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +153,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +195,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape &S) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape &S) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized to a null pointer value}}
+
+  (void)(1 / (bool)C);
+  // expected-note@-1 {{Division by zero}}
+  // expected-warning@-2 {{Division by zero}}
+  // logic-warning@-3 {{Division by zero}}
+}
+} // namespace test_notes
Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -16,168 +16,237 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/Optional.h"
+#include 
 
 using n

[PATCH] D65912: Add new check for math constants

[Alexander Zaitsev via Phabricator via cfe-commits]

ZaMaZaN4iK created this revision.
ZaMaZaN4iK added reviewers: JonasToth, Szelethus, aaron.ballman, lebedev.ri.
ZaMaZaN4iK added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

Hello.

I found quite interesting and useful FMPOV check for Clang Tidy - description 
, examples 
. Whole sense is in using standard 
(from cmath or math.h or math headers) math constants instead of written from 
scratch.

This patch is in status WIP. The main question is - do we want to see such 
check in Clang Tidy and should I continue work on it?

Thank you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D65912

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.cpp
  clang-tools-extra/clang-tidy/misc/MathConstantsCheck.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
  clang-tools-extra/test/clang-tidy/misc-math-constants.cpp

Index: clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-math-constants.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s misc-math-constants %t
+
+// FIXME: Add something that triggers the check here.
+void f();
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function 'f' is insufficiently awesome [misc-math-constants]
+
+// FIXME: Verify the applied fix.
+//   * Make the CHECK patterns specific enough and try to make verified lines
+// unique to avoid incorrect matches.
+//   * Use {{}} for regular expressions.
+// CHECK-FIXES: {{^}}void awesome_f();{{$}}
+
+// FIXME: Add something that doesn't trigger the check here.
+void foo()
+{
+double pi = 3.14;
+double p_2 = 1.57;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-math-constants.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - misc-math-constants
+
+misc-math-constants
+===
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -4,7 +4,6 @@
 =
 
 .. toctree::
-
abseil-duration-addition
abseil-duration-comparison
abseil-duration-conversion-cast
@@ -103,87 +102,87 @@
cert-msc51-cpp
cert-oop11-cpp (redirects to performance-move-constructor-init) 
cert-oop54-cpp (redirects to bugprone-unhandled-self-assignment) 
-   clang-analyzer-core.CallAndMessage
-   clang-analyzer-core.DivideZero
+   clang-analyzer-core.CallAndMessage (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.DivideZero (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.DynamicTypePropagation
-   clang-analyzer-core.NonNullParamChecker
-   clang-analyzer-core.NullDereference
-   clang-analyzer-core.StackAddressEscape
-   clang-analyzer-core.UndefinedBinaryOperatorResult
-   clang-analyzer-core.VLASize
-   clang-analyzer-core.uninitialized.ArraySubscript
-   clang-analyzer-core.uninitialized.Assign
-   clang-analyzer-core.uninitialized.Branch
+   clang-analyzer-core.NonNullParamChecker (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.NullDereference (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.StackAddressEscape (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.UndefinedBinaryOperatorResult (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.VLASize (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.ArraySubscript (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.Assign (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
+   clang-analyzer-core.uninitialized.Branch (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-core.uninitialized.CapturedBlockVariable
-   clang-analyzer-core.uninitialized.UndefReturn
+   clang-analyzer-core.uninitialized.UndefReturn (redirects to https://clang.llvm.org/docs/analyzer/checkers) 
clang-analyzer-cplusplus.InnerPointer
-   clang-analyzer-cplusplus.Move
-   clang-analyzer-cplusplus.NewDelete
-   clang-analyzer-cplusplus.NewDeleteLeaks
-   clang-analyzer-deadcode.DeadStores
-   clang-analyzer-nullability.NullPassedToNonnull
-

[PATCH] D65212: [analyzer] Fix exporting SARIF files from scan-build on Windows

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/tools/scan-build/libexec/ccc-analyzer:121
 END {
-  if (defined $ResultFile && -z $ResultFile) {
 unlink($ResultFile);

Whoops. I suspect that we're now unconditionally deleting all plist output, 
which isn't exactly what we wanted. I think we actually wanted to delete 
//empty// plist files here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65212



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 214038.
Charusso added a comment.

- Fix a comment.


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

https://reviews.llvm.org/D65889

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value.cpp

Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ clang/test/Analysis/cast-value.cpp
@@ -20,14 +20,26 @@
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+
+template 
+const X *dyn_cast_or_null(const Y &Value);
 } // namespace llvm
 
-using namespace llvm;
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
 
-class Shape {};
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +103,44 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalNonNullParamNonNullReturnReference(const Shape &S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +153,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +195,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape &S) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape &S) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized to a null pointer value}}
+
+  (void)(1 / (bool)C);
+  // expected-note@-1 {{Division by zero}}
+  // expected-warning@-2 {{Division by zero}}
+  // logic-warning@-3 {{Division by zero}}
+}
+} // namespace test_notes
Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -16,168 +16,235 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "llvm/ADT/Optional.h"
+#include 
 
 using namespace clang;
 using namespace ento;
 
 namespace {
 class CastVa

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51
+
+  const CallDescriptionMap SugarCastCDM = {
+  {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs},

NoQ wrote:
> I'd rather have only one map from call descriptions to (callback, kind) 
> pairs. This should make the `evalCall` code much more readable.
The problem with that I really want to use something like that:
```
auto &&[Check, Kind, MoreData] = *Lookup;
```
but I cannot. Until that time it equally non-readable and difficult to scale 
sadly. For now it is fine, but that C++17 feature would be more awesome.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:231
+// If we cannot obtain 'MCE' we cannot be sure how to model it.
+const auto *MCE = dyn_cast(CE->IgnoreParenImpCasts());
+if (!MCE)

NoQ wrote:
> ```lang=c++
> const auto *InstanceCall = dyn_cast(&Call);
> ...
> DV = InstanceCall->getCXXThisVal();
> ```
Hm, not bad. Thanks!



Comment at: clang/test/Analysis/cast-value.cpp:186
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Assuming dynamic cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}

NoQ wrote:
> To think: this note isn't very useful because hard casts aren't really 
> supposed to fail anyway.
I believe it is better than the generic `Assuming...` piece.


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

https://reviews.llvm.org/D65889



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Csaba Dabis via Phabricator via cfe-commits]

Charusso updated this revision to Diff 214037.
Charusso marked 9 inline comments as done.
Charusso added a comment.

- Make it usable with references.
- Test references.
- Better messages on simple `cast<>`.


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

https://reviews.llvm.org/D65889

Files:
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/test/Analysis/cast-value.cpp

Index: clang/test/Analysis/cast-value.cpp
===
--- clang/test/Analysis/cast-value.cpp
+++ clang/test/Analysis/cast-value.cpp
@@ -20,14 +20,26 @@
 const X *cast_or_null(Y Value);
 
 template 
-const X *dyn_cast_or_null(Y Value);
+const X *dyn_cast_or_null(Y *Value);
+
+template 
+const X *dyn_cast_or_null(const Y &Value);
 } // namespace llvm
 
-using namespace llvm;
+namespace clang {
+struct Shape {
+  template 
+  const T *castAs() const;
 
-class Shape {};
+  template 
+  const T *getAs() const;
+};
 class Triangle : public Shape {};
 class Circle : public Shape {};
+} // namespace clang
+
+using namespace llvm;
+using namespace clang;
 
 namespace test_cast {
 void evalLogic(const Shape *S) {
@@ -91,8 +103,44 @@
   if (!S)
 clang_analyzer_eval(!C); // logic-warning {{TRUE}}
 }
+} // namespace test_dyn_cast_or_null
 
-void evalNonNullParamNonNullReturn(const Shape *S) {
+namespace test_cast_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->castAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{1}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // no-warning
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_cast_as
+
+namespace test_get_as {
+void evalLogic(const Shape *S) {
+  const Circle *C = S->getAs();
+  clang_analyzer_numTimesReached(); // logic-warning {{2}}
+
+  if (S && C)
+clang_analyzer_eval(C == S);
+  // logic-warning@-1 {{TRUE}}
+
+  if (S && !C)
+clang_analyzer_warnIfReached(); // logic-warning {{REACHABLE}}
+
+  if (!S)
+clang_analyzer_warnIfReached(); // no-warning
+}
+} // namespace test_get_as
+
+namespace test_notes {
+void evalNonNullParamNonNullReturnReference(const Shape &S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' succeeds}}
   // expected-note@-2 {{Assuming pointer value is null}}
@@ -105,6 +153,19 @@
   // logic-warning@-4 {{Division by zero}}
 }
 
+void evalNonNullParamNonNullReturn(const Shape *S) {
+  const auto *C = cast(S);
+  // expected-note@-1 {{Dynamic cast from 'Shape' to 'Circle' succeeds}}
+  // expected-note@-2 {{Assuming pointer value is null}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
 void evalNonNullParamNullReturn(const Shape *S) {
   const auto *C = dyn_cast_or_null(S);
   // expected-note@-1 {{Assuming dynamic cast from 'Shape' to 'Circle' fails}}
@@ -134,4 +195,40 @@
   // expected-warning@-2 {{Division by zero}}
   // logic-warning@-3 {{Division by zero}}
 }
-} // namespace test_dyn_cast_or_null
+
+void evalZeroParamNonNullReturnPointer(const Shape *S) {
+  const auto *C = S->castAs();
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNonNullReturn(const Shape &S) {
+  const auto *C = S.castAs();
+  // expected-note@-1 {{Dynamic cast to 'Circle' succeeds}}
+  // expected-note@-2 {{'C' initialized here}}
+
+  (void)(1 / !(bool)C);
+  // expected-note@-1 {{'C' is non-null}}
+  // expected-note@-2 {{Division by zero}}
+  // expected-warning@-3 {{Division by zero}}
+  // logic-warning@-4 {{Division by zero}}
+}
+
+void evalZeroParamNullReturn(const Shape &S) {
+  const auto *C = S.getAs();
+  // expected-note@-1 {{Assuming dynamic cast to 'Circle' fails}}
+  // expected-note@-2 {{'C' initialized to a null pointer value}}
+
+  (void)(1 / (bool)C);
+  // expected-note@-1 {{Division by zero}}
+  // expected-warning@-2 {{Division by zero}}
+  // logic-warning@-3 {{Division by zero}}
+}
+} // namespace test_notes
Index: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -16,168 +16,235 @@
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #i

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

I spent some time becoming familiar with how `isDerivedFrom` behaves for C++ 
classes. I //think// that I have managed to get the behavior for Objective-C 
classes to mirror that of C++ classes. Please let me know if I overlooked 
anything.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2642-2649
+  if (const auto *InterfaceDecl = dyn_cast(&Node)) {
+// Check if any of the superclasses of the class match.
+for (const ObjCInterfaceDecl *SuperClass = InterfaceDecl->getSuperClass();
+ SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {
+  if (Base.matches(*SuperClass, Finder, Builder))
+return true;
+}

stephanemoore wrote:
> aaron.ballman wrote:
> > This should probably be done similar to how `classIsDerivedFrom()` works. 
> > For instance, there's some type alias matching logic that this does not 
> > replicate, but we probably want.
> Upon first glance I had determined that the type aliasing logic in 
> `classIsDerivedFrom()` was particular to C++. On second consideration, we 
> will probably want to make sure that compatibility aliases are handled 
> correctly for Objective-C. I will take a look into making sure that works as 
> expected.
Done.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2667-2672
+  if (const auto *InterfaceDecl = dyn_cast(&Node)) {
+return Matcher(M).matches(*InterfaceDecl, Finder,
+ Builder);
+  }
+
+  llvm_unreachable("Not a valid polymorphic type");

aaron.ballman wrote:
> How about:
> ```
> const auto *InterfaceDecl = cast(&Node);
> return Matcher...
> ```
> We can rely on `cast<>` to assert if the node is of an unexpected type. Same 
> suggestions below.
Good call. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

r368237 - Update fix-it hints for std::move warnings.

[Richard Trieu via cfe-commits]

Author: rtrieu
Date: Wed Aug  7 17:12:51 2019
New Revision: 368237

URL: http://llvm.org/viewvc/llvm-project?rev=368237&view=rev
Log:
Update fix-it hints for std::move warnings.

Fix -Wpessimizing-move and -Wredundant-move when warning on initializer lists.
The new fix-it hints for removing the std::move call will now also suggest
removing the braces for the initializer list so that the resulting code will
still be compilable.

This fixes PR42832

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp
cfe/trunk/test/SemaCXX/warn-redundant-move.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368237&r1=368236&r2=368237&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Aug  7 17:12:51 2019
@@ -7305,27 +7305,34 @@ static void CheckMoveOnConstruction(Sema
   if (!DestType->isRecordType())
 return;
 
-  unsigned DiagID = 0;
-  if (IsReturnStmt) {
-const CXXConstructExpr *CCE =
-dyn_cast(InitExpr->IgnoreParens());
-if (!CCE || CCE->getNumArgs() != 1)
-  return;
+  const CXXConstructExpr *CCE =
+  dyn_cast(InitExpr->IgnoreParens());
+  if (!CCE || CCE->getNumArgs() != 1)
+return;
 
-if (!CCE->getConstructor()->isCopyOrMoveConstructor())
-  return;
+  if (!CCE->getConstructor()->isCopyOrMoveConstructor())
+return;
 
-InitExpr = CCE->getArg(0)->IgnoreImpCasts();
-  }
+  InitExpr = CCE->getArg(0)->IgnoreImpCasts();
 
   // Find the std::move call and get the argument.
   const CallExpr *CE = dyn_cast(InitExpr->IgnoreParens());
   if (!CE || !CE->isCallToStdMove())
 return;
 
-  const Expr *Arg = CE->getArg(0)->IgnoreImplicit();
+  const Expr *Arg = CE->getArg(0);
+
+  unsigned DiagID = 0;
+
+  if (!IsReturnStmt && !isa(Arg))
+return;
 
-  if (IsReturnStmt) {
+  if (isa(Arg)) {
+DiagID = diag::warn_pessimizing_move_on_initialization;
+const Expr *ArgStripped = Arg->IgnoreImplicit()->IgnoreParens();
+if (!ArgStripped->isRValue() || !ArgStripped->getType()->isRecordType())
+  return;
+  } else { // IsReturnStmt
 const DeclRefExpr *DRE = dyn_cast(Arg->IgnoreParenImpCasts());
 if (!DRE || DRE->refersToEnclosingVariableOrCapture())
   return;
@@ -7352,24 +7359,18 @@ static void CheckMoveOnConstruction(Sema
   DiagID = diag::warn_redundant_move_on_return;
 else
   DiagID = diag::warn_pessimizing_move_on_return;
-  } else {
-DiagID = diag::warn_pessimizing_move_on_initialization;
-const Expr *ArgStripped = Arg->IgnoreImplicit()->IgnoreParens();
-if (!ArgStripped->isRValue() || !ArgStripped->getType()->isRecordType())
-  return;
   }
 
   S.Diag(CE->getBeginLoc(), DiagID);
 
   // Get all the locations for a fix-it.  Don't emit the fix-it if any location
   // is within a macro.
-  SourceLocation CallBegin = CE->getCallee()->getBeginLoc();
-  if (CallBegin.isMacroID())
+  SourceLocation BeginLoc = CCE->getBeginLoc();
+  if (BeginLoc.isMacroID())
 return;
   SourceLocation RParen = CE->getRParenLoc();
   if (RParen.isMacroID())
 return;
-  SourceLocation LParen;
   SourceLocation ArgLoc = Arg->getBeginLoc();
 
   // Special testing for the argument location.  Since the fix-it needs the
@@ -7380,14 +7381,16 @@ static void CheckMoveOnConstruction(Sema
 ArgLoc = 
S.getSourceManager().getImmediateExpansionRange(ArgLoc).getBegin();
   }
 
+  SourceLocation LParen = ArgLoc.getLocWithOffset(-1);
   if (LParen.isMacroID())
 return;
-
-  LParen = ArgLoc.getLocWithOffset(-1);
+  SourceLocation EndLoc = CCE->getEndLoc();
+  if (EndLoc.isMacroID())
+return;
 
   S.Diag(CE->getBeginLoc(), diag::note_remove_move)
-  << FixItHint::CreateRemoval(SourceRange(CallBegin, LParen))
-  << FixItHint::CreateRemoval(SourceRange(RParen, RParen));
+  << FixItHint::CreateRemoval(SourceRange(BeginLoc, LParen))
+  << FixItHint::CreateRemoval(SourceRange(RParen, EndLoc));
 }
 
 static void CheckForNullPointerDereference(Sema &S, const Expr *E) {

Modified: cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp?rev=368237&r1=368236&r2=368237&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-pessmizing-move.cpp Wed Aug  7 17:12:51 2019
@@ -122,13 +122,13 @@ A test7() {
   A a3 = (std::move(A()));
   // expected-warning@-1{{prevents copy elision}}
   // expected-note@-2{{remove std::move call}}
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:11-[[@LINE-3]]:21}:""
-  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:25}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:10-[[@LINE-3]]:21}:""
+  // CHECK: fix-it:"{{.*}}":{[[@LINE-4]]:24-[[@LINE-4]]:26}:""
   A a4 = (

[PATCH] D60543: [clang] Update isDerivedFrom to support Objective-C classes 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 214035.
stephanemoore marked 6 inline comments as done.
stephanemoore edited the summary of this revision.
stephanemoore added a comment.

Update `isDerivedFrom` to match aliased types and compatibility aliases of
derived Objective-C classes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/ASTMatchers/ASTMatchersInternal.h
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -573,6 +573,64 @@
   EXPECT_TRUE(notMatches(Code, cxxRecordDecl(isSameOrDerivedFrom("";
 }
 
+TEST(DeclarationMatcher, ObjCClassIsDerived) {
+  DeclarationMatcher IsDerivedFromX = objcInterfaceDecl(isDerivedFrom("X"));
+  EXPECT_TRUE(
+  matchesObjC("@interface X @end @interface Y : X @end", IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y<__covariant ObjectType> : X @end",
+  IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  IsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class X;", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end @compatibility_alias Y X;",
+ IsDerivedFromX));
+  EXPECT_TRUE(notMatchesObjC("@interface X @end typedef X Y;", IsDerivedFromX));
+
+  DeclarationMatcher IsAX = objcInterfaceDecl(isSameOrDerivedFrom("X"));
+  EXPECT_TRUE(matchesObjC("@interface X @end @interface Y : X @end", IsAX));
+  EXPECT_TRUE(matchesObjC("@interface X @end", IsAX));
+  EXPECT_TRUE(matchesObjC("@class X;", IsAX));
+  EXPECT_TRUE(notMatchesObjC("@interface Y @end", IsAX));
+  EXPECT_TRUE(notMatchesObjC("@class Y;", IsAX));
+
+  DeclarationMatcher ZIsDerivedFromX =
+  objcInterfaceDecl(hasName("Z"), isDerivedFrom("X"));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @interface Y : X @end @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC("@interface X @end @interface Y : X @end typedef Y "
+  "V; typedef V W; @interface Z : W @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end typedef X Y; @interface Z : Y @end", ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end typedef A X; typedef A Y; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface X @end @compatibility_alias Y X; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface Y @end @compatibility_alias X Y; @interface Z : Y @end",
+  ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end @compatibility_alias X A; @compatibility_alias Y A;"
+  "@interface Z : Y @end", ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface Y @end typedef Y X; @interface Z : X @end", ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end @compatibility_alias Y A; typedef Y X;"
+  "@interface Z : A @end", ZIsDerivedFromX));
+  EXPECT_TRUE(matchesObjC(
+  "@interface A @end typedef A Y; @compatibility_alias X Y;"
+  "@interface Z : A @end", ZIsDerivedFromX));
+}
+
 TEST(DeclarationMatcher, IsLambda) {
   const auto IsLambda = cxxMethodDecl(ofClass(cxxRecordDecl(isLambda(;
   EXPECT_TRUE(matches("auto x = []{};", IsLambda));
Index: clang/lib/ASTMatchers/ASTMatchFinder.cpp
===
--- clang/lib/ASTMatchers/ASTMatchFinder.cpp
+++ clang/lib/ASTMatchers/ASTMatchFinder.cpp
@@ -374,6 +374,12 @@
 return true;
   }
 
+  bool VisitObjCCompatibleAliasDecl(ObjCCompatibleAliasDecl *CAD) {
+const ObjCInterfaceDecl *InterfaceDecl = CAD->getClassInterface();
+CompatibleAliases[InterfaceDecl].insert(CAD);
+return true;
+  }
+
   bool TraverseDecl(Decl *DeclNode);
   bool TraverseStmt(Stmt *StmtNode, DataRecursionQueue *Queue = nullptr);
   bool TraverseType(QualType TypeNode);
@@ -433,6 +439,11 @@
   BoundNodesTreeBuilder *Builder,
   bool Directly) override;
 
+  bool objcClassIsDerivedFrom(const ObjCInterfaceDecl *Declaration,
+  const Matcher &Base,
+  BoundNodesTreeBuilder *Builder,
+   

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Hal Finkel via Phabricator via cfe-commits]

hfinkel added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > ABataev wrote:
> > > > > > > jdoerfert wrote:
> > > > > > > > I think this should cause an error or at least a warning. 
> > > > > > > > Telling the compiler `ps` is a device pointer only to create a 
> > > > > > > > local uninitialized shadowing variable seems like an error to 
> > > > > > > > me.
> > > > > > > It is allowed according to OpenMP 5.0. Private copy must be 
> > > > > > > created in the context of the parallel region, not the target 
> > > > > > > region. So, for OpenMP 5.0 we should not emit error here.
> > > > > > What does that mean and how does that affect my reasoning?
> > > > > It means, that for OpenMP 5.0 we should emit a warning/error here. It 
> > > > > is allowed according to the standard, we should allow it too.
> > > > > So, for OpenMP 5.0 we should not emit error here.
> > > > > that for OpenMP 5.0 we should emit a warning/error here.
> > > > 
> > > > The last answer contradicts what you said earlier. I expect there is a 
> > > > *not* missing, correct?
> > > > 
> > > > 
> > > > Assuming you do not want an error, which is fine, I still think a 
> > > > warning is appropriate as it seems to me there is never a reason to 
> > > > have a `is_device_ptr` clause for a private value. I mean, it is either 
> > > > a bug by the programmer, e.g., 5 letters of `firstprivate` went 
> > > > missing, or simply nonsensical code for which we warn in other 
> > > > situations as well.
> > > Missed `not`.
> > > These kind of construct are explicitly allowed in OpenMP. And we should 
> > > obey the standard unconditionally.
> > > Plus, there might be situations where user may require it explicitly. For 
> > > example, the device pointer is dereferenced in one of the clauses for the 
> > > subregions but in the deeper subregion it might be used as a private 
> > > pointer. Why we should emit a warning here?
> > If you have a different situation, e.g., the one you describe, you should 
> > not have a warning. Though, that is not the point. If you have the 
> > situation above (single directive), as per my reasoning, there doesn't seem 
> > to be a sensible use case. If you have one and we should create an explicit 
> > test for it.
> > 
> > > These kind of construct are explicitly allowed in OpenMP.
> > `explicitly allowed` != `not forbidded` (yet)
> > > And we should obey the standard unconditionally.
> > Nobody says we should not. We warn people all the time even if it is valid 
> > code.
> Warnings may prevent successful compilation in some cases, e.g. when warnings 
> are treated as errors. Why we should emit a warning if the construct is 
> allowed by the standard? Ask to change the standard if you did not agree with 
> it.
Warnings are specifically for constructs which are legal, but likely wrong 
(i.e., will not do what the user likely intended). Treating warnings as errors 
is not a conforming compilation mode - by design (specifically because that 
will reject conforming programs). Thus...

> Why we should emit a warning if the construct is allowed by the standard? Ask 
> to change the standard if you did not agree with it.

This is the wrong way to approach this. Warnings are specifically for legal 
code. They help users prevent errors, however, in cases where that legal code 
is likely problematic or won't do what the user intends.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D59963: [clang-tidy] Add a module for the Linux kernel.

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp:12
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
+

tmroeder wrote:
> tmroeder wrote:
> > nickdesaulniers wrote:
> > > tmroeder wrote:
> > > > lebedev.ri wrote:
> > > > > This looks wrong
> > > > Yeah, I'm not sure where that came from. I'll remove it.
> > > Hard to tell, but I don't think this `using` statement was ever removed 
> > > as requested?
> > It's not the "using" statement that was requested to be removed in the 
> > initial diff, but rather "#include ", which I did 
> > remove.
> > 
> > I'm not sure why the "This looks wrong" gets attached to the "using" 
> > statement in later diffs.
> Nick, can you confirm that this addresses your comment?
LGTM


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59963



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

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

Thanks for figuring out these issues! It looks like you're fixing several edge 
cases, would you mind splitting the patch up for easier review?

Regarding the invisible characters before `#ifdef`, are you sure that's the 
right behavior? If you run the preprocessor over your test case, it doesn't 
accept it as a valid input, so why should the minimizer be different:

  $ clang -cc1 -DTEST  %t.c -Eonly -o -
  %t.c:3:2: error: #endif without #if
  #endif
   ^
  1 error generated.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65906



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

[PATCH] D65287: [analyzer][CFG] Don't track the condition of asserts

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:1753-1755
+  // It so happens that CFGBlock::getTerminatorCondition returns 'A' for block
+  // B1, 'A && B' for B2, and 'A && B || C' for B3. Let's check whether we
+  // reached the end of the condition!

Szelethus wrote:
> NoQ wrote:
> > Clever trick, but why not match for logical operators directly? Something 
> > like this:
> > ```lang=c++
> > if (auto B = dyn_cast(OuterCond))
> >   if (B->isLogicalOp())
> > return isAssertlikeBlock(Else, Context);
> > ```
> What about `assert(a + b && "Shouldn't be zero!");`?
Mmm, could you elaborate? >.<



Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:469-471
+extern void __assert_fail (__const char *__assertion, __const char *__file,
+   unsigned int __line, __const char *__function)
+__attribute__ ((__noreturn__));

Szelethus wrote:
> NoQ wrote:
> > I'm pretty sure you can define this function only once.
> Note that it is in a namespace!
I mean, why is it in a namespace? Why not just define it once for the whole 
file? It's not like you're gonna be trying out different variants of 
`__assert_fail`.


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

https://reviews.llvm.org/D65287



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

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 214015.
arphaman added a comment.

Fix the FIXME.
The patch is now ready for review.


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

https://reviews.llvm.org/D65907

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Lex/DirectoryLookup.h
  clang/include/clang/Lex/HeaderMap.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/lib/Lex/HeaderMap.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Tooling/ToolingTest.cpp - Tooling unit tests --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+
+namespace {
+
+/// Prints out all of the gathered dependencies into a string.
+class TestFileCollector : public DependencyFileGenerator {
+public:
+  TestFileCollector(DependencyOutputOptions &Opts,
+std::vector &Deps)
+  : DependencyFileGenerator(Opts), Deps(Deps) {}
+
+  void finishedMainFile(DiagnosticsEngine &Diags) override {
+Deps = getDependencies();
+  }
+
+private:
+  std::vector &Deps;
+};
+
+class TestDependencyScanningAction : public tooling::ToolAction {
+public:
+  TestDependencyScanningAction(std::vector &Deps) : Deps(Deps) {}
+
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *FileMgr,
+ std::shared_ptr PCHContainerOps,
+ DiagnosticConsumer *DiagConsumer) override {
+CompilerInstance Compiler(std::move(PCHContainerOps));
+Compiler.setInvocation(std::move(Invocation));
+Compiler.setFileManager(FileMgr);
+
+Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+if (!Compiler.hasDiagnostics())
+  return false;
+
+Compiler.createSourceManager(*FileMgr);
+Compiler.addDependencyCollector(std::make_shared(
+Compiler.getInvocation().getDependencyOutputOpts(), Deps));
+
+auto Action = llvm::make_unique();
+return Compiler.ExecuteAction(*Action);
+  }
+
+private:
+  std::vector &Deps;
+};
+
+} // namespace
+
+TEST(DependencyScanner, ScanDepsReuseFilemanager) {
+  std::vector Compilation = {"-c", "-E", "-MT", "test.cpp.o"};
+  StringRef CWD = "/root";
+  FixedCompilationDatabase CDB(CWD, Compilation);
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  VFS->addFile("/root/header.h", 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addHardLink("/root/symlink.h", "/root/header.h");
+  VFS->addFile("/root/test.cpp", 0,
+   llvm::MemoryBuffer::getMemBuffer(
+   "#include \"symlink.h\"\n#include \"header.h\"\n"));
+
+  ClangTool Tool(CDB, {"test.cpp"}, std::make_shared(),
+ VFS);
+  Tool.clearArgumentsAdjusters();
+  std::vector Deps;
+  TestDependencyScanningAction Action(Deps);
+  Tool.run(&Action);
+  // The first invocation should return dependencies in order of access.
+  ASSERT_EQ(Deps.size(), 3u);
+  EXPECT_EQ(Deps[0], "/root/test.cpp");
+  EXPECT_EQ(Deps[1], "/root/symlink.h");
+  EXPECT_EQ(Deps[2], "/root/header.h");
+
+  // The file manager should still have two FileEntries, as one file is a
+  // hardlink.
+  FileManager &Files = Tool.getFiles();
+  EXPECT_EQ(Files.getNumUniqueRealFiles(), 2u);
+
+  Deps.clear();
+  Tool.run(&Action);
+  // The second invoca

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

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

xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:192-194
+  if (const auto *DRE = dyn_cast(CondVarExpr))
+if (const auto *VD = dyn_cast(DRE->getDecl()))
+  return State->getSVal(State->getLValue(VD, LCtx));

NoQ wrote:
> Ugh, so this code intentionally avoids the complexity of 
> `ExprEngine::VisitCommonDeclRefExpr()` so that to work differently with 
> references. Okay.
> 
> @xazax.hun, do you think this works correctly with lambda captures? (when the 
> `DeclRefExpr` to a variable might evaluate to a field within the lambda at 
> run-time).
Good point! This definitely worth a test case somewhere somehow!

I would expect `DRE->getDecl()` to return the captured declaration rather than 
the field and thus `State->getSVal(State->getLValue(VD, LCtx))` to compute the 
wrong answer. I might be a bit rusty on this topic so of course we would need a 
carefully crafted test case to make sure :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65724



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

[PATCH] D65863: [ARM] Add support for the s,j,x,N,O inline asm constraints

[Saleem Abdulrasool via Phabricator via cfe-commits]

compnerd added inline comments.



Comment at: clang/lib/Basic/Targets/ARM.cpp:904
+  case 'j': // An immediate integer between 0 and 65535 (valid for MOVW)
+if (CPUAttr.equals("6T2") ||
+ArchVersion >= 7) // only available in ARMv6T2 and above

I would hoist the comment above the `if` as that results in a more readable 
condition.



Comment at: clang/lib/Basic/Targets/ARM.cpp:938
+// Thumb1: An immediate integer which is a multiple of 4 between 0 and 
1020.
+Info.setRequiresImmediate();
 return true;

Can we leave this as a FIXME?  This needs additional validation on the input.



Comment at: clang/lib/Basic/Targets/ARM.cpp:941
+  case 'N':
+if (isThumb() && !supportsThumb2()) // Thumb1 only
+{

Please hoist the comment above the `if`.



Comment at: clang/lib/Basic/Targets/ARM.cpp:948
+  case 'O':
+if (isThumb() && !supportsThumb2()) // Thumb1 only
+{

Similar


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65863



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

[PATCH] D65578: [analyzer][NFC] Make sure that the BugReport is not modified during the construction of non-visitor pieces

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:108
   SmallVector Ranges;
+  const SourceRange ErrorNodeRange;
   ExtraTextList ExtraText;

I guess you should add a comment for anybody who would want to remove this 
`const` on why it's a bad idea(?)


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

https://reviews.llvm.org/D65578



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

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[Alex Lorenz via Phabricator via cfe-commits]

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

I forgot I left a FIXME that I intended to fix in there, I need to resolve that 
first. I'll update the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65907



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

[PATCH] D65549: [Sema] Prevent -Wunused-lambda-capture from generating false positive warnings on templated member function

[Ziang Wan via Phabricator via cfe-commits]

ziangwan updated this revision to Diff 214013.
ziangwan added a comment.

Fix style issue.


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

https://reviews.llvm.org/D65549

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-unused-lambda-capture-this.cpp

Index: clang/test/SemaCXX/warn-unused-lambda-capture-this.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-unused-lambda-capture-this.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -fsyntax-only -Wunused-lambda-capture -verify %s
+
+template 
+class TemplatedStatic {
+public:
+  TemplatedStatic() {
+[this](auto value) { Construct(value); }(5); // expected-warning {{lambda capture 'this' is not used}}
+  }
+
+  template 
+  static void Construct(Arg value) {}
+};
+
+template 
+class TemplatedMember {
+public:
+  TemplatedMember() {
+[this](auto value) { Construct(value); }(5);
+  }
+
+  template 
+  void Construct(Arg value) {}
+};
+
+template 
+class OverloadedStatic {
+public:
+  OverloadedStatic() {
+[this](auto value) { Construct(value); }(5); // expected-warning {{lambda capture 'this' is not used}}
+  }
+
+  static void Construct(int value) {}
+  static void Construct(float value) {}
+};
+
+template 
+class OverloadedMember {
+public:
+  OverloadedMember() {
+[this](auto value) { Construct(value); }(5);
+  }
+
+  void Construct(int value) {}
+  void Construct(float value) {}
+};
+
+template 
+class OverloadedMixTrueNegative {
+public:
+  OverloadedMixTrueNegative() {
+[this](auto value) { Construct(value); }(5);
+  }
+
+  void Construct(int value) {}
+  static void Construct(float value) {}
+};
+
+template 
+class OverloadedMixFalseNegative {
+public:
+  OverloadedMixFalseNegative() {
+// Currently we mark this capture as used whenever UnresolvedMemberExpr
+// happens, even if "Construct" will be resolved to the static one.
+// Ideally we should issue 'this' not used warning.
+[this](auto value) { Construct(value); }(5);
+  }
+
+  static void Construct(int value) {}
+  void Construct(float value) {}
+};
+
+int main() {
+  TemplatedStatic t1; // expected-note {{in instantiation of member function 'TemplatedStatic::TemplatedStatic' requested here}}
+  TemplatedMember t2;
+  OverloadedStatic t3; // expected-note {{in instantiation of member function 'OverloadedStatic::OverloadedStatic' requested here}}
+  OverloadedMember t4;
+  OverloadedMixTrueNegative t5;
+  OverloadedMixFalseNegative t6;
+  return 0;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -5619,6 +5619,14 @@
 *this, dyn_cast(Fn->IgnoreParens()),
 Fn->getBeginLoc());
 
+// Mark this capture as ODR-used if we are in a lambda
+// expression and the callee is UnresolvedMemberExpr.
+LambdaScopeInfo *CurLSI = getCurLambda();
+if (CurLSI && isa(Fn->IgnoreParens()))
+  for (auto &Capture : CurLSI->Captures)
+if (Capture.isThisCapture())
+  Capture.markUsed(/*IsODRUse=*/true);
+
 return CallExpr::Create(Context, Fn, ArgExprs, Context.DependentTy,
 VK_RValue, RParenLoc);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64943: [Clang][OpenMP offload] Eliminate use of OpenMP linker script

[Sergey Dmitriev via Phabricator via cfe-commits]

sdmitriev added a comment.

As I understand ‘atexit’ solution would be target dependent (‘__cxa_atexit’ on 
Linux and ‘atexit’ on Windows) whereas @llvm.global_ctors/dtors variables offer 
similar and platform neutral functionality 
(http://llvm.org/docs/LangRef.html#the-llvm-global-ctors-global-variable). Why 
do you think that ‘atexit’ is a better choice?


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

https://reviews.llvm.org/D64943



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

[PATCH] D65723: [analyzer][NFC] Add different interestingness kinds

[Artem Dergachev via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:92-101
   /// Default tracking kind -- specifies that as much information should be
   /// gathered about the tracked expression value as possible.
-  Thorough,
+  Thorough = 0,
   /// Specifies that a more moderate tracking should be used for the expression
   /// value. This will essentially make sure that functions relevant to the it
   /// aren't pruned, but otherwise relies on the user reading the code or
   /// following the arrows.

We usually rely on our "unhandled switch case" warnings for noticing all the 
places which we need to expand when we're adding a new case. Your approach 
requires adding static asserts all over the place instead - not sure what's 
better. I guess they're not generally mutually exclusive, however 
`NumTrackingKinds` makes it impossible to handle all cases in a switch (because 
`NumTrackingKinds` itself wouldn't ever be handled). I took some sort of 
middleground approach in `CFGTerminator::Kind` in D61814 but i'm very much open 
to suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65723



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

[PATCH] D65725: [analyzer] Mention whether an event is about a condition in a bug report part 2

[Artem Dergachev via Phabricator via cfe-commits]

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

Accept as long as we agree on the wording as part of D65575 
.




Comment at: clang/test/Analysis/track-control-dependency-conditions.cpp:465
 void assert(int b) {
-  if (!b) // tracking-note{{Assuming 'b' is not equal to 0}}
+  if (!b) // tracking-note{{Assuming 'b' is not equal to 0, which will be (a 
part of a) condition}}
   // tracking-note@-1{{Taking false branch}}

Do i understand correctly that this test was passing previously, because 
`expected-note` matches substrings rather than the whole string? If we stick to 
the current wording, should we slowly switch to 
`expected-note-re^}}...{{$` in order to avoid such situations?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65725



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

[PATCH] D65724: [analyzer] Don't make ConditionBRVisitor events prunable when the condition is an interesting field

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Nice!!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:225-226
   // FIXME: constexpr initialization isn't supported by MSVC2013.
-  static const char *const GenericTrueMessage;
-  static const char *const GenericFalseMessage;
+  static llvm::StringLiteral GenericTrueMessage;
+  static llvm::StringLiteral GenericFalseMessage;
 

https://llvm.org/doxygen/classllvm_1_1StringLiteral.html:
> In order to avoid the invocation of a global constructor, `StringLiteral` 
> should only be used in a `constexpr` context



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:192-194
+  if (const auto *DRE = dyn_cast(CondVarExpr))
+if (const auto *VD = dyn_cast(DRE->getDecl()))
+  return State->getSVal(State->getLValue(VD, LCtx));

Ugh, so this code intentionally avoids the complexity of 
`ExprEngine::VisitCommonDeclRefExpr()` so that to work differently with 
references. Okay.

@xazax.hun, do you think this works correctly with lambda captures? (when the 
`DeclRefExpr` to a variable might evaluate to a field within the lambda at 
run-time).



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:208
+  if (Optional V = getSValForVar(CondVarExpr, N))
+if (auto DeclCI = V->getAs())
+  return &DeclCI->getValue();

`Decl`? I guess it made sense in the original code but there's no longer an 
obvious `Decl` floating around.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65724



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

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 214010.
arphaman added a comment.

Fix a bug with dereferencing `None` File value in one call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65907

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Lex/DirectoryLookup.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Tooling/ToolingTest.cpp - Tooling unit tests --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+
+namespace {
+
+/// Prints out all of the gathered dependencies into a string.
+class TestFileCollector : public DependencyFileGenerator {
+public:
+  TestFileCollector(DependencyOutputOptions &Opts,
+std::vector &Deps)
+  : DependencyFileGenerator(Opts), Deps(Deps) {}
+
+  void finishedMainFile(DiagnosticsEngine &Diags) override {
+Deps = getDependencies();
+  }
+
+private:
+  std::vector &Deps;
+};
+
+class TestDependencyScanningAction : public tooling::ToolAction {
+public:
+  TestDependencyScanningAction(std::vector &Deps) : Deps(Deps) {}
+
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *FileMgr,
+ std::shared_ptr PCHContainerOps,
+ DiagnosticConsumer *DiagConsumer) override {
+CompilerInstance Compiler(std::move(PCHContainerOps));
+Compiler.setInvocation(std::move(Invocation));
+Compiler.setFileManager(FileMgr);
+
+Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+if (!Compiler.hasDiagnostics())
+  return false;
+
+Compiler.createSourceManager(*FileMgr);
+Compiler.addDependencyCollector(std::make_shared(
+Compiler.getInvocation().getDependencyOutputOpts(), Deps));
+
+auto Action = llvm::make_unique();
+return Compiler.ExecuteAction(*Action);
+  }
+
+private:
+  std::vector &Deps;
+};
+
+} // namespace
+
+TEST(DependencyScanner, ScanDepsReuseFilemanager) {
+  std::vector Compilation = {"-c", "-E", "-MT", "test.cpp.o"};
+  StringRef CWD = "/root";
+  FixedCompilationDatabase CDB(CWD, Compilation);
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  VFS->addFile("/root/header.h", 0, llvm::MemoryBuffer::getMemBuffer("\n"));
+  VFS->addHardLink("/root/symlink.h", "/root/header.h");
+  VFS->addFile("/root/test.cpp", 0,
+   llvm::MemoryBuffer::getMemBuffer(
+   "#include \"symlink.h\"\n#include \"header.h\"\n"));
+
+  ClangTool Tool(CDB, {"test.cpp"}, std::make_shared(),
+ VFS);
+  Tool.clearArgumentsAdjusters();
+  std::vector Deps;
+  TestDependencyScanningAction Action(Deps);
+  Tool.run(&Action);
+  // The first invocation should return dependencies in order of access.
+  ASSERT_EQ(Deps.size(), 3u);
+  EXPECT_EQ(Deps[0], "/root/test.cpp");
+  EXPECT_EQ(Deps[1], "/root/symlink.h");
+  EXPECT_EQ(Deps[2], "/root/header.h");
+
+  // The file manager should still have two FileEntries, as one file is a
+  // hardlink.
+  FileManager &Files = Tool.getFiles();
+  EXPECT_EQ(Files.getNumUniqueRealFiles(), 2u);
+
+  Deps.clear();
+  Tool.run(&Action);
+  // The second invocation should have th

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D62731#1619908 , @lebedev.ri wrote:

> In D62731#1619892 , @mibintc wrote:
>
> > Compared to the 2nd revision, this patch moves all the changes into clang, 
> > removing the FPState file.
> >
> > In the summary, I've copied information from Microsoft about the fp-model 
> > options into the differential Summary, as requested by Kevin, to create a 
> > legacy for future maintainers in case that information disappears from msdn.
>
>
> The the documentation should be documentation, it should reside somewhere in 
> `clang/docs/`.


Got it thanks


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D62731#1603030 , @kpn wrote:

> I actually don't have much of an opinion on what the command line argument 
> form should be. It may be helpful for it to be the same as one of the 
> commonly deployed compilers. The worst I think would be pretty close but with 
> subtle differences. So if it can be made to work like Intel's compiler I'm 
> fine with that. But I'm hoping that more people in the community chime in 
> since having a consensus would be best. Personally, I'm not yet giving any 
> final sign-offs to tickets since I don't think I've been here long enough.
>
> As far as the rounding metadata argument, the language reference says this:
>
> > For values other than “round.dynamic” optimization passes may assume that 
> > the actual runtime rounding mode (as defined in a target-specific manner) 
> > matches the specified rounding mode, but this is not guaranteed. Using a 
> > specific non-dynamic rounding mode which does not match the actual rounding 
> > mode at runtime results in undefined behavior.
>
> Be aware that currently neither of the metadata arguments does anything. They 
> get dropped when llvm reaches the SelectionDAG. And none of the optimization 
> passes that run before that know anything about constrained intrinsics at 
> all. This means they treat code that has them conservatively. Preserving and 
> using that metadata in the optimization passes and getting it down and used 
> by the MI layer is planned but hasn't happened yet. So the full set of 
> arguments may not make sense yet, but an on/off switch for strict mode 
> hopefully does.


@andrew.w.kaylor Can you please check over Kevin's comments about metadata?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D62960: Add SVE opaque built-in types

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/AST/ASTContext.cpp:1974
+  break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
 }

Why do SVE predicates have 16-bit alignment?  Should this be 128-bit 
(16-*byte*)?

I guess these alignments are reasonable to hard-code here since they're 
target-specific for now.  That might be worth including in the comment.



Comment at: lib/AST/ItaniumMangle.cpp:2680
+break;
+#include "clang/Basic/AArch64SVEACLETypes.def"
   }

rjmccall wrote:
> rsandifo-arm wrote:
> > rovka wrote:
> > > erik.pilkington wrote:
> > > > rsandifo-arm wrote:
> > > > > erik.pilkington wrote:
> > > > > > jfb wrote:
> > > > > > > @rjmccall you probably should review this part.
> > > > > > Sorry for the drive by comment, but: All of these mangling should 
> > > > > > really be using the "vendor extension" production IMO:
> > > > > > 
> > > > > > ` ::= u `
> > > > > > 
> > > > > > As is, these manglings intrude on the users's namespace, (i.e. if 
> > > > > > they had a type named `objc_selector` or something), and confuse 
> > > > > > demanglers which incorrectly assume these are substitutable (vendor 
> > > > > > extension builtin types are substitutable too though, but that 
> > > > > > should be handled here).
> > > > > It isn't obvious from the patch, but the SVE names that we're 
> > > > > mangling are predefined names like __SVInt8_t. rather than 
> > > > > user-facing names like svint8_t  The predefined names and their 
> > > > > mangling are defined by the platform ABI 
> > > > > (https://developer.arm.com/docs/100986/), so it wouldn't be valid 
> > > > > for another part of the implementation to use those names for 
> > > > > something else.
> > > > > 
> > > > > I realise you were making a general point here though, sorry.
> > > > > 
> > > > The mangling in the document you linked does use the vendor extension 
> > > > production here though, i.e. the example is `void f(int8x8_t)`, which 
> > > > mangles to _Z1f**u10__Int8x8_t**. It is true that this shouldn't ever 
> > > > collide with another mangling in practice, but my point is there isn't 
> > > > any need to smuggle it into the mangling by pretending it's a user 
> > > > defined type, when the itanium grammar and related tools have a special 
> > > > way for vendors to add builtin types.
> > > I agree with Erik here, the example in the PCS document seems to suggest 
> > > using u. I think either the patch needs to be updated or the document 
> > > needs to be more clear about what the mangling is supposed to look like.
> > Thanks for highlighting this problem, and sorry for not noticing myself 
> > when pointing you at the doc.
> > 
> > Unfortunately, the specification and implementation already difer for the 
> > Advanced SIMD types, with both clang and GCC omitting the 'u' despite the 
> > spec saying it should be present.  So we're considering changing the spec 
> > to match what's now the de facto ABI.
> > 
> > For SVE we do still have the opportunity to use 'u'.  I've left it as-is 
> > for now though, until we've reached a decision about whether to follow 
> > existing practice for Advanced SIMD or whether to do what the spec says.
> These do seem more "builtin" than the SIMD types, but I don't think it deeply 
> matters either way, since these are already reserved names.
Did you actually make the decision to use the `u` mangling?



Comment at: lib/AST/MicrosoftMangle.cpp:2073
+#define SVE_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/AArch64SVEACLETypes.def"
   case BuiltinType::ShortAccum:

Comment isn't adding anything here.



Comment at: lib/AST/NSAPI.cpp:487
+#define SVE_TYPE(Name, Id, SingletonId) case BuiltinType::Id:
+#include "clang/Basic/AArch64SVEACLETypes.def"
   case BuiltinType::BoundMember:

In this and other places, please follow nearby style about whether to place the 
`case` expansion on the following line, as is done above for the OpenCL 
extension types.



Comment at: lib/CodeGen/CGDebugInfo.cpp:709
+  case BuiltinType::Id: \
+return nullptr;
+#include "clang/Basic/AArch64SVEACLETypes.def"

rsandifo-arm wrote:
> rovka wrote:
> > I don't really know this code, but I can't help but notice that nullptr is 
> > only ever used for the void type. Is it safe to also use it for the SVE 
> > types, or can we do something else instead?
> Fixed to report an error here too and return a "safe" value until the TODO is 
> fixed.  Also added a test.
This is fine as long as you're planning to at least fill in a hacky 
implementation when you implement the rest of IRGen support.


Repository:
  rC Clang

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

https://reviews.llvm.org/D62960



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://li

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

In D62731#1601408 , @kpn wrote:

> In D62731#1601310 , @mibintc wrote:
>
> > I think it would be convenient to have an "unset" setting for the different 
> > constrained modes, otherwise you need a boolean that says "no value was 
> > provided for this option".  But i'm a frontend person so I may need to have 
> > my attitude adjusted.
>
>
> What "different constrained modes"? The IRBuilder is either in constrained 
> mode or it isn't. In constrained mode the exception behavior and rounding 
> mode both have defaults, and those defaults can be changed individually 
> without affecting the other setting. The current defaults can also be 
> retrieved if you need something for a function call where you don't want to 
> change it but need an argument anyway. When do you need this "no value 
> provided" setting?
>
> Oh, I'd check the tools/clang/CODE_OWNERS.txt file and add additional 
> appropriate reviewers. Perhaps John McCall and Richard Smith? I don't know 
> who has opinions on how command line options should be handled.
>
> Do we want the Unix driver to be compatible with gcc? Maybe, maybe not. 
> Opinions, anyone?


Yes we need opinions on this.

> The documentation request from lebedev.ri isn't in this ticket yet.

I'm not yet sure what I need for this, the requested documentation is still 
missing.

> Also, for future historical research purposes I'd cut and paste the relevant 
> portions of outside web pages (like intel.com's) and post them somewhere 
> llvm-ish where they are findable. This ticket, for example, is a good place. 
> Web sites gets reorganized or vanish in full or in part. It's helpful to have 
> some insulation from that over time. I've had to fix compiler bugs that 
> actually were 25 years old before. Yes, 25 years old. Being able to do the 
> research is very helpful.
> 
> Oh, it may be useful to know that constrained floating point and the 
> FastMathFlags are not mutually exclusive. I don't know if that matters here 
> or not, but you did mention FastMathFlags.

Yes they aren't mutually exclusive.  One of the fp-model options implies 
enabling fast math.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D62731#1619892 , @mibintc wrote:

> Compared to the 2nd revision, this patch moves all the changes into clang, 
> removing the FPState file.
>
> In the summary, I've copied information from Microsoft about the fp-model 
> options into the differential Summary, as requested by Kevin, to create a 
> legacy for future maintainers in case that information disappears from msdn.


The the documentation should be documentation, it should reside somewhere in 
`clang/docs/`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > ABataev wrote:
> > > > > > jdoerfert wrote:
> > > > > > > I think this should cause an error or at least a warning. Telling 
> > > > > > > the compiler `ps` is a device pointer only to create a local 
> > > > > > > uninitialized shadowing variable seems like an error to me.
> > > > > > It is allowed according to OpenMP 5.0. Private copy must be created 
> > > > > > in the context of the parallel region, not the target region. So, 
> > > > > > for OpenMP 5.0 we should not emit error here.
> > > > > What does that mean and how does that affect my reasoning?
> > > > It means, that for OpenMP 5.0 we should emit a warning/error here. It 
> > > > is allowed according to the standard, we should allow it too.
> > > > So, for OpenMP 5.0 we should not emit error here.
> > > > that for OpenMP 5.0 we should emit a warning/error here.
> > > 
> > > The last answer contradicts what you said earlier. I expect there is a 
> > > *not* missing, correct?
> > > 
> > > 
> > > Assuming you do not want an error, which is fine, I still think a warning 
> > > is appropriate as it seems to me there is never a reason to have a 
> > > `is_device_ptr` clause for a private value. I mean, it is either a bug by 
> > > the programmer, e.g., 5 letters of `firstprivate` went missing, or simply 
> > > nonsensical code for which we warn in other situations as well.
> > Missed `not`.
> > These kind of construct are explicitly allowed in OpenMP. And we should 
> > obey the standard unconditionally.
> > Plus, there might be situations where user may require it explicitly. For 
> > example, the device pointer is dereferenced in one of the clauses for the 
> > subregions but in the deeper subregion it might be used as a private 
> > pointer. Why we should emit a warning here?
> If you have a different situation, e.g., the one you describe, you should not 
> have a warning. Though, that is not the point. If you have the situation 
> above (single directive), as per my reasoning, there doesn't seem to be a 
> sensible use case. If you have one and we should create an explicit test for 
> it.
> 
> > These kind of construct are explicitly allowed in OpenMP.
> `explicitly allowed` != `not forbidded` (yet)
> > And we should obey the standard unconditionally.
> Nobody says we should not. We warn people all the time even if it is valid 
> code.
Warnings may prevent successful compilation in some cases, e.g. when warnings 
are treated as errors. Why we should emit a warning if the construct is allowed 
by the standard? Ask to change the standard if you did not agree with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65838: [Driver] Use enumeration for quoting mode. NFC

[Reid Kleckner via Phabricator via cfe-commits]

rnk accepted this revision.
rnk added a comment.

lgtm+2


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65838



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 214004.
DiegoAstiazaran added a comment.

Rebase to master


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

https://reviews.llvm.org/D65483

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/docs/clang-doc.rst
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
 
   template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
 if (I.first)
   EmittedInfos.emplace_back(std::move(I.first));
 if (I.second)
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -23,9 +23,9 @@
 }
 
 ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
-  ClangDocContext CDCtx;
-  CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+   StringRef RepositoryUrl = "") {
+  ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
   CDCtx.UserStylesheets.insert(
   CDCtx.UserStylesheets.begin(),
   "../share/clang/clang-doc-default-stylesheet.css");
@@ -127,7 +127,7 @@
   I.Path = "X/Y/Z";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -147,7 +147,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "http://www.repository.com";);
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -194,7 +194,12 @@
 
 
   class r
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+http://www.repository.com/dir/test.cpp#10";>10
+ of file 
+http://www.repository.com/dir/test.cpp";>test.cpp
+  
   
 Inherits from 
 F
@@ -232,7 +237,7 @@
   I.Name = "f";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -246,7 +251,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com";);
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -263,7 +268,7 @@
 int
  P)
   
-  Defined at line 10 of test.cpp
+  Defined at line 10 of file dir/test.cpp
 
 )raw";
 
@@ -275,7 +280,7 @@
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
@@ -285,7 +290,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -299,7 +304,12 @@
   
 X
   
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+https://www.repository.com/test.cpp#10";>10
+ of file 
+https://www.repository.com/test.cpp";>test.cpp
+  
 
 )raw";
 
@@ -368,7 +378,7 @@
 
   f
   void f(int I, int J)
-  Defined at line 10 of test.cpp
+  Defined at line 10 of file test.cpp
   
 
Brief description.
Index: 

[PATCH] D62731: [RFC] Add support for options -fp-model= and -fp-speculation= : specify floating point behavior

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 214003.
mibintc edited the summary of this revision.
mibintc added a comment.

Compared to the 2nd revision, this patch moves all the changes into clang, 
removing the FPState file.

In the summary, I've copied information from Microsoft about the fp-model 
options into the differential Summary, as requested by Kevin, to create a 
legacy for future maintainers in case that information disappears from msdn.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62731

Files:
  clang/include/clang/Basic/LangOptions.h
  clang/include/clang/Driver/CC1Options.td
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/fpconstrained.c
  clang/test/Driver/clang_f_opts.c

Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -217,6 +217,11 @@
 // RUN: %clang -### -S -fexec-charset=iso-8859-1 -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-INPUT-CHARSET %s
 // CHECK-INVALID-INPUT-CHARSET: error: invalid value 'iso-8859-1' in '-fexec-charset=iso-8859-1'
 
+// RUN: %clang -### -S -fp-model=fast -fp-model=except -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FAST-EXCEPT %s
+// RUN: %clang -### -S -fp-model=fast -fp-model=except -o /dev/null %s 2>&1 | FileCheck -check-prefix=CHECK-INVALID-FAST-EXCEPT %s
+// CHECK-INVALID-FAST-EXCEPT: error: invalid argument 'fp-model=fast' not allowed with 'fp-model=except'
+//
+
 // Test that we don't error on these.
 // RUN: %clang -### -S -Werror\
 // RUN: -falign-functions -falign-functions=2 -fno-align-functions\
Index: clang/test/CodeGen/fpconstrained.c
===
--- /dev/null
+++ clang/test/CodeGen/fpconstrained.c
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -fp-model=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICT
+// RUN: %clang_cc1 -fp-model=strict -fp-model-except -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICTEXCEPT
+// RUN: %clang_cc1 -fp-model=strict -no-fp-model-except -emit-llvm -o - %s | FileCheck %s -check-prefix=STRICTNOEXCEPT
+// RUN: %clang_cc1 -fp-model-except -emit-llvm -o - %s | FileCheck %s -check-prefix=EXCEPT
+// RUN: %clang_cc1 -no-fp-model-except -emit-llvm -o - %s | FileCheck %s -check-prefix=NOEXCEPT
+// RUN: %clang_cc1 -fp-model=precise -emit-llvm -o - %s | FileCheck %s -check-prefix=PRECISE
+// RUN: %clang_cc1 -fp-model=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=FAST
+// RUN: %clang_cc1 -fp-model=fast -fp-speculation=fast -emit-llvm -o - %s | FileCheck %s -check-prefix=NOEXCEPT
+// RUN: %clang_cc1 -fp-model=fast -fp-speculation=strict -emit-llvm -o - %s | FileCheck %s -check-prefix=EXCEPT
+// RUN: %clang_cc1 -fp-model=fast -fp-speculation=safe -emit-llvm -o - %s | FileCheck %s -check-prefix=MAYTRAP
+float f0, f1, f2;
+
+void foo(void) {
+  // CHECK-LABEL: define {{.*}}void @foo()
+
+  // MAYTRAP: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+  // EXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.tonearest", metadata !"fpexcept.strict")
+  // NOEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.tonearest", metadata !"fpexcept.ignore")
+  // STRICT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  // STRICTEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.strict")
+  // STRICTNOEXCEPT: llvm.experimental.constrained.fadd.f32(float %0, float %1, metadata !"round.dynamic", metadata !"fpexcept.ignore")
+  // PRECISE: fadd float
+  // FAST: fadd fast
+  f0 = f1 + f2;
+
+  // CHECK: ret
+}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3036,6 +3036,59 @@
   Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Val;
   }
 
+  LangOptions::FPModelKind FPM = LangOptions::FPM_Off;
+  if (Arg *A = Args.getLastArg(OPT_fp_model_EQ)) {
+StringRef Val = A->getValue();
+if (Val == "precise")
+  FPM = LangOptions::FPM_Precise;
+else if (Val == "strict")
+  FPM = LangOptions::FPM_Strict;
+else if (Val == "fast")
+  FPM = LangOptions::FPM_Fast;
+else
+  llvm_unreachable("invalid -fp-model setting");
+  }
+  Opts.getFPMOptions().setFPModelSetting(FPM);
+
+  LangOptions::FPModelExceptKind FPME = LangOptions::FPME_Off;
+  if (const Arg *A

[PATCH] D65907: Introduce FileEntryRef and use it when handling includes to report correct dependencies when the FileManager is reused across invocations

[Alex Lorenz via Phabricator via cfe-commits]

arphaman created this revision.
arphaman added reviewers: rsmith, bruno, Bigcheese, jkorous, harlanhaskins.
Herald added subscribers: dexonsmith, mgorny.
Herald added a project: clang.

This patch introduces a parallel API to FileManager's `getFile`: 
`getFileEntryRef`, which returns a reference to the `FileEntry`, and the name 
that was used to access the file.

The new API is adopted only in the HeaderSearch and Preprocessor for include 
file lookup, so that the accessed path can be propagated to SourceManager's 
`FileInfo`. SourceManager's `FileInfo` now can report this accessed path, using 
the new `getName` method. This API is then adopted in the dependency collector, 
which now correctly reports dependencies when a file is included both using a 
symlink and a real path in the case when the FileManager is reused across 
multiple Preprocessor invocations.

Note that this patch does not fix all dependency collector issues, as the same 
problem is still present in other cases when dependencies are obtained using 
`FileSkipped`, `InclusionDirective`, and `HasInclude`. This will be fixed in a 
follow-up patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65907

Files:
  clang/include/clang/Basic/FileManager.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Lex/DirectoryLookup.h
  clang/include/clang/Lex/HeaderSearch.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
  clang/lib/Lex/HeaderSearch.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/DependencyScannerTest.cpp

Index: clang/unittests/Tooling/DependencyScannerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/DependencyScannerTest.cpp
@@ -0,0 +1,118 @@
+//===- unittest/Tooling/ToolingTest.cpp - Tooling unit tests --===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclGroup.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/TargetRegistry.h"
+#include "llvm/Support/TargetSelect.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+
+namespace {
+
+/// Prints out all of the gathered dependencies into a string.
+class TestFileCollector : public DependencyFileGenerator {
+public:
+  TestFileCollector(DependencyOutputOptions &Opts,
+std::vector &Deps)
+  : DependencyFileGenerator(Opts), Deps(Deps) {}
+
+  void finishedMainFile(DiagnosticsEngine &Diags) override {
+Deps = getDependencies();
+  }
+
+private:
+  std::vector &Deps;
+};
+
+class TestDependencyScanningAction : public tooling::ToolAction {
+public:
+  TestDependencyScanningAction(std::vector &Deps) : Deps(Deps) {}
+
+  bool runInvocation(std::shared_ptr Invocation,
+ FileManager *FileMgr,
+ std::shared_ptr PCHContainerOps,
+ DiagnosticConsumer *DiagConsumer) override {
+CompilerInstance Compiler(std::move(PCHContainerOps));
+Compiler.setInvocation(std::move(Invocation));
+Compiler.setFileManager(FileMgr);
+
+Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
+if (!Compiler.hasDiagnostics())
+  return false;
+
+Compiler.createSourceManager(*FileMgr);
+Compiler.addDependencyCollector(std::make_shared(
+Compiler.getInvocation().getDependencyOutputOpts(), Deps));
+
+auto Action = llvm::make_unique();
+return Compiler.ExecuteAction(*Action);
+  }
+
+private:
+  std::vector &Deps;
+};
+
+} // namespace
+
+TEST(DependencyScanner, ScanDepsReuseFilemanager) {
+  std::vector Compilation = {"-c", "-E", "-MT", "test.cpp.o"};
+  StringRef CWD = "/root";
+  FixedCompilationDatabase CDB(CWD, Compilation);
+
+  auto VFS = new llvm::vfs::InMemoryFileSystem();
+  VFS->setCurrentWorkingDirectory(CWD);
+  VFS->addFile("/root/header.h", 0, llvm::MemoryBuffer::getMemBuffer(

[PATCH] D65846: Improve error message from FrontendAction

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added a comment.

In D65846#1619752 , @bruno wrote:

> > `clang -fmodules -fmodules-cache-path=...` is supposed to create the 
> > directory for the cache path, including the parent directories, AFAIK. If 
> > this doesn't happen it is a behavior change (and undesirable IMO).
>
> Is `c-index-test` invoking `clang` or do we just have a similar interface? 
> Perhaps it's not doing it right (haven't seen this problem happening directly 
> while invoking clang). Should `-fallow-pch-with-compiler-errors` be 
> considered somehow here?


To clarify, `c-index-test` only uses the libclang APIs. JF said "The next line 
expects the directory to exist", which I assume refers to the `clang` 
invocation, which is why I mention that a clang invocation does not need to 
expect for the cache directory to exist.
Also `-fallow-pch-with-compiler-errors` is always enabled by the libclang API 
that is used to create a PCH.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65846



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

[PATCH] D65461: [OPENMP]Add support for analysis of linear variables and step.

[Artem Dergachev via Phabricator via cfe-commits]

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

Thx again!




Comment at: lib/AST/OpenMPClause.cpp:473
+  // (Step and CalcStep), list of used expression + step.
+  void *Mem = C.Allocate(totalSizeToAlloc(5 * NumVars + 2 + NumVars  
+1));
   return new (Mem) OMPLinearClause(NumVars);

Formatting error ^.^



Comment at: lib/AST/OpenMPClause.cpp:477
 
+OMPClause::child_range OMPLinearClause::used_children() {
+  // Range includes only non-nullptr elements.

This function name almost makes me slightly uncomfortable -___-"



Comment at: test/OpenMP/distribute_parallel_for_simd_linear_messages.cpp:9
+  int i;
+#pragma omp distribute parallel for simd linear(i)
+  for (i = 0; i < 10; ++i)

I see you guys don't do the `// no-warning` thing very often? 'Cause even if 
it's meaningless for `VerifyDiagnosticsConsumer`, i find it pretty useful when 
reading other people's tests and trying to figure out what they were about. 
I.e., if somebody accidentally breaks your machinery and they see a warning on 
this test, it would be nice if they immediately understand that this is a false 
positive and they need to go fix their code rather than add an `// 
expected-warning`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D65461



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > ABataev wrote:
> > > > > jdoerfert wrote:
> > > > > > I think this should cause an error or at least a warning. Telling 
> > > > > > the compiler `ps` is a device pointer only to create a local 
> > > > > > uninitialized shadowing variable seems like an error to me.
> > > > > It is allowed according to OpenMP 5.0. Private copy must be created 
> > > > > in the context of the parallel region, not the target region. So, for 
> > > > > OpenMP 5.0 we should not emit error here.
> > > > What does that mean and how does that affect my reasoning?
> > > It means, that for OpenMP 5.0 we should emit a warning/error here. It is 
> > > allowed according to the standard, we should allow it too.
> > > So, for OpenMP 5.0 we should not emit error here.
> > > that for OpenMP 5.0 we should emit a warning/error here.
> > 
> > The last answer contradicts what you said earlier. I expect there is a 
> > *not* missing, correct?
> > 
> > 
> > Assuming you do not want an error, which is fine, I still think a warning 
> > is appropriate as it seems to me there is never a reason to have a 
> > `is_device_ptr` clause for a private value. I mean, it is either a bug by 
> > the programmer, e.g., 5 letters of `firstprivate` went missing, or simply 
> > nonsensical code for which we warn in other situations as well.
> Missed `not`.
> These kind of construct are explicitly allowed in OpenMP. And we should obey 
> the standard unconditionally.
> Plus, there might be situations where user may require it explicitly. For 
> example, the device pointer is dereferenced in one of the clauses for the 
> subregions but in the deeper subregion it might be used as a private pointer. 
> Why we should emit a warning here?
If you have a different situation, e.g., the one you describe, you should not 
have a warning. Though, that is not the point. If you have the situation above 
(single directive), as per my reasoning, there doesn't seem to be a sensible 
use case. If you have one and we should create an explicit test for it.

> These kind of construct are explicitly allowed in OpenMP.
`explicitly allowed` != `not forbidded` (yet)
> And we should obey the standard unconditionally.
Nobody says we should not. We warn people all the time even if it is valid code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65906: [clang-scan-deps] Fix edge cases in the minimizer

[Alexandre Ganea via Phabricator via cfe-commits]

aganea created this revision.
aganea added reviewers: arphaman, dexonsmith, Bigcheese.
aganea added a project: clang.
Herald added a subscriber: tschuett.

This patch fixes:

1. Invisible characters that come just before #include, such as #ifndef. 
( were hidden, depending on the display locale). I choose to simply skip 
over non-ASCII characters.
2. Double slashes in #include directive with angle brackets not handled 
correctly: #include 
3. #error directive with quoted, multi-line content, along with CR+LF line 
endings wasn't handled correctly.


Repository:
  rC Clang

https://reviews.llvm.org/D65906

Files:
  lib/Lex/DependencyDirectivesSourceMinimizer.cpp
  test/Lexer/minimize_source_to_dependency_directives_include.c
  test/Lexer/minimize_source_to_dependency_directives_invalid_chars.c
  test/Lexer/minimize_source_to_dependency_directives_invalid_error.c

Index: test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
===
--- test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
+++ test/Lexer/minimize_source_to_dependency_directives_invalid_error.c
@@ -0,0 +1,16 @@
+// Test CF+LF are properly handled along with quoted, multi-line #error
+// RUN: cat %s | unix2dos | %clang_cc1 -DOTHER -print-dependency-directives-minimized-source 2>&1 | FileCheck %s
+
+#ifndef TEST
+#error "message \
+   more message \
+   even more"
+#endif
+
+#ifdef OTHER
+#include 
+#endif
+
+// CHECK:  #ifdef OTHER
+// CHECK-NEXT: #include 
+// CHECK-NEXT: #endif
Index: test/Lexer/minimize_source_to_dependency_directives_invalid_chars.c
===
--- test/Lexer/minimize_source_to_dependency_directives_invalid_chars.c
+++ test/Lexer/minimize_source_to_dependency_directives_invalid_chars.c
@@ -0,0 +1,9 @@
+// Test invisible, bad characters just before #ifdef
+// RUN: echo -n -e '\xef\xbb\xbf#ifdef TEST\n' > %t.c
+// RUN: echo '#include ' >> %t.c
+// RUN: echo '#endif' >> %t.c
+// RUN: %clang_cc1 -DTEST -print-dependency-directives-minimized-source %t.c 2>&1 | FileCheck %s
+
+// CHECK:  #ifdef TEST
+// CHECK-NEXT: #include 
+// CHECK-NEXT: #endif
Index: test/Lexer/minimize_source_to_dependency_directives_include.c
===
--- test/Lexer/minimize_source_to_dependency_directives_include.c
+++ test/Lexer/minimize_source_to_dependency_directives_include.c
@@ -0,0 +1,8 @@
+// Test double slashes in #include directive along with angle brackets. Previously, this was interpreted as comments.
+// RUN: %clang_cc1 -DTEST -print-dependency-directives-minimized-source %s 2>&1 | FileCheck %s
+
+#include "a//b.h"
+#include 
+
+// CHECK: #include "a//b.h"
+// CHECK: #include 
Index: lib/Lex/DependencyDirectivesSourceMinimizer.cpp
===
--- lib/Lex/DependencyDirectivesSourceMinimizer.cpp
+++ lib/Lex/DependencyDirectivesSourceMinimizer.cpp
@@ -113,7 +113,8 @@
 }
 
 static void skipOverSpaces(const char *&First, const char *const End) {
-  while (First != End && isHorizontalWhitespace(*First))
+  while (First != End &&
+ (isHorizontalWhitespace(*First) || !clang::isASCII(*First)))
 ++First;
 }
 
@@ -185,8 +186,8 @@
 }
 
 static void skipString(const char *&First, const char *const End) {
-  assert(*First == '\'' || *First == '"');
-  const char Terminator = *First;
+  assert(*First == '\'' || *First == '"' || *First == '<');
+  const char Terminator = *First == '<' ? '>' : *First;
   for (++First; First != End && *First != Terminator; ++First)
 if (*First == '\\')
   if (++First == End)
@@ -195,15 +196,27 @@
 ++First; // Finish off the string.
 }
 
-static void skipNewline(const char *&First, const char *End) {
-  assert(isVerticalWhitespace(*First));
-  ++First;
+// Returns the length of EOL, either 0 (no end-of-line), 1 (\n) or 2 (\r\n)
+static unsigned isEOL(const char *First, const char *const End) {
   if (First == End)
-return;
+return 0;
+  if (End - First > 1) {
+if (isVerticalWhitespace(First[0]) && isVerticalWhitespace(First[1]) &&
+First[0] != First[1])
+  return 2;
+  }
+  return !!isVerticalWhitespace(First[0]);
+}
 
-  // Check for "\n\r" and "\r\n".
-  if (LLVM_UNLIKELY(isVerticalWhitespace(*First) && First[-1] != First[0]))
-++First;
+static unsigned skipNewline(const char *&First, const char *End) {
+  unsigned Len = isEOL(First, End);
+  assert(Len);
+  First += Len;
+  return Len;
+}
+
+static bool wasLineContinuation(const char *First, unsigned Len) {
+  return First[-(int)Len - 1] == '\\';
 }
 
 static void skipToNewlineRaw(const char *&First, const char *const End) {
@@ -211,17 +224,22 @@
 if (First == End)
   return;
 
-if (isVerticalWhitespace(*First))
+unsigned Len = isEOL(First, End);
+if (Len)
   return;
 
-while (!isVerticalWhitespace(*First))
+do {
   if (

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Nick Desaulniers via Phabricator via cfe-commits]

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



Comment at: clang/lib/Basic/Targets/ARM.cpp:325
+   ? "llvm.arm.gnu.eabi.mcount"
: "\01mcount";
 

Doesn't require changes, but for anyone curious about the `\01`, see the 
comment in `MangleContext::mangleName`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 214000.
DiegoAstiazaran marked 2 inline comments as done.
DiegoAstiazaran added a comment.

Add comments.
Move definition of ClangDocContext constructor to .cpp file.


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

https://reviews.llvm.org/D65483

Files:
  clang-tools-extra/clang-doc/BitcodeReader.cpp
  clang-tools-extra/clang-doc/BitcodeWriter.cpp
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Mapper.cpp
  clang-tools-extra/clang-doc/Mapper.h
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/Serialize.cpp
  clang-tools-extra/clang-doc/Serialize.h
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/docs/clang-doc.rst
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
  clang-tools-extra/unittests/clang-doc/SerializeTest.cpp

Index: clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/SerializeTest.cpp
@@ -37,7 +37,7 @@
 
   template  bool mapDecl(const T *D) {
 auto I = serialize::emitInfo(D, getComment(D), /*Line=*/0,
- /*File=*/"test.cpp", Public);
+ /*File=*/"test.cpp", true, Public);
 if (I.first)
   EmittedInfos.emplace_back(std::move(I.first));
 if (I.second)
Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -22,9 +22,9 @@
 }
 
 ClangDocContext
-getClangDocContext(std::vector UserStylesheets = {}) {
-  ClangDocContext CDCtx;
-  CDCtx.UserStylesheets = {UserStylesheets.begin(), UserStylesheets.end()};
+getClangDocContext(std::vector UserStylesheets = {},
+   StringRef RepositoryUrl = "") {
+  ClangDocContext CDCtx{{}, {}, {}, {}, RepositoryUrl, UserStylesheets, {}};
   CDCtx.UserStylesheets.insert(
   CDCtx.UserStylesheets.begin(),
   "../share/clang/clang-doc-default-stylesheet.css");
@@ -90,7 +90,7 @@
   I.Path = "X/Y/Z";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -110,7 +110,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "http://www.repository.com";);
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -121,7 +121,12 @@
 
 
   class r
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+http://www.repository.com/dir/test.cpp#10";>10
+ of file 
+http://www.repository.com/dir/test.cpp";>test.cpp
+  
   
 Inherits from 
 F
@@ -159,7 +164,7 @@
   I.Name = "f";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"dir/test.cpp"}, false);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   SmallString<16> PathTo;
@@ -173,7 +178,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "https://www.repository.com";);
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -190,7 +195,7 @@
 int
  P)
   
-  Defined at line 10 of test.cpp
+  Defined at line 10 of file dir/test.cpp
 
 )raw";
 
@@ -202,7 +207,7 @@
   I.Name = "e";
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
-  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
+  I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"}, true);
   I.Loc.emplace_back(12, llvm::SmallString<16>{"test.cpp"});
 
   I.Members.emplace_back("X");
@@ -212,7 +217,7 @@
   assert(G);
   std::string Buffer;
   llvm::raw_string_ostream Actual(Buffer);
-  ClangDocContext CDCtx = getClangDocContext();
+  ClangDocContext CDCtx = getClangDocContext({}, "www.repository.com");
   auto Err = G->generateDocForInfo(&I, Actual, CDCtx);
   assert(!Err);
   std::string Expected = R"raw(
@@ -226,7 +231,12 @@
   
 X
   
-  Defined at line 10 of test.cpp
+  
+Defined at line 
+https://www.repository.com/test.cpp#10";>10
+ of file 
+https://www.repository.com/test.cpp";>test.cpp
+  
 
 )raw";
 
@@ -295,7 +305,7 @@
 
   f
   void f(int I, int J)
-  Defined 

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368209: [clang-doc] Add second index for sections within 
info's content (authored by DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65030?vs=213991&id=213992#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65030

Files:
  clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/trunk/clang-doc/Representation.h
  clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
@@ -270,15 +270,22 @@
   return LinkNode;
 }
 
-static std::unique_ptr genTypeReference(const Reference &Type,
-  StringRef CurrentDirectory) {
-  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
-return llvm::make_unique(Type.Name);
+static std::unique_ptr
+genTypeReference(const Reference &Type, StringRef CurrentDirectory,
+ llvm::Optional JumpToSection = None) {
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace) {
+if (!JumpToSection)
+  return llvm::make_unique(Type.Name);
+else
+  return genLink(Type.Name, "#" + JumpToSection.getValue());
+  }
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
   llvm::sys::path::append(Path, Type.Name + ".html");
   // Paths in HTML must be in posix-style
   llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+  if (JumpToSection)
+Path += ("#" + JumpToSection.getValue()).str();
   return genLink(Type.Name, Path);
 }
 
@@ -305,6 +312,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Enums"));
+  Out.back()->Attributes.try_emplace("id", "Enums");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
   auto &DivBody = Out.back();
   for (const auto &E : Enums) {
@@ -333,6 +341,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Functions"));
+  Out.back()->Attributes.try_emplace("id", "Functions");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
   auto &DivBody = Out.back();
   for (const auto &F : Functions) {
@@ -350,6 +359,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Members"));
+  Out.back()->Attributes.try_emplace("id", "Members");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_UL));
   auto &ULBody = Out.back();
   for (const auto &M : Members) {
@@ -373,6 +383,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, Title));
+  Out.back()->Attributes.try_emplace("id", Title);
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_UL));
   auto &ULBody = Out.back();
   for (const auto &R : References)
@@ -409,6 +420,41 @@
   return Out;
 }
 
+template ::value>>
+static Index genInfoIndexItem(const std::vector &Infos, StringRef Title) {
+  Index Idx(Title, Title);
+  for (const auto &C : Infos)
+Idx.Children.emplace_back(C.extractName(),
+  llvm::toHex(llvm::toStringRef(C.USR)));
+  return Idx;
+}
+
+static std::vector> genHTML(const Index &Index,
+ StringRef InfoPath) {
+  std::vector> Out;
+  if (!Index.Name.empty()) {
+Out.emplace_back(llvm::make_unique(HTMLTag::TAG_SPAN));
+auto &SpanBody = Out.back();
+if (!Index.JumpToSection)
+  SpanBody->Children.emplace_back(genTypeReference(Index, InfoPath));
+else
+  SpanBody->Children.emplace_back(genTypeReference(
+  Index, InfoPath, StringRef{Index.JumpToSection.getValue()}));
+  }
+  if (Index.Children.empty())
+return Out;
+  Out.emplace_back(llvm::make_unique(HTMLTag::TAG_UL));
+  const auto &UlBody = Out.back();
+  for (const auto &C : Index.Children) {
+auto LiBody = llvm::make_unique(HTMLTag::TAG_LI);
+std::vector> Nodes = genHTML(C, InfoPath);
+AppendVector(std::move(Nodes), LiBody->Children);
+UlBody->Children.emplace_back(std::move(LiBody));
+  }
+  return Out;
+}
+
 static std::unique_ptr genHTML(const CommentInfo &I) {
   if (I.Kind == "FullComment") {
 auto FullComment = llvm::make_unique(HTMLTag::TAG_DIV);
@@ -455,6 +501,8 @@
 
   Out.emplace_back(
   llvm::make_unique(HTMLTag::TAG_H3, EnumType + I.Name));
+  Out.back()->Attributes.try_emplace("id",
+ llvm::toHex(llvm::toStringRef(I.USR)));
 
   std::unique_ptr Node = genEnumMembersBlock(I.Members);
   if (Node)
@@ -474,6 +522,10 @@
  StringRef ParentInfoDir) {
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H3, I.Name));
+

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213991.

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

https://reviews.llvm.org/D65030

Files:
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/HTMLGeneratorTest.cpp
@@ -9,6 +9,7 @@
 #include "ClangDocTest.h"
 #include "Generators.h"
 #include "Representation.h"
+#include "Serialize.h"
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -59,24 +60,60 @@
 
 
 
+
+  
+
+  Namespaces
+
+  
+  
+
+  Records
+
+  
+  
+
+  Functions
+
+
+  
+
+  OneFunction
+
+  
+
+  
+  
+
+  Enums
+
+
+  
+
+  OneEnum
+
+  
+
+  
+
 
   namespace Namespace
-  Namespaces
+  Namespaces
   
 ChildNamespace
   
-  Records
+  Records
   
 ChildStruct
   
-  Functions
+  Functions
   
-OneFunction
+OneFunction
 OneFunction()
   
-  Enums
+  Enums
   
-enum OneEnum
+enum OneEnum
   
 
 )raw";
@@ -119,6 +156,42 @@
 
 
 
+
+  
+
+  Members
+
+  
+  
+
+  Records
+
+  
+  
+
+  Functions
+
+
+  
+
+  OneFunction
+
+  
+
+  
+  
+
+  Enums
+
+
+  
+
+  OneEnum
+
+  
+
+  
+
 
   class r
   Defined at line 10 of test.cpp
@@ -127,7 +200,7 @@
 F
 , G
   
-  Members
+  Members
   
 
   private 
@@ -135,18 +208,18 @@
X
 
   
-  Records
+  Records
   
 ChildStruct
   
-  Functions
+  Functions
   
-OneFunction
+OneFunction
 OneFunction()
   
-  Enums
+  Enums
   
-enum OneEnum
+enum OneEnum
   
 
 )raw";
@@ -183,7 +256,7 @@
 
 
 
-  f
+  f
   
 float
  f(
@@ -222,7 +295,7 @@
 
 
 
-  enum class e
+  enum class e
   
 X
   
@@ -293,7 +366,7 @@
 
 
 
-  f
+  f
   void f(int I, int J)
   Defined at line 10 of test.cpp
   
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -350,12 +350,15 @@
 
 struct Index : public Reference {
   Index() = default;
+  Index(StringRef Name, StringRef JumpToSection)
+  : Reference(Name), JumpToSection(JumpToSection) {}
   Index(SymbolID USR, StringRef Name, InfoType IT, StringRef Path)
   : Reference(USR, Name, IT, Path) {}
   // This is used to look for a USR in a vector of Indexes using std::find
   bool operator==(const SymbolID &Other) const { return USR == Other; }
   bool operator<(const Index &Other) const { return Name < Other.Name; }
 
+  llvm::Optional> JumpToSection;
   std::vector Children;
 
   void sort();
Index: clang-tools-extra/clang-doc/HTMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/HTMLGenerator.cpp
+++ clang-tools-extra/clang-doc/HTMLGenerator.cpp
@@ -270,15 +270,22 @@
   return LinkNode;
 }
 
-static std::unique_ptr genTypeReference(const Reference &Type,
-  StringRef CurrentDirectory) {
-  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
-return llvm::make_unique(Type.Name);
+static std::unique_ptr
+genTypeReference(const Reference &Type, StringRef CurrentDirectory,
+ llvm::Optional JumpToSection = None) {
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace) {
+if (!JumpToSection)
+  return llvm::make_unique(Type.Name);
+else
+  return genLink(Type.Name, "#" + JumpToSection.getValue());
+  }
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
   llvm::sys::path::append(Path, Type.Name + ".html");
   // Paths in HTML must be in posix-style
   llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+  if (JumpToSection)
+Path += ("#" + JumpToSection.getValue()).str();
   return genLink(Type.Name, Path);
 }
 
@@ -305,6 +312,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Enums"));
+  Out.back()->Attributes.try_emplace("id", "Enums");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
   auto &DivBody = Out.back();
   for (const auto &E : Enums) {
@@ -333,6 +341,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Functions"));
+  Out.back()->Attributes.try_emplace("id", "Functions");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
   auto &DivBody = Out.back();
   for (const auto &F : Functions) {
@@ -350,6 +359,7 @@
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::

[clang-tools-extra] r368209 - [clang-doc] Add second index for sections within info's content

[Diego Astiazaran via cfe-commits]

Author: diegoastiazaran
Date: Wed Aug  7 14:04:52 2019
New Revision: 368209

URL: http://llvm.org/viewvc/llvm-project?rev=368209&view=rev
Log:
[clang-doc] Add second index for sections within info's content

This new index contains links to the main section of infos: Namespaces, 
Records, Functions, Enums, Members.
Also to each child function or enum.
Index is currently rendered on top of the info content, this will be fixed 
later with CSS.

Depends on D65690.

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

Modified:
clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
clang-tools-extra/trunk/clang-doc/Representation.h
clang-tools-extra/trunk/unittests/clang-doc/HTMLGeneratorTest.cpp

Modified: clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp?rev=368209&r1=368208&r2=368209&view=diff
==
--- clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/HTMLGenerator.cpp Wed Aug  7 14:04:52 2019
@@ -270,15 +270,22 @@ static std::unique_ptr genLink(
   return LinkNode;
 }
 
-static std::unique_ptr genTypeReference(const Reference &Type,
-  StringRef CurrentDirectory) {
-  if (Type.Path.empty() && !Type.IsInGlobalNamespace)
-return llvm::make_unique(Type.Name);
+static std::unique_ptr
+genTypeReference(const Reference &Type, StringRef CurrentDirectory,
+ llvm::Optional JumpToSection = None) {
+  if (Type.Path.empty() && !Type.IsInGlobalNamespace) {
+if (!JumpToSection)
+  return llvm::make_unique(Type.Name);
+else
+  return genLink(Type.Name, "#" + JumpToSection.getValue());
+  }
   llvm::SmallString<128> Path =
   computeRelativePath(Type.Path, CurrentDirectory);
   llvm::sys::path::append(Path, Type.Name + ".html");
   // Paths in HTML must be in posix-style
   llvm::sys::path::native(Path, llvm::sys::path::Style::posix);
+  if (JumpToSection)
+Path += ("#" + JumpToSection.getValue()).str();
   return genLink(Type.Name, Path);
 }
 
@@ -305,6 +312,7 @@ genEnumsBlock(const std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Enums"));
+  Out.back()->Attributes.try_emplace("id", "Enums");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
   auto &DivBody = Out.back();
   for (const auto &E : Enums) {
@@ -333,6 +341,7 @@ genFunctionsBlock(const std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Functions"));
+  Out.back()->Attributes.try_emplace("id", "Functions");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_DIV));
   auto &DivBody = Out.back();
   for (const auto &F : Functions) {
@@ -350,6 +359,7 @@ genRecordMembersBlock(const llvm::SmallV
 
   std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, "Members"));
+  Out.back()->Attributes.try_emplace("id", "Members");
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_UL));
   auto &ULBody = Out.back();
   for (const auto &M : Members) {
@@ -373,6 +383,7 @@ genReferencesBlock(const std::vector> Out;
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_H2, Title));
+  Out.back()->Attributes.try_emplace("id", Title);
   Out.emplace_back(llvm::make_unique(HTMLTag::TAG_UL));
   auto &ULBody = Out.back();
   for (const auto &R : References)
@@ -409,6 +420,41 @@ genCommonFileNodes(StringRef Title, Stri
   return Out;
 }
 
+template ::value>>
+static Index genInfoIndexItem(const std::vector &Infos, StringRef Title) {
+  Index Idx(Title, Title);
+  for (const auto &C : Infos)
+Idx.Children.emplace_back(C.extractName(),
+  llvm::toHex(llvm::toStringRef(C.USR)));
+  return Idx;
+}
+
+static std::vector> genHTML(const Index &Index,
+ StringRef InfoPath) {
+  std::vector> Out;
+  if (!Index.Name.empty()) {
+Out.emplace_back(llvm::make_unique(HTMLTag::TAG_SPAN));
+auto &SpanBody = Out.back();
+if (!Index.JumpToSection)
+  SpanBody->Children.emplace_back(genTypeReference(Index, InfoPath));
+else
+  SpanBody->Children.emplace_back(genTypeReference(
+  Index, InfoPath, StringRef{Index.JumpToSection.getValue()}));
+  }
+  if (Index.Children.empty())
+return Out;
+  Out.emplace_back(llvm::make_unique(HTMLTag::TAG_UL));
+  const auto &UlBody = Out.back();
+  for (const auto &C : Index.Children) {
+auto LiBody = llvm::make_unique(HTMLTag::TAG_LI);
+std::vector> Nodes = genHTML(C, InfoPath);
+AppendVector(std::move(Nodes), LiBody->Children);
+UlBody->Children.emplace_back(std::move(LiBody));
+  }
+  return Out;
+}
+
 static std::unique_ptr genHTML(const CommentInfo &I) {
   if (I.Kind == "FullComment") {
 auto FullComment = llvm::make_unique(HTMLTag::TAG_DIV);
@@ -455,6 +501,8 @@ static std::vector(HTMLTag::TAG_H3, EnumType + I.Name));
+  Out.back()->Att

[PATCH] D65853: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

  plotfi@grendel:/mnt/nvme0/llvm-project$ 
build/tools/clang/unittests/DirectoryWatcher/./DirectoryWatcherTests 
--gtest_filter=DirectoryWatcherTest.AddFilesNote: Google Test filter = 
DirectoryWatcherTest.AddFiles
  [==] Running 1 test from 1 test case.
  [--] Global test environment set-up.
  [--] 1 test from DirectoryWatcherTest
  [ RUN  ] DirectoryWatcherTest.AddFiles
  
/mnt/nvme0/llvm-project/clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:344:
 Failure
  Value of: llvm::detail::TakeError(DW.takeError())
  Expected: succeeded
Actual: failed  (inotify_init1() error: Illegal seek)
  [  FAILED  ] DirectoryWatcherTest.AddFiles (3 ms)
  [--] 1 test from DirectoryWatcherTest (3 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test case ran. (3 ms total)
  [  PASSED  ] 0 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] DirectoryWatcherTest.AddFiles
  
   1 FAILED TEST

Is the output in the failure case. This seems fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65853



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

[PATCH] D65853: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65853



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

[PATCH] D65853: Use ASSERT_THAT_ERROR instead of logAllUnhandledErrors/exit

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi added a comment.

I tested this out. Seems to work fine, and print the Expected's Error. I am 
fine with it if @compnerd and @lhames and @jkorous are cool with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65853



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > ABataev wrote:
> > > > jdoerfert wrote:
> > > > > I think this should cause an error or at least a warning. Telling the 
> > > > > compiler `ps` is a device pointer only to create a local 
> > > > > uninitialized shadowing variable seems like an error to me.
> > > > It is allowed according to OpenMP 5.0. Private copy must be created in 
> > > > the context of the parallel region, not the target region. So, for 
> > > > OpenMP 5.0 we should not emit error here.
> > > What does that mean and how does that affect my reasoning?
> > It means, that for OpenMP 5.0 we should emit a warning/error here. It is 
> > allowed according to the standard, we should allow it too.
> > So, for OpenMP 5.0 we should not emit error here.
> > that for OpenMP 5.0 we should emit a warning/error here.
> 
> The last answer contradicts what you said earlier. I expect there is a *not* 
> missing, correct?
> 
> 
> Assuming you do not want an error, which is fine, I still think a warning is 
> appropriate as it seems to me there is never a reason to have a 
> `is_device_ptr` clause for a private value. I mean, it is either a bug by the 
> programmer, e.g., 5 letters of `firstprivate` went missing, or simply 
> nonsensical code for which we warn in other situations as well.
Missed `not`.
These kind of construct are explicitly allowed in OpenMP. And we should obey 
the standard unconditionally.
Plus, there might be situations where user may require it explicitly. For 
example, the device pointer is dereferenced in one of the clauses for the 
subregions but in the deeper subregion it might be used as a private pointer. 
Why we should emit a warning here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:51
+
+  const CallDescriptionMap SugarCastCDM = {
+  {{{"clang", "castAs"}, 0}, &CastValueChecker::evalCastAs},

I'd rather have only one map from call descriptions to (callback, kind) pairs. 
This should make the `evalCall` code much more readable.



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:215
   const auto *CE = cast(Call.getOriginExpr());
-  if (!CE)
+  if (!CE->getType()->getPointeeCXXRecordDecl())
 return false;

Use `Call.getResultType()` instead.

And please add a test with references, as `getResultType` behaves more 
correctly for references:

```lang=c++
C &c = ...;
D &d = dyn_cast(c);
```



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:223
+// If we cannot obtain the arg's class we cannot be sure how to model it.
+if (!CE->getArg(0)->getType()->getPointeeCXXRecordDecl())
+  return false;

`*Call.parameters()[0]->getType()`. It should also help with references. 



Comment at: clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp:231
+// If we cannot obtain 'MCE' we cannot be sure how to model it.
+const auto *MCE = dyn_cast(CE->IgnoreParenImpCasts());
+if (!MCE)

```lang=c++
const auto *InstanceCall = dyn_cast(&Call);
...
DV = InstanceCall->getCXXThisVal();
```



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:718
+  assert(ThisVal.isUnknownOrUndef() || ThisVal.getAs() ||
+ ThisVal.getAs()->getValue().getExtValue() == 1);
   return ThisVal;

I don't think you need this anymore.



Comment at: clang/test/Analysis/cast-value.cpp:186
+  // expected-note@-1 {{Assuming pointer value is null}}
+  // expected-note@-2 {{Assuming dynamic cast to 'Circle' succeeds}}
+  // expected-note@-3 {{'C' initialized here}}

To think: this note isn't very useful because hard casts aren't really supposed 
to fail anyway.


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

https://reviews.llvm.org/D65889



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Diego Astiazarán via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368206: [clang-doc] Parallelize reducing phase (authored by 
DiegoAstiazaran, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65628?vs=213986&id=213988#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65628

Files:
  clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
@@ -28,6 +28,7 @@
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -38,7 +39,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -158,30 +161,6 @@
   return Path;
 }
 
-// Iterate through tool results and build string map of info vectors from the
-// encoded bitstreams.
-bool bitcodeResultsToInfos(
-tooling::ToolResults &Results,
-llvm::StringMap>> &Output) {
-  bool Err = false;
-  Results.forEachResult([&](StringRef Key, StringRef Value) {
-llvm::BitstreamCursor Stream(Value);
-doc::ClangDocBitcodeReader Reader(Stream);
-auto Infos = Reader.readBitcode();
-if (!Infos) {
-  llvm::errs() << toString(Infos.takeError()) << "\n";
-  Err = true;
-  return;
-}
-for (auto &I : Infos.get()) {
-  auto R =
-  Output.try_emplace(Key, std::vector>());
-  R.first->second.emplace_back(std::move(I));
-}
-  });
-  return Err;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -256,40 +235,72 @@
   // In ToolResults, the Key is the hashed USR and the value is the
   // bitcode-encoded representation of the Info object.
   llvm::outs() << "Collecting infos...\n";
-  llvm::StringMap>> USRToInfos;
-  if (bitcodeResultsToInfos(*Exec->get()->getToolResults(), USRToInfos))
-return 1;
+  llvm::StringMap> USRToBitcode;
+  Exec->get()->getToolResults()->forEachResult(
+  [&](StringRef Key, StringRef Value) {
+auto R = USRToBitcode.try_emplace(Key, std::vector());
+R.first->second.emplace_back(Value);
+  });
 
   // First reducing phase (reduce all decls into one info per decl).
-  llvm::outs() << "Reducing " << USRToInfos.size() << " infos...\n";
-  for (auto &Group : USRToInfos) {
-auto Reduced = doc::mergeInfos(Group.getValue());
-if (!Reduced) {
-  llvm::errs() << llvm::toString(Reduced.takeError());
-  continue;
-}
-
-doc::Info *I = Reduced.get().get();
-auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
-  "." + Format);
-if (!InfoPath) {
-  llvm::errs() << toString(InfoPath.takeError()) << "\n";
-  return 1;
-}
-std::error_code FileErr;
-llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
-llvm::sys::fs::OF_None);
-if (FileErr != OK) {
-  llvm::errs() << "Error opening info file: " << FileErr.message() << "\n";
-  continue;
-}
+  llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
+  std::atomic Error;
+  Error = false;
+  // ExecutorConcurrency is a flag exposed by AllTUsExecution.h
+  llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
+ : ExecutorConcurrency);
+  for (auto &Group : USRToBitcode) {
+Pool.async([&]() {
+  std::vector> Infos;
+
+  for (auto &Bitcode : Group.getValue()) {
+llvm::BitstreamCursor Stream(Bitcode);
+doc::ClangDocBitcodeReader Reader(Stream);
+auto ReadInfos = Reader.readBitcode();
+if (!ReadInfos) {
+  llvm::errs() << toString(ReadInfos.takeError()) << "\n";
+  Error = true;
+  return;
+}
+std::move(ReadInfos->begin(), ReadInfos->end(),
+  std::back_inserter(Infos));
+  }
+
+  auto Reduced = doc::mergeInfos(Infos);
+  if (!Reduced) {
+llvm::errs() << llvm::toString(Reduced.takeError());
+return;
+  }
+
+  doc::Info *I = Reduced.get().get();
+  auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
+"." + Format);
+  if (!InfoPath) {
+llvm::errs() << toString(InfoPath.take

[clang-tools-extra] r368206 - [clang-doc] Parallelize reducing phase

[Diego Astiazaran via cfe-commits]

Author: diegoastiazaran
Date: Wed Aug  7 13:54:26 2019
New Revision: 368206

URL: http://llvm.org/viewvc/llvm-project?rev=368206&view=rev
Log:
[clang-doc] Parallelize reducing phase

Reduce phase has been parallelized and a execution time was reduced by
60% with this.
The reading of bitcode (bitcode -> Info) was moved to this segment of
code parallelized so it now happens just before reducing.

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

Modified:
clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp

Modified: clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp?rev=368206&r1=368205&r2=368206&view=diff
==
--- clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp (original)
+++ clang-tools-extra/trunk/clang-doc/tool/ClangDocMain.cpp Wed Aug  7 13:54:26 
2019
@@ -28,6 +28,7 @@
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -38,7 +39,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -158,30 +161,6 @@ llvm::Expected> g
   return Path;
 }
 
-// Iterate through tool results and build string map of info vectors from the
-// encoded bitstreams.
-bool bitcodeResultsToInfos(
-tooling::ToolResults &Results,
-llvm::StringMap>> &Output) {
-  bool Err = false;
-  Results.forEachResult([&](StringRef Key, StringRef Value) {
-llvm::BitstreamCursor Stream(Value);
-doc::ClangDocBitcodeReader Reader(Stream);
-auto Infos = Reader.readBitcode();
-if (!Infos) {
-  llvm::errs() << toString(Infos.takeError()) << "\n";
-  Err = true;
-  return;
-}
-for (auto &I : Infos.get()) {
-  auto R =
-  Output.try_emplace(Key, std::vector>());
-  R.first->second.emplace_back(std::move(I));
-}
-  });
-  return Err;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -256,40 +235,72 @@ int main(int argc, const char **argv) {
   // In ToolResults, the Key is the hashed USR and the value is the
   // bitcode-encoded representation of the Info object.
   llvm::outs() << "Collecting infos...\n";
-  llvm::StringMap>> USRToInfos;
-  if (bitcodeResultsToInfos(*Exec->get()->getToolResults(), USRToInfos))
-return 1;
+  llvm::StringMap> USRToBitcode;
+  Exec->get()->getToolResults()->forEachResult(
+  [&](StringRef Key, StringRef Value) {
+auto R = USRToBitcode.try_emplace(Key, std::vector());
+R.first->second.emplace_back(Value);
+  });
 
   // First reducing phase (reduce all decls into one info per decl).
-  llvm::outs() << "Reducing " << USRToInfos.size() << " infos...\n";
-  for (auto &Group : USRToInfos) {
-auto Reduced = doc::mergeInfos(Group.getValue());
-if (!Reduced) {
-  llvm::errs() << llvm::toString(Reduced.takeError());
-  continue;
-}
-
-doc::Info *I = Reduced.get().get();
-auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
-  "." + Format);
-if (!InfoPath) {
-  llvm::errs() << toString(InfoPath.takeError()) << "\n";
-  return 1;
-}
-std::error_code FileErr;
-llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
-llvm::sys::fs::OF_None);
-if (FileErr != OK) {
-  llvm::errs() << "Error opening info file: " << FileErr.message() << "\n";
-  continue;
-}
+  llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
+  std::atomic Error;
+  Error = false;
+  // ExecutorConcurrency is a flag exposed by AllTUsExecution.h
+  llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
+ : ExecutorConcurrency);
+  for (auto &Group : USRToBitcode) {
+Pool.async([&]() {
+  std::vector> Infos;
+
+  for (auto &Bitcode : Group.getValue()) {
+llvm::BitstreamCursor Stream(Bitcode);
+doc::ClangDocBitcodeReader Reader(Stream);
+auto ReadInfos = Reader.readBitcode();
+if (!ReadInfos) {
+  llvm::errs() << toString(ReadInfos.takeError()) << "\n";
+  Error = true;
+  return;
+}
+std::move(ReadInfos->begin(), ReadInfos->end(),
+  std::back_inserter(Infos));
+  }
+
+  auto Reduced = doc::mergeInfos(Infos);
+  if (!Reduced) {
+llvm::errs() << llvm::toString(Reduced.takeError());
+return;
+  }
+
+  doc

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

ABataev wrote:
> jdoerfert wrote:
> > ABataev wrote:
> > > jdoerfert wrote:
> > > > I think this should cause an error or at least a warning. Telling the 
> > > > compiler `ps` is a device pointer only to create a local uninitialized 
> > > > shadowing variable seems like an error to me.
> > > It is allowed according to OpenMP 5.0. Private copy must be created in 
> > > the context of the parallel region, not the target region. So, for OpenMP 
> > > 5.0 we should not emit error here.
> > What does that mean and how does that affect my reasoning?
> It means, that for OpenMP 5.0 we should emit a warning/error here. It is 
> allowed according to the standard, we should allow it too.
> So, for OpenMP 5.0 we should not emit error here.
> that for OpenMP 5.0 we should emit a warning/error here.

The last answer contradicts what you said earlier. I expect there is a *not* 
missing, correct?


Assuming you do not want an error, which is fine, I still think a warning is 
appropriate as it seems to me there is never a reason to have a `is_device_ptr` 
clause for a private value. I mean, it is either a bug by the programmer, e.g., 
5 letters of `firstprivate` went missing, or simply nonsensical code for which 
we warn in other situations as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213986.
DiegoAstiazaran marked an inline comment as done.
DiegoAstiazaran added a comment.

Add comment


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

https://reviews.llvm.org/D65628

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -28,6 +28,7 @@
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -38,7 +39,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -158,30 +161,6 @@
   return Path;
 }
 
-// Iterate through tool results and build string map of info vectors from the
-// encoded bitstreams.
-bool bitcodeResultsToInfos(
-tooling::ToolResults &Results,
-llvm::StringMap>> &Output) {
-  bool Err = false;
-  Results.forEachResult([&](StringRef Key, StringRef Value) {
-llvm::BitstreamCursor Stream(Value);
-doc::ClangDocBitcodeReader Reader(Stream);
-auto Infos = Reader.readBitcode();
-if (!Infos) {
-  llvm::errs() << toString(Infos.takeError()) << "\n";
-  Err = true;
-  return;
-}
-for (auto &I : Infos.get()) {
-  auto R =
-  Output.try_emplace(Key, std::vector>());
-  R.first->second.emplace_back(std::move(I));
-}
-  });
-  return Err;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -256,41 +235,73 @@
   // In ToolResults, the Key is the hashed USR and the value is the
   // bitcode-encoded representation of the Info object.
   llvm::outs() << "Collecting infos...\n";
-  llvm::StringMap>> USRToInfos;
-  if (bitcodeResultsToInfos(*Exec->get()->getToolResults(), USRToInfos))
-return 1;
+  llvm::StringMap> USRToBitcode;
+  Exec->get()->getToolResults()->forEachResult(
+  [&](StringRef Key, StringRef Value) {
+auto R = USRToBitcode.try_emplace(Key, std::vector());
+R.first->second.emplace_back(Value);
+  });
 
   // First reducing phase (reduce all decls into one info per decl).
-  llvm::outs() << "Reducing " << USRToInfos.size() << " infos...\n";
-  for (auto &Group : USRToInfos) {
-auto Reduced = doc::mergeInfos(Group.getValue());
-if (!Reduced) {
-  llvm::errs() << llvm::toString(Reduced.takeError());
-  continue;
-}
+  llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
+  std::atomic Error;
+  Error = false;
+  // ExecutorConcurrency is a flag exposed by AllTUsExecution.h
+  llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
+ : ExecutorConcurrency);
+  for (auto &Group : USRToBitcode) {
+Pool.async([&]() {
+  std::vector> Infos;
 
-doc::Info *I = Reduced.get().get();
-auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
-  "." + Format);
-if (!InfoPath) {
-  llvm::errs() << toString(InfoPath.takeError()) << "\n";
-  return 1;
-}
-std::error_code FileErr;
-llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
-llvm::sys::fs::OF_None);
-if (FileErr != OK) {
-  llvm::errs() << "Error opening info file: " << FileErr.message() << "\n";
-  continue;
-}
+  for (auto &Bitcode : Group.getValue()) {
+llvm::BitstreamCursor Stream(Bitcode);
+doc::ClangDocBitcodeReader Reader(Stream);
+auto ReadInfos = Reader.readBitcode();
+if (!ReadInfos) {
+  llvm::errs() << toString(ReadInfos.takeError()) << "\n";
+  Error = true;
+  return;
+}
+std::move(ReadInfos->begin(), ReadInfos->end(),
+  std::back_inserter(Infos));
+  }
 
-// Add a reference to this Info in the Index
-clang::doc::Generator::addInfoToIndex(CDCtx.Idx, I);
+  auto Reduced = doc::mergeInfos(Infos);
+  if (!Reduced) {
+llvm::errs() << llvm::toString(Reduced.takeError());
+return;
+  }
 
-if (auto Err = G->get()->generateDocForInfo(I, InfoOS, CDCtx))
-  llvm::errs() << toString(std::move(Err)) << "\n";
+  doc::Info *I = Reduced.get().get();
+  auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
+"." + Format);
+  if (!InfoPath) {
+llvm::errs() << toString(InfoPath.takeError()

[PATCH] D65846: Improve error message from FrontendAction

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno added a comment.



>   Internal compiler error: LFS error for 
> "/Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx"failed
>  to create unique file 
> /Users/jfb/s/llvmm/debug/tools/clang/test/Index/Output/pch-from-libclang.c.tmp.mcp/14EC4PG1UTVLY/modules.idx.lock-0ae18733:
>  No such file or directory

yay! I really like how it looks now, thanks for pushing on this.

> `clang -fmodules -fmodules-cache-path=...` is supposed to create the 
> directory for the cache path, including the parent directories, AFAIK. If 
> this doesn't happen it is a behavior change (and undesirable IMO).

Is `c-index-test` invoking `clang` or do we just have a similar interface? 
Perhaps it's not doing it right (haven't seen this problem happening directly 
while invoking clang). Should `-fallow-pch-with-compiler-errors` be considered 
somehow here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65846



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Julie Hockett via Phabricator via cfe-commits]

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

LGTM (after comment nit addressed)




Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:249
+  Error = false;
+  llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
+ : ExecutorConcurrency);

nit: comment about where this var is coming from


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

https://reviews.llvm.org/D65628



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

Re: [PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

[Michał Górny via cfe-commits]

On Wed, 2019-08-07 at 17:10 -0300, Adhemerval Zanella wrote:
> On 05/06/2019 05:19, Michał Górny via Phabricator via llvm-commits wrote:
> > This revision was automatically updated to reflect the committed changes.
> > Closed by commit rL362587: [clang] [test] Add a (xfailing) test for PR41027 
> > (authored by mgorny, committed by ).
> > Herald added a project: LLVM.
> > Herald added a subscriber: llvm-commits.
> > 
> > Changed prior to commit:
> >   https://reviews.llvm.org/D60728?vs=195518&id=203097#toc
> > 
> > Repository:
> >   rL LLVM
> > 
> > CHANGES SINCE LAST ACTION
> >   https://reviews.llvm.org/D60728/new/
> > 
> > https://reviews.llvm.org/D60728
> > 
> > Files:
> >   cfe/trunk/test/Sema/pr41027.c
> 
> This patch is failing on aarch64-linux bot [1], I think you need to add a 
> 'requires: x86_64'.
> 
> [1] http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/7694
> 

Update: I see Bill Wendling already fixed it in r368202.

-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65483: [clang-doc] Add link to source code in file definitions

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-doc/Representation.h:385
+SourceRoot(SourceRoot), UserStylesheets(UserStylesheets),
+JsScripts(JsScripts) {
+if (!RepositoryUrl.empty()) {

Move to .cpp file (now that it has a non-trivial implementation)



Comment at: clang-tools-extra/clang-doc/Representation.h:396
+  std::string OutDirectory; // Directory for outputting generated files.
+  std::string SourceRoot;   //
+  llvm::Optional RepositoryUrl;

Comment


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

https://reviews.llvm.org/D65483



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

Re: [PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

[Michał Górny via cfe-commits]

On Wed, 2019-08-07 at 17:10 -0300, Adhemerval Zanella wrote:
> On 05/06/2019 05:19, Michał Górny via Phabricator via llvm-commits wrote:
> > This revision was automatically updated to reflect the committed changes.
> > Closed by commit rL362587: [clang] [test] Add a (xfailing) test for PR41027 
> > (authored by mgorny, committed by ).
> > Herald added a project: LLVM.
> > Herald added a subscriber: llvm-commits.
> > 
> > Changed prior to commit:
> >   https://reviews.llvm.org/D60728?vs=195518&id=203097#toc
> > 
> > Repository:
> >   rL LLVM
> > 
> > CHANGES SINCE LAST ACTION
> >   https://reviews.llvm.org/D60728/new/
> > 
> > https://reviews.llvm.org/D60728
> > 
> > Files:
> >   cfe/trunk/test/Sema/pr41027.c
> 
> This patch is failing on aarch64-linux bot [1], I think you need to add a 
> 'requires: x86_64'.
> 
> [1] http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/7694
> 

Thanks for the report.  You're probably right.  I'll verify the correct
REQUIRES for it, and commit it in a few minutes.


-- 
Best regards,
Michał Górny



signature.asc
Description: This is a digitally signed message part
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65030: [clang-doc] Add second index for sections within info's content

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added a comment.

LGTM


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

https://reviews.llvm.org/D65030



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

[PATCH] D65545: Handle some fs::remove failures

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 213980.
jfb marked 3 inline comments as done.
jfb added a comment.

- Don't report_fatal_error in code that can be called from LLDB


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Frontend/ASTUnit.h
  clang/include/clang/Frontend/PrecompiledPreamble.h
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/DependencyFile.cpp
  clang/lib/Frontend/PrecompiledPreamble.cpp
  clang/lib/Rewrite/Rewriter.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1gen_reproducer_main.cpp
  clang/tools/libclang/CIndex.cpp
  clang/unittests/CrossTU/CrossTranslationUnitTest.cpp
  clang/unittests/Frontend/ASTUnitTest.cpp
  llvm/include/llvm/Support/FileUtilities.h
  llvm/lib/Support/GraphWriter.cpp
  llvm/lib/Support/LockFileManager.cpp
  llvm/lib/Support/Path.cpp
  llvm/lib/Support/ToolOutputFile.cpp

Index: llvm/lib/Support/ToolOutputFile.cpp
===
--- llvm/lib/Support/ToolOutputFile.cpp
+++ llvm/lib/Support/ToolOutputFile.cpp
@@ -25,7 +25,9 @@
 ToolOutputFile::CleanupInstaller::~CleanupInstaller() {
   // Delete the file if the client hasn't told us not to.
   if (!Keep && Filename != "-")
-sys::fs::remove(Filename);
+if (std::error_code EC = sys::fs::remove(Filename))
+  report_fatal_error("Failed removing file \"" + Filename +
+ "\": " + EC.message());
 
   // Ok, the file is successfully written and closed, or deleted. There's no
   // further need to clean it up on signals.
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -1182,9 +1182,10 @@
   if (RenameEC) {
 // If we can't rename, try to copy to work around cross-device link issues.
 RenameEC = sys::fs::copy_file(TmpName, Name);
-// If we can't rename or copy, discard the temporary file.
+// If we can't rename or copy, discard the temporary file, ignoring any
+// further failure.
 if (RenameEC)
-  remove(TmpName);
+  (void)remove(TmpName);
   }
   sys::DontRemoveFileOnSignal(TmpName);
 #endif
Index: llvm/lib/Support/LockFileManager.cpp
===
--- llvm/lib/Support/LockFileManager.cpp
+++ llvm/lib/Support/LockFileManager.cpp
@@ -47,14 +47,16 @@
 /// \param LockFileName The name of the lock file to read.
 ///
 /// \returns The process ID of the process that owns this lock file
-Optional >
+Optional>
 LockFileManager::readLockFile(StringRef LockFileName) {
   // Read the owning host and PID out of the lock file. If it appears that the
   // owning process is dead, the lock file is invalid.
   ErrorOr> MBOrErr =
   MemoryBuffer::getFile(LockFileName);
   if (!MBOrErr) {
-sys::fs::remove(LockFileName);
+if (std::error_code EC = sys::fs::remove(LockFileName))
+  report_fatal_error("Unable to remove invalid lock file \"" +
+ LockFileName + "\": " + EC.message());
 return None;
   }
   MemoryBuffer &MB = *MBOrErr.get();
@@ -71,7 +73,9 @@
   }
 
   // Delete the lock file. It's invalid anyway.
-  sys::fs::remove(LockFileName);
+  if (std::error_code EC = sys::fs::remove(LockFileName))
+report_fatal_error("Unable to remove invalid lock file \"" + LockFileName +
+   "\": " + EC.message());
   return None;
 }
 
@@ -144,7 +148,9 @@
   // released.
   return;
 }
-sys::fs::remove(Filename);
+if (std::error_code EC = sys::fs::remove(Filename))
+  report_fatal_error("Unable to remove unique lock file on signal \"" +
+ Filename + "\": " + EC.message());
 sys::DontRemoveFileOnSignal(Filename);
   }
 
@@ -204,8 +210,9 @@
   // unique lock file, and fail.
   std::string S("failed to write to ");
   S.append(UniqueLockFileName.str());
+  if (std::error_code EC = sys::fs::remove(UniqueLockFileName))
+S.append(", also failed to remove the lockfile");
   setError(Out.error(), S);
-  sys::fs::remove(UniqueLockFileName);
   return;
 }
   }
@@ -234,8 +241,12 @@
 // Someone else managed to create the lock file first. Read the process ID
 // from the lock file.
 if ((Owner = readLockFile(LockFileName))) {
-  // Wipe out our unique lock file (it's useless now)
-  sys::fs::remove(UniqueLockFileName);
+  // Wipe out our unique lock file (it's useless now).
+  if ((EC = sys::fs::remove(UniqueLockFileName))) {
+std::string S("failed to remove useless lockfile ");
+S.append(UniqueLockFileName.str());
+setError(EC, S);
+  }
   return;
 }
 
@@ -283,8 +294,12 @@
 return;
 
   // Since we own the lock, remove the 

[PATCH] D65545: Handle some fs::remove failures

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545



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

[PATCH] D65846: Improve error message from FrontendAction

[Argyrios Kyrtzidis via Phabricator via cfe-commits]

akyrtzi added a comment.

In D65846#1619645 , @jfb wrote:

> My current guess is that this part of the test:
>
>   c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp 
> -Xclang -triple -Xclang x86_64-apple-darwin
>
>
> Is expected to generate the `unknown type name` error, but when than happens 
> it ignores `-fmodules-cache-path=%t.mcp` and doesn't create the directory. 
> The next line expects the directory to exist, which is why it can't create 
> the lock file (because the directory it's trying to create it in doesn't 
> exist).


`clang -fmodules -fmodules-cache-path=...` is supposed to create the directory 
for the cache path, including the parent directories, AFAIK. If this doesn't 
happen it is a behavior change (and undesirable IMO).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65846



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

Re: [PATCH] D60728: [clang] [test] Add a (xfailing) test for PR41027

[Adhemerval Zanella via cfe-commits]

On 05/06/2019 05:19, Michał Górny via Phabricator via llvm-commits wrote:
> This revision was automatically updated to reflect the committed changes.
> Closed by commit rL362587: [clang] [test] Add a (xfailing) test for PR41027 
> (authored by mgorny, committed by ).
> Herald added a project: LLVM.
> Herald added a subscriber: llvm-commits.
> 
> Changed prior to commit:
>   https://reviews.llvm.org/D60728?vs=195518&id=203097#toc
> 
> Repository:
>   rL LLVM
> 
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D60728/new/
> 
> https://reviews.llvm.org/D60728
> 
> Files:
>   cfe/trunk/test/Sema/pr41027.c

This patch is failing on aarch64-linux bot [1], I think you need to add a 
'requires: x86_64'.

[1] http://lab.llvm.org:8011/builders/clang-cmake-aarch64-full/builds/7694

> 
> 
> Index: cfe/trunk/test/Sema/pr41027.c
> ===
> --- cfe/trunk/test/Sema/pr41027.c
> +++ cfe/trunk/test/Sema/pr41027.c
> @@ -0,0 +1,10 @@
> +// RUN: %clang_cc1 -triple x86_64 -fsyntax-only %s
> +// XFAIL: *
> +
> +inline void pr41027(unsigned a, unsigned b) {
> +  if (__builtin_constant_p(a)) {
> +__asm__ volatile("outl %0,%w1" : : "a"(b), "n"(a));
> +  } else {
> +__asm__ volatile("outl %0,%w1" : : "a"(b), "d"(a));
> +  }
> +}
> 
> 
> 
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65878: [Refactor] Moving SourceExtraction header from lib to include

[Shaurya Gupta via Phabricator via cfe-commits]

SureYeaah updated this revision to Diff 213973.
SureYeaah added a comment.

Added endif comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65878

Files:
  clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
  clang/lib/Tooling/Refactoring/Extract/Extract.cpp
  clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
  clang/lib/Tooling/Refactoring/Extract/SourceExtraction.h


Index: clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
===
--- clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
+++ clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
@@ -6,7 +6,7 @@
 //
 
//===--===//
 
-#include "SourceExtraction.h"
+#include "clang/Tooling/Refactoring/Extract/SourceExtraction.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
Index: clang/lib/Tooling/Refactoring/Extract/Extract.cpp
===
--- clang/lib/Tooling/Refactoring/Extract/Extract.cpp
+++ clang/lib/Tooling/Refactoring/Extract/Extract.cpp
@@ -13,12 +13,12 @@
 
//===--===//
 
 #include "clang/Tooling/Refactoring/Extract/Extract.h"
-#include "SourceExtraction.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Refactoring/Extract/SourceExtraction.h"
 
 namespace clang {
 namespace tooling {
Index: clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
===
--- clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
+++ clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
@@ -6,8 +6,8 @@
 //
 
//===--===//
 
-#ifndef LLVM_CLANG_LIB_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
-#define LLVM_CLANG_LIB_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
+#ifndef LLVM_CLANG_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
+#define LLVM_CLANG_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
 
 #include "clang/Basic/LLVM.h"
 
@@ -48,4 +48,4 @@
 } // end namespace tooling
 } // end namespace clang
 
-#endif // LLVM_CLANG_LIB_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
+#endif //LLVM_CLANG_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H


Index: clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
===
--- clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
+++ clang/lib/Tooling/Refactoring/Extract/SourceExtraction.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 
-#include "SourceExtraction.h"
+#include "clang/Tooling/Refactoring/Extract/SourceExtraction.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
Index: clang/lib/Tooling/Refactoring/Extract/Extract.cpp
===
--- clang/lib/Tooling/Refactoring/Extract/Extract.cpp
+++ clang/lib/Tooling/Refactoring/Extract/Extract.cpp
@@ -13,12 +13,12 @@
 //===--===//
 
 #include "clang/Tooling/Refactoring/Extract/Extract.h"
-#include "SourceExtraction.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/Rewrite/Core/Rewriter.h"
+#include "clang/Tooling/Refactoring/Extract/SourceExtraction.h"
 
 namespace clang {
 namespace tooling {
Index: clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
===
--- clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
+++ clang/include/clang/Tooling/Refactoring/Extract/SourceExtraction.h
@@ -6,8 +6,8 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_LIB_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
-#define LLVM_CLANG_LIB_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
+#ifndef LLVM_CLANG_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
+#define LLVM_CLANG_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
 
 #include "clang/Basic/LLVM.h"
 
@@ -48,4 +48,4 @@
 } // end namespace tooling
 } // end namespace clang
 
-#endif // LLVM_CLANG_LIB_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
+#endif //LLVM_CLANG_TOOLING_REFACTORING_EXTRACT_SOURCE_EXTRACTION_H
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm

[PATCH] D65846: Improve error message from FrontendAction

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

My current guess is that this part of the test:

  c-index-test -write-pch %t.h.pch %s -fmodules -fmodules-cache-path=%t.mcp 
-Xclang -triple -Xclang x86_64-apple-darwin

Is expected to generate the `unknown type name` error, but when than happens it 
ignores `-fmodules-cache-path=%t.mcp` and doesn't create the directory. The 
next line expects the directory to exist, which is why it can't create the lock 
file (because the directory it's trying to create it in doesn't exist).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65846



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

[PATCH] D65545: Handle some fs::remove failures

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added a comment.

Thanks for looping me in, @vsapsai!




Comment at: llvm/include/llvm/Support/FileUtilities.h:52-53
   if (DeleteIt) {
-// Ignore problems deleting the file.
-sys::fs::remove(Filename);
+if (std::error_code EC = sys::fs::remove(Filename))
+  report_fatal_error("failed removing file \"" + Filename + "\": " + 
EC.message());
   }

vsapsai wrote:
> For this change opinion of LLDB developers can be useful as it changes 
> existing `FileRemover` behavior.
Aborting isn't acceptable for LLDB. Can we turn this into an assert instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[Bill Wendling via Phabricator via cfe-commits]

void added a comment.

In D60943#1619579 , @JamesNagurne 
wrote:

> In case you haven't seen, this commit breaks non-x86 build bots due to the 
> combination of '-triple x86_64*' and '-S'. Some tests that use this target 
> are only looking for AST dumps, and do not actually require such a target. 
> This is not one of those tests, as it's inspecting assembly.
>  See clang/test/CodeGen/addrsig.c to see how that is handled (via REQUIRES: 
> x86-registered-target).
>
> Oddly enough, other tests that use -triple x86_64* and only inspect the AST 
> don't require the registered target.


Sorry about that. Fixed in r368202.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60943



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran added inline comments.



Comment at: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp:72
 
+static llvm::cl::opt ThreadCount(
+"thread-count",

juliehockett wrote:
> Can we use the pre-existing concurrency flag instead 
> (https://github.com/llvm/llvm-project/blob/91e5cdfc93729c61c757db4efd4a82670ac7f929/clang/lib/Tooling/AllTUsExecution.cpp#L150)?
>  It seems counterintuitive to have to set two different concurrency flags for 
> one tool.
> 
> You'll have to put up a separate patch to expose that flag in the AllTUs 
> header, so that you can access it (see rL344335, as an example). 
Done in D65833.


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

https://reviews.llvm.org/D65628



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

[PATCH] D65628: [clang-doc] Parallelize reducing phase

[Diego Astiazarán via Phabricator via cfe-commits]

DiegoAstiazaran updated this revision to Diff 213968.
DiegoAstiazaran marked 2 inline comments as done.
DiegoAstiazaran added a comment.

Use pre-existing concurrency flag `--execute-concurrency` instead implementing 
a new one.


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

https://reviews.llvm.org/D65628

Files:
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp

Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -28,6 +28,7 @@
 #include "clang/ASTMatchers/ASTMatchersInternal.h"
 #include "clang/Driver/Options.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Execution.h"
 #include "clang/Tooling/Tooling.h"
@@ -38,7 +39,9 @@
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Signals.h"
+#include "llvm/Support/ThreadPool.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 
 using namespace clang::ast_matchers;
@@ -158,30 +161,6 @@
   return Path;
 }
 
-// Iterate through tool results and build string map of info vectors from the
-// encoded bitstreams.
-bool bitcodeResultsToInfos(
-tooling::ToolResults &Results,
-llvm::StringMap>> &Output) {
-  bool Err = false;
-  Results.forEachResult([&](StringRef Key, StringRef Value) {
-llvm::BitstreamCursor Stream(Value);
-doc::ClangDocBitcodeReader Reader(Stream);
-auto Infos = Reader.readBitcode();
-if (!Infos) {
-  llvm::errs() << toString(Infos.takeError()) << "\n";
-  Err = true;
-  return;
-}
-for (auto &I : Infos.get()) {
-  auto R =
-  Output.try_emplace(Key, std::vector>());
-  R.first->second.emplace_back(std::move(I));
-}
-  });
-  return Err;
-}
-
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
   std::error_code OK;
@@ -256,41 +235,72 @@
   // In ToolResults, the Key is the hashed USR and the value is the
   // bitcode-encoded representation of the Info object.
   llvm::outs() << "Collecting infos...\n";
-  llvm::StringMap>> USRToInfos;
-  if (bitcodeResultsToInfos(*Exec->get()->getToolResults(), USRToInfos))
-return 1;
+  llvm::StringMap> USRToBitcode;
+  Exec->get()->getToolResults()->forEachResult(
+  [&](StringRef Key, StringRef Value) {
+auto R = USRToBitcode.try_emplace(Key, std::vector());
+R.first->second.emplace_back(Value);
+  });
 
   // First reducing phase (reduce all decls into one info per decl).
-  llvm::outs() << "Reducing " << USRToInfos.size() << " infos...\n";
-  for (auto &Group : USRToInfos) {
-auto Reduced = doc::mergeInfos(Group.getValue());
-if (!Reduced) {
-  llvm::errs() << llvm::toString(Reduced.takeError());
-  continue;
-}
+  llvm::outs() << "Reducing " << USRToBitcode.size() << " infos...\n";
+  std::atomic Error;
+  Error = false;
+  llvm::ThreadPool Pool(ExecutorConcurrency == 0 ? llvm::hardware_concurrency()
+ : ExecutorConcurrency);
+  for (auto &Group : USRToBitcode) {
+Pool.async([&]() {
+  std::vector> Infos;
 
-doc::Info *I = Reduced.get().get();
-auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
-  "." + Format);
-if (!InfoPath) {
-  llvm::errs() << toString(InfoPath.takeError()) << "\n";
-  return 1;
-}
-std::error_code FileErr;
-llvm::raw_fd_ostream InfoOS(InfoPath.get(), FileErr,
-llvm::sys::fs::OF_None);
-if (FileErr != OK) {
-  llvm::errs() << "Error opening info file: " << FileErr.message() << "\n";
-  continue;
-}
+  for (auto &Bitcode : Group.getValue()) {
+llvm::BitstreamCursor Stream(Bitcode);
+doc::ClangDocBitcodeReader Reader(Stream);
+auto ReadInfos = Reader.readBitcode();
+if (!ReadInfos) {
+  llvm::errs() << toString(ReadInfos.takeError()) << "\n";
+  Error = true;
+  return;
+}
+std::move(ReadInfos->begin(), ReadInfos->end(),
+  std::back_inserter(Infos));
+  }
 
-// Add a reference to this Info in the Index
-clang::doc::Generator::addInfoToIndex(CDCtx.Idx, I);
+  auto Reduced = doc::mergeInfos(Infos);
+  if (!Reduced) {
+llvm::errs() << llvm::toString(Reduced.takeError());
+return;
+  }
 
-if (auto Err = G->get()->generateDocForInfo(I, InfoOS, CDCtx))
-  llvm::errs() << toString(std::move(Err)) << "\n";
+  doc::Info *I = Reduced.get().get();
+  auto InfoPath = getInfoOutputFile(OutDirectory, I->Path, I->extractName(),
+"." + Format);
+  if (!InfoPath) {
+llvm::errs() << toString(InfoPa

r368202 - Add target requirements for those bots which don't handle x86.

[Bill Wendling via cfe-commits]

Author: void
Date: Wed Aug  7 12:36:48 2019
New Revision: 368202

URL: http://llvm.org/viewvc/llvm-project?rev=368202&view=rev
Log:
Add target requirements for those bots which don't handle x86.

Modified:
cfe/trunk/test/CodeGen/pr41027.c

Modified: cfe/trunk/test/CodeGen/pr41027.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/pr41027.c?rev=368202&r1=368201&r2=368202&view=diff
==
--- cfe/trunk/test/CodeGen/pr41027.c (original)
+++ cfe/trunk/test/CodeGen/pr41027.c Wed Aug  7 12:36:48 2019
@@ -1,3 +1,4 @@
+// REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -O2 -o - %s | FileCheck %s
 
 // CHECK-LABEL: f:


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

[PATCH] D62960: Add SVE opaque built-in types

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm added a comment.

Ping.

@rjmccall: thanks for the positive feedback.  Is there a particular reviewer 
you'd like to hear from before the patch lands?


Repository:
  rC Clang

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

https://reviews.llvm.org/D62960



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

[PATCH] D65545: Handle some fs::remove failures

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

I have no other comments but for the fatal error in `FileRemover` I'd like to 
loop in Jonas as he was touching module cache in LLDB fairly recently.




Comment at: clang/lib/Frontend/CompilerInstance.cpp:1444-1445
   // Remove the file.
-  llvm::sys::fs::remove(File->path());
+  if ((EC = llvm::sys::fs::remove(File->path(
+break;
 

jfb wrote:
> vsapsai wrote:
> > jfb wrote:
> > > vsapsai wrote:
> > > > Why are you interrupting the loop when cannot remove a file? Don't know 
> > > > which option is the right one, just want to know your reasons.
> > > The loops already bail when `EC` is set, and here I figure if we can't 
> > > remove the base file we shouldn't try to remove its corresponding 
> > > timestamp.
> > I was thinking about using `continue` instead of `break`. After some 
> > thinking I believe `continue` is better because this way we'll try to 
> > remove all stale module files that we can. Otherwise one bad file can 
> > prevent the module cache pruning from working.
> > 
> > And I agree with your logic about not removing the timestamp when cannot 
> > remove the base file.
> `continue` will hit `File != FileEnd && !EC`, so it's equivalent to `break`. 
> Your thinking makes sense, but I think we'd want to do it in a separate patch.
OK, I see now. Thanks for explanation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65545



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

[PATCH] D60943: Delay diagnosing asm constraints that require immediates until after inlining

[James Nagurne via Phabricator via cfe-commits]

JamesNagurne added a comment.

In case you haven't seen, this commit breaks non-x86 build bots due to the 
combination of '-triple x86_64*' and '-S'. Some tests that use this target are 
only looking for AST dumps, and do not actually require such a target. This is 
not one of those tests, as it's inspecting assembly.
See clang/test/CodeGen/addrsig.c to see how that is handled (via REQUIRES: 
x86-registered-target).

Oddly enough, other tests that use -triple x86_64* and only inspect the AST 
don't require the registered target.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60943



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D65835#1619549 , @jdenny wrote:

> In D65835#1619526 , @ABataev wrote:
>
> > > Maybe, but I haven't found any statement in either version that states 
> > > that map restrictions apply to is_device_ptr.
> >
> > `is_device_ptr` is a kind of mapping clause. I assume we can extend the 
> > restrictions for `map` clause to this clause too.
>
>
> I'd like to understand this better.  Is there something from the spec we can 
> quote in the code?


See 2.19.7 Data-Mapping Attribute Rules, Clauses, and Directives

> 
> 
>>> Another question is whether the restriction would make sense. For example, 
>>> does it ever make sense to specify both is_device_ptr and firstprivate for 
>>> the same variable on a target construct?
>> 
>> On a `target` construct - no.
> 
> Why not?

It is meaningless. That's why it is prohibited in OpenMP 5.0 and allowed only 
for the combined constructs. These private clauses are applied to inner 
subconstructs.
For example, `target parallel map(p) private(p)`. In the context of `target` 
region the variable is mapped while in the `parallel` context it is private.
For `target map(p) private(p)` it is absolutely meaningless.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

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

jdenny added a comment.

In D65835#1619526 , @ABataev wrote:

> > Maybe, but I haven't found any statement in either version that states that 
> > map restrictions apply to is_device_ptr.
>
> `is_device_ptr` is a kind of mapping clause. I assume we can extend the 
> restrictions for `map` clause to this clause too.


I'd like to understand this better.  Is there something from the spec we can 
quote in the code?

>> Another question is whether the restriction would make sense. For example, 
>> does it ever make sense to specify both is_device_ptr and firstprivate for 
>> the same variable on a target construct?
> 
> On a `target` construct - no.

Why not?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

> I should have reported that the current implementation isn't complete for 
> OpenMP 4.5. For example, on target teams, reduction(+:x) map(x) is an error 
> but not map(x) reduction(+:x). So there are bugs to fix, and maybe this will 
> evolve into multiple patches, but I want to be sure I'm on the right path 
> first.

It is just a bug, not a missing feature. Just file a bug report for it.




Comment at: clang/lib/Sema/SemaOpenMP.cpp:10895
+// combined construct.
+if (CurrDir == OMPD_target) {
   OpenMPClauseKind ConflictKind;

jdenny wrote:
> ABataev wrote:
> > I would suggest to guard this change and limit this new functionality only 
> > for OpenMP 5.0. 
> Do you agree that this is strictly an extension to 4.5 that won't alter the 
> behavior of 4.5-conforming applications?
> 
> Do we generally want to complain about the use of extensions, or is there 
> another reason for the guard you suggest?
No. It is incorrect according to OpenMP 4.5 and we shall emit diagnostics here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

> Maybe, but I haven't found any statement in either version that states that 
> map restrictions apply to is_device_ptr.

`is_device_ptr` is a kind of mapping clause. I assume we can extend the 
restrictions for `map` clause to this clause too.

> Another question is whether the restriction would make sense. For example, 
> does it ever make sense to specify both is_device_ptr and firstprivate for 
> the same variable on a target construct?

On a `target` construct - no. On a `target parallel` - yes. This may be 
important especially in unified shared memory mode.




Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

jdoerfert wrote:
> ABataev wrote:
> > jdoerfert wrote:
> > > I think this should cause an error or at least a warning. Telling the 
> > > compiler `ps` is a device pointer only to create a local uninitialized 
> > > shadowing variable seems like an error to me.
> > It is allowed according to OpenMP 5.0. Private copy must be created in the 
> > context of the parallel region, not the target region. So, for OpenMP 5.0 
> > we should not emit error here.
> What does that mean and how does that affect my reasoning?
It means, that for OpenMP 5.0 we should emit a warning/error here. It is 
allowed according to the standard, we should allow it too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

ABataev wrote:
> jdoerfert wrote:
> > I think this should cause an error or at least a warning. Telling the 
> > compiler `ps` is a device pointer only to create a local uninitialized 
> > shadowing variable seems like an error to me.
> It is allowed according to OpenMP 5.0. Private copy must be created in the 
> context of the parallel region, not the target region. So, for OpenMP 5.0 we 
> should not emit error here.
What does that mean and how does that affect my reasoning?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65019: [ARM] push LR before __gnu_mcount_nc

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

This seems better.

I'm not sure I follow why this needs special handling in 
SelectionDAGBuilder::visitIntrinsicCall, as opposed to just using 
ISD::INTRINSIC_VOID like other similar target-specific intrinsics.  (You can 
custom-lower INTRINSIC_VOID in ARMTargetLowering::LowerOperation, if that makes 
it easier.)

I'd just skip changing fast-isel; with an intrinsic, if fast-isel misses, we 
just fall back to the SelectionDAG code that does the right thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65019



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

[PATCH] D65835: [OpenMP] Fix map/is_device_ptr with DSA on combined directive

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_parallel_for_is_device_ptr_messages.cpp:93
   ;
-#pragma omp target parallel for private(ps) is_device_ptr(ps) // 
expected-error{{private variable cannot be in a is_device_ptr clause in 
'#pragma omp target parallel for' directive}} expected-note{{defined as 
private}}
+#pragma omp target parallel for private(ps) is_device_ptr(ps)
 for (int ii=0; ii<10; ii++)

jdoerfert wrote:
> I think this should cause an error or at least a warning. Telling the 
> compiler `ps` is a device pointer only to create a local uninitialized 
> shadowing variable seems like an error to me.
It is allowed according to OpenMP 5.0. Private copy must be created in the 
context of the parallel region, not the target region. So, for OpenMP 5.0 we 
should not emit error here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65835



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

[PATCH] D65889: [analyzer] CastValueChecker: Model castAs(), getAs()

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Cheers! :)


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

https://reviews.llvm.org/D65889



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