r356981 - [RISCV] Pass -target-abi to -cc1as

[Roger Ferrer Ibanez via cfe-commits]

Author: rogfer01
Date: Tue Mar 26 01:01:18 2019
New Revision: 356981

URL: http://llvm.org/viewvc/llvm-project?rev=356981&view=rev
Log:
[RISCV] Pass -target-abi to -cc1as

The RISC-V assembler needs the target ABI because it defines a flag of the ELF
file, as described in [1].

Make clang (the driver) to pass the target ABI to -cc1as in exactly the same
way it does for -cc1.

Currently -cc1as knows about -target-abi but is not handling it. Handle it and
pass it to the MC layer via MCTargetOptions.

[1] 
https://github.com/riscv/riscv-elf-psabi-doc/blob/master/riscv-elf.md#file-header

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


Modified:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.h
cfe/trunk/test/Driver/riscv-abi.c
cfe/trunk/tools/driver/cc1as_main.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=356981&r1=356980&r2=356981&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Tue Mar 26 01:01:18 2019
@@ -5931,6 +5931,15 @@ void ClangAs::AddX86TargetArgs(const Arg
   }
 }
 
+void ClangAs::AddRISCVTargetArgs(const ArgList &Args,
+   ArgStringList &CmdArgs) const {
+  const llvm::Triple &Triple = getToolChain().getTriple();
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
+
+  CmdArgs.push_back("-target-abi");
+  CmdArgs.push_back(ABIName.data());
+}
+
 void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output, const InputInfoList 
&Inputs,
const ArgList &Args,
@@ -6100,6 +6109,11 @@ void ClangAs::ConstructJob(Compilation &
 CmdArgs.push_back("-arm-add-build-attributes");
 }
 break;
+
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+AddRISCVTargetArgs(Args, CmdArgs);
+break;
   }
 
   // Consume all the warning flags. Usually this would be handled more

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.h?rev=356981&r1=356980&r2=356981&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.h Tue Mar 26 01:01:18 2019
@@ -119,6 +119,8 @@ public:
  llvm::opt::ArgStringList &CmdArgs) const;
   void AddX86TargetArgs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const;
+  void AddRISCVTargetArgs(const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs) const;
   bool hasGoodDiagnostics() const override { return true; }
   bool hasIntegratedAssembler() const override { return false; }
   bool hasIntegratedCPP() const override { return false; }

Modified: cfe/trunk/test/Driver/riscv-abi.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/riscv-abi.c?rev=356981&r1=356980&r2=356981&view=diff
==
--- cfe/trunk/test/Driver/riscv-abi.c (original)
+++ cfe/trunk/test/Driver/riscv-abi.c Tue Mar 26 01:01:18 2019
@@ -2,6 +2,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
 // CHECK-ILP32: "-target-abi" "ilp32"
 
@@ -26,6 +30,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
 // CHECK-LP64: "-target-abi" "lp64"
 

Modified: cfe/trunk/tools/driver/cc1as_main.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/driver/cc1as_main.cpp?rev=356981&r1=356980&r2=356981&view=diff
==
--- cfe/trunk/tools/driver/cc1as_main.cpp (original)
+++ cfe/trunk/tools/driver/cc1as_main.cpp Tue Mar 26 01:01:18 2019
@@ -137,6 +137,10 @@ struct AssemblerInvocation {
   /// The name of the relocation model to use.
   std::string RelocationModel;
 
+  /// The ABI targeted by the backend. Specified using -targe

[PATCH] D59298: [RISCV] Pass -target-abi to -cc1as

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356981: [RISCV] Pass -target-abi to -cc1as (authored by 
rogfer01, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59298?vs=190409&id=192248#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59298

Files:
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.h
  cfe/trunk/test/Driver/riscv-abi.c
  cfe/trunk/tools/driver/cc1as_main.cpp

Index: cfe/trunk/test/Driver/riscv-abi.c
===
--- cfe/trunk/test/Driver/riscv-abi.c
+++ cfe/trunk/test/Driver/riscv-abi.c
@@ -2,6 +2,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
 // RUN: %clang -target riscv32-unknown-elf %s -### -o %t.o -mabi=ilp32 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-ILP32 %s
+// RUN: %clang -target riscv32-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=ilp32 2>&1 | FileCheck -check-prefix=CHECK-ILP32 %s
 
 // CHECK-ILP32: "-target-abi" "ilp32"
 
@@ -26,6 +30,10 @@
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
 // RUN: %clang -target riscv64-unknown-elf %s -### -o %t.o -mabi=lp64 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LP64 %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o 2>&1 \
+// RUN:   | FileCheck -check-prefix=CHECK-LP64  %s
+// RUN: %clang -target riscv64-unknown-elf -x assembler %s -### -o %t.o \
+// RUN:   -mabi=lp64 2>&1 | FileCheck -check-prefix=CHECK-LP64 %s
 
 // CHECK-LP64: "-target-abi" "lp64"
 
Index: cfe/trunk/lib/Driver/ToolChains/Clang.h
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.h
+++ cfe/trunk/lib/Driver/ToolChains/Clang.h
@@ -119,6 +119,8 @@
  llvm::opt::ArgStringList &CmdArgs) const;
   void AddX86TargetArgs(const llvm::opt::ArgList &Args,
 llvm::opt::ArgStringList &CmdArgs) const;
+  void AddRISCVTargetArgs(const llvm::opt::ArgList &Args,
+  llvm::opt::ArgStringList &CmdArgs) const;
   bool hasGoodDiagnostics() const override { return true; }
   bool hasIntegratedAssembler() const override { return false; }
   bool hasIntegratedCPP() const override { return false; }
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -5931,6 +5931,15 @@
   }
 }
 
+void ClangAs::AddRISCVTargetArgs(const ArgList &Args,
+   ArgStringList &CmdArgs) const {
+  const llvm::Triple &Triple = getToolChain().getTriple();
+  StringRef ABIName = riscv::getRISCVABI(Args, Triple);
+
+  CmdArgs.push_back("-target-abi");
+  CmdArgs.push_back(ABIName.data());
+}
+
 void ClangAs::ConstructJob(Compilation &C, const JobAction &JA,
const InputInfo &Output, const InputInfoList &Inputs,
const ArgList &Args,
@@ -6100,6 +6109,11 @@
 CmdArgs.push_back("-arm-add-build-attributes");
 }
 break;
+
+  case llvm::Triple::riscv32:
+  case llvm::Triple::riscv64:
+AddRISCVTargetArgs(Args, CmdArgs);
+break;
   }
 
   // Consume all the warning flags. Usually this would be handled more
Index: cfe/trunk/tools/driver/cc1as_main.cpp
===
--- cfe/trunk/tools/driver/cc1as_main.cpp
+++ cfe/trunk/tools/driver/cc1as_main.cpp
@@ -137,6 +137,10 @@
   /// The name of the relocation model to use.
   std::string RelocationModel;
 
+  /// The ABI targeted by the backend. Specified using -target-abi. Empty
+  /// otherwise.
+  std::string TargetABI;
+
   /// @}
 
 public:
@@ -282,6 +286,7 @@
   Opts.NoExecStack = Args.hasArg(OPT_mno_exec_stack);
   Opts.FatalWarnings = Args.hasArg(OPT_massembler_fatal_warnings);
   Opts.RelocationModel = Args.getLastArgValue(OPT_mrelocation_model, "pic");
+  Opts.TargetABI = Args.getLastArgValue(OPT_target_abi);
   Opts.IncrementalLinkerCompatible =
   Args.hasArg(OPT_mincremental_linker_compatible);
   Opts.SymbolDefs = Args.getAllArgValues(OPT_defsym);
@@ -426,6 +431,9 @@
   raw_pwrite_stream *Out = FDOS.get();
   std::unique_ptr BOS;
 
