r266048 - [Clang][BuiltIn][avx512] Adding avx512 (shuf, sqrt{ss|sd}, rsqrt ) builtin to clang

[Michael Zuckerman via cfe-commits]

Author: mzuckerm
Date: Tue Apr 12 02:59:39 2016
New Revision: 266048

URL: http://llvm.org/viewvc/llvm-project?rev=266048&view=rev
Log:
[Clang][BuiltIn][avx512] Adding avx512 (shuf,sqrt{ss|sd},rsqrt ) builtin to 
clang

Modified:
cfe/trunk/include/clang/Basic/BuiltinsX86.def
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vlintrin.h
cfe/trunk/test/CodeGen/avx512f-builtins.c
cfe/trunk/test/CodeGen/avx512vl-builtins.c

Modified: cfe/trunk/include/clang/Basic/BuiltinsX86.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsX86.def?rev=266048&r1=266047&r2=266048&view=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsX86.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsX86.def Tue Apr 12 02:59:39 2016
@@ -1942,6 +1942,26 @@ TARGET_BUILTIN(__builtin_ia32_pternlogq1
 TARGET_BUILTIN(__builtin_ia32_pternlogq128_maskz, 
"V2LLiV2LLiV2LLiV2LLiIiUc","","avx512vl")
 TARGET_BUILTIN(__builtin_ia32_pternlogq256_mask, 
"V4LLiV4LLiV4LLiV4LLiIiUc","","avx512vl")
 TARGET_BUILTIN(__builtin_ia32_pternlogq256_maskz, 
"V4LLiV4LLiV4LLiV4LLiIiUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shuf_f32x4_mask, 
"V16fV16fV16fIiV16fUs","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_shuf_f64x2_mask, "V8dV8dV8dIiV8dUc","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_shuf_i32x4_mask, 
"V16iV16iV16iIiV16iUs","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_shuf_i64x2_mask, 
"V8LLiV8LLiV8LLiIiV8LLiUc","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_shufpd512_mask, "V8dV8dV8dIiV8dUc","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_shufps512_mask, 
"V16fV16fV16fIiV16fUs","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_shuf_f32x4_256_mask, 
"V8fV8fV8fIiV8fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shuf_f64x2_256_mask, 
"V4dV4dV4dIiV4dUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shuf_i32x4_256_mask, 
"V8iV8iV8iIiV8iUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shuf_i64x2_256_mask, 
"V4LLiV4LLiV4LLiIiV4LLiUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shufpd128_mask, "V2dV2dV2dIiV2dUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shufpd256_mask, "V4dV4dV4dIiV4dUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shufps128_mask, "V4fV4fV4fIiV4fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_shufps256_mask, "V8fV8fV8fIiV8fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_sqrtsd_round_mask, 
"V2dV2dV2dV2dUcIi","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_sqrtss_round_mask, 
"V4fV4fV4fV4fUcIi","","avx512f")
+TARGET_BUILTIN(__builtin_ia32_rsqrt14pd128_mask, "V2dV2dV2dUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rsqrt14pd256_mask, "V4dV4dV4dUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rsqrt14ps128_mask, "V4fV4fV4fUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_rsqrt14ps256_mask, "V8fV8fV8fUc","","avx512vl")
 
 #undef BUILTIN
 #undef TARGET_BUILTIN

Modified: cfe/trunk/lib/Headers/avx512fintrin.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/avx512fintrin.h?rev=266048&r1=266047&r2=266048&view=diff
==
--- cfe/trunk/lib/Headers/avx512fintrin.h (original)
+++ cfe/trunk/lib/Headers/avx512fintrin.h Tue Apr 12 02:59:39 2016
@@ -48,6 +48,96 @@ typedef unsigned short __mmask16;
 
 typedef enum
 {
+  _MM_PERM_ = 0x00, _MM_PERM_AAAB = 0x01, _MM_PERM_AAAC = 0x02,
+  _MM_PERM_AAAD = 0x03, _MM_PERM_AABA = 0x04, _MM_PERM_AABB = 0x05,
+  _MM_PERM_AABC = 0x06, _MM_PERM_AABD = 0x07, _MM_PERM_AACA = 0x08,
+  _MM_PERM_AACB = 0x09, _MM_PERM_AACC = 0x0A, _MM_PERM_AACD = 0x0B,
+  _MM_PERM_AADA = 0x0C, _MM_PERM_AADB = 0x0D, _MM_PERM_AADC = 0x0E,
+  _MM_PERM_AADD = 0x0F, _MM_PERM_ABAA = 0x10, _MM_PERM_ABAB = 0x11,
+  _MM_PERM_ABAC = 0x12, _MM_PERM_ABAD = 0x13, _MM_PERM_ABBA = 0x14,
+  _MM_PERM_ABBB = 0x15, _MM_PERM_ABBC = 0x16, _MM_PERM_ABBD = 0x17,
+  _MM_PERM_ABCA = 0x18, _MM_PERM_ABCB = 0x19, _MM_PERM_ABCC = 0x1A,
+  _MM_PERM_ABCD = 0x1B, _MM_PERM_ABDA = 0x1C, _MM_PERM_ABDB = 0x1D,
+  _MM_PERM_ABDC = 0x1E, _MM_PERM_ABDD = 0x1F, _MM_PERM_ACAA = 0x20,
+  _MM_PERM_ACAB = 0x21, _MM_PERM_ACAC = 0x22, _MM_PERM_ACAD = 0x23,
+  _MM_PERM_ACBA = 0x24, _MM_PERM_ACBB = 0x25, _MM_PERM_ACBC = 0x26,
+  _MM_PERM_ACBD = 0x27, _MM_PERM_ACCA = 0x28, _MM_PERM_ACCB = 0x29,
+  _MM_PERM_ACCC = 0x2A, _MM_PERM_ACCD = 0x2B, _MM_PERM_ACDA = 0x2C,
+  _MM_PERM_ACDB = 0x2D, _MM_PERM_ACDC = 0x2E, _MM_PERM_ACDD = 0x2F,
+  _MM_PERM_ADAA = 0x30, _MM_PERM_ADAB = 0x31, _MM_PERM_ADAC = 0x32,
+  _MM_PERM_ADAD = 0x33, _MM_PERM_ADBA = 0x34, _MM_PERM_ADBB = 0x35,
+  _MM_PERM_ADBC = 0x36, _MM_PERM_ADBD = 0x37, _MM_PERM_ADCA = 0x38,
+  _MM_PERM_ADCB = 0x39, _MM_PERM_ADCC = 0x3A, _MM_PERM_ADCD = 0x3B,
+  _MM_PERM_ADDA = 0x3C, _MM_PERM_ADDB = 0x3D, _MM_PERM_ADDC = 0x3E,
+  _MM_PERM_ADDD = 0x3F, _MM_PERM_BAAA = 0x40, _MM_PERM_BAAB = 0x41,
+  _MM_PERM_BAAC = 0x42, _MM_PERM_BAAD = 0x43, _MM_PERM_BABA = 0x44,
+  _MM_PERM_BABB = 0x45, _MM_PERM_BABC = 0x46, _MM_PER

Re: [PATCH] D18551: Added Fixer implementation and fix() interface in clang-format for removing redundant code.

[Eric Liu via cfe-commits]

ioeric updated this revision to Diff 53370.
ioeric added a comment.

- minor changes in checkEmptyNamespace.


http://reviews.llvm.org/D18551

Files:
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  unittests/Format/CMakeLists.txt
  unittests/Format/CleanupTest.cpp
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11274,6 +11274,35 @@
   Code, formatReplacements(Code, Replaces, Style)));
 }
 
