Re: r358104 - Don't emit an unreachable return block.

[John McCall via cfe-commits]

On 10 Apr 2019, at 21:50, wolfgang.p...@sony.com wrote:


Hi,
This commit seems to be causing a test failure on several buildbots 
(e.g. 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/26305). 
instrprof-shared-gcov-flush.test is failing because of differences in 
profile info. The test passes when I revert your change, so perhaps 
it's just a case of updating the test.

Could you please take a look?


Adjusted the test in r358149 as a hopeful fix.  CC'ing Marco since I 
believe this was originally his test.


John.



Thanks,
Wolfgang Pieb

-Original Message-
From: cfe-commits  On Behalf Of 
John McCall via cfe-commits

Sent: Wednesday, April 10, 2019 10:03 AM
To: cfe-commits@lists.llvm.org
Subject: r358104 - Don't emit an unreachable return block.

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104=358103=358104=diff

==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();

-// If that was the only use of the return value, nuke it as 
well now.

-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = 
dyn_cast(returnValueInst)) {

-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104=358103=358104=diff

==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions 
called by this

   //function.
   CurFn->addFnAttr("min-legal-vector-width", 
llvm::utostr(LargestVectorWidth));

+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = 
dyn_cast(ReturnValue.getPointer());

+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }

 /// ShouldInstrumentFunction - Return true if the current function 
should be


Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104=auto

==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104=358103=358104=diff

==
--- 

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194633.
hintonda edited the summary of this revision.
hintonda added a comment.

- Fix bug in updateArgStr() where it didn't handle isInAllSubCommands() 
correctly, and refactored code based on how SubCommands work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -98,6 +98,8 @@
   // This collects additional help to be printed.
   std::vector MoreHelp;
 
+  SmallVector, 4> DefaultOptions;
+
   // This collects the different option categories that have been registered.
   SmallPtrSet RegisteredOptionCategories;
 
@@ -111,6 +113,10 @@
 
   void ResetAllOptionOccurrences();
 
+  bool tryRemoveDefaultOption(Option *O, SubCommand *SC, StringRef NewArg);
+
+  void handleDefaultOptions();
+
   bool ParseCommandLineOptions(int argc, const char *const *argv,
StringRef Overview, raw_ostream *Errs = nullptr);
 
@@ -186,6 +192,13 @@
   }
 
   void addOption(Option *O) {
+// Handle DefaultOptions
+if (O->getMiscFlags() & cl::DefaultOption) {
+  std::string DefaultArg(O->ArgStr + ":default_option");
+  DefaultOptions.push_back(std::make_pair(DefaultArg, O));
+  O->setArgStr(DefaultOptions.back().first);
+}
+
 if (O->Subs.empty()) {
   addOption(O, &*TopLevelSubCommand);
 } else {
@@ -266,8 +279,13 @@
 if (O->Subs.empty())
   updateArgStr(O, NewName, &*TopLevelSubCommand);
 else {
-  for (auto SC : O->Subs)
-updateArgStr(O, NewName, SC);
+  if (O->isInAllSubCommands()) {
+for (auto SC : RegisteredSubCommands)
+  updateArgStr(O, NewName, SC);
+  } else {
+for (auto SC : O->Subs)
+  updateArgStr(O, NewName, SC);
+  }
 }
   }
 
@@ -1118,6 +1136,41 @@
   }
 }
 
+bool CommandLineParser::tryRemoveDefaultOption(Option *O, SubCommand *SC,
+   StringRef NewArg) {
+  StringRef Val;
+  if (LookupOption(*SC, NewArg, Val)) {
+O->removeArgument();
+return true;
+  }
+  return false;
+}
+
+void CommandLineParser::handleDefaultOptions() {
+  for (auto Pair : DefaultOptions) {
+StringRef Arg(Pair.first);
+Option *O = Pair.second;
+StringRef NewArg = Arg.substr(0, Arg.find(":"));
+
+bool Removed = false;
+if (O->isInAllSubCommands()) {
+  for (auto SC : RegisteredSubCommands) {
+if (SC == &*AllSubCommands)
+  continue;
+if ((Removed = tryRemoveDefaultOption(O, SC, NewArg)))
+  break;
+  }
+} else {
+  for (auto SC : O->Subs) {
+if ((Removed = tryRemoveDefaultOption(O, SC, NewArg)))
+  break;
+  }
+}
+if (!Removed)
+  O->setArgStr(NewArg);
+  }
+}
+
 bool CommandLineParser::ParseCommandLineOptions(int argc,
 const char *const *argv,
 StringRef Overview,
@@ -1167,6 +1220,8 @@
   auto  = ChosenSubCommand->SinkOpts;
   auto  = ChosenSubCommand->OptionsMap;
 
+  handleDefaultOptions();
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2201,9 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -270,12 +273,12 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued 

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D60409#1461579 , @phosek wrote:

> Our Mac builders have started failing after this change with the following:
>
>   [3145/3502] Building CXX object 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o
>   FAILED: 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
>   /b/s/w/ir/cache/goma/client/gomacc /b/s/w/ir/k/cipd/bin/clang++  
> -DCLANG_VENDOR="\"Fuchsia \"" -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
> -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
> -Itools/clang/tools/extra/clangd/tool 
> -I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool 
> -I/b/s/w/ir/k/llvm-project/clang/include -Itools/clang/include 
> -I/usr/include/libxml2 -Iinclude -I/b/s/w/ir/k/llvm-project/llvm/include 
> -I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/.. 
> -Itools/clang/tools/extra/clangd/tool/.. -fPIC -fvisibility-inlines-hidden 
> -Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra 
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -fno-common 
> -Woverloaded-virtual -Wno-nested-anon-types -O3 -gline-tables-only   -UNDEBUG 
>  -fno-exceptions -fno-rtti -MD -MT 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
> -MF 
> tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o.d 
> -o tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
> -c /b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp
>   
> /b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:474:22: 
> error: use of undeclared identifier 'newXPCTransport'
>   TransportLayer = newXPCTransport();
>^
>   1 error generated.
>
>
> I don't understand why since this change hasn't touched the failing line.


It has been fixed by rCTE358103 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60409



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

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Yucheng Wu via Phabricator via cfe-commits]