+  MCTargetOptions MCOptions;
+  MCOptions.ABIName = Opts.TargetABI;
+
   // FIXME: There is a bit of code duplication with addPassesToEmitFile.
   if (Opts.OutputType == AssemblerInvocation::FT_Asm) {
 MCInstPrinter *IP = TheTarget->createMCInstPrinter(
@@ -434,7 +442,6 @@
 std::unique_ptr CE;
 if (Opts.ShowEncoding)
   CE.reset(TheTarget->createMCCodeEmitter(*MCII, *MRI, Ctx));
-MCTargetOptions M

[PATCH] D59725: Additions to creduce script

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:145
+  matches = re.findall(stacktrace_re, crash_output)
+  result = filter(lambda x: x and x.strip() not in filters, matches)[:5]
+  for msg in result:

george.burgess.iv wrote:
> nit: please prefer `[x for x in matches if x and x.strip() not in 
> filters][:5]`. py3's filter returns a generator, whereas py2's returns a list.
Stack traces also look different on macOS and it would be nice to handle that 
too.

Here's a sample I got from adding a llvm_unreachable at a random location:
```
My unreachable message...
UNREACHABLE executed at 
/Users/alex/cheri/llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4468!
Stack dump:
0.  Program arguments: 
/Users/alex/cheri/llvm-project/cmake-build-debug/bin/opt 
-mtriple=cheri-unknown-freebsd -mcpu=cheri128 -mattr=+cheri128 -target-abi 
purecap -relocation-model pic -S -instcombine -simplifycfg 
/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll
 -o - 
1.  Running pass 'Function Pass Manager' on module 
'/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll'.
2.  Running pass 'Combine redundant instructions' on function 
'@cannot_fold_tag_unknown'
0  libLLVMSupport.dylib 0x000114515a9d 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 45
1  libLLVMSupport.dylib 0x0001145153f1 llvm::sys::RunSignalHandlers() + 
65
2  libLLVMSupport.dylib 0x000114515fbf SignalHandler(int) + 111
3  libsystem_platform.dylib 0x7fff5b637b3d _sigtramp + 29
4  libsystem_platform.dylib 0x7ffee20d0cf0 _sigtramp + 2259259856
5  libsystem_c.dylib0x7fff5b4f51c9 abort + 127
6  libLLVMSupport.dylib 0x00011446bb12 
llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) + 162
7  libLLVMInstCombine.dylib 0x000112c345c8 
llvm::InstCombiner::foldICmpUsingKnownBits(llvm::ICmpInst&) + 4136
8  libLLVMInstCombine.dylib 0x000112c34d19 
llvm::InstCombiner::visitICmpInst(llvm::ICmpInst&) + 569
9  libLLVMInstCombine.dylib 0x000112bb9cf2 llvm::InstCombiner::run() + 1522
10 libLLVMInstCombine.dylib 0x000112bbb310 
combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, 
llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, 
llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, llvm::LoopInfo*) 
+ 624
11 libLLVMInstCombine.dylib 0x000112bbb6d6 
llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) + 214
12 libLLVMCore.dylib0x000111c0bb4d 
llvm::FPPassManager::runOnFunction(llvm::Function&) + 317
13 libLLVMCore.dylib0x000111c0be83 
llvm::FPPassManager::runOnModule(llvm::Module&) + 99
14 libLLVMCore.dylib0x000111c0c2c4 (anonymous 
namespace)::MPPassManager::runOnModule(llvm::Module&) + 420
15 libLLVMCore.dylib0x000111c0c036 
llvm::legacy::PassManagerImpl::run(llvm::Module&) + 182
16 opt  0x00010db6657b main + 7163
17 libdyld.dylib0x7fff5b44ced9 start + 1
```



Comment at: clang/utils/creduce-clang-crash.py:362
+  r = Reduce(crash_script, file_to_reduce)
+  r.simplify_clang_args()
+  r.write_interestingness_test()

george.burgess.iv wrote:
> akhuang wrote:
> > george.burgess.iv wrote:
> > > I'm unclear on why we do a partial simplification of clang args here, 
> > > then a full reduction after creduce. Is there a disadvantage to instead 
> > > doing:
> > > 
> > > r.write_interestingness_test()
> > > r.clang_preprocess()
> > > r.reduce_clang_args()
> > > r.run_creduce()
> > > r.reduce_clang_args()
> > > 
> > > ?
> > > 
> > > The final `reduce_clang_args` being there to remove extra `-D`/`-I`/etc. 
> > > args if preprocessing failed somehow, to remove `-std=foo` args if those 
> > > aren't relevant after reduction, etc.
> > > 
> > > The advantage to this being that we no longer need to maintain a 
> > > `simplify` function, and creduce runs with a relatively minimal set of 
> > > args to start with.
> > > 
> > > In any case, can we please add comments explaining why we chose whatever 
> > > order we end up going with, especially where subtleties make it counter 
> > > to what someone might naively expect?
> > Basically the disadvantage is that clang takes longer to run before the 
> > reduction, so it takes unnecessary time to iterate through all the 
> > arguments beforehand. 
> > And yeah, the final `reduce_clang_args` is there to minimize the clang 
> > arguments needed to reproduce the crash on the reduced source file. 
> > 
> > If that makes sense, I can add comments for this-
> Eh, I don't have a strong opinion here. My inclination is to prefer a simpler 
> reduction script if the difference is len(args) clang invocations, since I 
> assume creduce is going to invoke clang way more than len(args) times. I see 
> a doc

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:89
+  /// relevant name, not including qualifiers.
+  Name,
+};

ymandel wrote:
> ilya-biryukov wrote:
> > Same here, what happens to the template arguments and multi-token names, 
> > e.g.
> > `operator +` or `foo`?
> Good point. This seems difficult to get right, since NamedDecl does not carry 
> sufficient loc data.  However, I've updated the code to explicitly fail in 
> such cases, so at least we won't have bad rewrites.
> 
> BTW, any idea whether constructor initializers can ever be multi token?
> BTW, any idea whether constructor initializers can ever be multi token?

Two cases come to mind:
1. arbitrary names when initializing base classes,  something like 
`::ns::X(10)`
2. template packs with ellipsis (although ellipsis shouldn't be normally part 
that we replace, I guess): `Base(10)...`

Full example:
```
namespace ns {
  struct X {
X(int);
  };
}


template 
struct Y : ns::X, Bases... {
  Y() : ns::X(10), Bases(10)... {
  }
};

struct Z {
  Z(int);
};
struct W {
  W(int);
};

Y y;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



___
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

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:2044
+ : std::distance(RD->field_begin(), RD->field_end());
+  if (VerifyOnly && (LastIdx >= NextIdx || HasNonDesignatedInit) &&
+  SemaRef.getLangOpts().CPlusPlus2a)

hintonda wrote:
> Rakete wrote:
> > `!VerifyOnly` and braces please.
> Will commit this change as soon as check-clang finishes, but not sure I grok 
> the VerityOnly/!VerifyOnly criteria.  Could you help me out? 
`VerifyOnly` is used if you only want to verify and not actually generate any 
diagnostics or actually do anything that would commit to that change. So only 
generate the warning if `!VerifyOnly`.


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] D59811: [clangd] Add fallback mode for code completion when compile command or preamble is not ready.

[Eric Liu via Phabricator via cfe-commits]

ioeric created this revision.
ioeric added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, 
MaskRay, javed.absar.
Herald added a project: clang.

When calling TUScehduler::runWithPreamble (e.g. in code compleiton), allow
entering a fallback mode when compile command or preamble is not ready, instead 
of
waiting. This allows clangd to perform naive code completion e.g. using 
identifiers
in the current file or symbols in the index.

This patch simply returns empty result for code completion in fallback mode. 
Identifier-based
plus more advanced index-based completion will be added in followup patches.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59811

Files:
  clangd/ClangdServer.cpp
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  clangd/Threading.h
  unittests/clangd/ClangdTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -15,14 +15,13 @@
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 #include 
 
 namespace clang {
 namespace clangd {
 namespace {
 
-using ::testing::_;
-using ::testing::AllOf;
 using ::testing::AnyOf;
 using ::testing::Each;
 using ::testing::ElementsAre;
@@ -35,13 +34,18 @@
 
 class TUSchedulerTests : public ::testing::Test {
 protected:
-  ParseInputs getInputs(PathRef File, std::string Contents) {
+  FileUpdateInputs getInputs(PathRef File, std::string Contents) {
 ParseInputs Inputs;
-Inputs.CompileCommand = *CDB.getCompileCommand(File);
 Inputs.FS = buildTestFS(Files, Timestamps);
 Inputs.Contents = std::move(Contents);
 Inputs.Opts = ParseOptions();
-return Inputs;
+FileUpdateInputs UpdateInputs;
+UpdateInputs.InputSansCommand = std::move(Inputs);
+std::string FilePath = File;
+UpdateInputs.GetCompileCommand = [this, FilePath]() {
+  return *CDB.getCompileCommand(FilePath);
+};
+return UpdateInputs;
   }
 
   void updateWithCallback(TUScheduler &S, PathRef File,
@@ -72,7 +76,7 @@
   /// Schedule an update and call \p CB with the diagnostics it produces, if
   /// any. The TUScheduler should be created with captureDiags as a
   /// DiagsCallback for this to work.
-  void updateWithDiags(TUScheduler &S, PathRef File, ParseInputs Inputs,
+  void updateWithDiags(TUScheduler &S, PathRef File, FileUpdateInputs Inputs,
WantDiagnostics WD,
llvm::unique_function)> CB) {
 Path OrigFile = File.str();
@@ -245,7 +249,7 @@
 
 S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
   [&](Expected Pre) {
-ASSERT_TRUE(bool(Pre));
+EXPECT_TRUE((bool)Pre);
 assert(bool(Pre));
 EXPECT_THAT(includes(Pre->Preamble),
 ElementsAre(""));
@@ -397,8 +401,9 @@
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 ASSERT_TRUE((bool)AST);
-EXPECT_EQ(AST->Inputs.FS, Inputs.FS);
-EXPECT_EQ(AST->Inputs.Contents, Inputs.Contents);
+EXPECT_EQ(AST->Inputs.FS, Inputs.InputSansCommand.FS);
+EXPECT_EQ(AST->Inputs.Contents,
+  Inputs.InputSansCommand.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalASTReads;
@@ -415,7 +420,7 @@
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
 
 ASSERT_TRUE((bool)Preamble);
-EXPECT_EQ(Preamble->Contents, Inputs.Contents);
+EXPECT_EQ(Preamble->Contents, Inputs.InputSansCommand.Contents);
 
 std::lock_guard Lock(Mut);
 ++TotalPreambleReads;
@@ -504,6 +509,7 @@
   )cpp";
   auto WithEmptyPreamble = R"cpp(int main() {})cpp";
   S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto);
+
   S.runWithPreamble(
   "getNonEmptyPreamble", Foo, TUScheduler::Stale,
   [&](Expected Preamble) {
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -1358,6 +1358,7 @@
   // parsing.
   CDB.ExtraClangFlags.push_back("-fno-delayed-template-parsing");
   Server.addDocument(FooCpp, Source.code(), WantDiagnostics::Yes);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   CodeCompleteResult Completions = cantFail(runCodeComplete(
   Server, FooCpp, Source.point(), clangd::CodeCompleteOptions()));
 
Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTe

r356983 - update the release notes after the change of 'clang -dumpversion'

[Sylvestre Ledru via cfe-commits]

Author: sylvestre
Date: Tue Mar 26 02:48:23 2019
New Revision: 356983

URL: http://llvm.org/viewvc/llvm-project?rev=356983&view=rev
Log:
update the release notes after the change of 'clang -dumpversion'

Modified:
cfe/trunk/docs/ReleaseNotes.rst

Modified: cfe/trunk/docs/ReleaseNotes.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.rst?rev=356983&r1=356982&r2=356983&view=diff
==
--- cfe/trunk/docs/ReleaseNotes.rst (original)
+++ cfe/trunk/docs/ReleaseNotes.rst Tue Mar 26 02:48:23 2019
@@ -75,6 +75,8 @@ future versions of Clang.
 Modified Compiler Flags
 ---
 
+- `clang -dumpversion` now returns the version of Clang itself.
+
 - ...
 
 New Pragmas in Clang


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

[PATCH] D35068: [analyzer] Detect usages of unsafe I/O functions

[Daniel Kolozsvari via Phabricator via cfe-commits]

koldaniel added a comment.

In D35068#1441601 , @lebedev.ri wrote:

> In D35068#1441581 , @koldaniel wrote:
>
> > Bug fixing: faulty handling of built-in functions.
>
>
> Please open a new differential, that is a completely new patch.


Hi,

Done: https://reviews.llvm.org/D59812


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

https://reviews.llvm.org/D35068



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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: gribozavr.
Herald added subscribers: cfe-commits, jdoerfert, kadircet, arphaman, jkorous, 
MaskRay, ioeric, hiraditya, mgorny.
Herald added projects: clang, LLVM.

Annotations allow writing nice-looking unit test code when one needs
access to locations from the source code, e.g. running code completion
at particular offsets in a file. See comments in Annotations.cpp for
more details on the API.

Also got rid of a duplicate annotations parsing code in clang's code
complete tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59814

Files:
  clang-tools-extra/unittests/clangd/Annotations.cpp
  clang-tools-extra/unittests/clangd/Annotations.h
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/CodeCompleteTest.cpp
  llvm/include/llvm/Testing/Support/Annotations.h
  llvm/lib/Testing/Support/Annotations.cpp
  llvm/lib/Testing/Support/CMakeLists.txt

Index: llvm/lib/Testing/Support/CMakeLists.txt
===
--- llvm/lib/Testing/Support/CMakeLists.txt
+++ llvm/lib/Testing/Support/CMakeLists.txt
@@ -2,6 +2,7 @@
 add_definitions(-DGTEST_HAS_TR1_TUPLE=0)
 
 add_llvm_library(LLVMTestingSupport
+  Annotations.cpp
   Error.cpp
   SupportHelpers.cpp
 
Index: llvm/lib/Testing/Support/Annotations.cpp
===
--- llvm/lib/Testing/Support/Annotations.cpp
+++ llvm/lib/Testing/Support/Annotations.cpp
@@ -6,12 +6,11 @@
 //
 //===--===//
 
-#include "Annotations.h"
-#include "SourceCode.h"
-
-namespace clang {
-namespace clangd {
+#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/raw_ostream.h"
 
+using namespace llvm;
 // Crash if the assertion fails, printing the message and testcase.
 // More elegant error handling isn't needed for unit tests.
 static void require(bool Assertion, const char *Msg, llvm::StringRef Code) {
@@ -22,30 +21,31 @@
 }
 
 Annotations::Annotations(llvm::StringRef Text) {
-  auto Here = [this] { return offsetToPosition(Code, Code.size()); };
   auto Require = [Text](bool Assertion, const char *Msg) {
 require(Assertion, Msg, Text);
   };
   llvm::Optional Name;
-  llvm::SmallVector, 8> OpenRanges;
+  llvm::SmallVector, 8> OpenRanges;
 
   Code.reserve(Text.size());
   while (!Text.empty()) {
 if (Text.consume_front("^")) {
-  Points[Name.getValueOr("")].push_back(Here());
-  Name = None;
+  Points[Name.getValueOr("")].push_back(Code.size());
+  Name = llvm::None;
   continue;
 }
 if (Text.consume_front("[[")) {
-  OpenRanges.emplace_back(Name.getValueOr(""), Here());
-  Name = None;
+  OpenRanges.emplace_back(Name.getValueOr(""), Code.size());
+  Name = llvm::None;
   continue;
 }
 Require(!Name, "$name should be followed by ^ or [[");
 if (Text.consume_front("]]")) {
   Require(!OpenRanges.empty(), "unmatched ]]");
-  Ranges[OpenRanges.back().first].push_back(
-  {OpenRanges.back().second, Here()});
+  llvm::Range R;
+  R.Begin = OpenRanges.back().second;
+  R.End = Code.size();
+  Ranges[OpenRanges.back().first].push_back(R);
   OpenRanges.pop_back();
   continue;
 }
@@ -61,13 +61,13 @@
   Require(OpenRanges.empty(), "unmatched [[");
 }
 
-Position Annotations::point(llvm::StringRef Name) const {
+size_t Annotations::point(llvm::StringRef Name) const {
   auto I = Points.find(Name);
   require(I != Points.end() && I->getValue().size() == 1,
   "expected exactly one point", Code);
   return I->getValue()[0];
 }
-std::vector Annotations::points(llvm::StringRef Name) const {
+std::vector Annotations::points(llvm::StringRef Name) const {
   auto P = Points.lookup(Name);
   return {P.begin(), P.end()};
 }
@@ -81,6 +81,3 @@
   auto R = Ranges.lookup(Name);
   return {R.begin(), R.end()};
 }
-
-} // namespace clangd
-} // namespace clang
Index: llvm/include/llvm/Testing/Support/Annotations.h
===
--- llvm/include/llvm/Testing/Support/Annotations.h
+++ llvm/include/llvm/Testing/Support/Annotations.h
@@ -16,9 +16,9 @@
 //)cpp");
 //
 //StringRef Code = Example.code();  // annotations stripped.
-//std::vector PP = Example.points();  // all unnamed points
-//Position P = Example.point(); // there must be exactly one
-//Range R = Example.range("fail");  // find named ranges
+//std::vector PP = Example.points();// all unnamed points
+//size_t P = Example.point();   // there must be exactly one
+//llvm::Range R = Example.range("fail");// find named ranges
 //
 // Points/ranges are coordinates into `code()` which is stripped of annotations.
 //
@@ -27,16 +27,22 @@
 //
 //===---

r356987 - [OpenCL] Allow variadic macros as Clang feature.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Tue Mar 26 04:22:37 2019
New Revision: 356987

URL: http://llvm.org/viewvc/llvm-project?rev=356987&view=rev
Log:
[OpenCL] Allow variadic macros as Clang feature.


Modified:
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Misc/warning-flags.c
cfe/trunk/test/Preprocessor/macro_variadic.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=356987&r1=356986&r2=356987&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Mar 26 04:22:37 2019
@@ -395,8 +395,8 @@ def warn_cxx98_compat_empty_fnmacro_arg
 def note_macro_here : Note<"macro %0 defined here">;
 def note_macro_expansion_here : Note<"expansion of macro %0 requested here">;
 
-def err_pp_opencl_variadic_macros :
-  Error<"variadic macros not supported in OpenCL">;
+def ext_pp_opencl_variadic_macros : Extension<
+  "variadic macros are a Clang extension in OpenCL">;
 
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=356987&r1=356986&r2=356987&view=diff
==
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Tue Mar 26 04:22:37 2019
@@ -2181,8 +2181,7 @@ bool Preprocessor::ReadMacroParameterLis
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
   if (LangOpts.OpenCL) {
-Diag(Tok, diag::err_pp_opencl_variadic_macros);
-return true;
+Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 
   // Lex the token after the identifier.

Modified: cfe/trunk/test/Misc/warning-flags.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/warning-flags.c?rev=356987&r1=356986&r2=356987&view=diff
==
--- cfe/trunk/test/Misc/warning-flags.c (original)
+++ cfe/trunk/test/Misc/warning-flags.c Tue Mar 26 04:22:37 2019
@@ -96,4 +96,4 @@ CHECK-NEXT:   warn_weak_import
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 27
+CHECK: Number in -Wpedantic (not covered by other -W flags): 28

Modified: cfe/trunk/test/Preprocessor/macro_variadic.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/macro_variadic.cl?rev=356987&r1=356986&r2=356987&view=diff
==
--- cfe/trunk/test/Preprocessor/macro_variadic.cl (original)
+++ cfe/trunk/test/Preprocessor/macro_variadic.cl Tue Mar 26 04:22:37 2019
@@ -1,3 +1,20 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
 
-#define X(...) 1 // expected-error {{variadic macros not supported in OpenCL}}
+
+#define NO_VAR_FUNC(...)  5
+#define VAR_FUNC(...) func(__VA_ARGS__);
+#define VAR_PRINTF(str, ...) printf(str, __VA_ARGS__);
+#ifdef PEDANTIC
+// expected-warning@-4{{variadic macros are a Clang extension in OpenCL}}
+// expected-warning@-4{{variadic macros are a Clang extension in OpenCL}}
+// expected-warning@-4{{variadic macros are a Clang extension in OpenCL}}
+#endif
+
+int printf(__constant const char *st, ...);
+
+void foo() {
+  NO_VAR_FUNC(1, 2, 3);
+  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' 
is invalid in OpenCL}}
+  VAR_PRINTF("%i", 1);
+}


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

[PATCH] D59814: [Testing] Move clangd::Annotations to llvm testing support

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

Seeking feedback for this change, mostly interested if this a good place for 
test helpers like this?
I'm planning to author more tests in `clangTooling` using this soon, so moving 
it somewhere clang-specific is an option too.




Comment at: llvm/include/llvm/Testing/Support/Annotations.h:42
+/// represents a half-open range.
+struct Range {
+  size_t Begin = 0;

This name is probably too generic, happy to change the name or implement this 
in an alternative manner.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59814



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

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

[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.

Other than that, LGTM, thanks! Please, again, let @NoQ have the final say.


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

https://reviews.llvm.org/D59279



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

[PATCH] D59195: [analyzer] Remove the default value arg from getChecker*Option

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added inline comments.



Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:148-150
+  assert(Ret &&
+ "This option should be either 'true' or 'false', and should've been "
+ "validated by CheckerRegisrty!");

NoQ wrote:
> Even though `operator*()` would have asserted that there's a value, i really 
> appreciate the user-friendly assert message.
Sorry for the impertinence, but there's a typo: `CheckerRegisrty`.



Comment at: lib/StaticAnalyzer/Core/AnalyzerOptions.cpp:171
+ "This option should be numeric, and should've been validated by "
+ "CheckerRegisrty!");
   (void)HasFailed;

And the same typo here.


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

https://reviews.llvm.org/D59195



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

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added a comment.

In D59279#1438514 , @mgrang wrote:

> Yes, the reason we limit the checks only to //unordered// containers is to 
> reduce the false positive rate. Although, as you rightly pointed out that 
> //ordered// sets of pointers are as non-deterministic as //unordered// ones. 
> Once our checks have the capability to detect what happens inside the loop 
> maybe we can add //ordered// sets too. I will add this to the TODO. Thanks.


Great, thanks!


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

https://reviews.llvm.org/D59279



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D59665#1442328 , @shafik wrote:

> @martong your idea does not work b/c default construction `DeclarationName()` 
> treats it the same as being empty. So `if (!Name)` is still `true`.


And did you try moving the `push_back` to the other scope? I'd expect the the 
ConflictingDecls to be empty then.


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

https://reviews.llvm.org/D59665



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Looks good for me now, but we should make a similar change in VisitRecordDecl 
too.


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

https://reviews.llvm.org/D59665



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2463
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),

shafik wrote:
> a_sidorin wrote:
> > If I understand correctly, this will replace Name with SearchName causing 
> > an anonymous enum to be imported as a named a few lines below. It doesn't 
> > look like a correct behaviour to me.
> This is correct because either `SearchName` is `Name` or it is the name of 
> the typedef for the anonymous enum set via `ImportInto(SearchName, 
> D->getTypedefNameForAnonDecl()->getDeclName())`
Okay, this is indeed correct. But then we should make a similar change in 
VisitRecordDecl too (there we do exactly the same with typedefs).


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

https://reviews.llvm.org/D59665



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

[PATCH] D59815: [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau created this revision.
pgousseau added reviewers: probinson, filcab, gbedwell, rsmith, wristow.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Can be safely enabled on PS4.


Repository:
  rC Clang

https://reviews.llvm.org/D59815

Files:
  lib/Driver/SanitizerArgs.cpp
  test/Driver/fsanitize.c


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -221,6 +221,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address 
-fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -### -- %s 
2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
+// RUN: %clang -target x86_64-scei-ps4  -fsanitize=address %s -### 2>&1 | 
FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: 
-cc1{{.*}}-fsanitize-address-globals-dead-stripping
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -768,6 +768,7 @@
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
 !TC.getTriple().isOSBinFormatELF() || TC.getTriple().isOSFuchsia() ||
+TC.getTriple().isPS4() ||
 Args.hasArg(options::OPT_fsanitize_address_globals_dead_stripping);
 
 AsanUseOdrIndicator =


Index: test/Driver/fsanitize.c
===
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -221,6 +221,7 @@
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -fsanitize-address-globals-dead-stripping -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // RUN: %clang_cl --target=x86_64-windows-msvc -fsanitize=address -### -- %s 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
+// RUN: %clang -target x86_64-scei-ps4  -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-GLOBALS
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -768,6 +768,7 @@
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
 AsanGlobalsDeadStripping =
 !TC.getTriple().isOSBinFormatELF() || TC.getTriple().isOSFuchsia() ||
+TC.getTriple().isPS4() ||
 Args.hasArg(options::OPT_fsanitize_address_globals_dead_stripping);
 
 AsanUseOdrIndicator =
___
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.

In D59555#1442331 , @boga95 wrote:

> Why is it better not to use `static` functions/variables? Has it any 
> performance impact?


I don't think so, I just think that it's easier to follow what's happening with 
a non-static field. Also, I just don't see why it needs to be static, you only 
use `GenericTaintChecker::Custom*` where you could easily access it. Especially 
`GenericTaintChecker::getConfiguration` -- if it wasn't static, you wouldn't 
even need to take the checker as a parameter.


Repository:
  rC Clang

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] D59555: [analyzer] Add yaml parser to GenericTaintChecker

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

Szelethus added inline comments.



Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:73-75
   static const unsigned InvalidArgIndex = UINT_MAX;
   /// Denotes the return vale.
   static const unsigned ReturnValueIndex = UINT_MAX - 1;

boga95 wrote:
> Szelethus wrote:
> > We should definitely change these, not only is the large integer number 
> > impossible to remember, but this value could differ on different platforms.
> I tried to use int, but I got a lot of warnings because of the `getNumArgs()` 
> returns an unsigned value.
What warnings? I thought we have `-Wsign-conversion` disabled.


Repository:
  rC Clang

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] D59815: [Driver] Enable -fsanitize-address-globals-dead-stripping by default on PS4.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

This is fine for PS4, I'm just curious whether anyone knows if there's a 
predicate that means something like "target does not use GNU tools" so we don't 
have to itemize Fuschia and PS4 this way.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59815



___
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 marked 5 inline comments as done.
martong added inline comments.



Comment at: lib/AST/ASTImporter.cpp:2260
 
   ConflictingDecls.push_back(FoundDecl);
 }

a_sidorin wrote:
> Do we push the same decl twice?
Uhh, thanks, that is rebase error I've made.



Comment at: lib/AST/ASTImporter.cpp:8532
 unsigned NumDecls) {
-  return Name;
+  return DeclarationName();
 }

a_sidorin wrote:
> Empty DeclarationName can be valid sometimes. Should we return 
> ErrorOr instead? This can also simplify caller code a bit.
That's a good idea, thanks.
Though, I'd use Expected rather, that would be more consistent with the rest 
of the code. 


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 192263.
martong marked 2 inline comments as done.
martong added a comment.

- Add braces around else
- Remove falsely duplicated push_back
- Use Expected in HandleNameConflict


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
@@ -1964,7 +1964,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";
 }
 
@@ -2674,6 +2674,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) {
@@ -5534,6 +5592,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.
@@ -1944,15 +1945,6 @@
 
 bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
 RecordDecl *ToRecord, bool Complain) {
-  // Eliminate a potential failure point where we attempt to re-import
-  // something we're trying to import while completing ToRecord.
-  Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
-  if (ToOrigin) {
-auto *ToOriginRecord = dyn_cast(ToOrigin);
-if (ToOriginRecord)
-  ToRecord = ToOriginRecord;
-  }
-
   StructuralEquivalenceContext Ctx(Importer.getFromContext(),
ToRecord->getASTContext(),
Importer.getNonEquivalentDecls(),
@@ -2154,11 +2146,11 @@
 }
 
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, Decl::IDNS_Namespace,
- ConflictingDecls.data(),
- ConflictingDecls.size());
-  if (!Name)
-return make_error(ImportError

[PATCH] D59817: [clangd] Add activate command to the vscode extension.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.
Herald added a project: clang.

This would help minizime the annoying part of not activating the extension
for .cu file.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D59817

Files:
  clangd/clients/clangd-vscode/package.json
  clangd/clients/clangd-vscode/src/extension.ts


Index: clangd/clients/clangd-vscode/src/extension.ts
===
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -129,4 +129,8 @@
 status.clear();
 }
 })
+// An empty place holder for the activate command, otherwise we'll get an
+// "command is not registered" error.
+context.subscriptions.push(vscode.commands.registerCommand(
+'clangd-vscode.activate', async () => {}));
 }
Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -24,7 +24,8 @@
 "onLanguage:c",
 "onLanguage:cpp",
 "onLanguage:objective-c",
-"onLanguage:objective-cpp"
+"onLanguage:objective-cpp",
+"onCommand:clangd-vscode.activate"
 ],
 "main": "./out/src/extension",
 "scripts": {
@@ -81,6 +82,10 @@
 {
 "command": "clangd-vscode.switchheadersource",
 "title": "Switch between Source/Header"
+},
+{
+"command": "clangd-vscode.activate",
+"title": "Manually activate clangd extension"
 }
 ],
 "keybindings": [


Index: clangd/clients/clangd-vscode/src/extension.ts
===
--- clangd/clients/clangd-vscode/src/extension.ts
+++ clangd/clients/clangd-vscode/src/extension.ts
@@ -129,4 +129,8 @@
 status.clear();
 }
 })
+// An empty place holder for the activate command, otherwise we'll get an
+// "command is not registered" error.
+context.subscriptions.push(vscode.commands.registerCommand(
+'clangd-vscode.activate', async () => {}));
 }
Index: clangd/clients/clangd-vscode/package.json
===
--- clangd/clients/clangd-vscode/package.json
+++ clangd/clients/clangd-vscode/package.json
@@ -24,7 +24,8 @@
 "onLanguage:c",
 "onLanguage:cpp",
 "onLanguage:objective-c",
-"onLanguage:objective-cpp"
+"onLanguage:objective-cpp",
+"onCommand:clangd-vscode.activate"
 ],
 "main": "./out/src/extension",
 "scripts": {
@@ -81,6 +82,10 @@
 {
 "command": "clangd-vscode.switchheadersource",
 "title": "Switch between Source/Header"
+},
+{
+"command": "clangd-vscode.activate",
+"title": "Manually activate clangd extension"
 }
 ],
 "keybindings": [
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57978: [CodeGen] Generate follow-up metadata for loops with more than one transformation.

[Dave Green via Phabricator via cfe-commits]

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

Yeah, OK. This looks like a good patch to me. As I said, I'm not a clang 
expert, but the code looks sensible enough. (Perhaps wait a couple of days in 
case others have objections.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57978



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

[libunwind] r356991 - Creating release candidate rc1 from release_710 branch

[Tom Stellard via cfe-commits]

Author: tstellar
Date: Tue Mar 26 06:01:15 2019
New Revision: 356991

URL: http://llvm.org/viewvc/llvm-project?rev=356991&view=rev
Log:
Creating release candidate rc1 from release_710 branch

Added:
libunwind/tags/RELEASE_710/
libunwind/tags/RELEASE_710/rc1/
  - copied from r356990, libunwind/branches/release_70/

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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 192267.
ymandel marked 5 inline comments as done.
ymandel added a comment.

- Remove lvalue-ref overloads in builder
- add StringRef overloads for TextGenerator-taking methods
- constrain Name targeting for ctor initializers to explicitly written 
initializers.
- add multi-token member test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376

Files:
  clang/include/clang/Tooling/Refactoring/Transformer.h
  clang/lib/Tooling/Refactoring/CMakeLists.txt
  clang/lib/Tooling/Refactoring/Transformer.cpp
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/TransformerTest.cpp

Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -0,0 +1,416 @@
+//===- unittest/Tooling/TransformerTest.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/Transformer.h"
+
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Tooling/Tooling.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tooling {
+namespace {
+using ::clang::ast_matchers::anyOf;
+using ::clang::ast_matchers::argumentCountIs;
+using ::clang::ast_matchers::callee;
+using ::clang::ast_matchers::callExpr;
+using ::clang::ast_matchers::cxxMemberCallExpr;
+using ::clang::ast_matchers::cxxMethodDecl;
+using ::clang::ast_matchers::cxxRecordDecl;
+using ::clang::ast_matchers::declRefExpr;
+using ::clang::ast_matchers::expr;
+using ::clang::ast_matchers::functionDecl;
+using ::clang::ast_matchers::hasAnyName;
+using ::clang::ast_matchers::hasArgument;
+using ::clang::ast_matchers::hasDeclaration;
+using ::clang::ast_matchers::hasElse;
+using ::clang::ast_matchers::hasName;
+using ::clang::ast_matchers::hasType;
+using ::clang::ast_matchers::ifStmt;
+using ::clang::ast_matchers::member;
+using ::clang::ast_matchers::memberExpr;
+using ::clang::ast_matchers::namedDecl;
+using ::clang::ast_matchers::on;
+using ::clang::ast_matchers::pointsTo;
+using ::clang::ast_matchers::to;
+using ::clang::ast_matchers::unless;
+
+constexpr char KHeaderContents[] = R"cc(
+  struct string {
+string(const char*);
+char* c_str();
+int size();
+  };
+  int strlen(const char*);
+
+  namespace proto {
+  struct PCFProto {
+int foo();
+  };
+  struct ProtoCommandLineFlag : PCFProto {
+PCFProto& GetProto();
+  };
+  }  // namespace proto
+)cc";
+} // namespace
+
+static clang::ast_matchers::internal::Matcher
+isOrPointsTo(const DeclarationMatcher &TypeMatcher) {
+  return anyOf(hasDeclaration(TypeMatcher), pointsTo(TypeMatcher));
+}
+
+static std::string format(llvm::StringRef Code) {
+  const std::vector Ranges(1, Range(0, Code.size()));
+  auto Style = format::getLLVMStyle();
+  const auto Replacements = format::reformat(Style, Code, Ranges);
+  auto Formatted = applyAllReplacements(Code, Replacements);
+  if (!Formatted) {
+ADD_FAILURE() << "Could not format code: "
+  << llvm::toString(Formatted.takeError());
+return std::string();
+  }
+  return *Formatted;
+}
+
+void compareSnippets(llvm::StringRef Expected,
+ const llvm::Optional &MaybeActual) {
+  ASSERT_TRUE(MaybeActual) << "Rewrite failed. Expecting: " << Expected;
+  auto Actual = *MaybeActual;
+  std::string HL = "#include \"header.h\"\n";
+  auto I = Actual.find(HL);
+  if (I != std::string::npos) {
+Actual.erase(I, HL.size());
+  }
+  EXPECT_EQ(format(Expected), format(Actual));
+}
+
+// FIXME: consider separating this class into its own file(s).
+class ClangRefactoringTestBase : public testing::Test {
+protected:
+  void appendToHeader(llvm::StringRef S) { FileContents[0].second += S; }
+
+  void addFile(llvm::StringRef Filename, llvm::StringRef Content) {
+FileContents.emplace_back(Filename, Content);
+  }
+
+  llvm::Optional rewrite(llvm::StringRef Input) {
+std::string Code = ("#include \"header.h\"\n" + Input).str();
+auto Factory = newFrontendActionFactory(&MatchFinder);
+if (!runToolOnCodeWithArgs(
+Factory->create(), Code, std::vector(), "input.cc",
+"clang-tool", std::make_shared(),
+FileContents)) {
+  return None;
+}
+auto ChangedCodeOrErr =
+applyAtomicChanges("input.cc", Code, Changes, ApplyChangesSpec());
+if (auto Err = ChangedCodeOrErr.takeError()) {
+  llvm::errs() << "Change failed: " << llvm::toString(std::move(Err))
+   << "\n";
+  return None;
+}
+return *ChangedCodeOrErr;
+  }
+
+  clang::ast_matchers::MatchFinder Mat

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev marked 8 inline comments as done.
anton-afanasyev added inline comments.



Comment at: llvm/lib/Support/TimeProfiler.cpp:33
+switch (C) {
+case '"':
+case '\\':

takuto.ikuta wrote:
> Include escape for '/' too.
> https://tools.ietf.org/html/rfc8259#section-7
Yes, thanks!


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

https://reviews.llvm.org/D58675



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

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:89
+  /// relevant name, not including qualifiers.
+  Name,
+};

ilya-biryukov wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > Same here, what happens to the template arguments and multi-token names, 
> > > e.g.
> > > `operator +` or `foo`?
> > Good point. This seems difficult to get right, since NamedDecl does not 
> > carry sufficient loc data.  However, I've updated the code to explicitly 
> > fail in such cases, so at least we won't have bad rewrites.
> > 
> > BTW, any idea whether constructor initializers can ever be multi token?
> > BTW, any idea whether constructor initializers can ever be multi token?
> 
> Two cases come to mind:
> 1. arbitrary names when initializing base classes,  something like 
> `::ns::X(10)`
> 2. template packs with ellipsis (although ellipsis shouldn't be normally part 
> that we replace, I guess): `Base(10)...`
> 
> Full example:
> ```
> namespace ns {
>   struct X {
> X(int);
>   };
> }
> 
> 
> template 
> struct Y : ns::X, Bases... {
>   Y() : ns::X(10), Bases(10)... {
>   }
> };
> 
> struct Z {
>   Z(int);
> };
> struct W {
>   W(int);
> };
> 
> Y y;
> ```
Turns out the code was already filtering these cases. I added an addition 
constrain of I->isWritten() for initializers. So, only explicit initialization 
of fields is allowed, and therefore I'd venture guaranteed to be a single 
token. I noticed that Kythe seems to make the same assumption.  

That said, I could change to code to specify the range as a char-range of 
`getMemberLoc(), getLParenLoc()` if we can't rely on that (single-token) 
guarantee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Anton Afanasyev via Phabricator via cfe-commits]

anton-afanasyev updated this revision to Diff 192268.
anton-afanasyev marked an inline comment as done.

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

https://reviews.llvm.org/D58675

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Parse/ParseAST.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/GlobalModuleIndex.cpp
  clang/tools/driver/cc1_main.cpp
  llvm/include/llvm/Support/TimeProfiler.h
  llvm/lib/IR/LegacyPassManager.cpp
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/TimeProfiler.cpp

Index: llvm/lib/Support/TimeProfiler.cpp
===
--- /dev/null
+++ llvm/lib/Support/TimeProfiler.cpp
@@ -0,0 +1,184 @@
+//===-- TimeProfiler.cpp - Hierarchical Time Profiler -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+/// \file Hierarchical time profiler implementation.
+//
+//===--===//
+
+#include "llvm/Support/TimeProfiler.h"
+#include "llvm/Support/FileSystem.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace std::chrono;
+
+namespace llvm {
+
+TimeTraceProfiler *TimeTraceProfilerInstance = nullptr;
+
+static std::string escapeString(StringRef Src) {
+  std::string OS;
+  for (const unsigned char &C: Src) {
+switch (C) {
+case '"':
+case '/':
+case '\\':
+case '\b':
+case '\f':
+case '\n':
+case '\r':
+case '\t':
+  OS += '\\';
+  OS += C;
+  break;
+default:
+  if (isPrint(C)) {
+OS += C;
+  }
+}
+  }
+  return OS;
+}
+
+typedef duration DurationType;
+typedef std::pair NameAndDuration;
+
+struct Entry {
+  time_point Start;
+  DurationType Duration;
+  std::string Name;
+  std::string Detail;
+};
+
+struct TimeTraceProfiler {
+  TimeTraceProfiler() {
+Stack.reserve(8);
+Entries.reserve(128);
+StartTime = steady_clock::now();
+  }
+
+  void begin(std::string Name, llvm::function_ref Detail) {
+Entry E = {steady_clock::now(), {}, Name, Detail()};
+Stack.push_back(std::move(E));
+  }
+
+  void end() {
+assert(!Stack.empty() && "Must call Begin first");
+auto &E = Stack.back();
+E.Duration = steady_clock::now() - E.Start;
+
+// Only include sections longer than 500us.
+if (duration_cast(E.Duration).count() > 500)
+  Entries.emplace_back(E);
+
+// Track total time taken by each "name", but only the topmost levels of
+// them; e.g. if there's a template instantiation that instantiates other
+// templates from within, we only want to add the topmost one. "topmost"
+// happens to be the ones that don't have any currently open entries above
+// itself.
+if (std::find_if(++Stack.rbegin(), Stack.rend(), [&](const Entry &Val) {
+  return Val.Name == E.Name;
+}) == Stack.rend()) {
+  TotalPerName[E.Name] += E.Duration;
+  CountPerName[E.Name]++;
+}
+
+Stack.pop_back();
+  }
+
+  void Write(std::unique_ptr &OS) {
+assert(Stack.empty() &&
+   "All profiler sections should be ended when calling Write");
+
+*OS << "{ \"traceEvents\": [\n";
+
+// Emit all events for the main flame graph.
+for (const auto &E : Entries) {
+  auto StartUs = duration_cast(E.Start - StartTime).count();
+  auto DurUs = duration_cast(E.Duration).count();
+  *OS << "{ \"pid\":1, \"tid\":0, \"ph\":\"X\", \"ts\":" << StartUs
+  << ", \"dur\":" << DurUs << ", \"name\":\""
+  << escapeString(E.Name) << "\", \"args\":{ \"detail\":\""
+  << escapeString(E.Detail) << "\"} },\n";
+}
+
+// Emit totals by section name as additional "thread" events, sorted from
+// longest one.
+int Tid = 1;
+std::vector SortedTotals;
+SortedTotals.reserve(TotalPerName.size());
+for (const auto &E : TotalPerName) {
+  SortedTotals.push_back(E);
+}
+std::sort(SortedTotals.begin(), SortedTotals.end(),
+  [](const NameAndDuration &A, const NameAndDuration &B) {
+return A.second > B.second;
+  });
+for (const auto &E : SortedTotals) {
+  auto DurUs = duration_cast(E.second).count();
+  *OS << "{ \"pid\":1, \"tid\":" << Tid << ", \"ph\":\"X\", \"ts\":" << 0
+  << ", 

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D59008#1442515 , @dblaikie wrote:

> In D59008#1442256 , @t-tye wrote:
>
> > In D59008#1442014 , @dblaikie 
> > wrote:
> >
> > > In D59008#1441903 , @t-tye wrote:
> > >
> > > > LGTM
> > > >
> > > > Do we know the state of split DWARF and DWARF compression for DWARF 5 
> > > > (compared to DWARF 2)?
> > >
> > >
> > > State of them in what sense? Compression is pretty orthogonal to any 
> > > DWARF version - it's more about the container (ELF, etc) you use. Split 
> > > DWARF is non-standardly supported in pre-v5, and I think it's functioning 
> > > in the standards conformant v5 mode too.
> >
> >
> > I had heard mention that at least split DWARF may not be fully supported 
> > yet for DWARF 5 in LLVM. But maybe that is old news:-)
>
>
> Might be - nothing I know of right now. (type units aren't fully supported in 
> DWARFv5 - but that's the only big thing on my list)


Anything having to do with section format and placement ought to be correct at 
this point.  Certainly I see v5 type units coming out in the .debug_info 
section per spec, and the .dwo files look right.  If there's something missing 
please file a bug and CC me.

There was certainly a release where split-dwarf and type units didn't work yet, 
but that's all done now.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59008



___
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

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

As a rule I would prefer flags with positive names, as it's slightly easier to 
read `!isTrivial` than `!isNonTrivial`. And trivially shorter. :-)


Repository:
  rC Clang

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] D57858: [analyzer] Add a new frontend flag to display all checker options

[Daniel Krupp via Phabricator via cfe-commits]

dkrupp added a comment.

@dcoughlin I don't necessarily agree with you.
Let me explain why we think this feature is important.

We should give the users the possibility to list all possibly configurable 
checker options and their meaning.

Many of these options should be possible to be set by the end user to be able 
to fine tune the checker behaviour to match the analyzed code style.
Such examples are:
alpha.clone.CloneChecker:MinimumCloneComplexity
alpha.cplusplus.UninitializedObject:CheckPointeeInitialization
alpha.clone.CloneChecker:ReportNormalClones
alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField
etc.
Actually except for the debug checker options and  
unix.DynamicMemoryModeling:Optimistic all these options are meaningful end-user 
options.

Of course there are the debug checker options which are not interesting for the 
end user, but debug checkers should anyway be ignored by the end-users.

We always wanted to add checker option listing to CodeChecker, however did not 
want to duplicate the option list and option documentation in the CodeChecker 
codebase.
It belongs to the analyzer and actually to the checker implementation.
So from CodeChecker we would like to invoke the "clang -cc1 
-analyzer-checker-option-help" to be able to list these options to the end 
users.

The same feature is available already in clang-tidy: clang-tidy -dump-config

I think it is the responsibility of the end-user to decide what option he may 
want to configure.

I understand you would like to differentiate between "developer" and "end-user" 
options.
What we could do maybe is to indicate in the option explanation that the given 
option is "analyzer-internal", "experimental" or "developer only".

What do you think about that?

In D57858#1432714 , @dcoughlin wrote:

> I'm pretty worried about exposing this flag to end users.
>
> - Almost none of the options you've listed are user facing. Many represent 
> options intended for use by static analyzer developers: debugging options, 
> feature flags, and checkers that were never finished. Others represent 
> mechanisms for build systems to control the behavior of the analyzer. Even 
> these are not meant for end users to interact with but rather for 
> implementers of build systems and IDEs. I don't think end users should have 
> to understand these options to use the analyzer.
> - The help text refers to analyzer implementation details (such as 
> "SymRegion") that users won't have the context or knowledge to understand.
> - The help text also recommends invoking -cc1 directly or through the driver 
> with -Xclang. Neither of these are supported end-user interfaces to the 
> analyzer. Instead, users should use scan-build or another tool (such as 
> CodeChecker) that was designed to be used by humans.





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

https://reviews.llvm.org/D57858



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

[PATCH] D54978: Move the SMT API to LLVM

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho added a comment.

In D54978#1442493 , @aprantl wrote:

> I'm afraid this broke some bots that build with `LLVM_ENABLE_MODULES=1`.
>
> For example:
>
> http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/22411/consoleFull#710926295dd1929ea-7054-4089-b7ef-4624c3781fa4
>
>   Undefined symbols for architecture x86_64:
> "llvm::errs()", referenced from:
> llvm::SMTExpr::dump() const in 
> liblldbDebugserverCommon.a(RNBSocket.cpp.o)
> llvm::SMTSolver::dump() const in 
> liblldbDebugserverCommon.a(RNBSocket.cpp.o)
> llvm::SMTSort::dump() const in 
> liblldbDebugserverCommon.a(RNBSocket.cpp.o)
> llvm::SMTExpr::dump() const in 
> liblldbDebugserverCommon.a(SocketAddress.cpp.o)
> llvm::SMTSolver::dump() const in 
> liblldbDebugserverCommon.a(SocketAddress.cpp.o)
> llvm::SMTSort::dump() const in 
> liblldbDebugserverCommon.a(SocketAddress.cpp.o)
>
>
> Long story short: You can't have an LLVM_DUMP_METHOD defined inline inside of 
> a module.
>
> One way to fix this would be to move the function body of the various
>
>   LLVM_DUMP_METHOD void dump() const { print(llvm::errs()); }
>
>
> functions into .cpp files.


Unfortunately, I was not able to reproduce the bug locally (when I enable 
modules, clang complains about some `std::shared_ptr`), 
however, I just pushed r356994 and I'll keep an eye on the bot.

Thanks for the report.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



___
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 added a comment.

@asmith: Where's the LLVM-side change/review that goes with this, btw?

In D59347#1442970 , @probinson wrote:

> As a rule I would prefer flags with positive names, as it's slightly easier 
> to read `!isTrivial` than `!isNonTrivial`. And trivially shorter. :-)


Fair enough - I was mostly coming at this from the "the patch that was 
committed should be reverted" & then we could haggle over other things, but 
fair point.


Repository:
  rC Clang

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] D59008: [AMDGPU] Switch default dwarf version to 5

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D59008#1442962 , @probinson wrote:

> In D59008#1442515 , @dblaikie wrote:
>
> > In D59008#1442256 , @t-tye wrote:
> >
> > > In D59008#1442014 , @dblaikie 
> > > wrote:
> > >
> > > > In D59008#1441903 , @t-tye 
> > > > wrote:
> > > >
> > > > > LGTM
> > > > >
> > > > > Do we know the state of split DWARF and DWARF compression for DWARF 5 
> > > > > (compared to DWARF 2)?
> > > >
> > > >
> > > > State of them in what sense? Compression is pretty orthogonal to any 
> > > > DWARF version - it's more about the container (ELF, etc) you use. Split 
> > > > DWARF is non-standardly supported in pre-v5, and I think it's 
> > > > functioning in the standards conformant v5 mode too.
> > >
> > >
> > > I had heard mention that at least split DWARF may not be fully supported 
> > > yet for DWARF 5 in LLVM. But maybe that is old news:-)
> >
> >
> > Might be - nothing I know of right now. (type units aren't fully supported 
> > in DWARFv5 - but that's the only big thing on my list)
>
>
> Anything having to do with section format and placement ought to be correct 
> at this point.  Certainly I see v5 type units coming out in the .debug_info 
> section per spec, and the .dwo files look right.  If there's something 
> missing please file a bug and CC me.
>
> There was certainly a release where split-dwarf and type units didn't work 
> yet, but that's all done now.


Ah, thanks for the correction - I thought that was the case, and then I thought 
I'd tested it a couple of weeks ago and seen it wasn't (when I'd expected it 
was)... must've messed up my test, maybe used an old compiler build I had lying 
around.

Good stuff!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59008



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

[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Takuto Ikuta via Phabricator via cfe-commits]

takuto.ikuta added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1352
+
+  llvm::TimeTraceScope TimeScope("Backend", StringRef(""));
+

We don't need explicit StringRef constructor call here. Just passing "" is 
enough.



Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
+llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
 P.Initialize();

Remove StringRef?



Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:131
   NumIdentifierLookupHits() {
+  llvm::TimeTraceScope TimeScope("Module LoadIndex", StringRef(""));
   // Read the global index.

Remove StringRef?



Comment at: clang/lib/Serialization/GlobalModuleIndex.cpp:745
   using namespace llvm;
+  llvm::TimeTraceScope TimeScope("Module WriteIndex", StringRef(""));
 

Remove StringRef?



Comment at: clang/tools/driver/cc1_main.cpp:224
+  {
+llvm::TimeTraceScope TimeScope("ExecuteCompiler", StringRef(""));
+Success = ExecuteCompilerInvocation(Clang.get());

Remove StringRef?



Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+default:
+  if (isPrint(C)) {
+OS += C;

include StringExtras.h for this?


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

https://reviews.llvm.org/D58675



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:15692
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+

It looks like `SmallPtrSet`'s move constructor does actually guarantee to leave 
the source empty; you could probably just assert that.  But I don't think 
there's an algorithmic cost, so this is fine, too.

Please leave an assertion at the bottom of this function that the set is empty.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59670



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

[PATCH] D59426: [PR41010][OpenCL] Allow OpenCL C style vector initialization in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added a reviewer: rsmith.
rjmccall added inline comments.



Comment at: lib/Sema/SemaInit.cpp:1295
+? InitializedEntity::InitializeTemporary(ElemType)
+: Entity;
+

There should at least be a comment here explaining this, but mostly it's been 
so long since I've really understood this code that I'd like Richard to look at 
it.


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

https://reviews.llvm.org/D59426



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

[PATCH] D54978: Move the SMT API to LLVM

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Thanks!

The problem with these methods is that LLVM_DUMP_METHOD forces the function to 
be emitted even if it is not used and Clang modules without local submodule 
visibility will implicitly include all header files in a module, which will 
then cause the missing symbol error.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D54978



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

r357001 - [CodeGen] Delete never used LValueAlign

[Fangrui Song via cfe-commits]

Author: maskray
Date: Tue Mar 26 08:39:45 2019
New Revision: 357001

URL: http://llvm.org/viewvc/llvm-project?rev=357001&view=rev
Log:
[CodeGen] Delete never used LValueAlign

It was added by rC176658 but never used since then.

Modified:
cfe/trunk/lib/CodeGen/CGAtomic.cpp

Modified: cfe/trunk/lib/CodeGen/CGAtomic.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGAtomic.cpp?rev=357001&r1=357000&r2=357001&view=diff
==
--- cfe/trunk/lib/CodeGen/CGAtomic.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGAtomic.cpp Tue Mar 26 08:39:45 2019
@@ -35,7 +35,6 @@ namespace {
 uint64_t ValueSizeInBits;
 CharUnits AtomicAlign;
 CharUnits ValueAlign;
-CharUnits LValueAlign;
 TypeEvaluationKind EvaluationKind;
 bool UseLibcall;
 LValue LVal;
@@ -132,7 +131,6 @@ namespace {
 QualType getAtomicType() const { return AtomicTy; }
 QualType getValueType() const { return ValueTy; }
 CharUnits getAtomicAlignment() const { return AtomicAlign; }
-CharUnits getValueAlignment() const { return ValueAlign; }
 uint64_t getAtomicSizeInBits() const { return AtomicSizeInBits; }
 uint64_t getValueSizeInBits() const { return ValueSizeInBits; }
 TypeEvaluationKind getEvaluationKind() const { return EvaluationKind; }


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

[PATCH] D58236: Make address space conversions a bit stricter.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think C probably requires us to allow this under an explicit cast, but we can 
at least warn about it.  It does not require us to allow this as an implicit 
conversion, and that should be an error.


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

https://reviews.llvm.org/D58236



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:61
+  // Check if any of the superclasses of the class match.
+  for (const auto *SuperClass = Node.getClassInterface()->getSuperClass();
+   SuperClass != nullptr; SuperClass = SuperClass->getSuperClass()) {

Please don't use auto where type is not obvious.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:110
+  Finds invocations of -self on super instances in initializers of subclasses
+  of NSObject and recommends calling a superclass initializer instead.
+

Please enclose NSObject in ``. Probably same for -self if this is language 
construct.



Comment at: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst:6
+
+Finds invocations of -self on super instances (`[super self]`) in initializers
+of subclasses of NSObject and recommends invoking a superclass initializer

Please synchronize with Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

@lebedev.ri where are tests for AST serialization are located ? i didn't find 
them.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Shafik Yaghmour via Phabricator via cfe-commits]

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

In D59665#1442826 , @martong wrote:

> In D59665#1442328 , @shafik wrote:
>
> > @martong your idea does not work b/c default construction 
> > `DeclarationName()` treats it the same as being empty. So `if (!Name)` is 
> > still `true`.
>
>
> And did you try moving the `push_back` to the other scope? I'd expect the the 
> ConflictingDecls to be empty then.


Yes, I did and I think it looks good but I have to run a regression.




Comment at: lib/AST/ASTImporter.cpp:2463
 if (!ConflictingDecls.empty()) {
-  Name = Importer.HandleNameConflict(Name, DC, IDNS,
+  Name = Importer.HandleNameConflict(SearchName, DC, IDNS,
  ConflictingDecls.data(),

martong wrote:
> shafik wrote:
> > a_sidorin wrote:
> > > If I understand correctly, this will replace Name with SearchName causing 
> > > an anonymous enum to be imported as a named a few lines below. It doesn't 
> > > look like a correct behaviour to me.
> > This is correct because either `SearchName` is `Name` or it is the name of 
> > the typedef for the anonymous enum set via `ImportInto(SearchName, 
> > D->getTypedefNameForAnonDecl()->getDeclName())`
> Okay, this is indeed correct. But then we should make a similar change in 
> VisitRecordDecl too (there we do exactly the same with typedefs).
This is fixing a specific bug, so I would want to keep this change specifically 
related to that and I can add a second review for `VisitRecordDecl`


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

https://reviews.llvm.org/D59665



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

[PATCH] D58091: Customize warnings for missing built-in type

[Brian Cain via Phabricator via cfe-commits]

bcain added a comment.

In D58091#1397586 , @jyknight wrote:

> I think this warning (-Wbuiltin-requires-header) doesn't really make sense as 
> its own warning.
>
> We already have two related (on-by-default) warnings.


...

In D58091#1414250 , @bcain wrote:

>


...

> FWIW, I'm satisfied with the fix as proposed here for now and I wouldn't be 
> opposed to following up with an improvement over other warnings in lieu of 
> this warning.
> 
> @jyknight -- James, (or others) care to weigh in on this proposal?

@jdoerfert and @jyknight -- let's please un-stall this review.

James: is this change acceptable or unacceptable as-is?  Could we follow up 
with a change that removed this warning?

Johannes: if it's acceptable to James, let's please submit this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58091



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

[PATCH] D46140: [coroutines] Add std::experimental::task type

[Brian Gesiak via Phabricator via cfe-commits]

modocache closed this revision.
modocache added a comment.

Committed in rL357010 . Apologies, I forgot 
to include the differential revision in the commit message so this diff wasn't 
closed automatically as a result. I'll comment on rL357010 
 with the missing information.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D46140



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

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

[Zola Bridges via Phabricator via cfe-commits]

zbrid created this revision.
zbrid added reviewers: chandlerc, kristof.beyls, aaron.ballman, 
devinj.jeanpierre.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, jfb, aheejin, 
hiraditya, javed.absar, dschuff.
Herald added projects: clang, LLVM.

This is similar to the work Kristof did for ARM here: 
https://reviews.llvm.org/D49072

For now, I have only implemented the version that lowers the intrinsic using an 
LFENCE. I'm workign on a version that can be lowered as an LFENCE or lowered 
using the control flow speculation available, so users have the option just as 
they do in the ARM patch.

This is intended to add to the discussion rather than be a definitive patch 
relating to the way we will handle spot mitigations as far as the final 
API/implementation in LLVM goes. Any comments about the API, the way 
implemented this, or anything else are welcome.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59827

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-speculation-safe-value.c
  clang/test/Preprocessor/init.c
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Target/TargetSelectionDAG.td
  llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll

Index: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
@@ -0,0 +1,71 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+
+; ModuleID = 'hello.cpp'
+source_filename = "hello.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo32i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i32, align 4
+  %b_safe = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  store i32 %mul, i32* %b, align 4
+  %1 = load i32, i32* %b, align 4
+  %2 = call i32 @llvm.speculationsafevalue.i32(i32 %1)
+; X64: movl -12(%rbp), %eax
+; X64: lfence
+; X64: movl %eax, -8(%rbp)
+  store i32 %2, i32* %b_safe, align 4
+  %3 = load i32, i32* %b_safe, align 4
+  %add = add nsw i32 %3, 100
+  store i32 %add, i32* %c, align 4
+  %4 = load i32, i32* %c, align 4
+  ret i32 %4
+}
+
+; Function Attrs: nounwind
+declare i32 @llvm.speculationsafevalue.i32(i32) #1
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo64i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i64, align 8
+  %b_safe = alloca i64, align 8
+  %c = alloca i64, align 8
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  %conv = sext i32 %mul to i64
+  store i64 %conv, i64* %b, align 8
+  %1 = load i64, i64* %b, align 8
+  %2 = call i64 @llvm.speculationsafevalue.i64(i64 %1)
+; X64: movq -32(%rbp), %rax
+; X64: lfence
+; X64: movq %rax, -24(%rbp)
+  store i64 %2, i64* %b_safe, align 8
+  %3 = load i64, i64* %b_safe, align 8
+  %add = add nsw i64 %3, 100
+  store i64 %add, i64* %c, align 8
+  %4 = load i64, i64* %c, align 8
+  %conv1 = trunc i64 %4 to i32
+  ret i32 %conv1
+}
+
+; Function Attrs: nounwind
+declare i64 @llvm.speculationsafevalue.i64(i64) #1
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 6fd90b5505fe7cddd0fd798fe9608ea0e0325302)"}
Index: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
==

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: clang/test/Preprocessor/init.c:9678
 // WEBASSEMBLY-NEXT:#define __GXX_ABI_VERSION 1002
+// WEBASSEMBLY-NEXT: #define __HAVE_SPECULATION_SAFE_VALUE 1
 // WEBASSEMBLY32-NEXT:#define __ILP32__ 1

Nit: Remove the whitespace to be consistent with adjacent lines? (I think 
having a whitespace is better in general though)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59827



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

[PATCH] D59827: [slh] x86 impl of ARM instrinsic for SLH

[Zola Bridges via Phabricator via cfe-commits]

zbrid updated this revision to Diff 192297.
zbrid added a comment.
Herald added a subscriber: jsji.

update whitespace in wasm file to match surrounding


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59827

Files:
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtin-speculation-safe-value.c
  clang/test/Preprocessor/init.c
  llvm/include/llvm/CodeGen/ISDOpcodes.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/include/llvm/Target/TargetSelectionDAG.td
  llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp
  llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGDumper.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrInfo.td
  llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
  llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll

Index: llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/speculative-load-hardening-intrinsic.ll
@@ -0,0 +1,71 @@
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu | FileCheck %s --check-prefix=X64
+
+; ModuleID = 'hello.cpp'
+source_filename = "hello.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo32i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i32, align 4
+  %b_safe = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  store i32 %mul, i32* %b, align 4
+  %1 = load i32, i32* %b, align 4
+  %2 = call i32 @llvm.speculationsafevalue.i32(i32 %1)
+; X64: movl -12(%rbp), %eax
+; X64: lfence
+; X64: movl %eax, -8(%rbp)
+  store i32 %2, i32* %b_safe, align 4
+  %3 = load i32, i32* %b_safe, align 4
+  %add = add nsw i32 %3, 100
+  store i32 %add, i32* %c, align 4
+  %4 = load i32, i32* %c, align 4
+  ret i32 %4
+}
+
+; Function Attrs: nounwind
+declare i32 @llvm.speculationsafevalue.i32(i32) #1
+
+; Function Attrs: noinline nounwind optnone uwtable
+define dso_local i32 @_Z5foo64i(i32 %a) #0 {
+entry:
+  %a.addr = alloca i32, align 4
+  %b = alloca i64, align 8
+  %b_safe = alloca i64, align 8
+  %c = alloca i64, align 8
+  store i32 %a, i32* %a.addr, align 4
+  %0 = load i32, i32* %a.addr, align 4
+  %mul = mul nsw i32 %0, 100
+  %conv = sext i32 %mul to i64
+  store i64 %conv, i64* %b, align 8
+  %1 = load i64, i64* %b, align 8
+  %2 = call i64 @llvm.speculationsafevalue.i64(i64 %1)
+; X64: movq -32(%rbp), %rax
+; X64: lfence
+; X64: movq %rax, -24(%rbp)
+  store i64 %2, i64* %b_safe, align 8
+  %3 = load i64, i64* %b_safe, align 8
+  %add = add nsw i64 %3, 100
+  store i64 %add, i64* %c, align 8
+  %4 = load i64, i64* %c, align 8
+  %conv1 = trunc i64 %4 to i32
+  ret i32 %conv1
+}
+
+; Function Attrs: nounwind
+declare i64 @llvm.speculationsafevalue.i64(i64) #1
+
+attributes #0 = { noinline nounwind optnone uwtable "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "min-legal-vector-width"="0" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+fxsr,+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+attributes #1 = { nounwind }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 9.0.0 (https://github.com/llvm/llvm-project.git 6fd90b5505fe7cddd0fd798fe9608ea0e0325302)"}
Index: llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
===
--- llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
+++ llvm/lib/Target/X86/X86SpeculativeLoadHardening.cpp
@@ -212,6 +212,7 @@
   void hardenIndirectCallOrJumpInstr(
   MachineInstr &MI,
   SmallDenseMap &AddrRegToHardenedReg);
+  bool lowerIntrinsic(MachineFunction &MF);
 };
 
 } // end anonymous namespace
@@ -402,16 +403,19 @@
   LLVM_DEBUG(dbgs() << "** " << getPassName() << " : " << MF.getName()
 << " **\n");
 
-  // Only run if this pass is forced enabled or we detect the relevant function
-  // attribute requesting SLH.
-  if (!EnableSpeculativeLoadHardening &&
-  !MF.getFunction().hasFnAt

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return-type check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:274
+
+  if (F->getLocation().isInvalid())
+return;

bernhardmgruber wrote:
> aaron.ballman wrote:
> > bernhardmgruber wrote:
> > > aaron.ballman wrote:
> > > > Should this also bail out if the function is `main()`?
> > > How strange does
> > > 
> > > ```
> > > auto main(int argc, const char* argv[]) -> int {
> > > return 0;
> > > }
> > > ```
> > > look to you?
> > > 
> > > I think the transformation is valid here. But I can understand that 
> > > people feel uneasy after typing `int main ...` for decades. Should we 
> > > also create an option here to turn it off for main? Or just not transform 
> > > it? I do not mind. If I want `main` to start with `auto` I could also do 
> > > this transformation manually.
> > This comment was marked as done, but I don't see any changes or mention of 
> > what should happen. I suppose the more general question is: should there be 
> > a list of functions that should not have this transformation applied, like 
> > program entrypoints? Or do you want to see this check diagnose those 
> > functions as well?
> I am sorry for marking it as done. I do not know how people work here exactly 
> and how phabricator behaves. I thought the check boxes are handled for 
> everyone individually. Also, if I add a new comment, it is checked by default.
> 
> How are you/most people going to use clang-tidy? Do you run it regularly and 
> automatic? Do you expect it to find zero issues once you applied the check?
> In other words, if you do not want to transform some functions, do you need 
> an option to disable the check for these, so it runs clean on the full source 
> code?
> 
> Personally, I do not need this behavior, as I run clang-tidy manually once in 
> a while and revert transformations I do not want before checking in the 
> changes.
> I am sorry for marking it as done. I do not know how people work here exactly 
> and how phabricator behaves. I thought the check boxes are handled for 
> everyone individually. Also, if I add a new comment, it is checked by default.

No worries -- that new comments are automatically marked done by default is 
certainly a surprise to me!

> How are you/most people going to use clang-tidy? Do you run it regularly and 
> automatic? Do you expect it to find zero issues once you applied the check? 
> In other words, if you do not want to transform some functions, do you need 
> an option to disable the check for these, so it runs clean on the full source 
> code?

I think it's hard to gauge how most people do anything, really. However, I 
think there are people who enable certain clang-tidy checks and have them run 
automatically as part of CI, etc. Those kind of folks may need the ability to 
silence the diagnostics. We could do this in a few ways (options to control 
methods not to diagnose, NOLINT comments, etc).

I kind of think we don't need an option for the user to list functions not to 
transform. They can use NOLINT to cover those situations as a one-off. The only 
situation where I wonder if everyone is going to want to write NOLINT is for 
`main()`. It might make sense to have an option to not check program 
entrypoints, but there is technically nothing stopping someone from using a 
trailing return type with a program entrypoint so maybe this option isn't 
needed at all.

How about we not add any options and if someone files a bug report, we can 
address it then?



Comment at: clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp:180-184
+  if (Info.hasMacroDefinition()) {
+// The CV qualifiers of the return type are inside macros.
+diag(F.getLocation(), Message);
+return {};
+  }

bernhardmgruber wrote:
> aaron.ballman wrote:
> > Perhaps I'm a bit dense on a Monday morning, but why should this be a 
> > diagnostic? I am worried this will diagnose situations like (untested 
> > guesses):
> > ```
> > #define const const
> > const int f(void); // Why no fixit?
> > 
> > #define attr __attribute__((frobble))
> > attr void g(void); // Why diagnosed as needing a trailing return type?
> > ```
> Because I would also like to rewrite functions which contain macros in the 
> return type. However, I cannot provide a fixit in all cases. Clang can give 
> me a `SourceRange` without CV qualifiers which seems to work in all my tests 
> so far. But to include CV qualifiers I have to do some manual lexing left and 
> right of the return type `SourceRange`. If I encounter macros along this way, 
> I bail out because handling these cases becomes compilated very easily.
> 
> Your second case does not give a diagnostic, as it is not matched by the 
> check, because it returns `void`.
> Because I would also like to rewrite functions which contain macros in the 
> return type. However, I cannot provide a fixit in all cases. Clang can give 
> me a SourceRange 

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia closed this revision.
Anastasia added a comment.

Committed in r356987.


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

https://reviews.llvm.org/D59492



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 192300.
erik.pilkington marked an inline comment as done.
erik.pilkington added a comment.

Add an assert.


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

https://reviews.llvm.org/D59670

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/blocks.cpp


Index: clang/test/SemaCXX/blocks.cpp
===
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -145,3 +145,11 @@
 A::foo(); });
   }
 }
+
+namespace test7 {
+struct S {};
+void f() {
+  constexpr S s;
+  auto some_block = ^{ (void)s; };
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15690,7 +15690,12 @@
 }
 
 void Sema::CleanupVarDeclMarking() {
-  for (Expr *E : MaybeODRUseExprs) {
+  // Iterate through a local copy in case MarkVarDeclODRUsed makes a recursive
+  // call.
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+
+  for (Expr *E : LocalMaybeODRUseExprs) {
 VarDecl *Var;
 SourceLocation Loc;
 if (DeclRefExpr *DRE = dyn_cast(E)) {
@@ -15707,10 +15712,10 @@
/*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
 
-  MaybeODRUseExprs.clear();
+  assert(MaybeODRUseExprs.empty() &&
+ "MarkVarDeclODRUsed failed to cleanup MaybeODRUseExprs?");
 }
 
-
 static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
 VarDecl *Var, Expr *E) {
   assert((!E || isa(E) || isa(E)) &&
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -587,13 +587,13 @@
   /// element type here is ExprWithCleanups::Object.
   SmallVector ExprCleanupObjects;
 
-  /// Store a list of either DeclRefExprs or MemberExprs
-  ///  that contain a reference to a variable (constant) that may or may not
-  ///  be odr-used in this Expr, and we won't know until all lvalue-to-rvalue
-  ///  and discarded value conversions have been applied to all subexpressions
-  ///  of the enclosing full expression.  This is cleared at the end of each
-  ///  full expression.
-  llvm::SmallPtrSet MaybeODRUseExprs;
+  /// Store a set of either DeclRefExprs or MemberExprs that contain a 
reference
+  /// to a variable (constant) that may or may not be odr-used in this Expr, 
and
+  /// we won't know until all lvalue-to-rvalue and discarded value conversions
+  /// have been applied to all subexpressions of the enclosing full expression.
+  /// This is cleared at the end of each full expression.
+  using MaybeODRUseExprSet = llvm::SmallPtrSet;
+  MaybeODRUseExprSet MaybeODRUseExprs;
 
   std::unique_ptr PreallocatedFunctionScope;
 
@@ -1029,7 +1029,7 @@
 /// context (i.e. the number of TypoExprs created).
 unsigned NumTypos;
 
-llvm::SmallPtrSet SavedMaybeODRUseExprs;
+MaybeODRUseExprSet SavedMaybeODRUseExprs;
 
 /// The lambdas that are present within this context, if it
 /// is indeed an unevaluated context.


Index: clang/test/SemaCXX/blocks.cpp
===
--- clang/test/SemaCXX/blocks.cpp
+++ clang/test/SemaCXX/blocks.cpp
@@ -145,3 +145,11 @@
 A::foo(); });
   }
 }
+
+namespace test7 {
+struct S {};
+void f() {
+  constexpr S s;
+  auto some_block = ^{ (void)s; };
+}
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15690,7 +15690,12 @@
 }
 
 void Sema::CleanupVarDeclMarking() {
-  for (Expr *E : MaybeODRUseExprs) {
+  // Iterate through a local copy in case MarkVarDeclODRUsed makes a recursive
+  // call.
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+
+  for (Expr *E : LocalMaybeODRUseExprs) {
 VarDecl *Var;
 SourceLocation Loc;
 if (DeclRefExpr *DRE = dyn_cast(E)) {
@@ -15707,10 +15712,10 @@
/*MaxFunctionScopeIndex Pointer*/ nullptr);
   }
 
-  MaybeODRUseExprs.clear();
+  assert(MaybeODRUseExprs.empty() &&
+ "MarkVarDeclODRUsed failed to cleanup MaybeODRUseExprs?");
 }
 
-
 static void DoMarkVarDeclReferenced(Sema &SemaRef, SourceLocation Loc,
 VarDecl *Var, Expr *E) {
   assert((!E || isa(E) || isa(E)) &&
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -587,13 +587,13 @@
   /// element type here is ExprWithCleanups::Object.
   SmallVector ExprCleanupObjects;
 
-  /// Store a list of either DeclRefExprs or MemberExprs
-  ///  that contain a refer

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:15692
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+

rjmccall wrote:
> It looks like `SmallPtrSet`'s move constructor does actually guarantee to 
> leave the source empty; you could probably just assert that.  But I don't 
> think there's an algorithmic cost, so this is fine, too.
> 
> Please leave an assertion at the bottom of this function that the set is 
> empty.
If you have to actually check the constructor to verify the client code is 
doing the right thing then it makes more sense to just be explicit, IMO. New 
patch adds the assert though.


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

https://reviews.llvm.org/D59670



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

[PATCH] D58659: [Sema] Fix assertion when `auto` parameter in lambda has an attribute.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/lib/Sema/SemaType.cpp:261
+/// necessary.
+QualType ReplaceAutoType(QualType TypeWithAuto, QualType Replacement) {
+  QualType T = sema.ReplaceAutoType(TypeWithAuto, Replacement);

vsapsai wrote:
> aaron.ballman wrote:
> > I think this work should be done within `SubstituteDeducedTypeTransform` 
> > rather than on the side. Any caller to `Sema::ReplaceAutoType()` should get 
> > this same behavior.
> Doing this work in `SubstituteDeducedTypeTransform` involves teaching 
> `SubstituteDeducedTypeTransform` about `TypeProcessingState` and probably 
> adding `TypeProcessingState` to `Sema::ReplaceAutoType()`. As for me, it 
> exposes `TypeProcessingState` in more places than necessary. And it feels 
> somewhat awkward that `SubstituteDeducedTypeTransform` is used in multiple 
> places but `TypeProcessingState` is required only here.
> 
> I've modelled my approach after `TypeProcessingState::getAttributedType` 
> where it forwards the call to Sema and keeps `AttrsForTypes` up to date. Do 
> you still think `SubstituteDeducedTypeTransform` would be a better place for 
> this code?
Hmm, yeah, it doesn't seem like it would make sense there. This is keeping the 
`TypeProcessingState` up to date which is really only of interest during some 
stages of processing.


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

https://reviews.llvm.org/D58659



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

[PATCH] D59670: [Sema] Fix an assert when a block captures a constexpr local

[John McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExpr.cpp:15692
+  MaybeODRUseExprSet LocalMaybeODRUseExprs;
+  std::swap(LocalMaybeODRUseExprs, MaybeODRUseExprs);
+

erik.pilkington wrote:
> rjmccall wrote:
> > It looks like `SmallPtrSet`'s move constructor does actually guarantee to 
> > leave the source empty; you could probably just assert that.  But I don't 
> > think there's an algorithmic cost, so this is fine, too.
> > 
> > Please leave an assertion at the bottom of this function that the set is 
> > empty.
> If you have to actually check the constructor to verify the client code is 
> doing the right thing then it makes more sense to just be explicit, IMO. New 
> patch adds the assert though.
If it were perf-significant I might disagree, but it shouldn't be, so this is 
fine.  LGTM.


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

https://reviews.llvm.org/D59670



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58797#1439888 , @erik.pilkington 
wrote:

> Ah, I didn't consider that case. Presumably `st` is configured to have 
> different sizes based on the target? I agree that this is a false-positive, 
> but it seems like a pretty narrow edge case, and there is a pretty obvious 
> source workaround that doesn't affect readability: `memcpy(&buf, st, 
> sizeof(buf))`. @aaron.ballman/@rsmith Any thoughts here? IMO keeping this 
> diagnostic is worth it.


Yes, I think we should keep this diagnostic. However, if we can find a way to 
silence it for this particular false-positive pattern, that would be great!


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Should this check also try to find this pattern:

  if (dyn_cast(Bobble))
...

in order to recommend the proper replacement:

  if (isa(Bobble))
...

I ask because the name `llvm-avoid-cast-in-conditional` sounds like it would 
also cover this situation and I run into it during code reviews with some 
frequency (more often than I run into `cast<>` being used in a conditional).




Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:20
+void AvoidCastInConditionalCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  ifStmt(anyOf(

No need to register this matcher if C++ isn't enabled.



Comment at: 
clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:35-36
+diag(MatchedDecl->getBeginLoc(),
+ "Found cast<> in conditional statement; consider using dyn_cast<> if "
+ "it can fail, or removal of conditional if it can't.");
+;

clang-tidy diagnostics should not be complete sentences; you should make this 
ungrammatical. How about: `cast<> in conditional will assert rather than return 
a null pointer`?



Comment at: clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp:37
+ "it can fail, or removal of conditional if it can't.");
+;
+  }

Spurious semicolon?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst:8
+statements which will assert on failure in Debug builds. The use of
+cast<> implies that the operation cannot fail, and should not be used
+as the condition of a conditional statement.

This cast<> still needs to be enclosed in ``.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57
+/// \endcode
+AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf,
+  ast_matchers::internal::Matcher,

This matcher seems like it would be generally useful -- would you mind adding 
it to the AST matcher interface rather than local to this check? It doesn't 
have to be done as part of this patch (we can leave the matcher here for the 
moment).



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110
+   "invoke a superclass initializer?")
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),

Is there a %0 missing from the diagnostic for this method declaration you're 
passing in?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

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

Removed usage of `auto` with a nonobvious type in isSubclassOf matcher.
Added backticks around `NSObject` in docs.
Synchronized check description in release notes and check documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of -self on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of -self on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.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_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initial

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

For that particular case, I think we could suppress the false positive using 
DiagRuntimeBehavior.  How many total false positives are we talking about, and 
how many can the compiler statically prove are unreachable?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192315.
stephanemoore marked 5 inline comments as done.
stephanemoore added a comment.

Fix diagnostic format string to actually use the message's method declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of -self on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of -self on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.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_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore updated this revision to Diff 192316.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.

Enclose -self in backticks in release notes and check documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of `-self` on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking -self on super instances in initializers is a common programmer error
+when the programmer's original intent is to call a superclass initializer.
+Failing to call a superclass initializer breaks initializer chaining and can
+result in invalid object initialization.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.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_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+///

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

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

Enclose -self in backticks in check documentation (overlooked in previous 
attempt).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of `-self` on super instances in initializers of subclasses of
+`NSObject` and recommends calling a superclass initializer instead.
+
+Invoking `-self` on super instances in initializers is a common programmer
+error when the programmer's original intent is to call a superclass
+initializer. Failing to call a superclass initializer breaks initializer
+chaining and can result in invalid object initialization.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,11 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.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_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/c

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore added inline comments.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:57
+/// \endcode
+AST_MATCHER_P(ObjCImplementationDecl, isSubclassOf,
+  ast_matchers::internal::Matcher,

aaron.ballman wrote:
> This matcher seems like it would be generally useful -- would you mind adding 
> it to the AST matcher interface rather than local to this check? It doesn't 
> have to be done as part of this patch (we can leave the matcher here for the 
> moment).
Definitely agreed. I will send out a followup for a new AST matcher.



Comment at: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp:110
+   "invoke a superclass initializer?")
+  << Message->getMethodDecl()
+  << FixItHint::CreateReplacement(Message->getSourceRange(),

aaron.ballman wrote:
> Is there a %0 missing from the diagnostic for this method declaration you're 
> passing in?
Good catch! That's egg on my face 😅🥚

I think I was internally conflicted over specifically mentioning -[NSObject 
self] versus using the method declaration of the message expression and somehow 
got stuck halfway in-between 😓 I think it's better to mention the method 
declaration of the message. Let me know if you think that I should instead 
mention -[NSObject self].



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:110
+  Finds invocations of -self on super instances in initializers of subclasses
+  of NSObject and recommends calling a superclass initializer instead.
+

Eugene.Zelenko wrote:
> Please enclose NSObject in ``. Probably same for -self if this is language 
> construct.
Good catch. Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59646: [PR40778][PR41157][OpenCL] Prevent implicit initialization of local address space objects

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:11648-11651
+// In OpenCL we don't allow to initialize objects in local address space.
+if (getLangOpts().OpenCL &&
+Var->getType().getAddressSpace() == LangAS::opencl_local)
+  return;

Anastasia wrote:
> bader wrote:
> > Shouldn't we invalidate Var declaration?
> This early exit is to prevent adding default initializer implicitly to the 
> declarations that are valid.
"In OpenCL, we can't initialize objects in the __local address space, even 
implicitly, so don't synthesize an implicit initializer."

I think it's important to add the underscores on `__local` to make it obvious 
that we're talking about OpenCL's "local" address space, not the address space 
that local variables appear in.


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

https://reviews.llvm.org/D59646



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443340 , @aaron.ballman 
wrote:

> Should this check also try to find this pattern:
>
>   if (dyn_cast(Bobble))
> ...
>
>
> in order to recommend the proper replacement:
>
>   if (isa(Bobble))
> ...
>
>
> I ask because the name `llvm-avoid-cast-in-conditional` sounds like it would 
> also cover this situation and I run into it during code reviews with some 
> frequency (more often than I run into `cast<>` being used in a conditional).


Yes, I can add that, and provide a fix-it too.  Thanks...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59838: gn build: Add build files for clang-include-fixer and find-all-symbols

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: mbonadei.
Herald added a project: LLVM.

(Not adding a build file for clang-include-fixer/plugin yet.)


https://reviews.llvm.org/D59838

Files:
  llvm/utils/gn/secondary/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
  
llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
  
llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn

Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/tool/BUILD.gn
@@ -0,0 +1,18 @@
+executable("clang-include-fixer") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer",
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Rewrite",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"ClangIncludeFixer.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/tool/BUILD.gn
@@ -0,0 +1,17 @@
+executable("find-all-symbols") {
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+  ]
+  include_dirs = [ ".." ]
+  sources = [
+"FindAllSymbolsMain.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/find-all-symbols/BUILD.gn
@@ -0,0 +1,23 @@
+static_library("find-all-symbols") {
+  output_name = "findAllSymbols"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang/lib/AST",
+"//clang/lib/ASTMatchers",
+"//clang/lib/Basic",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Tooling",
+"//llvm/lib/Support",
+  ]
+  sources = [
+"FindAllMacros.cpp",
+"FindAllSymbols.cpp",
+"FindAllSymbolsAction.cpp",
+"HeaderMapCollector.cpp",
+"PathConfig.cpp",
+"PragmaCommentHandler.cpp",
+"STLPostfixHeaderMap.cpp",
+"SymbolInfo.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
===
--- /dev/null
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-include-fixer/BUILD.gn
@@ -0,0 +1,26 @@
+static_library("clang-include-fixer") {
+  output_name = "clangIncludeFixer"
+  configs += [ "//llvm/utils/gn/build:clang_code" ]
+  deps = [
+"//clang-tools-extra/clang-include-fixer/find-all-symbols",
+"//clang/lib/AST",
+"//clang/lib/Basic",
+"//clang/lib/Format",
+"//clang/lib/Frontend",
+"//clang/lib/Lex",
+"//clang/lib/Parse",
+"//clang/lib/Sema",
+"//clang/lib/Serialization",
+"//clang/lib/Tooling",
+"//clang/lib/Tooling/Core",
+"//llvm/lib/Support",
+  ]
+  sources = [
+"FuzzySymbolIndex.cpp",
+"InMemorySymbolIndex.cpp",
+"IncludeFixer.cpp",
+"IncludeFixerContext.cpp",
+"SymbolIndexManager.cpp",
+"YamlSymbolIndex.cpp",
+  ]
+}
Index: llvm/utils/gn/secondary/BUILD.gn
===
--- llvm/utils/gn/secondary/BUILD.gn
+++ llvm/utils/gn/secondary/BUILD.gn
@@ -7,6 +7,8 @@
 "//clang-tools-extra/clang-apply-replacements/tool:clang-apply-replacements",
 "//clang-tools-extra/clang-change-namespace/tool:clang-change-namespace",
 "//clang-tools-extra/clang-doc/tool:clang-doc",
+"//clang-tools-extra/clang-include-fixer/find-all-symbols/tool:find-all-symbols",
+"//clang-tools-extra/clang-include-fixer/tool:clang-include-fixer",
 "//clang-tools-extra/clang-move/tool:clang-move",
 "//clang-tools-extra/clang-query/tool:clang-query",
 "//clang-tools-extra/clang-reorder-fields/tool:clang-reorder-fields",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.

Please use double backticks for language constructs. Same in other places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443451 , @hintonda wrote:

> In D59802#1443340 , @aaron.ballman 
> wrote:
>
> > Should this check also try to find this pattern:
> >
> >   if (dyn_cast(Bobble))
> > ...
> >
> >
> > in order to recommend the proper replacement:
> >
> >   if (isa(Bobble))
> > ...
> >
> >
> > I ask because the name `llvm-avoid-cast-in-conditional` sounds like it 
> > would also cover this situation and I run into it during code reviews with 
> > some frequency (more often than I run into `cast<>` being used in a 
> > conditional).
>
>
> Yes, I can add that, and provide a fix-it too.  Thanks...


I did a quick grep and found a few of these, but also found the `_or_null<>` 
variety.  Guess they'll need to stay the same since `isa<>` can't handle nulls.

Btw, I also found the same pattern used for `while()`, so I'll add that too.  
Here's a sample of the patterns I'm seeing:

./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
while (dyn_cast(last_stmt)) {
./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
(dyn_cast_or_null(D)) . // <--- this one's okay
./clang/lib/CodeGen/CodeGenModule.cpp:3950:if 
(dyn_cast(callSite)) {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59802#1443515 , @hintonda wrote:

> In D59802#1443451 , @hintonda wrote:
>
> > In D59802#1443340 , @aaron.ballman 
> > wrote:
> >
> > > Should this check also try to find this pattern:
> > >
> > >   if (dyn_cast(Bobble))
> > > ...
> > >
> > >
> > > in order to recommend the proper replacement:
> > >
> > >   if (isa(Bobble))
> > > ...
> > >
> > >
> > > I ask because the name `llvm-avoid-cast-in-conditional` sounds like it 
> > > would also cover this situation and I run into it during code reviews 
> > > with some frequency (more often than I run into `cast<>` being used in a 
> > > conditional).
> >
> >
> > Yes, I can add that, and provide a fix-it too.  Thanks...
>
>
> I did a quick grep and found a few of these, but also found the `_or_null<>` 
> variety.  Guess they'll need to stay the same since `isa<>` can't handle 
> nulls.


Would it make sense to transform `if (dyn_cast_or_null(Obj))` into `if (Obj 
&& isa(Obj))`  or are there bad transformations from that?

> Btw, I also found the same pattern used for `while()`, so I'll add that too.  
> Here's a sample of the patterns I'm seeing:
> 
> ./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
> while (dyn_cast(last_stmt)) {

Hah, good catch!

> ./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
> (dyn_cast_or_null(D)) . // <--- this one's okay

I think this could be expressed as `if (D && isa(D))`, no?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

r357027 - [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via cfe-commits]

Author: rdwampler
Date: Tue Mar 26 13:18:14 2019
New Revision: 357027

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

Summary:
This option `AllowShortLambdasOnASingleLine` similar to the other `AllowShort*` 
options, but applied to C++ lambdas.

Reviewers: djasper, klimek

Reviewed By: klimek

Subscribers: MyDeveloperDay, cfe-commits

Tags: #clang

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

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

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=357027&r1=357026&r2=357027&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Tue Mar 26 13:18:14 2019
@@ -449,6 +449,45 @@ the configuration (without a prefix: ``A
   return;
}
 
+**AllowShortLambdasOnASingleLine** (``ShortLambdaStyle``)
+  Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  single line.
+
+  Possible values:
+
+  * ``SLS_None`` (in configuration: ``None``)
+Never merge lambdas into a single line.
+
+  * ``SLS_Empty`` (in configuration: ``Empty``)
+Only merge empty lambdas.
+
+.. code-block:: c++
+
+  auto lambda = [](int a) {}
+  auto lambda2 = [](int a) {
+  return a;
+  };
+
+  * ``SLS_Inline`` (in configuration: ``Inline``)
+Merge lambda into a single line if argument of a function.
+
+.. code-block:: c++
+
+  auto lambda = [](int a) {
+  return a;
+  };
+  sort(a.begin(), a.end(), ()[] { return x < y; })
+
+  * ``SLS_All`` (in configuration: ``All``)
+Merge all lambdas fitting on a single line.
+
+.. code-block:: c++
+
+  auto lambda = [](int a) {}
+  auto lambda2 = [](int a) { return a; };
+
+
+
 **AllowShortLoopsOnASingleLine** (``bool``)
   If ``true``, ``while (true) continue;`` can be put on a single
   line.

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=357027&r1=357026&r2=357027&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Tue Mar 26 13:18:14 2019
@@ -306,6 +306,39 @@ struct FormatStyle {
   /// If ``true``, ``if (a) return;`` can be put on a single line.
   ShortIfStyle AllowShortIfStatementsOnASingleLine;
 
+  /// Different styles for merging short lambdas containing at most one
+  /// statement.
+  enum ShortLambdaStyle {
+/// Never merge lambdas into a single line.
+SLS_None,
+/// Only merge empty lambdas.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) {
+///   return a;
+///   };
+/// \endcode
+SLS_Empty,
+/// Merge lambda into a single line if argument of a function.
+/// \code
+///   auto lambda = [](int a) {
+///   return a;
+///   };
+///   sort(a.begin(), a.end(), ()[] { return x < y; })
+/// \endcode
+SLS_Inline,
+/// Merge all lambdas fitting on a single line.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) { return a; };
+/// \endcode
+SLS_All,
+  };
+
+  /// Dependent on the value, ``auto lambda []() { return 0; }`` can be put on 
a
+  /// single line.
+  ShortLambdaStyle AllowShortLambdasOnASingleLine;
+
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.
   bool AllowShortLoopsOnASingleLine;
@@ -1805,6 +1838,7 @@ struct FormatStyle {
R.AllowShortFunctionsOnASingleLine &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
+   AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine 
&&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=357027&r1=357026&r2=357027&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Tue Mar 26 13:18:14 2019
@@ -119,6 +119,17 @@ template <> struct ScalarEnumerationTrai
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO 

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL357027: [clang-format] Add style option 
AllowShortLambdasOnASingleLine (authored by rdwampler, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57687?vs=191872&id=192337#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57687

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

Index: cfe/trunk/include/clang/Format/Format.h
===
--- cfe/trunk/include/clang/Format/Format.h
+++ cfe/trunk/include/clang/Format/Format.h
@@ -306,6 +306,39 @@
   /// If ``true``, ``if (a) return;`` can be put on a single line.
   ShortIfStyle AllowShortIfStatementsOnASingleLine;
 
+  /// Different styles for merging short lambdas containing at most one
+  /// statement.
+  enum ShortLambdaStyle {
+/// Never merge lambdas into a single line.
+SLS_None,
+/// Only merge empty lambdas.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) {
+///   return a;
+///   };
+/// \endcode
+SLS_Empty,
+/// Merge lambda into a single line if argument of a function.
+/// \code
+///   auto lambda = [](int a) {
+///   return a;
+///   };
+///   sort(a.begin(), a.end(), ()[] { return x < y; })
+/// \endcode
+SLS_Inline,
+/// Merge all lambdas fitting on a single line.
+/// \code
+///   auto lambda = [](int a) {}
+///   auto lambda2 = [](int a) { return a; };
+/// \endcode
+SLS_All,
+  };
+
+  /// Dependent on the value, ``auto lambda []() { return 0; }`` can be put on a
+  /// single line.
+  ShortLambdaStyle AllowShortLambdasOnASingleLine;
+
   /// If ``true``, ``while (true) continue;`` can be put on a single
   /// line.
   bool AllowShortLoopsOnASingleLine;
@@ -1805,6 +1838,7 @@
R.AllowShortFunctionsOnASingleLine &&
AllowShortIfStatementsOnASingleLine ==
R.AllowShortIfStatementsOnASingleLine &&
+   AllowShortLambdasOnASingleLine == R.AllowShortLambdasOnASingleLine &&
AllowShortLoopsOnASingleLine == R.AllowShortLoopsOnASingleLine &&
AlwaysBreakAfterReturnType == R.AlwaysBreakAfterReturnType &&
AlwaysBreakBeforeMultilineStrings ==
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -65,6 +65,7 @@
   TYPE(JsTypeOperator) \
   TYPE(JsTypeOptionalQuestion) \
   TYPE(LambdaArrow)\
+  TYPE(LambdaLBrace)   \
   TYPE(LambdaLSquare)  \
   TYPE(LeadingJavaAnnotation)  \
   TYPE(LineComment)\
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -119,6 +119,17 @@
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO &IO, FormatStyle::ShortLambdaStyle &Value) {
+IO.enumCase(Value, "None", FormatStyle::SLS_None);
+IO.enumCase(Value, "false", FormatStyle::SLS_None);
+IO.enumCase(Value, "Empty", FormatStyle::SLS_Empty);
+IO.enumCase(Value, "Inline", FormatStyle::SLS_Inline);
+IO.enumCase(Value, "All", FormatStyle::SLS_All);
+IO.enumCase(Value, "true", FormatStyle::SLS_All);
+  }
+};
+
 template <> struct ScalarEnumerationTraits {
   static void enumeration(IO &IO, FormatStyle::BinPackStyle &Value) {
 IO.enumCase(Value, "Auto", FormatStyle::BPS_Auto);
@@ -347,6 +358,8 @@
Style.AllowShortCaseLabelsOnASingleLine);
 IO.mapOptional("AllowShortFunctionsOnASingleLine",
Style.AllowShortFunctionsOnASingleLine);
+IO.mapOptional("AllowShortLambdasOnASingleLine",
+   S

[PATCH] D58236: Make address space conversions a bit stricter.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D58236#1443082 , @rjmccall wrote:

> I think C probably requires us to allow this under an explicit cast, but we 
> can at least warn about it.  It does not require us to allow this as an 
> implicit conversion, and that should be an error.


Are you referring to casts to and from `void*`? I think the standard says those 
have to be allowed, and I don't see anything about whether or not the 
conversion has to be explicit.


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

https://reviews.llvm.org/D58236



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

[PATCH] D59841: [Gnu Driver] Is -pie and -static-pie are both passed, let -static-pie win.

[Siva Chandra via Phabricator via cfe-commits]

sivachandra created this revision.
sivachandra added a reviewer: saugustine.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

A static-pie binary is a PIE after all, so letting -static-pie win makes
the resulting binary statically linked as well as being a PIE.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59841

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/linux-ld.c


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -189,6 +189,19 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
 //
+// RUN: %clang -pie -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-PIE-STATIC-PIE %s
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -308,7 +308,7 @@
 
 static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,


Index: clang/test/Driver/linux-ld.c
===
--- clang/test/Driver/linux-ld.c
+++ clang/test/Driver/linux-ld.c
@@ -189,6 +189,19 @@
 // CHECK-CLANG-LD-STATIC-PIE: "{{.*}}rcrt1.o"
 // CHECK-CLANG-LD-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
 //
+// RUN: %clang -pie -static-pie -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-LD-PIE-STATIC-PIE %s
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-static"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-pie"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--no-dynamic-linker"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "-m" "elf_x86_64"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "{{.*}}rcrt1.o"
+// CHECK-CLANG-LD-PIE-STATIC-PIE: "--start-group" "-lgcc" "-lgcc_eh" "-lc" "--end-group"
+//
 // RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
 // RUN: --gcc-toolchain="" \
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -308,7 +308,7 @@
 
 static bool getPIE(const ArgList &Args, const toolchains::Linux &ToolChain) {
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;
 
   Arg *A = Args.getLastArg(options::OPT_pie, options::OPT_no_pie,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

We have warnings like -Wdivision-by-zero that take reachability into account: 
https://godbolt.org/z/3PD-eF. I don't immediately know how it's all done (e.g. 
in batch because CFG construction is expensive? ...), but looking there for 
inspiration may be useful.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D58797: [Sema] Add some compile time _FORTIFY_SOURCE diagnostics

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

(Forgot to refresh before pressing 'Submit', so maybe efriedma's comment 
answers all of the questions in mine ;) )


Repository:
  rC Clang

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

https://reviews.llvm.org/D58797



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D59802#1443527 , @aaron.ballman 
wrote:

> Would it make sense to transform `if (dyn_cast_or_null(Obj))` into `if 
> (Obj && isa(Obj))`  or are there bad transformations from that?


Sure, that sounds reasonable.  I only see about 12 cases across all repos, so 
it isn't that common.  Whereas the idiom you present is used quite often.

I haven't looked yet, but wouldn't the use of `isa_or_null<>` be more efficient 
in cases like this?

./clang/lib/Sema/AnalysisBasedWarnings.cpp:401:  if (B->getTerminator() 
&& isa(B->getTerminator()))
./clang/lib/AST/Expr.cpp:2734:if (MCE->getMethodDecl() && 
isa(MCE->getMethodDecl()))

Granted, there aren't a lot of these.

> 
> 
>> Btw, I also found the same pattern used for `while()`, so I'll add that too. 
>>  Here's a sample of the patterns I'm seeing:
>> 
>> ./lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp:213:  
>> while (dyn_cast(last_stmt)) {
> 
> Hah, good catch!
> 
>> ./clang/lib/CodeGen/CodeGenModule.cpp:1390:  if 
>> (dyn_cast_or_null(D)) . // <--- this one's okay
> 
> I think this could be expressed as `if (D && isa(D))`, no?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802



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

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

akhuang marked 8 inline comments as done.
akhuang added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:145
+  matches = re.findall(stacktrace_re, crash_output)
+  result = filter(lambda x: x and x.strip() not in filters, matches)[:5]
+  for msg in result:

arichardson wrote:
> george.burgess.iv wrote:
> > nit: please prefer `[x for x in matches if x and x.strip() not in 
> > filters][:5]`. py3's filter returns a generator, whereas py2's returns a 
> > list.
> Stack traces also look different on macOS and it would be nice to handle that 
> too.
> 
> Here's a sample I got from adding a llvm_unreachable at a random location:
> ```
> My unreachable message...
> UNREACHABLE executed at 
> /Users/alex/cheri/llvm-project/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp:4468!
> Stack dump:
> 0.Program arguments: 
> /Users/alex/cheri/llvm-project/cmake-build-debug/bin/opt 
> -mtriple=cheri-unknown-freebsd -mcpu=cheri128 -mattr=+cheri128 -target-abi 
> purecap -relocation-model pic -S -instcombine -simplifycfg 
> /Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll
>  -o - 
> 1.Running pass 'Function Pass Manager' on module 
> '/Users/alex/cheri/llvm-project/llvm/test/CodeGen/Mips/cheri/simplifycfg-ptrtoint.ll'.
> 2.Running pass 'Combine redundant instructions' on function 
> '@cannot_fold_tag_unknown'
> 0  libLLVMSupport.dylib 0x000114515a9d 
> llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 45
> 1  libLLVMSupport.dylib 0x0001145153f1 llvm::sys::RunSignalHandlers() 
> + 65
> 2  libLLVMSupport.dylib 0x000114515fbf SignalHandler(int) + 111
> 3  libsystem_platform.dylib 0x7fff5b637b3d _sigtramp + 29
> 4  libsystem_platform.dylib 0x7ffee20d0cf0 _sigtramp + 2259259856
> 5  libsystem_c.dylib0x7fff5b4f51c9 abort + 127
> 6  libLLVMSupport.dylib 0x00011446bb12 
> llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) + 162
> 7  libLLVMInstCombine.dylib 0x000112c345c8 
> llvm::InstCombiner::foldICmpUsingKnownBits(llvm::ICmpInst&) + 4136
> 8  libLLVMInstCombine.dylib 0x000112c34d19 
> llvm::InstCombiner::visitICmpInst(llvm::ICmpInst&) + 569
> 9  libLLVMInstCombine.dylib 0x000112bb9cf2 llvm::InstCombiner::run() + 
> 1522
> 10 libLLVMInstCombine.dylib 0x000112bbb310 
> combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, 
> llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, 
> llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, 
> llvm::LoopInfo*) + 624
> 11 libLLVMInstCombine.dylib 0x000112bbb6d6 
> llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) + 214
> 12 libLLVMCore.dylib0x000111c0bb4d 
> llvm::FPPassManager::runOnFunction(llvm::Function&) + 317
> 13 libLLVMCore.dylib0x000111c0be83 
> llvm::FPPassManager::runOnModule(llvm::Module&) + 99
> 14 libLLVMCore.dylib0x000111c0c2c4 (anonymous 
> namespace)::MPPassManager::runOnModule(llvm::Module&) + 420
> 15 libLLVMCore.dylib0x000111c0c036 
> llvm::legacy::PassManagerImpl::run(llvm::Module&) + 182
> 16 opt  0x00010db6657b main + 7163
> 17 libdyld.dylib0x7fff5b44ced9 start + 1
> ```
I changed the regex to ignore the # at the beginning of the line - I think that 
should cover the mac os stack trace 



Comment at: clang/utils/creduce-clang-crash.py:303
+opts_startswith=["-O"])
+self.clang_args = new_args
+verbose_print("Simplified command:", quote_cmd(self.get_crash_cmd()))

arichardson wrote:
> george.burgess.iv wrote:
> > FWIW, opportunistically trying to add `-fsyntax-only` may help here: if the 
> > crash is in the frontend, it means that non-repros will stop before 
> > codegen, rather than trying to generate object files, or whatever they were 
> > trying to generate in the first place.
> Yes that sounds like a good idea! I just do -emit-llvm to avoid assembly 
> output but for parser/sema crashes -fsyntax-only would save some time.
> 
> Another one I found useful was `-Werror=implicit-int` to get more readable 
> test cases from creduce: 
> https://github.com/CTSRD-CHERI/llvm-project/blob/master/clang/utils/creduce_crash_testcase.py#L851
> 
> Without that flag lots of test cases look really weird due to the implicit 
> int and various inferred semicolons.
> 
Sounds good-- I added `-fsyntax-only`, `-emit-llvm` and `-Werror=implicit-int`



Comment at: clang/utils/creduce-clang-crash.py:64
+
+class Reduce:
+  def __init__(self, crash_script, file_to_reduce):

arichardson wrote:
> Does this need to be `Reduce(object):` for python2? 
I think it still works, but adding `object` makes sense. 



Comment at: clang/utils/creduce-clang-crash.py:123
+# Look for specific error messages
+regexes = [r"Asserti

[PATCH] D59725: Additions to creduce script

[Amy Huang via Phabricator via cfe-commits]

akhuang updated this revision to Diff 192350.
akhuang marked 2 inline comments as done.
akhuang added a comment.
Herald added a subscriber: jdoerfert.

added to error message regexes and command line flags


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

https://reviews.llvm.org/D59725

Files:
  clang/utils/creduce-clang-crash.py

Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -1,8 +1,14 @@
 #!/usr/bin/env python
 """Calls C-Reduce to create a minimal reproducer for clang crashes.
+
+Output files:
+  *.reduced.sh -- crash reproducer with minimal arguments
+  *.reduced.cpp -- the reduced file
+  *.test.sh -- interestingness test for C-Reduce
 """
 
-from argparse import ArgumentParser
+from __future__ import print_function
+from argparse import ArgumentParser, RawTextHelpFormatter
 import os
 import re
 import stat
@@ -15,10 +21,14 @@
 from distutils.spawn import find_executable
 
 verbose = False
-llvm_bin = None
 creduce_cmd = None
+clang_cmd = None
 not_cmd = None
 
+def verbose_print(*args, **kwargs):
+  if verbose:
+print(*args, **kwargs)
+
 def check_file(fname):
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
@@ -33,166 +43,334 @@
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd
-sys.exit("ERROR: executable %s not found" % (cmd_path))
+sys.exit("ERROR: executable `%s` not found" % (cmd_path))
 
   cmd = find_executable(cmd_name, path=cmd_dir)
   if cmd:
 return cmd
-  sys.exit("ERROR: %s not found in %s" % (cmd_name, cmd_dir))
 
-def quote_cmd(cmd):
-  return ' '.join(arg if arg.startswith('$') else pipes.quote(arg)
-  for arg in cmd)
-
-def get_crash_cmd(crash_script):
-  with open(crash_script) as f:
-# Assume clang call is on the last line of the script
-line = f.readlines()[-1]
-cmd = shlex.split(line)
-
-# Overwrite the script's clang with the user's clang path
-new_clang = check_cmd('clang', llvm_bin)
-cmd[0] = pipes.quote(new_clang)
-return cmd
+  if not cmd_dir:
+cmd_dir = "$PATH"
+  sys.exit("ERROR: `%s` not found in %s" % (cmd_name, cmd_dir))
 
-def has_expected_output(crash_cmd, expected_output):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-  return all(msg in crash_output for msg in expected_output)
-
-def get_expected_output(crash_cmd):
-  p = subprocess.Popen(crash_cmd,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT)
-  crash_output, _ = p.communicate()
-
-  # If there is an assertion failure, use that;
-  # otherwise use the last five stack trace functions
-  assertion_re = r'Assertion `([^\']+)\' failed'
-  assertion_match = re.search(assertion_re, crash_output)
-  if assertion_match:
-return [assertion_match.group(1)]
-  else:
-stacktrace_re = r'#[0-9]+\s+0[xX][0-9a-fA-F]+\s*([^(]+)\('
-matches = re.findall(stacktrace_re, crash_output)
-return matches[-5:]
-
-def write_interestingness_test(testfile, crash_cmd, expected_output,
-   file_to_reduce):
-  filename = os.path.basename(file_to_reduce)
-  if filename not in crash_cmd:
-sys.exit("ERROR: expected %s to be in the crash command" % filename)
-
-  # Replace all instances of file_to_reduce with a command line variable
-  output = ['#!/bin/bash',
-'if [ -z "$1" ] ; then',
-'  f=%s' % (pipes.quote(filename)),
-'else',
-'  f="$1"',
-'fi']
-  cmd = ['$f' if s == filename else s for s in crash_cmd]
-
-  output.append('%s --crash %s >& t.log || exit 1' % (pipes.quote(not_cmd),
-  quote_cmd(cmd)))
-
-  for msg in expected_output:
-output.append('grep %s t.log || exit 1' % pipes.quote(msg))
-
-  with open(testfile, 'w') as f:
-f.write('\n'.join(output))
-  os.chmod(testfile, os.stat(testfile).st_mode | stat.S_IEXEC)
-
-def check_interestingness(testfile, file_to_reduce):
-  testfile = os.path.abspath(testfile)
-
-  # Check that the test considers the original file interesting
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call(testfile, stdout=devnull)
-  if returncode:
-sys.exit("The interestingness test does not pass for the original file.")
-
-  # Check that an empty file is not interesting
-  _, empty_file = tempfile.mkstemp()
-  with open(os.devnull, 'w') as devnull:
-returncode = subprocess.call([testfile, empty_file], stdout=devnull)
-  os.remove(empty_file)
-  if not returncode:
-sys.exit("The interestingness test passes for an empty file.")
-
-def clang_preprocess(file_to_reduce, crash_cmd, expected_output):
-  _, tmpfile = tempfile.mkstemp()
-  shutil.copy(file_to_reduce, tmpfile)

[PATCH] D59603: [PR40707][PR41011][OpenCL] Allow addr space spelling without double underscore in C++ mode

[Jinsong Ji via Phabricator via cfe-commits]

jsji added a comment.

Looks like this may cause some unexpected failures. See 
https://bugs.llvm.org/show_bug.cgi?id=41247 for more details.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59603



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

[PATCH] D59841: [Gnu Driver] If -pie and -static-pie are both passed, let -static-pie win.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:311
   if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_static) ||
-  Args.hasArg(options::OPT_r))
+  Args.hasArg(options::OPT_r) || Args.hasArg(options::OPT_static_pie))
 return false;