+TEST_F(ReplacementTest, FixOnlyAffectedCodeAfterReplacements) {
+  std::string Code = "namespace A {\n"
+ "namespace B {\n"
+ "  int x;\n"
+ "} // namespace B\n"
+ "} // namespace A\n"
+ "\n"
+ "namespace C {\n"
+ "namespace D { int i; }\n"
+ "inline namespace E { namespace { int y; } }\n"
+ "int x= 0;"
+ "}";
+  std::string Expected = "\n\nnamespace C {\n"
+ "namespace D { int i; }\n\n"
+ "int x= 0;"
+ "}";
+  FileID ID = Context.createInMemoryFile("fix.cpp", Code);
+  tooling::Replacements Replaces;
+  Replaces.insert(tooling::Replacement(
+  Context.Sources, Context.getLocation(ID, 3, 3), 6, ""));
+  Replaces.insert(tooling::Replacement(
+  Context.Sources, Context.getLocation(ID, 9, 34), 6, ""));
+
+  format::FormatStyle Style = format::getLLVMStyle();
+  auto FinalReplaces = formatReplacements(
+  Code, cleanupAroundReplacements(Code, Replaces, Style), Style);
+  EXPECT_EQ(Expected, applyAllReplacements(Code, FinalReplaces));
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: unittests/Format/CleanupTest.cpp
===
--- /dev/null
+++ unittests/Format/CleanupTest.cpp
@@ -0,0 +1,85 @@
+//===- unittest/Format/CleanupTest.cpp - Code cleanup unit tests --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/Format/Format.h"
+
+#include "clang/Tooling/Core/Replacement.h"
+
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace format {
+namespace {
+
+class CleanupTest : public ::testing::Test {
+protected:
+  std::string cleanup(llvm::StringRef Code,
+  const std::vector &Ranges,
+  const FormatStyle &Style = getLLVMStyle()) {
+tooling::Replacements Replaces = format::cleanup(Style, Code, Ranges);
+
+std::string Result = applyAllReplacements(Code, Replaces);
+EXPECT_NE("", Result);
+return Result;
+  }
+};
+
+TEST_F(CleanupTest, DeleteEmptyNamespaces) {
+  std::string Code = "namespace A {\n"
+ "namespace B {\n"
+ "} // namespace B\n"
+ "} // namespace A\n\n"
+ "namespace C {\n"
+ "namespace D { int i; }\n"
+ "inline namespace E { namespace { } }\n"
+ "}";
+  std::string Expected = "\n\nnamespace C {\n"
+ "namespace D { int i; }\n\n"
+ "}";
+  std::vector Ranges;
+  Ranges.push_back(tooling::Range(28, 0));
+  Ranges.push_back(tooling::Range(91, 6));
+  Ranges.push_back(tooling::Range(132, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, NamespaceWithSyntaxError) {
+  std::string Code = "namespace A {\n"
+ "namespace B {\n" // missing r_brace
+ "} // namespace A\n\n"
+ "namespace C {\n"
+ "namespace D int i; }\n"
+ "inline namespace E { namespace { } }\n"
+ "}";
+  std::string Expected = "namespace A {\n"
+ "\n\nnamespace C {\n"
+ "namespace D int i; }\n\n"
+ "}";
+  std::vector Ranges(1, tooling::Range(0, Code.size()));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ(Expected, Result);
+}
+
+TEST_F(CleanupTest, EmptyNamespaceNotAffected) {
+  std::string Code = "namespace A {\n\n"
+ "namespace {\n\n}}";
+  // Even though the namespaces are empty, but the inner most empty namespace
+  // block is not affected by the changed ranges.
+  std::string Expected = "namespace A {\n\n"
+ "namespace {\n\n}}";
+  // Set the changed range to be the second "\n".
+  std::vector Ranges(1, tooling::Range(14, 0));
+  std::string Result = cleanup(Code, Ranges);
+  EXPECT_EQ

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53371.
pilki marked an inline comment as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is lacking a default constructor; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable nor movable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added inline comments.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:35
@@ +34,3 @@
+ this);
+  Finder->addMatcher(cxxMethodDecl(anyOf(isCopyAssignmentOperator(),
+ isMoveAssignmentOperator()),

alexfh wrote:
> It's not overly important, but you can further reduce the code by folding the 
> first matcher into the second one (move `cxxConstructorDecl()` inside `anyOf` 
> here, since `CXXConstructorDecl` is a `CXXMethodDecl`). You can also use just 
> a single id to bind the node and distinguish the constructor using 
> `dyn_cast`. Then you'll be able to use a single `diag()` call below and 
> remove the unneeded variables `Message` and `isBadlyDefaulted`.
Mmh, I'm not convinced it really helps with readability here. I can do it if 
you insist though.


http://reviews.llvm.org/D18961



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

Re: [PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

[Haojian Wu via cfe-commits]

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

Keep track all the fields of ClangTidyOptions.


http://reviews.llvm.org/D18694

Files:
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/Inputs/explain-config/.clang-tidy
  test/clang-tidy/explain-checks.cpp

Index: test/clang-tidy/explain-checks.cpp
===
--- /dev/null
+++ test/clang-tidy/explain-checks.cpp
@@ -0,0 +1,14 @@
+// RUN: clang-tidy -checks=-*,modernize-use-nullptr -explain-config | FileCheck --check-prefix=CHECK-MESSAGE1 %s
+// RUN: clang-tidy -config="{Checks: '-*,modernize-use-nullptr'}" -explain-config | FileCheck --check-prefix=CHECK-MESSAGE2 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -config="{Checks: '-*,modernize-use-nullptr'}" -explain-config | FileCheck --check-prefix=CHECK-MESSAGE3 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -config="{Checks: '-*,-modernize-use-nullptr'}" %S/Inputs/explain-config/a.cc -explain-config | FileCheck --check-prefix=CHECK-MESSAGE4 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -config="{Checks: '-*,modernize-*'}" -explain-config | FileCheck --check-prefix=CHECK-MESSAGE5 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -explain-config | FileCheck --check-prefix=CHECK-MESSAGE6 %s
+// RUN: clang-tidy -explain-config %S/Inputs/explain-config/a.cc | grep "'modernize-use-nullptr' is enabled in the %S/Inputs/explain-config/.clang-tidy."
+
+// CHECK-MESSAGE1: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE2: 'modernize-use-nullptr' is enabled in the command-line option '-config'.
+// CHECK-MESSAGE3: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE4: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE5: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE6: 'clang-analyzer-unix.API' is enabled in the clang-tidy binary.
Index: test/clang-tidy/Inputs/explain-config/.clang-tidy
===
--- /dev/null
+++ test/clang-tidy/Inputs/explain-config/.clang-tidy
@@ -0,0 +1 @@
+Checks: '-*,modernize-use-nullptr'
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -128,6 +128,12 @@
 )"),
 cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt ExplainConfig("explain-config", cl::desc(R"(
+for each enabled check explains, where it is enabled, i.e. in clang-tidy binary,
+command line or a specific configuration file.
+)"),
+   cl::init(false), cl::cat(ClangTidyCategory));
+
 static cl::opt Config("config", cl::desc(R"(
 Specifies a configuration in YAML/JSON format:
   -config="{Checks: '*',
@@ -179,6 +185,12 @@
 namespace clang {
 namespace tidy {
 
+static constexpr char CheckSourceTypeDefaultBinary[] = "clang-tidy binary";
+static constexpr char CheckSourceTypeCheckCommandLineOption[] =
+"command-line option '-checks'";
+static constexpr char CheckSourceTypeConfigCommandLineOption[] =
+"command-line option '-config'";
+
 static void printStats(const ClangTidyStats &Stats) {
   if (Stats.errorsIgnored()) {
 llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings (";
@@ -256,6 +268,7 @@
 
   ClangTidyOptions DefaultOptions;
   DefaultOptions.Checks = DefaultChecks;
+  DefaultOptions.Source = CheckSourceTypeDefaultBinary;
   DefaultOptions.WarningsAsErrors = "";
   DefaultOptions.HeaderFilterRegex = HeaderFilter;
   DefaultOptions.SystemHeaders = SystemHeaders;
@@ -266,8 +279,10 @@
 DefaultOptions.User = llvm::sys::Process::GetEnv("USERNAME");
 
   ClangTidyOptions OverrideOptions;
-  if (Checks.getNumOccurrences() > 0)
+  OverrideOptions.Source = CheckSourceTypeCheckCommandLineOption;
+  if (Checks.getNumOccurrences() > 0) {
 OverrideOptions.Checks = Checks;
+  }
   if (WarningsAsErrors.getNumOccurrences() > 0)
 OverrideOptions.WarningsAsErrors = WarningsAsErrors;
   if (HeaderFilter.getNumOccurrences() > 0)
@@ -280,6 +295,7 @@
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
 parseConfiguration(Config)) {
+  ParsedConfig->Source = CheckSourceTypeConfigCommandLineOption;
   return llvm::make_unique(
   GlobalOptions, ClangTidyOptions::getDefaults()
  .mergeWith(DefaultOptions)
@@ -311,6 +327,26 @@
   ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FileName);
   std::vector EnabledChecks = getCheckNames(EffectiveOptions);
 
+  if (ExplainConfig) {
+for (const std::string &Check : EnabledChecks) {
+  bool IsFindMatch = false;
+  for (auto It = EffectiveOptions.HigherPriorityOptions.rbegin();
+

Re: [PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

[Haojian Wu via cfe-commits]

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

In http://reviews.llvm.org/D18694#396797, @alexfh wrote:

> I've just realized that the approach is artificially limited to just keeping 
> track of the origin of the `Checks` option, while it could be easily extended 
> to track the origin of all configuration items. What if instead of 
> `std::vector CheckSources;` we introduce 
> `std::vector> Sources;`? Then we 
> could be able to find where a certain check option or an extra argument was 
> introduced, for example. WDYT?


Update the patch according to our offline discussion. One different thing is 
that we can't assume that `Other` in `merge(CTO& Other)` has empty 
`HigherPriorityOptions` because some usages 

 break this assumption.


http://reviews.llvm.org/D18694



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

Re: [PATCH] D18694: [ClangTidy] Add an 'explain-checks' option to diagnose where each checks comes from.

[Haojian Wu via cfe-commits]

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

Don't modify unrelevant code.


http://reviews.llvm.org/D18694

Files:
  clang-tidy/ClangTidyOptions.cpp
  clang-tidy/ClangTidyOptions.h
  clang-tidy/tool/ClangTidyMain.cpp
  test/clang-tidy/Inputs/explain-config/.clang-tidy
  test/clang-tidy/explain-checks.cpp

Index: test/clang-tidy/explain-checks.cpp
===
--- /dev/null
+++ test/clang-tidy/explain-checks.cpp
@@ -0,0 +1,14 @@
+// RUN: clang-tidy -checks=-*,modernize-use-nullptr -explain-config | FileCheck --check-prefix=CHECK-MESSAGE1 %s
+// RUN: clang-tidy -config="{Checks: '-*,modernize-use-nullptr'}" -explain-config | FileCheck --check-prefix=CHECK-MESSAGE2 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -config="{Checks: '-*,modernize-use-nullptr'}" -explain-config | FileCheck --check-prefix=CHECK-MESSAGE3 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -config="{Checks: '-*,-modernize-use-nullptr'}" %S/Inputs/explain-config/a.cc -explain-config | FileCheck --check-prefix=CHECK-MESSAGE4 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -config="{Checks: '-*,modernize-*'}" -explain-config | FileCheck --check-prefix=CHECK-MESSAGE5 %s
+// RUN: clang-tidy -checks=modernize-use-nullptr -explain-config | FileCheck --check-prefix=CHECK-MESSAGE6 %s
+// RUN: clang-tidy -explain-config %S/Inputs/explain-config/a.cc | grep "'modernize-use-nullptr' is enabled in the %S/Inputs/explain-config/.clang-tidy."
+
+// CHECK-MESSAGE1: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE2: 'modernize-use-nullptr' is enabled in the command-line option '-config'.
+// CHECK-MESSAGE3: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE4: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE5: 'modernize-use-nullptr' is enabled in the command-line option '-checks'.
+// CHECK-MESSAGE6: 'clang-analyzer-unix.API' is enabled in the clang-tidy binary.
Index: test/clang-tidy/Inputs/explain-config/.clang-tidy
===
--- /dev/null
+++ test/clang-tidy/Inputs/explain-config/.clang-tidy
@@ -0,0 +1 @@
+Checks: '-*,modernize-use-nullptr'
Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -128,6 +128,12 @@
 )"),
 cl::init(false), cl::cat(ClangTidyCategory));
 
+static cl::opt ExplainConfig("explain-config", cl::desc(R"(
+for each enabled check explains, where it is enabled, i.e. in clang-tidy binary,
+command line or a specific configuration file.
+)"),
+   cl::init(false), cl::cat(ClangTidyCategory));
+
 static cl::opt Config("config", cl::desc(R"(
 Specifies a configuration in YAML/JSON format:
   -config="{Checks: '*',
@@ -179,6 +185,12 @@
 namespace clang {
 namespace tidy {
 
+static constexpr char CheckSourceTypeDefaultBinary[] = "clang-tidy binary";
+static constexpr char CheckSourceTypeCheckCommandLineOption[] =
+"command-line option '-checks'";
+static constexpr char CheckSourceTypeConfigCommandLineOption[] =
+"command-line option '-config'";
+
 static void printStats(const ClangTidyStats &Stats) {
   if (Stats.errorsIgnored()) {
 llvm::errs() << "Suppressed " << Stats.errorsIgnored() << " warnings (";
@@ -256,6 +268,7 @@
 
   ClangTidyOptions DefaultOptions;
   DefaultOptions.Checks = DefaultChecks;
+  DefaultOptions.Source = CheckSourceTypeDefaultBinary;
   DefaultOptions.WarningsAsErrors = "";
   DefaultOptions.HeaderFilterRegex = HeaderFilter;
   DefaultOptions.SystemHeaders = SystemHeaders;
@@ -266,6 +279,7 @@
 DefaultOptions.User = llvm::sys::Process::GetEnv("USERNAME");
 
   ClangTidyOptions OverrideOptions;
+  OverrideOptions.Source = CheckSourceTypeCheckCommandLineOption;
   if (Checks.getNumOccurrences() > 0)
 OverrideOptions.Checks = Checks;
   if (WarningsAsErrors.getNumOccurrences() > 0)
@@ -280,6 +294,7 @@
   if (!Config.empty()) {
 if (llvm::ErrorOr ParsedConfig =
 parseConfiguration(Config)) {
+  ParsedConfig->Source = CheckSourceTypeConfigCommandLineOption;
   return llvm::make_unique(
   GlobalOptions, ClangTidyOptions::getDefaults()
  .mergeWith(DefaultOptions)
@@ -311,6 +326,28 @@
   ClangTidyOptions EffectiveOptions = OptionsProvider->getOptions(FileName);
   std::vector EnabledChecks = getCheckNames(EffectiveOptions);
 
+  if (ExplainConfig) {
+// FIXME: Figure out a more elegant way to show other ClangTidyOptions'
+// fields like ExtraArg.
+for (const std::string &Check : EnabledChecks) {
+  bool IsFindMatch = false;
+  for (auto It = EffectiveOptions.HigherPriorityOptions.rbegin();
+   It != EffectiveOptions.HigherPriorityOptions.rend(); ++It

r266052 - [OPENMP 4.0] Support for 'aligned' clause in 'declare simd' directive.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr 12 04:35:56 2016
New Revision: 266052

URL: http://llvm.org/viewvc/llvm-project?rev=266052&view=rev
Log:
[OPENMP 4.0] Support for 'aligned' clause in 'declare simd' directive.

The aligned clause declares that the object to which each list item points is 
aligned to the number of bytes expressed in the optional parameter of the 
aligned clause.
'aligned' '('  [ ':'  ] ')'
The optional parameter of the aligned clause, alignment, must be a constant 
positive integer expression. If no optional parameter is specified, 
implementation-defined default alignments for SIMD instructions on the target 
platforms are assumed.
The special this pointer can be used as if was one of the arguments to the 
function in any of the linear, aligned, or uniform clauses.

Modified:
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/OpenMP/declare_simd_ast_print.c
cfe/trunk/test/OpenMP/declare_simd_ast_print.cpp
cfe/trunk/test/OpenMP/declare_simd_messages.cpp

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=266052&r1=266051&r2=266052&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Apr 12 04:35:56 2016
@@ -2276,7 +2276,8 @@ def OMPDeclareSimdDecl : Attr {
 EnumArgument<"BranchState", "BranchStateTy",
  [ "", "inbranch", "notinbranch" ],
  [ "BS_Undefined", "BS_Inbranch", "BS_Notinbranch" ]>,
-ExprArgument<"Simdlen">, VariadicExprArgument<"Uniforms">
+ExprArgument<"Simdlen">, VariadicExprArgument<"Uniforms">,
+VariadicExprArgument<"Aligneds">, VariadicExprArgument<"Alignments">
   ];
   let AdditionalMembers = [{
 void printPrettyPragma(raw_ostream & OS, const PrintingPolicy &Policy)
@@ -2298,6 +2299,17 @@ def OMPDeclareSimdDecl : Attr {
 }
 OS << ") ";
   }
+  alignments_iterator NI = alignments_begin();
+  for (auto E : aligneds()) {
+OS << "aligned(";
+E->printPretty(OS, nullptr, Policy);
+if (*NI) {
+  OS << ": ";
+  (*NI)->printPretty(OS, nullptr, Policy);
+}
+OS << ") ";
+++NI;
+  }
 }
   }];
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=266052&r1=266051&r2=266052&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 12 04:35:56 
2016
@@ -7950,7 +7950,7 @@ def err_omp_aligned_expected_array_or_pt
   "%select{ or pointer|, pointer, reference to array or reference to pointer}1"
   ", not %0">;
 def err_omp_aligned_twice : Error<
-  "a variable cannot appear in more than one aligned clause">;
+  "%select{a variable|a parameter|'this'}0 cannot appear in more than one 
aligned clause">;
 def err_omp_local_var_in_threadprivate_init : Error<
   "variable with local storage in initial value of threadprivate variable">;
 def err_omp_loop_not_canonical_init : Error<

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=266052&r1=266051&r2=266052&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Apr 12 04:35:56 2016
@@ -8112,7 +8112,8 @@ public:
   /// the associated method/function.
   DeclGroupPtrTy ActOnOpenMPDeclareSimdDirective(
   DeclGroupPtrTy DG, OMPDeclareSimdDeclAttr::BranchStateTy BS,
-  Expr *Simdlen, ArrayRef Uniforms, SourceRange SR);
+  Expr *Simdlen, ArrayRef Uniforms, ArrayRef Aligneds,
+  ArrayRef Alignments, SourceRange SR);
 
   OMPClause *ActOnOpenMPSingleExprClause(OpenMPClauseKind Kind,
  Expr *Expr,

Modified: cfe/trunk/lib/Parse/ParseOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseOpenMP.cpp?rev=266052&r1=266051&r2=266052&view=diff
==
--- cfe/trunk/lib/Parse/ParseOpenMP.cpp (original)
+++ cfe/trunk/lib/Parse/ParseOpenMP.cpp Tue Apr 12 04:35:56 2016
@@ -329,63 +329,6 @@ Parser::ParseOpenMPDeclareReductionDirec
  IsCorrect);
 }
 
-/// Parses clauses for 'declare simd' directive.
-///clause:
-///  'inbranch' | 'notinbranch'
-///  'simdlen' '('  ')'
-///  {

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alexander Kornienko via cfe-commits]

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

Looks good. A couple of comments.

I'm happy to commit the patch for you once you answer the comments.

Thank you for the work!



Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:36
@@ +35,3 @@
+}
+
+void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) {

I would at least join matchers, since, I believe, it might be more effective 
that way. 

cxxMethodDecl(anyOf(cxxConstructorDecl().bind("constructor"),
 isCopyAssignmentOperator(),
 isMoveAssignmentOperator()),
   isBadlyDefaulted)
 .bind("assignment")

Optionally, you might want to inline the `isBadlyDefaulted` matcher (without 
the external `allOf` matcher).

As for restructuring the `check()` method, I don't insist.



Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:49
@@ +48,3 @@
+  "constructor";
+  return;
+}

If you don't feel strongly, I'd mildly suggest to turn `if { ... return }` to a 
chain of `if/else` (also the top-level ones). The code will become denser, but 
it will be easier to see the whole logic in a glance.


http://reviews.llvm.org/D18961



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

r266056 - [OPENMP 4.0] Support for 'linear' clause in 'declare simd' directive.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Apr 12 06:02:11 2016
New Revision: 266056

URL: http://llvm.org/viewvc/llvm-project?rev=266056&view=rev
Log:
[OPENMP 4.0] Support for 'linear' clause in 'declare simd' directive.

The linear clause declares one or more list items to be private to a SIMD lane 
and to have a linear relationship with respect to the iteration space of a loop.
'linear' '('  [ ':'  ] ')'
When a linear-step expression is specified in a linear clause it must be
either a constant integer expression or an integer-typed parameter that is 
specified in a uniform clause on the directive.
The special this pointer can be used as if was one of the arguments to the 
function in any of the linear, aligned, or uniform clauses.

Modified:
cfe/trunk/include/clang/AST/Attr.h
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp
cfe/trunk/test/OpenMP/declare_simd_ast_print.c
cfe/trunk/test/OpenMP/declare_simd_ast_print.cpp
cfe/trunk/test/OpenMP/declare_simd_messages.cpp

Modified: cfe/trunk/include/clang/AST/Attr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Attr.h?rev=266056&r1=266055&r2=266056&view=diff
==
--- cfe/trunk/include/clang/AST/Attr.h (original)
+++ cfe/trunk/include/clang/AST/Attr.h Tue Apr 12 06:02:11 2016
@@ -20,6 +20,7 @@
 #include "clang/AST/Type.h"
 #include "clang/Basic/AttrKinds.h"
 #include "clang/Basic/LLVM.h"
+#include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/Sanitizers.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/VersionTuple.h"

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=266056&r1=266055&r2=266056&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Tue Apr 12 06:02:11 2016
@@ -2277,7 +2277,9 @@ def OMPDeclareSimdDecl : Attr {
  [ "", "inbranch", "notinbranch" ],
  [ "BS_Undefined", "BS_Inbranch", "BS_Notinbranch" ]>,
 ExprArgument<"Simdlen">, VariadicExprArgument<"Uniforms">,
-VariadicExprArgument<"Aligneds">, VariadicExprArgument<"Alignments">
+VariadicExprArgument<"Aligneds">, VariadicExprArgument<"Alignments">,
+VariadicExprArgument<"Linears">, VariadicUnsignedArgument<"Modifiers">,
+VariadicExprArgument<"Steps">
   ];
   let AdditionalMembers = [{
 void printPrettyPragma(raw_ostream & OS, const PrintingPolicy &Policy)
@@ -2300,7 +2302,7 @@ def OMPDeclareSimdDecl : Attr {
 OS << ") ";
   }
   alignments_iterator NI = alignments_begin();
-  for (auto E : aligneds()) {
+  for (auto *E : aligneds()) {
 OS << "aligned(";
 E->printPretty(OS, nullptr, Policy);
 if (*NI) {
@@ -2310,6 +2312,23 @@ def OMPDeclareSimdDecl : Attr {
 OS << ") ";
 ++NI;
   }
+  steps_iterator I = steps_begin();
+  modifiers_iterator MI = modifiers_begin();
+  for (auto *E : linears()) {
+OS << "linear(";
+if (*MI != OMPC_LINEAR_unknown)
+  OS << getOpenMPSimpleClauseTypeName(OMPC_linear, *MI) << "(";
+E->printPretty(OS, nullptr, Policy);
+if (*MI != OMPC_LINEAR_unknown)
+  OS << ")";
+if (*I) {
+  OS << ": ";
+  (*I)->printPretty(OS, nullptr, Policy);
+}
+OS << ") ";
+++I;
+++MI;
+  }
 }
   }];
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=266056&r1=266055&r2=266056&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 12 06:02:11 
2016
@@ -8182,6 +8182,10 @@ def err_omp_variable_in_map_and_dsa : Er
   "%0 variable cannot be in a map clause in '#pragma omp %1' directive">;
 def err_omp_param_or_this_in_clause : Error<
   "expected reference to one of the parameters of function %0%select{| or 
'this'}1">;
+def err_omp_expected_uniform_param : Error<
+  "expected a reference to a parameter specified in a 'uniform' clause">;
+def err_omp_expected_int_param : Error<
+  "expected a reference to an integer-typed parameter">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=266056&r1=266055&r2=266056&view=diff

Re: [PATCH] D18474: [OPENMP] Enable correct generation of runtime call when target directive is separated from teams directive by multiple curly brackets

[Alexey Bataev via cfe-commits]

ABataev added inline comments.


Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4260-4273
@@ -4259,2 +4259,16 @@
 
+/// \brief look inside a Body stmt for a LF statement discarding any
+/// intervening CompoundStmt's
+template 
+const static LF *hasEnclosingOpenMPDirective(const Stmt *Body) {
+  const CompoundStmt *S = nullptr;
+  // keep iterating until we find an LF or not a CompoundStmt or a nullptr
+  while ((S = dyn_cast_or_null(Body)) &&
+ (!dyn_cast_or_null(Body)))
+  Body = S->body_front();
+
+  return (Body) ? dyn_cast_or_null(Body) :
+  nullptr;
+}
+
 /// \brief Emit the num_teams clause of an enclosed teams directive at the

I don't like this function at all. We need to add a template function. Each 
instantiation will produce a new function with almost the same body. MI prepfer 
to see another solution, like:
```
static const Stmt *ignoreCompoundStmts(const Stmt *Body) {
  while (auto *CS = dyn_cast_or_null(Body))
Body = CS->body_front();
  return Body;
}
...
if (auto *TeamsDir = 
dyn_cast(ignoreCompoundStmts(CS.getCapturedStmt( {
...
```



Repository:
  rL LLVM

http://reviews.llvm.org/D18474



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

Re: [PATCH] D18510: [cxx1z-constexpr-lambda] Make conversion function constexpr

[Faisal Vali via cfe-commits]

faisalv added a comment.

*ping*


http://reviews.llvm.org/D18510



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

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53381.
pilki marked an inline comment as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is lacking a default constructor; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable nor movable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted, probably because an instance variable or a base class is not copyable; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted, probably because a base class or an instance variable is not assignable, e.g. because the latter is marked 'const'; this definition should either be removed or explicitly marked as '= delete' [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is marked '= default' but is actually implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is marked '= default' but is actually implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is marked '= default' but is actually implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Michael Miller via cfe-commits]

michael_miller updated this revision to Diff 53380.
michael_miller marked 2 inline comments as done.
michael_miller added a comment.
Herald added a subscriber: klimek.

Moved isDelegatingConstructor and isUserProvided into 
clang/include/clang/ASTMatchers/ASTMatchers.h and updated the tests and docs.
Renamed getRecordDecl to getCanonicalRecordDecl to better match its behavior.


http://reviews.llvm.org/D18584

Files:
  clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
  clang-tidy/utils/Matchers.h
  clang-tidy/utils/TypeTraits.cpp
  clang-tidy/utils/TypeTraits.h
  docs/LibASTMatchersReference.html
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  include/clang/ASTMatchers/ASTMatchers.h
  test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
  unittests/ASTMatchers/ASTMatchersTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2327,6 +2327,37 @@
   cxxConstructorDecl(isMoveConstructor(;
 }
 
+TEST(ConstructorDeclaration, IsUserProvided) {
+  EXPECT_TRUE(notMatches("struct S { int X = 0; };",
+ cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(notMatches("struct S { S() = default; };",
+ cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(notMatches("struct S { S() = delete; };",
+ cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(
+  matches("struct S { S(); };", cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(matches("struct S { S(); }; S::S(){}",
+  cxxConstructorDecl(isUserProvided(;
+}
+
+TEST(DestructorDeclaration, MatchesVirtualDestructor) {
+  EXPECT_TRUE(matches("class Foo { virtual ~Foo(); };",
+  cxxDestructorDecl(ofClass(hasName("Foo");
+}
+
+TEST(ConstructorDeclaration, IsDelegatingConstructor) {
+  EXPECT_TRUE(notMatches("struct S { S(); S(int); int X; };",
+ cxxConstructorDecl(isDelegatingConstructor(;
+  EXPECT_TRUE(notMatches("struct S { S(){} S(int X) : X(X) {} int X; };",
+ cxxConstructorDecl(isDelegatingConstructor(;
+  EXPECT_TRUE(matches(
+  "struct S { S() : S(0) {} S(int X) : X(X) {} int X; };",
+  cxxConstructorDecl(isDelegatingConstructor(), parameterCountIs(0;
+  EXPECT_TRUE(matches(
+  "struct S { S(); S(int X); int X; }; S::S(int X) : S() {}",
+  cxxConstructorDecl(isDelegatingConstructor(), parameterCountIs(1;
+}
+
 TEST(DestructorDeclaration, MatchesVirtualDestructor) {
   EXPECT_TRUE(matches("class Foo { virtual ~Foo(); };",
   cxxDestructorDecl(ofClass(hasName("Foo");
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4911,6 +4911,38 @@
   return Node.isDefaultConstructor();
 }
 
+/// \brief Matches constructor declarations that are user-provided.
+///
+/// Given
+/// \code
+///   struct S {
+/// S(); // #1
+/// S(const S &) = default; // #2
+/// S(S &&) = delete; // #3
+///   };
+/// \endcode
+/// cxxConstructorDecl(isUserProvided()) will match #1, but not #2 or #3.
+AST_MATCHER(CXXConstructorDecl, isUserProvided) {
+  return Node.isUserProvided();
+}
+
+/// \brief Matches constructors that delegate to another constructor.
+///
+/// Given
+/// \code
+///   struct S {
+/// S(); // #1
+/// S(int) {} // #2
+/// S(S &&) : S() {} // #3
+///   };
+///   S::S() : S(0) {} // #4
+/// \endcode
+/// cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
+/// #1 or #2.
+AST_MATCHER(CXXConstructorDecl, isDelegatingConstructor) {
+  return Node.isDelegatingConstructor();
+}
+
 /// \brief Matches constructor and conversion declarations that are marked with
 /// the explicit keyword.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1799,6 +1799,21 @@
 
 
 
+MatcherCXXConstructorDecl>isDelegatingConstructor
+Matches constructors that delegate to another constructor.
+
+Given
+  struct S {
+S(); #1
+S(int) {} #2
+S(S &&) : S() {} #3
+  };
+  S::S() : S(0) {} #4
+cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
+#1 or #2.
+
+
+
 MatcherCXXConstructorDecl>isExplicit
 Matches constructor and conversion

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Michael Miller via cfe-commits]

michael_miller marked 3 inline comments as done.
michael_miller added a comment.

Sounds good! Updated with suggested changes.


http://reviews.llvm.org/D18584



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

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added inline comments.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:36
@@ +35,3 @@
+}
+
+void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) {

alexfh wrote:
> I would at least join matchers, since, I believe, it might be more effective 
> that way. 
> 
> cxxMethodDecl(anyOf(cxxConstructorDecl().bind("constructor"),
>  isCopyAssignmentOperator(),
>  isMoveAssignmentOperator()),
>isBadlyDefaulted)
>  .bind("assignment")
> 
> Optionally, you might want to inline the `isBadlyDefaulted` matcher (without 
> the external `allOf` matcher).
> 
> As for restructuring the `check()` method, I don't insist.
> 
I'm a bit confused by this suggestion. It will bind to "assignment" even when 
it is a constructor. It works because we get "constructor" first in check(), 
but I find the resulting contract between the check() and registerMatchers 
awkward.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:49
@@ +48,3 @@
+  "constructor";
+  return;
+}

alexfh wrote:
> If you don't feel strongly, I'd mildly suggest to turn `if { ... return }` to 
> a chain of `if/else` (also the top-level ones). The code will become denser, 
> but it will be easier to see the whole logic in a glance.
I thought that in the clang codebase, return were preferred to 'else'. Done.


http://reviews.llvm.org/D18961



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

r266057 - Remove unused diagnostics. NFC.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Tue Apr 12 06:49:52 2016
New Revision: 266057

URL: http://llvm.org/viewvc/llvm-project?rev=266057&view=rev
Log:
Remove unused diagnostics. NFC.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=266057&r1=266056&r2=266057&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Apr 12 06:49:52 2016
@@ -417,8 +417,6 @@ def err_pp_used_poisoned_id : Error<"att
 def err_feature_check_malformed : Error<
   "builtin feature check macro requires a parenthesized identifier">;
 
-def err_warning_check_malformed : Error<
-  "builtin warning check macro requires a parenthesized string">;
 def warn_has_warning_invalid_option :
ExtWarn<"__has_warning expected option name (e.g. \"-Wundef\")">,
InGroup;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=266057&r1=266056&r2=266057&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 12 06:49:52 
2016
@@ -6490,9 +6490,6 @@ def err_global_call_not_config : Error<
 def err_ref_bad_target : Error<
   "reference to %select{__device__|__global__|__host__|__host__ __device__}0 "
   "function %1 in %select{__device__|__global__|__host__|__host__ __device__}2 
function">;
-def warn_host_calls_from_host_device : Warning<
-  "calling __host__ function %0 from __host__ __device__ function %1 can lead 
to runtime errors">,
-  InGroup;
 def warn_kern_is_method : Extension<
   "kernel function %0 is a member function; this may not be accepted by nvcc">,
   InGroup;


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

Re: [PATCH] D18953: [ms][dll] #26935 Defining a dllimport function should cause it to be exported

[Andrew V. Tischenko via cfe-commits]

avt77 marked an inline comment as done.


Comment at: lib/Sema/SemaDecl.cpp:5570
@@ -5565,3 +5569,3 @@
   // and qualified friend declarations.
-  // NB: MSVC converts such a declaration to dllexport.
+  // NB: MSVC converts such a declaration to dllexport that's why we do it 
also.
   bool IsInline = false, IsStaticDataMember = false, IsQualifiedFriend = false;

rsmith wrote:
> Just describe this as the semantics of this situation rather than suggesting 
> this is some MSVC oddity we're emulating. "In such a case, the declaration is 
> treated as if it were marked dllexport." or similar.
> 
> It also seems bizarre for this behavior to apply for local extern 
> declarations and qualified friend declarations. Does the "dllimport gets 
> turned into dllexport" behavior actually only apply to the definition case? 
> And does the definition need to be inline?
Yes, I see "dllimport gets turned into dllexport" for definitions only. 
No, the definition should not be inline: 
   if (OldImportAttr && !HasNewAttr && **!IsInline**


Comment at: lib/Sema/SemaDecl.cpp:5595
@@ +5594,3 @@
+  // Replace DLLImportAttr with DLLExportAttr
+  OldDecl->dropAttr();
+  NewDecl->dropAttr();

rsmith wrote:
> Don't change attributes on a prior declaration; AST nodes should generally be 
> immutable once created (this would lose source fidelity, and break under 
> PCH/modules). Instead, make sure that anyone who looks at this gets the 
> attribute from the appropriate (most recent) declaration and only change the 
> attributes there.
 I don't understand how I could "make sure that anyone...". Please, clarify. 


Comment at: lib/Sema/SemaDecl.cpp:5596
@@ +5595,3 @@
+  OldDecl->dropAttr();
+  NewDecl->dropAttr();
+  OldDecl->addAttr(::new (S.Context) DLLExportAttr(

rsmith wrote:
> Is this really valid and treated as `__dllexport` if the new declaration 
> explicitly specifies `__dllimport` (rather than inheriting it)?
The new declaration does not have explicitly specified __dllimport attribute: 
if (OldImportAttr && **!HasNewAttr**. It's inherited.


Comment at: lib/Sema/SemaDecl.cpp:5600-5602
@@ +5599,5 @@
+  OldImportAttr->getSpellingListIndex()));
+  NewDecl->addAttr(::new (S.Context) DLLExportAttr(
+  NewImportAttr->getRange(), S.Context,
+  NewImportAttr->getSpellingListIndex()));
+} else {

rsmith wrote:
> The new attribute should be marked implicit.
What do you mean? Please, clarify.


http://reviews.llvm.org/D18953



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

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

[Aleksei Sidorin via cfe-commits]

a.sidorin updated this revision to Diff 53389.
a.sidorin added a comment.

Update to current master;
Fix a memory leak introduced in cf8ccff5: an array is allocated in ImportArray 
and never freed.


http://reviews.llvm.org/D14286

Files:
  include/clang/AST/Type.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp
  unittests/AST/CMakeLists.txt

Index: unittests/AST/CMakeLists.txt
===
--- unittests/AST/CMakeLists.txt
+++ unittests/AST/CMakeLists.txt
@@ -4,6 +4,7 @@
 
 add_clang_unittest(ASTTests
   ASTContextParentMapTest.cpp
+  ASTImporterTest.cpp
   ASTTypeTraitsTest.cpp
   ASTVectorTest.cpp
   CommentLexer.cpp
Index: unittests/AST/ASTImporterTest.cpp
===
--- /dev/null
+++ unittests/AST/ASTImporterTest.cpp
@@ -0,0 +1,470 @@
+//===- unittest/AST/ASTImporterTest.cpp - AST node import test ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// Tests for the correct import of AST nodes from one AST context to another.
+//
+//===--===//
+
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTImporter.h"
+#include "MatchVerifier.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace ast_matchers {
+
+using clang::tooling::newFrontendActionFactory;
+using clang::tooling::runToolOnCodeWithArgs;
+using clang::tooling::FrontendActionFactory;
+
+typedef std::vector StringVector;
+
+void getLangArgs(Language Lang, StringVector &Args) {
+  switch (Lang) {
+  case Lang_C:
+Args.insert(Args.end(), { "-x", "c", "-std=c99" });
+break;
+  case Lang_C89:
+Args.insert(Args.end(), { "-x", "c", "-std=c89" });
+break;
+  case Lang_CXX:
+Args.push_back("-std=c++98");
+break;
+  case Lang_CXX11:
+Args.push_back("-std=c++11");
+break;
+  case Lang_OpenCL:
+  case Lang_OBJCXX:
+break;
+  }
+}
+
+template
+testing::AssertionResult
+testImport(const std::string &FromCode, Language FromLang,
+   const std::string &ToCode, Language ToLang,
+   MatchVerifier &Verifier,
+   const MatcherType &AMatcher) {
+  StringVector FromArgs, ToArgs;
+  getLangArgs(FromLang, FromArgs);
+  getLangArgs(ToLang, ToArgs);
+
+  const char *const InputFileName = "input.cc";
+  const char *const OutputFileName = "output.cc";
+
+  std::unique_ptr
+  FromAST = tooling::buildASTFromCodeWithArgs(
+FromCode, FromArgs, InputFileName),
+  ToAST = tooling::buildASTFromCodeWithArgs(ToCode, ToArgs, OutputFileName);
+
+  ASTContext &FromCtx = FromAST->getASTContext(),
+  &ToCtx = ToAST->getASTContext();
+
+  // Add input.cc to virtual file system so importer can 'find' it
+  // while importing SourceLocations.
+  vfs::OverlayFileSystem *OFS = static_cast(
+ToCtx.getSourceManager().getFileManager().getVirtualFileSystem().get());
+  vfs::InMemoryFileSystem *MFS = static_cast(
+OFS->overlays_begin()->get());
+  MFS->addFile(InputFileName, 0,
+   llvm::MemoryBuffer::getMemBuffer(FromCode.c_str()));
+
+  ASTImporter Importer(ToCtx, ToAST->getFileManager(),
+   FromCtx, FromAST->getFileManager(), false);
+
+  IdentifierInfo *ImportedII = &FromCtx.Idents.get("declToImport");
+  assert(ImportedII && "Declaration with 'declToImport' name"
+   "should be specified in test!");
+  DeclarationName ImportDeclName(ImportedII);
+  SmallVector FoundDecls;
+  FromCtx.getTranslationUnitDecl()->localUncachedLookup(
+ImportDeclName, FoundDecls);
+
+  if (FoundDecls.size() != 1)
+return testing::AssertionFailure() << "Multiple declarations were found!";
+
+  auto Imported = Importer.Import(*FoundDecls.begin());
+  if (!Imported)
+return testing::AssertionFailure() << "Import failed, nullptr returned!";
+
+  // This should dump source locations and assert if some source locations
+  // were not imported
+  SmallString<1024> ImportChecker;
+  llvm::raw_svector_ostream ToNothing(ImportChecker);
+  ToCtx.getTranslationUnitDecl()->print(ToNothing);
+
+  return Verifier.match(Imported, AMatcher);
+}
+
+TEST(ImportExpr, ImportStringLiteral) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(testImport("void declToImport() { \"foo\"; }",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(
+   hasBody(
+ compoundStmt(
+   has(
+ stringLiteral(
+   hasType(
+  

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Aaron Ballman via cfe-commits]

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

LGTM with one minor correction (you can correct when committing it).



Comment at: unittests/ASTMatchers/ASTMatchersTest.cpp:2343
@@ +2342,3 @@
+
+TEST(DestructorDeclaration, MatchesVirtualDestructor) {
+  EXPECT_TRUE(matches("class Foo { virtual ~Foo(); };",

I think this test got duplicated accidentally; I don't see it removed as part 
of the diff.


http://reviews.llvm.org/D18584



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

Re: [PATCH] D18970: Add functions in ctype.h to builtin function database

[Aaron Ballman via cfe-commits]

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

LGTM, with one small nit.



Comment at: include/clang/Basic/Builtins.h:97
@@ -96,1 +96,3 @@
 
+  /// \brief Return true if this function has no side effects
+  bool isPure(unsigned ID) const {

Missing period at the end of the comment.


http://reviews.llvm.org/D18970



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

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

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

xazax.hun added a comment.

What is the status of this?

As far as I understand it is blocked on that there is no checker that we could 
use to test this function with unknown variables as indexes?


http://reviews.llvm.org/D16044



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

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Aaron Ballman via cfe-commits]

aaron.ballman added a subscriber: aaron.ballman.
aaron.ballman added a comment.

I share David's consideration for making this a compiler warning instead of a 
clang-tidy check. Perhaps it would make sense to gather some data using this 
check over some large code bases to see how often it triggers. If it's overly 
chatty (or we discover interesting usage scenarios), then leaving it as a 
clang-tidy check may make more sense.



Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:38
@@ +37,3 @@
+void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) {
+  const StringRef Message = "%0 is marked '= default' but is actually "
+"implicitly deleted, probably because %1; this "

This is a bit verbose. How about:

"%0 is explicitly defaulted but implicitly deleted, probably because %1; 
definition can either be removed or explicitly deleted"

?


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:47
@@ +46,3 @@
+  Diag << "default constructor"
+   << "an instance variable or a base class is lacking a default "
+  "constructor";

"non-static data member" instead of "instance variable"


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:55
@@ +54,3 @@
+  << "move constructor"
+  << "an instance variable or a base class is not copyable nor 
movable";
+}

s/not/neither


http://reviews.llvm.org/D18961



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

Re: [PATCH] D16183: Added CheckName field to YAML report

[Ilia Gromov via cfe-commits]

Elijah_Th added a comment.

alexfh,

OK. I'll take a look at apply-replacements and fix if needed, in the beginning 
of the next week!


http://reviews.llvm.org/D16183



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

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

[Alexander Droste via cfe-commits]

Alexander_Droste added a comment.

> What is the status of this?

> As far as I understand it is blocked on that there is no checker that we 
> could use to test this function with unknown variables as indexes?


Yes, this is blocked due to the MPI-Checker dependency. The best thing I can 
offer at the moment is to remove my tests and leave the coverage up to you. 
I think the question is if an initial commit of this function without tests 
would be tolerated, what would also remove the cyclic commit dependency.


http://reviews.llvm.org/D16044



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

Re: [PATCH] D18970: Add functions in ctype.h to builtin function database

[Taewook Oh via cfe-commits]

twoh updated this revision to Diff 53395.
twoh added a comment.

Addressing comments from @aaron.ballman. It would be great if someone can 
commit this patch for me, as I don't have the privilege yet.


http://reviews.llvm.org/D18970

Files:
  include/clang/Basic/Builtins.def
  include/clang/Basic/Builtins.h
  lib/Sema/SemaDecl.cpp
  test/FixIt/typo.m
  test/Sema/enable_if.c
  test/Sema/libbuiltins-ctype.c

Index: test/Sema/libbuiltins-ctype.c
===
--- test/Sema/libbuiltins-ctype.c
+++ test/Sema/libbuiltins-ctype.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -emit-llvm < %s | FileCheck %s
+
+int isalnum(int);
+int isalpha(int);
+int isblank(int);
+int iscntrl(int);
+int isdigit(int);
+int isgraph(int);
+int islower(int);
+int isprint(int);
+int ispunct(int);
+int isspace(int);
+int isupper(int);
+int isxdigit(int);
+int tolower(int);
+int toupper(int);
+
+void test(int x) {
+  // CHECK: call i32 @isalnum(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isalnum(x);
+  // CHECK: call i32 @isalpha(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isalpha(x);
+  // CHECK: call i32 @isblank(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isblank(x);
+  // CHECK: call i32 @iscntrl(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)iscntrl(x);
+  // CHECK: call i32 @isdigit(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isdigit(x);
+  // CHECK: call i32 @isgraph(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isgraph(x);
+  // CHECK: call i32 @islower(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)islower(x);
+  // CHECK: call i32 @isprint(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isprint(x);
+  // CHECK: call i32 @ispunct(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)ispunct(x);
+  // CHECK: call i32 @isspace(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isspace(x);
+  // CHECK: call i32 @isupper(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isupper(x);
+  // CHECK: call i32 @isxdigit(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)isxdigit(x);
+  // CHECK: call i32 @tolower(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)tolower(x);
+  // CHECK: call i32 @toupper(i32 {{%[0-9]+}}) [[NUW_RO_CALL:#[0-9]+]]
+  (void)toupper(x);
+}
+
+// CHECK: declare i32 @isalnum(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isalpha(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isblank(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @iscntrl(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isdigit(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isgraph(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @islower(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isprint(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @ispunct(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isspace(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isupper(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @isxdigit(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @tolower(i32) [[NUW_RO:#[0-9]+]]
+// CHECK: declare i32 @toupper(i32) [[NUW_RO:#[0-9]+]]
+
+// CHECK: attributes [[NUW_RO]] = { nounwind readonly{{.*}} }
+// CHECK: attributes [[NUW_RO_CALL]] = { nounwind readonly }
Index: test/Sema/enable_if.c
===
--- test/Sema/enable_if.c
+++ test/Sema/enable_if.c
@@ -72,8 +72,8 @@
   __attribute__((unavailable("'c' must have the value of an unsigned char or EOF")));
 
 void test3(int c) {
-  isdigit(c);
-  isdigit(10);
+  isdigit(c); // expected-warning{{ignoring return value of function declared with pure attribute}}
+  isdigit(10); // expected-warning{{ignoring return value of function declared with pure attribute}}
 #ifndef CODEGEN
   isdigit(-10);  // expected-error{{call to unavailable function 'isdigit': 'c' must have the value of an unsigned char or EOF}}
 #endif
Index: test/FixIt/typo.m
===
--- test/FixIt/typo.m
+++ test/FixIt/typo.m
@@ -113,8 +113,6 @@
   
 @end
 
-double *isupper(int);
-
 @interface Sub2 : Super
 - (int)method2;
 @end
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -273,16 +273,16 @@
 // so build a dependent node to describe the type.
 if (WantNontrivialTypeSourceInfo)
   return ActOnTypenameType(S, SourceLocation(), *SS, II, NameLoc).get();
-
+
 NestedNameSpecifierLoc QualifierLoc = SS->getWithLocInContext(Context);
 QualType T = CheckTypenameType(ETK_None, SourceLocation(), QualifierLoc,
II, NameLoc);
 return ParsedType::make(T);
   }
 
   return nullptr;
 }
-
+
 if (!LookupCtx->isDependentContext() &&
 RequireCompleteDeclContext(*SS, LookupCtx))
   return nullptr;
@@ -303,7 +303,7 @@
 if (ObjectTypePtr && Result.empty()) {
   // C++ [basic.lookup.classref]p3:
   //   If the unqualified

Re: [PATCH] D18963: PR27216: Only define __ARM_FEATURE_FMA when the target has VFPv4

[silviu.bara...@arm.com via cfe-commits]

sbaranga added inline comments.


Comment at: test/Sema/arm_vfma.c:1
@@ -1,2 +1,2 @@
-// RUN: %clang_cc1 -triple thumbv7s-apple-ios7.0 -target-feature +neon 
-fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple thumbv7s-apple-ios7.0 -target-feature +neon 
-target-feature +vfp4 -fsyntax-only -verify %s
 #include 

t.p.northover wrote:
> rengolin wrote:
> > t.p.northover wrote:
> > > v7s is Swift, which has FMA. v7 for us is Cortex-A9, which I think also 
> > > has FMA (not that it matters much these days).
> > v7 is Cortex-A8, and neither A8 nor A9 have FMA in VFP, only NEON.
> > 
> > Does Swift have FMA in VFP? or just NEON?
> Sorry, it appears virtually every part of my statement was wrong then. v7 
> really does seem to be Cortex-A8 even for us, and Swift doesn't have scalar 
> VFMA.
The error seems to be coming from how the getDefaultFPU() is called when the 
cpu is not specified. It turns out that it gets called with an empty CPU string 
(perhaps we meant to call with either "generic" or the CPU that was set in 
ARMTargetInfo (which does get correctly recognized as swift in this case).

FWIW, Cortex-A9 doesn't have FMA,


http://reviews.llvm.org/D18963



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

Re: [PATCH] D18914: [clang-tidy] new readability-redundant-inline

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.


Comment at: clang-tidy/readability/RedundantInlineCheck.cpp:68
@@ +67,3 @@
+  }
+  llvm_unreachable("InlineTok() did not encounter the 'inline' token");
+}

This is still reachable.
The token might be hidden through macros, for example.
But even then we still can keep a simple API. The possible return values are a 
token or 'not found'.
Optional should be ok.


http://reviews.llvm.org/D18914



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

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

[Samuel Benzaquen via cfe-commits]

sbenza updated this revision to Diff 53398.
sbenza marked an inline comment as done.
sbenza added a comment.

Change warning message.


http://reviews.llvm.org/D18766

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/MultipleStatementMacroCheck.cpp
  clang-tidy/misc/MultipleStatementMacroCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-multiple-statement-macro.rst
  test/clang-tidy/misc-multiple-statement-macro.cpp

Index: test/clang-tidy/misc-multiple-statement-macro.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-multiple-statement-macro.cpp
@@ -0,0 +1,85 @@
+// RUN: %check_clang_tidy %s misc-multiple-statement-macro %t
+
+void F();
+
+#define BAD_MACRO(x) \
+  F();   \
+  F()
+
+#define GOOD_MACRO(x) \
+  do {\
+F();  \
+F();  \
+  } while (0)
+
+#define GOOD_MACRO2(x) F()
+
+#define GOOD_MACRO3(x) F();
+
+#define MACRO_ARG_MACRO(X) \
+  if (54)  \
+  X(2)
+
+#define ALL_IN_MACRO(X) \
+  if (43)   \
+F();\
+  F()
+
+#define GOOD_NESTED(x)   \
+  if (x)\
+GOOD_MACRO3(x); \
+  F();
+
+#define IF(x) if(x)
+
+void positives() {
+  if (1)
+BAD_MACRO(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used without braces; some statements will be unconditionally executed [misc-multiple-statement-macro]
+  if (1) {
+  } else
+BAD_MACRO(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used
+  while (1)
+BAD_MACRO(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used
+  for (;;)
+BAD_MACRO(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: multiple statement macro used
+
+  MACRO_ARG_MACRO(BAD_MACRO);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: multiple statement macro used
+  MACRO_ARG_MACRO(F(); int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: multiple statement macro used
+  IF(1) BAD_MACRO(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: multiple statement macro used
+}
+
+void negatives() {
+  if (1) {
+BAD_MACRO(1);
+  } else {
+BAD_MACRO(1);
+  }
+  while (1) {
+BAD_MACRO(1);
+  }
+  for (;;) {
+BAD_MACRO(1);
+  }
+
+  if (1)
+GOOD_MACRO(1);
+  if (1) {
+GOOD_MACRO(1);
+  }
+  if (1)
+GOOD_MACRO2(1);
+  if (1)
+GOOD_MACRO3(1);
+
+  MACRO_ARG_MACRO(GOOD_MACRO);
+  ALL_IN_MACRO(1);
+
+  IF(1) GOOD_MACRO(1);
+}
Index: docs/clang-tidy/checks/misc-multiple-statement-macro.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/misc-multiple-statement-macro.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - misc-multiple-statement-macro
+
+misc-multiple-statement-macro
+=
+
+Detect multiple statement macros that are used in unbraced conditionals.
+Only the first statement of the macro will be inside the conditional and the other ones will be executed unconditionally.
+
+Example:
+
+.. code:: c++
+
+  #define INCREMENT_TWO(x, y) (x)++; (y)++
+  if (do_increment)
+INCREMENT_TWO(a, b);  // `(b)++;` will be executed unconditionally.
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -60,6 +60,7 @@
misc-macro-repeated-side-effects
misc-misplaced-widening-cast
misc-move-constructor-init
+   misc-multiple-statement-macro
misc-new-delete-overloads
misc-noexcept-move-constructor
misc-non-copyable-objects
Index: clang-tidy/misc/MultipleStatementMacroCheck.h
===
--- /dev/null
+++ clang-tidy/misc/MultipleStatementMacroCheck.h
@@ -0,0 +1,37 @@
+//===--- MultipleStatementMacroCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MULTIPLE_STATEMENT_MACRO_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MULTIPLE_STATEMENT_MACRO_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Detect multiple statement macros that are used in unbraced conditionals.
+/// Only the first statement of the macro will be inside the conditional and the
+/// other ones will be executed unconditionally.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-multiple-statement-macro.html
+class MultipleStatementMacroCheck : public ClangTidyCheck {
+public:
+  MultipleStatementMacroCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

[Samuel Benzaquen via cfe-commits]

sbenza added inline comments.


Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:99
@@ +98,3 @@
+
+  diag(InnerRanges.back().first, "multiple statement macro spans unbraced "
+ "conditional and the following statement");

alexfh wrote:
> If I saw this message from a tool, I wouldn't get what it means right away. 
> Can you come up with an easier to read alternative? I can only suggest 
> `multiple statement macro used without braces`, but maybe a more 
> self-documenting message comes to your mind.
Sure.
I wrote my methodology to find the problem, not the actual problem.
What about this?


http://reviews.llvm.org/D18766



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

r266079 - Moving clang-test-depends into the Clang tests folder and moving vtables_blacklist into the Misc folder; NFC, this simply cleans up the generated solution so that these targets don't live in

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Apr 12 10:09:17 2016
New Revision: 266079

URL: http://llvm.org/viewvc/llvm-project?rev=266079&view=rev
Log:
Moving clang-test-depends into the Clang tests folder and moving 
vtables_blacklist into the Misc folder; NFC, this simply cleans up the 
generated solution so that these targets don't live in the root folder of the 
IDE.

Modified:
cfe/trunk/runtime/CMakeLists.txt
cfe/trunk/test/CMakeLists.txt

Modified: cfe/trunk/runtime/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/runtime/CMakeLists.txt?rev=266079&r1=266078&r2=266079&view=diff
==
--- cfe/trunk/runtime/CMakeLists.txt (original)
+++ cfe/trunk/runtime/CMakeLists.txt Tue Apr 12 10:09:17 2016
@@ -156,6 +156,7 @@ add_custom_command(OUTPUT ${dst}
COMMAND ${CMAKE_COMMAND} -E copy_if_different ${src} ${dst}
COMMENT "Copying vtables blacklist")
 add_custom_target(vtables_blacklist DEPENDS ${dst})
+set_target_properties(vtables_blacklist PROPERTIES FOLDER "Misc")
 if(TARGET clang)
   add_dependencies(clang vtables_blacklist)
 endif()

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=266079&r1=266078&r2=266079&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Tue Apr 12 10:09:17 2016
@@ -73,6 +73,7 @@ if( NOT CLANG_BUILT_STANDALONE )
 endif()
 
 add_custom_target(clang-test-depends DEPENDS ${CLANG_TEST_DEPS})
+set_target_properties(clang-test-depends PROPERTIES FOLDER "Clang tests")
 
 add_lit_testsuite(check-clang "Running the Clang regression tests"
   ${CMAKE_CURRENT_BINARY_DIR}


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

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki added a comment.

Fixed all the comment but the one about merging the two matchers.


http://reviews.llvm.org/D18961



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

[PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb created this revision.
etienneb added a reviewer: alexfh.
etienneb added a subscriber: cfe-commits.

This check found suspicious cases of sizeof expression.

Sizeof expression is retuning the size (in bytes) of a type or an
expression. Programmers often abuse of misuse this expression.

This checker is adding common set of patterns to detect some
of these bad constructs.


Some examples found by this checker:

R/packages/ifultools/ifultools/src/fra_neig.c
```
/* free buffer memory */
(void) mutil_free( dist_buff, sizeof( ctr * sizeof( double ) ) );
(void) mutil_free( nidx_buff, sizeof( ctr * sizeof( sint32 ) ) );
```


graphviz/v2_20_2/lib/common/utils.c
```
static Dtdisc_t mapDisc = {
offsetof(item, p),
sizeof(2 * sizeof(void *)),
offsetof(item, link),
(Dtmake_f) newItem,
(Dtfree_f) freeItem,
(Dtcompar_f) cmpItem,
NIL(Dthash_f),
NIL(Dtmemory_f),
NIL(Dtevent_f)
};
```


mDNSResponder/mDNSShared/dnsextd.c
```
context = ( TCPContext* ) malloc( sizeof( TCPContext ) );
require_action( context, exit, err = mStatus_NoMemoryErr; LogErr( 
"AcceptTCPConnection", "malloc" ) );
mDNSPlatformMemZero( context, sizeof( sizeof( TCPContext ) ) );
context->d   = self;
```

http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,179 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; sizes are incompatible
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; sizes are incompatible
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; sizes are incompatible
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; sizes are incompatible
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspic

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alex Pilkiewicz via cfe-commits]

pilki updated this revision to Diff 53404.
pilki marked 3 inline comments as done.

http://reviews.llvm.org/D18961

Files:
   clang-tidy/readability/CMakeLists.txt
   clang-tidy/readability/DeletedDefaultCheck.cpp
   clang-tidy/readability/DeletedDefaultCheck.h
   clang-tidy/readability/ReadabilityTidyModule.cpp
   docs/ReleaseNotes.rst
   docs/clang-tidy/checks/list.rst
   docs/clang-tidy/checks/readability-deleted-default.rst
   test/clang-tidy/readability-deleted-default.cpp

Index:  test/clang-tidy/readability-deleted-default.cpp
===
---  test/clang-tidy/readability-deleted-default.cpp
+++  test/clang-tidy/readability-deleted-default.cpp
@@ -0,0 +1,127 @@
+// RUN: %check_clang_tidy %s readability-deleted-default %t
+
+class NoDefault {
+public:
+  NoDefault() = delete;
+  NoDefault(NoDefault &&Other) = delete;
+  NoDefault(const NoDefault &Other) = delete;
+};
+
+class MissingEverything {
+public:
+  MissingEverything() = default;
+  // CHECK-MESSAGES: warning: default constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is lacking a default constructor; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is neither copyable nor movable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted, probably because a non-static data member or a base class is not copyable; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(MissingEverything &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+  MissingEverything &operator=(const MissingEverything &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted, probably because a base class or a non-static data member is not assignable, e.g. because the latter is marked 'const'; definition can either be removed or explicitly deleted [readability-deleted-default]
+
+private:
+  NoDefault ND;
+};
+
+class NotAssignable {
+public:
+  NotAssignable(NotAssignable &&Other) = default;
+  NotAssignable(const NotAssignable &Other) = default;
+  NotAssignable &operator=(NotAssignable &&Other) = default;
+  // CHECK-MESSAGES: warning: move assignment operator is explicitly defaulted but implicitly deleted
+  NotAssignable &operator=(const NotAssignable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+
+private:
+  const int I = 0;
+};
+
+class Movable {
+public:
+  Movable() = default;
+  Movable(Movable &&Other) = default;
+  Movable(const Movable &Other) = delete;
+  Movable &operator=(Movable &&Other) = default;
+  Movable &operator=(const Movable &Other) = delete;
+};
+
+class NotCopyable {
+public:
+  NotCopyable(NotCopyable &&Other) = default;
+  NotCopyable(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy constructor is explicitly defaulted but implicitly deleted
+  NotCopyable &operator=(NotCopyable &&Other) = default;
+  NotCopyable &operator=(const NotCopyable &Other) = default;
+  // CHECK-MESSAGES: warning: copy assignment operator is explicitly defaulted but implicitly deleted
+private:
+  Movable M;
+};
+
+template  class Templated {
+public:
+  // No warning here, it is a templated class.
+  Templated() = default;
+  Templated(Templated &&Other) = default;
+  Templated(const Templated &Other) = default;
+  Templated &operator=(Templated &&Other) = default;
+  Templated &operator=(const Templated &Other) = default;
+
+  class InnerTemplated {
+  public:
+// This class is not in itself templated, but we still don't have warning.
+InnerTemplated() = default;
+InnerTemplated(InnerTemplated &&Other) = default;
+InnerTemplated(const InnerTemplated &Other) = default;
+InnerTemplated &operator=(InnerTemplated &&Other) = default;
+InnerTemplated &operator=(const InnerTemplated &Other) = default;
+
+  private:
+T TVar;
+  };
+
+  class InnerNotTemplated {
+  public:
+// This one could technically have warnings, but currently doesn't.
+InnerNotTemplated() = default;
+InnerNotTemplated(InnerNotTemplated &&Other) = default;
+InnerNotTemplated(const InnerNotTemplated &Other) = default

Re: [PATCH] D18700: [Inline asm][GCC compatibility] Handle %v-prefixed code in inline assembly

[Denis Zobnin via cfe-commits]

d.zobnin.bugzilla updated this revision to Diff 53400.
d.zobnin.bugzilla added a comment.

Eric, thank you for the review!

I've renamed the test, but have also made it a .cpp file for the reasons 
described below.

You are right -- GCC looks at "target" attribute applied to the function in 
which inline asm exists. But GCC seems to take into account lambda's attributes 
as well, if asm statement is inside a lambda. To be correct, GCC seems to 
collect all target features from command line, function and lambda's 
attributes, so such code:

  void f8(void *x) __attribute__((__target__("no-avx")));
  void f8(void *x) {
auto g1 = [](void *arg) __attribute__((__target__("avx2"))) {
  auto g2 = [&arg]() __attribute__((__target__("no-mmx"))) {
int a = 10, b;
__asm__ ("%vmovq (%%rbp), %%xmm0" : "=r"(b) : "r"(a) : "%eax");
  };
  g2();
};
g1(x);
  }

will produce "vmovq" instruction regardless of command line arguments because 
of g1's attribute.

I tried several approaches to implement this behavior and came up with this 
patch, please take a look!


http://reviews.llvm.org/D18700

Files:
  include/clang/AST/Stmt.h
  lib/AST/Stmt.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/CodeGen/avx-v-modifier-inline-asm.cpp

Index: lib/AST/Stmt.cpp
===
--- lib/AST/Stmt.cpp
+++ lib/AST/Stmt.cpp
@@ -632,6 +632,14 @@
 
   CurPtr = NameEnd+1;
   continue;
+} else if (*Begin == 'v') {
+  // GCC accepts code staring with "%v", e. g. "%vpcmpestri" and transforms
+  // it into "vpcmpestri" instruction if target processor supports AVX and
+  // into "pcmpestri" otherwise.
+  if (SupportsAVX)
+CurStringPiece = "v" + CurStringPiece;
+  CurStringPiece += EscapedChar;
+  continue;
 }
 
 DiagOffs = CurPtr-StrStart-1;
@@ -682,13 +690,15 @@
 //===--===//
 
 GCCAsmStmt::GCCAsmStmt(const ASTContext &C, SourceLocation asmloc,
-   bool issimple, bool isvolatile, unsigned numoutputs,
-   unsigned numinputs, IdentifierInfo **names,
-   StringLiteral **constraints, Expr **exprs,
-   StringLiteral *asmstr, unsigned numclobbers,
-   StringLiteral **clobbers, SourceLocation rparenloc)
-  : AsmStmt(GCCAsmStmtClass, asmloc, issimple, isvolatile, numoutputs,
-numinputs, numclobbers), RParenLoc(rparenloc), AsmStr(asmstr) {
+   bool issimple, bool isvolatile, bool supportsavx,
+   unsigned numoutputs, unsigned numinputs,
+   IdentifierInfo **names, StringLiteral **constraints,
+   Expr **exprs, StringLiteral *asmstr,
+   unsigned numclobbers, StringLiteral **clobbers,
+   SourceLocation rparenloc)
+: AsmStmt(GCCAsmStmtClass, asmloc, issimple, isvolatile, numoutputs,
+  numinputs, numclobbers),
+  RParenLoc(rparenloc), AsmStr(asmstr), SupportsAVX(supportsavx) {
 
   unsigned NumExprs = NumOutputs + NumInputs;
 
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -138,6 +138,75 @@
   return false;
 }
 
+// Collect all target features specified for the current context by the "target"
+// attribute. Start from current DeclContext and iterate its parents until we
+// find a function context.
+static void CollectTargetFeatures(Sema &S,
+  std::vector &TargetFeatures,
+  StringRef &TargetCPU) {
+  DeclContext *DC = S.CurContext;
+
+  auto Collect = [&TargetFeatures, &TargetCPU](const FunctionDecl *FD) {
+if (!FD)
+  return;
+if (const auto *TA = FD->getAttr()) {
+  TargetAttr::ParsedTargetAttr ParsedAttr = TA->parse();
+  TargetFeatures.insert(TargetFeatures.begin(), ParsedAttr.first.begin(),
+ParsedAttr.first.end());
+  if (TargetCPU == "")
+TargetCPU = ParsedAttr.second;
+}
+  };
+
+  // Climb up the tree of nested DeclContext-s, collecting the target features.
+  while (true) {
+if (isa(DC) || isa(DC) || isa(DC)) {
+  DC = DC->getParent();
+} else if (const auto *MD = dyn_cast(DC)) {
+  // Check if it's a lambda and collect its target features.
+  if (MD->getOverloadedOperator() == OO_Call &&
+  cast(DC->getParent())->isLambda()) {
+Collect(MD);
+  }
+  DC = DC->getParent()->getParent();
+} else {
+  // It's the nearest function, collect its features and break.
+  Collect(dyn_cast(DC));
+  break;
+}
+  }
+}
+
+// Check whether a particular target feature is enabled in current context,
+// either by command-line options or by __attribute__((__target__("..."))),
+// applied to any surrounding lambda or func

Re: [PATCH] D18963: PR27216: Only define __ARM_FEATURE_FMA when the target has VFPv4

[silviu.bara...@arm.com via cfe-commits]

sbaranga updated this revision to Diff 53409.
sbaranga added a comment.

If no cpu has been passed to the command line, use the generic cpu when 
selecting
features/FPU, instead of using an empty string (which is not recognized by the
TargetParser).


http://reviews.llvm.org/D18963

Files:
  lib/Basic/Targets.cpp
  test/CodeGen/arm-long-calls.c
  test/CodeGen/arm-neon-fma.c
  test/CodeGen/arm-no-movt.c
  test/Preprocessor/arm-acle-6.5.c
  test/Sema/arm_vfma.c
  test/Sema/neon-vector-types-support.c

Index: lib/Basic/Targets.cpp
===
--- lib/Basic/Targets.cpp
+++ lib/Basic/Targets.cpp
@@ -4707,6 +4707,8 @@
   initFeatureMap(llvm::StringMap &Features, DiagnosticsEngine &Diags,
  StringRef CPU,
  const std::vector &FeaturesVec) const override {
+if (CPU == "")
+  CPU = "generic";
 
 std::vector TargetFeatures;
 unsigned Arch = llvm::ARM::parseArch(getTriple().getArchName());
@@ -4927,7 +4929,7 @@
 Builder.defineMacro("__ARM_FP16_ARGS", "1");
 
 // ACLE 6.5.3 Fused multiply-accumulate (FMA)
-if (ArchVersion >= 7 && (CPUProfile != "M" || CPUAttr == "7EM"))
+if (ArchVersion >= 7 && (FPU & VFP4FPU))
   Builder.defineMacro("__ARM_FEATURE_FMA", "1");
 
 // Subtarget options.
Index: test/CodeGen/arm-no-movt.c
===
--- test/CodeGen/arm-no-movt.c
+++ test/CodeGen/arm-no-movt.c
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple thumbv7-apple-ios5  -target-feature +no-movt -emit-llvm -o - %s | FileCheck -check-prefix=NO-MOVT %s
+// RUN: %clang_cc1 -triple thumbv7-apple-ios5 -target-feature +no-movt -emit-llvm -o - %s | FileCheck -check-prefix=NO-MOVT %s
 // RUN: %clang_cc1 -triple thumbv7-apple-ios5 -emit-llvm -o - %s | FileCheck -check-prefix=MOVT %s
 
-// NO-MOVT: attributes #0 = { {{.*}} "target-features"="+no-movt"
-// MOVT-NOT: attributes #0 = { {{.*}} "target-features"="+no-movt"
+// NO-MOVT: attributes #0 = { {{.*}} "target-features"="{{.*}}+no-movt{{.*}}"
+// MOVT-NOT: attributes #0 = { {{.*}} "target-features"="{{.*}}+no-movt{{.*}}"
 
 int foo1(int a) { return a; }
Index: test/CodeGen/arm-long-calls.c
===
--- test/CodeGen/arm-long-calls.c
+++ test/CodeGen/arm-long-calls.c
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -triple thumbv7-apple-ios5  -target-feature +long-calls -emit-llvm -o - %s | FileCheck -check-prefix=LONGCALL %s
 // RUN: %clang_cc1 -triple thumbv7-apple-ios5 -emit-llvm -o - %s | FileCheck -check-prefix=NOLONGCALL %s
 
-// LONGCALL: attributes #0 = { {{.*}} "target-features"="+long-calls"
-// NOLONGCALL-NOT: attributes #0 = { {{.*}} "target-features"="+long-calls"
+// LONGCALL: attributes #0 = { {{.*}} "target-features"="{{.*}}+long-calls{{.*}}"
+// NOLONGCALL-NOT: attributes #0 = { {{.*}} "target-features"="{{.*}}+long-calls{{.*}}"
 
 int foo1(int a) { return a; }
Index: test/CodeGen/arm-neon-fma.c
===
--- test/CodeGen/arm-neon-fma.c
+++ test/CodeGen/arm-neon-fma.c
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -triple thumbv7-none-linux-gnueabihf \
 // RUN:   -target-abi aapcs \
-// RUN:   -target-cpu cortex-a8 \
+// RUN:   -target-cpu cortex-a7 \
 // RUN:   -mfloat-abi hard \
 // RUN:   -ffreestanding \
 // RUN:   -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
Index: test/Preprocessor/arm-acle-6.5.c
===
--- test/Preprocessor/arm-acle-6.5.c
+++ test/Preprocessor/arm-acle-6.5.c
@@ -49,10 +49,13 @@
 
 // CHECK-NO-FMA-NOT: __ARM_FEATURE_FMA
 
-// RUN: %clang -target armv7a-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
-// RUN: %clang -target armv7r-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
+// RUN: %clang -target armv7a-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-NO-FMA
+// RUN: %clang -target armv7a-eabi -mfpu=vfpv4 -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
+// RUN: %clang -target armv7r-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-NO-FMA
+// RUN: %clang -target armv7r-eabi -mfpu=vfpv4 -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
 // RUN: %clang -target armv7em-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
-// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
+// RUN: %clang -target armv8-eabi -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-NO-FMA
+// RUN: %clang -target armv8-eabi -mfpu=vfpv4 -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-FMA
 
 // CHECK-FMA: __ARM_FEATURE_FMA 1
 
Index: test/Sema/arm_vfma.c
===
--- test/Sema/arm_vfma.c
+++ test/Sema/arm_vfma.c
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple thumbv7s-apple-ios7.0 -target-feature +neon -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple t

Re: [PATCH] D18963: PR27216: Only define __ARM_FEATURE_FMA when the target has VFPv4

[silviu.bara...@arm.com via cfe-commits]

sbaranga added a comment.

I've updated the patch to fix the defaults when the cpu is not specified. 
Renato, Tim, could you have a look at this again please?

Thanks,
Silviu


http://reviews.llvm.org/D18963



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

The check is really awesome! Thank you for coming up with all the patterns that 
frequently happen to result from coding errors!

Please update release notes.

A few inline comments as well.



Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:22
@@ +21,3 @@
+
+AST_MATCHER(QualType, isAnyChar) {
+  return Node->isCharType() || Node->isWideCharType() || Node->isChar16Type() 
||

Seems like you could use the `isAnyCharacter` matcher.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:27
@@ +26,3 @@
+
+AST_MATCHER(BinaryOperator, isRelationnalOperator) {
+  return Node.isRelationalOp();

nit: extra 'n' in "relational"


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:27
@@ +26,3 @@
+
+AST_MATCHER(BinaryOperator, isRelationnalOperator) {
+  return Node.isRelationalOp();

alexfh wrote:
> nit: extra 'n' in "relational"
I'd put this to ASTMatchers.h together with a test and docs.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:40
@@ +39,3 @@
+  ASTContext::DynTypedNodeList Parents = 
Finder->getASTContext().getParents(*E);
+  if (Parents.size() != 1 || !(E = Parents[0].get()))
+return false;

The `!(E = ...)` construct is a bit hard to read (needs time to figure out the 
intention). I'd prefer a simpler alternative.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:196
@@ +195,3 @@
+  expr(SizeOfExpr, unless(SizeOfZero),
+   hasSizeOfAncestor(SizeOfExpr.bind("sizeof-sizeof-expr"))),
+  this);

It looks like you've created a tool for what can be done in a simpler way. It 
should be possible to express this matcher without going through the parent 
map: `sizeOfExpr(hasDescendant(sizeOfExpr(unless(SizeOfZero` or something 
like that.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:236
@@ +235,3 @@
+  diag(E->getLocStart(), "suspicious usage of 'sizeof(...)/sizeof(...)';"
+ " sizes are incompatible");
+} else if (ElementSize > CharUnits::Zero() &&

I'm not sure I would understand what "incompatible" means here without reading 
the code of the check. Please expand the message. Same in the messages below.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:258
@@ +257,3 @@
+ Result.Nodes.getNodeAs("sizeof-multiply-sizeof")) {
+diag(E->getLocStart(), "suspicious multiplation of two 'sizeof'");
+  }

s/multiplation/multiplication/


http://reviews.llvm.org/D19014



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

r266083 - Verify commit right by adding a blank line to test/CodeGenOpenCL/address-spaces-conversions.cl.

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 12 10:46:24 2016
New Revision: 266083

URL: http://llvm.org/viewvc/llvm-project?rev=266083&view=rev
Log:
Verify commit right by adding a blank line to 
test/CodeGenOpenCL/address-spaces-conversions.cl.

Modified:
cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl

Modified: cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl?rev=266083&r1=266082&r2=266083&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl Tue Apr 12 
10:46:24 2016
@@ -7,6 +7,7 @@ void test(global int *arg_glob, generic
   int var_priv;
   arg_gen = arg_glob; // implicit cast global -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 
addrspace(4)*
+
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
   arg_glob = (global int *)arg_gen; // explicit cast


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

Re: [PATCH] D18617: Call TargetMachine::addEarlyAsPossiblePasses from BackendUtil.

[Justin Lebar via cfe-commits]

jlebar updated this revision to Diff 53415.
jlebar added a comment.

Switch [this] to [&].


http://reviews.llvm.org/D18617

Files:
  lib/CodeGen/BackendUtil.cpp

Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -340,6 +340,14 @@
 return;
   }
 
+  // Add target-specific passes that need to run as early as possible.
+  if (TM)
+PMBuilder.addExtension(
+PassManagerBuilder::EP_EarlyAsPossible,
+[&](const PassManagerBuilder &, legacy::PassManagerBase &PM) {
+  TM->addEarlyAsPossiblePasses(PM);
+});
+
   PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
  addAddDiscriminatorsPass);
 


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -340,6 +340,14 @@
 return;
   }
 
+  // Add target-specific passes that need to run as early as possible.
+  if (TM)
+PMBuilder.addExtension(
+PassManagerBuilder::EP_EarlyAsPossible,
+[&](const PassManagerBuilder &, legacy::PassManagerBase &PM) {
+  TM->addEarlyAsPossiblePasses(PM);
+});
+
   PMBuilder.addExtension(PassManagerBuilder::EP_EarlyAsPossible,
  addAddDiscriminatorsPass);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:37
@@ +36,3 @@
+
+void DeletedDefaultCheck::check(const MatchFinder::MatchResult &Result) {
+  const StringRef Message = "%0 is explicitly defaulted but implicitly "

Will it be less confusing to you, if you change "assignment" to something more 
generic, e.g. "method" or "decl"?

Also, if you find it less readable that way, we can leave it as is for now.


Comment at:  clang-tidy/readability/DeletedDefaultCheck.cpp:50
@@ +49,3 @@
+  Diag << "copy constructor"
+   << "a non-static data member or a base class is not copyable";
+} else if (Constructor->isMoveConstructor()) {

I usually prefer early returns, however, in this case I find it better to make 
the code compact. This way it's also immediately clear that the code does 
nothing but mapping certain properties of matched objects to a message.


http://reviews.llvm.org/D18961



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

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:100
@@ +99,3 @@
+  diag(InnerRanges.back().first, "multiple statement macro used without "
+ "braces; some statements will be "
+ "unconditionally executed");

Now it seems clear to me. Thanks!


http://reviews.llvm.org/D18766



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

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Gabor,

Can other checkers use this function? I am Ok with this being committed with 
limited coverage.


http://reviews.llvm.org/D16044



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

Re: [patch] clang-cl: Remove -isystem, add -imsvc, expose -nostdlibinc

[Hans Wennborg via cfe-commits]

On Mon, Apr 11, 2016 at 7:16 PM, Nico Weber  wrote:
> r260990 exposed -isystem in clang-cl. -isystem adds a directory to the front
> of the system include search path. The idea was to use this to point to a
> hermetic msvc install, but as it turns out this doesn't work: -isystem then
> adds the hermetic headers in front of clang's builtin headers, and clang's
> headers that are supposed to wrap msvc headers (say, stdarg.h) aren't picked
> up at all anymore.
>
> So revert that, and instead expose -imsvc which works as if the passed
> directory was part of %INCLUDE%: The header is treated as a system header,
> but it is searched after clang's lib/Header headers.

Sounds great.

I wonder if this is useful beyond clang-cl and should have a more
general name, but I can't think of anything, so let's go with this.

> Also expose -nostdlibinc so that clang-cl can be told to not look for system
> headers in the usual places.

Can you land this in a separate commit?


> With this change, it's possible to use -imsvc to point clang-cl at a
> hermetic MSVC installation and not make it look at the headers of a possibly
> installed msvc. Fixes PR26751.
>
> I'd upload this to phab, but it's currently down.

Old-school review below:

> Index: include/clang/Driver/CLCompatOptions.td
> ===
> --- include/clang/Driver/CLCompatOptions.td (revision 265325)
> +++ include/clang/Driver/CLCompatOptions.td (working copy)
> @@ -210,6 +210,8 @@
>HelpText<"Enable exception handling">;
>  def _SLASH_GX_ : CLFlag<"GX-">,
>HelpText<"Enable exception handling">;
> +def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">,
> +  HelpText<"Add directory to system include search path, as if part of 
> %INCLUDE%">, MetaVarName<"">;

Nit: I'd have gone with a line break before MetaVarName to pretend
we're at least trying to keep the lines short.

> Index: include/clang/Driver/Options.td
> ===
> --- include/clang/Driver/Options.td (revision 265325)
> +++ include/clang/Driver/Options.td (working copy)
> @@ -1246,7 +1246,7 @@
>  def isysroot : JoinedOrSeparate<["-"], "isysroot">, Group, 
> Flags<[CC1Option]>,
>HelpText<"Set the system root directory (usually /)">, 
> MetaVarName<"">;
>  def isystem : JoinedOrSeparate<["-"], "isystem">, Group,
> -  Flags<[CC1Option, CoreOption]>,
> +  Flags<[CC1Option]>,
>HelpText<"Add directory to SYSTEM include search path">, 
> MetaVarName<"">;
>  def iwithprefixbefore : JoinedOrSeparate<["-"], "iwithprefixbefore">, 
> Group,
>HelpText<"Set directory to include search path with prefix">, 
> MetaVarName<"">,
> @@ -1673,7 +1673,7 @@
>  def noseglinkedit : Flag<["-"], "noseglinkedit">;
>  def nostartfiles : Flag<["-"], "nostartfiles">;
>  def nostdinc : Flag<["-"], "nostdinc">;
> -def nostdlibinc : Flag<["-"], "nostdlibinc">;
> +def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>;

Commit this one separately.

> Index: lib/Driver/MSVCToolChain.cpp
> ===
> --- lib/Driver/MSVCToolChain.cpp (revision 265325)
> +++ lib/Driver/MSVCToolChain.cpp (working copy)
> @@ -527,6 +527,10 @@
>"include");
>}
>
> +  // Add %INCLUDE%-like directories from the -systemI flag.
> +  for (const auto &path : 
> DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
> +addSystemInclude(DriverArgs, CC1Args, path);

Update flag name in the comment. Spelling the loop variable Path would
be more LLVM-style.

> Index: test/Driver/cl-options.c
> ===
> --- test/Driver/cl-options.c (revision 265325)
> +++ test/Driver/cl-options.c (working copy)
> @@ -82,6 +82,10 @@
>  // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck 
> -check-prefix=SLASH_I %s
>  // SLASH_I: "-I" "myincludedir"
>
> +// RUN: %clang_cl /imsvcmyincludedir -### -- %s 2>&1 | FileCheck 
> -check-prefix=SLASH_imsvc %s
> +// RUN: %clang_cl /imsvc myincludedir -### -- %s 2>&1 | FileCheck 
> -check-prefix=SLASH_imsvc %s
> +// SLASH_imsvc: "-internal-isystem" "myincludedir"

Would it be possible to check that this flag comes after the
-internal-isystem for the resource headers?
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18657: Propagate missing empty exception spec from function declared in system header

[Denis Zobnin via cfe-commits]

d.zobnin.bugzilla updated this revision to Diff 53414.
d.zobnin.bugzilla added a comment.

John, thanks for the review!

Please take a look at the updated patch -- did I understand your comments 
correctly?
The test passes and ds2 application is compiled successfully.

Thank you,
Denis Zobnin


http://reviews.llvm.org/D18657

Files:
  lib/Sema/SemaExceptionSpec.cpp
  test/SemaCXX/Inputs/malloc.h
  test/SemaCXX/builtin-exception-spec.cpp

Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -254,10 +254,10 @@
   // to many libc functions as an optimization. Unfortunately, that
   // optimization isn't permitted by the C++ standard, so we're forced
   // to work around it here.
-  if (MissingEmptyExceptionSpecification && NewProto &&
-  (Old->getLocation().isInvalid() ||
-   Context.getSourceManager().isInSystemHeader(Old->getLocation())) &&
-  Old->isExternC()) {
+  FunctionDecl *First = Old->getFirstDecl();
+  if (MissingEmptyExceptionSpecification && NewProto && First->isExternC() &&
+  (First->getLocation().isInvalid() ||
+   Context.getSourceManager().isInSystemHeader(First->getLocation( {
 New->setType(Context.getFunctionType(
 NewProto->getReturnType(), NewProto->getParamTypes(),
 NewProto->getExtProtoInfo().withExceptionSpec(EST_DynamicNone)));
Index: test/SemaCXX/builtin-exception-spec.cpp
===
--- test/SemaCXX/builtin-exception-spec.cpp
+++ test/SemaCXX/builtin-exception-spec.cpp
@@ -4,4 +4,7 @@
 
 extern "C" {
 void *malloc(__SIZE_TYPE__);
+
+void MyFunc() __attribute__((__weak__));
+void MyFunc() { return; }
 }
Index: test/SemaCXX/Inputs/malloc.h
===
--- test/SemaCXX/Inputs/malloc.h
+++ test/SemaCXX/Inputs/malloc.h
@@ -1,3 +1,5 @@
 extern "C" {
 extern void *malloc (__SIZE_TYPE__ __size) throw () __attribute__ 
((__malloc__)) ;
+
+void MyFunc() throw();
 }


Index: lib/Sema/SemaExceptionSpec.cpp
===
--- lib/Sema/SemaExceptionSpec.cpp
+++ lib/Sema/SemaExceptionSpec.cpp
@@ -254,10 +254,10 @@
   // to many libc functions as an optimization. Unfortunately, that
   // optimization isn't permitted by the C++ standard, so we're forced
   // to work around it here.
-  if (MissingEmptyExceptionSpecification && NewProto &&
-  (Old->getLocation().isInvalid() ||
-   Context.getSourceManager().isInSystemHeader(Old->getLocation())) &&
-  Old->isExternC()) {
+  FunctionDecl *First = Old->getFirstDecl();
+  if (MissingEmptyExceptionSpecification && NewProto && First->isExternC() &&
+  (First->getLocation().isInvalid() ||
+   Context.getSourceManager().isInSystemHeader(First->getLocation( {
 New->setType(Context.getFunctionType(
 NewProto->getReturnType(), NewProto->getParamTypes(),
 NewProto->getExtProtoInfo().withExceptionSpec(EST_DynamicNone)));
Index: test/SemaCXX/builtin-exception-spec.cpp
===
--- test/SemaCXX/builtin-exception-spec.cpp
+++ test/SemaCXX/builtin-exception-spec.cpp
@@ -4,4 +4,7 @@
 
 extern "C" {
 void *malloc(__SIZE_TYPE__);
+
+void MyFunc() __attribute__((__weak__));
+void MyFunc() { return; }
 }
Index: test/SemaCXX/Inputs/malloc.h
===
--- test/SemaCXX/Inputs/malloc.h
+++ test/SemaCXX/Inputs/malloc.h
@@ -1,3 +1,5 @@
 extern "C" {
 extern void *malloc (__SIZE_TYPE__ __size) throw () __attribute__ ((__malloc__)) ;
+
+void MyFunc() throw();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r266089 - [FileManager] Don't crash if reading from stdin and stat(".") fails

[David Majnemer via cfe-commits]

Author: majnemer
Date: Tue Apr 12 11:33:53 2016
New Revision: 266089

URL: http://llvm.org/viewvc/llvm-project?rev=266089&view=rev
Log:
[FileManager] Don't crash if reading from stdin and stat(".") fails

addAncestorsAsVirtualDirs("") quickly returns without doing work
because "" has no parent_path.  This violates the expectation
that a subsequent call to getDirectoryFromFile("") would succeed.
Instead, it fails because it uses the "." if the file has no path
component.

Fix this by keeping the behavior between addAncestorsAsVirtualDirs and
getDirectoryFromFile symmetric.

Modified:
cfe/trunk/lib/Basic/FileManager.cpp

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=266089&r1=266088&r2=266089&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Tue Apr 12 11:33:53 2016
@@ -123,7 +123,7 @@ static const DirectoryEntry *getDirector
 void FileManager::addAncestorsAsVirtualDirs(StringRef Path) {
   StringRef DirName = llvm::sys::path::parent_path(Path);
   if (DirName.empty())
-return;
+DirName = ".";
 
   auto &NamedDirEnt =
   *SeenDirEntries.insert(std::make_pair(DirName, nullptr)).first;


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

r266090 - clang-cl: Expose -nostdlibinc.

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Apr 12 11:38:07 2016
New Revision: 266090

URL: http://llvm.org/viewvc/llvm-project?rev=266090&view=rev
Log:
clang-cl: Expose -nostdlibinc.

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=266090&r1=266089&r2=266090&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Apr 12 11:38:07 2016
@@ -1681,7 +1681,7 @@ def noprebind : Flag<["-"], "noprebind">
 def noseglinkedit : Flag<["-"], "noseglinkedit">;
 def nostartfiles : Flag<["-"], "nostartfiles">;
 def nostdinc : Flag<["-"], "nostdinc">;
-def nostdlibinc : Flag<["-"], "nostdlibinc">;
+def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>;
 def nostdincxx : Flag<["-"], "nostdinc++">, Flags<[CC1Option]>,
   HelpText<"Disable standard #include directories for the C++ standard 
library">;
 def nostdlib : Flag<["-"], "nostdlib">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=266090&r1=266089&r2=266090&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Tue Apr 12 11:38:07 2016
@@ -454,6 +454,7 @@
 // RUN: -fno-ms-extensions \
 // RUN: -isystem=some/path \
 // RUN: -mllvm -disable-llvm-optzns \
+// RUN: -nostdlibinc \
 // RUN: -Wunused-variable \
 // RUN: -fmacro-backtrace-limit=0 \
 // RUN: -Werror /Zs -- %s 2>&1


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

Re: r265994 - libclang: fix two memory leaks (PR26292)

[David Blaikie via cfe-commits]

Any chance of using unique_ptr, or at least a scoped cleanup device, here?

On Mon, Apr 11, 2016 at 1:54 PM, Hans Wennborg via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: hans
> Date: Mon Apr 11 15:53:59 2016
> New Revision: 265994
>
> URL: http://llvm.org/viewvc/llvm-project?rev=265994&view=rev
> Log:
> libclang: fix two memory leaks (PR26292)
>
> Modified:
> cfe/trunk/tools/libclang/CIndex.cpp
>
> Modified: cfe/trunk/tools/libclang/CIndex.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=265994&r1=265993&r2=265994&view=diff
>
> ==
> --- cfe/trunk/tools/libclang/CIndex.cpp (original)
> +++ cfe/trunk/tools/libclang/CIndex.cpp Mon Apr 11 15:53:59 2016
> @@ -3571,12 +3571,13 @@ static const ExprEvalResult* evaluateExp
>rettype = callExpr->getCallReturnType(ctx);
>
>if (rettype->isVectorType() || callExpr->getNumArgs() > 1) {
> +clang_EvalResult_dispose((CXEvalResult *)result);
>  return nullptr;
>}
>if (rettype->isIntegralType(ctx) || rettype->isRealFloatingType()) {
>  if(callExpr->getNumArgs() == 1 &&
> -  !callExpr->getArg(0)->getType()->isIntegralType(ctx)){
> -
> +  !callExpr->getArg(0)->getType()->isIntegralType(ctx)) {
> +  clang_EvalResult_dispose((CXEvalResult *)result);
>return nullptr;
>  }
>} else if(rettype.getAsString() == "CFStringRef") {
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r266091 - Revert 266090, needs more testing first.

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Apr 12 11:52:30 2016
New Revision: 266091

URL: http://llvm.org/viewvc/llvm-project?rev=266091&view=rev
Log:
Revert 266090, needs more testing first.

Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/test/Driver/cl-options.c

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=266091&r1=266090&r2=266091&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Apr 12 11:52:30 2016
@@ -1681,7 +1681,7 @@ def noprebind : Flag<["-"], "noprebind">
 def noseglinkedit : Flag<["-"], "noseglinkedit">;
 def nostartfiles : Flag<["-"], "nostartfiles">;
 def nostdinc : Flag<["-"], "nostdinc">;
-def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>;
+def nostdlibinc : Flag<["-"], "nostdlibinc">;
 def nostdincxx : Flag<["-"], "nostdinc++">, Flags<[CC1Option]>,
   HelpText<"Disable standard #include directories for the C++ standard 
library">;
 def nostdlib : Flag<["-"], "nostdlib">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=266091&r1=266090&r2=266091&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Tue Apr 12 11:52:30 2016
@@ -454,7 +454,6 @@
 // RUN: -fno-ms-extensions \
 // RUN: -isystem=some/path \
 // RUN: -mllvm -disable-llvm-optzns \
-// RUN: -nostdlibinc \
 // RUN: -Wunused-variable \
 // RUN: -fmacro-backtrace-limit=0 \
 // RUN: -Werror /Zs -- %s 2>&1


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

Re: [PATCH] D18849: [modules] Avoid module relocation error when building modules from VFS

[Ben Langmuir via cfe-commits]

benlangmuir added a comment.

> From what I understand from 
> http://lists.llvm.org/pipermail/cfe-dev/2014-July/038273.html, changing the 
> behavior of the cached result has side-effects and demands a non trivial 
> change, am I assuming the right thing?


Yes.

Threading through the Filename seems fine to me.  Specific comments below.



Comment at: lib/Basic/FileManager.cpp:221-223
@@ -220,2 +220,5 @@
   // See if there is already an entry in the map.
+  // FIXME: Note that when first requested, the returned file name equals to 
the
+  // requested path. This is not true when returning a cached result. This is
+  // inconsistent and might lead to clients making the wrong assumptions.
   if (NamedFileEnt.second)

The first time we lookup a file, we will return whatever name comes back from 
the VFS, which may or may not be the requested path.  If this is a real file 
system, it will be the requested path.  If it is a redirecting file system and 
"use-external-names" is true (which it is by default), then the VFS will return 
the external contents path, which is generally not the same as the requested 
path.


Comment at: lib/Lex/HeaderSearch.cpp:493
@@ -492,3 +492,3 @@
 // Find the framework in which this header occurs.
-StringRef FrameworkPath = FE->getDir()->getName();
+StringRef FrameworkPath = llvm::sys::path::parent_path(FrameworkName);
 bool FoundFramework = false;

What does this change do?  FE->getDir()->getName() will be the virtual path if 
there is VFS, which is usually the one you want to operate on.


http://reviews.llvm.org/D18849



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

Re: [PATCH] D18617: Call TargetMachine::addEarlyAsPossiblePasses from BackendUtil.

[Chandler Carruth via cfe-commits]

chandlerc added inline comments.


Comment at: lib/CodeGen/BackendUtil.cpp:347
@@ +346,3 @@
+PassManagerBuilder::EP_EarlyAsPossible,
+[&](const PassManagerBuilder &, legacy::PassManagerBase &PM) {
+  TM->addEarlyAsPossiblePasses(PM);

Wow, C++ is silly here. You can't capture it *by value*. You have to use an 
init capture (which we can't use yet). How silly. Anyways, thanks for the 
explanation.


http://reviews.llvm.org/D18617



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

RE: r266005 - Allow simultaneous safestack and stackprotector attributes.

[Robinson, Paul via cfe-commits]

> -Original Message-
> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
> Evgeniy Stepanov via cfe-commits
> Sent: Monday, April 11, 2016 3:28 PM
> To: cfe-commits@lists.llvm.org
> Subject: r266005 - Allow simultaneous safestack and stackprotector
> attributes.
> 
> Author: eugenis
> Date: Mon Apr 11 17:27:55 2016
> New Revision: 266005
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=266005&view=rev
> Log:
> Allow simultaneous safestack and stackprotector attributes.
> 
> This is the clang part of http://reviews.llvm.org/D18846.
> SafeStack instrumentation pass adds stack protector canaries if both
> attributes are present on a function. StackProtector pass will step
> back if the function has a safestack attribute.
> 
> Modified:
> cfe/trunk/lib/Driver/Tools.cpp
> cfe/trunk/test/CodeGen/stack-protector.c
> cfe/trunk/test/Driver/fsanitize.c
> 
> Modified: cfe/trunk/lib/Driver/Tools.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/cfe/trunk/lib/Driver/Tools.cpp?rev=266005&r1=266004&r2=266005&view
> =diff
> ==
> 
> --- cfe/trunk/lib/Driver/Tools.cpp (original)
> +++ cfe/trunk/lib/Driver/Tools.cpp Mon Apr 11 17:27:55 2016
> @@ -4878,15 +4878,10 @@ void Clang::ConstructJob(Compilation &C,
> 
>// -stack-protector=0 is default.
>unsigned StackProtectorLevel = 0;
> -  if (getToolChain().getSanitizerArgs().needsSafeStackRt()) {
> -Args.ClaimAllArgs(options::OPT_fno_stack_protector);
> -Args.ClaimAllArgs(options::OPT_fstack_protector_all);
> -Args.ClaimAllArgs(options::OPT_fstack_protector_strong);
> -Args.ClaimAllArgs(options::OPT_fstack_protector);
> -  } else if (Arg *A = Args.getLastArg(options::OPT_fno_stack_protector,
> -  options::OPT_fstack_protector_all,
> -
> options::OPT_fstack_protector_strong,
> -  options::OPT_fstack_protector)) {
> +  if (Arg *A = Args.getLastArg(options::OPT_fno_stack_protector,
> +   options::OPT_fstack_protector_all,
> +   options::OPT_fstack_protector_strong,
> +   options::OPT_fstack_protector)) {
>  if (A->getOption().matches(options::OPT_fstack_protector)) {
>StackProtectorLevel = std::max(
>LangOptions::SSPOn,
> 
> Modified: cfe/trunk/test/CodeGen/stack-protector.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/stack-
> protector.c?rev=266005&r1=266004&r2=266005&view=diff
> ==
> 
> --- cfe/trunk/test/CodeGen/stack-protector.c (original)
> +++ cfe/trunk/test/CodeGen/stack-protector.c Mon Apr 11 17:27:55 2016
> @@ -1,13 +1,13 @@
> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 0 | FileCheck -
> check-prefix=NOSSP %s
> -// NOSSP: define {{.*}}void @test1(i8* %msg) #0 {
> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 1 | FileCheck -
> check-prefix=WITHSSP %s
> -// WITHSSP: define {{.*}}void @test1(i8* %msg) #0 {
> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 2 | FileCheck -
> check-prefix=SSPSTRONG %s
> -// SSPSTRONG: define {{.*}}void @test1(i8* %msg) #0 {
> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 3 | FileCheck -
> check-prefix=SSPREQ %s
> -// SSPREQ: define {{.*}}void @test1(i8* %msg) #0 {
> -// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack | FileCheck -
> check-prefix=SAFESTACK %s
> -// SAFESTACK: define {{.*}}void @test1(i8* %msg) #0 {
> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 0 | FileCheck -
> check-prefix=DEF -check-prefix=NOSSP %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 1 | FileCheck -
> check-prefix=DEF -check-prefix=SSP %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 2 | FileCheck -
> check-prefix=DEF -check-prefix=SSPSTRONG %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 3 | FileCheck -
> check-prefix=DEF -check-prefix=SSPREQ %s
> +
> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack | FileCheck -
> check-prefix=DEF -check-prefix=SAFESTACK-NOSSP %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
> protector 0 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-NOSSP %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
> protector 1 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-SSP %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
> protector 2 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-
> SSPSTRONG %s
> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
> protector 3 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-SSPREQ
> %s
> 
>  typedef __SIZE_TYPE__ size_t;
> 
> @@ -15,18 +15,21 @@ int printf(const char * _Format, ...);
>  size_t strlen(const char *s);
>  char *strcpy(char *s1, const char *s2);
> 
> +// DEF: def

Re: [PATCH] D16044: getDescriptiveName() for MemRegion

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

xazax.hun added a comment.

In http://reviews.llvm.org/D16044#398463, @zaks.anna wrote:

> Can other checkers use this function? I am Ok with this being committed with 
> limited coverage.


Once I rebase http://reviews.llvm.org/D15227, it will provide us with the 
limited coverage. In case that patch is accepted before the MPI checkers I can 
commit this patch along with that one.


http://reviews.llvm.org/D16044



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

Re: r262817 - [CLANG][AVX512][BUILTIN] Adding vpmultishiftqb{128|256|512}

[Chandler Carruth via cfe-commits]

On Mon, Mar 7, 2016 at 12:33 AM Michael Zuckerman via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: mzuckerm
> Date: Mon Mar  7 02:29:10 2016
> New Revision: 262817
>
> URL: http://llvm.org/viewvc/llvm-project?rev=262817&view=rev
> Log:
> [CLANG][AVX512][BUILTIN] Adding vpmultishiftqb{128|256|512}
>

This commit log seems super confusing combined with:

+TARGET_BUILTIN(__builtin_ia32_pbroadcastb512_gpr_mask,
> "V64ccV64cULLi","","avx512bw")
> +TARGET_BUILTIN(__builtin_ia32_pbroadcastb128_gpr_mask,
> "V16ccV16cUs","","avx512bw,avx512vl")
> +TARGET_BUILTIN(__builtin_ia32_pbroadcastb256_gpr_mask,
> "V32ccV32cUi","","avx512bw,avx512vl")
> +TARGET_BUILTIN(__builtin_ia32_pbroadcastd128_gpr_mask,
> "V4iiV4iUc","","avx512vl")
> +TARGET_BUILTIN(__builtin_ia32_pbroadcastd256_gpr_mask,
> "V8iiV8iUc","","avx512vl")
> +TARGET_BUILTIN(__builtin_ia32_pbroadcastq128_gpr_mask,
> "V2LLiULLiV2LLiUc","","avx512vl")
> +TARGET_BUILTIN(__builtin_ia32_pbroadcastq256_gpr_mask,
> "V4LLiULLiV4LLiUc","","avx512vl")
>

This is adding *broadcast* intrinsics!!! Not multishift!

Please be much more careful with your patch descriptions. I just happened
to randomly notice this.

Elena, please actually carefully review these patches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18474: [OPENMP] Enable correct generation of runtime call when target directive is separated from teams directive by multiple curly brackets

[Carlo Bertolli via cfe-commits]

carlo.bertolli marked an inline comment as done.
carlo.bertolli added a comment.

Great, thanks for the hint. I do much prefer your version of the function. I am 
updating the diff.


Repository:
  rL LLVM

http://reviews.llvm.org/D18474



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

Re: [PATCH] D18474: [OPENMP] Enable correct generation of runtime call when target directive is separated from teams directive by multiple curly brackets

[Carlo Bertolli via cfe-commits]

carlo.bertolli updated this revision to Diff 53425.
carlo.bertolli added a comment.

[OPENMP] Remove unnecessary templating of function.


Repository:
  rL LLVM

http://reviews.llvm.org/D18474

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/teams_codegen.cpp

Index: test/OpenMP/teams_codegen.cpp
===
--- test/OpenMP/teams_codegen.cpp
+++ test/OpenMP/teams_codegen.cpp
@@ -29,6 +29,16 @@
 ++comp;
   }
 
+  // CK1: call i32 @__tgt_target_teams(i32 -1, i8* @{{[^,]+}}, i32 1, i8** 
%{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32* 
{{.+}}@{{[^,]+}}, i32 0, i32 0), i32 0, i32 0)
+  // CK1: call void @{{.+}}(i{{64|32}} %{{.+}})
+  #pragma omp target
+  {{{
+#pragma omp teams
+{
+  ++comp;
+}
+  }}}
+  
   // CK1-DAG: call i32 @__tgt_target_teams(i32 -1, i8* @{{[^,]+}}, i32 2, i8** 
%{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32* 
{{.+}}@{{[^,]+}}, i32 0, i32 0), i32 [[NT:%[^,]+]], i32 0)
   // CK1-DAG: [[NT]] = load i32, i32* [[NTA:%[^,]+]],
 
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4257,6 +4257,14 @@
   DeviceID, FileID, ParentName, Line, OutlinedFn, OutlinedFnID);
 }
 
+/// \brief discard all CompoundStmts intervening between two constructs
+static const Stmt *ignoreCompoundStmts(const Stmt *Body) {
+  while (auto *CS = dyn_cast_or_null(Body))
+Body = CS->body_front();
+
+  return Body;
+}
+
 /// \brief Emit the num_teams clause of an enclosed teams directive at the
 /// target region scope. If there is no teams directive associated with the
 /// target directive, or if there is no num_teams clause associated with the
@@ -4287,7 +4295,8 @@
 
   // FIXME: Accommodate other combined directives with teams when they become
   // available.
-  if (auto *TeamsDir = dyn_cast(CS.getCapturedStmt())) {
+  if (auto *TeamsDir = dyn_cast_or_null(
+  ignoreCompoundStmts(CS.getCapturedStmt( {
 if (auto *NTE = TeamsDir->getSingleClause()) {
   CGOpenMPInnerExprInfo CGInfo(CGF, CS);
   CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, &CGInfo);
@@ -4335,7 +4344,8 @@
 
   // FIXME: Accommodate other combined directives with teams when they become
   // available.
-  if (auto *TeamsDir = dyn_cast(CS.getCapturedStmt())) {
+  if (auto *TeamsDir = dyn_cast_or_null(
+  ignoreCompoundStmts(CS.getCapturedStmt( {
 if (auto *TLE = TeamsDir->getSingleClause()) {
   CGOpenMPInnerExprInfo CGInfo(CGF, CS);
   CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, &CGInfo);


Index: test/OpenMP/teams_codegen.cpp
===
--- test/OpenMP/teams_codegen.cpp
+++ test/OpenMP/teams_codegen.cpp
@@ -29,6 +29,16 @@
 ++comp;
   }
 
+  // CK1: call i32 @__tgt_target_teams(i32 -1, i8* @{{[^,]+}}, i32 1, i8** %{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32 0, i32 0)
+  // CK1: call void @{{.+}}(i{{64|32}} %{{.+}})
+  #pragma omp target
+  {{{
+#pragma omp teams
+{
+  ++comp;
+}
+  }}}
+  
   // CK1-DAG: call i32 @__tgt_target_teams(i32 -1, i8* @{{[^,]+}}, i32 2, i8** %{{[^,]+}}, i8** %{{[^,]+}}, i{{64|32}}* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32* {{.+}}@{{[^,]+}}, i32 0, i32 0), i32 [[NT:%[^,]+]], i32 0)
   // CK1-DAG: [[NT]] = load i32, i32* [[NTA:%[^,]+]],
 
Index: lib/CodeGen/CGOpenMPRuntime.cpp
===
--- lib/CodeGen/CGOpenMPRuntime.cpp
+++ lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4257,6 +4257,14 @@
   DeviceID, FileID, ParentName, Line, OutlinedFn, OutlinedFnID);
 }
 
+/// \brief discard all CompoundStmts intervening between two constructs
+static const Stmt *ignoreCompoundStmts(const Stmt *Body) {
+  while (auto *CS = dyn_cast_or_null(Body))
+Body = CS->body_front();
+
+  return Body;
+}
+
 /// \brief Emit the num_teams clause of an enclosed teams directive at the
 /// target region scope. If there is no teams directive associated with the
 /// target directive, or if there is no num_teams clause associated with the
@@ -4287,7 +4295,8 @@
 
   // FIXME: Accommodate other combined directives with teams when they become
   // available.
-  if (auto *TeamsDir = dyn_cast(CS.getCapturedStmt())) {
+  if (auto *TeamsDir = dyn_cast_or_null(
+  ignoreCompoundStmts(CS.getCapturedStmt( {
 if (auto *NTE = TeamsDir->getSingleClause()) {
   CGOpenMPInnerExprInfo CGInfo(CGF, CS);
   CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, &CGInfo);
@@ -4335,7 +4344,8 @@
 
   // FIXME: Accommodate other combined directives with teams when they become
   // available.
-  if (auto *TeamsDir = dyn_cast(CS.getCapturedStmt())) {
+  if (auto *TeamsDir = dyn_cast_or_null(
+  ignoreCompoundStmts(C

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 53428.
etienneb marked 6 inline comments as done.
etienneb added a comment.

fix alexfh@ comments


http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(int) * sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(ptr) * sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(int) * (2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE1 = sizeof +sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE2 = sizeof sizeof(const char*);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE3 = sizeof sizeof(const volatile char* const*);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE4 = sizeof sizeof(MyConstCh

Re: r265994 - libclang: fix two memory leaks (PR26292)

[Hans Wennborg via cfe-commits]

While useful, that would require a lot of restructuring, I think.

This file seems to mostly be dealing with C-style objects;
ExprEvalResult doesn't even have a C++ destructor. I'm not going to
dig into that, I just wanted to address this bug that someone
reported.

 - Hans


On Tue, Apr 12, 2016 at 9:51 AM, David Blaikie via cfe-commits
 wrote:
> Any chance of using unique_ptr, or at least a scoped cleanup device, here?
>
> On Mon, Apr 11, 2016 at 1:54 PM, Hans Wennborg via cfe-commits
>  wrote:
>>
>> Author: hans
>> Date: Mon Apr 11 15:53:59 2016
>> New Revision: 265994
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=265994&view=rev
>> Log:
>> libclang: fix two memory leaks (PR26292)
>>
>> Modified:
>> cfe/trunk/tools/libclang/CIndex.cpp
>>
>> Modified: cfe/trunk/tools/libclang/CIndex.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/CIndex.cpp?rev=265994&r1=265993&r2=265994&view=diff
>>
>> ==
>> --- cfe/trunk/tools/libclang/CIndex.cpp (original)
>> +++ cfe/trunk/tools/libclang/CIndex.cpp Mon Apr 11 15:53:59 2016
>> @@ -3571,12 +3571,13 @@ static const ExprEvalResult* evaluateExp
>>rettype = callExpr->getCallReturnType(ctx);
>>
>>if (rettype->isVectorType() || callExpr->getNumArgs() > 1) {
>> +clang_EvalResult_dispose((CXEvalResult *)result);
>>  return nullptr;
>>}
>>if (rettype->isIntegralType(ctx) || rettype->isRealFloatingType())
>> {
>>  if(callExpr->getNumArgs() == 1 &&
>> -  !callExpr->getArg(0)->getType()->isIntegralType(ctx)){
>> -
>> +  !callExpr->getArg(0)->getType()->isIntegralType(ctx)) {
>> +  clang_EvalResult_dispose((CXEvalResult *)result);
>>return nullptr;
>>  }
>>} else if(rettype.getAsString() == "CFStringRef") {
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r266005 - Allow simultaneous safestack and stackprotector attributes.

[Evgenii Stepanov via cfe-commits]

Thanks, fixed in r266095

On Tue, Apr 12, 2016 at 10:15 AM, Robinson, Paul
 wrote:
>
>
>> -Original Message-
>> From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf Of
>> Evgeniy Stepanov via cfe-commits
>> Sent: Monday, April 11, 2016 3:28 PM
>> To: cfe-commits@lists.llvm.org
>> Subject: r266005 - Allow simultaneous safestack and stackprotector
>> attributes.
>>
>> Author: eugenis
>> Date: Mon Apr 11 17:27:55 2016
>> New Revision: 266005
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=266005&view=rev
>> Log:
>> Allow simultaneous safestack and stackprotector attributes.
>>
>> This is the clang part of http://reviews.llvm.org/D18846.
>> SafeStack instrumentation pass adds stack protector canaries if both
>> attributes are present on a function. StackProtector pass will step
>> back if the function has a safestack attribute.
>>
>> Modified:
>> cfe/trunk/lib/Driver/Tools.cpp
>> cfe/trunk/test/CodeGen/stack-protector.c
>> cfe/trunk/test/Driver/fsanitize.c
>>
>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/cfe/trunk/lib/Driver/Tools.cpp?rev=266005&r1=266004&r2=266005&view
>> =diff
>> ==
>> 
>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>> +++ cfe/trunk/lib/Driver/Tools.cpp Mon Apr 11 17:27:55 2016
>> @@ -4878,15 +4878,10 @@ void Clang::ConstructJob(Compilation &C,
>>
>>// -stack-protector=0 is default.
>>unsigned StackProtectorLevel = 0;
>> -  if (getToolChain().getSanitizerArgs().needsSafeStackRt()) {
>> -Args.ClaimAllArgs(options::OPT_fno_stack_protector);
>> -Args.ClaimAllArgs(options::OPT_fstack_protector_all);
>> -Args.ClaimAllArgs(options::OPT_fstack_protector_strong);
>> -Args.ClaimAllArgs(options::OPT_fstack_protector);
>> -  } else if (Arg *A = Args.getLastArg(options::OPT_fno_stack_protector,
>> -  options::OPT_fstack_protector_all,
>> -
>> options::OPT_fstack_protector_strong,
>> -  options::OPT_fstack_protector)) {
>> +  if (Arg *A = Args.getLastArg(options::OPT_fno_stack_protector,
>> +   options::OPT_fstack_protector_all,
>> +   options::OPT_fstack_protector_strong,
>> +   options::OPT_fstack_protector)) {
>>  if (A->getOption().matches(options::OPT_fstack_protector)) {
>>StackProtectorLevel = std::max(
>>LangOptions::SSPOn,
>>
>> Modified: cfe/trunk/test/CodeGen/stack-protector.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/stack-
>> protector.c?rev=266005&r1=266004&r2=266005&view=diff
>> ==
>> 
>> --- cfe/trunk/test/CodeGen/stack-protector.c (original)
>> +++ cfe/trunk/test/CodeGen/stack-protector.c Mon Apr 11 17:27:55 2016
>> @@ -1,13 +1,13 @@
>> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 0 | FileCheck -
>> check-prefix=NOSSP %s
>> -// NOSSP: define {{.*}}void @test1(i8* %msg) #0 {
>> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 1 | FileCheck -
>> check-prefix=WITHSSP %s
>> -// WITHSSP: define {{.*}}void @test1(i8* %msg) #0 {
>> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 2 | FileCheck -
>> check-prefix=SSPSTRONG %s
>> -// SSPSTRONG: define {{.*}}void @test1(i8* %msg) #0 {
>> -// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 3 | FileCheck -
>> check-prefix=SSPREQ %s
>> -// SSPREQ: define {{.*}}void @test1(i8* %msg) #0 {
>> -// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack | FileCheck -
>> check-prefix=SAFESTACK %s
>> -// SAFESTACK: define {{.*}}void @test1(i8* %msg) #0 {
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 0 | FileCheck -
>> check-prefix=DEF -check-prefix=NOSSP %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 1 | FileCheck -
>> check-prefix=DEF -check-prefix=SSP %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 2 | FileCheck -
>> check-prefix=DEF -check-prefix=SSPSTRONG %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -stack-protector 3 | FileCheck -
>> check-prefix=DEF -check-prefix=SSPREQ %s
>> +
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack | FileCheck -
>> check-prefix=DEF -check-prefix=SAFESTACK-NOSSP %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
>> protector 0 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-NOSSP %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
>> protector 1 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-SSP %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
>> protector 2 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-
>> SSPSTRONG %s
>> +// RUN: %clang_cc1 -emit-llvm -o - %s -fsanitize=safe-stack -stack-
>> protector 3 | FileCheck -check-prefix=DEF -check-prefix=SAFESTACK-SSPREQ
>> %s
>>
>>  typedef 

r266095 - Stricter checks in the stack-protector codegen test.

[Evgeniy Stepanov via cfe-commits]

Author: eugenis
Date: Tue Apr 12 12:51:59 2016
New Revision: 266095

URL: http://llvm.org/viewvc/llvm-project?rev=266095&view=rev
Log:
Stricter checks in the stack-protector codegen test.

Modified:
cfe/trunk/test/CodeGen/stack-protector.c

Modified: cfe/trunk/test/CodeGen/stack-protector.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/stack-protector.c?rev=266095&r1=266094&r2=266095&view=diff
==
--- cfe/trunk/test/CodeGen/stack-protector.c (original)
+++ cfe/trunk/test/CodeGen/stack-protector.c Tue Apr 12 12:51:59 2016
@@ -23,13 +23,13 @@ void test1(const char *msg) {
 }
 
 // NOSSP-NOT: attributes #[[A]] = {{.*}} ssp
-// SSP: attributes #[[A]] = {{.*}} ssp
+// SSP: attributes #[[A]] = {{.*}} ssp{{ }}
 // SSPSTRONG: attributes #[[A]] = {{.*}} sspstrong
 // SSPREQ: attributes #[[A]] = {{.*}} sspreq
 
 // SAFESTACK-NOSSP: attributes #[[A]] = {{.*}} safestack
 // SAFESTACK-NOSSP-NOT: ssp
 
-// SAFESTACK-SSP: attributes #[[A]] = {{.*}} safestack ssp
+// SAFESTACK-SSP: attributes #[[A]] = {{.*}} safestack ssp{{ }}
 // SAFESTACK-SSPSTRONG: attributes #[[A]] = {{.*}} safestack sspstrong
 // SAFESTACK-SSPREQ: attributes #[[A]] = {{.*}} safestack sspreq


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

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

[Sean Callanan via cfe-commits]

spyffe added a subscriber: spyffe.
spyffe added a comment.

Aleksei,

it looks like I might have made a bad assumption in my ImportArray – namely 
that there were some entities that required arrays of things they’re 
constructed with to be allocated in the ASTContext.  Looking at the 
constructors for those entities, it looks like most of them actually do the 
allocation and copying themselves.

Thanks for cleaning that up.  I’m going to try running the LLDB testsuite with 
your patch applied and I’ll let you know what happens.

Sean


http://reviews.llvm.org/D14286



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

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

[Sean Callanan via cfe-commits]

Aleksei,

it looks like I might have made a bad assumption in my ImportArray – namely 
that there were some entities that required arrays of things they’re 
constructed with to be allocated in the ASTContext.  Looking at the 
constructors for those entities, it looks like most of them actually do the 
allocation and copying themselves.

Thanks for cleaning that up.  I’m going to try running the LLDB testsuite with 
your patch applied and I’ll let you know what happens.

Sean


> On Apr 12, 2016, at 5:46 AM, Aleksei Sidorin  wrote:
> 
> 

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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 53429.
etienneb added a comment.

release notes.


http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,177 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(int) * sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(ptr) * sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(int) * (2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE1 = sizeof +sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE2 = sizeof sizeof(const char*);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE3 = sizeof sizeof(const volatile char* const*);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE4 = sizeof sizeof(MyConstChar);
+// CHECK-MESSAGES: 

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 53431.
etienneb added a comment.

more unittests


http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,183 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(int) * sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(ptr) * sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(int) * (2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += (2 * sizeof(char)) * sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  if (sizeof(A) < 0x10) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  if (sizeof(A) <= 0xFFFEU) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE1 = sizeof +sizeof(ch

Re: [PATCH] D18474: [OPENMP] Enable correct generation of runtime call when target directive is separated from teams directive by multiple curly brackets

[Alexey Bataev via cfe-commits]

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

LG, except for the small comment



Comment at: lib/CodeGen/CGOpenMPRuntime.cpp:4260
@@ -4259,1 +4259,3 @@
 
+/// \brief discard all CompoundStmts intervening between two constructs
+static const Stmt *ignoreCompoundStmts(const Stmt *Body) {

\brief tag is not required anymore, just a brief description is required.


Repository:
  rL LLVM

http://reviews.llvm.org/D18474



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 53436.
etienneb added a comment.

fix typos and doc.


http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,186 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(int) * sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(ptr) * sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += sizeof(int) * (2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  sum += (2 * sizeof(char)) * sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious multiplication of two 'sizeof'
+  if (sizeof(A) < 0x10) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  if (sizeof(A) <= 0xFFFEU) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of 'sizeof(sizeof(...))'
+int CE1 = sizeof +sizeo

Re: [PATCH] D18997: [SemaObjC] Properly diagnose type arguments and protocols mix in parameterized classes

[Manman Ren via cfe-commits]

manmanren added inline comments.


Comment at: lib/Parse/ParseObjc.cpp:1697
@@ -1696,3 +1696,3 @@
   // We syntactically matched a type argument, so commit to parsing
-  // type arguments.
+  // type arguments. Finding protocol arguments after here is an error.
 

I am a little confused about the original comment. Have we syntactically 
matched a type argument? All we have done is trying to parse the single 
identifiers, right?


Comment at: lib/Parse/ParseObjc.cpp:1720
@@ -1714,3 +1719,3 @@
   if (fullTypeArg.isUsable())
 typeArgs.push_back(fullTypeArg.get());
   else

Should we set foundValidTypeId here too?


Comment at: lib/Parse/ParseObjc.cpp:1722
@@ -1716,3 +1721,3 @@
   else
 invalid = true;
 } else {

Should we emit diagnostics for all cases we set invalid to true?


Comment at: lib/Parse/ParseObjc.cpp:1726
@@ +1725,3 @@
+  auto *P = Actions.LookupProtocol(identifiers[i], identifierLocs[i]);
+  if (!P) {
+unknownTypeArgs.push_back(identifiers[i]);

Please add comments here: not a type argument, not a protocol, treat it as 
unknown type.


Comment at: lib/Parse/ParseObjc.cpp:1729
@@ +1728,3 @@
+unknownTypeArgsLoc.push_back(identifierLocs[i]);
+continue;
+  }

Is it necessary to have a "continue" here? Can we restructure this to get rid 
of the "continue"?


http://reviews.llvm.org/D18997



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

Re: [PATCH] D18748: [AMDGPU] Add debugger related target options

[Konstantin Zhuravlyov via cfe-commits]

kzhuravl added a comment.

ping


http://reviews.llvm.org/D18748



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197
@@ +196,3 @@
+  this);
+}
+

What about this comment?


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:203
@@ +202,3 @@
+  if (const auto *E = Result.Nodes.getNodeAs("sizeof-constant")) {
+diag(E->getLocStart(), "suspicious usage of 'sizeof(K)'; did you mean 
'K'");
+  } else if (const auto *E = Result.Nodes.getNodeAs("sizeof-this")) {

nit: missing trailing question mark


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:209
@@ +208,3 @@
+diag(E->getLocStart(),
+ "suspicious usage of 'sizeof(char*)'; do you mean strlen?'");
+  } else if (const auto *E =

nit: 'strlen' should be in single quotes for consistency with other messages.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:257
@@ +256,3 @@
+ Result.Nodes.getNodeAs("sizeof-multiply-sizeof")) {
+diag(E->getLocStart(), "suspicious multiplication of two 'sizeof'");
+  }

nit: I'd say `of 'sizeof' by 'sizeof'` instead of `of two 'sizeof'`.


Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:58
@@ +57,3 @@
+
+warning: suspicious usage of 'sizeof(A*)'
+^

nit: the titles need to be consistent - either all have or not have `warning:`.

I'd also make the first letters capital.


Comment at: docs/clang-tidy/checks/misc-sizeof-expression.rst:102
@@ +101,3 @@
+^
+Multiplying ``sizeof`` expressions typically make no sense and is probably a
+logic error. In the following example, the programmer used ``*`` instead of

s/make/makes/


http://reviews.llvm.org/D19014



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

Re: [patch] clang-cl: Remove -isystem, add -imsvc, expose -nostdlibinc

[Nico Weber via cfe-commits]

All done. phab's back, but since we started with a patch attachment, let's
keep it that way.



On Tue, Apr 12, 2016 at 12:12 PM, Hans Wennborg  wrote:

> On Mon, Apr 11, 2016 at 7:16 PM, Nico Weber  wrote:
> > r260990 exposed -isystem in clang-cl. -isystem adds a directory to the
> front
> > of the system include search path. The idea was to use this to point to a
> > hermetic msvc install, but as it turns out this doesn't work: -isystem
> then
> > adds the hermetic headers in front of clang's builtin headers, and
> clang's
> > headers that are supposed to wrap msvc headers (say, stdarg.h) aren't
> picked
> > up at all anymore.
> >
> > So revert that, and instead expose -imsvc which works as if the passed
> > directory was part of %INCLUDE%: The header is treated as a system
> header,
> > but it is searched after clang's lib/Header headers.
>
> Sounds great.
>
> I wonder if this is useful beyond clang-cl and should have a more
> general name, but I can't think of anything, so let's go with this.
>
> > Also expose -nostdlibinc so that clang-cl can be told to not look for
> system
> > headers in the usual places.
>
> Can you land this in a separate commit?
>
>
> > With this change, it's possible to use -imsvc to point clang-cl at a
> > hermetic MSVC installation and not make it look at the headers of a
> possibly
> > installed msvc. Fixes PR26751.
> >
> > I'd upload this to phab, but it's currently down.
>
> Old-school review below:
>
> > Index: include/clang/Driver/CLCompatOptions.td
> > ===
> > --- include/clang/Driver/CLCompatOptions.td (revision 265325)
> > +++ include/clang/Driver/CLCompatOptions.td (working copy)
> > @@ -210,6 +210,8 @@
> >HelpText<"Enable exception handling">;
> >  def _SLASH_GX_ : CLFlag<"GX-">,
> >HelpText<"Enable exception handling">;
> > +def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">,
> > +  HelpText<"Add directory to system include search path, as if part of
> %INCLUDE%">, MetaVarName<"">;
>
> Nit: I'd have gone with a line break before MetaVarName to pretend
> we're at least trying to keep the lines short.
>
> > Index: include/clang/Driver/Options.td
> > ===
> > --- include/clang/Driver/Options.td (revision 265325)
> > +++ include/clang/Driver/Options.td (working copy)
> > @@ -1246,7 +1246,7 @@
> >  def isysroot : JoinedOrSeparate<["-"], "isysroot">,
> Group, Flags<[CC1Option]>,
> >HelpText<"Set the system root directory (usually /)">,
> MetaVarName<"">;
> >  def isystem : JoinedOrSeparate<["-"], "isystem">, Group,
> > -  Flags<[CC1Option, CoreOption]>,
> > +  Flags<[CC1Option]>,
> >HelpText<"Add directory to SYSTEM include search path">,
> MetaVarName<"">;
> >  def iwithprefixbefore : JoinedOrSeparate<["-"], "iwithprefixbefore">,
> Group,
> >HelpText<"Set directory to include search path with prefix">,
> MetaVarName<"">,
> > @@ -1673,7 +1673,7 @@
> >  def noseglinkedit : Flag<["-"], "noseglinkedit">;
> >  def nostartfiles : Flag<["-"], "nostartfiles">;
> >  def nostdinc : Flag<["-"], "nostdinc">;
> > -def nostdlibinc : Flag<["-"], "nostdlibinc">;
> > +def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>;
>
> Commit this one separately.
>
> > Index: lib/Driver/MSVCToolChain.cpp
> > ===
> > --- lib/Driver/MSVCToolChain.cpp (revision 265325)
> > +++ lib/Driver/MSVCToolChain.cpp (working copy)
> > @@ -527,6 +527,10 @@
> >"include");
> >}
> >
> > +  // Add %INCLUDE%-like directories from the -systemI flag.
> > +  for (const auto &path :
> DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
> > +addSystemInclude(DriverArgs, CC1Args, path);
>
> Update flag name in the comment. Spelling the loop variable Path would
> be more LLVM-style.
>
> > Index: test/Driver/cl-options.c
> > ===
> > --- test/Driver/cl-options.c (revision 265325)
> > +++ test/Driver/cl-options.c (working copy)
> > @@ -82,6 +82,10 @@
> >  // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck
> -check-prefix=SLASH_I %s
> >  // SLASH_I: "-I" "myincludedir"
> >
> > +// RUN: %clang_cl /imsvcmyincludedir -### -- %s 2>&1 | FileCheck
> -check-prefix=SLASH_imsvc %s
> > +// RUN: %clang_cl /imsvc myincludedir -### -- %s 2>&1 | FileCheck
> -check-prefix=SLASH_imsvc %s
> > +// SLASH_imsvc: "-internal-isystem" "myincludedir"
>
> Would it be possible to check that this flag comes after the
> -internal-isystem for the resource headers?
>


clang-cl-imsvc.patch
Description: Binary data
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D15469: Expose cxx constructor and method properties through libclang and python bindings.

[Sergey Kalinichev via cfe-commits]

skalinichev accepted this revision.
skalinichev added a comment.

LGTM!



Comment at: bindings/python/tests/cindex/test_cursor.py:197
@@ +196,3 @@
+
+def test_is_deleted_method():
+"""Ensure Cursor.is_deleted_method works."""

You should remove it too.


http://reviews.llvm.org/D15469



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

Re: [patch] clang-cl: Remove -isystem, add -imsvc, expose -nostdlibinc

[Hans Wennborg via cfe-commits]

LGTM

On Tue, Apr 12, 2016 at 11:40 AM, Nico Weber  wrote:
> All done. phab's back, but since we started with a patch attachment, let's
> keep it that way.
>
>
>
> On Tue, Apr 12, 2016 at 12:12 PM, Hans Wennborg  wrote:
>>
>> On Mon, Apr 11, 2016 at 7:16 PM, Nico Weber  wrote:
>> > r260990 exposed -isystem in clang-cl. -isystem adds a directory to the
>> > front
>> > of the system include search path. The idea was to use this to point to
>> > a
>> > hermetic msvc install, but as it turns out this doesn't work: -isystem
>> > then
>> > adds the hermetic headers in front of clang's builtin headers, and
>> > clang's
>> > headers that are supposed to wrap msvc headers (say, stdarg.h) aren't
>> > picked
>> > up at all anymore.
>> >
>> > So revert that, and instead expose -imsvc which works as if the passed
>> > directory was part of %INCLUDE%: The header is treated as a system
>> > header,
>> > but it is searched after clang's lib/Header headers.
>>
>> Sounds great.
>>
>> I wonder if this is useful beyond clang-cl and should have a more
>> general name, but I can't think of anything, so let's go with this.
>>
>> > Also expose -nostdlibinc so that clang-cl can be told to not look for
>> > system
>> > headers in the usual places.
>>
>> Can you land this in a separate commit?
>>
>>
>> > With this change, it's possible to use -imsvc to point clang-cl at a
>> > hermetic MSVC installation and not make it look at the headers of a
>> > possibly
>> > installed msvc. Fixes PR26751.
>> >
>> > I'd upload this to phab, but it's currently down.
>>
>> Old-school review below:
>>
>> > Index: include/clang/Driver/CLCompatOptions.td
>> > ===
>> > --- include/clang/Driver/CLCompatOptions.td (revision 265325)
>> > +++ include/clang/Driver/CLCompatOptions.td (working copy)
>> > @@ -210,6 +210,8 @@
>> >HelpText<"Enable exception handling">;
>> >  def _SLASH_GX_ : CLFlag<"GX-">,
>> >HelpText<"Enable exception handling">;
>> > +def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">,
>> > +  HelpText<"Add directory to system include search path, as if part of
>> > %INCLUDE%">, MetaVarName<"">;
>>
>> Nit: I'd have gone with a line break before MetaVarName to pretend
>> we're at least trying to keep the lines short.
>>
>> > Index: include/clang/Driver/Options.td
>> > ===
>> > --- include/clang/Driver/Options.td (revision 265325)
>> > +++ include/clang/Driver/Options.td (working copy)
>> > @@ -1246,7 +1246,7 @@
>> >  def isysroot : JoinedOrSeparate<["-"], "isysroot">,
>> > Group, Flags<[CC1Option]>,
>> >HelpText<"Set the system root directory (usually /)">,
>> > MetaVarName<"">;
>> >  def isystem : JoinedOrSeparate<["-"], "isystem">, Group,
>> > -  Flags<[CC1Option, CoreOption]>,
>> > +  Flags<[CC1Option]>,
>> >HelpText<"Add directory to SYSTEM include search path">,
>> > MetaVarName<"">;
>> >  def iwithprefixbefore : JoinedOrSeparate<["-"], "iwithprefixbefore">,
>> > Group,
>> >HelpText<"Set directory to include search path with prefix">,
>> > MetaVarName<"">,
>> > @@ -1673,7 +1673,7 @@
>> >  def noseglinkedit : Flag<["-"], "noseglinkedit">;
>> >  def nostartfiles : Flag<["-"], "nostartfiles">;
>> >  def nostdinc : Flag<["-"], "nostdinc">;
>> > -def nostdlibinc : Flag<["-"], "nostdlibinc">;
>> > +def nostdlibinc : Flag<["-"], "nostdlibinc">, Flags<[CoreOption]>;
>>
>> Commit this one separately.
>>
>> > Index: lib/Driver/MSVCToolChain.cpp
>> > ===
>> > --- lib/Driver/MSVCToolChain.cpp (revision 265325)
>> > +++ lib/Driver/MSVCToolChain.cpp (working copy)
>> > @@ -527,6 +527,10 @@
>> >"include");
>> >}
>> >
>> > +  // Add %INCLUDE%-like directories from the -systemI flag.
>> > +  for (const auto &path :
>> > DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
>> > +addSystemInclude(DriverArgs, CC1Args, path);
>>
>> Update flag name in the comment. Spelling the loop variable Path would
>> be more LLVM-style.
>>
>> > Index: test/Driver/cl-options.c
>> > ===
>> > --- test/Driver/cl-options.c (revision 265325)
>> > +++ test/Driver/cl-options.c (working copy)
>> > @@ -82,6 +82,10 @@
>> >  // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck
>> > -check-prefix=SLASH_I %s
>> >  // SLASH_I: "-I" "myincludedir"
>> >
>> > +// RUN: %clang_cl /imsvcmyincludedir -### -- %s 2>&1 | FileCheck
>> > -check-prefix=SLASH_imsvc %s
>> > +// RUN: %clang_cl /imsvc myincludedir -### -- %s 2>&1 | FileCheck
>> > -check-prefix=SLASH_imsvc %s
>> > +// SLASH_imsvc: "-internal-isystem" "myincludedir"
>>
>> Would it be possible to check that this flag comes after the
>> -internal-isystem for the resource headers?
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 53437.
etienneb marked 5 inline comments as done.
etienneb added a comment.

alexfh comments.


http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,186 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(int) * sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' expression
+  sum += sizeof(ptr) * sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' expression
+  sum += sizeof(int) * (2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' expression
+  sum += (2 * sizeof(char)) * sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' expression
+  if (sizeof(A) < 0x10) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  if (sizeof(A) <= 0xFFFEU) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspicious usage of '

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb added inline comments.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:27
@@ +26,3 @@
+
+AST_MATCHER(BinaryOperator, isRelationnalOperator) {
+  return Node.isRelationalOp();

alexfh wrote:
> alexfh wrote:
> > nit: extra 'n' in "relational"
> I'd put this to ASTMatchers.h together with a test and docs.
There are a few matchers used in different checkers that worth lifting.
It seems there is a place to lift some of them in clang-tidy/utils, and some of 
them should be lifted to astmatcher directly.
I wonder what should be the rules to lift them.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:196
@@ +195,3 @@
+  expr(SizeOfExpr, unless(SizeOfZero),
+   hasSizeOfAncestor(SizeOfExpr.bind("sizeof-sizeof-expr"))),
+  this);

alexfh wrote:
> It looks like you've created a tool for what can be done in a simpler way. It 
> should be possible to express this matcher without going through the parent 
> map: `sizeOfExpr(hasDescendant(sizeOfExpr(unless(SizeOfZero` or something 
> like that.
This is 'almost' working!
We cannot pass through all expression.
As an example, callExpr or arrayExpr

```
sizeof( A[index(sizeof(int))] )
```

In this example, the checker won't warn.

So, the hasDescendant (or has ancestor) in this case is only allow to follow a 
subset of Expressions.

The name may be wrong, or unclear. If you have better proposition...


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197
@@ +196,3 @@
+  this);
+}
+

alexfh wrote:
> What about this comment?
unsubmited. (not sure to get when arc is pushing them)


http://reviews.llvm.org/D19014



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

Re: [PATCH] D14286: ASTImporter: expressions, pt.1

[Sean Callanan via cfe-commits]

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

This all looks fine to me, and the LLDB test suite is happy also.


http://reviews.llvm.org/D14286



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

Re: [PATCH] D12761: MPI-Checker patch for Clang Static Analyzer

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

Alexander,

This patch is in a pretty good shape. I am fine with committing it and 
iterating with smaller updates in tree if it is more convenient for you.

One task that I would like to very strongly encourage is running this on a lot 
of code. You will find a lot of issues this way that are hard to discover 
during code review. Both false positives and crashes.

Thanks!
Anna.



Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIBugReporter.cpp:96
@@ +95,3 @@
+if (const CallExpr *CE = clang::dyn_cast(SP->getStmt())) {
+
+  auto FuncIdentifier = CE->getDirectCallee()->getIdentifier();

The advantage of using the state is that it will be much more robust to any 
further changes to the compiler/checker because you will not be pattern 
matching the AST but instead will be checking the state, which the core 
reasoning is based on. One example that comes to mind is indirect calls. You 
will reduce the amount of code here as well, simplifying maintainability. This 
is the pattern we use in other checkers as well, so there is a remote chance we 
could introduce a new simplified API that will do the walk for the checker 
writers.

With respect to your example. Does it come up in practice? Wouldn't you warn on 
the second nonblocking request anyway? Could you add such an example to the 
tests? (Would be good in any case. If you leave the code as is, you can point 
to that example as the motivation.)


Comment at: lib/StaticAnalyzer/Checkers/MPI-Checker/MPIChecker.cpp:87
@@ +86,3 @@
+  }
+  // A wait has no matching nonblocking call.
+  BReporter->reportUnmatchedWait(PreCallEvent, ReqRegion, ErrorNode);

This is done, right?


Comment at: test/Analysis/MPIChecker.cpp:99
@@ +98,3 @@
+  MPI_Wait(&req, MPI_STATUS_IGNORE);
+}
+

This are explaining the path on which the problem occurs; the users will see 
them as well. There should not be a lot of those, you do not have a lot of 
conditions. Would it be reasonable to change the tests to incorporate those. 
Other alternative is to have another tests file that tests the notes in that 
mode.

What do you think?


Comment at: test/Analysis/MPIChecker.cpp:114
@@ +113,3 @@
+
+void doubleNonblocking4() {
+  int rank = 0;

> I would then simply create a new pair of .cpp and .h files in the test folder 
> where I define those functions so that the MPI-Checker tests can use them.

You do not have to do that. You could just declare the functions and not define 
them. It will be equivalent to having the definitions in the other TUs.




http://reviews.llvm.org/D12761



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

Re: [PATCH] D18748: [AMDGPU] Add debugger related target options

[Matt Arsenault via cfe-commits]

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

LGTM


http://reviews.llvm.org/D18748



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

Re: [PATCH] D18713: [OpenCL] Generate bitcast when target address space does not change.

[Yaxun Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL266107: [OpenCL] Handle AddressSpaceConversion when target 
address space does not… (authored by yaxunl).

Changed prior to commit:
  http://reviews.llvm.org/D18713?vs=52800&id=53439#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D18713

Files:
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl

Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -1411,7 +1411,10 @@
   }
   case CK_AddressSpaceConversion: {
 Value *Src = Visit(const_cast(E));
-return Builder.CreateAddrSpaceCast(Src, ConvertType(DestTy));
+// Since target may map different address spaces in AST to the same address
+// space, an address space conversion may end up as a bitcast.
+return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
+   ConvertType(DestTy));
   }
   case CK_AtomicToNonAtomic:
   case CK_NonAtomicToAtomic:
Index: cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
===
--- cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -1,23 +1,41 @@
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 
-ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 
-emit-llvm -o - | FileCheck --check-prefix=CHECK-NOFAKE %s
+// When -ffake-address-space-map is not used, all addr space mapped to 0 for 
x86_64.
 
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
 void test(global int *arg_glob, generic int *arg_gen) {
   int var_priv;
   arg_gen = arg_glob; // implicit cast global -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 
addrspace(4)*
+  // CHECK-NOFAKE-NOT: addrspacecast
 
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  // CHECK-NOFAKE-NOT: addrspacecast
+
   arg_glob = (global int *)arg_gen; // explicit cast
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(4)* %{{[0-9]+}} to i32 
addrspace(1)*
+  // CHECK-NOFAKE-NOT: addrspacecast
+
   global int *var_glob =
   (global int *)arg_glob; // explicit cast in the same address space
   // CHECK-NOT: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to 
i32 addrspace(1)*
+  // CHECK-NOFAKE-NOT: addrspacecast
+
   var_priv = arg_gen - arg_glob; // arithmetic operation
   // CHECK: %{{.*}} = ptrtoint i32 addrspace(4)* %{{.*}} to i64
   // CHECK: %{{.*}} = ptrtoint i32 addrspace(1)* %{{.*}} to i64
+  // CHECK-NOFAKE: %{{.*}} = ptrtoint i32* %{{.*}} to i64
+  // CHECK-NOFAKE: %{{.*}} = ptrtoint i32* %{{.*}} to i64
+
   var_priv = arg_gen > arg_glob; // comparison
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 
addrspace(4)*
+
+  generic void *var_gen_v = arg_glob;
+  // CHECK: addrspacecast
+  // CHECK-NOT: bitcast
+  // CHECK-NOFAKE: bitcast
+  // CHECK-NOFAKE-NOT: addrspacecast
 }


Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -1411,7 +1411,10 @@
   }
   case CK_AddressSpaceConversion: {
 Value *Src = Visit(const_cast(E));
-return Builder.CreateAddrSpaceCast(Src, ConvertType(DestTy));
+// Since target may map different address spaces in AST to the same address
+// space, an address space conversion may end up as a bitcast.
+return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
+   ConvertType(DestTy));
   }
   case CK_AtomicToNonAtomic:
   case CK_NonAtomicToAtomic:
Index: cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
===
--- cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -1,23 +1,41 @@
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 -emit-llvm -o - | FileCheck --check-prefix=CHECK-NOFAKE %s
+// When -ffake-address-space-map is not used, all addr space mapped to 0 for x86_64.
 
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
 void test(global int *arg_glob, generic int *arg_gen) {
   int var_priv;
   arg_gen = arg_glob; 

r266108 - clang-cl: Remove -isystem, add -imsvc.

[Nico Weber via cfe-commits]

Author: nico
Date: Tue Apr 12 14:04:37 2016
New Revision: 266108

URL: http://llvm.org/viewvc/llvm-project?rev=266108&view=rev
Log:
clang-cl: Remove -isystem, add -imsvc.

r260990 exposed -isystem in clang-cl. -isystem adds a directory to the front of
the system include search path. The idea was to use this to point to a hermetic
msvc install, but as it turns out this doesn't work: -isystem then adds the
hermetic headers in front of clang's builtin headers, and clang's headers that
are supposed to wrap msvc headers (say, stdarg.h) aren't picked up at all
anymore.

So revert that, and instead expose -imsvc which works as if the passed
directory was part of %INCLUDE%: The header is treated as a system header, but
it is searched after clang's lib/Header headers.

Fixes half of PRPR26751.

Modified:
cfe/trunk/include/clang/Driver/CLCompatOptions.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/Driver/MSVCToolChain.cpp
cfe/trunk/test/Driver/cl-options.c
cfe/trunk/test/Driver/cl-pch-search.cpp

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=266108&r1=266107&r2=266108&view=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Tue Apr 12 14:04:37 2016
@@ -210,6 +210,9 @@ def _SLASH_GX : CLFlag<"GX">,
   HelpText<"Enable exception handling">;
 def _SLASH_GX_ : CLFlag<"GX-">,
   HelpText<"Enable exception handling">;
+def _SLASH_imsvc : CLJoinedOrSeparate<"imsvc">,
+  HelpText<"Add directory to system include search path, as if part of 
%INCLUDE%">,
+  MetaVarName<"">;
 def _SLASH_LD : CLFlag<"LD">, HelpText<"Create DLL">;
 def _SLASH_LDd : CLFlag<"LDd">, HelpText<"Create debug DLL">;
 def _SLASH_link : CLRemainingArgs<"link">,

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=266108&r1=266107&r2=266108&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Apr 12 14:04:37 2016
@@ -1254,7 +1254,7 @@ def iquote : JoinedOrSeparate<["-"], "iq
 def isysroot : JoinedOrSeparate<["-"], "isysroot">, Group, 
Flags<[CC1Option]>,
   HelpText<"Set the system root directory (usually /)">, MetaVarName<"">;
 def isystem : JoinedOrSeparate<["-"], "isystem">, Group,
-  Flags<[CC1Option, CoreOption]>,
+  Flags<[CC1Option]>,
   HelpText<"Add directory to SYSTEM include search path">, 
MetaVarName<"">;
 def iwithprefixbefore : JoinedOrSeparate<["-"], "iwithprefixbefore">, 
Group,
   HelpText<"Set directory to include search path with prefix">, 
MetaVarName<"">,

Modified: cfe/trunk/lib/Driver/MSVCToolChain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/MSVCToolChain.cpp?rev=266108&r1=266107&r2=266108&view=diff
==
--- cfe/trunk/lib/Driver/MSVCToolChain.cpp (original)
+++ cfe/trunk/lib/Driver/MSVCToolChain.cpp Tue Apr 12 14:04:37 2016
@@ -527,6 +527,10 @@ void MSVCToolChain::AddClangSystemInclud
   "include");
   }
 
+  // Add %INCLUDE%-like directories from the -imsvc flag.
+  for (const auto &Path : 
DriverArgs.getAllArgValues(options::OPT__SLASH_imsvc))
+addSystemInclude(DriverArgs, CC1Args, Path);
+
   if (DriverArgs.hasArg(options::OPT_nostdlibinc))
 return;
 

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=266108&r1=266107&r2=266108&view=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Tue Apr 12 14:04:37 2016
@@ -82,6 +82,12 @@
 // RUN: %clang_cl /I myincludedir -### -- %s 2>&1 | FileCheck 
-check-prefix=SLASH_I %s
 // SLASH_I: "-I" "myincludedir"
 
+// RUN: %clang_cl /imsvcmyincludedir -### -- %s 2>&1 | FileCheck 
-check-prefix=SLASH_imsvc %s
+// RUN: %clang_cl /imsvc myincludedir -### -- %s 2>&1 | FileCheck 
-check-prefix=SLASH_imsvc %s
+// Clang's resource header directory should be first:
+// SLASH_imsvc: "-internal-isystem" "{{[^"]*}}lib{{.}}clang{{[^"]*}}include"
+// SLASH_imsvc: "-internal-isystem" "myincludedir"
+
 // RUN: %clang_cl /J -### -- %s 2>&1 | FileCheck -check-prefix=J %s
 // J: -fno-signed-char
 
@@ -452,7 +458,6 @@
 // RUN: -fno-ms-compatibility \
 // RUN: -fms-extensions \
 // RUN: -fno-ms-extensions \
-// RUN: -isystem=some/path \
 // RUN: -mllvm -disable-llvm-optzns \
 // RUN: -Wunused-variable \
 // RUN: -fmacro-backtrace-limit=0 \

Modified: cfe/trunk/test/Driver/cl-pch-search.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test

RE: r262817 - [CLANG][AVX512][BUILTIN] Adding vpmultishiftqb{128|256|512}

[Demikhovsky, Elena via cfe-commits]

Yes, we’ll try make the patches smaller and avoid such mistakes.

-   Elena

From: Chandler Carruth [mailto:chandl...@google.com]
Sent: Tuesday, April 12, 2016 20:25
To: Zuckerman, Michael ; 
cfe-commits@lists.llvm.org; Demikhovsky, Elena 
Subject: Re: r262817 - [CLANG][AVX512][BUILTIN] Adding 
vpmultishiftqb{128|256|512}

On Mon, Mar 7, 2016 at 12:33 AM Michael Zuckerman via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: mzuckerm
Date: Mon Mar  7 02:29:10 2016
New Revision: 262817

URL: http://llvm.org/viewvc/llvm-project?rev=262817&view=rev
Log:
[CLANG][AVX512][BUILTIN] Adding vpmultishiftqb{128|256|512}

This commit log seems super confusing combined with:

+TARGET_BUILTIN(__builtin_ia32_pbroadcastb512_gpr_mask, 
"V64ccV64cULLi","","avx512bw")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastb128_gpr_mask, 
"V16ccV16cUs","","avx512bw,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastb256_gpr_mask, 
"V32ccV32cUi","","avx512bw,avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastd128_gpr_mask, 
"V4iiV4iUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastd256_gpr_mask, 
"V8iiV8iUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastq128_gpr_mask, 
"V2LLiULLiV2LLiUc","","avx512vl")
+TARGET_BUILTIN(__builtin_ia32_pbroadcastq256_gpr_mask, 
"V4LLiULLiV4LLiUc","","avx512vl")

This is adding *broadcast* intrinsics!!! Not multishift!

Please be much more careful with your patch descriptions. I just happened to 
randomly notice this.

Elena, please actually carefully review these patches.
-
Intel Israel (74) Limited

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

r266107 - [OpenCL] Handle AddressSpaceConversion when target address space does not change.

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 12 14:03:49 2016
New Revision: 266107

URL: http://llvm.org/viewvc/llvm-project?rev=266107&view=rev
Log:
[OpenCL] Handle AddressSpaceConversion when target address space does not 
change.

In codegen different address spaces may be mapped to the same address
space for a target, e.g. in x86/x86-64 all address spaces are mapped
to 0. Therefore AddressSpaceConversion should be translated by
CreatePointerBitCastOrAddrSpaceCast instead of CreateAddrSpaceCast.

Differential Revision: http://reviews.llvm.org/D18713

Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=266107&r1=266106&r2=266107&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Apr 12 14:03:49 2016
@@ -1411,7 +1411,10 @@ Value *ScalarExprEmitter::VisitCastExpr(
   }
   case CK_AddressSpaceConversion: {
 Value *Src = Visit(const_cast(E));
-return Builder.CreateAddrSpaceCast(Src, ConvertType(DestTy));
+// Since target may map different address spaces in AST to the same address
+// space, an address space conversion may end up as a bitcast.
+return Builder.CreatePointerBitCastOrAddrSpaceCast(Src,
+   ConvertType(DestTy));
   }
   case CK_AtomicToNonAtomic:
   case CK_NonAtomicToAtomic:

Modified: cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl?rev=266107&r1=266106&r2=266107&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl Tue Apr 12 
14:03:49 2016
@@ -1,4 +1,6 @@
 // RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 
-ffake-address-space-map -cl-std=CL2.0 -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1 %s -triple x86_64-unknown-linux-gnu -O0 -cl-std=CL2.0 
-emit-llvm -o - | FileCheck --check-prefix=CHECK-NOFAKE %s
+// When -ffake-address-space-map is not used, all addr space mapped to 0 for 
x86_64.
 
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
@@ -7,17 +9,33 @@ void test(global int *arg_glob, generic
   int var_priv;
   arg_gen = arg_glob; // implicit cast global -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 
addrspace(4)*
+  // CHECK-NOFAKE-NOT: addrspacecast
 
   arg_gen = &var_priv; // implicit cast with obtaining adr, private -> generic
   // CHECK: %{{[0-9]+}} = addrspacecast i32* %var_priv to i32 addrspace(4)*
+  // CHECK-NOFAKE-NOT: addrspacecast
+
   arg_glob = (global int *)arg_gen; // explicit cast
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(4)* %{{[0-9]+}} to i32 
addrspace(1)*
+  // CHECK-NOFAKE-NOT: addrspacecast
+
   global int *var_glob =
   (global int *)arg_glob; // explicit cast in the same address space
   // CHECK-NOT: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to 
i32 addrspace(1)*
+  // CHECK-NOFAKE-NOT: addrspacecast
+
   var_priv = arg_gen - arg_glob; // arithmetic operation
   // CHECK: %{{.*}} = ptrtoint i32 addrspace(4)* %{{.*}} to i64
   // CHECK: %{{.*}} = ptrtoint i32 addrspace(1)* %{{.*}} to i64
+  // CHECK-NOFAKE: %{{.*}} = ptrtoint i32* %{{.*}} to i64
+  // CHECK-NOFAKE: %{{.*}} = ptrtoint i32* %{{.*}} to i64
+
   var_priv = arg_gen > arg_glob; // comparison
   // CHECK: %{{[0-9]+}} = addrspacecast i32 addrspace(1)* %{{[0-9]+}} to i32 
addrspace(4)*
+
+  generic void *var_gen_v = arg_glob;
+  // CHECK: addrspacecast
+  // CHECK-NOT: bitcast
+  // CHECK-NOFAKE: bitcast
+  // CHECK-NOFAKE-NOT: addrspacecast
 }


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

Re: [PATCH] D18997: [SemaObjC] Properly diagnose type arguments and protocols mix in parameterized classes

[Bruno Cardoso Lopes via cfe-commits]

bruno added a comment.

Hi Manman,



Comment at: lib/Parse/ParseObjc.cpp:1697
@@ -1696,3 +1696,3 @@
   // We syntactically matched a type argument, so commit to parsing
-  // type arguments.
+  // type arguments. Finding protocol arguments after here is an error.
 

manmanren wrote:
> I am a little confused about the original comment. Have we syntactically 
> matched a type argument? All we have done is trying to parse the single 
> identifiers, right?
Yes, you're right! Going to update the entire comment to be more accurate


Comment at: lib/Parse/ParseObjc.cpp:1720
@@ -1714,3 +1719,3 @@
   if (fullTypeArg.isUsable())
 typeArgs.push_back(fullTypeArg.get());
   else

manmanren wrote:
> Should we set foundValidTypeId here too?
See the comment below


Comment at: lib/Parse/ParseObjc.cpp:1722
@@ -1716,3 +1721,3 @@
   else
 invalid = true;
 } else {

manmanren wrote:
> Should we emit diagnostics for all cases we set invalid to true?
The initial version of the patch was doing this. However, I could not come up 
with a testcase to exercise it, so I left it out. Gonna try harder!


Comment at: lib/Parse/ParseObjc.cpp:1726
@@ +1725,3 @@
+  auto *P = Actions.LookupProtocol(identifiers[i], identifierLocs[i]);
+  if (!P) {
+unknownTypeArgs.push_back(identifiers[i]);

manmanren wrote:
> Please add comments here: not a type argument, not a protocol, treat it as 
> unknown type.
Ok


Comment at: lib/Parse/ParseObjc.cpp:1729
@@ +1728,3 @@
+unknownTypeArgsLoc.push_back(identifierLocs[i]);
+continue;
+  }

manmanren wrote:
> Is it necessary to have a "continue" here? Can we restructure this to get rid 
> of the "continue"?
Did this to remove some level of nested "if"s which I personally dislike, but 
can easily change that


http://reviews.llvm.org/D18997



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:28
@@ +27,3 @@
+  return Node.getValue().getZExtValue() > N;
+}
+

There are no firm rules. It mostly depends on how generic/useful in other tools 
the matcher could be. This one seems pretty generic and it could be used in a 
couple of other clang-tidy checks at least (e.g. 
readability/ContainerSizeEmptyCheck.cpp), so it seems to be a good fit for 
ASTMatchers.h. This can also be done as a separate step, if you prefer.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:197
@@ +196,3 @@
+  this);
+}
+

I see the reasoning, but in this case I'd still prefer if this custom matcher 
looked at the children instead of the parents, since it's a more effective 
approach in general (at least because it doesn't use the parent map).

We should probably add a variant of `hasDescendant` (and maybe `hasAncestor`) 
with a way to limit which nodes it can look through.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:257
@@ +256,3 @@
+ Result.Nodes.getNodeAs("sizeof-multiply-sizeof")) {
+diag(E->getLocStart(), "suspicious 'sizeof' by 'sizeof' expression");
+  }

"'sizeof' by 'sizeof' expression" is ambiguous, "'sizeof' by 'sizeof' 
multiplication" would be better.


http://reviews.llvm.org/D19014



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

Re: [PATCH] D18849: [modules] Avoid module relocation error when building modules from VFS

[Bruno Cardoso Lopes via cfe-commits]

bruno added a comment.

Hi Ben,



Comment at: lib/Basic/FileManager.cpp:221-223
@@ -220,2 +220,5 @@
   // See if there is already an entry in the map.
+  // FIXME: Note that when first requested, the returned file name equals to 
the
+  // requested path. This is not true when returning a cached result. This is
+  // inconsistent and might lead to clients making the wrong assumptions.
   if (NamedFileEnt.second)

benlangmuir wrote:
> The first time we lookup a file, we will return whatever name comes back from 
> the VFS, which may or may not be the requested path.  If this is a real file 
> system, it will be the requested path.  If it is a redirecting file system 
> and "use-external-names" is true (which it is by default), then the VFS will 
> return the external contents path, which is generally not the same as the 
> requested path.
The behaviour I'm seeing for VFS is a bit different: If it is a redirecting 
file system and "use-external-names" is true, then the FileManager returns the 
requested file path. Sub-sequential cached queries return the 
"use-external-names" / real-path instead. This is the rationale why I'm trying 
to thread the FileName instead of the FileEntry. Am I missing something?


Comment at: lib/Lex/HeaderSearch.cpp:493
@@ -492,3 +492,3 @@
 // Find the framework in which this header occurs.
-StringRef FrameworkPath = FE->getDir()->getName();
+StringRef FrameworkPath = llvm::sys::path::parent_path(FrameworkName);
 bool FoundFramework = false;

benlangmuir wrote:
> What does this change do?  FE->getDir()->getName() will be the virtual path 
> if there is VFS, which is usually the one you want to operate on.
You're right! I assumed FE->getDir()->getName() would give me the real path 
directory for the file in question, but I double checked here and that doesn't 
happen.


http://reviews.llvm.org/D18849



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

r266109 - [analyzer] Nullability: Suppress return diagnostics in inlined functions.

[Devin Coughlin via cfe-commits]

Author: dcoughlin
Date: Tue Apr 12 14:29:52 2016
New Revision: 266109

URL: http://llvm.org/viewvc/llvm-project?rev=266109&view=rev
Log:
[analyzer] Nullability: Suppress return diagnostics in inlined functions.

The nullability checker can sometimes miss detecting nullability precondition
violations in inlined functions because the binding for the parameter
that violated the precondition becomes dead before the return:

int * _Nonnull callee(int * _Nonnull p2) {
  if (!p2)
// p2 becomes dead here, so binding removed.
return 0; // warning here because value stored in p2 is symbolic.
  else
   return p2;
}

int *caller(int * _Nonnull p1) {
  return callee(p1);
}

The fix, which is quite blunt, is to not warn about null returns in inlined
methods/functions. This won’t lose much coverage for ObjC because the analyzer
always analyzes each ObjC method at the top level in addition to inlined. It
*will* lose coverage for C — but there aren’t that many codebases with C
nullability annotations.

rdar://problem/25615050

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
cfe/trunk/test/Analysis/nullability.mm

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp?rev=266109&r1=266108&r2=266109&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp Tue Apr 12 
14:29:52 2016
@@ -562,7 +562,8 @@ void NullabilityChecker::checkPreStmt(co
   if (Filter.CheckNullReturnedFromNonnull &&
   NullReturnedFromNonNull &&
   RetExprTypeLevelNullability != Nullability::Nonnull &&
-  !InSuppressedMethodFamily) {
+  !InSuppressedMethodFamily &&
+  C.getLocationContext()->inTopFrame()) {
 static CheckerProgramPointTag Tag(this, "NullReturnedFromNonnull");
 ExplodedNode *N = C.generateErrorNode(State, &Tag);
 if (!N)

Modified: cfe/trunk/test/Analysis/nullability.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullability.mm?rev=266109&r1=266108&r2=266109&view=diff
==
--- cfe/trunk/test/Analysis/nullability.mm (original)
+++ cfe/trunk/test/Analysis/nullability.mm Tue Apr 12 14:29:52 2016
@@ -238,6 +238,41 @@ void testPreconditionViolationInInlinedF
   doNotWarnWhenPreconditionIsViolated(p);
 }
 
+@interface TestInlinedPreconditionViolationClass : NSObject
+@end
+
+@implementation TestInlinedPreconditionViolationClass
+-(Dummy * _Nonnull) calleeWithParam:(Dummy * _Nonnull) p2 {
+  Dummy *x = 0;
+  if (!p2) // p2 binding becomes dead at this point.
+return x; // no-warning
+  else
+   return p2;
+}
+
+-(Dummy *)callerWithParam:(Dummy * _Nonnull) p1 {
+  return [self calleeWithParam:p1];
+}
+
+@end
+
+int * _Nonnull InlinedPreconditionViolationInFunctionCallee(int * _Nonnull p2) 
{
+  int *x = 0;
+  if (!p2) // p2 binding becomes dead at this point.
+return x; // no-warning
+  else
+   return p2;
+}
+
+int * _Nonnull InlinedReturnNullOverSuppressionCallee(int * _Nonnull p2) {
+  int *result = 0;
+  return result; // no-warning; but this is an over suppression
+}
+
+int *InlinedReturnNullOverSuppressionCaller(int * _Nonnull p1) {
+  return InlinedReturnNullOverSuppressionCallee(p1);
+}
+
 void inlinedNullable(Dummy *_Nullable p) {
   if (p) return;
 }


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

Re: [PATCH] D13610: [CodeGen] Fix CodeGenModule::CreateGlobalInitOrDestructFunction

[Keno Fischer via cfe-commits]

loladiro added a subscriber: loladiro.
loladiro added a comment.

Thanks for this. I was about to create the same patch (after seeing this broken 
on 3.7) only to see it had already been done!


Repository:
  rL LLVM

http://reviews.llvm.org/D13610



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

Re: [PATCH] D18821: Add modernize-bool-to-integer-conversion

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

BTW, why is the check in the 'modernize' module? It doesn't seem to make 
anything more modern. I would guess, the pattern it detects is most likely to 
result from a programming error. Also, the fix, though it retains the behavior, 
has a high chance to be incorrect. Can you share the results of running this 
check on LLVM? At least, how many problems it found and how many times the 
suggested fix was correct.

I'd suggest to move the check to `misc` or maybe it's time to create a separate 
directory for checks targeting various bug-prone patterns.



Comment at: clang-tidy/modernize/BoolToIntegerConversionCheck.cpp:43
@@ +42,3 @@
+
+  if (!Location.isMacroID())
+Diag << 0

nit: Since both positive and negative branches are present, I'd prefer to make 
the condition slightly simple by removing the negation.


http://reviews.llvm.org/D18821



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Etienne Bergeron via cfe-commits]

etienneb updated this revision to Diff 53442.
etienneb marked an inline comment as done.
etienneb added a comment.

alexfh comments.


http://reviews.llvm.org/D19014

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/MiscTidyModule.cpp
  clang-tidy/misc/SizeofExpressionCheck.cpp
  clang-tidy/misc/SizeofExpressionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-sizeof-expression.rst
  test/clang-tidy/misc-sizeof-expression.cpp

Index: test/clang-tidy/misc-sizeof-expression.cpp
===
--- /dev/null
+++ test/clang-tidy/misc-sizeof-expression.cpp
@@ -0,0 +1,186 @@
+// RUN: %check_clang_tidy %s misc-sizeof-expression %t
+
+class C {
+  int size() { return sizeof(this); }
+  // CHECK-MESSAGES: :[[@LINE-1]]:23: warning: suspicious usage of 'sizeof(this)'
+};
+
+#define LEN 8
+
+int X;
+extern int A[10];
+extern short B[10];
+
+#pragma pack(1)
+struct  S { char a, b, c; };
+
+int Test1(const char* ptr) {
+  int sum = 0;
+  sum += sizeof(LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(LEN + 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(K)'
+  sum += sizeof(sum, LEN);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(..., ...)'
+  sum += sizeof(sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + LEN + sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + (LEN + sizeof(X)));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(LEN + - + -sizeof(X));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(sizeof(...))'
+  sum += sizeof(char) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T)/sizeof(T)'
+  sum += sizeof(A) / sizeof(S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(char) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(B[0]) / sizeof(A);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(...)/sizeof(...)'; numerator is not a multiple of denominator
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(char*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(void*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(const void volatile*);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(P*)/sizeof(Q*)'
+  sum += sizeof(ptr) / sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(ptr) / sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of sizeof pointer 'sizeof(T*)/sizeof(T)'
+  sum += sizeof(int) * sizeof(char);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+  sum += sizeof(ptr) * sizeof(ptr[0]);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+  sum += sizeof(int) * (2 * sizeof(char));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+  sum += (2 * sizeof(char)) * sizeof(int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious 'sizeof' by 'sizeof' multiplication
+  if (sizeof(A) < 0x10) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  if (sizeof(A) <= 0xFFFEU) sum += 42;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: suspicious comparison of 'sizeof(expr)' to a constant 
+  return sum;
+}
+
+typedef char MyChar;
+typedef const MyChar MyConstChar;
+
+int CE0 = sizeof sizeof(char);
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: suspi

Re: [PATCH] D18849: [modules] Avoid module relocation error when building modules from VFS

[Ben Langmuir via cfe-commits]

benlangmuir added inline comments.


Comment at: lib/Basic/FileManager.cpp:221-223
@@ -220,2 +220,5 @@
   // See if there is already an entry in the map.
+  // FIXME: Note that when first requested, the returned file name equals to 
the
+  // requested path. This is not true when returning a cached result. This is
+  // inconsistent and might lead to clients making the wrong assumptions.
   if (NamedFileEnt.second)

bruno wrote:
> benlangmuir wrote:
> > The first time we lookup a file, we will return whatever name comes back 
> > from the VFS, which may or may not be the requested path.  If this is a 
> > real file system, it will be the requested path.  If it is a redirecting 
> > file system and "use-external-names" is true (which it is by default), then 
> > the VFS will return the external contents path, which is generally not the 
> > same as the requested path.
> The behaviour I'm seeing for VFS is a bit different: If it is a redirecting 
> file system and "use-external-names" is true, then the FileManager returns 
> the requested file path. Sub-sequential cached queries return the 
> "use-external-names" / real-path instead. This is the rationale why I'm 
> trying to thread the FileName instead of the FileEntry. Am I missing 
> something?
That's not supposed to happen.  You can see below that we always use the name 
from the StatCache, which in turn comes from the the VFS status() result.  Have 
you traced through what's happening here?  We should figure out how we got the 
requested path as the name.


http://reviews.llvm.org/D18849



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

r266111 - PR19957: [OpenCL] Incorrectly accepts implicit address space conversion with ternary operator.

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Apr 12 14:43:36 2016
New Revision: 266111

URL: http://llvm.org/viewvc/llvm-project?rev=266111&view=rev
Log:
PR19957: [OpenCL] Incorrectly accepts implicit address space conversion with 
ternary operator.

Generates addrspacecast instead of bitcast for ternary operator when necessary, 
and diagnose ternary operator with incompatible second and third operands.

https://llvm.org/bugs/show_bug.cgi?id=19957

Differential Revision: http://reviews.llvm.org/D17412

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=266111&r1=266110&r2=266111&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Apr 12 14:43:36 
2016
@@ -5376,7 +5376,9 @@ def ext_typecheck_comparison_of_distinct
   "composite pointer type %2">, InGroup;
 def err_typecheck_op_on_nonoverlapping_address_space_pointers : Error<
   "%select{comparison between %diff{ ($ and $)|}0,1"
-  "|arithmetic operation with operands of type %diff{ ($ and $)|}0,1}2"
+  "|arithmetic operation with operands of type %diff{ ($ and $)|}0,1"
+  "|conditional operator with the second and third operands of type "
+  "%diff{ ($ and $)|}0,1}2"
   " which are pointers to non-overlapping address spaces">;
 
 def err_typecheck_assign_const : Error<

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=266111&r1=266110&r2=266111&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Tue Apr 12 14:43:36 2016
@@ -7617,6 +7617,15 @@ QualType ASTContext::mergeTypes(QualType
   Qualifiers LQuals = LHSCan.getLocalQualifiers();
   Qualifiers RQuals = RHSCan.getLocalQualifiers();
   if (LQuals != RQuals) {
+if (getLangOpts().OpenCL) {
+  if (LHS.getUnqualifiedType() != RHS.getUnqualifiedType() ||
+  LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers())
+return QualType();
+  if (LQuals.isAddressSpaceSupersetOf(RQuals))
+return LHS;
+  if (RQuals.isAddressSpaceSupersetOf(LQuals))
+return RHS;
+}
 // If any of these qualifiers are different, we have a type
 // mismatch.
 if (LQuals.getCVRQualifiers() != RQuals.getCVRQualifiers() ||

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=266111&r1=266110&r2=266111&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Apr 12 14:43:36 2016
@@ -6193,30 +6193,87 @@ static QualType checkConditionalPointerC
   lhptee = S.Context.getQualifiedType(lhptee.getUnqualifiedType(), lhQual);
   rhptee = S.Context.getQualifiedType(rhptee.getUnqualifiedType(), rhQual);
 
+  // For OpenCL:
+  // 1. If LHS and RHS types match exactly and:
+  //  (a) AS match => use standard C rules, no bitcast or addrspacecast
+  //  (b) AS overlap => generate addrspacecast
+  //  (c) AS don't overlap => give an error
+  // 2. if LHS and RHS types don't match:
+  //  (a) AS match => use standard C rules, generate bitcast
+  //  (b) AS overlap => generate addrspacecast instead of bitcast
+  //  (c) AS don't overlap => give an error
+
+  // For OpenCL, non-null composite type is returned only for cases 1a and 1b.
   QualType CompositeTy = S.Context.mergeTypes(lhptee, rhptee);
 
+  // OpenCL cases 1c, 2a, 2b, and 2c.
   if (CompositeTy.isNull()) {
-S.Diag(Loc, diag::ext_typecheck_cond_incompatible_pointers)
-  << LHSTy << RHSTy << LHS.get()->getSourceRange()
-  << RHS.get()->getSourceRange();
 // In this situation, we assume void* type. No especially good
 // reason, but this is what gcc does, and we do have to pick
 // to get a consistent AST.
-QualType incompatTy = S.Context.getPointerType(S.Context.VoidTy);
-LHS = S.ImpCastExprToType(LHS.get(), incompatTy, CK_BitCast);
-RHS = S.ImpCastExprToType(RHS.get(), incompatTy, CK_BitCast);
+QualType incompatTy;
+if (S.getLangOpts().OpenCL) {
+  // OpenCL v1.1 s6.5 - Conversion between pointers to distinct address
+  // spaces is disallowed.
+  unsigned ResultAddrSpace;
+  if (lhQual.isAddressSpaceSupersetOf(rhQual)) {
+// Cases 2a and 2b.
+ResultAddrSpace = lhQual.getAddressSpace();
+  } else if (rhQual.isAddressSpaceSupersetOf(lhQual)) {
+// Cases 2a and 

Re: [PATCH] D17412: PR19957: [OpenCL] incorrectly accepts implicit address space conversion with ternary operator

[Yaxun Liu via cfe-commits]

This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Closed by commit rL266111: PR19957: [OpenCL] Incorrectly accepts implicit 
address space conversion with… (authored by yaxunl).

Changed prior to commit:
  http://reviews.llvm.org/D17412?vs=52080&id=53445#toc

Repository:
  rL LLVM

http://reviews.llvm.org/D17412

Files:
  cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
  cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl

Index: cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
===
--- cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
+++ cfe/trunk/test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
@@ -225,3 +225,69 @@
 // expected-error@-2{{passing '__constant int *' to parameter of type '__generic int *' changes address space of pointer}}
 #endif
 }
+
+void test_ternary() {
+  AS int *var_cond;
+  generic int *var_gen;
+  global int *var_glob;
+  var_gen = 0 ? var_cond : var_glob;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local int *var_loc;
+  var_gen = 0 ? var_cond : var_loc;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant int *var_const;
+  var_cond = 0 ? var_cond : var_const;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private int *var_priv;
+  var_gen = 0 ? var_cond : var_priv;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  var_gen = 0 ? var_cond : var_gen;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic int *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  void *var_void_gen;
+  global char *var_glob_ch;
+  var_void_gen = 0 ? var_cond : var_glob_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__global char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  local char *var_loc_ch;
+  var_void_gen = 0 ? var_cond : var_loc_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and '__local char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  constant void *var_void_const;
+  constant char *var_const_ch;
+  var_void_const = 0 ? var_cond : var_const_ch;
+#ifndef CONSTANT
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|generic}} int *' and '__constant char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  private char *var_priv_ch;
+  var_void_gen = 0 ? var_cond : var_priv_ch;
+#ifndef GENERIC
+// expected-error-re@-2{{conditional operator with the second and third operands of type  ('__{{global|constant}} int *' and 'char *') which are pointers to non-overlapping address spaces}}
+#endif
+
+  generic char *var_gen_ch;
+  var_void_gen = 0 ? var_cond : var_gen_ch;
+#ifdef CONSTANT
+// expected-error@-2{{conditional operator with the second and third operands of type  ('__constant int *' and '__generic char *') which are pointers to non-overlapping address spaces}}
+#endif
+}
+
Index: cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
===
--- cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
+++ cfe/trunk/test/CodeGenOpenCL/address-spaces-conversions.cl
@@ -5,6 +5,7 @@
 // test that we generate address space casts everywhere we need conversions of
 // pointers to different address spaces
 
+// CHECK: define void @test
 void test(global int *arg_glob, generic int *arg_gen) {
   int var_priv;
   arg_gen = arg_glob; // implicit cast global -> generic
@@ -39,3 +40,41 @@
   // CHECK-NOFAKE: bitcast
   // CHECK-NOFAKE-NOT: addrspacecast
 }
+
+// Test ternary operator.
+// CHECK: define void @test_ternary
+void test_ternary(void) {
+  global int *var_glob;
+  generic int *var_gen;
+  generic int *var_gen2;
+  generic float *var_gen_f;
+  generic void *var_gen_v;
+
+  var_gen = var_gen ? var_gen : var_gen2; // operands of the sam

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Michael Miller via cfe-commits]

michael_miller updated this revision to Diff 53441.
michael_miller added a comment.

Removed accidentally duplicated test case.


http://reviews.llvm.org/D18584

Files:
  clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.h
  clang-tidy/utils/Matchers.h
  clang-tidy/utils/TypeTraits.cpp
  clang-tidy/utils/TypeTraits.h
  docs/LibASTMatchersReference.html
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  include/clang/ASTMatchers/ASTMatchers.h
  test/clang-tidy/cppcoreguidelines-pro-type-member-init-cxx98.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init-delayed.cpp
  test/clang-tidy/cppcoreguidelines-pro-type-member-init.cpp
  unittests/ASTMatchers/ASTMatchersTest.cpp

Index: unittests/ASTMatchers/ASTMatchersTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTest.cpp
@@ -2327,6 +2327,32 @@
   cxxConstructorDecl(isMoveConstructor(;
 }
 
+TEST(ConstructorDeclaration, IsUserProvided) {
+  EXPECT_TRUE(notMatches("struct S { int X = 0; };",
+ cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(notMatches("struct S { S() = default; };",
+ cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(notMatches("struct S { S() = delete; };",
+ cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(
+  matches("struct S { S(); };", cxxConstructorDecl(isUserProvided(;
+  EXPECT_TRUE(matches("struct S { S(); }; S::S(){}",
+  cxxConstructorDecl(isUserProvided(;
+}
+
+TEST(ConstructorDeclaration, IsDelegatingConstructor) {
+  EXPECT_TRUE(notMatches("struct S { S(); S(int); int X; };",
+ cxxConstructorDecl(isDelegatingConstructor(;
+  EXPECT_TRUE(notMatches("struct S { S(){} S(int X) : X(X) {} int X; };",
+ cxxConstructorDecl(isDelegatingConstructor(;
+  EXPECT_TRUE(matches(
+  "struct S { S() : S(0) {} S(int X) : X(X) {} int X; };",
+  cxxConstructorDecl(isDelegatingConstructor(), parameterCountIs(0;
+  EXPECT_TRUE(matches(
+  "struct S { S(); S(int X); int X; }; S::S(int X) : S() {}",
+  cxxConstructorDecl(isDelegatingConstructor(), parameterCountIs(1;
+}
+
 TEST(DestructorDeclaration, MatchesVirtualDestructor) {
   EXPECT_TRUE(matches("class Foo { virtual ~Foo(); };",
   cxxDestructorDecl(ofClass(hasName("Foo");
Index: include/clang/ASTMatchers/ASTMatchers.h
===
--- include/clang/ASTMatchers/ASTMatchers.h
+++ include/clang/ASTMatchers/ASTMatchers.h
@@ -4911,6 +4911,38 @@
   return Node.isDefaultConstructor();
 }
 
+/// \brief Matches constructor declarations that are user-provided.
+///
+/// Given
+/// \code
+///   struct S {
+/// S(); // #1
+/// S(const S &) = default; // #2
+/// S(S &&) = delete; // #3
+///   };
+/// \endcode
+/// cxxConstructorDecl(isUserProvided()) will match #1, but not #2 or #3.
+AST_MATCHER(CXXConstructorDecl, isUserProvided) {
+  return Node.isUserProvided();
+}
+
+/// \brief Matches constructors that delegate to another constructor.
+///
+/// Given
+/// \code
+///   struct S {
+/// S(); // #1
+/// S(int) {} // #2
+/// S(S &&) : S() {} // #3
+///   };
+///   S::S() : S(0) {} // #4
+/// \endcode
+/// cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
+/// #1 or #2.
+AST_MATCHER(CXXConstructorDecl, isDelegatingConstructor) {
+  return Node.isDelegatingConstructor();
+}
+
 /// \brief Matches constructor and conversion declarations that are marked with
 /// the explicit keyword.
 ///
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -1799,6 +1799,21 @@
 
 
 
+MatcherCXXConstructorDecl>isDelegatingConstructor
+Matches constructors that delegate to another constructor.
+
+Given
+  struct S {
+S(); #1
+S(int) {} #2
+S(S &&) : S() {} #3
+  };
+  S::S() : S(0) {} #4
+cxxConstructorDecl(isDelegatingConstructor()) will match #3 and #4, but not
+#1 or #2.
+
+
+
 MatcherCXXConstructorDecl>isExplicit
 Matches constructor and conversion declarations that are marked with
 the explicit keyword.
@@ -1828,6 +1843,19 @@
 
 
 
+MatcherCXXConstructorDecl>isUserProvided
+Matches constructor declarations that are user-provided.
+
+Given
+  struct S {
+S(); #1
+S(const S &) = default; #2
+S(S &&) = delete; #3
+  };
+cxxConstructorDecl(isUserProvided()) will match #1, but not #2 or #3.
+
+
+
 Matc

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Michael Miller via cfe-commits]

michael_miller added a comment.

In http://reviews.llvm.org/D18584#398298, @aaron.ballman wrote:

> LGTM with one minor correction (you can correct when committing it).


Oops! Not sure how that happened. I submitted a revised diff.


http://reviews.llvm.org/D18584



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

Re: [PATCH] D18073: Add memory allocating functions

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

"Since we are adding support for so many new APIs that are only available on 
Windows, could you please condition checking them only when we build for 
Windows. You probably can look and Language Options to figure that out."

malloc-uses.c -> "alternative-malloc-api.c" ?


http://reviews.llvm.org/D18073



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:198
@@ +197,3 @@
+}
+
+void SizeofExpressionCheck::check(const MatchFinder::MatchResult &Result) {

I'm probably wrong about "a more effective approach in general", but for 
`sizeof` case it might be more effective, if we assume that `sizeof` doesn't 
have large subtrees under it.

A separate concern is that the matchers looking up the tree are somewhat more 
difficult to understand (at least to me).

Ultimately, I'm not sure what's the best approach here. Maybe find a huge file 
and measure runtime of both implementations? ;)


http://reviews.llvm.org/D19014



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

Re: [PATCH] D19014: [clang-tidy] Add new checker for suspicious sizeof expressions

[Alexander Kornienko via cfe-commits]

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

Looks good with one more nit. I'm not sure what to do with `hasSizeOfAncestor`, 
we can leave it as is for now.



Comment at: clang-tidy/misc/SizeofExpressionCheck.cpp:47
@@ +46,3 @@
+  isa(E)) {
+ast_matchers::internal::Matcher M = hasSizeOfAncestor(InnerMatcher);
+return M.matches(*E, Finder, Builder);

The type here doesn't help, I'd use `auto`.


http://reviews.llvm.org/D19014



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

Re: [PATCH] D18595: MPI-Checker test helper

[Anna Zaks via cfe-commits]

zaks.anna added a comment.

You do not have to supply these. Just declaring and not defining the functions 
in the main test file should be sufficient.


http://reviews.llvm.org/D18595



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

r266113 - [modules] When an incompatible module file is explicitly provided for a module,

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Apr 12 14:58:30 2016
New Revision: 266113

URL: http://llvm.org/viewvc/llvm-project?rev=266113&view=rev
Log:
[modules] When an incompatible module file is explicitly provided for a module,
and we fall back to textual inclusion, don't require the module as a whole to
be marked available; it's OK if some other file in the same module is missing,
just as it would be if the header were explicitly marked textual.

Modified:
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Modules/explicit-build-missing-files.cpp

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=266113&r1=266112&r2=266113&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Tue Apr 12 14:58:30 2016
@@ -1675,7 +1675,10 @@ void Preprocessor::HandleIncludeDirectiv
   getLangOpts().CurrentModule) {
 // If this include corresponds to a module but that module is
 // unavailable, diagnose the situation and bail out.
-if (!SuggestedModule.getModule()->isAvailable()) {
+// FIXME: Remove this; loadModule does the same check (but produces
+// slightly worse diagnostics).
+if (!SuggestedModule.getModule()->isAvailable() &&
+!SuggestedModule.getModule()->HasIncompatibleModuleFile) {
   clang::Module::Requirement Requirement;
   clang::Module::UnresolvedHeaderDirective MissingHeader;
   Module *M = SuggestedModule.getModule();

Modified: cfe/trunk/test/Modules/explicit-build-missing-files.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/explicit-build-missing-files.cpp?rev=266113&r1=266112&r2=266113&view=diff
==
--- cfe/trunk/test/Modules/explicit-build-missing-files.cpp (original)
+++ cfe/trunk/test/Modules/explicit-build-missing-files.cpp Tue Apr 12 14:58:30 
2016
@@ -18,7 +18,7 @@
 // RUN:-fmodules-embed-all-files
 // RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s
 // RUN: not %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s -DERRORS 2>&1 
| FileCheck %s
-// RUN: rm %t/modulemap
+// RUN: mv %t/modulemap %t/modulemap.moved
 // RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s
 // RUN: not %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s -DERRORS 2>&1 
| FileCheck %s
 // RUN: rm %t/other.modulemap
@@ -32,6 +32,9 @@
 // RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s
 // RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/b.pcm %s
 // RUN: not %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s -DERRORS 2>&1 
| FileCheck %s --check-prefix=MISSING-B
+// RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm 
-fmodule-map-file=%t/modulemap.moved %s
+// RUN: not %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm 
-fmodule-map-file=%t/modulemap.moved -std=c++1z %s
+// RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm 
-fmodule-map-file=%t/modulemap.moved -std=c++1z 
-Wno-module-file-config-mismatch %s -Db=a
 // RUN: rm %t/a.h
 // RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/a.pcm %s -verify
 // RUN: %clang_cc1 -fmodules -I %t -fmodule-file=%t/b.pcm %s -verify


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

Re: [PATCH] D18584: Complete support for C++ Core Guidelines Type.6: Always initialize a member variable.

[Michael Miller via cfe-commits]

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

In http://reviews.llvm.org/D18584#398298, @aaron.ballman wrote:

> LGTM with one minor correction (you can correct when committing it).


Also, what's the next step? Is there anything else for me to do?


http://reviews.llvm.org/D18584



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

r266116 - [modules] Extend r266113 to cope with submodules.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Tue Apr 12 15:20:33 2016
New Revision: 266116

URL: http://llvm.org/viewvc/llvm-project?rev=266116&view=rev
Log:
[modules] Extend r266113 to cope with submodules.

Modified:
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Modules/explicit-build-missing-files.cpp

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=266116&r1=266115&r2=266116&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Tue Apr 12 15:20:33 2016
@@ -1678,7 +1678,9 @@ void Preprocessor::HandleIncludeDirectiv
 // FIXME: Remove this; loadModule does the same check (but produces
 // slightly worse diagnostics).
 if (!SuggestedModule.getModule()->isAvailable() &&
-!SuggestedModule.getModule()->HasIncompatibleModuleFile) {
+!SuggestedModule.getModule()
+ ->getTopLevelModule()
+ ->HasIncompatibleModuleFile) {
   clang::Module::Requirement Requirement;
   clang::Module::UnresolvedHeaderDirective MissingHeader;
   Module *M = SuggestedModule.getModule();

Modified: cfe/trunk/test/Modules/explicit-build-missing-files.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/explicit-build-missing-files.cpp?rev=266116&r1=266115&r2=266116&view=diff
==
--- cfe/trunk/test/Modules/explicit-build-missing-files.cpp (original)
+++ cfe/trunk/test/Modules/explicit-build-missing-files.cpp Tue Apr 12 15:20:33 
2016
@@ -3,7 +3,7 @@
 // RUN: echo 'extern int a; template int a2 = T::error;' > %t/a.h
 // RUN: echo 'extern int b;' > %t/b.h
 // RUN: echo 'extern int c = 0;' > %t/c.h
-// RUN: echo 'module a { header "a.h" header "b.h" header "c.h" }' > 
%t/modulemap
+// RUN: echo 'module a { module aa { header "a.h" header "b.h" header "c.h" } 
}' > %t/modulemap
 // RUN: echo 'module other {}' > %t/other.modulemap
 
 // We lazily check that the files referenced by an explicitly-specified .pcm


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