Ywicheng updated this revision to Diff 194631.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at 
+// function (instead of namespace) scope.
+namespace outer {
+
+  int fun_scope() {
+using ::bar::A;
+return 0;
+  } // function scope
+  
+  namespace inner {
+
+  } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,12 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`abseil-safely-scoped
+  ` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy ---===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open 

[PATCH] D60552: Enable intrinsics of AVX512_BF16, which are supported for BFLOAT16 in Cooper Lake

[Tianle Liu via Phabricator via cfe-commits]

liutianle created this revision.
liutianle added reviewers: craig.topper, smaslov, LuoYuanke, wxiao3, 
annita.zhang.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.

1. Enable infrastructure of AVX512_BF16, which is supported for BFLOAT16 in 
Cooper Lake;
2. Enable intrinsics for VCVTNE2PS2BF16, VCVTNEPS2BF16 and DPBF16PS 
instructions, which are Vector Neural Network Instructions supporting BFLOAT16 
inputs and conversion instructions from IEEE single precision.

For more details about BF16 intrinsic, please refer to the latest ISE document: 
https://software.intel.com/en-us/download/intel-architecture-instruction-set-extensions-programming-reference


Repository:
  rC Clang

https://reviews.llvm.org/D60552

Files:
  docs/ClangCommandLineReference.rst
  include/clang/Basic/BuiltinsX86.def
  include/clang/Driver/Options.td
  lib/Basic/Targets/X86.cpp
  lib/Basic/Targets/X86.h
  lib/CodeGen/CGBuiltin.cpp
  lib/Headers/CMakeLists.txt
  lib/Headers/avx512bf16intrin.h
  lib/Headers/avx512vlbf16intrin.h
  lib/Headers/cpuid.h
  lib/Headers/immintrin.h
  test/CodeGen/attr-target-x86.c
  test/CodeGen/avx512bf16-builtins.c
  test/CodeGen/avx512vlbf16-builtins.c

Index: test/CodeGen/avx512vlbf16-builtins.c
===
--- /dev/null
+++ test/CodeGen/avx512vlbf16-builtins.c
@@ -0,0 +1,142 @@
+//  RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin \
+//  RUN:-target-feature +avx512bf16 -target-feature \
+//  RUN:+avx512vl -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+__m128bh test_mm_cvtne2ps2bf16(__m128 A, __m128 B) {
+  // CHECK-LABEL: @test_mm_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  return _mm_cvtne2ps_pbh(A, B);
+}
+
+__m128bh test_mm_maskz_cvtne2ps2bf16(__m128 A, __m128 B, __mmask8 U) {
+  // CHECK-LABEL: @test_mm_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  return _mm_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m128bh test_mm_mask_cvtne2ps2bf16(__m128bh C, __mmask8 U, __m128 A, __m128 B) {
+  // CHECK-LABEL: @test_mm_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.128
+  // CHECK: select <8 x i1> %{{.*}}, <8 x i16> %{{.*}}, <8 x i16> %{{.*}}
+  return _mm_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m256bh test_mm256_cvtne2ps2bf16(__m256 A, __m256 B) {
+  // CHECK-LABEL: @test_mm256_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  return _mm256_cvtne2ps_pbh(A, B);
+}
+
+__m256bh test_mm256_maskz_cvtne2ps2bf16(__m256 A, __m256 B, __mmask16 U) {
+  // CHECK-LABEL: @test_mm256_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
+  return _mm256_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m256bh test_mm256_mask_cvtne2ps2bf16(__m256bh C, __mmask16 U, __m256 A, __m256 B) {
+  // CHECK-LABEL: @test_mm256_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.256
+  // CHECK: select <16 x i1> %{{.*}}, <16 x i16> %{{.*}}, <16 x i16> %{{.*}}
+  return _mm256_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m512bh test_mm512_cvtne2ps2bf16(__m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  return _mm512_cvtne2ps_pbh(A, B);
+}
+
+__m512bh test_mm512_maskz_cvtne2ps2bf16(__m512 A, __m512 B, __mmask32 U) {
+  // CHECK-LABEL: @test_mm512_maskz_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
+  return _mm512_maskz_cvtne2ps_pbh(U, A, B);
+}
+
+__m512bh test_mm512_mask_cvtne2ps2bf16(__m512bh C, __mmask32 U, __m512 A, __m512 B) {
+  // CHECK-LABEL: @test_mm512_mask_cvtne2ps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtne2ps2bf16.512
+  // CHECK: select <32 x i1> %{{.*}}, <32 x i16> %{{.*}}, <32 x i16> %{{.*}}
+  return _mm512_mask_cvtne2ps_pbh(C, U, A, B);
+}
+
+__m128bh test_mm_cvtneps2bf16(__m128 A) {
+  // CHECK-LABEL: @test_mm_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.128
+  return _mm_cvtneps_pbh(A);
+}
+
+__m128bh test_mm_mask_cvtneps2bf16(__m128bh C, __mmask8 U, __m128 A) {
+  // CHECK-LABEL: @test_mm_mask_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.
+  return _mm_mask_cvtneps_pbh(C, U, A);
+}
+
+__m128bh test_mm_maskz_cvtneps2bf16(__m128 A, __mmask8 U) {
+  // CHECK-LABEL: @test_mm_maskz_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.mask.cvtneps2bf16.128
+  return _mm_maskz_cvtneps_pbh(U, A);
+}
+
+__m128bh test_mm256_cvtneps2bf16(__m256 A) {
+  // CHECK-LABEL: @test_mm256_cvtneps2bf16
+  // CHECK: @llvm.x86.avx512bf16.cvtneps2bf16.256
+  return _mm256_cvtneps_pbh(A);
+}
+
+__m128bh test_mm256_mask_cvtneps2bf16(__m128bh C, __mmask8 U, __m256 A) {
+  // CHECK-LABEL: @test_mm256_mask_cvtneps2bf16
+  // CHECK: 

RE: r358104 - Don't emit an unreachable return block.

[via cfe-commits]

Hi, 
This commit seems to be causing a test failure on several buildbots (e.g. 
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/26305). 
instrprof-shared-gcov-flush.test is failing because of differences in profile 
info. The test passes when I revert your change, so perhaps it's just a case of 
updating the test.
Could you please take a look? 

Thanks,
Wolfgang Pieb

-Original Message-
From: cfe-commits  On Behalf Of John McCall 
via cfe-commits
Sent: Wednesday, April 10, 2019 10:03 AM
To: cfe-commits@lists.llvm.org
Subject: r358104 - Don't emit an unreachable return block.

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();
 
-// If that was the only use of the return value, nuke it as well now.
-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = dyn_cast(returnValueInst)) {
-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions called by this
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = dyn_cast(ReturnValue.getPointer());
+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be

Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104=auto
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp Wed Apr 10 
+++ 10:03:09 2019
@@ -622,7 +622,7 @@ int main() {
 
 // CHECK-NOT: call i32 

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Yucheng Wu via Phabricator via cfe-commits]

Ywicheng updated this revision to Diff 194623.
Ywicheng marked 2 inline comments as done.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-5]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at 
+// function (instead of namespace) scope.
+namespace outer {
+
+  int fun_scope() {
+using ::bar::A;
+return 0;
+  } // function scope
+  
+  namespace inner {
+
+  } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,8 +67,15 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`abseil-safely-scoped
+  ` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   ` check.
+>>> master
 
   Checks for cases where addition should be performed in the ``absl::Time``
   domain.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- SafelyScopedCheck.cpp - clang-tidy 

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194622.
Dosi-Dough added a comment.

added white space to check doc.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194621.
Dosi-Dough marked 2 inline comments as done.
Dosi-Dough added a comment.

added whitespace to check doc


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,35 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/abseil-wrap-unique.rst:5
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using  
``absl::wrap_unique(new T(...))``.

Eugene.Zelenko wrote:
> Please separate with empty line.
Sorry, if I was not clear. I meant empty line between ==  and 
statement.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435



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

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194618.
Dosi-Dough marked 3 inline comments as done.
Dosi-Dough added a comment.

removed parens from turnary, added whitespace.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,34 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+
+that returns a ``std::unqiue_ptr`` then recommends using  ``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

Re: Notice: The buildbot bb.pgr.jp will be suspended within a few days

[Mike Edwards via cfe-commits]

Takumi,
I am very sorry it took me almost 18 months to respond to this note.  I'm
sorry to see you had to shutter bb.pgr.jp.  It was a very helpful resource
to the community.  Thank you very much for your contribution of effort and
resources to make bb.pgr.jp available to the LLVM community for as long as
you were able.  Your efforts are always very much appreciated.

Respectfully,
Mike

On Mon, Nov 20, 2017 at 1:44 AM NAKAMURA Takumi via llvm-commits <
llvm-comm...@lists.llvm.org> wrote:

> Due to resource issue, I have to terminate it. As you know, it has been
> working for several years and I am certain it has been useful and helpful
> to guys.
> I am not sure whether I could restart it or not.
> I loved it.
>
> Thank you,
> Takumi Nakamura
> ___
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D55409: [clang-tidy] check for using declarations not in an anonymous namespace when there exists one

[Shyan Akmal via Phabricator via cfe-commits]

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



Comment at: clang-tidy/abseil/AnonymousEnclosedAliasesCheck.cpp:41
+// to the vector containing all candidate using declarations.
+if (AnonymousNamespaceDecl) {
+   diag(MatchedUsingDecl->getLocation(),

aaron.ballman wrote:
> I don't think this logic works in practice because there's no way to 
> determine that the anonymous namespace is even a candidate for putting the 
> using declaration into it. Consider a common scenario where there's an 
> anonymous namespace declared in a header file (like an STL header outside of 
> the user's control), and a using declaration inside of an implementation 
> file. Just because the STL declared an anonymous namespace doesn't mean that 
> the user could have put their using declaration in it.
If we altered the check to only apply to anonymous namespaces and using 
declarations at namespace scope (that is, we only suggest aliases be moved to 
anonymous namespaces when the unnamed namespace and alias are themselves inside 
some other namespace), would this issue be resolved? 


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

https://reviews.llvm.org/D55409



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

[PATCH] D55411: [clang-tidy] check for flagging using declarations not in the inner most namespace

[Yucheng Wu via Phabricator via cfe-commits]

Ywicheng updated this revision to Diff 194614.
Ywicheng marked an inline comment as done.
Ywicheng edited the summary of this revision.

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

https://reviews.llvm.org/D55411

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/SafelyScopedCheck.cpp
  clang-tidy/abseil/SafelyScopedCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-safely-scoped.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-safely-scoped.cpp

Index: test/clang-tidy/abseil-safely-scoped.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-safely-scoped.cpp
@@ -0,0 +1,43 @@
+// RUN: %check_clang_tidy %s abseil-safely-scoped %t
+namespace bar {
+
+class A {};
+class B {};
+} // namespace bar
+
+namespace foo1 {
+
+using bar::A;
+void f(A a);
+
+namespace {} // anonymous namespace
+} // namespace foo1
+// CHECK-MESSAGES: :[[@LINE-4]]:12: warning: using declaration 'A' not
+// declared in the innermost namespace. [abseil-safely-scoped]
+
+namespace foo2 {
+
+namespace {
+
+using ::bar::B;
+
+} // anonymous namespace
+void g(B b);
+} // namespace foo2
+
+
+// Check should not be triggered below when we are at 
+// function (instead of namespace) scope.
+namespace outer {
+
+  int fun_scope() {
+using ::bar::A;
+return 0;
+  } // function scope
+  
+  namespace inner {
+
+  } // namespace inner
+
+} // namespace outer
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
abseil-no-internal-dependencies
abseil-no-namespace
abseil-redundant-strcat-calls
+   abseil-safely-scoped
abseil-str-cat-append
abseil-string-find-startswith
abseil-time-subtraction
Index: docs/clang-tidy/checks/abseil-safely-scoped.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-safely-scoped.rst
@@ -0,0 +1,28 @@
+.. title:: clang-tidy - abseil-safely-scoped
+
+abseil-safely-scoped
+
+
+Flags using declarations that are not contained in an innermost
+namespace, and suggests these declarations be moved elsewhere.
+
+Example:
+
+.. code-block:: c++
+
+  using something; // should be inside the innermost namespace bar below
+
+  namespace foo {
+  namespace bar {
+	
+  } // bar
+
+  using something_else; // shoulw be inside the innermost namespace bar above
+
+  } // foo
+
+Placing convenience aliases in upper level namespaces can lead to ambiguity in 
+which name the compiler should use. 
+
+See https://abseil.io/tips/119 for more explanation. 
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,8 +67,15 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`abseil-safely-scoped
+  ` check.
+
+  Finds instances of using declarations not in the innermost layer
+  of a series of namespaces.
+
 - New :doc:`abseil-duration-addition
   ` check.
+>>> master
 
   Checks for cases where addition should be performed in the ``absl::Time``
   domain.
Index: clang-tidy/abseil/SafelyScopedCheck.h
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.h
@@ -0,0 +1,36 @@
+//===--- SafelyScopedCheck.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_ABSEIL_SAFELYSCOPEDCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Detecting using declarations not in a namespace declaration or not in
+/// the innermost layer of namespace declarations.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-safely-scoped.html
+class SafelyScopedCheck : public ClangTidyCheck {
+public:
+  SafelyScopedCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_SAFELYSCOPEDCHECK_H
Index: clang-tidy/abseil/SafelyScopedCheck.cpp
===
--- /dev/null
+++ clang-tidy/abseil/SafelyScopedCheck.cpp
@@ -0,0 +1,43 @@
+//===--- 

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



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

LLVM buildmaster will be updated and restarted tonight

[Galina Kistanova via cfe-commits]

 Hello everyone,

LLVM buildmaster will be updated and restarted after 7PM Pacific time today.

Thanks

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

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

[Andy Zhang via Phabricator via cfe-commits]

axzhang added a comment.

If no more changes need to be made, is this okay to be merged in?


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

https://reviews.llvm.org/D55044



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

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/abseil/WrapUniqueCheck.cpp:85
+
+std::string Left = (ConsDecl) ? "auto " + NameRef.str() + " = " : "";
+std::string NewText =

I don't think that you need round brackets around ConsDecl. Same below.



Comment at: docs/clang-tidy/checks/abseil-wrap-unique.rst:5
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using  
``absl::wrap_unique(new T(...))``.

Please separate with empty line.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435



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

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 194609.
jkorous added a comment.

rename RawCommentAndCacheFlags -> CommentAndOrigin


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

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -370,70 +370,69 @@
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
 
-  // Check whether we have cached a comment for this declaration already.
-  {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
+llvm::DenseMap::iterator Pos =
+RedeclComments.find(SpecificD);
 if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  const CommentAndOrigin  = Pos->second;
+  if (OriginalDecl)
+*OriginalDecl = CachedComment.getOriginalDecl();
+  return CachedComment.getRaw();
 }
-  }
+return nullptr;
+  };
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+  auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional {
+if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+
+  CommentAndOrigin NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(SpecificD);
+
+  if (OriginalDecl)
+*OriginalDecl = SpecificD;
+
+  return NewCachedComment;
 }
-  }
+return llvm::None;
+  };
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  // Try to find comment for this specific declaration.
+  if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(D)) {
+const CommentAndOrigin& CommentAndFlags = OptCommentAndFlags.getValue();
+RedeclComments[D] = CommentAndFlags;
+return CommentAndFlags.getRaw();
+  }
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+return nullptr;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  // Check whether we have cached a comment for canonical declaration of this declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;
 
+  // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment attached to itself.
+  // Search for any comment attached to redeclarations of D and cache it for canonical declaration of D.
   for (auto I : D->redecls()) {
-RawCommentAndCacheFlags  = RedeclComments[I];
-if (R.getKind() == RawCommentAndCacheFlags::NoCommentInDecl)
-  R = Raw;
+if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(I)) {
+  const CommentAndOrigin& CommentAndFlags = OptCommentAndFlags.getValue();
+  RedeclComments[CanonicalD] = 

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194608.
Dosi-Dough added a comment.

fixed docs and white spacing


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,33 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using  ``absl::wrap_unique(new T(...))``.
+
+Examples:
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h

[PATCH] D57435: [clang-tidy] Add abseil-wrap-unique check

[Ryan Piantedosi via Phabricator via cfe-commits]

Dosi-Dough updated this revision to Diff 194607.
Dosi-Dough marked an inline comment as done.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57435

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/WrapUniqueCheck.cpp
  clang-tidy/abseil/WrapUniqueCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-wrap-unique.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-wrap-unique.cpp

Index: test/clang-tidy/abseil-wrap-unique.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-wrap-unique.cpp
@@ -0,0 +1,87 @@
+// RUN: %check_clang_tidy %s abseil-wrap-unique %t
+
+namespace std {
+
+template 
+class default_delete {};
+
+template >
+class unique_ptr {
+public:
+  unique_ptr() {}
+  unique_ptr(type *ptr) {}
+  unique_ptr(const unique_ptr ) = delete;
+  unique_ptr(unique_ptr &) {}
+  ~unique_ptr() {}
+  type *() { return *ptr; }
+  type *operator->() { return ptr; }
+  type *release() { return ptr; }
+  void reset() {}
+  void reset(type *pt) {}
+  void reset(type pt) {}
+  unique_ptr =(unique_ptr &&) { return *this; }
+  template 
+  unique_ptr =(unique_ptr &&) { return *this; }
+
+private:
+  type *ptr;
+};
+}  // namespace std
+
+class A {
+ public:
+  static A* NewA() {
+return new A();
+  }
+
+ private:
+  A() {}
+};
+
+class B {
+ public:
+  static B* NewB(int bIn) {
+return new B();
+  }
+
+ private:
+  B() {}
+};
+
+struct C {
+  int x;
+  int y;
+};
+/*
+std::unique_ptr returnPointer() {
+  return std::unique_ptr(A::NewA());
+}
+*/
+void positives() {
+  std::unique_ptr a;
+  a.reset(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: a = absl::WrapUnique(A::NewA())
+  
+  std::unique_ptr b(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto b = absl::WrapUnique(A::NewA())
+
+  int cIn;
+  std::unique_ptr c(B::NewB(cIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: Perfer absl::WrapUnique to constructing unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: auto c = absl::WrapUnique(B::NewB(cIn))
+
+  int dIn;
+  std::unique_ptr d;
+  d.reset(B::NewB(dIn));
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: d = absl::WrapUnique(B::NewB(dIn))
+  
+  auto e = std::unique_ptr(A::NewA());
+  //CHECK-MESSAGE: :[[@LINE-1]]:3: warning: prefer absl::WrapUnique for resetting unique_ptr [abseil-wrap-unique]
+  //CHECK-FIXES: e = absl::WrapUnique(A::NewA())
+
+  //std::unique_ptr e(new int[2] {1,2});
+}
+
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -20,6 +20,7 @@
abseil-string-find-startswith
abseil-time-subtraction
abseil-upgrade-duration-conversions
+   abseil-wrap-unique
android-cloexec-accept
android-cloexec-accept4
android-cloexec-creat
Index: docs/clang-tidy/checks/abseil-wrap-unique.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/abseil-wrap-unique.rst
@@ -0,0 +1,32 @@
+.. title:: clang-tidy - abseil-wrap-unique
+
+abseil-wrap-unique
+==
+Looks for instances of factory functions which uses a non-public constructor
+that returns a ``std::unqiue_ptr`` then recommends using 
+``absl::wrap_unique(new T(...))``.
+
+.. code-block:: c++
+ 
+  class A {
+  public:
+static A* NewA() { return new A(); }
+
+  private:
+A() = default; 
+  };
+
+  std::unique_ptr a;
+
+  // Original - reset called with a static function returning a std::unqiue_ptr
+  a.reset(A::NewA());
+
+  // Suggested - reset ptr with absl::WrapUnique
+  a = absl::WrapUnique(A::NewA());
+
+  // Original - std::unique_ptr initialized with static function
+  std::unique_ptr b(A::NewA());
+
+  // Suggested - initialize with absl::WrapUnique instead
+  auto b = absl::WrapUnique(A::NewA())
+
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -91,6 +91,13 @@
   Finds and fixes ``absl::Time`` subtraction expressions to do subtraction
   in the Time domain instead of the numeric domain.
 
+- New :doc:`abseil-wrap-unique
+  ` check.
+
+  Looks for instances of factory functions which uses a non-public constructor
+  that returns a ``std::unqiue_ptr`` then recommends using 
+  ``absl::wrap_unique(new T(...))``.
+
 - New :doc:`google-readability-avoid-underscore-in-googletest-name
   `
   check.
Index: clang-tidy/abseil/WrapUniqueCheck.h
===
--- /dev/null
+++ 

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 194604.
jkorous added a comment.

Second attempt on reducing the cache size and number of operations.

I try in this order

1. cache lookup for the specific provided declaration
2. try to find comment attached to the specific provided declaration without 
using cache (and cache it using the specific declaration as a key)
3. cache lookup using the canonical declaration (which would return comment 
from any redeclaration - see the next step)
4. try to find comment attached to any redeclaration (and cache it using the 
canonical declaration as a key)
5. give up, return nullptr


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

https://reviews.llvm.org/D60432

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp

Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -370,70 +370,69 @@
 const Decl **OriginalDecl) const {
   D = adjustDeclToTemplate(D);
 
-  // Check whether we have cached a comment for this declaration already.
-  {
+  auto CacheLookup = [&](const Decl *SpecificD) -> const RawComment * {
 llvm::DenseMap::iterator Pos =
-RedeclComments.find(D);
+RedeclComments.find(SpecificD);
 if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-if (OriginalDecl)
-  *OriginalDecl = Raw.getOriginalDecl();
-return Raw.getRaw();
-  }
+  const RawCommentAndCacheFlags  = Pos->second;
+  if (OriginalDecl)
+*OriginalDecl = CachedComment.getOriginalDecl();
+  return CachedComment.getRaw();
 }
-  }
+return nullptr;
+  };
 
-  // Search for comments attached to declarations in the redeclaration chain.
-  const RawComment *RC = nullptr;
-  const Decl *OriginalDeclForRC = nullptr;
-  for (auto I : D->redecls()) {
-llvm::DenseMap::iterator Pos =
-RedeclComments.find(I);
-if (Pos != RedeclComments.end()) {
-  const RawCommentAndCacheFlags  = Pos->second;
-  if (Raw.getKind() != RawCommentAndCacheFlags::NoCommentInDecl) {
-RC = Raw.getRaw();
-OriginalDeclForRC = Raw.getOriginalDecl();
-break;
-  }
-} else {
-  RC = getRawCommentForDeclNoCache(I);
-  OriginalDeclForRC = I;
-  RawCommentAndCacheFlags Raw;
-  if (RC) {
-// Call order swapped to work around ICE in VS2015 RTM (Release Win32)
-// https://connect.microsoft.com/VisualStudio/feedback/details/1741530
-Raw.setKind(RawCommentAndCacheFlags::FromDecl);
-Raw.setRaw(RC);
-  } else
-Raw.setKind(RawCommentAndCacheFlags::NoCommentInDecl);
-  Raw.setOriginalDecl(I);
-  RedeclComments[I] = Raw;
-  if (RC)
-break;
+  auto GetCommentNoCache = [&](const Decl *SpecificD) -> const llvm::Optional {
+if (const RawComment *RC = getRawCommentForDeclNoCache(SpecificD)) {
+  // If we find a comment, it should be a documentation comment.
+  assert(RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+
+  RawCommentAndCacheFlags NewCachedComment;
+  // Call order swapped to work around ICE in VS2015 RTM (Release Win32)
+  // https://connect.microsoft.com/VisualStudio/feedback/details/1741530
+  NewCachedComment.setRaw(RC);
+  NewCachedComment.setOriginalDecl(SpecificD);
+
+  if (OriginalDecl)
+*OriginalDecl = SpecificD;
+
+  return NewCachedComment;
 }
-  }
+return llvm::None;
+  };
 
-  // If we found a comment, it should be a documentation comment.
-  assert(!RC || RC->isDocumentation() || LangOpts.CommentOpts.ParseAllComments);
+  // Check whether we have cached a comment for this specific declaration.
+  if (auto * CachedComment = CacheLookup(D))
+return CachedComment;
 
-  if (OriginalDecl)
-*OriginalDecl = OriginalDeclForRC;
+  // Try to find comment for this specific declaration.
+  if (llvm::Optional OptCommentAndFlags = GetCommentNoCache(D)) {
+const RawCommentAndCacheFlags& CommentAndFlags = OptCommentAndFlags.getValue();
+RedeclComments[D] = CommentAndFlags;
+return CommentAndFlags.getRaw();
+  }
+
+  // In case this is the canonical Decl we're done.
+  auto CanonicalD = D->getCanonicalDecl();
+  if (!CanonicalD)
+return nullptr;
 
-  // Update cache for every declaration in the redeclaration chain.
-  RawCommentAndCacheFlags Raw;
-  Raw.setRaw(RC);
-  Raw.setKind(RawCommentAndCacheFlags::FromRedecl);
-  Raw.setOriginalDecl(OriginalDeclForRC);
+  // Check whether we have cached a comment for canonical declaration of this declaration.
+  if (auto * CachedComment = CacheLookup(CanonicalD))
+return CachedComment;
 
+  // We don't have comment for neither D, nor it's canonical declaration in cache and D doesn't have any comment 

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

One downside to alloca is that we can's use `__attribute__((uninitialized))` 
because it's a builtin. Maybe we need a separate uninitialized alloca? What do 
you all think?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60548



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

[PATCH] D60548: Variable auto-init: also auto-init alloca

[JF Bastien via Phabricator via cfe-commits]

jfb created this revision.
Herald added subscribers: cfe-commits, dexonsmith, jkorous.
Herald added a project: clang.

alloca isn’t auto-init’d right now because it’s a different path in clang that 
all the other stuff we support (it’s a builtin, not an expression). 
Interestingly, alloca doesn’t have a type (as opposed to even VLA) so we can 
really only initialize it with memset.

rdar://problem/49794007


Repository:
  rC Clang

https://reviews.llvm.org/D60548

Files:
  lib/CodeGen/CGBuiltin.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 
[[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] 
%[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 
[[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] 
%[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  test_alloca_with_align(
+// ZERO-LABEL:test_alloca_with_align(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 
0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca_with_align(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align 128
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align 128 %[[ALLOCA]], i8 
-86, i64 %[[SIZE]], i1 false)
+void test_alloca_with_align(int size) {
+  void *ptr = __builtin_alloca_with_align(size, 1024);
+  used(ptr);
+}
+
 // UNINIT-LABEL:  test_struct_vla(
 // ZERO-LABEL:test_struct_vla(
 // ZERO:  %[[SIZE:[0-9]+]] = mul nuw i64 %{{.*}}, 16
Index: lib/CodeGen/CGBuiltin.cpp
===
--- lib/CodeGen/CGBuiltin.cpp
+++ lib/CodeGen/CGBuiltin.cpp
@@ -44,6 +44,22 @@
   return std::min(High, std::max(Low, Value));
 }
 
+static void initializeAlloca(CodeGenFunction , AllocaInst *AI, Value 
*Size, unsigned AlignmentInBytes) {
+  ConstantInt *Byte;
+  switch (CGF.getLangOpts().getTrivialAutoVarInit()) {
+  case LangOptions::TrivialAutoVarInitKind::Uninitialized:
+// Nothing to initialize.
+return;
+  case LangOptions::TrivialAutoVarInitKind::Zero:
+Byte = CGF.Builder.getInt8(0);
+break;
+  case LangOptions::TrivialAutoVarInitKind::Pattern:
+Byte = CGF.Builder.getInt8(0xAA);
+break;
+  }
+  CGF.Builder.CreateMemSet(AI, Byte, Size, AlignmentInBytes);
+}
+
 /// getBuiltinLibFunction - Given a builtin id for a function like
 /// "__builtin_fabsf", return a Function* for "fabsf".
 llvm::Constant *CodeGenModule::getBuiltinLibFunction(const FunctionDecl *FD,
@@ -2259,6 +2275,7 @@
 .getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(SuitableAlignmentInBytes);
+initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
 return RValue::get(AI);
   }
 
@@ -2271,6 +2288,7 @@
 CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getQuantity();
 AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
 AI->setAlignment(AlignmentInBytes);
+initializeAlloca(*this, AI, Size, AlignmentInBytes);
 return RValue::get(AI);
   }
 


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -173,6 +173,34 @@
   used(ptr);
 }
 
+// UNINIT-LABEL:  test_alloca(
+// ZERO-LABEL:test_alloca(
+// ZERO:  %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// ZERO-NEXT: %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// ZERO-NEXT: call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 0, i64 %[[SIZE]], i1 false)
+// PATTERN-LABEL: test_alloca(
+// PATTERN:   %[[SIZE:[a-z0-9]+]] = sext i32 %{{.*}} to i64
+// PATTERN-NEXT:  %[[ALLOCA:[a-z0-9]+]] = alloca i8, i64 %[[SIZE]], align [[ALIGN:[0-9]+]]
+// PATTERN-NEXT:  call void @llvm.memset{{.*}}(i8* align [[ALIGN]] %[[ALLOCA]], i8 -86, i64 %[[SIZE]], i1 false)
+void test_alloca(int size) {
+  void *ptr = __builtin_alloca(size);
+  used(ptr);
+}
+
+// UNINIT-LABEL:  

[PATCH] D60432: [clang][ASTContext] Simplify caching for declaration-related comments

[Jan Korous via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTContext.cpp:372
   D = adjustDeclToTemplate(D);
+  const Decl* CanonicalDecl = D->getCanonicalDecl();
 

gribozavr wrote:
> jkorous wrote:
> > arphaman wrote:
> > > Why are we now checking for the canonical declaration instead of `D` as 
> > > before?
> > Before this we were caching comment for every redeclaration separately but 
> > for every redeclaration the comment was the same.
> > 
> > As I understand it - for a given declaration we found the first comment in 
> > the redeclaration chain and then saved it to the cache for every 
> > redeclaration (the same comment).
> > Later, during lookup, we iterated the redeclaration chain again and did a 
> > lookup for every redeclaration (see L392 in the original implementation).
> > 
> > Unless I missed something, it's suboptimal in both memory consumption and 
> > runtime overhead.
> > 
> > That's the reason why I want to cache the comment for the redeclaration 
> > group as a whole. The only thing I am not entirely sure about is what key 
> > to use to represent the whole redeclaration chain - maybe the first 
> > declaration in the redeclaration chain would be better then the canonical 
> > declaration...
> > 
> > WDYT?
> > As I understand it - for a given declaration we found the first comment in 
> > the redeclaration chain and then saved it to the cache for every 
> > redeclaration (the same comment).
> 
> Only if there was only one comment in the redeclaration chain.  If a 
> redeclaration had a different comment, this function would return that 
> comment.
I see. I made a wrong assumption - that for any two redeclarations the 
redeclaration chain always starts from the same declaration.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60432



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

[PATCH] D59754: [Sema] Add c++2a designated initializer warnings

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

ping, and should we add new warning messages as suggested by @Rakete ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59754



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

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

[Stephane Moore via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:1479
+/// \endcode
+AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface,
+  internal::Matcher,

I am still uncertain about the naming.

`isSubclassOf` seemed too generic as that could apply to C++ classes.
`objcIsSubclassOf` seemed unconventional as a matcher name.
`isSubclassOfObjCInterface` and `isSubclassOfObjCClass` seemed awkwardly 
lengthy.
Creating a new namespace `clang::ast_matchers::objc` seemed unprecedented.

I am happy to change the name if you think another name would be more 
appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60543



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

[PATCH] D59121: [analyzer] Fix macro names in diagnostics within bigger macros.

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

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

LGTM! Thanks!


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

https://reviews.llvm.org/D59121



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

[PATCH] D60543: [clang] Add matcher for subclasses of Objective-C interfaces 

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
stephanemoore added a reviewer: aaron.ballman.

This change adds a new AST matcher to detect Objective-C classes that
are subclasses of matching Objective-C interfaces.

Test Notes:
Ran clang unit tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60543

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -1389,6 +1389,23 @@
   EXPECT_TRUE(matchesObjC("void f() { ^{}(); }", blockExpr()));
 }
 
+TEST(IsSubclassOfInterfaceMatcher, SubclassMatching) {
+  std::string ObjCString = "@interface A @end"
+   "@interface B : A @end"
+   "@interface C : B @end";
+  EXPECT_TRUE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("A");
+  EXPECT_TRUE(matchesObjC(ObjCString,
+  objcInterfaceDecl(isSubclassOfInterface(hasName("A")),
+unless(hasName("B");
+  EXPECT_TRUE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("B");
+  EXPECT_FALSE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("C");
+  EXPECT_FALSE(matchesObjC(
+  ObjCString, objcInterfaceDecl(isSubclassOfInterface(hasName("X");
+}
+
 TEST(StatementCountIs, FindsNoStatementsInAnEmptyCompoundStatement) {
   EXPECT_TRUE(matches("void f() { }",
   compoundStmt(statementCountIs(0;
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -393,6 +393,7 @@
   REGISTER_MATCHER(isStaticLocal);
   REGISTER_MATCHER(isStaticStorageClass);
   REGISTER_MATCHER(isStruct);
+  REGISTER_MATCHER(isSubclassOfInterface);
   REGISTER_MATCHER(isTemplateInstantiation);
   REGISTER_MATCHER(isTypeDependent);
   REGISTER_MATCHER(isUnion);
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -1461,6 +1461,35 @@
 extern const internal::VariadicDynCastAllOfMatcher
 objcFinallyStmt;
 
+/// Matches Objective-C classes that directly or indirectly subclass
+/// a matching superclass.
+///
+/// Note that a class is not considered to be a subclass of itself.
+///
+/// Example matches implementation declarations for Y and Z.
+///   (matcher = objcInterfaceDecl(isSubclassOfInterface(hasName("X"
+/// \code
+///   @interface X
+///   @end
+///   @interface Y : X  // directly derived
+///   @end
+///   @interface Z : Y  // indirectly derived
+///   @end
+/// \endcode
+AST_MATCHER_P(ObjCInterfaceDecl, isSubclassOfInterface,
+  internal::Matcher,
+  InnerMatcher) {
+  // Check if any of the superclasses of the class match.
+  for (const ObjCInterfaceDecl *SuperClass = Node.getSuperClass();
+   SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {
+if (InnerMatcher.matches(*SuperClass, Finder, Builder))
+  return true;
+  }
+
+  // No matches found.
+  return false;
+}
+
 /// Matches expressions that introduce cleanups to be run at the end
 /// of the sub-expression's evaluation.
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -6285,6 +6285,23 @@
 
 
 
+Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCInterfaceDecl.html;>ObjCInterfaceDeclisSubclassOfInterfaceMatcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCInterfaceDecl.html;>ObjCInterfaceDecl InnerMatcher
+Matches Objective-C classes that directly or indirectly subclass
+a matching superclass.
+
+Note that a class is not considered to be a subclass of itself.
+
+Example matches implementation declarations for Y and Z.
+  (matcher = objcInterfaceDecl(isSubclassOfInterface(hasName("X"
+  @interface X
+  @end
+  @interface Y : X  // directly derived
+  @end
+  @interface Z : Y  // indirectly derived
+  @end
+
+
+
 Matcherhttps://clang.llvm.org/doxygen/classclang_1_1ObjCMessageExpr.html;>ObjCMessageExprhasAnyArgumentMatcherhttps://clang.llvm.org/doxygen/classclang_1_1Expr.html;>Expr InnerMatcher
 Matches any argument of a call expression or a constructor call
 expression, or an ObjC-message-send expression.

[PATCH] D60544: Support objc_nonlazy_class attribute on Objective-C implementations

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, aaron.ballman.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

Fixes rdar://49523079


Repository:
  rC Clang

https://reviews.llvm.org/D60544

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/test/CodeGenObjC/non-lazy-classes.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-objc-non-lazy.m

Index: clang/test/SemaObjC/attr-objc-non-lazy.m
===
--- clang/test/SemaObjC/attr-objc-non-lazy.m
+++ clang/test/SemaObjC/attr-objc-non-lazy.m
@@ -29,7 +29,11 @@
 
 @interface E
 @end
-// expected-error@+1 {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
+
 __attribute__((objc_nonlazy_class))
 @implementation E
 @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E (MyCat)
+@end
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -102,7 +102,7 @@
 // CHECK-NEXT: ObjCExplicitProtocolImpl (SubjectMatchRule_objc_protocol)
 // CHECK-NEXT: ObjCExternallyRetained (SubjectMatchRule_variable_not_is_parameter, SubjectMatchRule_function, SubjectMatchRule_block, SubjectMatchRule_objc_method)
 // CHECK-NEXT: ObjCMethodFamily (SubjectMatchRule_objc_method)
-// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface)
+// CHECK-NEXT: ObjCNonLazyClass (SubjectMatchRule_objc_interface, SubjectMatchRule_objc_implementation)
 // CHECK-NEXT: ObjCPreciseLifetime (SubjectMatchRule_variable)
 // CHECK-NEXT: ObjCRequiresPropertyDefs (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: ObjCRequiresSuper (SubjectMatchRule_objc_method)
Index: clang/test/CodeGenObjC/non-lazy-classes.m
===
--- clang/test/CodeGenObjC/non-lazy-classes.m
+++ clang/test/CodeGenObjC/non-lazy-classes.m
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | \
-// RUN: FileCheck %s
-// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [2 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
-// CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [1 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -Wno-objc-root-class -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @"OBJC_LABEL_NONLAZY_CLASS_$" = private global [3 x {{.*}}]{{.*}}@"OBJC_CLASS_$_A"{{.*}},{{.*}}@"OBJC_CLASS_$_D"{{.*}},{{.*}}"OBJC_CLASS_$_E"{{.*}} section "__DATA,__objc_nlclslist,regular,no_dead_strip", align 8
+// CHECK: @"OBJC_LABEL_NONLAZY_CATEGORY_$" = private global [2 x {{.*}}] {{.*}}@"\01l_OBJC_$_CATEGORY_A_$_Cat"{{.*}},{{.*}}@"\01l_OBJC_$_CATEGORY_E_$_MyCat"{{.*}}, section "__DATA,__objc_nlcatlist,regular,no_dead_strip", align 8
 
 @interface A @end
 @implementation A
@@ -35,3 +35,11 @@
 @interface D @end
 
 @implementation D @end
+
+@interface E @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E @end
+
+__attribute__((objc_nonlazy_class))
+@implementation E (MyCat) @end
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -6262,7 +6262,8 @@
 bool CGObjCNonFragileABIMac::ImplementationIsNonLazy(
 const ObjCImplDecl *OD) const {
   return OD->getClassMethod(GetNullarySelector("load")) != nullptr ||
- OD->getClassInterface()->hasAttr();
+ OD->getClassInterface()->hasAttr() ||
+ OD->hasAttr();
 }
 
 void CGObjCNonFragileABIMac::GetClassSizeInfo(const ObjCImplementationDecl *OID,
Index: clang/include/clang/Basic/AttrDocs.td
===
--- clang/include/clang/Basic/AttrDocs.td
+++ clang/include/clang/Basic/AttrDocs.td
@@ -3711,14 +3711,14 @@
 def ObjCNonLazyClassDocs : Documentation {
   let Category = DocCatDecl;
   let Content = [{
-This attribute can be added to an Objective-C ``@interface`` declaration to
-add the class to the list of non-lazily initialized classes. A non-lazy class
-will be initialized eagerly when the Objective-C runtime is loaded.  This is
-required for certain system classes which have instances allocated in
-non-standard ways, such as the classes for blocks and constant strings.  Adding
-this attribute is essentially equivalent to providing a trivial `+load` method 
-but avoids the (fairly small) load-time overheads associated with defining and
-calling such a method.
+This attribute can 

[PATCH] D60542: Add support for attributes on @implementations in Objective-C

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, rjmccall.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

We want to make `objc_nonlazy_class` apply to implementations, but ran into 
this. There doesn't seem to be any reason that this isn't supported.

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D60542

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/FixIt/fixit-pragma-attribute.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Parser/attributes.mm
  clang/test/Parser/objc-implementation-attrs.m
  clang/test/Parser/placeholder-recovery.m
  clang/test/Sema/pragma-attribute-strict-subjects.c
  clang/test/SemaObjC/attr-objc-non-lazy.m
  clang/test/SemaObjC/objc-asm-attribute-neg-test.m

Index: clang/test/SemaObjC/objc-asm-attribute-neg-test.m
===
--- clang/test/SemaObjC/objc-asm-attribute-neg-test.m
+++ clang/test/SemaObjC/objc-asm-attribute-neg-test.m
@@ -19,7 +19,7 @@
   id MyIVAR;
 }
 __attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@property int MyProperty; // expected-error {{prefix attribute must be followed by an interface or protocol
+@property int MyProperty; // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation
 
 - (int) getMyProperty __attribute__((objc_runtime_name("MySecretNamespace.Message"))); // expected-error {{'objc_runtime_name' attribute only applies to}}
 
@@ -28,7 +28,7 @@
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardClass")))
-@class ForwardClass; // expected-error {{prefix attribute must be followed by an interface or protocol}}
+@class ForwardClass; // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation}}
 
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
 @protocol ForwardProtocol;
@@ -45,6 +45,6 @@
 
 @interface NoImpl @end
 
+// expected-error@+1 {{'objc_runtime_name' attribute only applies to Objective-C interfaces and Objective-C protocols}}
 __attribute__((objc_runtime_name("MySecretNamespace.Message")))
-// expected-error@+1 {{prefix attribute must be followed by an interface or protocol}}
 @implementation NoImpl @end
Index: clang/test/SemaObjC/attr-objc-non-lazy.m
===
--- clang/test/SemaObjC/attr-objc-non-lazy.m
+++ clang/test/SemaObjC/attr-objc-non-lazy.m
@@ -29,6 +29,7 @@
 
 @interface E
 @end
+// expected-error@+1 {{'objc_nonlazy_class' attribute only applies to Objective-C interfaces}}
 __attribute__((objc_nonlazy_class))
-@implementation E // expected-error {{prefix attribute must be followed by an interface or protocol}}
+@implementation E
 @end
Index: clang/test/Sema/pragma-attribute-strict-subjects.c
===
--- clang/test/Sema/pragma-attribute-strict-subjects.c
+++ clang/test/Sema/pragma-attribute-strict-subjects.c
@@ -56,7 +56,8 @@
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((abi_tag("a"))), apply_to = any(enum_constant, function, record(unless(is_union)), variable, variable(is_parameter)))
-// expected-error@-1 {{attribute 'abi_tag' can't be applied to 'variable(is_parameter)', and 'enum_constant'}}
+// FIXME: comma in this diagnostic is wrong.
+// expected-error@-2 {{attribute 'abi_tag' can't be applied to 'enum_constant', and 'variable(is_parameter)'}}
 #pragma clang attribute pop
 
 #pragma clang attribute push (__attribute__((abi_tag("a"))), apply_to = any(function, record(unless(is_union)), enum))
Index: clang/test/Parser/placeholder-recovery.m
===
--- clang/test/Parser/placeholder-recovery.m
+++ clang/test/Parser/placeholder-recovery.m
@@ -11,4 +11,4 @@
 // bogus 'archaic' warnings with bad location info.
 <#methods#> // expected-error {{editor placeholder in source file}}
 
-@end // expected-error {{prefix attribute must be followed by an interface or protocol}} expected-error {{missing '@end'}}
+@end // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation}} expected-error {{missing '@end'}}
Index: clang/test/Parser/objc-implementation-attrs.m
===
--- /dev/null
+++ clang/test/Parser/objc-implementation-attrs.m
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsyntax-only -Wno-objc-root-class -verify %s
+
+@interface I1 @end
+
+// expected-warning@+1 {{'always_inline' attribute only applies to functions}}

[PATCH] D60541: [clang-format] Use SpacesBeforeTrailingComments for "option" directive

[Donald Chai via Phabricator via cfe-commits]

dchai created this revision.
dchai added a reviewer: krasimir.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

AnnotatingParser::next() is needed to implicitly set TT_BlockComment
versus TT_LineComment.  On most other paths through
AnnotatingParser::parseLine(), all tokens are consumed to achieve that.
This change updates one place where this wasn't done.


Repository:
  rC Clang

https://reviews.llvm.org/D60541

Files:
  lib/Format/TokenAnnotator.cpp
  unittests/Format/FormatTestProto.cpp


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -193,6 +193,10 @@
  "\"some.really.long.package.that.exceeds.the.column.limit\";"));
 }
 
+TEST_F(FormatTestProto, TrailingCommentAfterFileOption) {
+  verifyFormat("option java_package = \"foo.pkg\";  // comment\n");
+}
+
 TEST_F(FormatTestProto, FormatsOptions) {
   verifyFormat("option (MyProto.options) = {\n"
"  field_a: OK\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1124,8 +1124,11 @@
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
 CurrentToken->is(Keywords.kw_option)) {
   next();
-  if (CurrentToken && CurrentToken->is(tok::identifier))
+  if (CurrentToken && CurrentToken->is(tok::identifier)) {
+while (CurrentToken)
+  next();
 return LT_ImportStatement;
+  }
 }
 
 bool KeywordVirtualFound = false;


Index: unittests/Format/FormatTestProto.cpp
===
--- unittests/Format/FormatTestProto.cpp
+++ unittests/Format/FormatTestProto.cpp
@@ -193,6 +193,10 @@
  "\"some.really.long.package.that.exceeds.the.column.limit\";"));
 }
 
+TEST_F(FormatTestProto, TrailingCommentAfterFileOption) {
+  verifyFormat("option java_package = \"foo.pkg\";  // comment\n");
+}
+
 TEST_F(FormatTestProto, FormatsOptions) {
   verifyFormat("option (MyProto.options) = {\n"
"  field_a: OK\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -1124,8 +1124,11 @@
 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 &&
 CurrentToken->is(Keywords.kw_option)) {
   next();
-  if (CurrentToken && CurrentToken->is(tok::identifier))
+  if (CurrentToken && CurrentToken->is(tok::identifier)) {
+while (CurrentToken)
+  next();
 return LT_ImportStatement;
+  }
 }
 
 bool KeywordVirtualFound = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58367: [analyzer] NFC: Improve upon the concept of BugReporterVisitor.

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

Szelethus added a comment.

Hmmm, interesting. Could there be an issue with `NoteTag` not being trivially 
destructible?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58367



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

[PATCH] D60281: [analyzer] Add docs for cplusplus.InnerPointer

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

Szelethus added inline comments.



Comment at: docs/analyzer/checkers.rst:225-226
+``std::string``s, by recognizing member functions that may re/deallocate the 
buffer
+before use. In the future, it would be great to add support for other STL and
+non-STL containers, and most notably, ``std::string_view``s.
+

dkrupp wrote:
> Szelethus wrote:
> > Hmm. While this page is a documentation, I would still expect regular users 
> > to browse through it -- are we sure that we want to add future plans for a 
> > non-alpha checker? I'm not against it, just a question.
> I think it is a good idea. A non-alpha checker can also be further developed, 
> by anyone else. It is good that we don't forget about further features. This 
> note also highlights the limitations of the checker.
How about this: "Future plans include to add support for blahblah". The current 
statement should rather be a TODO in the code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60281



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

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus added a comment.

Sorry, I rushed my earlier comment -- I definitely think that we should get rid 
of the `UINT_MAX` thing before landing this.


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

https://reviews.llvm.org/D59555



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

[PATCH] D60112: [analyzer] Treat write into a top-level parameter variable with destructor as escape.

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

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

Okay, I played around with this patch, I see now where this is going! LGTM!

> Do you think i should document it somehow?

Aye, the description you gave was enlightening, thanks! If you can squeeze it 
somewhere in the code where it isn't out of place, it's all the better! :)




Comment at: clang/test/Analysis/malloc.cpp:151
+  char *getName() {
+if (!name) {
+  name = static_cast(malloc(10));

NoQ wrote:
> Szelethus wrote:
> > Is this relevant? `name` will never be null.
> Not really, just makes the code look a bit more sensible and idiomatic and 
> less warning-worthy-anyway, to make it as clear as possible that the positive 
> here is indeed false. We don't really have a constructor in this class, but 
> we can imagine that it zero-initializes name. Without this check calling 
> `getName()` multiple times would immediately result in a leak.
Convinced ;)


Repository:
  rC Clang

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

https://reviews.llvm.org/D60112



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

[PATCH] D59347: [DebugInfo] Combine Trivial and NonTrivial flags

[David Blaikie via Phabricator via cfe-commits]

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

Approved pending the LLVM side changes/discussion.


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

https://reviews.llvm.org/D59347



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

[PATCH] D59673: [Driver] Allow setting the DWO name DWARF attribute separately

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59673#1460605 , @aaronpuchert 
wrote:

> Ok, here is an idea. We introduce `-split-dwarf-output` in Clang instead of 
> `-fsplit-dwarf-dwo-name-attr`. If given, it overrides the output file name 
> for the Split DWARF file, which we otherwise take from `-split-dwarf-file`. 
> The option is obviously incompatible with `-enable-split-dwarf=single`, so we 
> will disallow that. This should be backwards-compatible, and bring the 
> behavior of `llc` and `clang -cc1` closer together. What do you think?


Sure, I think the naming's a bit weird (but hard to come up with good names for 
any of this) - but consistency seems like a good first pass at least, given 
we're halfway there anyway.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59673



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

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

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

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

LGTM

Generally it's a good thing for our test suite to be robust against changes to 
Clang's default language mode.


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

https://reviews.llvm.org/D60539



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

[PATCH] D60539: Add -std=c++14 language standard option to tests that require C++14 default

[Amy Kwan via Phabricator via cfe-commits]

amyk created this revision.
amyk added reviewers: ilya-biryukov, sammccall, ioeric, hokein, akyrtzi, yvvan.
amyk added projects: clang, LLVM.
Herald added subscribers: kadircet, arphaman, dexonsmith, jkorous.

On one of the platforms that we build on, we build with the CMake macro, 
`CLANG_DEFAULT_STD_CXX` to set the default language level when building Clang 
and LLVM.

In our case, we set the default to be `gnucxx11`. However, doing so will cause 
the test cases in this patch to fail as they rely on the C++14 default.

This patch explicitly adds the `-std=c++14` to the affected test cases so they 
will work when the default language level is set.

I have added individuals who have worked with these test cases in the past as 
reviewers. I would greatly appreciate it if any of you can inform me on whether 
or not this change is acceptable.


https://reviews.llvm.org/D60539

Files:
  clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
  clang/test/Index/print-type-size.cpp


Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -1,6 +1,6 @@
 // from SemaCXX/class-layout.cpp
-// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | 
FileCheck -check-prefix=CHECK64 %s
-// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 | 
FileCheck -check-prefix=CHECK32 %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu 
-std=c++14 | FileCheck -check-prefix=CHECK64 %s
+// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 
-std=c++14 | FileCheck -check-prefix=CHECK32 %s
 
 namespace basic {
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -109,6 +109,7 @@
 File.Filename = FileName;
 File.HeaderCode = HeaderCode;
 File.Code = Code;
+File.ExtraArgs.push_back("-std=c++14");
 AST = File.build();
   }
 


Index: clang/test/Index/print-type-size.cpp
===
--- clang/test/Index/print-type-size.cpp
+++ clang/test/Index/print-type-size.cpp
@@ -1,6 +1,6 @@
 // from SemaCXX/class-layout.cpp
-// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu | FileCheck -check-prefix=CHECK64 %s
-// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 | FileCheck -check-prefix=CHECK32 %s
+// RUN: c-index-test -test-print-type-size %s -target x86_64-pc-linux-gnu -std=c++14 | FileCheck -check-prefix=CHECK64 %s
+// RUN: c-index-test -test-print-type-size %s -target i386-apple-darwin9 -std=c++14 | FileCheck -check-prefix=CHECK32 %s
 
 namespace basic {
 
Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -109,6 +109,7 @@
 File.Filename = FileName;
 File.HeaderCode = HeaderCode;
 File.Code = Code;
+File.ExtraArgs.push_back("-std=c++14");
 AST = File.build();
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358136 - Fix a test, NFC

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Apr 10 14:18:21 2019
New Revision: 358136

URL: http://llvm.org/viewvc/llvm-project?rev=358136=rev
Log:
Fix a test, NFC

This test was duplicated, and the last declaration had some syntax errors since
the invalid attribute caused the @implementation to be skipped by the parser.

Removed:
cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m
Modified:
cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m

Removed: cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m?rev=358135=auto
==
--- cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m (original)
+++ cfe/trunk/test/CodeGenObjC/objc-asm-attribute-neg-test.m (removed)
@@ -1,34 +0,0 @@
-// RUN: %clang_cc1  -fsyntax-only -verify -Wno-objc-root-class %s
-// rdar://16462586
-
-__attribute__((objc_runtime_name("MySecretNamespace.Protocol")))
-@protocol Protocol
-@end
-
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@interface Message  { 
-__attribute__((objc_runtime_name("MySecretNamespace.Message"))) // 
expected-error {{'objc_runtime_name' attribute only applies to Objective-C 
interfaces and Objective-C protocols}}
-  id MyIVAR;
-}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@property int MyProperty; // expected-error {{prefix attribute must be 
followed by an interface or protocol
-
-- (int) getMyProperty 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))); // 
expected-error {{'objc_runtime_name' attribute only applies to}}
-
-- (void) setMyProperty : (int) arg 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))); // 
expected-error {{'objc_runtime_name' attribute only applies to}}
-
-@end
-
-__attribute__((objc_runtime_name("MySecretNamespace.ForwardClass")))
-@class ForwardClass; // expected-error {{prefix attribute must be followed by 
an interface or protocol}}
-
-__attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
-@protocol ForwardProtocol;
-
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@implementation Message // expected-error {{prefix attribute must be followed 
by an interface or protocol}}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-- (id) MyMethod {
-  return MyIVAR;
-}
-@end

Modified: cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m?rev=358136=358135=358136=diff
==
--- cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m (original)
+++ cfe/trunk/test/SemaObjC/objc-asm-attribute-neg-test.m Wed Apr 10 14:18:21 
2019
@@ -33,10 +33,18 @@ __attribute__((objc_runtime_name("MySecr
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
 @protocol ForwardProtocol;
 
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-@implementation Message // expected-error {{prefix attribute must be followed 
by an interface or protocol}}
-__attribute__((objc_runtime_name("MySecretNamespace.Message")))
-- (id) MyMethod {
+@implementation Message
+// expected-error@+1 {{'objc_runtime_name' attribute only applies to 
Objective-C interfaces and Objective-C protocols}}
+- (id) MyMethod 
__attribute__((objc_runtime_name("MySecretNamespace.Message"))) {
   return MyIVAR;
 }
+
+-(int)getMyProperty { return 0; }
+-(void)setMyProperty:(int)arg {}
 @end
+
+@interface NoImpl @end
+
+__attribute__((objc_runtime_name("MySecretNamespace.Message")))
+// expected-error@+1 {{prefix attribute must be followed by an interface or 
protocol}}
+@implementation NoImpl @end


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

[PATCH] D59923: [Driver] Simplify -g level computation and its interaction with -gsplit-dwarf

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59923#1460696 , @MaskRay wrote:

> > is that to imply that the first block all do not use split DWARF?
>
> The first block do not use split DWARF.


That doesn't sound like what I'd expect (& would represent a change in behavior 
as well). The first block reads:

-gsplit-dwarf -g0 => 0
-gsplit-dwarf -gline-directives-only => DebugDirectivesOnly
-gsplit-dwarf -gmlt -fsplit-dwarf-inlining => 1
-gsplit-dwarf -gmlt -fno-split-dwarf-inlining => 1

This last one currently produces split-dwarf (if there's any DWARF worth 
splitting - if there are any subprogram descriptions, etc, otherwise it saves 
the indirection and produces an empty .dwo file).

>> In a previous message I think you said that the only change was "-gmlt 
>> -gsplit-dwarf -fno-split-dwarf-inlining => 1 (before) 2 (after)" - which I'm 
>> not sure is an improvement.
> 
> Yes, this is the only behavioral change.
> 
>> You mentioned that the inconsistency between "-g0 -gsplit-dwarf" and "-gmlt 
>> -gsplit-dwarf -fno-split-dwarf-inlining" was confusing. But still there will 
>> be an inconsistency between "-gsplit-dwarf -g0" and "-gsplit-dwarf -gmlt 
>> -fno-split-dwarf-inlining", yes?
> 
> The debug info level will be consistent after this patch:  the last of 
> `-gsplit-dwarf -g0 -g1 -g2 -g3 -ggdb[0-3] -gdwarf-*` will decide the debug 
> info level (`-gsplit-dwarf -gdwarf-*` have level 2). Next, a separate rule 
> decides if the `-gsplit-dwarf` takes effect (not if `DebugInfoKind == 
> codegenoptions::NoDebugInfo || DebugInfoKind == 
> codegenoptions::DebugDirectivesOnly || (DebugInfoKind == 
> codegenoptions::DebugLineTablesOnly && SplitDWARFInlining)`)
> 
>> I think that under -fno-split-dwarf-inlining, -gmlt and -gsplit-dwarf should 
>> be order independent and compositional rather than overriding. Having them 
>> compose in one order but not the other seems confusing to me.
> 
> The existence of `-fno-split-dwarf-inlining` changing the position dependence 
> makes me confused:
> 
> - Without it, the latter of `-gmlt` and `-gsplit-dwarf` decides the debug 
> info level
> - With it, `-gmlt` decides the debug info level

Seems there's going to be confusion either way, though - either the 
presence/absence of -fno-split-dwarf-inlining changes whether -gsplit-dwarf is 
respected/ignored (in the presence of -gmlt), or changes whethe -gmlt composes 
with -gsplit-dwarf or overrides it? Seems these are both a bit confusing, no?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59923



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358134: Check i  FD-getNumParams() before querying 
(authored by gribozavr, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60055?vs=194152=194581#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60055

Files:
  lib/Sema/SemaTemplateInstantiate.cpp
  test/SemaCXX/PR41139.cpp
  test/SemaCXX/cxx1y-generic-lambdas.cpp


Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2892,7 +2892,7 @@
   unsigned i = PV->getFunctionScopeIndex();
   // This parameter might be from a freestanding function type within the
   // function and isn't necessarily referring to one of FD's parameters.
-  if (FD->getParamDecl(i) == PV)
+  if (i < FD->getNumParams() && FD->getParamDecl(i) == PV)
 return FD->getCanonicalDecl()->getParamDecl(i);
 }
   }
Index: test/SemaCXX/cxx1y-generic-lambdas.cpp
===
--- test/SemaCXX/cxx1y-generic-lambdas.cpp
+++ test/SemaCXX/cxx1y-generic-lambdas.cpp
@@ -944,6 +944,15 @@
   }(0)(0);
 }
 
+namespace PR41139 {
+  int y = [](auto outer) {
+return [](auto inner) {
+  using T = int(decltype(outer), decltype(inner));
+  return 0;
+};
+  }(0)(0);
+}
+
 namespace PR23716 {
 template
 auto f(T x) {
Index: test/SemaCXX/PR41139.cpp
===
--- test/SemaCXX/PR41139.cpp
+++ test/SemaCXX/PR41139.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+// This test should not crash.
+int f1( unsigned ) { return 0; }
+
+template 
+struct S1 {
+S1( R(*f)(Args...) ) {}
+};
+
+int main() {
+S1 s1( f1 );
+}


Index: lib/Sema/SemaTemplateInstantiate.cpp
===
--- lib/Sema/SemaTemplateInstantiate.cpp
+++ lib/Sema/SemaTemplateInstantiate.cpp
@@ -2892,7 +2892,7 @@
   unsigned i = PV->getFunctionScopeIndex();
   // This parameter might be from a freestanding function type within the
   // function and isn't necessarily referring to one of FD's parameters.
-  if (FD->getParamDecl(i) == PV)
+  if (i < FD->getNumParams() && FD->getParamDecl(i) == PV)
 return FD->getCanonicalDecl()->getParamDecl(i);
 }
   }
Index: test/SemaCXX/cxx1y-generic-lambdas.cpp
===
--- test/SemaCXX/cxx1y-generic-lambdas.cpp
+++ test/SemaCXX/cxx1y-generic-lambdas.cpp
@@ -944,6 +944,15 @@
   }(0)(0);
 }
 
+namespace PR41139 {
+  int y = [](auto outer) {
+return [](auto inner) {
+  using T = int(decltype(outer), decltype(inner));
+  return 0;
+};
+  }(0)(0);
+}
+
 namespace PR23716 {
 template
 auto f(T x) {
Index: test/SemaCXX/PR41139.cpp
===
--- test/SemaCXX/PR41139.cpp
+++ test/SemaCXX/PR41139.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+// This test should not crash.
+int f1( unsigned ) { return 0; }
+
+template 
+struct S1 {
+S1( R(*f)(Args...) ) {}
+};
+
+int main() {
+S1 s1( f1 );
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r358134 - Check i < FD->getNumParams() before querying

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Wed Apr 10 13:25:07 2019
New Revision: 358134

URL: http://llvm.org/viewvc/llvm-project?rev=358134=rev
Log:
Check i < FD->getNumParams() before querying

Summary:
As was already stated in a previous comment, the parameter isn't
necessarily referring to one of the DeclContext's parameter. We
should check the index is within the range to avoid out-of-boundary
access.

Reviewers: gribozavr, rsmith, lebedev.ri

Reviewed By: gribozavr, rsmith

Subscribers: lebedev.ri, cfe-commits

Tags: #clang

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

Patch by Violet.

Added:
cfe/trunk/test/SemaCXX/PR41139.cpp
Modified:
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=358134=358133=358134=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Wed Apr 10 13:25:07 2019
@@ -2892,7 +2892,7 @@ static const Decl *getCanonicalParmVarDe
   unsigned i = PV->getFunctionScopeIndex();
   // This parameter might be from a freestanding function type within the
   // function and isn't necessarily referring to one of FD's parameters.
-  if (FD->getParamDecl(i) == PV)
+  if (i < FD->getNumParams() && FD->getParamDecl(i) == PV)
 return FD->getCanonicalDecl()->getParamDecl(i);
 }
   }

Added: cfe/trunk/test/SemaCXX/PR41139.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/PR41139.cpp?rev=358134=auto
==
--- cfe/trunk/test/SemaCXX/PR41139.cpp (added)
+++ cfe/trunk/test/SemaCXX/PR41139.cpp Wed Apr 10 13:25:07 2019
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify %s
+
+// expected-no-diagnostics
+
+// This test should not crash.
+int f1( unsigned ) { return 0; }
+
+template 
+struct S1 {
+S1( R(*f)(Args...) ) {}
+};
+
+int main() {
+S1 s1( f1 );
+}

Modified: cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp?rev=358134=358133=358134=diff
==
--- cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp (original)
+++ cfe/trunk/test/SemaCXX/cxx1y-generic-lambdas.cpp Wed Apr 10 13:25:07 2019
@@ -944,6 +944,15 @@ namespace PR22117 {
   }(0)(0);
 }
 
+namespace PR41139 {
+  int y = [](auto outer) {
+return [](auto inner) {
+  using T = int(decltype(outer), decltype(inner));
+  return 0;
+};
+  }(0)(0);
+}
+
 namespace PR23716 {
 template
 auto f(T x) {


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

r358133 - [clang][ASTContext] Try to exit early before loading serialized comments from AST files

[Jan Korous via cfe-commits]

Author: jkorous
Date: Wed Apr 10 13:23:33 2019
New Revision: 358133

URL: http://llvm.org/viewvc/llvm-project?rev=358133=rev
Log:
[clang][ASTContext] Try to exit early before loading serialized comments from 
AST files

Loading external comments is expensive. This change probably doesn't apply to 
common cases but is almost for free and would save some work in case none of 
the declaration needs external comments to be loaded.

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

Modified:
cfe/trunk/lib/AST/ASTContext.cpp

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=358133=358132=358133=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Apr 10 13:23:33 2019
@@ -99,20 +99,13 @@ enum FloatingRank {
 };
 
 RawComment *ASTContext::getRawCommentForDeclNoCache(const Decl *D) const {
-  if (!CommentsLoaded && ExternalSource) {
-ExternalSource->ReadComments();
-
-#ifndef NDEBUG
-ArrayRef RawComments = Comments.getComments();
-assert(std::is_sorted(RawComments.begin(), RawComments.end(),
-  BeforeThanCompare(SourceMgr)));
-#endif
-
-CommentsLoaded = true;
-  }
-
   assert(D);
 
+  // If we already tried to load comments but there are none,
+  // we won't find anything.
+  if (CommentsLoaded && Comments.getComments().empty())
+return nullptr;
+
   // User can not attach documentation to implicit declarations.
   if (D->isImplicit())
 return nullptr;
@@ -162,12 +155,6 @@ RawComment *ASTContext::getRawCommentFor
   isa(D))
 return nullptr;
 
-  ArrayRef RawComments = Comments.getComments();
-
-  // If there are no comments anywhere, we won't find anything.
-  if (RawComments.empty())
-return nullptr;
-
   // Find declaration location.
   // For Objective-C declarations we generally don't expect to have multiple
   // declarators, thus use declaration starting location as the "declaration
@@ -206,6 +193,23 @@ RawComment *ASTContext::getRawCommentFor
   if (DeclLoc.isInvalid() || !DeclLoc.isFileID())
 return nullptr;
 
+  if (!CommentsLoaded && ExternalSource) {
+ExternalSource->ReadComments();
+
+#ifndef NDEBUG
+ArrayRef RawComments = Comments.getComments();
+assert(std::is_sorted(RawComments.begin(), RawComments.end(),
+  BeforeThanCompare(SourceMgr)));
+#endif
+
+CommentsLoaded = true;
+  }
+
+  ArrayRef RawComments = Comments.getComments();
+  // If there are no comments anywhere, we won't find anything.
+  if (RawComments.empty())
+return nullptr;
+
   // Find the comment that occurs just after this declaration.
   ArrayRef::iterator Comment;
   {


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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[John McCall via Phabricator via cfe-commits]

rjmccall closed this revision.
rjmccall added a comment.

Committed as r358132.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

r358132 - Add IRGen APIs to fetch ctor/dtor helper functions for non-trivial structs.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 12:57:20 2019
New Revision: 358132

URL: http://llvm.org/viewvc/llvm-project?rev=358132=rev
Log:
Add IRGen APIs to fetch ctor/dtor helper functions for non-trivial structs.

Patch by Tony Allevato!

Modified:
cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp

Modified: cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h?rev=358132=358131=358132=diff
==
--- cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h (original)
+++ cfe/trunk/include/clang/CodeGen/CodeGenABITypes.h Wed Apr 10 12:57:20 2019
@@ -30,6 +30,7 @@
 namespace llvm {
   class DataLayout;
   class Module;
+  class Function;
   class FunctionType;
   class Type;
 }
@@ -83,6 +84,59 @@ llvm::Type *convertTypeForMemory(CodeGen
 unsigned getLLVMFieldNumber(CodeGenModule ,
 const RecordDecl *RD, const FieldDecl *FD);
 
+/// Returns the default constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes a single argument that is a
+/// pointer to the address of the struct.
+llvm::Function *getNonTrivialCStructDefaultConstructor(CodeGenModule ,
+   CharUnits DstAlignment,
+   bool IsVolatile,
+   QualType QT);
+
+/// Returns the copy constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes two arguments: pointers to the
+/// addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructCopyConstructor(CodeGenModule ,
+CharUnits DstAlignment,
+CharUnits SrcAlignment,
+bool IsVolatile,
+QualType QT);
+
+/// Returns the move constructor for a C struct with non-trivially copyable
+/// fields, generating it if necessary. The returned function uses the `cdecl`
+/// calling convention, returns void, and takes two arguments: pointers to the
+/// addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructMoveConstructor(CodeGenModule ,
+CharUnits DstAlignment,
+CharUnits SrcAlignment,
+bool IsVolatile,
+QualType QT);
+
+/// Returns the copy assignment operator for a C struct with non-trivially
+/// copyable fields, generating it if necessary. The returned function uses the
+/// `cdecl` calling convention, returns void, and takes two arguments: pointers
+/// to the addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT);
+
+/// Return the move assignment operator for a C struct with non-trivially
+/// copyable fields, generating it if necessary. The returned function uses the
+/// `cdecl` calling convention, returns void, and takes two arguments: pointers
+/// to the addresses of the destination and source structs, respectively.
+llvm::Function *getNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT);
+
+/// Returns the destructor for a C struct with non-trivially copyable fields,
+/// generating it if necessary. The returned function uses the `cdecl` calling
+/// convention, returns void, and takes a single argument that is a pointer to
+/// the address of the struct.
+llvm::Function *getNonTrivialCStructDestructor(CodeGenModule ,
+   CharUnits DstAlignment,
+   bool IsVolatile, QualType QT);
+
 }  // end namespace CodeGen
 }  // end namespace clang
 

Modified: cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp?rev=358132=358131=358132=diff
==
--- cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGNonTrivialStruct.cpp Wed Apr 10 12:57:20 2019
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/NonTrivialTypeVisitor.h"

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

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

rsmith added a comment.

I expect that we get this wrong in the same way for all the `SemaDeclRefs` 
declarations (`StdNamespace`, `StdBadAlloc`, and `StdAlignValT`).

I think the place where this is going wrong is `ASTDeclReader::findExisting`, 
which is assuming that the prior declaration for normal `NamedDecl`s can be 
found by name lookup; that's not true for these particular implicitly-declared 
entities. Perhaps the simplest fix would be to add a check around here:

  } else if (DeclContext *MergeDC = getPrimaryContextForMerging(Reader, DC)) {
DeclContext::lookup_result R = MergeDC->noload_lookup(Name);
for (DeclContext::lookup_iterator I = R.begin(), E = R.end(); I != E; ++I) {
  if (NamedDecl *Existing = getDeclForMerging(*I, TypedefNameForLinkage))
if (isSameEntity(Existing, D))
  return FindExistingResult(Reader, D, Existing, AnonymousDeclNumber,
TypedefNameForLinkage);
}
// [HERE]
  }

... to see if `D` is one of our three special entities, and if so to ask `Sema` 
for its current declaration of that entity (without triggering deserialization) 
and if not, set `D` as the declaration of that entity.




Comment at: lib/Sema/SemaDeclCXX.cpp:8915-8919
+  if (II->isStr("std") && PrevNS->isStdNamespace() &&
+  PrevNS != getStdNamespace()) {
+PrevNS = getStdNamespace();
+IsStd = true;
+  }

This doesn't seem right to me. This change appears to affect the case where we 
already have two distinct `std` namespaces and are declaring a third, which is 
too late -- things already went wrong if we reach that situation.



Comment at: lib/Sema/SemaDeclCXX.cpp:9215
+if (getLangOpts().Modules)
+  getStdNamespace()->setHasExternalVisibleStorage(true);
   }

This also doesn't look right: the "has external visible storage" flag should 
only be set by the external source, to indicate that it has lookup results for 
the decl context.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58920



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

RE: [clang-tools-extra] r358103 - clangd: fix the build with XPC

[via cfe-commits]

Hi Saleem,

This fix does not seem to work on the PS4 linux build bot. Can you take a look?

http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/46575

FAILED: 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o
 
/usr/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/clangd/fuzzer 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer
 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/include
 -Itools/clang/include -Iinclude 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/include
 
-I/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/..
 -std=c++11 -Wdocumentation -Wno-documentation-deprecated-sync -fPIC 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -std=c++11 -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections 
-fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3-UNDEBUG  
-fno-exceptions -fno-rtti -MD -MT 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o
 -MF 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o.d
 -o 
tools/clang/tools/extra/clangd/fuzzer/CMakeFiles/clangd-fuzzer.dir/clangd-fuzzer.cpp.o
 -c 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/clangd-fuzzer.cpp
In file included from 
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/clangd-fuzzer.cpp:15:
/home/buildslave/ps4-buildslave4/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/tools/extra/clangd/fuzzer/../ClangdLSPServer.h:14:10:
 fatal error: 'Features.inc' file not found
#include "Features.inc"
 ^~

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Saleem 
Abdulrasool via cfe-commits
Sent: Wednesday, April 10, 2019 9:49
To: cfe-commits@lists.llvm.org
Subject: [clang-tools-extra] r358103 - clangd: fix the build with XPC

Author: compnerd
Date: Wed Apr 10 09:48:52 2019
New Revision: 358103

URL: http://llvm.org/viewvc/llvm-project?rev=358103=rev
Log:
clangd: fix the build with XPC

`Transport.h` does not include `Features.inc`.  However, since it is used in a 
subdirectory, it cannot directly include the header as it is not available.
Include `Features.inc` in `ClangdLSPServer.h` prior to the inclusion of 
`Transport.h` which will provide the interfaces in `ClangdMain.cpp` where the 
symbol `newXPCTransport` will not be defined due to it being preprocessed away 
since the configuration is not passed along to the initial inclusion.

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=358103=358102=358103=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Apr 10 09:48:52 
+++ 2019
@@ -11,6 +11,7 @@
 
 #include "ClangdServer.h"
 #include "DraftStore.h"
+#include "Features.inc"
 #include "FindSymbols.h"
 #include "GlobalCompilationDatabase.h"
 #include "Path.h"


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

[clang-tools-extra] r358127 - clangd-fuzzer: repair the build

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 12:16:14 2019
New Revision: 358127

URL: http://llvm.org/viewvc/llvm-project?rev=358127=rev
Log:
clangd-fuzzer: repair the build

The inclusion of private headers of clangd percolates into the fuzzer.

Modified:
clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt

Modified: clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt?rev=358127=358126=358127=diff
==
--- clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/fuzzer/CMakeLists.txt Wed Apr 10 12:16:14 
2019
@@ -1,4 +1,5 @@
-include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..
+  ${CMAKE_CURRENT_BINARY_DIR}/..)
 
 set(LLVM_LINK_COMPONENTS
   FuzzMutate


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

r358126 - [OPENMP]Improve detection of number of teams, threads in target

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Apr 10 12:11:33 2019
New Revision: 358126

URL: http://llvm.org/viewvc/llvm-project?rev=358126=rev
Log:
[OPENMP]Improve detection of number of teams, threads in target
regions.

Added more complex analysis for number of teams and number of threads in
the target regions, also merged related common code between CGOpenMPRuntime
and CGOpenMPRuntimeNVPTX classes.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/distribute_simd_reduction_codegen.cpp
cfe/trunk/test/OpenMP/target_map_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_depend_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_private_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_reduction_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_private_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_simd_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=358126=358125=358126=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Apr 10 12:11:33 2019
@@ -6475,12 +6475,59 @@ void CGOpenMPRuntime::emitTargetOutlined
   OffloadEntriesInfoManagerTy::OMPTargetRegionEntryTargetRegion);
 }
 
-/// discard all CompoundStmts intervening between two constructs
-static const Stmt *ignoreCompoundStmts(const Stmt *Body) {
-  while (const auto *CS = dyn_cast_or_null(Body))
-Body = CS->body_front();
+/// Checks if the expression is constant or does not have non-trivial function
+/// calls.
+static bool isTrivial(ASTContext , const Expr * E) {
+  // We can skip constant expressions.
+  // We can skip expressions with trivial calls or simple expressions.
+  return (E->isEvaluatable(Ctx, Expr::SE_AllowUndefinedBehavior) ||
+  !E->hasNonTrivialCall(Ctx)) &&
+ !E->HasSideEffects(Ctx, /*IncludePossibleEffects=*/true);
+}
 
-  return Body;
+const Stmt *CGOpenMPRuntime::getSingleCompoundChild(ASTContext ,
+const Stmt *Body) {
+  const Stmt *Child = Body->IgnoreContainers();
+  while (const auto *C = dyn_cast_or_null(Child)) {
+Child = nullptr;
+for (const Stmt *S : C->body()) {
+  if (const auto *E = dyn_cast(S)) {
+if (isTrivial(Ctx, E))
+  continue;
+  }
+  // Some of the statements can be ignored.
+  if (isa(S) || isa(S) || isa(S) ||
+  isa(S) || isa(S))
+continue;
+  // Analyze declarations.
+  if (const auto *DS = dyn_cast(S)) {
+if (llvm::all_of(DS->decls(), [](const Decl *D) {
+  if (isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) || isa(D) ||
+  isa(D) ||
+  isa(D) ||
+  isa(D) || isa(D))
+return true;
+  const auto *VD = dyn_cast(D);
+  if (!VD)
+return false;
+  return VD->isConstexpr() ||
+ ((VD->getType().isTrivialType(Ctx) ||
+   VD->getType()->isReferenceType()) &&
+  (!VD->hasInit() || isTrivial(Ctx, VD->getInit(;
+}))
+  continue;
+  }
+  // Found multiple children - cannot get the one child only.
+  if (Child)
+return nullptr;
+  Child = S;
+}
+if (Child)
+  Child = Child->IgnoreContainers();
+  }
+  return Child;
 }
 
 /// Emit the number of teams for a target directive.  Inspect the num_teams
@@ -6492,63 +6539,163 @@ static const Stmt *ignoreCompoundStmts(c
 ///
 /// Otherwise, return nullptr.
 static llvm::Value *
-emitNumTeamsForTargetDirective(CGOpenMPRuntime ,
-   CodeGenFunction ,
+emitNumTeamsForTargetDirective(CodeGenFunction ,
const OMPExecutableDirective ) {
-  assert(!CGF.getLangOpts().OpenMPIsDevice && "Clauses associated with the "
-  "teams directive expected to be "
-  "emitted only for the host!");
-
+  assert(!CGF.getLangOpts().OpenMPIsDevice &&
+ "Clauses associated with the teams directive expected to be emitted "
+ "only for the host!");
+  OpenMPDirectiveKind DirectiveKind = D.getDirectiveKind();
+  

r358125 - Fix for different build configurations.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 12:11:32 2019
New Revision: 358125

URL: http://llvm.org/viewvc/llvm-project?rev=358125=rev
Log:
Fix for different build configurations.

Modified:
cfe/trunk/test/CodeGen/unreachable-ret.c

Modified: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358125=358124=358125=diff
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (original)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 12:11:32 2019
@@ -5,7 +5,7 @@ extern void abort() __attribute__((noret
 void f1() {
   abort();
 }
-// CHECK-LABEL: define void @f1()
+// CHECK-LABEL: define {{.*}}void @f1()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @abort()
 // CHECK-NEXT:   unreachable
@@ -15,7 +15,7 @@ void *f2() {
   abort();
   return 0;
 }
-// CHECK-LABEL: define i8* @f2()
+// CHECK-LABEL: define {{.*}}i8* @f2()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:   call void @abort()
 // CHECK-NEXT:   unreachable


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

[PATCH] D60263: [clang-format] Preserve include blocks in ObjC Google style

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added a comment.

Looks good to me.




Comment at: cfe/trunk/lib/Format/Format.cpp:881
+// "Regroup" doesn't work well for ObjC yet (main header heuristic,
+// relationship between ObjC standard library headers and other heades,
+// #imports, etc.)

nit: "headers"


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60263



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

[PATCH] D60523: [clang] Bugfixe for 41400

[Gauthier via Phabricator via cfe-commits]

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

this is a bugfixe for bugtracker 

added nullptr check at the relevent place and test


Repository:
  rC Clang

https://reviews.llvm.org/D60523

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/using-decl-1.cpp


Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -396,3 +396,10 @@
   using N::Y;
   using N::Z;
 }
+
+// expected-error@+5 {{requires a qualified name}}
+// expected-error@+4 {{expected ';'}}
+// expected-error@+3 {{expected '}'}}
+// expected-note@+2 {{to match this '{'}}
+// expected-error@+1 {{expected ';'}}
+template struct S { using S
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -90,7 +90,7 @@
   // When naming a constructor as a member of a dependent context (eg, in a
   // friend declaration or an inherited constructor declaration), form an
   // unresolved "typename" type.
-  if (CurClass->isDependentContext() && !EnteringContext) {
+  if (CurClass->isDependentContext() && !EnteringContext && SS.getScopeRep()) {
 QualType T = Context.getDependentNameType(ETK_None, SS.getScopeRep(), );
 return ParsedType::make(T);
   }


Index: clang/test/SemaCXX/using-decl-1.cpp
===
--- clang/test/SemaCXX/using-decl-1.cpp
+++ clang/test/SemaCXX/using-decl-1.cpp
@@ -396,3 +396,10 @@
   using N::Y;
   using N::Z;
 }
+
+// expected-error@+5 {{requires a qualified name}}
+// expected-error@+4 {{expected ';'}}
+// expected-error@+3 {{expected '}'}}
+// expected-note@+2 {{to match this '{'}}
+// expected-error@+1 {{expected ';'}}
+template struct S { using S
\ No newline at end of file
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -90,7 +90,7 @@
   // When naming a constructor as a member of a dependent context (eg, in a
   // friend declaration or an inherited constructor declaration), form an
   // unresolved "typename" type.
-  if (CurClass->isDependentContext() && !EnteringContext) {
+  if (CurClass->isDependentContext() && !EnteringContext && SS.getScopeRep()) {
 QualType T = Context.getDependentNameType(ETK_None, SS.getScopeRep(), );
 return ParsedType::make(T);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 194551.
ymandel added a comment.

tweaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,222 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::sNode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher ) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext  = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], )};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil ,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
+} else {
+  auto Err = llvm::handleErrors(ResultOrErr.takeError(),
+

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Thanks for the review. I've changed most of the things with only a few open 
questions. PTAL.




Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45
+  /// Evaluates this part to a string and appends it to `result`.
+  virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult ,
+   std::string *Result) const = 0;

sbenza wrote:
> Why not use `llvm::Expected` as the return type?
so that each stencil part can append to a single string that we construct when 
evaluating the whole stencil.  this was an (attempted?) optimization. if 
concatenating is about as efficient, I'd prefer that approach. What do you 
advise?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75
+  // Copy constructor/assignment produce a deep copy.
+  StencilPart(const StencilPart ) : Impl(P.Impl->clone()) {}
+  StencilPart(StencilPart &&) = default;

sbenza wrote:
> The interface API is all const. Why not keep a `shared_ptr` instead?
> That way we don't have to have users implement a clone function.
Excellent! I guess I'm allergic to shared_ptr because of its atomics but its 
clearly a win here. Thanks!



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127
+  // Allow Stencils to operate as std::function, for compatibility with
+  // Transformer's TextGenerator. Calls `eval()` and asserts on failure.
+  std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const;

sbenza wrote:
> "asserts" as it only fails in debug mode?
I thought `assert()` also fails in normal mode, based on my reading of the llvm 
docs, but it's not explicit about this: 
http://llvm.org/docs/ProgrammersManual.html#programmatic-errors

FWIW, I'm on the fence whether `eval()` should actually have the same signature 
and similarly just crash the program on errors. Its not clear there's any value 
to propogating the errors up the stack via `Expected`.



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45
+
+namespace {
+// An arbitrary fragment of code within a stencil.

sbenza wrote:
> maybe this anon namespace should cover the functions above?
the style guide advises against. I've been told to only put type definitions 
inside anon namespaces.



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76
+
+static bool operator==(const RawTextData , const RawTextData ) {
+  return A.Text == B.Text;

sbenza wrote:
> If you define ==, imo you should define != too.
> Otherwise just call it `isEqual`
Since I don't have a general need for these as operators, just went w/ the 
latter. Used the same style as `evalData()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371



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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 194550.
ymandel added a comment.

deleted some stray comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,222 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::sNode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher ) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext  = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], )};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil ,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but succeeded: " << *ResultOrErr;
+} else {
+  auto Err = 

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 194549.
ymandel marked 13 inline comments as done.
ymandel added a comment.

Responded to comments and added wrapper for Stencil::cat in stencil namespace.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371

Files:
  clang/include/clang/Tooling/Refactoring/Stencil.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Stencil.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/StencilTest.cpp

Index: clang/unittests/Tooling/StencilTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/StencilTest.cpp
@@ -0,0 +1,222 @@
+//===- unittest/Tooling/StencilTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Refactoring/Stencil.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/FixIt.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace tooling;
+using namespace ast_matchers;
+
+namespace {
+using ::testing::AllOf;
+using ::testing::Eq;
+using ::testing::HasSubstr;
+using MatchResult = MatchFinder::MatchResult;
+using tooling::stencil::node;
+using tooling::stencil::sNode;
+using tooling::stencil::text;
+
+// In tests, we can't directly match on llvm::Expected since its accessors
+// mutate the object. So, we collapse it to an Optional.
+static llvm::Optional toOptional(llvm::Expected V) {
+  if (V)
+return *V;
+  ADD_FAILURE() << "Losing error in conversion to IsSomething: "
+<< llvm::toString(V.takeError());
+  return llvm::None;
+}
+
+// A very simple matcher for llvm::Optional values.
+MATCHER_P(IsSomething, ValueMatcher, "") {
+  if (!arg)
+return false;
+  return ::testing::ExplainMatchResult(ValueMatcher, *arg, result_listener);
+}
+
+// Create a valid translation-unit from a statement.
+static std::string wrapSnippet(llvm::Twine StatementCode) {
+  return ("auto stencil_test_snippet = []{" + StatementCode + "};").str();
+}
+
+static DeclarationMatcher wrapMatcher(const StatementMatcher ) {
+  return varDecl(hasName("stencil_test_snippet"),
+ hasDescendant(compoundStmt(hasAnySubstatement(Matcher;
+}
+
+struct TestMatch {
+  // The AST unit from which `result` is built. We bundle it because it backs
+  // the result. Users are not expected to access it.
+  std::unique_ptr AstUnit;
+  // The result to use in the test. References `ast_unit`.
+  MatchResult Result;
+};
+
+// Matches `Matcher` against the statement `StatementCode` and returns the
+// result. Handles putting the statement inside a function and modifying the
+// matcher correspondingly. `Matcher` should match `StatementCode` exactly --
+// that is, produce exactly one match.
+static llvm::Optional matchStmt(llvm::Twine StatementCode,
+   StatementMatcher Matcher) {
+  auto AstUnit = buildASTFromCode(wrapSnippet(StatementCode));
+  if (AstUnit == nullptr) {
+ADD_FAILURE() << "AST construction failed";
+return llvm::None;
+  }
+  ASTContext  = AstUnit->getASTContext();
+  auto Matches = ast_matchers::match(wrapMatcher(Matcher), Context);
+  // We expect a single, exact match for the statement.
+  if (Matches.size() != 1) {
+ADD_FAILURE() << "Wrong number of matches: " << Matches.size();
+return llvm::None;
+  }
+  return TestMatch{std::move(AstUnit), MatchResult(Matches[0], )};
+}
+
+class StencilTest : public ::testing::Test {
+protected:
+  // Verifies that the given stencil fails when evaluated on a valid match
+  // result. Binds a statement to "stmt", a (non-member) ctor-initializer to
+  // "init", an expression to "expr" and a (nameless) declaration to "decl".
+  void testError(const Stencil ,
+ ::testing::Matcher Matcher) {
+const std::string Snippet = R"cc(
+  struct A {};
+  class F : public A {
+   public:
+F(int) {}
+  };
+  F(1);
+)cc";
+auto StmtMatch = matchStmt(
+Snippet,
+stmt(hasDescendant(
+ cxxConstructExpr(
+ hasDeclaration(decl(hasDescendant(cxxCtorInitializer(
+   isBaseInitializer())
+   .bind("init")))
+.bind("decl")))
+ .bind("expr")))
+.bind("stmt"));
+ASSERT_TRUE(StmtMatch);
+if (auto ResultOrErr = Stencil.eval(StmtMatch->Result)) {
+  ADD_FAILURE() << "Expected failure but 

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

[John McCall via Phabricator via cfe-commits]

rjmccall closed this revision.
rjmccall added a comment.

Committed as r358115.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58016



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

r358115 - Fix an off-by-one mistake in IRGen's copy-construction

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 11:07:18 2019
New Revision: 358115

URL: http://llvm.org/viewvc/llvm-project?rev=358115=rev
Log:
Fix an off-by-one mistake in IRGen's copy-construction
special cases in the presence of zero-length arrays.

Patch by Joran Bigalet!

Modified:
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp

Modified: cfe/trunk/lib/CodeGen/CGClass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGClass.cpp?rev=358115=358114=358115=diff
==
--- cfe/trunk/lib/CodeGen/CGClass.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGClass.cpp Wed Apr 10 11:07:18 2019
@@ -1008,7 +1008,7 @@ namespace {
   if (FOffset < FirstFieldOffset) {
 FirstField = F;
 FirstFieldOffset = FOffset;
-  } else if (FOffset > LastFieldOffset) {
+  } else if (FOffset >= LastFieldOffset) {
 LastField = F;
 LastFieldOffset = FOffset;
   }

Modified: cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp?rev=358115=358114=358115=diff
==
--- cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/pod-member-memcpys.cpp Wed Apr 10 11:07:18 2019
@@ -45,6 +45,13 @@ struct ArrayMember {
   int w, x, y, z;
 };
 
+struct ZeroLengthArrayMember {
+NonPOD np;
+int a;
+int b[0];
+int c;
+};
+
 struct VolatileMember {
   int a, b, c, d;
   volatile int v;
@@ -109,6 +116,7 @@ CALL_AO(Basic)
 CALL_AO(PODMember)
 CALL_AO(PODLikeMember)
 CALL_AO(ArrayMember)
+CALL_AO(ZeroLengthArrayMember)
 CALL_AO(VolatileMember)
 CALL_AO(BitfieldMember)
 CALL_AO(InnerClassMember)
@@ -142,6 +150,12 @@ CALL_AO(PackedMembers)
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 64, i1 {{.*}})
 // CHECK: ret %struct.ArrayMember*
 
+// ZeroLengthArrayMember copy-assignment:
+// CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) 
%struct.ZeroLengthArrayMember* 
@_ZN21ZeroLengthArrayMemberaSERKS_(%struct.ZeroLengthArrayMember* %this, 
%struct.ZeroLengthArrayMember* dereferenceable({{[0-9]+}}))
+// CHECK: call dereferenceable({{[0-9]+}}) %struct.NonPOD* @_ZN6NonPODaSERKS_
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 8, i1 {{.*}})
+// CHECK: ret %struct.ZeroLengthArrayMember*
+
 // VolatileMember copy-assignment:
 // CHECK-LABEL: define linkonce_odr dereferenceable({{[0-9]+}}) 
%struct.VolatileMember* @_ZN14VolatileMemberaSERKS_(%struct.VolatileMember* 
%this, %struct.VolatileMember* dereferenceable({{[0-9]+}}))
 // CHECK: call void @llvm.memcpy.p0i8.p0i8.i64({{.*}} align 4 {{.*}} align 4 
{{.*}}i64 16, i1 {{.*}})


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

[PATCH] D60513: [HIP] Use -mlink-builtin-bitcode to link device library

[Aaron Enye Shi via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D60513



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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

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



Comment at: llvm/lib/Support/CommandLine.cpp:1187
+// FIXME: This is needed, but not sure why.
+updateArgStr(Opt, NewArg, ChosenSubCommand);
+Opt->setArgStr(NewArg);

Looks like this is a bug in the way sub commands are handled, but I'd like to 
get feedback on how it *should* work.  

The problem is that if an option is added with `cl::sub(*AllSubCommands)` it 
gets added to all registered subcommands, including `TopLevelSubCommand`.  
However, `TopLevelSubCommand` isn't included in `Option::Subs`, so I have to 
update/remove via two commands, one at the parser level for the 
`ChosenSubCommand`, which happens to be `TopLevelSubCommand` in this case, and 
another for the Option itself, which doesn't have `TopLevelSubCommand` in it's 
subs.

Should `CommandLineParser::addOption` specifically exclude `TopLevelSubCommand` 
when it add subs to an Option?  That seems like a bug to me, but I'm not sure I 
grok the reason it was excluded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[clang-tools-extra] r358107 - build: add binary dir to the unittests

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 10:25:14 2019
New Revision: 358107

URL: http://llvm.org/viewvc/llvm-project?rev=358107=rev
Log:
build: add binary dir to the unittests

Missed part of the change to make clangd build on Darwin.  Fixes the build after
SVN r358103.

Modified:
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt

Modified: clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt?rev=358107=358106=358107=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt Wed Apr 10 10:25:14 
2019
@@ -4,8 +4,11 @@ set(LLVM_LINK_COMPONENTS
 
 get_filename_component(CLANGD_SOURCE_DIR
   ${CMAKE_CURRENT_SOURCE_DIR}/../../clangd REALPATH)
+get_filename_component(CLANGD_BINARY_DIR
+  ${CMAKE_CURRENT_BINARY_DIR}/../../clangd REALPATH)
 include_directories(
   ${CLANGD_SOURCE_DIR}
+  ${CLANGD_BINARY_DIR}
   )
 
 add_extra_unittest(ClangdTests


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

[PATCH] D60409: [clangd] Add -header-insertion=never flag to disable include insertion in code completion

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

Our Mac builders have started failing after this change with the following:

  [3145/3502] Building CXX object 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o
  FAILED: 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o 
  /b/s/w/ir/cache/goma/client/gomacc /b/s/w/ir/k/cipd/bin/clang++  
-DCLANG_VENDOR="\"Fuchsia \"" -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/clangd/tool 
-I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool 
-I/b/s/w/ir/k/llvm-project/clang/include -Itools/clang/include 
-I/usr/include/libxml2 -Iinclude -I/b/s/w/ir/k/llvm-project/llvm/include 
-I/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/.. 
-Itools/clang/tools/extra/clangd/tool/.. -fPIC -fvisibility-inlines-hidden 
-Werror=date-time -Werror=unguarded-availability-new -std=c++11 -Wall -Wextra 
-Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers 
-pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default 
-Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor 
-Wstring-conversion -fdiagnostics-color -fno-common -Woverloaded-virtual 
-Wno-nested-anon-types -O3 -gline-tables-only   -UNDEBUG  -fno-exceptions 
-fno-rtti -MD -MT 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o -MF 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o.d -o 
tools/clang/tools/extra/clangd/tool/CMakeFiles/clangd.dir/ClangdMain.cpp.o -c 
/b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp
  /b/s/w/ir/k/llvm-project/clang-tools-extra/clangd/tool/ClangdMain.cpp:474:22: 
error: use of undeclared identifier 'newXPCTransport'
  TransportLayer = newXPCTransport();
   ^
  1 error generated.

I don't understand why since this change hasn't touched the failing line.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D60409



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

[PATCH] D59988: [PR41276] Generate address space cast of 'this' for objects attributed by an address space in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: test/CodeGenCXX/address-space-of-this.cpp:9
+//CHECK: call void @_ZN6MyTypeC1Ei(%struct.MyType* addrspacecast 
(%struct.MyType addrspace(10)* @m to %struct.MyType*), i32 123)
+MyType __attribute__((address_space(10))) m = 123;

Sorry I didn't catch this before, but I don't see why this test is expected to 
work.  We can't actually pass a pointer in address space 10 to that constructor.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59988



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

[PATCH] D60507: [clang-tidy] new check: bugprone-unhandled-self-assignment

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:20
+void UnhandledSelfAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+
+  // We don't care about deleted, default or implicit operator implementations.

Unnecessary empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnhandledSelfAssignmentCheck.cpp:76
+const MatchFinder::MatchResult ) {
+
+  const auto *MatchedDecl =

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:6
+
+Finds user-defined copy assignment operators which do not protect the code
+against self-assignment either by checking self-assignment explicitly or

Please synchronize this statement with Release Notes.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:10-12
+This check corresponds to the CERT C++ Coding Standard rule
+`OOP54-CPP. Gracefully handle self-copy assignment
+`_.

alexfh wrote:
> JonasToth wrote:
> > It is worth an alias into the cert module. Please add this with this 
> > revision already.
> Please add a corresponding alias into the cert module.
See also other aliased checks for documentation examples.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:41
+
+
+There are two common C++ patterns to avoid this problem. The first is

Unnecessary empty line.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/bugprone-unhandled-self-assignment.rst:66
+
+
+The second one is the copy-and-swap method when we create a temporary copy

Unnecessary empty line.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D60507



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

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus added a comment.

We agreed to disagree on the static function stuff -- it doesn't really matter, 
and I don't insist. I have no objections against this patch, great job! I won't 
formally accept to make it stand out a little more. Thanks!


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

https://reviews.llvm.org/D59555



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

[PATCH] D57072: Don't codegen an unreachable return block

[John McCall via Phabricator via cfe-commits]

rjmccall commandeered this revision.
rjmccall edited reviewers, added: bmoody; removed: rjmccall.
rjmccall added a comment.
This revision now requires review to proceed.

Committed as r358104, thanks for your patience.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57072



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

r358104 - Don't emit an unreachable return block.

[John McCall via cfe-commits]

Author: rjmccall
Date: Wed Apr 10 10:03:09 2019
New Revision: 358104

URL: http://llvm.org/viewvc/llvm-project?rev=358104=rev
Log:
Don't emit an unreachable return block.

Patch by Brad Moody.

Added:
cfe/trunk/test/CodeGen/unreachable-ret.c
Modified:
cfe/trunk/lib/CodeGen/CGCall.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGCall.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Wed Apr 10 10:03:09 2019
@@ -2873,15 +2873,6 @@ void CodeGenFunction::EmitFunctionEpilog
 RV = SI->getValueOperand();
 SI->eraseFromParent();
 
-// If that was the only use of the return value, nuke it as well now.
-auto returnValueInst = ReturnValue.getPointer();
-if (returnValueInst->use_empty()) {
-  if (auto alloca = dyn_cast(returnValueInst)) {
-alloca->eraseFromParent();
-ReturnValue = Address::invalid();
-  }
-}
-
   // Otherwise, we have to do a simple load.
   } else {
 RV = Builder.CreateLoad(ReturnValue);

Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Apr 10 10:03:09 2019
@@ -255,6 +255,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
 if (CurBB->empty() || ReturnBlock.getBlock()->use_empty()) {
   ReturnBlock.getBlock()->replaceAllUsesWith(CurBB);
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
 } else
   EmitBlock(ReturnBlock.getBlock());
 return llvm::DebugLoc();
@@ -274,6 +275,7 @@ llvm::DebugLoc CodeGenFunction::EmitRetu
   Builder.SetInsertPoint(BI->getParent());
   BI->eraseFromParent();
   delete ReturnBlock.getBlock();
+  ReturnBlock = JumpDest();
   return Loc;
 }
   }
@@ -448,6 +450,19 @@ void CodeGenFunction::FinishFunction(Sou
   // 5. Width of vector aguments and return types for functions called by this
   //function.
   CurFn->addFnAttr("min-legal-vector-width", llvm::utostr(LargestVectorWidth));
+
+  // If we generated an unreachable return block, delete it now.
+  if (ReturnBlock.isValid() && ReturnBlock.getBlock()->use_empty()) {
+Builder.ClearInsertionPoint();
+ReturnBlock.getBlock()->eraseFromParent();
+  }
+  if (ReturnValue.isValid()) {
+auto *RetAlloca = dyn_cast(ReturnValue.getPointer());
+if (RetAlloca && RetAlloca->use_empty()) {
+  RetAlloca->eraseFromParent();
+  ReturnValue = Address::invalid();
+}
+  }
 }
 
 /// ShouldInstrumentFunction - Return true if the current function should be

Added: cfe/trunk/test/CodeGen/unreachable-ret.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/unreachable-ret.c?rev=358104=auto
==
--- cfe/trunk/test/CodeGen/unreachable-ret.c (added)
+++ cfe/trunk/test/CodeGen/unreachable-ret.c Wed Apr 10 10:03:09 2019
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+extern void abort() __attribute__((noreturn));
+
+void f1() {
+  abort();
+}
+// CHECK-LABEL: define void @f1()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+
+void *f2() {
+  abort();
+  return 0;
+}
+// CHECK-LABEL: define i8* @f2()
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   call void @abort()
+// CHECK-NEXT:   unreachable
+// CHECK-NEXT: }
+

Modified: cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp?rev=358104=358103=358104=diff
==
--- cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp Wed Apr 10 10:03:09 
2019
@@ -622,7 +622,7 @@ int main() {
 
 // CHECK-NOT: call i32 @__kmpc_reduce
 
-// CHECK: ret void
+// CHECK: }
 
 // CHECK: define {{.*}} i{{[0-9]+}} [[TMAIN_INT]]()
 // CHECK: [[TEST:%.+]] = alloca [[S_INT_TY]],


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

[PATCH] D58920: [Modules][PR39287] Consolidate multiple std's

[Andrew Gallagher via Phabricator via cfe-commits]

andrewjcg added a comment.

Sorry for the delay.  Just catching up on the code this covers, so apologies if 
the questions don't make sense.




Comment at: lib/Sema/SemaDeclCXX.cpp:9214-9215
 getStdNamespace()->setImplicit(true);
+if (getLangOpts().Modules)
+  getStdNamespace()->setHasExternalVisibleStorage(true);
   }

I think you mentioned in another forum that one side effect of this change is 
that it's causing multiple candidates for names in `std`.  Is this the implicit 
align constructros/destructors?  Is this because we're marking the implicit 
namespace as being externally visible?



Comment at: lib/Serialization/ASTReader.cpp:9291-9295
 // Load pending declaration chains.
 for (unsigned I = 0; I != PendingDeclChains.size(); ++I)
   loadPendingDeclChain(PendingDeclChains[I].first,
PendingDeclChains[I].second);
 PendingDeclChains.clear();

How does modules normally "connect up" definitions of the same namspace 
occurring in the imported module?  Is it done here (e.g. so that a name lookup 
will search all namespace definitions)?  Is an alternate solution to this diff 
to make sure this handles the `std` implicit special case?  Apologies for the 
naivety here -- still getting up to speed on this code.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58920



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

[clang-tools-extra] r358103 - clangd: fix the build with XPC

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 09:48:52 2019
New Revision: 358103

URL: http://llvm.org/viewvc/llvm-project?rev=358103=rev
Log:
clangd: fix the build with XPC

`Transport.h` does not include `Features.inc`.  However, since it is used in a
subdirectory, it cannot directly include the header as it is not available.
Include `Features.inc` in `ClangdLSPServer.h` prior to the inclusion of
`Transport.h` which will provide the interfaces in `ClangdMain.cpp` where the
symbol `newXPCTransport` will not be defined due to it being preprocessed away
since the configuration is not passed along to the initial inclusion.

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=358103=358102=358103=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Wed Apr 10 09:48:52 2019
@@ -11,6 +11,7 @@
 
 #include "ClangdServer.h"
 #include "DraftStore.h"
+#include "Features.inc"
 #include "FindSymbols.h"
 #include "GlobalCompilationDatabase.h"
 #include "Path.h"


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

[PATCH] D59746: [LibTooling] Fix '-h' option

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 194539.
hintonda added a comment.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

- Add DefaultOption logic.

Still needs tests, but wanted to get early feedback on basic approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746

Files:
  clang/lib/Tooling/CommonOptionsParser.cpp
  llvm/docs/CommandLine.rst
  llvm/include/llvm/Support/CommandLine.h
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -144,6 +144,13 @@
   }
 
   void addOption(Option *O, SubCommand *SC) {
+// Handle DefaultOptions
+if (O->getMiscFlags() & cl::DefaultOption) {
+  std::string DefaultArg(O->ArgStr.str() + ":default_option");
+  SC->DefaultOptions.push_back(DefaultArg);
+  O->setArgStr(SC->DefaultOptions.back());
+}
+
 bool HadErrors = false;
 if (O->hasArgStr()) {
   // Add argument to the argument map!
@@ -1167,6 +1174,22 @@
   auto  = ChosenSubCommand->SinkOpts;
   auto  = ChosenSubCommand->OptionsMap;
 
+  // Handle DefaultOptions.
+  for (auto const  : ChosenSubCommand->DefaultOptions) {
+StringRef Arg(DA);
+StringRef Val;
+if (Option *Opt = LookupOption(*ChosenSubCommand, Arg, Val)) {
+  StringRef NewArg = Arg.substr(0, Arg.find(":"));
+  if (LookupOption(*ChosenSubCommand, NewArg, Val)) {
+removeOption(Opt, ChosenSubCommand);
+  } else {
+// FIXME: This is needed, but not sure why.
+updateArgStr(Opt, NewArg, ChosenSubCommand);
+Opt->setArgStr(NewArg);
+  }
+}
+  }
+
   if (ConsumeAfterOpt) {
 assert(PositionalOpts.size() > 0 &&
"Cannot specify cl::ConsumeAfter without a positional argument!");
@@ -2146,6 +2169,10 @@
 cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
 cl::cat(GenericCategory), cl::sub(*AllSubCommands));
 
+static cl::alias HOpA("h", cl::desc("Alias for -help"), cl::aliasopt(HOp),
+  cl::cat(GenericCategory), cl::sub(*AllSubCommands),
+  cl::DefaultOption);
+
 static cl::opt>
 HHOp("help-hidden", cl::desc("Display all available options"),
  cl::location(WrappedHiddenPrinter), cl::Hidden, cl::ValueDisallowed,
Index: llvm/include/llvm/Support/CommandLine.h
===
--- llvm/include/llvm/Support/CommandLine.h
+++ llvm/include/llvm/Support/CommandLine.h
@@ -175,7 +175,10 @@
   // If this is enabled, multiple letter options are allowed to bunch together
   // with only a single hyphen for the whole group.  This allows emulation
   // of the behavior that ls uses for example: ls -la === ls -l -a
-  Grouping = 0x08
+  Grouping = 0x08,
+
+  // Default option
+  DefaultOption = 0x10
 };
 
 //===--===//
@@ -231,6 +234,7 @@
   SmallVector PositionalOpts;
   SmallVector SinkOpts;
   StringMap OptionsMap;
+  SmallVector DefaultOptions;
 
   Option *ConsumeAfterOpt = nullptr; // The ConsumeAfter option if it exists.
 };
@@ -270,7 +274,7 @@
   unsigned Value : 2;
   unsigned HiddenFlag : 2; // enum OptionHidden
   unsigned Formatting : 2; // enum FormattingFlags
-  unsigned Misc : 4;
+  unsigned Misc : 5;
   unsigned Position = 0;   // Position of last occurrence of the option
   unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.
 
Index: llvm/docs/CommandLine.rst
===
--- llvm/docs/CommandLine.rst
+++ llvm/docs/CommandLine.rst
@@ -128,6 +128,7 @@
   USAGE: compiler [options]
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -194,6 +195,7 @@
   USAGE: compiler [options] 
 
   OPTIONS:
+-h- Alias for -help
 -help - display available options (-help-hidden for more)
 -o  - Specify output filename
 
@@ -1251,6 +1253,14 @@
   with ``cl::CommaSeparated``, this modifier only makes sense with a `cl::list`_
   option.
 
+.. _cl::DefaultOption:
+
+* The **cl::DefaultOption** modifier is used to specify that the option is a
+  default that can be overridden by application specific parsers. For example,
+  the ``-help`` alias, ``-h``, is registered this way, so it can be overridden
+  by applications that need to use the ``-h`` option for another purpose,
+  either as a regular option or an alias for another option.
+
 .. _response files:
 
 Response files
Index: clang/lib/Tooling/CommonOptionsParser.cpp
===
--- clang/lib/Tooling/CommonOptionsParser.cpp
+++ 

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato added a comment.

Oh, and in case I need to mention this (I don't contribute to llvm/clang 
frequently), I don't have commit access so I'll need someone else to merge it 
in. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[John McCall via Phabricator via cfe-commits]

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

LGTM, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[clang-tools-extra] r358098 - [clangd] Fix non-indexing of builtin functions like printf when the TU is C

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 09:26:58 2019
New Revision: 358098

URL: http://llvm.org/viewvc/llvm-project?rev=358098=rev
Log:
[clangd] Fix non-indexing of builtin functions like printf when the TU is C

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

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=358098=358097=358098=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Apr 10 
09:26:58 2019
@@ -236,8 +236,6 @@ bool SymbolCollector::shouldCollectSymbo
   const ASTContext ,
   const Options ,
   bool IsMainFileOnly) {
-  if (ND.isImplicit())
-return false;
   // Skip anonymous declarations, e.g (anonymous enum/class/struct).
   if (ND.getDeclName().isEmpty())
 return false;
@@ -328,7 +326,9 @@ bool SymbolCollector::handleDeclOccurenc
   // then no public declaration was visible, so assume it's main-file only.
   bool IsMainFileOnly =
   SM.isWrittenInMainFile(SM.getExpansionLoc(ND->getBeginLoc()));
-  if (!shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
+  // In C, printf is a redecl of an implicit builtin! So check OrigD instead.
+  if (ASTNode.OrigD->isImplicit() ||
+  !shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
 return true;
   // Do not store references to main-file symbols.
   if (CollectRef && !IsMainFileOnly && !isa(ND) &&

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=358098=358097=358098=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed Apr 
10 09:26:58 2019
@@ -254,9 +254,8 @@ public:
 auto Factory = llvm::make_unique(
 CollectorOpts, PragmaHandler.get());
 
-std::vector Args = {
-"symbol_collector", "-fsyntax-only", "-xc++",
-"-std=c++11",   "-include",  TestHeaderName};
+std::vector Args = {"symbol_collector", "-fsyntax-only",
+ "-xc++", "-include", TestHeaderName};
 Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end());
 // This allows to override the "-xc++" with something else, i.e.
 // -xobjective-c++.
@@ -1165,6 +1164,15 @@ TEST_F(SymbolCollectorTest, UsingDecl) {
   EXPECT_THAT(Symbols, Contains(QName("std::foo")));
 }
 
+TEST_F(SymbolCollectorTest, CBuiltins) {
+  // In C, printf in stdio.h is a redecl of an implicit builtin.
+  const char *Header = R"(
+extern int printf(const char*, ...);
+  )";
+  runSymbolCollector(Header, /**/ "", {"-xc"});
+  EXPECT_THAT(Symbols, Contains(QName("printf")));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang


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

[PATCH] D59371: [LibTooling] Add Stencil library for format-string style codegen.

[Samuel Benzaquen via Phabricator via cfe-commits]

sbenza added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:45
+  /// Evaluates this part to a string and appends it to `result`.
+  virtual llvm::Error eval(const ast_matchers::MatchFinder::MatchResult ,
+   std::string *Result) const = 0;

Why not use `llvm::Expected` as the return type?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:75
+  // Copy constructor/assignment produce a deep copy.
+  StencilPart(const StencilPart ) : Impl(P.Impl->clone()) {}
+  StencilPart(StencilPart &&) = default;

The interface API is all const. Why not keep a `shared_ptr` instead?
That way we don't have to have users implement a clone function.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:94
+  return false;
+return Impl->isEqual(*(Other.Impl));
+  }

Parens around Other.Impl are not necessary.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:110
+  template  static Stencil cat(Ts &&... Parts) {
+Stencil Stencil;
+Stencil.Parts.reserve(sizeof...(Parts));

I don't like naming the variable the same as the type.
You could just use `S` as the variable name. That is ok with llvm style for 
small functions.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:127
+  // Allow Stencils to operate as std::function, for compatibility with
+  // Transformer's TextGenerator. Calls `eval()` and asserts on failure.
+  std::string operator()(const ast_matchers::MatchFinder::MatchResult &) const;

"asserts" as it only fails in debug mode?



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:130
+
+  bool operator==(const Stencil ) const {
+return Parts == Other.Parts;

I usually prefer free functions for binary operators (friends is fine).
It makes them symmetric.



Comment at: clang/include/clang/Tooling/Refactoring/Stencil.h:155
+/// statement.
+StencilPart snode(llvm::StringRef Id);
+

`sNode` ?
ie camelCase



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:45
+
+namespace {
+// An arbitrary fragment of code within a stencil.

maybe this anon namespace should cover the functions above?



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:76
+
+static bool operator==(const RawTextData , const RawTextData ) {
+  return A.Text == B.Text;

If you define ==, imo you should define != too.
Otherwise just call it `isEqual`



Comment at: clang/lib/Tooling/Refactoring/Stencil.cpp:94
+Error evalData(const T , const MatchFinder::MatchResult ,
+   std::string *Result);
+

alignment.
(below too)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59371



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato marked 2 inline comments as done.
allevato added a comment.

Thanks for the review! This is ready to go on my end if there aren't any other 
comments.




Comment at: clang/lib/CodeGen/CGNonTrivialStruct.cpp:845
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,

ahatanak wrote:
> I think `getFunction` shouldn't require passing addresses, but that's not 
> this patch's fault. I'll see if I can remove the parameter. 
Yeah, this hack bummed me out and I tried cleaning up the other related 
functions to have them not reuse the array in this way, but the way std::array 
and the template arg size_t N are currently being used throughout made those 
attempts ripple through in ways that would have required a deeper refactor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161



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

[PATCH] D60161: Expose non-trivial struct helpers for Swift.

[Tony Allevato via Phabricator via cfe-commits]

allevato updated this revision to Diff 194530.
allevato added a comment.

- Rename generate* to get* and cleanup param names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60161

Files:
  clang/include/clang/CodeGen/CodeGenABITypes.h
  clang/lib/CodeGen/CGNonTrivialStruct.cpp

Index: clang/lib/CodeGen/CGNonTrivialStruct.cpp
===
--- clang/lib/CodeGen/CGNonTrivialStruct.cpp
+++ clang/lib/CodeGen/CGNonTrivialStruct.cpp
@@ -14,6 +14,7 @@
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "clang/AST/NonTrivialTypeVisitor.h"
+#include "clang/CodeGen/CodeGenABITypes.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include 
 
@@ -822,6 +823,29 @@
   Gen.callFunc(FuncName, QT, Addrs, CGF);
 }
 
+template  std::array createNullAddressArray();
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero())});
+}
+
+template <> std::array createNullAddressArray() {
+  return std::array({Address(nullptr, CharUnits::Zero()),
+ Address(nullptr, CharUnits::Zero())});
+}
+
+template 
+static llvm::Function *
+getSpecialFunction(G &, StringRef FuncName, QualType QT, bool IsVolatile,
+   std::array Alignments, CodeGenModule ) {
+  QT = IsVolatile ? QT.withVolatile() : QT;
+  // The following call requires an array of addresses as arguments, but doesn't
+  // actually use them (it overwrites them with the addresses of the arguments
+  // of the created function).
+  return Gen.getFunction(FuncName, QT, createNullAddressArray(), Alignments,
+ CGM);
+}
+
 // Functions to emit calls to the special functions of a non-trivial C struct.
 void CodeGenFunction::callCStructDefaultConstructor(LValue Dst) {
   bool IsVolatile = Dst.isVolatile();
@@ -907,3 +931,69 @@
   callSpecialFunction(GenMoveAssignment(getContext()), FuncName, QT, IsVolatile,
   *this, std::array({{DstPtr, SrcPtr}}));
 }
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructDefaultConstructor(
+CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenDefaultInitializeFuncName GenName(DstAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(GenDefaultInitialize(Ctx), FuncName, QT, IsVolatile,
+std::array({{DstAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructCopyConstructor(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_constructor_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenCopyConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructMoveConstructor(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__move_constructor_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenMoveConstructor(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructCopyAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__copy_assignment_", DstAlignment,
+   SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenCopyAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructMoveAssignmentOperator(
+CodeGenModule , CharUnits DstAlignment, CharUnits SrcAlignment,
+bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenBinaryFuncName GenName("__move_assignment_", DstAlignment,
+  SrcAlignment, Ctx);
+  std::string FuncName = GenName.getName(QT, IsVolatile);
+  return getSpecialFunction(
+  GenMoveAssignment(Ctx), FuncName, QT, IsVolatile,
+  std::array({{DstAlignment, SrcAlignment}}), CGM);
+}
+
+llvm::Function *clang::CodeGen::getNonTrivialCStructDestructor(
+CodeGenModule , CharUnits DstAlignment, bool IsVolatile, QualType QT) {
+  ASTContext  = CGM.getContext();
+  GenDestructorFuncName GenName("__destructor_", DstAlignment, Ctx);
+  std::string 

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 194528.
kadircet marked 10 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -45,6 +46,17 @@
   return arg.Range == Range && arg.Message == Message;
 }
 
+MATCHER_P3(Diag, Range, Message, IncludeStack,
+   "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
+  if (arg.Range != Range || arg.Message != Message ||
+  arg.IncludeStack.size() != IncludeStack.size())
+return false;
+  for (size_t I = 0, E = IncludeStack.size(); I < E; ++I)
+if (IncludeStack[I] != arg.IncludeStack[I])
+  return false;
+  return true;
+}
+
 MATCHER_P3(Fix, Range, Replacement, Message,
"Fix " + llvm::to_string(Range) + " => " +
testing::PrintToString(Replacement) + " = [" + Message + "]") {
@@ -73,7 +85,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -251,6 +262,8 @@
   D.InsideMainFile = true;
   D.Severity = DiagnosticsEngine::Error;
   D.File = "foo/bar/main.cpp";
+  D.IncludeStack.push_back("a/b.h:1:2");
+  D.IncludeStack.push_back("a/c.h:2:2");
 
   clangd::Note NoteInMain;
   NoteInMain.Message = "declared somewhere in the main file";
@@ -282,7 +295,8 @@
 
   // Diagnostics should turn into these:
   clangd::Diagnostic MainLSP =
-  MatchingLSP(D, R"(Something terrible happened (fix available)
+  MatchingLSP(D, R"(In file included from: a/b.h:1:2: 
+a/c.h:2:2: something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
@@ -603,7 +617,185 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST build(const std::string ,
+const llvm::StringMap ) {
+  std::vector Cmd = {"clang", MainFile.c_str()};
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = MainFile;
+  Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.Contents = Files.lookup(MainFile);
+  Inputs.FS = buildTestFS(Files);
+  Inputs.Opts = ParseOptions();
+  auto PCHs = std::make_shared();
+  auto CI = buildCompilerInvocation(Inputs);
+  assert(CI && "Failed to build compilation invocation.");
+  auto Preamble =
+  buildPreamble(MainFile, *CI,
+/*OldPreamble=*/nullptr,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs,
+/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+  Preamble);
+  if (!AST.hasValue()) {
+ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+llvm_unreachable("Failed to build TestTU!");
+  }
+  return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector{"/clangd-test/a.h:1:1"})));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "#include \"b.h\""},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "C++ requires a type specifier for all declarations",
+   std::vector{"/clangd-test/a.h:1:10",
+"/clangd-test/b.h:1:1"})));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+  Annotations Main(R"cpp(
+#include $a[["a.h"]]
+#include $b[["b.h"]]
+void foo() {})cpp");
+  std::string 

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/Diagnostics.cpp:396
+  for (llvm::StringRef Inc : IncludeStack)
+OS << "In file included from: " << Inc << ":\n";
+}

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > NIT: could we reuse the function from clang that does the same thing?
> > > 
> > > The code here is pretty simple, though, so writing it here is fine. Just 
> > > wanted to make sure we considered this option and found it's easier to 
> > > redo this work ourselves.
> > there is `TextDiagnostic` which prints the desired output expect the fact 
> > that it also prints the first line saying the header was included in main 
> > file. Therefore, I didn't make use of it since we decided that was not very 
> > useful for our use case. And it requires inheriting from 
> > `DiagnosticRenderer` which was requiring too much boiler plate code just to 
> > get this functionality so instead I've chosen doing it like this.
> > 
> > Can fallback to `TextDiagnostic` if you believe showing `Included in 
> > mainfile.cc:x:y:` at the beginning of the diagnostic won't be annoying.
> LG, it's not too hard to reconstruct the same output.
> Note that D60267 starts outputting notes in a structured way, using the 
> `RelatedInformation` struct from the LSP.
> 
> We should eventually add the include stack into related information too. With 
> that in mind, we should probably add the include stack as a new field to the 
> struct we use for diagnostics.
> 
SG, delaying serialization to LSP layer then.



Comment at: clangd/Diagnostics.cpp:372
+auto IncludeLocation = Info.getSourceManager()
+   .getPresumedLoc(Info.getLocation(),
+   /*UseLineDirectives=*/false)

ilya-biryukov wrote:
> Why use `getPresumedLoc`? Making clangd sensitive to pp directives that 
> rename file or change line numbers does not make any sense.
> 
> Could you also add tests for that?
It was the way `DiagnosticRenderer` emitted include stacks, I had just copied 
it over.
Changing with `SourceManger::getIncludeLoc`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[clang-tools-extra] r358094 - [clangd] Use #if CLANGD_BUILD_XPC because it may be defined as 0

[Fangrui Song via cfe-commits]

Author: maskray
Date: Wed Apr 10 08:45:54 2019
New Revision: 358094

URL: http://llvm.org/viewvc/llvm-project?rev=358094=rev
Log:
[clangd] Use #if CLANGD_BUILD_XPC because it may be defined as 0

Modified:
clang-tools-extra/trunk/clangd/Transport.h

Modified: clang-tools-extra/trunk/clangd/Transport.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Transport.h?rev=358094=358093=358094=diff
==
--- clang-tools-extra/trunk/clangd/Transport.h (original)
+++ clang-tools-extra/trunk/clangd/Transport.h Wed Apr 10 08:45:54 2019
@@ -85,7 +85,7 @@ newJSONTransport(std::FILE *In, llvm::ra
  llvm::raw_ostream *InMirror, bool Pretty,
  JSONStreamStyle = JSONStreamStyle::Standard);
 
-#ifdef CLANGD_BUILD_XPC
+#if CLANGD_BUILD_XPC
 // Returns a Transport for macOS based on XPC.
 // Clangd with this transport is meant to be run as bundled XPC service.
 std::unique_ptr newXPCTransport();


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

[clang-tools-extra] r358093 - clangd: repair the build after SVN r358091

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Wed Apr 10 08:45:05 2019
New Revision: 358093

URL: http://llvm.org/viewvc/llvm-project?rev=358093=rev
Log:
clangd: repair the build after SVN r358091

Fix the name of the variable being checked.  NFCI.

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358093=358092=358093=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 08:45:05 2019
@@ -581,7 +581,7 @@ getQueryScopes(CodeCompletionContext 
 return {EnclosingAtFront, Opts.AllScopes};
   }
   // Case 3: sema saw and resolved a scope qualifier.
-  if (Specifier && SemaSpecifier->isValid())
+  if (SemaSpecifier && SemaSpecifier->isValid())
 return {Scopes.scopesForIndexQuery(), false};
 
   // Case 4: There was a qualifier, and Sema didn't resolve it.


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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping @a_sidorin @shafik


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 194527.
martong marked an inline comment as done.
martong added a comment.

- Put back the call to GetOriginalDecl


Repository:
  rC Clang

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

https://reviews.llvm.org/D59692

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Analysis/Inputs/ctu-other.c
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1982,7 +1982,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2692,6 +2692,64 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5552,6 +5610,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
 DefaultTestValuesForRunOptions, );
 
Index: test/Analysis/Inputs/ctu-other.c
===
--- test/Analysis/Inputs/ctu-other.c
+++ test/Analysis/Inputs/ctu-other.c
@@ -12,11 +12,11 @@
 }
 
 // Test enums.
-enum B { x = 42,
- l,
- s };
+enum B { x2 = 42,
+ y2,
+ z2 };
 int enumCheck(void) {
-  return x;
+  return x2;
 }
 
 // Test reporting an error in macro definition
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -79,6 +79,7 @@
   using ExpectedExpr = llvm::Expected;
   using ExpectedDecl = llvm::Expected;
   using ExpectedSLoc = llvm::Expected;
+  using ExpectedName = llvm::Expected;
 
   std::string ImportError::toString() const {
 // FIXME: Improve error texts.
@@ -2135,11 +2136,11 @@
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
- ConflictingDecls.data(),
- ConflictingDecls.size());
-  if (!Name)
-return make_error(ImportError::NameConflict);
+  ExpectedName NameOrErr = Importer.HandleNameConflict(
+  Name, DC, Decl::IDNS_Namespace, ConflictingDecls.data(),
+  ConflictingDecls.size());
+  if (!NameOrErr)
+return NameOrErr.takeError();
 }
   }
 
@@ -2243,20 +2244,17 @@
   // already have a complete underlying type then return with that.
   if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
 return Importer.MapImported(D, FoundTypedef);
+} else {
+  ConflictingDecls.push_back(FoundDecl);
 }
-// FIXME Handle redecl chain.
-break;
   }
-
-  ConflictingDecls.push_back(FoundDecl);
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = 

[PATCH] D59746: [LibTooling] Fix '-h' option

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Just chiming in from a LLVM binutils developer perspective. Some of the 
binutils are missing -h as an alias, when they really should have it for 
compatibility (specifically I checked llvm-nm and llvm-symbolizer). As a 
result, a solution that auto-adds -h as an alias would be good, as long as we 
have the ability to override it somehow. I wouldn't mind having logic in 
llvm-objdump/llvm-readobj to explicitly override the short alias if it is 
required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59746



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

[PATCH] D60516: [LTO] Add plumbing to save stats during LTO on MacOS.

[Florian Hahn via Phabricator via cfe-commits]

fhahn created this revision.
fhahn added reviewers: anemet, tejohnson, thegameg.
Herald added subscribers: cfe-commits, dang, dexonsmith, steven_wu, hiraditya, 
inglorion, mehdi_amini.
Herald added projects: clang, LLVM.

Gold and ld on Linux already support saving stats, but the
infrastructure is missing on MacOS. Unfortunately it seems like the
configuration from lib/LTO/LTO.cpp is not used.

This patch adds a new LTOStatsFile option and adds plumbing in Clang to
use it on MacOS, similar to the way remarks are handled.

Currnetly the handling of LTO flags seems quite spread out, with a bunch
of duplication. But I am not sure if there is an easy way to improve
that?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D60516

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  llvm/include/llvm/LTO/LTO.h
  llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp

Index: llvm/lib/LTO/LTOCodeGenerator.cpp
===
--- llvm/lib/LTO/LTOCodeGenerator.cpp
+++ llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -95,6 +95,11 @@
 "lto-pass-remarks-with-hotness",
 cl::desc("With PGO, include profile count in optimization remarks"),
 cl::Hidden);
+
+cl::opt LTOStatsFile(
+"lto-stats-file",
+cl::desc("With PGO, include profile count in optimization remarks"),
+cl::Hidden);
 }
 
 LTOCodeGenerator::LTOCodeGenerator(LLVMContext )
@@ -518,6 +523,14 @@
   }
   DiagnosticOutputFile = std::move(*DiagFileOrErr);
 
+  // Setup output file to emit statistics.
+  auto StatsFileOrErr = lto::setupStatsFile(LTOStatsFile);
+  if (!StatsFileOrErr) {
+errs() << "Error: " << toString(StatsFileOrErr.takeError()) << "\n";
+report_fatal_error("Can't get an output file for the statistics");
+  }
+  StatsFile = std::move(StatsFileOrErr.get());
+
   // We always run the verifier once on the merged module, the `DisableVerify`
   // parameter only applies to subsequent verify.
   verifyMergedModuleOnce();
@@ -585,8 +598,12 @@
   ShouldRestoreGlobalsLinkage);
 
   // If statistics were requested, print them out after codegen.
-  if (llvm::AreStatisticsEnabled())
+  if (llvm::AreStatisticsEnabled() && !StatsFile)
 llvm::PrintStatistics();
+
+  if (StatsFile)
+PrintStatisticsJSON(StatsFile->os());
+
   reportAndResetTimings();
 
   finishOptimizationRemarks();
Index: llvm/lib/LTO/LTO.cpp
===
--- llvm/lib/LTO/LTO.cpp
+++ llvm/lib/LTO/LTO.cpp
@@ -880,16 +880,10 @@
   isPrevailing, Conf.OptLevel > 0);
 
   // Setup output file to emit statistics.
-  std::unique_ptr StatsFile = nullptr;
-  if (!Conf.StatsFile.empty()) {
-EnableStatistics(false);
-std::error_code EC;
-StatsFile =
-llvm::make_unique(Conf.StatsFile, EC, sys::fs::F_None);
-if (EC)
-  return errorCodeToError(EC);
-StatsFile->keep();
-  }
+  auto StatsFileOrErr = setupStatsFile(Conf.StatsFile);
+  if (!StatsFileOrErr)
+return StatsFileOrErr.takeError();
+  std::unique_ptr StatsFile = std::move(StatsFileOrErr.get());
 
   // Finalize linking of regular LTO modules containing summaries now that
   // we have computed liveness information.
@@ -1338,3 +1332,20 @@
   DiagnosticFile->keep();
   return std::move(DiagnosticFile);
 }