It's not clear to me that the command line -static-pie -no-pie should result in 
static-pie, given the way the rest of that function is coded.

I might make it a ternary enum: "nothing" "pie" "static-pie" with the last one 
winning. That method seems more consistent with current behavior.

This would allow anyone checking the state of pie to use a single function and 
just check the result.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59841



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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: martong, teemperor, friss, a_sidorin.
Herald added subscribers: jdoerfert, rnkovacs.

We may try and re-import an EnumDecl while trying to complete it in 
`IsStructuralMatch(...)` specialization for `EnumDecl`. This change mirrors a 
similar fix for the specialization for `RecordDecl`.


https://reviews.llvm.org/D59845

Files:
  lib/AST/ASTImporter.cpp


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,17 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+
+  if (ToOrigin) {
+auto *ToOriginEnum = dyn_cast(ToOrigin);
+
+if (ToOriginEnum)
+ToEnum = ToOriginEnum;
+  }
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1946,6 +1946,17 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
+  // Eliminate a potential failure point where we attempt to re-import
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+
+  if (ToOrigin) {
+auto *ToOriginEnum = dyn_cast(ToOrigin);
+
+if (ToOriginEnum)
+ToEnum = ToOriginEnum;
+  }
+
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r357031 - Make -mno-outline pass -enable-machine-outliner=never to ld in LTO