+
+Expected>
+lto::setupStatsFile(StringRef StatsFilename) {
+  // Setup output file to emit statistics.
+  if (StatsFilename.empty())
+return nullptr;
+
+  llvm::EnableStatistics(false);
+  std::error_code EC;
+  auto StatsFile =
+  llvm::make_unique(StatsFilename, EC, sys::fs::F_None);
+  if (EC)
+return errorCodeToError(EC);
+
+  StatsFile->keep();
+  return std::move(StatsFile);
+}
Index: llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
===
--- llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
+++ llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h
@@ -241,6 +241,7 @@
   TargetMachine::CodeGenFileType FileType = TargetMachine::CGFT_ObjectFile;
   std::unique_ptr DiagnosticOutputFile;
   bool Freestanding = false;
+  std::unique_ptr StatsFile = nullptr;
 };
 }
 #endif
Index: llvm/include/llvm/LTO/LTO.h
===
--- llvm/include/llvm/LTO/LTO.h
+++ llvm/include/llvm/LTO/LTO.h
@@ -87,6 +87,10 @@
  StringRef LTORemarksPasses,
  bool LTOPassRemarksWithHotness, int Count = -1);
 
+/// Setups the output file for saving statistics.
+Expected>
+setupStatsFile(StringRef StatsFilename);
+
 class LTO;
 struct SymbolResolution;
 class ThinBackendProc;
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -514,6 +514,14 @@
 }
   }
 