[Jessica Paquette via cfe-commits]

Author: paquette
Date: Tue Mar 26 14:22:42 2019
New Revision: 357031

URL: http://llvm.org/viewvc/llvm-project?rev=357031&view=rev
Log:
Make -mno-outline pass -enable-machine-outliner=never to ld in LTO

Since AArch64 has default outlining behaviour, we need to make sure that
-mno-outline is actually passed along to the linker in this case. Otherwise,
it will run by default on minsize functions even when -mno-outline is specified.

Also fix the darwin-ld test for this, which wasn't actually doing anything.

Modified:
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld.c

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=357031&r1=357030&r2=357031&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Tue Mar 26 14:22:42 2019
@@ -494,14 +494,23 @@ void darwin::Linker::ConstructJob(Compil
   }
 
   // Propagate the -moutline flag to the linker in LTO.
-  if (Args.hasFlag(options::OPT_moutline, options::OPT_mno_outline, false)) {
-if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
-  CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-machine-outliner");
+  if (Arg *A =
+  Args.getLastArg(options::OPT_moutline, options::OPT_mno_outline)) {
+if (A->getOption().matches(options::OPT_moutline)) {
+  if (getMachOToolChain().getMachOArchName(Args) == "arm64") {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-machine-outliner");
 
-  // Outline from linkonceodr functions by default in LTO.
+// Outline from linkonceodr functions by default in LTO.
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-enable-linkonceodr-outlining");
+  }
+} else {
+  // Disable all outlining behaviour if we have mno-outline. We need to do
+  // this explicitly, because targets which support default outlining will
+  // try to do work if we don't.
   CmdArgs.push_back("-mllvm");
-  CmdArgs.push_back("-enable-linkonceodr-outlining");
+  CmdArgs.push_back("-enable-machine-outliner=never");
 }
   }
 

Modified: cfe/trunk/test/Driver/darwin-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld.c?rev=357031&r1=357030&r2=357031&view=diff
==
--- cfe/trunk/test/Driver/darwin-ld.c (original)
+++ cfe/trunk/test/Driver/darwin-ld.c Tue Mar 26 14:22:42 2019
@@ -372,5 +372,11 @@
 // Check that we can pass the outliner down to the linker.
 // RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
 // RUN:   %clang -target arm64-apple-darwin -moutline -### %t.o 2> %t.log
-// MOUTLINE: ld
+// RUN: FileCheck -check-prefix=MOUTLINE %s < %t.log
+// MOUTLINE: {{ld(.exe)?"}}
 // MOUTLINE-SAME: "-mllvm" "-enable-machine-outliner" "-mllvm" 
"-enable-linkonceodr-outlining"
+// RUN: env IPHONEOS_DEPLOYMENT_TARGET=7.0 \
+// RUN:   %clang -target arm64-apple-darwin -mno-outline -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=MNO_OUTLINE %s < %t.log
+// MNO_OUTLINE: {{ld(.exe)?"}}
+// MNO_OUTLINE-SAME: "-mllvm" "-enable-machine-outliner=never"


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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

LLDB regression test that goes with this fix: https://reviews.llvm.org/D59847


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

https://reviews.llvm.org/D59845



___
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

[Aaron Smith via Phabricator via cfe-commits]

asmith added a comment.

Okay if we change the flag then I believe the tests under 
llvm/tests/lib/DebugInfo/CodeView must be updated. If we use NonTrivial all the 
existing tests work as expected.

LLVM change is here (you are all reviewers).
https://reviews.llvm.org/D59348


Repository:
  rC Clang

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] D59838: gn build: Add build files for clang-include-fixer and find-all-symbols