+  // Setup statistics 

[PATCH] D60503: [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL358091: [clangd] Dont insert extra namespace 
qualifiers when Sema gets lost. (authored by sammccall, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D60503?vs=194486=194520#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D60503

Files:
  clang-tools-extra/trunk/clangd/CodeComplete.cpp
  clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
@@ -2384,19 +2384,19 @@
   UnorderedElementsAre(AllOf(Qualifier(""), Named("XYZ";
 }
 
-// Regression test: clang parser gets confused here and doesn't report the ns::
-// prefix - naive behavior is to insert it again.
-// However we can recognize this from the source code.
-// Test disabled until we can make it pass.
-TEST(CompletionTest, DISABLED_NamespaceDoubleInsertion) {
+// Clang parser gets confused here and doesn't report the ns:: prefix.
+// Naive behavior is to insert it again. We examine the source and recover.
+TEST(CompletionTest, NamespaceDoubleInsertion) {
   clangd::CodeCompleteOptions Opts = {};
 
   auto Results = completions(R"cpp(
+namespace foo {
 namespace ns {}
 #define M(X) < X
 M(ns::ABC^
+}
   )cpp",
- {cls("ns::ABCDE")}, Opts);
+ {cls("foo::ns::ABCDE")}, Opts);
   EXPECT_THAT(Results.Completions,
   UnorderedElementsAre(AllOf(Qualifier(""), Named("ABCDE";
 }
Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp
===
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -543,54 +544,59 @@
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext , const Sema ,
+   const CompletionPrefix ,
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
+
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
 }
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
-
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto  : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto  : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope qualifier.
-return {Scopes, Opts.AllScopes};
-  }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return 

[clang-tools-extra] r358091 - [clangd] Don't insert extra namespace qualifiers when Sema gets lost.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Apr 10 08:16:54 2019
New Revision: 358091

URL: http://llvm.org/viewvc/llvm-project?rev=358091=rev
Log:
[clangd] Don't insert extra namespace qualifiers when Sema gets lost.

Summary:
There are cases where Sema can't tell that "foo" in foo::Bar is a
namespace qualifier, like in incomplete macro expansions.

After this patch, if sema reports no specifier but it looks like there's one in
the source code, then we take it into account.

Reworked structure and comments in getQueryScopes to try to fight
creeping complexity - unsure if I succeeded.

I made the test harder (the original version also passes).

Reviewers: ioeric

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=358091=358090=358091=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Wed Apr 10 08:16:54 2019
@@ -45,6 +45,7 @@
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/CodeCompleteConsumer.h"
+#include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/None.h"
@@ -543,54 +544,59 @@ struct SpecifiedScope {
 // (e.g. enclosing namespace).
 std::pair, bool>
 getQueryScopes(CodeCompletionContext , const Sema ,
+   const CompletionPrefix ,
const CodeCompleteOptions ) {
-  auto GetAllAccessibleScopes = [](CodeCompletionContext ) {
-SpecifiedScope Info;
-for (auto *Context : CCContext.getVisitedContexts()) {
-  if (isa(Context))
-Info.AccessibleScopes.push_back(""); // global namespace
-  else if (isa(Context))
-Info.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  SpecifiedScope Scopes;
+  for (auto *Context : CCContext.getVisitedContexts()) {
+if (isa(Context))
+  Scopes.AccessibleScopes.push_back(""); // global namespace
+else if (isa(Context))
+  Scopes.AccessibleScopes.push_back(printNamespaceScope(*Context));
+  }
+
+  const CXXScopeSpec *SemaSpecifier =
+  CCContext.getCXXScopeSpecifier().getValueOr(nullptr);
+  // Case 1: unqualified completion.
+  if (!SemaSpecifier) {
+// Case 2 (exception): sema saw no qualifier, but there appears to be one!
+// This can happen e.g. in incomplete macro expansions. Use heuristics.
+if (!HeuristicPrefix.Qualifier.empty()) {
+  vlog("Sema said no scope specifier, but we saw {0} in the source code",
+   HeuristicPrefix.Qualifier);
+  StringRef SpelledSpecifier = HeuristicPrefix.Qualifier;
+  if (SpelledSpecifier.consume_front("::"))
+Scopes.AccessibleScopes = {""};
+  Scopes.UnresolvedQualifier = SpelledSpecifier;
+  return {Scopes.scopesForIndexQuery(), false};
 }
-return Info;
-  };
-
-  auto SS = CCContext.getCXXScopeSpecifier();
-
-  // Unqualified completion (e.g. "vec^").
-  if (!SS) {
-std::vector Scopes;
+// The enclosing namespace must be first, it gets a quality boost.
+std::vector EnclosingAtFront;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
-Scopes.push_back(EnclosingScope);
-for (auto  : GetAllAccessibleScopes(CCContext).scopesForIndexQuery()) {
+EnclosingAtFront.push_back(EnclosingScope);
+for (auto  : Scopes.scopesForIndexQuery()) {
   if (EnclosingScope != S)
-Scopes.push_back(std::move(S));
+EnclosingAtFront.push_back(std::move(S));
 }
-// Allow AllScopes completion only for there is no explicit scope 
qualifier.
-return {Scopes, Opts.AllScopes};
-  }
-
-  // Qualified completion ("std::vec^"), we have two cases depending on whether
-  // the qualifier can be resolved by Sema.
-  if ((*SS)->isValid()) { // Resolved qualifier.
-return {GetAllAccessibleScopes(CCContext).scopesForIndexQuery(), false};
+// Allow AllScopes completion as there is no explicit scope qualifier.
+return {EnclosingAtFront, Opts.AllScopes};
   }
-
-  // Unresolved qualifier.
-  SpecifiedScope Info = GetAllAccessibleScopes(CCContext);
-  Info.AccessibleScopes.push_back(""); // Make sure global scope is included.
-
-  llvm::StringRef SpelledSpecifier =
-  Lexer::getSourceText(CharSourceRange::getCharRange((*SS)->getRange()),
-   CCSema.SourceMgr, clang::LangOptions());
+  // Case 3: sema saw and resolved a scope qualifier.
+  if (Specifier && SemaSpecifier->isValid())
+return {Scopes.scopesForIndexQuery(), false};
+
+  // Case 4: There was a qualifier, and Sema didn't resolve it.
+  

[PATCH] D60513: [HIP] Use -mlink-builtin-bitcode to link device library

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added reviewers: tra, ashi1.
Herald added a subscriber: jdoerfert.

Use -mlink-builtin-bitcode instead of llvm-link to link
device library so that device library bitcode and user
device code can be compiled in a consistent way.

This is the same approach used by CUDA and OpenMP.


https://reviews.llvm.org/D60513

Files:
  lib/Driver/ToolChains/HIP.cpp
  test/Driver/hip-device-libs.hip
  test/Driver/hip-toolchain-no-rdc.hip
  test/Driver/hip-toolchain-rdc.hip

Index: test/Driver/hip-toolchain-rdc.hip
===
--- test/Driver/hip-toolchain-rdc.hip
+++ test/Driver/hip-toolchain-rdc.hip
@@ -18,6 +18,7 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
@@ -27,11 +28,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
@@ -49,18 +50,21 @@
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
 // CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx900"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fgpu-rdc"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC]]
 
 // CHECK: [[LLVM_LINK]] [[A_BC]] [[B_BC]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
Index: test/Driver/hip-toolchain-no-rdc.hip
===
--- test/Driver/hip-toolchain-no-rdc.hip
+++ test/Driver/hip-toolchain-no-rdc.hip
@@ -22,11 +22,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC_803:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_803]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_A_803:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_803]] "-mtriple=amdgcn-amd-amdhsa"
@@ -50,11 +50,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[A_BC_900:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC_900]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_A_900:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_A_900]] "-mtriple=amdgcn-amd-amdhsa"
@@ -94,11 +94,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
 // CHECK-SAME: "-fapply-global-visibility-to-externs"
+// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: {{.*}} "-o" [[B_BC_803:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[B_BC_803]]
-// CHECK-SAME: "{{.*}}lib1.bc" "{{.*}}lib2.bc"
 // CHECK-SAME: "-o" [[LINKED_BC_DEV_B_803:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV_B_803]] "-mtriple=amdgcn-amd-amdhsa"
@@ -122,11 +122,11 @@
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: "-fcuda-is-device" 

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

In D60472#1461351 , @peter.smith wrote:

>




> Is that not a limitation of the build system? I'd expect a package to be able 
> to locally override a global default rather than vice-versa. Although crypto 
> might be the area of concern here and there may be a conveniently named 
> option for PPC, where does this stop? Crypto is not the only architectural 
> extension, for example see 
> https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a 
> consistent interface we'd need command line options for all the extensions. 
> May I encourage you to reply to the RFC on command line options that I 
> mentioned earlier if it doesn't work for you? I think the extensions need to 
> be considered as a whole and not just individually.

I'll also read the RFC and respond.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

In D60472#1461351 , @peter.smith wrote:

>




> Is that not a limitation of the build system? I'd expect a package to be able 
> to locally override a global default rather than vice-versa. Although crypto 
> might be the area of concern here and there may be a conveniently named 
> option for PPC, where does this stop? Crypto is not the only architectural 
> extension, for example see 
> https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To maintain a 
> consistent interface we'd need command line options for all the extensions. 
> May I encourage you to reply to the RFC on command line options that I 
> mentioned earlier if it doesn't work for you? I think the extensions need to 
> be considered as a whole and not just individually.

While it partly is a build system issue, another problem is enabling crypto via 
"-march" requires picking an architecture as well. So even if it could override 
the global default, it would also override the global "-march" as well. If the 
global "-march" was a better/higher option, it does not make sense to override 
it e.g. "-march=armv8-a+crypto" overriding "-march=armv8.3-a" is not helpful 
since it will disable 8.3 features.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

In D60472#1461336 , @manojgupta wrote:

> The motivation for this change is to make "crypto" setting an additive option 
> e.g. like "-mavx" used in many media packages.  Some packages in Chrome want 
> to enable crypto conditionally for a few files to allow crypto feature to be 
> used based on runtime cpu detection. They set "-march=armv8+crypto" flag but 
> it gets overridden by the global "-march=armv8a" flag set by the build system 
> in Chrome OS because the target cpu does not support crypto causing 
> compile-time errors. 
>  Ability to specify "-mcrypto"  standalone makes it an additive option and 
> ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" 
> from "-march" so that they could be set independently. The current additive 
> alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.


Is that not a limitation of the build system? I'd expect a package to be able 
to locally override a global default rather than vice-versa. Although crypto 
might be the area of concern here and there may be a conveniently named option 
for PPC, where does this stop? Crypto is not the only architectural extension, 
for example see https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html . To 
maintain a consistent interface we'd need command line options for all the 
extensions. May I encourage you to reply to the RFC on command line options 
that I mentioned earlier if it doesn't work for you? I think the extensions 
need to be considered as a whole and not just individually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

The motivation for this change is to make "crypto" setting an additive option 
e.g. like "-mavx" used in many media packages.  Some packages in Chrome want to 
enable crypto conditionally for a few files to allow crypto feature to be used 
based on runtime cpu detection. They set "-march=armv8+crypto" flag but it gets 
overridden by the global "-march=armv8a" flag set by the build system in Chrome 
OS because the target cpu does not support crypto causing compile-time errors. 
Ability to specify "-mcrypto"  standalone makes it an additive option and 
ensures that it it is not lost. i.e. this will help in decoupling  "-mcrypto" 
from "-march" so that they could be set independently. The current additive 
alternate is  '-Xclang -target-feature -Xclang "+crypto" ' which is ugly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

r358087 - clang-cl: Fix parsing of the /F option (PR41405)

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Apr 10 07:27:47 2019
New Revision: 358087

URL: http://llvm.org/viewvc/llvm-project?rev=358087=rev
Log:
clang-cl: Fix parsing of the /F option (PR41405)

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

Modified: cfe/trunk/include/clang/Driver/CLCompatOptions.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/CLCompatOptions.td?rev=358087=358086=358087=diff
==
--- cfe/trunk/include/clang/Driver/CLCompatOptions.td (original)
+++ cfe/trunk/include/clang/Driver/CLCompatOptions.td Wed Apr 10 07:27:47 2019
@@ -404,7 +404,7 @@ def _SLASH_d2 : CLJoined<"d2">;
 def _SLASH_doc : CLJoined<"doc">;
 def _SLASH_FA_joined : CLJoined<"FA">;
 def _SLASH_favor : CLJoined<"favor">;
-def _SLASH_F : CLFlag<"F">;
+def _SLASH_F : CLJoinedOrSeparate<"F">;
 def _SLASH_Fm : CLJoined<"Fm">;
 def _SLASH_Fr : CLJoined<"Fr">;
 def _SLASH_FR : CLJoined<"FR">;

Modified: cfe/trunk/test/Driver/cl-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/cl-options.c?rev=358087=358086=358087=diff
==
--- cfe/trunk/test/Driver/cl-options.c (original)
+++ cfe/trunk/test/Driver/cl-options.c Wed Apr 10 07:27:47 2019
@@ -400,7 +400,7 @@
 // RUN: /d2FH4 \
 // RUN: /docname \
 // RUN: /EHsc \
-// RUN: /F \
+// RUN: /F 42 \
 // RUN: /FA \
 // RUN: /FAc \
 // RUN: /Fafilename \


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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I'm not in favour of adding AArch64 support to -mcrypto -mnocrypto for a few 
reasons:

- Arm are trying to keep the options for controlling target features as 
consistent as possible with GCC and this option isn't supported in GCC 
(https://gcc.gnu.org/onlinedocs/gcc/AArch64-Options.html)
- There is http://lists.llvm.org/pipermail/llvm-dev/2018-September/126346.html 
which is Arm's proposal for how target options can be better supported in 
Clang. I think that supporting -mcrypto as well would add more complexity as 
there are interactions between the options.
- Arm 32-bit also supports crypto so this would need to be added to Arm as well 
for consistency.

Can you expand on why you need -m[no]crypto?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60472: [AArch64][PowerPC][Driver] Allow setting crypto feature through -mcrypto for ARM/AArch64

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

What's the motivation for this change, are you working towards common flags for 
both platforms? The current way to select crypto on AArch64 is 
'-march=armv8.x-a+crypto/nocrypto'. I can see that would be an issue if Power 
PC doesn't support that syntax, or doesn't have a specific crypto extension.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60472



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added a comment.

I'd be happy to land it, but I do want @rsmith to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60055: Check i < FD->getNumParams() before querying

[Violet via Phabricator via cfe-commits]

Violet added a comment.

Thanks. Can you land it for me? I'm a newcommer without landing privilege.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60055



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

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL updated this revision to Diff 194505.
DennisL marked an inline comment as done.
DennisL added a comment.

Changed the following

- addressed reviewer feedback
- fetch the placement parameter as written
- add further test cases as requested
- stylistic changes
- add nothrow parameter support
- ignore matches with unresolved template parameters
- add examples of bad and good code


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

https://reviews.llvm.org/D60139

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp
  clang-tidy/bugprone/PlacementNewTargetTypeMismatch.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp

Index: test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-placement-new-target-type-mismatch.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s bugprone-placement-new-target-type-mismatch %t
+
+// definitions
+	
+namespace std {
+struct nothrow_t { explicit nothrow_t() = default; } nothrow;
+template T* addressof(T& arg) noexcept;
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference  {typedef T type;};
+template< class T > struct remove_reference {typedef T type;};
+template< class T >
+T&& forward( typename std::remove_reference::type& t ) noexcept;
+} // namespace std
+
+using size_type = unsigned long;
+void *operator new(size_type, void *);
+void *operator new[](size_type, void *);
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new(size_type size, const std::nothrow_t&) noexcept;
+void* operator new[](size_type size, const std::nothrow_t&) noexcept;
+
+struct Foo {
+  int a;
+  int b;
+  int c;
+  int d;
+};
+
+template
+T& getT() {
+  static T f;
+  return f;
+}
+
+// instances emitting warnings
+
+void f1() {
+  struct Dummy {
+int a;
+int b;
+  };
+  int *ptr = new int;
+  new (ptr) Dummy;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f2() {
+  int * ptr = new int;
+  new (ptr) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f3() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float[13];
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f4() {
+  new (std::addressof(getT())) Foo;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f5() {
+  char *ptr = new char[17*sizeof(char)];
+  new (ptr) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+void f6() {
+  char array[17*sizeof(char)];
+  new (array) float{13.f};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: placement new parameter and allocated type mismatch [bugprone-placement-new-target-type-mismatch]
+}
+
+// instances not emitting a warning
+
+void g1() {
+  Foo * ptr = new Foo;
+  new (ptr) Foo;
+}
+
+void g2() {
+  char *ptr = new char[17*sizeof(char)];
+  new ((float *)ptr) float{13.f};
+}
+
+void g3() {
+  char array[17*sizeof(char)];
+  new (array) char('A');
+}
+
+void g4() {
+  new ((void *)std::addressof(getT())) Foo;
+}
+
+union
+{
+  char * buffer;
+} Union;
+
+template 
+void g5(U &&... V) {
+  new (static_cast(Union.buffer)) T(std::forward(V)...);
+}
+
+template 
+void g6(U &&... V) {
+  new (std::nothrow) T(std::forward(V)...);
+}
+
+template  void g7(U &&... Us) {
+  new (static_cast(Union.buffer)) T(std::forward(Us)...);
+}
+
+template  void g8(U &&... Us) {
+  new (Union.buffer) T(std::forward(Us)...);
+}
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -55,6 +55,7 @@
bugprone-move-forwarding-reference
bugprone-multiple-statement-macro
bugprone-parent-virtual-call
+   bugprone-placement-new-target-type-mismatch
bugprone-sizeof-container
bugprone-sizeof-expression
bugprone-string-constructor
Index: docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/bugprone-placement-new-target-type-mismatch.rst
@@ -0,0 +1,23 @@
+.. title:: clang-tidy - misc-placement-new-target-type-mismatch
+

[PATCH] D60139: [clang-tidy] Add bugprone-placement-new-target-type-mismatch check

[Dennis Luxen via Phabricator via cfe-commits]

DennisL marked 12 inline comments as done.
DennisL added a comment.

In D60139#1460233 , @JonasToth wrote:

> Hey Dennis,
>
> my 2cents on the check. I think it is very good to have! Did you check coding 
> guidelines if they say something to this issue? (e.g. cppcoreguidelines, 
> hicpp, cert) As we have modules for them it would be great to make aliases to 
> this check if they demand this to be checked.


Thanks for the great suggestions. Updated the diff according to the feedback. 
Also checked with cppcoreguidelines, hicpp as well as cert. Only cert has a 
related, yet different rule 

 stating that calls to placement new shall be provided with properly aligned 
pointers. I'd say this should be a distinct check. Happy to work on it after 
this one.




Comment at: clang-tidy/bugprone/PlacementNewTargetTypeMismatch.cpp:42
+
+  assert((Cast->getSubExpr()->getType()->isPointerType() ||
+ Cast->getSubExpr()->getType()->isArrayType()) &&

JonasToth wrote:
> Is this universally true? What about the nothrow-overload, would that 
> interfere?
Thanks, rewrote that check.


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

https://reviews.llvm.org/D60139



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