[Mirko Bonadei via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D59838



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

[PATCH] D59845: Fix IsStructuralMatch specialization for EnumDecl to prevent re-importing an EnumDecl while trying to complete it

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: lib/AST/ASTImporter.cpp:1951
+  // something we're trying to import while completin ToEnum
+  Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum);
+

```
if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum)) 
  if (auto *ToOriginEnum = dyn_cast(ToOrigin))
ToEnum = ToOriginEnum;
```


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

https://reviews.llvm.org/D59845



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

[PATCH] D59628: Add support for __attribute__((objc_class_stub))

[Jordan Rose via Phabricator via cfe-commits]

jordan_rose added inline comments.



Comment at: lib/CodeGen/CGObjCMac.cpp:7285
 
-Entry = new llvm::GlobalVariable(CGM.getModule(), 
ObjCTypes.ClassnfABIPtrTy,
+Entry = new llvm::GlobalVariable(CGM.getModule(), ClassGV->getType(),
  false, llvm::GlobalValue::PrivateLinkage,

Is there a concern here in the non-stub case if GetClassGlobal no longer 
produces a ObjCTypes.ClassnfABIPtrTy? (Probably not, but thought I'd check 
[again].)


Repository:
  rC Clang

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

https://reviews.llvm.org/D59628



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

[PATCH] D58236: Make address space conversions a bit stricter.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D58236#1443556 , @ebevhan wrote:

> In D58236#1443082 , @rjmccall wrote:
>
> > I think C probably requires us to allow this under an explicit cast, but we 
> > can at least warn about it.  It does not require us to allow this as an 
> > implicit conversion, and that should be an error.
>
>
> Are you referring to casts to and from `void*`? I think the standard says 
> those have to be allowed, and I don't see anything about whether or not the 
> conversion has to be explicit.


No, I mean casts between pointer types that change the pointee address space.


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

https://reviews.llvm.org/D58236



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

[PATCH] D59802: [clang-tidy] Add new checker: llvm-avoid-cast-in-conditional

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 192370.
hintonda added a comment.

- Address additional comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59802

Files:
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.cpp
  clang-tools-extra/clang-tidy/llvm/AvoidCastInConditionalCheck.h
  clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
  clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
  clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
  clang/include/clang/Basic/LLVM.h

Index: clang/include/clang/Basic/LLVM.h
===
--- clang/include/clang/Basic/LLVM.h
+++ clang/include/clang/Basic/LLVM.h
@@ -31,6 +31,7 @@
   template class ArrayRef;
   template class MutableArrayRef;
   template class OwningArrayRef;
+  template  class SmallSet;
   template class SmallString;
   template class SmallVector;
   template class SmallVectorImpl;
@@ -66,6 +67,7 @@
   using llvm::Optional;
   using llvm::OwningArrayRef;
   using llvm::SaveAndRestore;
+  using llvm::SmallSet;
   using llvm::SmallString;
   using llvm::SmallVector;
   using llvm::SmallVectorImpl;
Index: clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/llvm-avoid-cast-in-conditional.cpp
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s llvm-avoid-cast-in-conditional %t
+
+struct X;
+struct Y;
+struct Z {
+  int foo();
+};
+
+template 
+X* cast(Y*);
+
+bool foo(Y* y) {
+  if (auto x = cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: {{.*llvm-avoid-cast-in-conditional}}
+return true;
+  if (cast(y))
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning:  {{.*llvm-avoid-cast-in-conditional}}
+return true;
+
+  // Does not trigger warning.
+  if (cast(y)->foo())
+return true;
+  return false;
+}
+
Index: clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/llvm-avoid-cast-in-conditional.rst
@@ -0,0 +1,19 @@
+.. title:: clang-tidy - llvm-avoid-cast-in-conditional
+
+llvm-avoid-cast-in-conditional
+==
+
+Finds cases of ``cast<>`` used as the condition of a conditional
+statements which will assert on failure in Debug builds. The use of
+``cast<>`` implies that the operation cannot fail, and should not be
+used as the condition of a conditional statement.
+
+.. code-block:: c++
+
+  // Finds cases like these:
+  if (auto x = cast(y)) <...>
+  if (cast(y)) <...>
+
+  // But not cases like these:
+  if (auto f = cast(y)->foo()) <...>
+  if (cast(y)->foo()) <...>
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -175,6 +175,7 @@
hicpp-use-nullptr (redirects to modernize-use-nullptr) 
hicpp-use-override (redirects to modernize-use-override) 
hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) 
+   llvm-avoid-cast-in-conditional
llvm-header-guard
llvm-include-order
llvm-namespace-comment
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -130,6 +130,12 @@
   ` now supports `OverrideSpelling`
   and `FinalSpelling` options.
 
+- New :doc:`llvm-avoid-cast-in-conditional
+  ` check.
+
+  Finds cases of ``cast<>`` used as condition in conditional statements
+  which will assert on failure in Debug builds.
+
 - New :doc:`openmp-exception-escape
   ` check.
 
Index: clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
===
--- clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
+++ clang-tools-extra/clang-tidy/utils/HeaderFileExtensionsUtils.h
@@ -18,7 +18,7 @@
 namespace tidy {
 namespace utils {
 
-typedef llvm::SmallSet HeaderFileExtensionsSet;
+using HeaderFileExtensionsSet = SmallSet;
 
 /// \brief Checks whether expansion location of \p Loc is in header file.
 bool isExpansionLocInHeaderFile(
Index: clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #includ

r357036 - [cmake] Reset variable before using it

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Tue Mar 26 15:16:53 2019
New Revision: 357036

URL: http://llvm.org/viewvc/llvm-project?rev=357036&view=rev
Log:
[cmake] Reset variable before using it

A bunch of macros use the same variable name, and since CMake macros
don't get their own scope, the value persists across macro invocations,
and we can end up exporting targets which shouldn't be exported. Clear
the variable before each use to avoid this.

Converting these macros to functions would also help, since it would
avoid the variable leaking into its parent scope, and that's something I
plan to follow up with. It won't fully address the problem, however,
since functions still inherit variables from their parent scopes, so if
someone in the parent scope just happened to use the same variable name
we'd still have the same issue.

Modified:
cfe/trunk/cmake/modules/AddClang.cmake

Modified: cfe/trunk/cmake/modules/AddClang.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/modules/AddClang.cmake?rev=357036&r1=357035&r2=357036&view=diff
==
--- cfe/trunk/cmake/modules/AddClang.cmake (original)
+++ cfe/trunk/cmake/modules/AddClang.cmake Tue Mar 26 15:16:53 2019
@@ -89,7 +89,7 @@ macro(add_clang_library name)
 target_link_libraries(${name} INTERFACE ${LLVM_COMMON_LIBS})
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "libclang")
-
+  set(export_to_clangtargets)
   if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   "clang-libraries" IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
   NOT LLVM_DISTRIBUTION_COMPONENTS)
@@ -137,6 +137,7 @@ macro(add_clang_tool name)
   add_dependencies(${name} clang-resource-headers)
 
   if (CLANG_BUILD_TOOLS)
+set(export_to_clangtargets)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
 NOT LLVM_DISTRIBUTION_COMPONENTS)
   set(export_to_clangtargets EXPORT ClangTargets)


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

[clang-tools-extra] r357037 - Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Tue Mar 26 15:18:52 2019
New Revision: 357037

URL: http://llvm.org/viewvc/llvm-project?rev=357037&view=rev
Log:
Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

Remove CompilerInstance::VirtualFileSystem and
CompilerInstance::setVirtualFileSystem, instead relying on the VFS in
the FileManager.  CompilerInstance and its clients already went to some
trouble to make these match.  Now they are guaranteed to match.

As part of this, I added a VFS parameter (defaults to nullptr) to
CompilerInstance::createFileManager, to avoid repeating construction
logic in clients that just wanted to customize the VFS.

https://reviews.llvm.org/D59377

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

Modified: clang-tools-extra/trunk/clangd/Compiler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Compiler.cpp?rev=357037&r1=357036&r2=357037&view=diff
==
--- clang-tools-extra/trunk/clangd/Compiler.cpp (original)
+++ clang-tools-extra/trunk/clangd/Compiler.cpp Tue Mar 26 15:18:52 2019
@@ -95,7 +95,7 @@ prepareCompilerInstance(std::unique_ptr<
   if (auto VFSWithRemapping = createVFSFromCompilerInvocation(
   Clang->getInvocation(), Clang->getDiagnostics(), VFS))
 VFS = VFSWithRemapping;
-  Clang->setVirtualFileSystem(VFS);
+  Clang->createFileManager(VFS);
 
   Clang->setTarget(TargetInfo::CreateTargetInfo(
   Clang->getDiagnostics(), Clang->getInvocation().TargetOpts));


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

r357037 - Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Tue Mar 26 15:18:52 2019
New Revision: 357037

URL: http://llvm.org/viewvc/llvm-project?rev=357037&view=rev
Log:
Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

Remove CompilerInstance::VirtualFileSystem and
CompilerInstance::setVirtualFileSystem, instead relying on the VFS in
the FileManager.  CompilerInstance and its clients already went to some
trouble to make these match.  Now they are guaranteed to match.

As part of this, I added a VFS parameter (defaults to nullptr) to
CompilerInstance::createFileManager, to avoid repeating construction
logic in clients that just wanted to customize the VFS.

https://reviews.llvm.org/D59377

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Frontend/CompilerInstance.h
cfe/trunk/lib/Frontend/ASTUnit.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/StaticAnalyzer/Frontend/ModelInjector.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=357037&r1=357036&r2=357037&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Tue Mar 26 15:18:52 2019
@@ -175,6 +175,10 @@ class FileManager : public RefCountedBas
   void fillRealPathName(FileEntry *UFE, llvm::StringRef FileName);
 
 public:
+  /// Construct a file manager, optionally with a custom VFS.
+  ///
+  /// \param FS if non-null, the VFS to use.  Otherwise uses
+  /// llvm::vfs::getRealFileSystem().
   FileManager(const FileSystemOptions &FileSystemOpts,
   IntrusiveRefCntPtr FS = nullptr);
   ~FileManager();

Modified: cfe/trunk/include/clang/Frontend/CompilerInstance.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/CompilerInstance.h?rev=357037&r1=357036&r2=357037&view=diff
==
--- cfe/trunk/include/clang/Frontend/CompilerInstance.h (original)
+++ cfe/trunk/include/clang/Frontend/CompilerInstance.h Tue Mar 26 15:18:52 2019
@@ -82,9 +82,6 @@ class CompilerInstance : public ModuleLo
   /// Auxiliary Target info.
   IntrusiveRefCntPtr AuxTarget;
 
-  /// The virtual file system.
-  IntrusiveRefCntPtr VirtualFileSystem;
-
   /// The file manager.
   IntrusiveRefCntPtr FileMgr;
 
@@ -382,20 +379,8 @@ public:
   /// @name Virtual File System
   /// {
 
-  bool hasVirtualFileSystem() const { return VirtualFileSystem != nullptr; }
-
   llvm::vfs::FileSystem &getVirtualFileSystem() const {
-assert(hasVirtualFileSystem() &&
-   "Compiler instance has no virtual file system");
-return *VirtualFileSystem;
-  }
-
-  /// Replace the current virtual file system.
-  ///
-  /// \note Most clients should use setFileManager, which will implicitly reset
-  /// the virtual file system to the one contained in the file manager.
-  void setVirtualFileSystem(IntrusiveRefCntPtr FS) {
-VirtualFileSystem = std::move(FS);
+return *getFileManager().getVirtualFileSystem();
   }
 
   /// }
@@ -645,7 +630,8 @@ public:
   /// Create the file manager and replace any existing one with it.
   ///
   /// \return The new file manager on success, or null on failure.
-  FileManager *createFileManager();
+  FileManager *
+  createFileManager(IntrusiveRefCntPtr VFS = nullptr);
 
   /// Create the source manager and replace any existing one with it.
   void createSourceManager(FileManager &FileMgr);

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=357037&r1=357036&r2=357037&view=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue Mar 26 15:18:52 2019
@@ -1078,28 +1078,29 @@ bool ASTUnit::Parse(std::shared_ptrgetVirtualFileSystem() &&
+   "VFS passed to Parse and VFS in FileMgr are different");
+
   auto CCInvocation = std::make_shared(*Invocation);
   if (OverrideMainBuffer) {
 assert(Preamble &&
"No preamble was built, but OverrideMainBuffer is not null");
-IntrusiveRefCntPtr OldVFS = VFS;
 Preamble->AddImplicitPreamble(*CCInvocation, VFS, 
OverrideMainBuffer.get());
-if (OldVFS != VFS && FileMgr) {
-  assert(OldVFS == FileMgr->getVirtualFileSystem() &&
- "VFS passed to Parse and VFS in FileMgr are different");
-  FileMgr = new FileManager(FileMgr->getFileSystemOpts(), VFS);
-}
+// VFS may have changed...
   }
 
   // Create the compiler instance to use for building the AST.
   std::unique_ptr Clang(
   new CompilerInstance(std::move(PCHContainerOps)));
-  if (FileMgr && VFS) {
-assert(VFS == FileMgr->getVirtualFileSystem() &&
-   "VFS passed to Parse and VFS in FileMgr are different");
-  } else if (VFS) {
-Clang

[PATCH] D59377: Frontend: Remove CompilerInstance::VirtualFileSystem, NFC

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith closed this revision.
dexonsmith added a comment.

Committed r357037.


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

https://reviews.llvm.org/D59377



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

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

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

Use double backticks rather than single backticks for symbols in documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.cpp
  clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
  clang-tools-extra/test/clang-tidy/objc-super-self.m

Index: clang-tools-extra/test/clang-tidy/objc-super-self.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/objc-super-self.m
@@ -0,0 +1,54 @@
+// RUN: %check_clang_tidy %s objc-super-self %t
+
+@interface NSObject
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NSObjectDerivedClass : NSObject
+@end
+
+@implementation NSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: return [super init];
+}
+
+- (instancetype)initWithObject:(NSObject *)obj {
+  self = [super self];
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious invocation of -[NSObject self] in initializer; did you mean to invoke a superclass initializer? [objc-super-self]
+// CHECK-FIXES: self = [super init];
+  if (self) {
+// ...
+  }
+  return self;
+}
+
+- (instancetype)foo {
+  return [super self];
+}
+
+- (instancetype)bar {
+  return [self self];
+}
+
+@end
+
+@interface RootClass
+- (instancetype)init;
+- (instancetype)self;
+@end
+
+@interface NotNSObjectDerivedClass : RootClass
+@end
+
+@implementation NotNSObjectDerivedClass
+
+- (instancetype)init {
+  return [super self];
+}
+
+@end
+
Index: clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-super-self.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-super-self
+
+objc-super-self
+===
+
+Finds invocations of ``-self`` on super instances in initializers of subclasses
+of ``NSObject`` and recommends calling a superclass initializer instead.
+
+Invoking ``-self`` on super instances in initializers is a common programmer
+error when the programmer's original intent is to call a superclass
+initializer. Failing to call a superclass initializer breaks initializer
+chaining and can result in invalid object initialization.
+
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -227,6 +227,7 @@
objc-avoid-spinlock
objc-forbidden-subclassing
objc-property-declaration
+   objc-super-self
openmp-exception-escape
openmp-use-default-none
performance-faster-string-find
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -108,6 +108,12 @@
   Checks whether there are underscores in googletest test and test case names in
   test macros, which is prohibited by the Googletest FAQ.
 
+- New :doc:`objc-super-self ` check.
+
+  Finds invocations of ``-self`` on super instances in initializers of
+  subclasses of ``NSObject`` and recommends calling a superclass initializer
+  instead.
+
 - New alias :doc:`cppcoreguidelines-explicit-virtual-functions
   ` to
   :doc:`modernize-use-override
Index: clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/objc/SuperSelfCheck.h
@@ -0,0 +1,36 @@
+//===--- SuperSelfCheck.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_OBJC_SUPERSELFCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_OBJC_SUPERSELFCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace objc {
+
+/// Finds invocations of -self on super instances in initializers of subclasses
+/// of NSObject and recommends calling a superclass initializer instead.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org

[PATCH] D59806: [clang-tidy] Add a check for [super self] in initializers 🔍

[Stephane Moore via Phabricator via cfe-commits]

stephanemoore marked 2 inline comments as done.
stephanemoore added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:113
+
+  Finds invocations of `-self` on super instances in initializers of subclasses
+  of `NSObject` and recommends calling a superclass initializer instead.

Eugene.Zelenko wrote:
> Please use double backticks for language constructs. Same in other places.
Sorry I misread your earlier comment. I have been editing too much Markdown 
recently 😅 I have updated to use double backticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59806



___
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.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ updated this revision to Diff 192372.
NoQ marked 6 inline comments as done.
NoQ added a comment.

Remove memoization for now. Address a few inline comments. I'm mostly done with 
this first patch, did i accidentally miss anything?

In D58367#1402796 , @Szelethus wrote:

> 1. It would be great to have unit tests for this.


Mm, i have problems now that checker registry is super locked down: i need a 
bug report object => i need a checker (or at least a `CheckName`) => i need the 
whole `AnalysisConsumer` => i can no longer override `ASTConsumer` methods for 
testing purposes (because `AnalysisConsumer` is a final sub-class of 
`ASTConsumer`). Do you have a plan B for that?

I also generally don't see many good unit tests we could write here, that would 
add much to the integration tests we already have, but this memoization problem 
could have made a nice unit test.

In D58367#1423497 , @Charusso wrote:

> In D58367#1423451 , @NoQ wrote:
>
> > Found a bug! The lambda has to manually make sure that the bug report 
> > actually does have something to do with the checker.
>
>
> I think message-semantic is better than storing some data in two places. 
> BugReport has `getCheckName()` and may if the `NoteTag` knows the name of its 
> author then it is comparable.
>  I am not sure but may you could hack the lifetime of that `StringRef` from 
> `CheckName` to not to report on something purged out (/not exists?).


Storing and comparing the checker pointer ought to be cheaper and more precise 
than storing and comparing its name as a string that can be easily extracted 
from it. Still, storing a string may be interesting when it comes to 
identifying non-checker note sources. I guess we'll have to think more about 
that.


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

https://reviews.llvm.org/D58367

Files:
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
  clang/lib/StaticAnalyzer/Core/BugReporter.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/test/Analysis/mig.mm

Index: clang/test/Analysis/mig.mm
===
--- clang/test/Analysis/mig.mm
+++ clang/test/Analysis/mig.mm
@@ -91,6 +91,14 @@
  // expected-note@-1{{MIG callback fails with error after deallocating argument value. This is a use-after-free vulnerability because the caller will try to deallocate it again}}
 }
 
+MIG_SERVER_ROUTINE
+kern_return_t no_unrelated_notes(mach_port_name_t port, vm_address_t address, vm_size_t size) {
+  vm_deallocate(port, address, size); // no-note
+  1 / 0; // expected-warning{{Division by zero}}
+ // expected-note@-1{{Division by zero}}
+  return KERN_SUCCESS;
+}
+
 // Make sure we find the bug when the object is destroyed within an
 // automatic destructor.
 MIG_SERVER_ROUTINE
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -201,7 +201,9 @@
   svalBuilder(StateMgr.getSValBuilder()),
   ObjCNoRet(mgr.getASTContext()),
   BR(mgr, *this),
-  VisitedCallees(VisitedCalleesIn), HowToInline(HowToInlineIn) {
+  VisitedCallees(VisitedCalleesIn),
+  HowToInline(HowToInlineIn),
+  NoteTags(G.getAllocator()) {
   unsigned TrimInterval = mgr.options.GraphTrimInterval;
   if (TrimInterval != 0) {
 // Enable eager node reclamation when constructing the ExplodedGraph.
Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -2500,6 +2500,30 @@
   return nullptr;
 }
 
+int NoteTag::Kind = 0;
+
+void TagVisitor::Profile(llvm::FoldingSetNodeID &ID) const {
+  static int Tag = 0;
+  ID.AddPointer(&Tag);
+}
+
+std::shared_ptr
+TagVisitor::VisitNode(const ExplodedNode *N, BugReporterContext &BRC,
+  BugReport &R) {
+  ProgramPoint PP = N->getLocation();
+  const NoteTag *T = dyn_cast_or_null(PP.getTag());
+  if (!T)
+return nullptr;
+
+  if (Optional Msg = T->generateMessage(BRC, R)) {
+PathDiagnosticLocation Loc =
+PathDiagnosticLocation::create(PP, BRC.getSourceManager());
+return std::make_shared(Loc, *Msg);
+  }
+
+  return nullptr;
+}
+
 void FalsePositiveRefutationBRVisitor::Profile(
 llv

[clang-tools-extra] r357038 - Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Tue Mar 26 15:32:06 2019
New Revision: 357038

URL: http://llvm.org/viewvc/llvm-project?rev=357038&view=rev
Log:
Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

FileManager constructs a VFS in its constructor if it isn't passed one,
and there's no way to reset it.  Make that contract clear by returning a
reference from its accessor.

https://reviews.llvm.org/D59388

Modified:
clang-tools-extra/trunk/clang-move/Move.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clangd/SourceCode.cpp

Modified: clang-tools-extra/trunk/clang-move/Move.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-move/Move.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- clang-tools-extra/trunk/clang-move/Move.cpp (original)
+++ clang-tools-extra/trunk/clang-move/Move.cpp Tue Mar 26 15:32:06 2019
@@ -87,8 +87,7 @@ std::string MakeAbsolutePath(StringRef C
 std::string MakeAbsolutePath(const SourceManager &SM, StringRef Path) {
   llvm::SmallString<128> AbsolutePath(Path);
   if (std::error_code EC =
-  SM.getFileManager().getVirtualFileSystem()->makeAbsolute(
-  AbsolutePath))
+  
SM.getFileManager().getVirtualFileSystem().makeAbsolute(AbsolutePath))
 llvm::errs() << "Warning: could not make absolute file: '" << EC.message()
  << '\n';
   // Handle symbolic link path cases.

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Tue Mar 26 15:32:06 2019
@@ -362,7 +362,7 @@ ClangTidyASTConsumerFactory::CreateASTCo
   auto WorkingDir = Compiler.getSourceManager()
 .getFileManager()
 .getVirtualFileSystem()
-->getCurrentWorkingDirectory();
+.getCurrentWorkingDirectory();
   if (WorkingDir)
 Context.setCurrentBuildDirectory(WorkingDir.get());
 
@@ -555,7 +555,7 @@ void handleErrors(llvm::ArrayRef BaseFS) {
   ErrorReporter Reporter(Context, Fix, BaseFS);
   llvm::vfs::FileSystem &FileSystem =
-  *Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
+  Reporter.getSourceManager().getFileManager().getVirtualFileSystem();
   auto InitialWorkingDir = FileSystem.getCurrentWorkingDirectory();
   if (!InitialWorkingDir)
 llvm::report_fatal_error("Cannot get current working path.");

Modified: clang-tools-extra/trunk/clangd/SourceCode.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SourceCode.cpp?rev=357038&r1=357037&r2=357038&view=diff
==
--- clang-tools-extra/trunk/clangd/SourceCode.cpp (original)
+++ clang-tools-extra/trunk/clangd/SourceCode.cpp Tue Mar 26 15:32:06 2019
@@ -265,7 +265,7 @@ llvm::Optional getCanonical
   llvm::SmallString<128> FilePath = F->getName();
   if (!llvm::sys::path::is_absolute(FilePath)) {
 if (auto EC =
-SourceMgr.getFileManager().getVirtualFileSystem()->makeAbsolute(
+SourceMgr.getFileManager().getVirtualFileSystem().makeAbsolute(
 FilePath)) {
   elog("Could not turn relative path '{0}' to absolute: {1}", FilePath,
EC.message());


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