[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

@jhibbits sorry for not answering earlier, I was occupied with NY holidays, and 
then had a lot of stuff on the road.

Trying to sync to your latest changes, I merged the updated 
https://reviews.llvm.org/D54583 in my local copy with your fixes to libcall 
expansion.
From what I understood (correct me if I am wrong) it should have fixed the 
__divdc3 case.
However, it still appears appears unresolved to me, with a segmentation fault 
instead of selection error.

I narrowed a potentially smaller example, which reproduces the issue. Does it 
reproduce for you locally?

  void f1(long double v)
  {
  }
  
  void f2(void* v, __builtin_va_list arg)
  {
f1(__builtin_va_arg(arg, long double));
  }



  $ clang -c sample.c -target powerpc-gnu-linux-eabi -ffreestanding -nostdlib 
-femulated-tls -mcpu=8548 -mspe
  Stack dump:
  0.Program arguments: clang-7 -cc1 -triple powerpc-gnu-linux-eabi 
-emit-obj -mrelax-all -disable-free -disable-llvm-verifier -discard-value-names 
-main-file-name sample.c -mrelocation-model static -mthread-model posix 
-mdisable-fp-elim -fmath-errno -masm-verbose -mconstructor-aliases 
-ffreestanding -fuse-init-array -target-cpu ppc -target-feature +spe 
-mfloat-abi hard -dwarf-column-info -debugger-tuning=gdb -target-linker-version 
409.12 -coverage-notes-file Desktop/sample/sample.gcno -resource-dir 
/llvm/llvm-7.0.1-Darwin-x86_64/lib/clang/7.0.1 -internal-isystem 
/usr/local/include -internal-isystem 
/llvm/llvm-7.0.1-Darwin-x86_64/lib/clang/7.0.1/include 
-internal-externc-isystem /include -internal-externc-isystem /usr/include 
-fdebug-compilation-dir Desktop/sample -ferror-limit 19 -fmessage-length 265 
-femulated-tls -fno-signed-char -fobjc-runtime=gcc -fdiagnostics-show-option 
-fcolor-diagnostics -o sample.o -x c Desktop/sample/sample.c -faddrsig 
  1. parser at end of file
  2.Code generation
  3.Running pass 'Function Pass Manager' on module 
'Desktop/sample/sample.c'.
  4.Running pass 'PowerPC DAG->DAG Pattern Instruction Selection' on 
function '@f2'
  0  clang-7  0x0001099af90a 
llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37
  1  clang-7  0x0001099afcfa SignalHandler(int) + 200
  2  libsystem_platform.dylib 0x7fff52975f5a _sigtramp + 26
  3  libsystem_platform.dylib 0x7fe219f0eec0 _sigtramp + 3344535424
  4  clang-7  0x0001091093e1 
llvm::PPCTargetLowering::LowerCall(llvm::TargetLowering::CallLoweringInfo&, 
llvm::SmallVectorImpl&) const + 867
  5  clang-7  0x000109f55b7a 
llvm::TargetLowering::LowerCallTo(llvm::TargetLowering::CallLoweringInfo&) 
const + 3856
  6  clang-7  0x000109f686fd 
llvm::SelectionDAGBuilder::lowerInvokable(llvm::TargetLowering::CallLoweringInfo&,
 llvm::BasicBlock const*) + 357
  7  clang-7  0x000109f5ad64 
llvm::SelectionDAGBuilder::LowerCallTo(llvm::ImmutableCallSite, llvm::SDValue, 
bool, llvm::BasicBlock const*) + 1178
  8  clang-7  0x000109f4e05a 
llvm::SelectionDAGBuilder::visitCall(llvm::CallInst const&) + 1016
  9  clang-7  0x000109f474c0 
llvm::SelectionDAGBuilder::visit(llvm::Instruction const&) + 114
  10 clang-7  0x000109fa76d3 
llvm::SelectionDAGISel::SelectBasicBlock(llvm::ilist_iterator, false, true>, 
llvm::ilist_iterator, false, true>, bool&) + 177
  11 clang-7  0x000109fa6dde 
llvm::SelectionDAGISel::SelectAllBasicBlocks(llvm::Function const&) + 5532
  12 clang-7  0x000109fa50f4 
llvm::SelectionDAGISel::runOnMachineFunction(llvm::MachineFunction&) + 1474
  13 clang-7  0x0001090e0bcc (anonymous 
namespace)::PPCDAGToDAGISel::runOnMachineFunction(llvm::MachineFunction&) + 70
  14 clang-7  0x0001094d4785 
llvm::MachineFunctionPass::runOnFunction(llvm::Function&) + 117
  15 clang-7  0x00010965c36b 
llvm::FPPassManager::runOnFunction(llvm::Function&) + 335
  16 clang-7  0x00010965c505 
llvm::FPPassManager::runOnModule(llvm::Module&) + 49
  17 clang-7  0x00010965c7e6 
llvm::legacy::PassManagerImpl::run(llvm::Module&) + 558
  18 clang-7  0x000109ae2839 
clang::EmitBackendOutput(clang::DiagnosticsEngine&, clang::HeaderSearchOptions 
const&, clang::CodeGenOptions const&, clang::TargetOptions const&, 
clang::LangOptions const&, llvm::DataLayout const&, llvm::Module*, 
clang::BackendAction, std::__1::unique_ptr >) + 12981
  19 clang-7  0x000109c48ea2 
clang::BackendConsumer::HandleTranslationUnit(clang::ASTContext&) + 864
  20 clang-7  0x00010a40bbbf clang::ParseAST(clang::Sema&, 
bool, bool) + 458
  21 clang-7  0x000109dcb7d7 
clang::FrontendAction::Execute() + 69
  22 clang-7  0x000109d9d9fc 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) + 664
  23 clang-

[PATCH] D55741: Implementation Feature Test Macros for P0722R3

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added a comment.

I'd be happy to restrict this to > C++17 only (which is automatically the case 
when using G++ because there's no `-fdestroying-delete` to enable it, you only 
get it with `-std=c++2a`,  `-std=gnu++2a` etc.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D55741



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

[PATCH] D56829: Move decl context dumping to TextNodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

steveire created this revision.
steveire added a reviewer: aaron.ballman.
Herald added a subscriber: cfe-commits.

Only an obscure case is moved.


Repository:
  rC Clang

https://reviews.llvm.org/D56829

Files:
  lib/AST/ASTDumper.cpp
  lib/AST/TextNodeDumper.cpp


Index: lib/AST/TextNodeDumper.cpp
===
--- lib/AST/TextNodeDumper.cpp
+++ lib/AST/TextNodeDumper.cpp
@@ -256,6 +256,17 @@
   if (const FunctionDecl *FD = dyn_cast(D))
 if (FD->isConstexpr())
   OS << " constexpr";
+
+  if (!isa(*D)) {
+auto MD = dyn_cast(D);
+if (!MD || !MD->isThisDeclarationADefinition()) {
+  auto DC = dyn_cast(D);
+  if (DC && DC->hasExternalLexicalStorage()) {
+ColorScope Color(OS, ShowColors, UndeserializedColor);
+OS << "";
+  }
+}
+  }
 }
 
 void TextNodeDumper::Visit(const CXXCtorInitializer *Init) {
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -355,18 +355,8 @@
 }
 
 void ASTDumper::dumpDeclContext(const DeclContext *DC) {
-  if (!DC)
-return;
-
   for (auto *D : (Deserialize ? DC->decls() : DC->noload_decls()))
 dumpDecl(D);
-
-  if (DC->hasExternalLexicalStorage()) {
-dumpChild([=] {
-  ColorScope Color(OS, ShowColors, UndeserializedColor);
-  OS << "";
-});
-  }
 }
 
 void ASTDumper::dumpLookups(const DeclContext *DC, bool DumpDecls) {
@@ -514,10 +504,7 @@
 // Decls within functions are visited by the body.
 if (!isa(*D) && !isa(*D)) {
   auto DC = dyn_cast(D);
-  if (DC &&
-  (DC->hasExternalLexicalStorage() ||
-   (Deserialize ? DC->decls_begin() != DC->decls_end()
-: DC->noload_decls_begin() != DC->noload_decls_end(
+  if (DC)
 dumpDeclContext(DC);
 }
   });
@@ -1238,12 +1225,11 @@
   if (D->isVariadic())
 OS << " variadic";
 
-  if (D->isThisDeclarationADefinition()) {
+  if (isa(D) && D->isThisDeclarationADefinition())
 dumpDeclContext(D);
-  } else {
+  else
 for (const ParmVarDecl *Parameter : D->parameters())
   dumpDecl(Parameter);
-  }
 
   if (D->hasBody())
 dumpStmt(D->getBody());


Index: lib/AST/TextNodeDumper.cpp
===
--- lib/AST/TextNodeDumper.cpp
+++ lib/AST/TextNodeDumper.cpp
@@ -256,6 +256,17 @@
   if (const FunctionDecl *FD = dyn_cast(D))
 if (FD->isConstexpr())
   OS << " constexpr";
+
+  if (!isa(*D)) {
+auto MD = dyn_cast(D);
+if (!MD || !MD->isThisDeclarationADefinition()) {
+  auto DC = dyn_cast(D);
+  if (DC && DC->hasExternalLexicalStorage()) {
+ColorScope Color(OS, ShowColors, UndeserializedColor);
+OS << "";
+  }
+}
+  }
 }
 
 void TextNodeDumper::Visit(const CXXCtorInitializer *Init) {
Index: lib/AST/ASTDumper.cpp
===
--- lib/AST/ASTDumper.cpp
+++ lib/AST/ASTDumper.cpp
@@ -355,18 +355,8 @@
 }
 
 void ASTDumper::dumpDeclContext(const DeclContext *DC) {
-  if (!DC)
-return;
-
   for (auto *D : (Deserialize ? DC->decls() : DC->noload_decls()))
 dumpDecl(D);
-
-  if (DC->hasExternalLexicalStorage()) {
-dumpChild([=] {
-  ColorScope Color(OS, ShowColors, UndeserializedColor);
-  OS << "";
-});
-  }
 }
 
 void ASTDumper::dumpLookups(const DeclContext *DC, bool DumpDecls) {
@@ -514,10 +504,7 @@
 // Decls within functions are visited by the body.
 if (!isa(*D) && !isa(*D)) {
   auto DC = dyn_cast(D);
-  if (DC &&
-  (DC->hasExternalLexicalStorage() ||
-   (Deserialize ? DC->decls_begin() != DC->decls_end()
-: DC->noload_decls_begin() != DC->noload_decls_end(
+  if (DC)
 dumpDeclContext(DC);
 }
   });
@@ -1238,12 +1225,11 @@
   if (D->isVariadic())
 OS << " variadic";
 
-  if (D->isThisDeclarationADefinition()) {
+  if (isa(D) && D->isThisDeclarationADefinition())
 dumpDeclContext(D);
-  } else {
+  else
 for (const ParmVarDecl *Parameter : D->parameters())
   dumpDecl(Parameter);
-  }
 
   if (D->hasBody())
 dumpStmt(D->getBody());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r351316 - Reapply [Tooling] Make clang-tool find libc++ dir on mac when running on a file without compilation database.

[Hans Wennborg via cfe-commits]

On Wed, Jan 16, 2019 at 7:46 PM Greg Bedwell  wrote:
>
> > > Unfortunately this trips up a variety of buildbots:
> > Platform REQUIRES added in r351360, please do revert & fix if this test is 
> > supposed to work elsewhere.
>
> Looks like the test fail just made it into the release-8.0.0 branch by a few 
> commits.  I think we'll need Jeremy's fix r351360 or/and any subsequent 'more 
> correct' test updates (if required) to get check-all on the branch into a 
> passing state.

Thanks for letting me know. I've merged that one in r351419. Please
let me know if there are more follow-ups.



> On Wed, 16 Jan 2019 at 17:49, Jeremy Morse via cfe-commits 
>  wrote:
>>
>> Platform REQUIRES added in r351360, please do revert & fix if this test is 
>> supposed to work elsewhere.
>>
>> --
>> Thanks,
>> Jeremy
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r351340 - [MSP430] Fix msp430-toolchain.c on Windows (added in r351228)

[Hans Wennborg via cfe-commits]

Merged to 8.0 in r351420.

On Wed, Jan 16, 2019 at 2:32 PM Anton Korobeynikov via cfe-commits
 wrote:
>
> Author: asl
> Date: Wed Jan 16 05:28:30 2019
> New Revision: 351340
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351340&view=rev
> Log:
> [MSP430] Fix msp430-toolchain.c on Windows (added in r351228)
>
> Patch by Kristina Bessonova!
>
> Differential Revision: https://reviews.llvm.org/D56776
>
>
> Modified:
> cfe/trunk/test/Driver/msp430-toolchain.c
>
> Modified: cfe/trunk/test/Driver/msp430-toolchain.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/msp430-toolchain.c?rev=351340&r1=351339&r2=351340&view=diff
> ==
> --- cfe/trunk/test/Driver/msp430-toolchain.c (original)
> +++ cfe/trunk/test/Driver/msp430-toolchain.c Wed Jan 16 05:28:30 2019
> @@ -8,44 +8,44 @@
>  // RUN:   --gcc-toolchain=%S/Inputs/basic_msp430_tree 2>&1 \
>  // RUN:   | FileCheck -check-prefix=MSP430 %s
>
> -// MSP430: 
> "{{.*}}Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../bin/msp430-elf-ld"
> +// MSP430: 
> "{{.*}}Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}bin{{/|}}msp430-elf-ld"
>  // MSP430: "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430"
> -// MSP430: 
> "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../msp430-elf/lib/430"
> -// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../msp430-elf/lib/430/crt0.o"
> -// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430/crtbegin.o"
> +// MSP430: 
> "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}msp430-elf{{/|}}lib/430"
> +// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}msp430-elf{{/|}}lib/430{{/|}}crt0.o"
> +// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430{{/|}}crtbegin.o"
>  // MSP430: "--start-group" "-lmul_none" "-lgcc" "-lc" "-lcrt" "-lnosys" 
> "--end-group"
> -// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430/crtend.o"
> -// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../msp430-elf/lib/430/crtn.o"
> +// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430{{/|}}crtend.o"
> +// MSP430: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}msp430-elf{{/|}}lib/430{{/|}}crtn.o"
>
>  // RUN: %clang %s -### -no-canonical-prefixes -target msp430 -nodefaultlibs \
>  // RUN:   --gcc-toolchain=%S/Inputs/basic_msp430_tree 2>&1 \
>  // RUN:   | FileCheck -check-prefix=MSP430-NO-DFT-LIB %s
>
> -// MSP430-NO-DFT-LIB: 
> "{{.*}}Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../bin/msp430-elf-ld"
> +// MSP430-NO-DFT-LIB: 
> "{{.*}}Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}bin{{/|}}msp430-elf-ld"
>  // MSP430-NO-DFT-LIB: 
> "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430"
> -// MSP430-NO-DFT-LIB: 
> "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../msp430-elf/lib/430"
> -// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../msp430-elf/lib/430/crt0.o"
> -// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430/crtbegin.o"
> +// MSP430-NO-DFT-LIB: 
> "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}msp430-elf{{/|}}lib/430"
> +// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}msp430-elf{{/|}}lib/430{{/|}}crt0.o"
> +// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430{{/|}}crtbegin.o"
>  // MSP430-NO-DFT-LIB: "--start-group" "-lmul_none" "-lgcc" "--end-group"
> -// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430/crtend.o"
> -// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../msp430-elf/lib/430/crtn.o"
> +// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/430{{/|}}crtend.o"
> +// MSP430-NO-DFT-LIB: 
> "{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}msp430-elf{{/|}}lib/430{{/|}}crtn.o"
>
>  // RUN: %clang %s -### -no-canonical-prefixes -target msp430 -nostartfiles \
>  // RUN:   --gcc-toolchain=%S/Inputs/basic_msp430_tree 2>&1 \
>  // RUN:   | FileCheck -check-prefix=MSP430-NO-START %s
>
> -// MSP430-NO-START: 
> "{{.*}}Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../../../bin/msp430-elf-ld"
> +// MSP430-NO-START: 
> "{{.*}}Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/../../..{{/|}}..{{/|}}bin{{/|}}msp430-elf-ld"
>  // MSP430-NO-START: 
> "-L{{.*}}/Inputs/basic_msp430_tree/lib/gcc/msp430-elf/7.3.1/4

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Mostly just comments on Tweak, I think the comments on other parts still apply)




Comment at: clangd/SourceCode.h:58
 
+/// Return the file location, corresponding to \p P. Note that one should take
+/// care to avoid comparing the result with expansion locations.

naming nits:
- this should mention "main-file"
- doesn't need to mention "position" as it's just the param type
- 'sourceLoc' seems an odd abbreviation

Maybe `sourceLocationInMainFile`?

(Current name is consistent with `sourceLocToPosition` but that name is bad 
too...)



Comment at: clangd/refactor/Tweak.cpp:38
+namespace {
+const llvm::StringMap()>> &
+tweaksRegistry() {

Can we drop this indirection and use the registry directly?



Comment at: clangd/refactor/Tweak.cpp:62
+  }
+  // Ensure deterministic order of the results.
+  llvm::sort(Available,

(nit: I'd make this `operator<` of `Tweak`? e.g. if priority is added, it 
should be private)



Comment at: clangd/refactor/Tweak.h:78
+/// Contains all actions supported by clangd.
+typedef llvm::Registry TweakRegistry;
+

nit: can we move this to the cpp file and out of the public interface?
(or keep the registry public, but drop prepareTweak[s] and just make callers do 
it)



Comment at: clangd/refactor/Tweak.h:93
+// If prepare() returns true, return the corresponding tweak.
+std::unique_ptr prepareTweak(TweakID ID, const Tweak::Selection &S);
+

should this return ErrorOr> so we can treat both missing ID 
and prepare failed as errors with messages?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267



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

[PATCH] D55782: Fix isInSystemMacro to handle pasted token

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

@rsmith up!


Repository:
  rC Clang

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

https://reviews.llvm.org/D55782



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

[PATCH] D56830: Prototype for include-fixer workflow in clangd. [NOT FOR REVIEW]

[Eric Liu via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56830

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -38,8 +38,8 @@
 class TUSchedulerTests : public ::testing::Test {
 protected:
   ParseInputs getInputs(PathRef File, std::string Contents) {
-return ParseInputs{*CDB.getCompileCommand(File),
-   buildTestFS(Files, Timestamps), std::move(Contents)};
+return ParseInputs(*CDB.getCompileCommand(File),
+   buildTestFS(Files, Timestamps), std::move(Contents));
   }
 
   void updateWithCallback(TUScheduler &S, PathRef File,
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -361,10 +361,10 @@
   /*StoreInMemory=*/true,
   [&](ASTContext &Ctx, std::shared_ptr PP) {});
   // Build AST for main file with preamble.
-  auto AST =
-  ParsedAST::build(createInvocationFromCommandLine(Cmd), PreambleData,
-   llvm::MemoryBuffer::getMemBufferCopy(Main.code()),
-   std::make_shared(), PI.FS);
+  auto AST = ParsedAST::build(
+  createInvocationFromCommandLine(Cmd), PreambleData,
+  llvm::MemoryBuffer::getMemBufferCopy(Main.code()),
+  std::make_shared(), PI.FS, nullptr);
   ASSERT_TRUE(AST);
   FileIndex Index;
   Index.updateMain(MainFile, *AST);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2320,6 +2320,26 @@
   EXPECT_THAT(C, ElementsAre(SnippetSuffix("${1:(unsigned int)}")));
 }
 
+TEST(CompletionTest, CorrectTypo) {
+  auto Results = completions(R"cpp(
+namespace nx {
+namespace nz {
+void f() {
+  nx::Clangd x;
+  ::Clangg y;
+  Clangc xx;
+  plus(Clangd, some());
+  ^
+}
+}
+}
+  )cpp",
+ {cls("nx::Clangd1"), cls("nx::Clangd2")});
+  // Although Clangd1 is from another namespace, Sema tells us it's in-scope and
+  // needs no qualifier.
+  EXPECT_THAT(Results.Completions, UnorderedElementsAre());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/Diagnostics.h
===
--- clangd/Diagnostics.h
+++ clangd/Diagnostics.h
@@ -108,6 +108,8 @@
   llvm::Optional LastDiag;
 };
 
+Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L);
+
 } // namespace clangd
 } // namespace clang
 
Index: clangd/Diagnostics.cpp
===
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -46,37 +46,6 @@
   return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd());
 }
 
-// Clang diags have a location (shown as ^) and 0 or more ranges ().
-// LSP needs a single range.
-Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
-  auto &M = D.getSourceManager();
-  auto Loc = M.getFileLoc(D.getLocation());
-  for (const auto &CR : D.getRanges()) {
-auto R = Lexer::makeFileCharRange(CR, M, L);
-if (locationInRange(Loc, R, M))
-  return halfOpenToRange(M, R);
-  }
-  llvm::Optional FallbackRange;
-  // The range may be given as a fixit hint instead.
-  for (const auto &F : D.getFixItHints()) {
-auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L);
-if (locationInRange(Loc, R, M))
-  return halfOpenToRange(M, R);
-// If there's a fixit that performs insertion, it has zero-width. Therefore
-// it can't contain the location of the diag, but it might be possible that
-// this should be reported as range. For example missing semicolon.
-if (R.getBegin() == R.getEnd() && Loc == R.getBegin())
-  FallbackRange = halfOpenToRange(M, R);
-  }
-  if (FallbackRange)
-return *FallbackRange;
-  // If no suitable range is found, just use the token at the location.
-  auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
-  if (!R.isValid()) // Fall back to location only, let the editor deal with it.
-R = CharSourceRange::getCharRange(Loc);
-  return halfOpenToRange(M, R);
-}
-
 bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
   return Loc.isValid() && M.isWrittenInMainFile(M.getFileLoc(Loc));
 }
@@ -188,6 +157,38 @@
 }
 } // namespace
 
+
+// Cl

r351423 - [NewPM] Add -fsanitize={memory,thread} handling to clang

[Philip Pfaffe via cfe-commits]

Author: pfaffe
Date: Thu Jan 17 02:10:47 2019
New Revision: 351423

URL: http://llvm.org/viewvc/llvm-project?rev=351423&view=rev
Log:
[NewPM] Add -fsanitize={memory,thread} handling to clang

Summary: This is the missing bit to drive thread and memory sanitizers through 
clang using the new PassManager.

Reviewers: chandlerc, fedor.sergeev, vitalybuka, leonardchan

Subscribers: bollu, llvm-commits

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

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

Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=351423&r1=351422&r2=351423&view=diff
==
--- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
+++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Thu Jan 17 02:10:47 2019
@@ -1015,11 +1015,22 @@ void EmitAssemblyHelper::EmitAssemblyWit
 
   // Register callbacks to schedule sanitizer passes at the appropriate 
part of
   // the pipeline.
+  // FIXME: either handle asan/the remaining sanitizers or error out
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
 PB.registerScalarOptimizerLateEPCallback(
 [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
   FPM.addPass(BoundsCheckingPass());
 });
+  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
+PB.registerOptimizerLastEPCallback(
+[](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
+  FPM.addPass(MemorySanitizerPass());
+});
+  if (LangOpts.Sanitize.has(SanitizerKind::Thread))
+PB.registerOptimizerLastEPCallback(
+[](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
+  FPM.addPass(ThreadSanitizerPass());
+});
   if (Optional Options = getGCOVOptions(CodeGenOpts))
 PB.registerPipelineStartEPCallback([Options](ModulePassManager &MPM) {
   MPM.addPass(GCOVProfilerPass(*Options));


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

[PATCH] D56831: [NewPM] Add -fsanitize={memory, thread} handling to clang

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC351423: [NewPM] Add -fsanitize={memory,thread} handling to 
clang (authored by pfaffe, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56831?vs=18&id=182230#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56831

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -1015,11 +1015,22 @@
 
   // Register callbacks to schedule sanitizer passes at the appropriate 
part of
   // the pipeline.
+  // FIXME: either handle asan/the remaining sanitizers or error out
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
 PB.registerScalarOptimizerLateEPCallback(
 [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
   FPM.addPass(BoundsCheckingPass());
 });
+  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
+PB.registerOptimizerLastEPCallback(
+[](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
+  FPM.addPass(MemorySanitizerPass());
+});
+  if (LangOpts.Sanitize.has(SanitizerKind::Thread))
+PB.registerOptimizerLastEPCallback(
+[](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) 
{
+  FPM.addPass(ThreadSanitizerPass());
+});
   if (Optional Options = getGCOVOptions(CodeGenOpts))
 PB.registerPipelineStartEPCallback([Options](ModulePassManager &MPM) {
   MPM.addPass(GCOVProfilerPass(*Options));


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -1015,11 +1015,22 @@
 
   // Register callbacks to schedule sanitizer passes at the appropriate part of
   // the pipeline.
+  // FIXME: either handle asan/the remaining sanitizers or error out
   if (LangOpts.Sanitize.has(SanitizerKind::LocalBounds))
 PB.registerScalarOptimizerLateEPCallback(
 [](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) {
   FPM.addPass(BoundsCheckingPass());
 });
+  if (LangOpts.Sanitize.has(SanitizerKind::Memory))
+PB.registerOptimizerLastEPCallback(
+[](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) {
+  FPM.addPass(MemorySanitizerPass());
+});
+  if (LangOpts.Sanitize.has(SanitizerKind::Thread))
+PB.registerOptimizerLastEPCallback(
+[](FunctionPassManager &FPM, PassBuilder::OptimizationLevel Level) {
+  FPM.addPass(ThreadSanitizerPass());
+});
   if (Optional Options = getGCOVOptions(CodeGenOpts))
 PB.registerPipelineStartEPCallback([Options](ModulePassManager &MPM) {
   MPM.addPass(GCOVProfilerPass(*Options));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56836: [mips] Include whole lpthread when using both -pthread and -static

[Aleksandar Beserminji via Phabricator via cfe-commits]

abeserminji created this revision.
abeserminji added reviewers: petarj, atanasyan, MatzeB, mstojanovic.
Herald added subscribers: jfb, arichardson, sdardis, srhines.

When executing test-suite tests with -static option, we get stdthreadbug test 
to fail.
Sometimes it segfault, sometimes it gets stuck in an infinite loop.

The reason for such behavior is that pthread library uses weak symbols for some 
functions
(ex. __pthread_mutex_lock), and such functions are not included in the 
executable when
-static option is used. To force the linker to include these functions as well, 
we use the following
options: //-Wl,--whole-archive -lpthread -Wl,--no-whole-archive//.

I proposed adding this options to the test-suite for the stdthreadbug test here 
D52878 , 
and got a recommendation to try to teach the clang to handle this case by 
always including
given options when -pthread is used.

This patch does exactly that.

My concern about this is that we are forcing whole pthread into an executable, 
and user
does not have an option do decide if he wants to use lpthread or something 
else. Now,
I am not sure what exactly that something else could be, and maybe there isn't 
something else
and my concerns aren't justified.


Repository:
  rC Clang

https://reviews.llvm.org/D56836

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChains/Gnu.cpp


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -501,6 +501,7 @@
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
+  bool WantLPthread = Args.hasArg(options::OPT_lpthread);
 
   // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
   // require librt. Most modern Linux platforms do, but some may not.
@@ -513,8 +514,35 @@
 
   AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
-  if (WantPthread && !isAndroid)
-CmdArgs.push_back("-lpthread");
+  if ((WantPthread || WantLPthread) && !isAndroid) {
+if (Triple.isMIPS() && Args.hasArg(options::OPT_static)) {
+  // When -pthread option is used in combination with -static,
+  // we want to avoid problems which weak symbols may cause,
+  // and therefore we include whole lpthread library
+  bool wholeArchiveFlag = false;
+  bool lpthreadAsWholeArchive = false;
+  for (Arg *A : Args) {
+if (A->getOption().matches(options::OPT_whole_archive))
+  wholeArchiveFlag = true;
+if (A->getOption().matches(options::OPT_no_whole_archive))
+  wholeArchiveFlag = false;
+if (wholeArchiveFlag) {
+  if (A->getOption().matches(options::OPT_lpthread)) {
+lpthreadAsWholeArchive = true;
+break;
+  }
+}
+  }
+
+  if (!lpthreadAsWholeArchive) {
+CmdArgs.push_back("--whole-archive");
+CmdArgs.push_back("-lpthread");
+CmdArgs.push_back("--no-whole-archive");
+  }
+} else {
+  CmdArgs.push_back("-lpthread");
+}
+  }
 
   if (Args.hasArg(options::OPT_fsplit_stack))
 CmdArgs.push_back("--wrap=pthread_create");
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -2491,6 +2491,7 @@
 def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>,
   HelpText<"Support POSIX threads in generated code">;
 def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
+def lpthread : Flag<["-"], "lpthread">;
 def p : Flag<["-"], "p">;
 def pie : Flag<["-"], "pie">;
 def read__only__relocs : Separate<["-"], "read_only_relocs">;
@@ -2602,6 +2603,8 @@
 def weak__library : Separate<["-"], "weak_library">, Flags<[LinkerInput]>;
 def weak__reference__mismatches : Separate<["-"], "weak_reference_mismatches">;
 def whatsloaded : Flag<["-"], "whatsloaded">;
+def whole_archive : Flag<["--"], "whole-archive">, Group;
+def no_whole_archive : Flag<["--"], "no-whole-archive">, Group;
 def whyload : Flag<["-"], "whyload">;
 def w : Flag<["-"], "w">, HelpText<"Suppress all warnings">, 
Flags<[CC1Option]>;
 def x : JoinedOrSeparate<["-"], "x">, Flags<[DriverOption,CC1Option]>,


Index: lib/Driver/ToolChains/Gnu.cpp
===
--- lib/Driver/ToolChains/Gnu.cpp
+++ lib/Driver/ToolChains/Gnu.cpp
@@ -501,6 +501,7 @@
 
   bool WantPthread = Args.hasArg(options::OPT_pthread) ||
  Args.hasArg(options::OPT_pthreads);
+  bool WantLPthread = Args.hasArg(options::OPT_lpthread);
 
   // FIXME: Only pass GompNeedsRT = true for platforms with libgomp that
   // require librt. Most modern Linux platforms do, but some

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

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

xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
Herald added subscribers: donat.nagy, Szelethus, rnkovacs, baloghadamsoftware.

LGTM!

Any objections to commit this? 
I think this is quiet coding guideline specific check which is useful for a set 
of  security critical projects. As this is an opt in kind of check, I think it 
does no harm to have it upstream.


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

https://reviews.llvm.org/D35068



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

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

[Marina Kalashina via Phabricator via cfe-commits]

MarinaKalashina added a comment.

@Eugene.Zelenko  @alexfh Thank you! How should I proceed with the revision now? 
Do I get it right from 
https://llvm.org/docs/Phabricator.html#committing-a-change, that without commit 
access, I need to ask for committing the changes for me?


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

https://reviews.llvm.org/D54945



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Some projects make use of clang plugins when building, but clangd is
not aware of those plugins therefore can't work with the same compile command
arguments.

We invoke clang in two different codepaths one in backgroundindex and
another when building asts. This patch adds the filtering logic to their
intersection.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56841

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdTests.cpp


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -1037,6 +1037,27 @@
 }
 #endif
 
+TEST_F(ClangdVFSTest, FlagsWithPlugins) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {
+  "-Xclang",
+  "-add-plugin",
+  "-Xclang",
+  "random-plugin",
+  };
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  const auto SourceContents = "int main() { return 0; }";
+  FS.Files[FooCpp] = FooCpp;
+  Server.addDocument(FooCpp, SourceContents);
+  auto Result = dumpASTWithoutMemoryLocs(Server, FooCpp);
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  EXPECT_NE(Result, "");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -425,8 +425,21 @@
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs &Inputs) {
   std::vector ArgStrs;
-  for (const auto &S : Inputs.CompileCommand.CommandLine)
-ArgStrs.push_back(S.c_str());
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html
+// -Xclang {-load, -plugin, -plugin-arg-, -add-plugin} -Xclang
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||
+ llvm::StringRef(CommandLine[I + 1]).startswith("-plugin-arg-") ||
+ CommandLine[I + 1] == "-add-plugin") &&
+CommandLine[I + 2] == "-Xclang") {
+  I += 3;
+  continue;
+}
+ArgStrs.push_back(CommandLine[I].c_str());
+  }
 
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
 log("Couldn't set working directory when creating compiler invocation.");


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -1037,6 +1037,27 @@
 }
 #endif
 
+TEST_F(ClangdVFSTest, FlagsWithPlugins) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {
+  "-Xclang",
+  "-add-plugin",
+  "-Xclang",
+  "random-plugin",
+  };
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  const auto SourceContents = "int main() { return 0; }";
+  FS.Files[FooCpp] = FooCpp;
+  Server.addDocument(FooCpp, SourceContents);
+  auto Result = dumpASTWithoutMemoryLocs(Server, FooCpp);
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  EXPECT_NE(Result, "");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -425,8 +425,21 @@
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs &Inputs) {
   std::vector ArgStrs;
-  for (const auto &S : Inputs.CompileCommand.CommandLine)
-ArgStrs.push_back(S.c_str());
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html
+// -Xclang {-load, -plugin, -plugin-arg-, -add-plugin} -Xclang
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||
+ llvm::StringRef(CommandLine[I + 1]).startswith("-plugin-arg-") ||
+ CommandLine[I + 1] == "-add-plugin") &&
+CommandLine[I + 2] == "-Xclang") {
+  I += 3;
+  continue;
+}
+ArgStrs.push_back(CommandLine[I].c_str());
+  }
 
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
 log("Couldn't set working directory when creating compiler invocation.");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.

[PATCH] D56836: [mips] Include whole lpthread when using both -pthread and -static

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan added a comment.

Now I'm not sure that we really need to make this code more complicated. As to 
the `stdthreadbug` test case, maybe we can just exclude this test if LDFLAGS 
contains the `-static` option.

As to this patch, is it possible to implement the following algorithm? Will it 
be more easy and clear?

1. if there is no `-pthread` or `-pthreads` flag, do nothing.
2. if the flag provided, iterate over `-l` options (do not introduce new 
`lpthread` first-class argument). If there is the `-lpthread` (with or without 
`whole-archive` surrounds), consider a user know what he/she wants to do.
3. if there is no `-lpthread`, pass `--whole-archive -lpthread 
--no-whole-archive` to the linker.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56836



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdUnit.cpp:429
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html

This doesn't seem to be clangd specific; clang-tidy seems to have the same 
issue. Could we share the filtering logic (e.g. in 
lib/Tooling/ArgumentsAdjusters.cpp)?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56735: [OpenCL] Fix overloading ranking rules to work correctly for address space conversions

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 182245.
Anastasia added a comment.

Improved test.


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

https://reviews.llvm.org/D56735

Files:
  lib/Sema/SemaOverload.cpp
  test/SemaOpenCLCXX/address_space_overloading.cl


Index: test/SemaOpenCLCXX/address_space_overloading.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address_space_overloading.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
+
+// expected-no-diagnostics
+
+struct RetGlob {
+  int dummy;
+};
+
+struct RetGen {
+  char dummy;
+};
+
+RetGlob foo(const __global int *);
+RetGen foo(const __generic int *);
+
+void kernel k() {
+  __global int *ArgGlob;
+  __generic int *ArgGen;
+  __local int *ArgLoc;
+  RetGlob TestGlob = foo(ArgGlob);
+  RetGen TestGen = foo(ArgGen);
+  TestGen = foo(ArgLoc);
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -4019,9 +4019,12 @@
 // to unwrap. This essentially mimics what
 // IsQualificationConversion does, but here we're checking for a
 // strict subset of qualifiers.
-if (T1.getCVRQualifiers() == T2.getCVRQualifiers())
+if (T1.getQualifiers().withoutObjCLifetime() ==
+T2.getQualifiers().withoutObjCLifetime())
   // The qualifiers are the same, so this doesn't tell us anything
   // about how the sequences rank.
+  // ObjC ownership quals are omitted above as they interfere with
+  // the ARC overload rule.
   ;
 else if (T2.isMoreQualifiedThan(T1)) {
   // T1 has fewer qualifiers, so it could be the better sequence.


Index: test/SemaOpenCLCXX/address_space_overloading.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address_space_overloading.cl
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
+
+// expected-no-diagnostics
+
+struct RetGlob {
+  int dummy;
+};
+
+struct RetGen {
+  char dummy;
+};
+
+RetGlob foo(const __global int *);
+RetGen foo(const __generic int *);
+
+void kernel k() {
+  __global int *ArgGlob;
+  __generic int *ArgGen;
+  __local int *ArgLoc;
+  RetGlob TestGlob = foo(ArgGlob);
+  RetGen TestGen = foo(ArgGen);
+  TestGen = foo(ArgLoc);
+}
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -4019,9 +4019,12 @@
 // to unwrap. This essentially mimics what
 // IsQualificationConversion does, but here we're checking for a
 // strict subset of qualifiers.
-if (T1.getCVRQualifiers() == T2.getCVRQualifiers())
+if (T1.getQualifiers().withoutObjCLifetime() ==
+T2.getQualifiers().withoutObjCLifetime())
   // The qualifiers are the same, so this doesn't tell us anything
   // about how the sequences rank.
+  // ObjC ownership quals are omitted above as they interfere with
+  // the ARC overload rule.
   ;
 else if (T2.isMoreQualifiedThan(T1)) {
   // T1 has fewer qualifiers, so it could be the better sequence.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24461: CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[Michael Skvortsov via Phabricator via cfe-commits]

mskvortsov added a reviewer: asl.
mskvortsov added a subscriber: asl.
mskvortsov added a comment.

@asl The change enables to build newlib. Could you commit this?


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

https://reviews.llvm.org/D24461



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added subscribers: sammccall, klimek.
ilya-biryukov added a comment.

LG, just a few comments. Supporting compiler plugins in clangd is non-trivial 
and we don't support them so far, so filtering out the options make sense.

It feels these helper belong in the Tooling library, that would allow to reuse 
code between clang-tidy, clangd, etc. Could even be the default behavior, since 
most of the tools won't handle compiler plugins well, unless compiled with the 
same exact version of the compiler.
I'd be interested in what other people think, @sammccall, @klimek?




Comment at: clangd/ClangdUnit.cpp:429
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html

ioeric wrote:
> This doesn't seem to be clangd specific; clang-tidy seems to have the same 
> issue. Could we share the filtering logic (e.g. in 
> lib/Tooling/ArgumentsAdjusters.cpp)?
We need a comment mentioning that we are filtering out the plugin options and 
why we're doing that here



Comment at: clangd/ClangdUnit.cpp:433
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||

Maybe move this code into `ClangdServer::getCompileCommand` to keep **all** 
argument adjustments that we do in clangd in one place.
We add `-resource-dir` there.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56445: [Sema] Teach Clang that aligned allocation is not supported with macosx10.13

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

This (I think) caused a bunch of libc++ tests to start failing during testing 
of the newly created 8.0 branch on my machine, see 
https://bugs.llvm.org/show_bug.cgi?id=40354


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56445



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

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

That really looks awesome, thanks!

One general comment: I am not really sure if the handling done in 
`ParseCastExpression` is extensive enough. But IIUC, this should not cause any 
regressions, rather should result in missing preferred types just as before 
this patch ?




Comment at: lib/Parse/ParseExpr.cpp:1157
 
   case tok::kw_co_await: {  // unary-expression: 'co_await' cast-expression
 SourceLocation CoawaitLoc = ConsumeToken();

shouldn't we have an enterunknown here as well?



Comment at: lib/Sema/SemaCodeComplete.cpp:352
+PreferredTypeBuilder::RestoreRAII
+PreferredTypeBuilder::update(llvm::function_ref Updater) {
+  RestoreRAII R(*this);

I am not sure it carries the weight, why not just inline it?



Comment at: lib/Sema/SemaCodeComplete.cpp:360
+  return update([&]() {
+if (isa(S.CurContext)) {
+  if (sema::BlockScopeInfo *BSI = S.getCurBlock())

Is this check necessary? According to comments `getCurBlock` returns null if 
there is no block.



Comment at: lib/Sema/SemaCodeComplete.cpp:373
+}
+Type = QualType();
+  });

nit: maybe move this into top and get rid of the return statements inside 
`if`s(also by changing them to `else if`s)



Comment at: lib/Sema/SemaCodeComplete.cpp:380
+  return update([&]() {
+auto *VD = llvm::dyn_cast_or_null(D);
+Type = VD ? VD->getType() : QualType();

Is it really possible for D to be null ?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56723



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

[PATCH] D56829: Move decl context dumping to TextNodeDumper

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

It appears we have no test coverage for this case -- can you introduce some?




Comment at: lib/AST/ASTDumper.cpp:506-507
 if (!isa(*D) && !isa(*D)) {
   auto DC = dyn_cast(D);
-  if (DC &&
-  (DC->hasExternalLexicalStorage() ||
-   (Deserialize ? DC->decls_begin() != DC->decls_end()
-: DC->noload_decls_begin() != DC->noload_decls_end(
+  if (DC)
 dumpDeclContext(DC);

Might as well clean this up to be `if (const auto *DC = 
dyn_cast(D))`



Comment at: lib/AST/ASTDumper.cpp:519-520
-  (DC->hasExternalLexicalStorage() ||
-   (Deserialize ? DC->decls_begin() != DC->decls_end()
-: DC->noload_decls_begin() != DC->noload_decls_end(
 dumpDeclContext(DC);

Why did this condition get dropped?



Comment at: lib/AST/ASTDumper.cpp:1228
 
-  if (D->isThisDeclarationADefinition()) {
+  if (isa(D) && D->isThisDeclarationADefinition())
 dumpDeclContext(D);

Why is the isa<> test needed? `D` must be an `ObjCMethodDecl` which inherits 
from `DeclContext`, so this seems unneeded.



Comment at: lib/AST/TextNodeDumper.cpp:261
+  if (!isa(*D)) {
+auto MD = dyn_cast(D);
+if (!MD || !MD->isThisDeclarationADefinition()) {

`const auto *`



Comment at: lib/AST/TextNodeDumper.cpp:263
+if (!MD || !MD->isThisDeclarationADefinition()) {
+  auto DC = dyn_cast(D);
+  if (DC && DC->hasExternalLexicalStorage()) {

`const auto *`


Repository:
  rC Clang

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

https://reviews.llvm.org/D56829



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:429
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html

ilya-biryukov wrote:
> ioeric wrote:
> > This doesn't seem to be clangd specific; clang-tidy seems to have the same 
> > issue. Could we share the filtering logic (e.g. in 
> > lib/Tooling/ArgumentsAdjusters.cpp)?
> We need a comment mentioning that we are filtering out the plugin options and 
> why we're doing that here
+1 to the suggestion.
WDYT about landing it into clangd as a quick workaround and moving into tooling 
later?
The reason I think it might be better is because that would unblock usage of 
clangd in Chromium.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

Re: r351344 - [MSP430] Improve support of 'interrupt' attribute

[Hans Wennborg via cfe-commits]

Merged to 8.0 in r351441.

On Wed, Jan 16, 2019 at 2:47 PM Anton Korobeynikov via cfe-commits
 wrote:
>
> Author: asl
> Date: Wed Jan 16 05:44:01 2019
> New Revision: 351344
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351344&view=rev
> Log:
> [MSP430] Improve support of 'interrupt' attribute
>
> * Accept as an argument constants in range 0..63 (aligned with TI headers and 
> linker scripts provided with TI GCC toolchain).
> * Emit function attribute 'interrupt'='xx' instead of aliases (used in the 
> backend to create a section for particular interrupt vector).
> * Add more diagnostics.
>
> Patch by Kristina Bessonova!
>
> Differential Revision: https://reviews.llvm.org/D56663
>
> Added:
> cfe/trunk/test/CodeGen/attr-msp430.c
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/CodeGen/TargetInfo.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/test/Sema/attr-msp430.c
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=351344&r1=351343&r2=351344&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Jan 16 05:44:01 
> 2019
> @@ -274,6 +274,10 @@ def warn_riscv_interrupt_attribute : War
> "RISC-V 'interrupt' attribute only applies to functions that have "
> "%select{no parameters|a 'void' return type}0">,
> InGroup;
> +def warn_msp430_interrupt_attribute : Warning<
> +   "MSP430 'interrupt' attribute only applies to functions that have "
> +   "%select{no parameters|a 'void' return type}0">,
> +   InGroup;
>  def warn_unused_parameter : Warning<"unused parameter %0">,
>InGroup, DefaultIgnore;
>  def warn_unused_variable : Warning<"unused variable %0">,
>
> Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=351344&r1=351343&r2=351344&view=diff
> ==
> --- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Jan 16 05:44:01 2019
> @@ -6774,21 +6774,19 @@ void MSP430TargetCodeGenInfo::setTargetA
>if (GV->isDeclaration())
>  return;
>if (const FunctionDecl *FD = dyn_cast_or_null(D)) {
> -if (const MSP430InterruptAttr *attr = 
> FD->getAttr()) {
> -  // Handle 'interrupt' attribute:
> -  llvm::Function *F = cast(GV);
> +const auto *InterruptAttr = FD->getAttr();
> +if (!InterruptAttr)
> +  return;
>
> -  // Step 1: Set ISR calling convention.
> -  F->setCallingConv(llvm::CallingConv::MSP430_INTR);
> +// Handle 'interrupt' attribute:
> +llvm::Function *F = cast(GV);
>
> -  // Step 2: Add attributes goodness.
> -  F->addFnAttr(llvm::Attribute::NoInline);
> +// Step 1: Set ISR calling convention.
> +F->setCallingConv(llvm::CallingConv::MSP430_INTR);
>
> -  // Step 3: Emit ISR vector alias.
> -  unsigned Num = attr->getNumber() / 2;
> -  llvm::GlobalAlias::create(llvm::Function::ExternalLinkage,
> -"__isr_" + Twine(Num), F);
> -}
> +// Step 2: Add attributes goodness.
> +F->addFnAttr(llvm::Attribute::NoInline);
> +F->addFnAttr("interrupt", llvm::utostr(InterruptAttr->getNumber()));
>}
>  }
>
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=351344&r1=351343&r2=351344&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jan 16 05:44:01 2019
> @@ -5377,6 +5377,27 @@ static void handleARMInterruptAttr(Sema
>  }
>
>  static void handleMSP430InterruptAttr(Sema &S, Decl *D, const ParsedAttr 
> &AL) {
> +  // MSP430 'interrupt' attribute is applied to
> +  // a function with no parameters and void return type.
> +  if (!isFunctionOrMethod(D)) {
> +S.Diag(D->getLocation(), diag::warn_attribute_wrong_decl_type)
> +<< "'interrupt'" << ExpectedFunctionOrMethod;
> +return;
> +  }
> +
> +  if (hasFunctionProto(D) && getFunctionOrMethodNumParams(D) != 0) {
> +S.Diag(D->getLocation(), diag::warn_msp430_interrupt_attribute)
> +<< 0;
> +return;
> +  }
> +
> +  if (!getFunctionOrMethodResultType(D)->isVoidType()) {
> +S.Diag(D->getLocation(), diag::warn_msp430_interrupt_attribute)
> +<< 1;
> +return;
> +  }
> +
> +  // The attribute takes one integer argument.
>if (!checkAttributeNumArgs(S, AL, 1))
>  return;
>
> @@ -5386,8 +5407,6 @@ static void handleMSP430InterruptAttr(Se
>  return;
>}
>
> -  // FIXME: Check for decl - it should be void ()(void).
> -
>Expr *NumPar

[PATCH] D56723: [CodeComplete] Propagate preferred types through parser in more cases

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for a quick response, @kadircet!
Leaving some first comments, will address the rest later.

In D56723#1361366 , @kadircet wrote:

> One general comment: I am not really sure if the handling done in 
> `ParseCastExpression` is extensive enough. But IIUC, this should not cause 
> any regressions, rather should result in missing preferred types just as 
> before this patch ?


Exactly, no regressions are expected, we should start providing a preferred 
type in more places after this change.
This change is aiming to establish a well-defined pattern on how to add these 
types, more changes will follow to actually propagate the types in more cases.




Comment at: lib/Sema/SemaCodeComplete.cpp:360
+  return update([&]() {
+if (isa(S.CurContext)) {
+  if (sema::BlockScopeInfo *BSI = S.getCurBlock())

kadircet wrote:
> Is this check necessary? According to comments `getCurBlock` returns null if 
> there is no block.
This was copy-pasted from the removed `codeCompleteReturn`. I'd keep it this 
way in the initial patch to make sure we don't change semantics in existing 
cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56723



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/ClangdUnit.cpp:429
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > This doesn't seem to be clangd specific; clang-tidy seems to have the 
> > > same issue. Could we share the filtering logic (e.g. in 
> > > lib/Tooling/ArgumentsAdjusters.cpp)?
> > We need a comment mentioning that we are filtering out the plugin options 
> > and why we're doing that here
> +1 to the suggestion.
> WDYT about landing it into clangd as a quick workaround and moving into 
> tooling later?
> The reason I think it might be better is because that would unblock usage of 
> clangd in Chromium.
> WDYT about landing it into clangd as a quick workaround and moving into 
> tooling later?
> The reason I think it might be better is because that would unblock usage of 
> clangd in Chromium.
I don't see why it would take much more effort to move the shared logic into 
tooling now? 


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

It seems like the most relevant place in tooling is ArgumentsAdjuster as 
@ioeric pointed out. Happy to move it to there if @sammccall and @klimek agrees 
as well.

As a side note `ArgumentsAdjuster`s unfortunately causes a copy of the original 
command line arguments. Not sure how important this factor is compared to 
spinning up a compiler instance, just wanted to point it out.




Comment at: clangd/ClangdUnit.cpp:433
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||

ilya-biryukov wrote:
> Maybe move this code into `ClangdServer::getCompileCommand` to keep **all** 
> argument adjustments that we do in clangd in one place.
> We add `-resource-dir` there.
As mentioned in the change description, we also have a different compiler 
invocation in `BackgroundIndex` which is not aware of 
`ClangdServer::getCompileCommand`


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 182260.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841

Files:
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdTests.cpp


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -1037,6 +1037,27 @@
 }
 #endif
 
+TEST_F(ClangdVFSTest, FlagsWithPlugins) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {
+  "-Xclang",
+  "-add-plugin",
+  "-Xclang",
+  "random-plugin",
+  };
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  const auto SourceContents = "int main() { return 0; }";
+  FS.Files[FooCpp] = FooCpp;
+  Server.addDocument(FooCpp, SourceContents);
+  auto Result = dumpASTWithoutMemoryLocs(Server, FooCpp);
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  EXPECT_NE(Result, "");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -425,8 +425,24 @@
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs &Inputs) {
   std::vector ArgStrs;
-  for (const auto &S : Inputs.CompileCommand.CommandLine)
-ArgStrs.push_back(S.c_str());
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// Skip plugin related command line arguments. Clangd does not support
+// plugins currently. Therefore it breaks if compiler tries to load 
plugins.
+// According to https://clang.llvm.org/docs/ClangPlugins.html plugin
+// arguments are in the form:
+// -Xclang {-load, -plugin, -plugin-arg-, -add-plugin} -Xclang
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||
+ llvm::StringRef(CommandLine[I + 1]).startswith("-plugin-arg-") ||
+ CommandLine[I + 1] == "-add-plugin") &&
+CommandLine[I + 2] == "-Xclang") {
+  I += 3;
+  continue;
+}
+ArgStrs.push_back(CommandLine[I].c_str());
+  }
 
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
 log("Couldn't set working directory when creating compiler invocation.");


Index: unittests/clangd/ClangdTests.cpp
===
--- unittests/clangd/ClangdTests.cpp
+++ unittests/clangd/ClangdTests.cpp
@@ -1037,6 +1037,27 @@
 }
 #endif
 
+TEST_F(ClangdVFSTest, FlagsWithPlugins) {
+  MockFSProvider FS;
+  ErrorCheckingDiagConsumer DiagConsumer;
+  MockCompilationDatabase CDB;
+  CDB.ExtraClangFlags = {
+  "-Xclang",
+  "-add-plugin",
+  "-Xclang",
+  "random-plugin",
+  };
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  auto FooCpp = testPath("foo.cpp");
+  const auto SourceContents = "int main() { return 0; }";
+  FS.Files[FooCpp] = FooCpp;
+  Server.addDocument(FooCpp, SourceContents);
+  auto Result = dumpASTWithoutMemoryLocs(Server, FooCpp);
+  EXPECT_TRUE(Server.blockUntilIdleForTest()) << "Waiting for diagnostics";
+  EXPECT_NE(Result, "");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -425,8 +425,24 @@
 std::unique_ptr
 buildCompilerInvocation(const ParseInputs &Inputs) {
   std::vector ArgStrs;
-  for (const auto &S : Inputs.CompileCommand.CommandLine)
-ArgStrs.push_back(S.c_str());
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// Skip plugin related command line arguments. Clangd does not support
+// plugins currently. Therefore it breaks if compiler tries to load plugins.
+// According to https://clang.llvm.org/docs/ClangPlugins.html plugin
+// arguments are in the form:
+// -Xclang {-load, -plugin, -plugin-arg-, -add-plugin} -Xclang
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||
+ llvm::StringRef(CommandLine[I + 1]).startswith("-plugin-arg-") ||
+ CommandLine[I + 1] == "-add-plugin") &&
+CommandLine[I + 2] == "-Xclang") {
+  I += 3;
+  continue;
+}
+ArgStrs.push_back(CommandLine[I].c_str());
+  }
 
   if (Inputs.FS->setCurrentWorkingDirectory(Inputs.CompileCommand.Directory)) {
 log("Couldn't set working directory when creating compi

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:429
+  const auto &CommandLine = Inputs.CompileCommand.CommandLine;
+  for (size_t I = 0, E = CommandLine.size(); I != E; I++) {
+// According to https://clang.llvm.org/docs/ClangPlugins.html

ioeric wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > This doesn't seem to be clangd specific; clang-tidy seems to have the 
> > > > same issue. Could we share the filtering logic (e.g. in 
> > > > lib/Tooling/ArgumentsAdjusters.cpp)?
> > > We need a comment mentioning that we are filtering out the plugin options 
> > > and why we're doing that here
> > +1 to the suggestion.
> > WDYT about landing it into clangd as a quick workaround and moving into 
> > tooling later?
> > The reason I think it might be better is because that would unblock usage 
> > of clangd in Chromium.
> > WDYT about landing it into clangd as a quick workaround and moving into 
> > tooling later?
> > The reason I think it might be better is because that would unblock usage 
> > of clangd in Chromium.
> I don't see why it would take much more effort to move the shared logic into 
> tooling now? 
My approximation would be half an hour to finish this and land in clangd and 
2-3 hours to land this in tooling, update clangd, clang-tidy, etc.
Not a big difference, though, up to Kadir to actually quantify how hard he 
thinks this would be.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56786: [ASTMatchers] Changes to `CXXMemberExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D56786#1360706 , @steveire wrote:

> It seems that you update docs for existing matchers without changing those 
> matchers. You could put all of that in one patch.
>
> Then, you seem to add some tests for existing matches. You could put that in 
> the second patch.
>
> Then your third patch would add the new matcher with its tests and its docs.
>
> That would be easy to review.


Done:
Comments: https://reviews.llvm.org/D56849
Tests: https://reviews.llvm.org/D56850
New matcher with tests: https://reviews.llvm.org/D56851


Repository:
  rC Clang

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

https://reviews.llvm.org/D56786



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

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added subscribers: cfe-commits, lebedev.ri.
lebedev.ri added a comment.

Please always subscribe lists.
The reviews that happen without lists are null and void.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56849



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

[PATCH] D56850: [ASTMatchers] Add tests for assorted `CXXMemberCallExpr` matchers.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added subscribers: cfe-commits, lebedev.ri.
lebedev.ri added a comment.

Please always subscribe lists.
The reviews that happen without lists are null and void.

Is this testing pre-existing behavior, and thus NFC?


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

https://reviews.llvm.org/D56850



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

[PATCH] D56851: [ASTMatchers] Adds `CXXMemberCallExpr` matcher `invokedAtType`.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added subscribers: cfe-commits, lebedev.ri.
lebedev.ri added a comment.

Please always subscribe lists.
The reviews that happen without lists are null and void.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56851



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

RE: r349201 - Add extension to always default-initialize nullptr_t.

[Keane, Erich via cfe-commits]

Hi Richard-
I finally got a chance to take a look at this patch and run it through the 
nullptr_t tests that we have, and it seems to solve the problem I was 
attempting.  It would be my preference to commit this.

I currently have my existing tests (from the previous patch) and am happy with 
the messages.  What work to you believe needs to happen in order to get this 
committed?  I note that you didn’t have tests in it and I’m not sure what you 
were attempting to test initially, but if you can tell me what you were 
attempting to solve, I could perhaps come up with a handful of tests and commit 
this.

Thanks!
-Erich

From: Keane, Erich
Sent: Friday, December 14, 2018 2:57 PM
To: Richard Smith 
Cc: cfe-commits 
Subject: RE: r349201 - Add extension to always default-initialize nullptr_t.

Thanks!  I’ll take a look in a bit!
-Erich

From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, December 14, 2018 2:55 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.

Attached. With the functional part of your change reverted and this applied, 
the modified tests still pass except

error: 'note' diagnostics expected but not seen:
  File 
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
 Line 128: 'p' initialized to a null pointer value
error: 'note' diagnostics seen but not expected:
  File 
/usr/local/google/home/richardsmith/llvm-git-1/src/tools/clang/test/Analysis/nullptr.cpp
 Line 128: 'p' declared without an initial value

... which seems accurate (but perhaps not useful).

On Fri, 14 Dec 2018 at 14:47, Richard Smith 
mailto:rich...@metafoo.co.uk>> wrote:
I have a patch I put together a while back as an attempt to fix a different 
bug, that creates a new CastKind for this operation (didn't work out there, but 
it might do so here). I'll dig it out and mail it to you in case it's a useful 
starting point.

On Fri, 14 Dec 2018 at 14:42, Keane, Erich via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Alright, no problem.  I’ll push a revert in a few minutes and give it another 
try.  Perhaps I can suppress the creation of a CK_LValueToRValue in the case 
where it’s a read from nullptr_t


From: Richard Smith [mailto:rich...@metafoo.co.uk]
Sent: Friday, December 14, 2018 2:41 PM
To: Keane, Erich mailto:erich.ke...@intel.com>>
Cc: cfe-commits mailto:cfe-commits@lists.llvm.org>>
Subject: Re: r349201 - Add extension to always default-initialize nullptr_t.

Sorry, I was late with my review comment. I think this is the wrong way to 
approach this problem. This does not "fix all situations where nullptr_t would 
seem uninitialized", and it makes our AST representation lose source fidelity.

On Fri, 14 Dec 2018 at 14:25, Erich Keane via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:
Author: erichkeane
Date: Fri Dec 14 14:22:29 2018
New Revision: 349201

URL: http://llvm.org/viewvc/llvm-project?rev=349201&view=rev
Log:
Add extension to always default-initialize nullptr_t.

Core issue 1013 suggests that having an uninitialied std::nullptr_t be
UB is a bit foolish, since there is only a single valid value. This DR
reports that DR616 fixes it, which does so by making lvalue-to-rvalue
conversions from nullptr_t be equal to nullptr.

However, just implementing that results in warnings/etc in many places.
In order to fix all situations where nullptr_t would seem uninitialized,
this patch instead (as an otherwise transparent extension) default
initializes uninitialized VarDecls of nullptr_t.

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

Change-Id: I84d72a9290054fa55341e8cbdac43c8e7f25b885

Added:
cfe/trunk/test/SemaCXX/nullptr_t-init.cpp   (with props)
Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Analysis/nullptr.cpp
cfe/trunk/test/SemaCXX/ast-print-crash.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=349201&r1=349200&r2=349201&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Dec 14 14:22:29 2018
@@ -4881,6 +4881,13 @@ static void TryDefaultInitialization(Sem
 return;
   }

+  // As an extension, and to fix Core issue 1013, zero initialize nullptr_t.
+  // Since there is only 1 valid value of nullptr_t, we can just use that.
+  if (DestType->isNullPtrType()) {
+Sequence.AddZeroInitializationStep(Entity.getType());
+return;
+  }
+
   // - otherwise, no initialization is performed.

   //   If a program calls for the default initialization of an object of

Modified: cfe/trunk/test/Analysis/nullptr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nullptr.cpp?rev=349201&r1=349200&r2=349201&view=diff
=

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D56850#1361436 , @lebedev.ri wrote:

> Please always subscribe lists.
>  The reviews that happen without lists are null and void.


Thanks for updating my diffs with the relevant list. Noted for next time.

> Is this testing pre-existing behavior, and thus NFC?

Yes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56850



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

[PATCH] D56829: Move decl context dumping to TextNodeDumper

[Stephen Kelly via Phabricator via cfe-commits]

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

I don't know what C++ code results in the `undeserialized declarations` output. 
Can you suggest some?




Comment at: lib/AST/ASTDumper.cpp:519-520
-  (DC->hasExternalLexicalStorage() ||
-   (Deserialize ? DC->decls_begin() != DC->decls_end()
-: DC->noload_decls_begin() != DC->noload_decls_end(
 dumpDeclContext(DC);

aaron.ballman wrote:
> Why did this condition get dropped?
The condition is using internal knowledge of the `dumpDeclContext` method - 
that it doesn't do anything if these conditions are not met, so avoid calling 
the method in the first place. We can just call the method. The for loop which 
remains will simply do nothing.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56829



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

[PATCH] D56852: [AArch64] Use LLU for 64-bit crc32 arguments

[Sam Parker via Phabricator via cfe-commits]

samparker created this revision.
samparker added a reviewer: t.p.northover.
Herald added subscribers: kristof.beyls, javed.absar.

The ACLE states that 64-bit crc32 operands are uint64_t so we should have the 
clang builtin match this description - which is what we already do for AArch32.


https://reviews.llvm.org/D56852

Files:
  include/clang/Basic/BuiltinsAArch64.def
  test/CodeGen/arm64-crc32.c


Index: test/CodeGen/arm64-crc32.c
===
--- test/CodeGen/arm64-crc32.c
+++ test/CodeGen/arm64-crc32.c
@@ -1,54 +1,57 @@
 // REQUIRES: aarch64-registered-target
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu \
 // RUN:  -disable-O0-optnone -S -emit-llvm -o - %s | opt -S -mem2reg | 
FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-windows \
+// RUN:  -disable-O0-optnone -S -emit-llvm -o - %s | opt -S -mem2reg | 
FileCheck %s
+#include 
 
-int crc32b(int a, char b)
+uint32_t crc32b(uint32_t a, uint8_t b)
 {
 return __builtin_arm_crc32b(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i8 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32b(i32 %a, i32 [[T0]])
 }
 
-int crc32cb(int a, char b)
+uint32_t crc32cb(uint32_t a, uint8_t b)
 {
 return __builtin_arm_crc32cb(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i8 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32cb(i32 %a, i32 [[T0]])
 }
 
-int crc32h(int a, short b)
+uint32_t crc32h(uint32_t a, uint16_t b)
 {
 return __builtin_arm_crc32h(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i16 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32h(i32 %a, i32 [[T0]])
 }
 
-int crc32ch(int a, short b)
+uint32_t crc32ch(uint32_t a, uint16_t b)
 {
 return __builtin_arm_crc32ch(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i16 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32ch(i32 %a, i32 [[T0]])
 }
 
-int crc32w(int a, int b)
+uint32_t crc32w(uint32_t a, uint32_t b)
 {
 return __builtin_arm_crc32w(a,b);
 // CHECK: call i32 @llvm.aarch64.crc32w(i32 %a, i32 %b)
 }
 
-int crc32cw(int a, int b)
+uint32_t crc32cw(uint32_t a, uint32_t b)
 {
 return __builtin_arm_crc32cw(a,b);
 // CHECK: call i32 @llvm.aarch64.crc32cw(i32 %a, i32 %b)
 }
 
-int crc32d(int a, long b)
+uint32_t crc32d(uint32_t a, uint64_t b)
 {
 return __builtin_arm_crc32d(a,b);
 // CHECK: call i32 @llvm.aarch64.crc32x(i32 %a, i64 %b)
 }
 
-int crc32cd(int a, long b)
+uint32_t crc32cd(uint32_t a, uint64_t b)
 {
 return __builtin_arm_crc32cd(a,b);
 // CHECK: call i32 @llvm.aarch64.crc32cx(i32 %a, i64 %b)
Index: include/clang/Basic/BuiltinsAArch64.def
===
--- include/clang/Basic/BuiltinsAArch64.def
+++ include/clang/Basic/BuiltinsAArch64.def
@@ -50,8 +50,8 @@
 BUILTIN(__builtin_arm_crc32ch, "UiUiUs", "nc")
 BUILTIN(__builtin_arm_crc32w, "UiUiUi", "nc")
 BUILTIN(__builtin_arm_crc32cw, "UiUiUi", "nc")
-BUILTIN(__builtin_arm_crc32d, "UiUiLUi", "nc")
-BUILTIN(__builtin_arm_crc32cd, "UiUiLUi", "nc")
+BUILTIN(__builtin_arm_crc32d, "UiUiLLUi", "nc")
+BUILTIN(__builtin_arm_crc32cd, "UiUiLLUi", "nc")
 
 // Memory barrier
 BUILTIN(__builtin_arm_dmb, "vUi", "nc")


Index: test/CodeGen/arm64-crc32.c
===
--- test/CodeGen/arm64-crc32.c
+++ test/CodeGen/arm64-crc32.c
@@ -1,54 +1,57 @@
 // REQUIRES: aarch64-registered-target
 // RUN: %clang_cc1 -triple arm64-none-linux-gnu \
 // RUN:  -disable-O0-optnone -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+// RUN: %clang_cc1 -triple aarch64-windows \
+// RUN:  -disable-O0-optnone -S -emit-llvm -o - %s | opt -S -mem2reg | FileCheck %s
+#include 
 
-int crc32b(int a, char b)
+uint32_t crc32b(uint32_t a, uint8_t b)
 {
 return __builtin_arm_crc32b(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i8 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32b(i32 %a, i32 [[T0]])
 }
 
-int crc32cb(int a, char b)
+uint32_t crc32cb(uint32_t a, uint8_t b)
 {
 return __builtin_arm_crc32cb(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i8 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32cb(i32 %a, i32 [[T0]])
 }
 
-int crc32h(int a, short b)
+uint32_t crc32h(uint32_t a, uint16_t b)
 {
 return __builtin_arm_crc32h(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i16 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32h(i32 %a, i32 [[T0]])
 }
 
-int crc32ch(int a, short b)
+uint32_t crc32ch(uint32_t a, uint16_t b)
 {
 return __builtin_arm_crc32ch(a,b);
 // CHECK: [[T0:%[0-9]+]] = zext i16 %b to i32
 // CHECK: call i32 @llvm.aarch64.crc32ch(i32 %a, i32 [[T0]])
 }
 
-int crc32w(int a, int b)
+uint32_t crc32w(uint32_t a, uint32_t b)
 {
 return __builtin_arm_crc32w(a,b);
 // CHECK: call i32 @llvm.aarch64.crc32w(i32 %a, i32 %b)
 }
 
-int crc32cw(int a, int b)
+uint32_t crc32cw(uint32_t a, uint32_t b)
 {
 return __builtin_arm_crc32cw(a,b);
 // CHECK: call i32 @llvm.aarch64.crc32cw(i32 %a, i32 %b)
 }
 
-int crc32d(int a, long b)
+uint32_t crc32d(uint32_t a, uint64_t b)
 {
  

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:475
+  auto MatchesType = 
cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(
+  R"cc(

Could you add a test which uses the same c++ snippet as this, but with a 
matcher using `hasName("Y")`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56850



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

[PATCH] D56829: Move decl context dumping to TextNodeDumper

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D56829#1361465 , @steveire wrote:

> I don't know what C++ code results in the `undeserialized declarations` 
> output. Can you suggest some?


I haven't the foggiest idea. I'd recommend doing an svn blame to see what 
commit added that code and try to work backwards from there -- presumably 
there's some related test code that could be run under -ast-dump.




Comment at: lib/AST/ASTDumper.cpp:519-520
-  (DC->hasExternalLexicalStorage() ||
-   (Deserialize ? DC->decls_begin() != DC->decls_end()
-: DC->noload_decls_begin() != DC->noload_decls_end(
 dumpDeclContext(DC);

steveire wrote:
> aaron.ballman wrote:
> > Why did this condition get dropped?
> The condition is using internal knowledge of the `dumpDeclContext` method - 
> that it doesn't do anything if these conditions are not met, so avoid calling 
> the method in the first place. We can just call the method. The for loop 
> which remains will simply do nothing.
Ah, I see what's happening now, thank you.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56829



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

[PATCH] D56786: [ASTMatchers] Changes to `CXXMemberExpr` matchers.

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

Should this one now be closed? The 'Add Action...' dropdown has an action for 
that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56786



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D56841#1361395 , @kadircet wrote:

> It seems like the most relevant place in tooling is ArgumentsAdjuster as 
> @ioeric pointed out. Happy to move it to there if @sammccall and @klimek 
> agrees as well.
>
> As a side note `ArgumentsAdjuster`s unfortunately causes a copy of the 
> original command line arguments. Not sure how important this factor is 
> compared to spinning up a compiler instance, just wanted to point it out.


From my point of view, it looks like this isn't enough to be a huge thing in 
tooling. That said, if we can find a nice abstraction to pull out, I'm 
generally in favor :) (I know, I know, I'm not a big help)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D56841#1361395 , @kadircet wrote:

> It seems like the most relevant place in tooling is ArgumentsAdjuster as 
> @ioeric pointed out. Happy to move it to there if @sammccall and @klimek 
> agrees as well.
>
> As a side note `ArgumentsAdjuster`s unfortunately causes a copy of the 
> original command line arguments. Not sure how important this factor is 
> compared to spinning up a compiler instance, just wanted to point it out.


I do think this should be a standard arguments adjuster available in Tooling.
This is a pattern that's likely to recur (clang-tidy will need the same fix to 
work with chromium). The fix is small but fiddly enough that it might warrant 
some improvement over time - I think we want to avoid people cloning their own 
copies of the fix.
The ArgumentsAdjuster interface is a little unfortunate regarding the copy, but 
I don't think this is significant in practice.

Once verifying this fixes the issue, we should consider requesting the tooling 
and clangd patches be cherrypicked to the LLVM-8 branch.
(And possibly do the same for clang-tidy)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182272.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Build tweaks as an object library. To make sure linker does not optimize away 
global constructors.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/tool/CMakeLists.txt
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -25,7 +25,8 @@
 # CHECK-NEXT:  "documentSymbolProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.applyCodeAction"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
Index: clangd/tool/CMakeLists.txt
===
--- clangd/tool/CMakeLists.txt
+++ clangd/tool/CMakeLists.txt
@@ -3,6 +3,7 @@
 
 add_clang_tool(clangd
   ClangdMain.cpp
+  $
   )
 
 set(LLVM_LINK_COMPONENTS
Index: clangd/refactor/tweaks/CMakeLists.txt
===
--- /dev/null
+++ clangd/refactor/tweaks/CMakeLists.txt
@@ -0,0 +1,11 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..)
+
+# A target containing all code tweaks (i.e. mini-refactorings) provided by
+# clangd.
+# Built as an object library to make sure linker does not remove global
+# constructors that register individual tweaks in a global registry.
+# To enable these tweaks in exectubales or shared libraries, add
+# $ to a list of sources, see
+# clangd/tool/CMakeLists.txt for an example.
+add_clang_library(clangDaemonTweaks OBJECT
+  )
Index: clangd/refactor/Tweak.h
===
--- /dev/null
+++ clangd/refactor/Tweak.h
@@ -0,0 +1,98 @@
+//===--- Tweak.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Tweaks are small refactoring-like actions that run over the AST and produce
+// the set of edits as a result. They are local, i.e. they should take the
+// current editor context, e.g. the cursor position and selection into account.
+// The actions are executed in two stages:
+//   - Stage 1 should check whether the action is available in a current
+// context. It should be cheap and fast to compute as it is executed for all
+// available actions on every client request, which happen quite frequently.
+//   - Stage 2 is performed after stage 1 and can be more expensive to compute.
+// It is performed when the user actually chooses the action.
+// To avoid extra round-trips and AST reads, actions can also produce results on
+// stage 1 in cases when the actions are fast to compute.
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H
+
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+namespace clang {
+namespace clangd {
+
+using TweakID = llvm::StringRef;
+
+/// An interface base for small context-sensitive refactoring actions.
+class Tweak {
+public:
+  /// Input to prepare and apply tweaks.
+  struct Selection {
+static llvm::Optional create(llvm::StringRef File,
+llvm::StringRef Code,
+ParsedAST &AST, Range Selection);
+
+/// The path of an active document the action was invoked in.
+llvm::StringRef File;
+/// The text of the active document.
+llvm::StringRef Code;
+/// Parsed AST of the active file.
+ParsedAST *
+/// A location of the cursor in the editor.
+SourceLocation Cursor;
+// FIXME: add selection when there are checks relying on it.
+// FIXME: provide a way to get sources and ASTs for other files.
+// FIXME: cache some commonly required information (e.g. AST nodes under
+//cursor) to avoid redundant AST visit in every action.
+  };
+  virtual ~Tweak() = default;
+  /// A unique id of the

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182273.
ilya-biryukov added a comment.

- Rebase after parent change


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp
  clangd/refactor/tweaks/QualifyName.h

Index: clangd/refactor/tweaks/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.h
@@ -0,0 +1,42 @@
+//===--- QualifyName.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Fully qualifies a name under a cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+
+#include "refactor/Tweak.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public Tweak {
+public:
+  TweakID id() override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,163 @@
+//===--- QualifyName.cpp -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(QualifyName);
+
+namespace {
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+return nullptr; // FIXME: handle this situation, retrieve the thing
+// referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+   // TranslationUnitDecl.
+return nullptr;
+return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor {
+public:
+  LocateInsertLoc(ASTContext &Ctx, SourceLocation CursorLoc,
+  Reference &UnqualRef)
+  : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+  return false;
+return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (NNS.getPrefix())
+  return true; // we want only unqualified names.
+auto &SM = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+if (!Target)
+  return true; // continue traversal to recurse into types, if any.
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Target;
+return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRef

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182274.
ilya-biryukov added a comment.

- Rebase after parent change


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56611

Files:
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/SwapIfBranches.cpp
  clangd/refactor/tweaks/SwapIfBranches.h

Index: clangd/refactor/tweaks/SwapIfBranches.h
===
--- /dev/null
+++ clangd/refactor/tweaks/SwapIfBranches.h
@@ -0,0 +1,41 @@
+//===--- SwapIfBrances.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Swaps the 'then' and 'else' branch of the if statement.
+// Before:
+//   if (foo) { return 10; } else { continue; }
+// After:
+//   if (foo) { continue; } else { return 10; }
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_SWAPIFBRANCHES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_SWAPIFBRANCHES_H
+
+#include "refactor/Tweak.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class SwapIfBranches : public Tweak {
+public:
+  TweakID id() override { return llvm::StringLiteral("swap-if-branches"); }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  tooling::Replacements Result;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
\ No newline at end of file
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -0,0 +1,102 @@
+//===--- SwapIfBranches.cpp --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "SwapIfBranches.h"
+#include "ClangdUnit.h"
+#include "Logger.h"
+#include "SourceCode.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(SwapIfBranches);
+
+namespace {
+class FindIfUnderCursor : public RecursiveASTVisitor {
+public:
+  FindIfUnderCursor(ASTContext &Ctx, SourceLocation CursorLoc, IfStmt *&Result)
+  : Ctx(Ctx), CursorLoc(CursorLoc), Result(Result) {}
+
+  bool VisitIfStmt(IfStmt *If) {
+auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(),
+ SourceRange(If->getIfLoc()));
+if (!R)
+  return true;
+if (!halfOpenRangeContains(Ctx.getSourceManager(), *R, CursorLoc))
+  return true;
+Result = If;
+return false;
+  }
+
+private:
+  ASTContext &Ctx;
+  SourceLocation CursorLoc;
+  IfStmt *&Result;
+};
+} // namespace
+
+bool SwapIfBranches::prepare(const Selection &Inputs) {
+
+  auto &Ctx = Inputs.AST.getASTContext();
+  auto &SrcMgr = Ctx.getSourceManager();
+  IfStmt *If = nullptr;
+  FindIfUnderCursor(Ctx, Inputs.Cursor, If).TraverseAST(Ctx);
+  if (!If)
+return false;
+
+  // avoid dealing with single-statement brances, they require careful handling
+  // to avoid changing semantics of the code (i.e. dangling else).
+  if (!llvm::dyn_cast_or_null(If->getThen()) ||
+  !llvm::dyn_cast_or_null(If->getElse()))
+return false;
+
+  auto ThenRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getThen()->getSourceRange());
+  if (!ThenRng)
+return false;
+  auto ElseRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getElse()->getSourceRange());
+  if (!ElseRng)
+return false;
+
+  llvm::StringRef ThenCode = toSourceCode(SrcMgr, *ThenRng);
+  llvm::StringRef ElseCode = toSourceCode(SrcMgr, *ElseRng);
+
+  if (auto Err = Result.add(tooling::Replacement(SrcMgr, ThenRng->getBegin(),
+ ThenCode.size(), ElseCode))) {
+llvm::consumeError(std::move(Err));
+return false;
+  }
+  if (auto Err = Result.add(

[PATCH] D56612: [clangd] A code action to remove 'using namespace'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182276.
ilya-biryukov added a comment.

- Rebase after parent change


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56612

Files:
  clangd/AST.cpp
  clangd/AST.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clangd/refactor/tweaks/RemoveUsingNamespace.h

Index: clangd/refactor/tweaks/RemoveUsingNamespace.h
===
--- /dev/null
+++ clangd/refactor/tweaks/RemoveUsingNamespace.h
@@ -0,0 +1,42 @@
+//===--- RemoveUsingNamespace.h --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Removes the 'using namespace' under the cursor and qualifies all accesses in 
+// the current file. E.g.,
+//   using namespace std;
+//   vector foo(std::map);
+// Would become:
+//   std::vector foo(std::map);
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_REMOVEUSINGNAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_REMOVEUSINGNAMESPACE_H
+
+#include "refactor/Tweak.h"
+
+namespace clang {
+namespace clangd {
+
+class RemoveUsingNamespace : public Tweak {
+public:
+  TweakID id() override {
+return llvm::StringLiteral("remove-using-namespace");
+  }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  UsingDirectiveDecl *TargetDirective = nullptr;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -0,0 +1,188 @@
+//===--- RemoveUsingNamespace.cpp *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "RemoveUsingNamespace.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(RemoveUsingNamespace);
+
+namespace {
+class FindNodeUnderCursor : public RecursiveASTVisitor {
+public:
+  FindNodeUnderCursor(SourceLocation SearchedLoc, UsingDirectiveDecl *&Result)
+  : SearchedLoc(SearchedLoc), Result(Result) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getUsingLoc() != SearchedLoc)
+  return true;
+
+Result = D;
+return false;
+  }
+
+private:
+  SourceLocation SearchedLoc;
+  UsingDirectiveDecl *&Result;
+};
+
+class FindSameUsings : public RecursiveASTVisitor {
+public:
+  FindSameUsings(UsingDirectiveDecl &Target,
+ std::vector &Results)
+  : TargetNS(Target.getNominatedNamespace()),
+TargetCtx(Target.getDeclContext()), Results(Results) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getNominatedNamespace() != TargetNS ||
+D->getDeclContext() != TargetCtx)
+  return true;
+
+Results.push_back(D);
+return true;
+  }
+
+private:
+  NamespaceDecl *TargetNS;
+  DeclContext *TargetCtx;
+  std::vector &Results;
+};
+
+class FindIdentsToQualify
+: public tooling::RecursiveSymbolVisitor {
+public:
+  FindIdentsToQualify(ASTContext &Ctx, NamespaceDecl &TargetNS,
+  std::vector &ResultIdents)
+  : RecursiveSymbolVisitor(Ctx.getSourceManager(), Ctx.getLangOpts()),
+Ctx(Ctx), TargetNS(TargetNS), ResultIdents(ResultIdents) {}
+
+  bool visitSymbolOccurrence(const NamedDecl *D,
+ llvm::ArrayRef NameRanges) {
+if (!D || D->getCanonicalDecl() == TargetNS.getCanonicalDecl())
+  return true;
+if (!D->getDeclName().isIdentifier() ||
+D->getDeclName().getNameKind() == DeclarationName::CXXOperatorName)
+  return true; // do not add qualifiers for non-idents, e.g. 'operator+'.
+// Check the symbol is unqualified and references something inside our
+// namespace.
+// FIXME: add a check it's unqualified.
+if (!TargetNS.InEnclosingNamespaceSetOf(D->getDeclContext()))
+  return true;
+// FIXME: handle more tricky cases, e.g. we don't need the qualifier if we
+//have the using decls for some entit

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Haven't yet addressed all the comments, but switched to use the "object 
library" (i.e. a collection of .o files) to make sure linker does not optimize 
away global ctors required by registry.




Comment at: clangd/refactor/Tweak.cpp:38
+namespace {
+const llvm::StringMap()>> &
+tweaksRegistry() {

sammccall wrote:
> Can we drop this indirection and use the registry directly?
Sure, would mean linear time for prepareTweak, but we probably don't care.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267



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

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Stephen Kelly via Phabricator via cfe-commits]

steveire added inline comments.



Comment at: clang/docs/LibASTMatchersReference.html:4823
+  matches "x.m", but not "m"
+memberExpr(hasObjectExpression(hasType(pointsTo(
+ cxxRecordDecl(hasName("X"))

If we're going to put such examples and details into the docs, maybe we should 
explain why things are and are not matched. What do you think?

It seems in this case the type of the object expression of `m` is `X*` because 
of the hidden `this->`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D56849



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

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 182281.
Anastasia marked an inline comment as done.
Anastasia added a comment.

- Removed else case in DiagnoseMultipleAddrSpaceAttributes
- Added extra test to check correctness of addr space for 'this' when 
statically used in a class scope.


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

https://reviews.llvm.org/D55850

Files:
  include/clang/AST/Type.h
  include/clang/Sema/ParsedAttr.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/method-overload-address-space.cl
  test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
  test/SemaOpenCLCXX/method-overload-address-space.cl

Index: test/SemaOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,20 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+struct C {
+  void m1() __local __local; //expected-warning{{multiple identical address spaces specified for type}}
+  //expected-note@-1{{candidate function}}
+  void m1() __global;
+  //expected-note@-1{{candidate function}}
+  void m2() __global __local; //expected-error{{multiple address spaces specified for type}}
+};
+
+__global C c_glob;
+
+__kernel void bar() {
+  __local C c_loc;
+  C c_priv;
+
+  c_glob.m1();
+  c_loc.m1();
+  c_priv.m1(); //expected-error{{no matching member function for call to 'm1'}}
+}
Index: test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
@@ -0,0 +1,18 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+struct C {
+  auto fGlob() __global -> decltype(this);
+  auto fGen() -> decltype(this);
+  auto fErr() __global __local -> decltype(this); //expected-error{{multiple address spaces specified for type}}
+};
+
+void bar(__local C*);
+// expected-note@-1{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka '__global C *')), parameter type must be '__local C *'}}
+// expected-note@-2{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka 'C *')), parameter type must be '__local C *'}}
+
+__global C Glob;
+void foo(){
+bar(Glob.fGlob()); // expected-error{{no matching function for call to 'bar'}}
+// FIXE: AS of 'this' below should be correctly deduced to generic
+bar(Glob.fGen()); // expected-error{{no matching function for call to 'bar'}}
+}
Index: test/CodeGenOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,35 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct C {
+  void foo() __local;
+  void foo() __global;
+  void foo();
+  void bar();
+};
+
+__global C c1;
+
+__kernel void k() {
+  __local C c2;
+  C c3;
+  __global C &c_ref = c1;
+  __global C *c_ptr;
+
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c1.foo();
+  // CHECK: call void @_ZNU3AS31C3fooEv(%struct.C addrspace(3)*
+  c2.foo();
+  // CHECK: call void @_ZNU3AS41C3fooEv(%struct.C addrspace(4)*
+  c3.foo();
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ptr->foo();
+  // CHECK: void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ref.foo();
+
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c1.bar();
+  //FIXME: Doesn't compile yet
+  //c_ptr->bar();
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c_ref.bar();
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -3916,6 +3916,25 @@
   llvm_unreachable("unknown NullabilityKind");
 }
 
+// Diagnose whether this is a case with the multiple addr spaces.
+// Returns true if this is an invalid case.
+// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
+// by qualifiers for two or more different address spaces."
+static bool DiagnoseMultipleAddrSpaceAttributes(Sema &S, LangAS ASOld,
+LangAS ASNew,
+SourceLocation AttrLoc) {
+  if (ASOld != LangAS::Default) {
+if (ASOld != ASNew) {
+  S.Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
+  return true;
+}
+// Emit a warning if they are identical; it's likely unintended.
+S.Diag(AttrLoc,
+   diag::warn_attribute_address_multiple_identical_qualifiers);
+  }
+  return false;
+}
+
 static TypeSourceInfo 

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

[Vladimir Alyamkin via Phabricator via cfe-commits]

ufna added a comment.

@krasimir BeforeHash style is widely used for Unreal Engine 4 based projects 
development. I was looking for this option too.


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

https://reviews.llvm.org/D52150



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

[PATCH] D56836: [mips] Include whole lpthread when using both -pthread and -static

[Aleksandar Beserminji via Phabricator via cfe-commits]

abeserminji added a comment.

I am not sure if -static option passed as --cflag to lnt runtest enters the 
LDFLAGS. I guess it doesn't. 
But if there is a way to check which options are passed as --cflags, we might 
be able to add -Wl, -whole-archive -lpthread -Wl -no-whole-archive only in case 
when static is used.
That would be my preferable option in this situation, but I couldn't find any 
clues on how to do that.

It is possible to modify an algorithm in such way. Shouldn't differ too much.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56836



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

clang-tidy also has the same fix in place, https://reviews.llvm.org/D18806


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Parse/ParseDecl.cpp:6170
+  }
+}
+  }

rjmccall wrote:
> Does this not need to diagnose redundant qualifiers?  Why is this path 
> required in addition to the path in SemaType, anyway?
We discussed earlier to collect addr space qualifiers for completeness: 
https://reviews.llvm.org/D55850#inline-495037

Without this change the following doesn't work for addr spaces correctly:
  auto fGlob() __global -> decltype(this);
I added a test that check this now in 
**test/SemaOpenCLCXX/address-space-of-this-class-scope.cl**.

Here we only collect the quals to be applied to 'this'. If there are multiple 
address space specified on a method it will be diagnosed in SemaType.cpp. I 
think we probably don't need to give the same diagnostic twice?



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

https://reviews.llvm.org/D55850



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

[PATCH] D55850: [OpenCL] Allow address spaces as method qualifiers

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 182282.
Anastasia added a comment.

- Fixed typo in FIXME


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

https://reviews.llvm.org/D55850

Files:
  include/clang/AST/Type.h
  include/clang/Sema/ParsedAttr.h
  lib/Parse/ParseDecl.cpp
  lib/Sema/SemaOverload.cpp
  lib/Sema/SemaType.cpp
  test/CodeGenOpenCLCXX/method-overload-address-space.cl
  test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
  test/SemaOpenCLCXX/method-overload-address-space.cl

Index: test/SemaOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,20 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+struct C {
+  void m1() __local __local; //expected-warning{{multiple identical address spaces specified for type}}
+  //expected-note@-1{{candidate function}}
+  void m1() __global;
+  //expected-note@-1{{candidate function}}
+  void m2() __global __local; //expected-error{{multiple address spaces specified for type}}
+};
+
+__global C c_glob;
+
+__kernel void bar() {
+  __local C c_loc;
+  C c_priv;
+
+  c_glob.m1();
+  c_loc.m1();
+  c_priv.m1(); //expected-error{{no matching member function for call to 'm1'}}
+}
Index: test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-of-this-class-scope.cl
@@ -0,0 +1,18 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+struct C {
+  auto fGlob() __global -> decltype(this);
+  auto fGen() -> decltype(this);
+  auto fErr() __global __local -> decltype(this); //expected-error{{multiple address spaces specified for type}}
+};
+
+void bar(__local C*);
+// expected-note@-1{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka '__global C *')), parameter type must be '__local C *'}}
+// expected-note@-2{{candidate function not viable: address space mismatch in 1st argument ('decltype(this)' (aka 'C *')), parameter type must be '__local C *'}}
+
+__global C Glob;
+void foo(){
+bar(Glob.fGlob()); // expected-error{{no matching function for call to 'bar'}}
+// FIXME: AS of 'this' below should be correctly deduced to generic
+bar(Glob.fGen()); // expected-error{{no matching function for call to 'bar'}}
+}
Index: test/CodeGenOpenCLCXX/method-overload-address-space.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/method-overload-address-space.cl
@@ -0,0 +1,35 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct C {
+  void foo() __local;
+  void foo() __global;
+  void foo();
+  void bar();
+};
+
+__global C c1;
+
+__kernel void k() {
+  __local C c2;
+  C c3;
+  __global C &c_ref = c1;
+  __global C *c_ptr;
+
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c1.foo();
+  // CHECK: call void @_ZNU3AS31C3fooEv(%struct.C addrspace(3)*
+  c2.foo();
+  // CHECK: call void @_ZNU3AS41C3fooEv(%struct.C addrspace(4)*
+  c3.foo();
+  // CHECK: call void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ptr->foo();
+  // CHECK: void @_ZNU3AS11C3fooEv(%struct.C addrspace(1)*
+  c_ref.foo();
+
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c1.bar();
+  //FIXME: Doesn't compile yet
+  //c_ptr->bar();
+  // CHECK: call void @_ZNU3AS41C3barEv(%struct.C addrspace(4)* addrspacecast (%struct.C addrspace(1)* @c1 to %struct.C addrspace(4)*))
+  c_ref.bar();
+}
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -3916,6 +3916,25 @@
   llvm_unreachable("unknown NullabilityKind");
 }
 
+// Diagnose whether this is a case with the multiple addr spaces.
+// Returns true if this is an invalid case.
+// ISO/IEC TR 18037 S5.3 (amending C99 6.7.3): "No type shall be qualified
+// by qualifiers for two or more different address spaces."
+static bool DiagnoseMultipleAddrSpaceAttributes(Sema &S, LangAS ASOld,
+LangAS ASNew,
+SourceLocation AttrLoc) {
+  if (ASOld != LangAS::Default) {
+if (ASOld != ASNew) {
+  S.Diag(AttrLoc, diag::err_attribute_address_multiple_qualifiers);
+  return true;
+}
+// Emit a warning if they are identical; it's likely unintended.
+S.Diag(AttrLoc,
+   diag::warn_attribute_address_multiple_identical_qualifiers);
+  }
+  return false;
+}
+
 static TypeSourceInfo *
 GetTypeSourceInfoForDeclarator(TypeProcessingState &State,
QualType T, TypeSourceInfo *ReturnTypeInfo);
@@ -4823,18 +4842,35 @@
  

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D56841#1361492 , @klimek wrote:

> (I know, I know, I'm not a big help)


That's fine, mostly wanted to hear your opinion :-)

> That said, if we can find a nice abstraction to pull out, I'm generally in 
> favor :)

Eric suggested putting this into `lib/Tooling/ArgumentsAdjusters.cpp`, looks 
like a proper place for this kind of thing.

> As a side note ArgumentsAdjusters unfortunately causes a copy of the original 
> command line arguments. Not sure how important this factor is compared to 
> spinning up a compiler instance, just wanted to point it out.

Yeah, running the frontend later would definitely outweigh this inefficiency, 
so we don't care.
I do agree that's an unfortunate design, though.




Comment at: clangd/ClangdUnit.cpp:433
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||

kadircet wrote:
> ilya-biryukov wrote:
> > Maybe move this code into `ClangdServer::getCompileCommand` to keep **all** 
> > argument adjustments that we do in clangd in one place.
> > We add `-resource-dir` there.
> As mentioned in the change description, we also have a different compiler 
> invocation in `BackgroundIndex` which is not aware of 
> `ClangdServer::getCompileCommand`
Ah, right :-( That's annoying. Does it also manually override `-resource-dir`? 
I'd move both into a single place, maybe this function is a better place to do 
so.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56753: [ASTDump] Mark null params with a tag rather than a child node

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/AST/ASTDumper.cpp:642
 
-  if (!D->param_begin() && D->getNumParams())
-dumpChild([=] { OS << ">"; });
-  else
-for (const ParmVarDecl *Parameter : D->parameters())
+  if (D->param_begin() || !D->getNumParams())
+for (const auto *Parameter : D->parameters())

aaron.ballman wrote:
> You can drop `getNumParams()` here -- if `param_begin()` returns a non-null 
> value, `getNumParams()` must return a non-zero value.
> 
> (`param_begin()` is implemented in terms of `parameters()`, which calls 
> `getNumParams()` when setting up the returned `ArrayRef`.)
It seems that this is needed because the situation can arise when the function 
is not fully constructed yet, such as when calling `dump()` under the debugger.

I think this can be more clearly expressed as `if (!D->param_empty() && 
!D->param_begin())`, but should also have some comments explaining that this is 
guarding against a situation that only comes up while debugging, and thus is 
not covered by any existing test cases.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56753



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

r351449 - CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[Anton Korobeynikov via cfe-commits]

Author: asl
Date: Thu Jan 17 07:21:55 2019
New Revision: 351449

URL: http://llvm.org/viewvc/llvm-project?rev=351449&view=rev
Log:
CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

llvm.flt.rounds returns an i32, but the builtin expects an integer. 
On targets where integers are not 32-bits clang tries to bitcast the result, 
causing an assertion failure.

The patch enables newlib build for msp430.

Patch by Edward Jones!

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


Added:
cfe/trunk/test/CodeGen/builtins-msp430.c
Modified:
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/test/CodeGen/builtins.c

Modified: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBuiltin.cpp?rev=351449&r1=351448&r2=351449&view=diff
==
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp Thu Jan 17 07:21:55 2019
@@ -2130,6 +2130,17 @@ RValue CodeGenFunction::EmitBuiltinExpr(
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 
+  case Builtin::BI__builtin_flt_rounds: {
+Value *F = CGM.getIntrinsic(Intrinsic::flt_rounds);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Result = Builder.CreateCall(F);
+if (Result->getType() != ResultType)
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+ "cast");
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 Value *V = EmitScalarExpr(E->getArg(5));
 llvm::Type *Ty = ConvertType(E->getArg(5)->getType());

Added: cfe/trunk/test/CodeGen/builtins-msp430.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-msp430.c?rev=351449&view=auto
==
--- cfe/trunk/test/CodeGen/builtins-msp430.c (added)
+++ cfe/trunk/test/CodeGen/builtins-msp430.c Thu Jan 17 07:21:55 2019
@@ -0,0 +1,10 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
+
+int test_builtin_flt_rounds() {
+  // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.flt.rounds()
+  // CHECK-DAG: [[V1:[%A-Za-z0-9.]+]] = trunc i32 [[V0]] to i16
+  // CHECK-DAG: ret i16 [[V1]]
+  return __builtin_flt_rounds();
+}
+

Modified: cfe/trunk/test/CodeGen/builtins.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins.c?rev=351449&r1=351448&r2=351449&view=diff
==
--- cfe/trunk/test/CodeGen/builtins.c (original)
+++ cfe/trunk/test/CodeGen/builtins.c Thu Jan 17 07:21:55 2019
@@ -246,6 +246,9 @@ void test_float_builtins(float F, double
   // CHECK: fcmp uge float {{.*}}, 0x3810
   // CHECK: and i1
   // CHECK: and i1
+
+  res = __builtin_flt_rounds();
+  // CHECK: call i32 @llvm.flt.rounds(
 }
 
 // CHECK-LABEL: define void @test_float_builtin_ops


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

[PATCH] D24461: CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[Anton Korobeynikov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351449: CodeGen: Cast llvm.flt.rounds result to match 
__builtin_flt_rounds (authored by asl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D24461?vs=71016&id=182286#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D24461

Files:
  cfe/trunk/lib/CodeGen/CGBuiltin.cpp
  cfe/trunk/test/CodeGen/builtins-msp430.c
  cfe/trunk/test/CodeGen/builtins.c


Index: cfe/trunk/test/CodeGen/builtins-msp430.c
===
--- cfe/trunk/test/CodeGen/builtins-msp430.c
+++ cfe/trunk/test/CodeGen/builtins-msp430.c
@@ -0,0 +1,10 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -emit-llvm %s -o - | 
FileCheck %s
+
+int test_builtin_flt_rounds() {
+  // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.flt.rounds()
+  // CHECK-DAG: [[V1:[%A-Za-z0-9.]+]] = trunc i32 [[V0]] to i16
+  // CHECK-DAG: ret i16 [[V1]]
+  return __builtin_flt_rounds();
+}
+
Index: cfe/trunk/test/CodeGen/builtins.c
===
--- cfe/trunk/test/CodeGen/builtins.c
+++ cfe/trunk/test/CodeGen/builtins.c
@@ -246,6 +246,9 @@
   // CHECK: fcmp uge float {{.*}}, 0x3810
   // CHECK: and i1
   // CHECK: and i1
+
+  res = __builtin_flt_rounds();
+  // CHECK: call i32 @llvm.flt.rounds(
 }
 
 // CHECK-LABEL: define void @test_float_builtin_ops
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -2130,6 +2130,17 @@
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 
+  case Builtin::BI__builtin_flt_rounds: {
+Value *F = CGM.getIntrinsic(Intrinsic::flt_rounds);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Result = Builder.CreateCall(F);
+if (Result->getType() != ResultType)
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+ "cast");
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 Value *V = EmitScalarExpr(E->getArg(5));
 llvm::Type *Ty = ConvertType(E->getArg(5)->getType());


Index: cfe/trunk/test/CodeGen/builtins-msp430.c
===
--- cfe/trunk/test/CodeGen/builtins-msp430.c
+++ cfe/trunk/test/CodeGen/builtins-msp430.c
@@ -0,0 +1,10 @@
+// REQUIRES: msp430-registered-target
+// RUN: %clang_cc1 -triple msp430-unknown-unknown -emit-llvm %s -o - | FileCheck %s
+
+int test_builtin_flt_rounds() {
+  // CHECK: [[V0:[%A-Za-z0-9.]+]] = call i32 @llvm.flt.rounds()
+  // CHECK-DAG: [[V1:[%A-Za-z0-9.]+]] = trunc i32 [[V0]] to i16
+  // CHECK-DAG: ret i16 [[V1]]
+  return __builtin_flt_rounds();
+}
+
Index: cfe/trunk/test/CodeGen/builtins.c
===
--- cfe/trunk/test/CodeGen/builtins.c
+++ cfe/trunk/test/CodeGen/builtins.c
@@ -246,6 +246,9 @@
   // CHECK: fcmp uge float {{.*}}, 0x3810
   // CHECK: and i1
   // CHECK: and i1
+
+  res = __builtin_flt_rounds();
+  // CHECK: call i32 @llvm.flt.rounds(
 }
 
 // CHECK-LABEL: define void @test_float_builtin_ops
Index: cfe/trunk/lib/CodeGen/CGBuiltin.cpp
===
--- cfe/trunk/lib/CodeGen/CGBuiltin.cpp
+++ cfe/trunk/lib/CodeGen/CGBuiltin.cpp
@@ -2130,6 +2130,17 @@
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 
+  case Builtin::BI__builtin_flt_rounds: {
+Value *F = CGM.getIntrinsic(Intrinsic::flt_rounds);
+
+llvm::Type *ResultType = ConvertType(E->getType());
+Value *Result = Builder.CreateCall(F);
+if (Result->getType() != ResultType)
+  Result = Builder.CreateIntCast(Result, ResultType, /*isSigned*/true,
+ "cast");
+return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_fpclassify: {
 Value *V = EmitScalarExpr(E->getArg(5));
 llvm::Type *Ty = ConvertType(E->getArg(5)->getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D24461: CodeGen: Cast llvm.flt.rounds result to match __builtin_flt_rounds

[Anton Korobeynikov via Phabricator via cfe-commits]

asl accepted this revision.
asl added a comment.

Committed, thanks!


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

https://reviews.llvm.org/D24461



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

[PATCH] D56841: [clangd] Filter out plugin related flags.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet planned changes to this revision.
kadircet marked an inline comment as done.
kadircet added a comment.

Moving stripper to tooling.




Comment at: clangd/ClangdUnit.cpp:433
+// 
+if (I + 4 < E && CommandLine[I] == "-Xclang" &&
+(CommandLine[I + 1] == "-load" || CommandLine[I + 1] == "-plugin" ||

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Maybe move this code into `ClangdServer::getCompileCommand` to keep 
> > > **all** argument adjustments that we do in clangd in one place.
> > > We add `-resource-dir` there.
> > As mentioned in the change description, we also have a different compiler 
> > invocation in `BackgroundIndex` which is not aware of 
> > `ClangdServer::getCompileCommand`
> Ah, right :-( That's annoying. Does it also manually override 
> `-resource-dir`? I'd move both into a single place, maybe this function is a 
> better place to do so.
It turned out to be more complicated actually, for example CodeCompletion also 
invokes a frontend action and it doesn't go through this function.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56841



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

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Some minor nits, but this is a good improvement!




Comment at: clang/docs/LibASTMatchersReference.html:4823
+  matches "x.m", but not "m"
+memberExpr(hasObjectExpression(hasType(pointsTo(
+ cxxRecordDecl(hasName("X"))

steveire wrote:
> If we're going to put such examples and details into the docs, maybe we 
> should explain why things are and are not matched. What do you think?
> 
> It seems in this case the type of the object expression of `m` is `X*` 
> because of the hidden `this->`?
I think that adding more clarity like this would be very nice.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2905
+/// cxxMemberCallExpr(on(callExpr()))
+///   matches `(g()).m()`
 ///

Missing a full stop.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5017
+/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+///   matches "x.m", but not "m"
+/// memberExpr(hasObjectExpression(hasType(pointsTo(

how about `matches "x.m", but not "m"; however,`



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5020
+//  cxxRecordDecl(hasName("X"))
+///   matches "m", but not "x.m"
 AST_POLYMORPHIC_MATCHER_P(

Missing a full stop at the end of the sentence.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56849



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

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, ioeric.

Currently both clangd and clang-tidy makes use of this mechanism so
putting it into tooling so that all tools can make use of it.


Repository:
  rC Clang

https://reviews.llvm.org/D56856

Files:
  include/clang/Tooling/ArgumentsAdjusters.h
  lib/Tooling/ArgumentsAdjusters.cpp
  unittests/Tooling/ToolingTest.cpp


Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -450,6 +450,37 @@
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripPluginsAdjuster strips plugin related args.
+TEST(ClangToolTest, StripPluginsAdjuster) {
+  FixedCompilationDatabase Compilations(
+  "/", {"-Xclang", "-add-plugin", "-Xclang", "random-plugin"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+return std::find(FinalArgs.begin(), FinalArgs.end(), Flag) !=
+   FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("-Xclang"));
+  EXPECT_FALSE(HasFlag("-add-plugin"));
+  EXPECT_FALSE(HasFlag("-random-plugin"));
+}
+
 namespace {
 /// Find a target name such that looking for it in TargetRegistry by that name
 /// returns the same target. We expect that there is at least one target
Index: lib/Tooling/ArgumentsAdjusters.cpp
===
--- lib/Tooling/ArgumentsAdjusters.cpp
+++ lib/Tooling/ArgumentsAdjusters.cpp
@@ -108,5 +108,28 @@
   };
 }
 
+// Strips plugin related command line arguments.
+ArgumentsAdjuster getClangStripPluginsAdjuster() {
+  return [](const CommandLineArguments &Args, StringRef /*unused*/) {
+CommandLineArguments AdjustedArgs;
+for (size_t I = 0, E = Args.size(); I != E; I++) {
+  // According to https://clang.llvm.org/docs/ClangPlugins.html
+  // plugin arguments are in the form:
+  // -Xclang {-load, -plugin, -plugin-arg-, -add-plugin}
+  // -Xclang 
+  if (I + 4 < E && Args[I] == "-Xclang" &&
+  (Args[I + 1] == "-load" || Args[I + 1] == "-plugin" ||
+   llvm::StringRef(Args[I + 1]).startswith("-plugin-arg-") ||
+   Args[I + 1] == "-add-plugin") &&
+  Args[I + 2] == "-Xclang") {
+I += 3;
+continue;
+  }
+  AdjustedArgs.push_back(Args[I]);
+}
+return AdjustedArgs;
+  };
+}
+
 } // end namespace tooling
 } // end namespace clang
Index: include/clang/Tooling/ArgumentsAdjusters.h
===
--- include/clang/Tooling/ArgumentsAdjusters.h
+++ include/clang/Tooling/ArgumentsAdjusters.h
@@ -66,6 +66,10 @@
 ArgumentsAdjuster combineAdjusters(ArgumentsAdjuster First,
ArgumentsAdjuster Second);
 
+/// Gets an argument adjuster which strips plugin related command line
+/// arguments.
+ArgumentsAdjuster getClangStripPluginsAdjuster();
+
 } // namespace tooling
 } // namespace clang
 


Index: unittests/Tooling/ToolingTest.cpp
===
--- unittests/Tooling/ToolingTest.cpp
+++ unittests/Tooling/ToolingTest.cpp
@@ -450,6 +450,37 @@
   EXPECT_TRUE(HasFlag("-w"));
 }
 
+// Check getClangStripPluginsAdjuster strips plugin related args.
+TEST(ClangToolTest, StripPluginsAdjuster) {
+  FixedCompilationDatabase Compilations(
+  "/", {"-Xclang", "-add-plugin", "-Xclang", "random-plugin"});
+
+  ClangTool Tool(Compilations, std::vector(1, "/a.cc"));
+  Tool.mapVirtualFile("/a.cc", "void a() {}");
+
+  std::unique_ptr Action(
+  newFrontendActionFactory());
+
+  CommandLineArguments FinalArgs;
+  ArgumentsAdjuster CheckFlagsAdjuster =
+  [&FinalArgs](const CommandLineArguments &Args, StringRef /*unused*/) {
+FinalArgs = Args;
+return Args;
+  };
+  Tool.clearArgumentsAdjusters();
+  Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
+  Tool.appendArgumentsAdjuster(CheckFlagsAdjuster);
+  Tool.run(Action.get());
+
+  auto HasFlag = [&FinalArgs](const std::string &Flag) {
+return std::find(FinalArgs.begin(), FinalArgs.end(), Flag) !=
+   FinalArgs.end();
+  };
+  EXPECT_FALSE(HasFlag("-Xclang"));
+  EXPECT_FALSE(HasFlag("-add-plugin"));
+  EXPECT_FALSE(HasFlag("-random-plugin"));
+}
+
 namespace {
 /// Find a target name such

Re: r350768 - [ObjC] Allow the use of implemented unavailable methods from within

[Nico Weber via cfe-commits]

For the archives, here's said patch: https://reviews.llvm.org/D56816

(Thanks!)

On Wed, Jan 16, 2019 at 5:26 PM Alex L  wrote:

> Hi,
>
> We are planning to fix this issue by not checking if the method is
> defined. I will post a patch this week.
>
> Cheers,
> Alex
>
> On Fri, 11 Jan 2019 at 10:26, Alex L  wrote:
>
>> Thanks, we might have similar cases in our code base as well. We'll see
>> if we can fix that too.
>>
>> On Fri, 11 Jan 2019 at 06:13, Nico Weber  wrote:
>>
>>> Here's some user feedback on this new feature.
>>>
>>> It looks like the warning is only suppressed if `init` has a definition
>>> in the @interface block. In the 4 cases where we saw this warning fire
>>> after r349841, it still fires after this change because in all 4 cases a
>>> class marked init as unavailable in the @interface but didn't add a
>>> definition of it in the classes @implementation (instead relying on calling
>>> the superclass's (NSObject) init).
>>>
>>> It's pretty easy to just add
>>>
>>> - (instancetype)init { return [super init]; }
>>>
>>> to these 4 classes, but:
>>> a) it doesn't Just Work
>>> b) the diagnostic doesn't make it clear that adding a definition of init
>>> will suppress the warning.
>>>
>>> Up to you to decide what (if anything) to do with this feedback :-)
>>>
>>> (https://bugs.chromium.org/p/chromium/issues/detail?id=917351#c17 has a
>>> full reduced code snippet.)
>>>
>>> On Wed, Jan 9, 2019 at 5:35 PM Alex Lorenz via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: arphaman
 Date: Wed Jan  9 14:31:37 2019
 New Revision: 350768

 URL: http://llvm.org/viewvc/llvm-project?rev=350768&view=rev
 Log:
 [ObjC] Allow the use of implemented unavailable methods from within
 the @implementation context

 In Objective-C, it's common for some frameworks to mark some methods
 like init
 as unavailable in the @interface to prohibit their usage. However, these
 frameworks then often implemented said method and refer to it in
 another method
 that acts as a factory for that object. The recent change to how
 messages to
 self are type checked in clang (r349841) introduced a regression which
 started
 to prohibit this pattern with an X is unavailable error. This commit
 addresses
 the aforementioned regression.

 rdar://47134898

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

 Added:
 cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
 Modified:
 cfe/trunk/lib/Sema/SemaDeclAttr.cpp

 Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=350768&r1=350767&r2=350768&view=diff

 ==
 --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
 +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Wed Jan  9 14:31:37 2019
 @@ -7269,9 +7269,10 @@ ShouldDiagnoseAvailabilityOfDecl(Sema &S
  /// whether we should emit a diagnostic for \c K and \c DeclVersion in
  /// the context of \c Ctx. For example, we should emit an unavailable
 diagnostic
  /// in a deprecated context, but not the other way around.
 -static bool ShouldDiagnoseAvailabilityInContext(Sema &S,
 AvailabilityResult K,
 -VersionTuple
 DeclVersion,
 -Decl *Ctx) {
 +static bool
 +ShouldDiagnoseAvailabilityInContext(Sema &S, AvailabilityResult K,
 +VersionTuple DeclVersion, Decl
 *Ctx,
 +const NamedDecl *OffendingDecl) {
assert(K != AR_Available && "Expected an unavailable declaration
 here!");

// Checks if we should emit the availability diagnostic in the
 context of C.
 @@ -7280,9 +7281,22 @@ static bool ShouldDiagnoseAvailabilityIn
if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context,
 C))
  if (AA->getIntroduced() >= DeclVersion)
return true;
 -} else if (K == AR_Deprecated)
 +} else if (K == AR_Deprecated) {
if (C->isDeprecated())
  return true;
 +} else if (K == AR_Unavailable) {
 +  // It is perfectly fine to refer to an 'unavailable' Objective-C
 method
 +  // when it's actually defined and is referenced from within the
 +  // @implementation itself. In this context, we interpret
 unavailable as a
 +  // form of access control.
 +  if (const auto *MD = dyn_cast(OffendingDecl)) {
 +if (const auto *Impl = dyn_cast(C)) {
 +  if (MD->getClassInterface() == Impl->getClassInterface() &&
 +  MD->isDefined())
 +return true;
 +}
 +  }
 +}

  if (C->is

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Extended test for matcher `on`.


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

https://reviews.llvm.org/D56850

Files:
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -470,6 +470,97 @@
 
 }
 
+TEST(MatcherCXXMemberCallExpr, On) {
+  auto Snippet1 = R"cc(
+struct Y {
+  void m();
+};
+void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+struct Y {
+  void m();
+};
+struct X : public Y {};
+void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y");
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(notMatches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(notMatches(Snippet1, MatchesX));
+  EXPECT_TRUE(matches(Snippet2, MatchesX));
+
+  // Parens are ignored.
+  auto MatchesCall = cxxMemberCallExpr(on(callExpr()));
+  EXPECT_TRUE(matches(
+  R"cc(
+struct Y {
+  void m();
+};
+Y g();
+void z(Y y) { (g()).m(); }
+  )cc",
+  MatchesCall));
+}
+
+TEST(MatcherCXXMemberCallExpr, OnImplicitObjectArgument) {
+  auto Snippet1 = R"cc(
+struct Y {
+  void m();
+};
+void z(Y y) { y.m(); }
+  )cc";
+  auto Snippet2 = R"cc(
+struct Y {
+  void m();
+};
+struct X : public Y {};
+void z(X x) { x.m(); }
+  )cc";
+  auto MatchesY = cxxMemberCallExpr(
+  onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("Y");
+  EXPECT_TRUE(matches(Snippet1, MatchesY));
+  EXPECT_TRUE(matches(Snippet2, MatchesY));
+
+  auto MatchesX = cxxMemberCallExpr(
+  onImplicitObjectArgument(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(notMatches(Snippet2, MatchesX));
+
+  // Parens are not ignored.
+  auto MatchesCall = cxxMemberCallExpr(onImplicitObjectArgument(callExpr()));
+  EXPECT_TRUE(notMatches(
+  R"cc(
+struct Y {
+  void m();
+};
+Y g();
+void z(Y y) { (g()).m(); }
+  )cc",
+  MatchesCall));
+}
+
+TEST(Matcher, HasObjectExpr) {
+  auto M = memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(
+  R"cc(
+struct X {
+  int m;
+  int f(X x) { return x.m; }
+};
+  )cc",
+  M));
+  EXPECT_TRUE(notMatches(
+  R"cc(
+struct X {
+  int m;
+  int f(X x) { return m; }
+};
+  )cc",
+  M));
+}
+
 TEST(ForEachArgumentWithParam, ReportsNoFalsePositives) {
   StatementMatcher ArgumentY =
 declRefExpr(to(varDecl(hasName("y".bind("arg");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56850: [ASTMatchers][NFC] Add tests for assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:475
+  auto MatchesType = 
cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X");
+  EXPECT_TRUE(matches(
+  R"cc(

steveire wrote:
> Could you add a test which uses the same c++ snippet as this, but with a 
> matcher using `hasName("Y")`?
Good suggestion -- this caught a mistake in the comments. I'll update the other 
diff to reflect.


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

https://reviews.llvm.org/D56850



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

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Respond to comments; fix mistake (revealed by test).


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

https://reviews.llvm.org/D56849

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h

Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2887,14 +2887,22 @@
  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
 }
 
-/// Matches on the implicit object argument of a member call expression.
+/// Matches on the implicit object argument of a member call expression, after
+/// stripping off any parentheses or implicit casts.
 ///
-/// Example matches y.x()
-///   (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))
+/// Given
 /// \code
-///   class Y { public: void x(); };
-///   void z() { Y y; y.x(); }
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y {};
+///   void z(Y y, X x) { y.m(); (g()).m(); x.m(); }
 /// \endcode
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y")
+///   matches `y.m()` and `(g()).m()`.
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")
+///   matches `x.m()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   matches `(g()).m()`.
 ///
 /// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, on, internal::Matcher,
@@ -3254,6 +3262,23 @@
   .matches(Node, Finder, Builder);
 }
 
+/// Matches on the implicit object argument of a member call expression. Unlike
+/// `on`, matches the argument directly without stripping away anything.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y { void g(); };
+///   void z(Y y, X x) { y.m(); x.m(); x.g(); (g()).m(); }
+/// \endcode
+/// cxxMemberCallExpr(onImplicitObjectArgument(hasType(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `x.m()` and (g()).m(), but not `x.g()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   does not match `(g()).m()`, because the parens are not ignored.
+///
+/// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, onImplicitObjectArgument,
   internal::Matcher, InnerMatcher) {
   const Expr *ExprNode = Node.getImplicitObjectArgument();
@@ -3261,8 +3286,22 @@
   InnerMatcher.matches(*ExprNode, Finder, Builder));
 }
 
-/// Matches if the expression's type either matches the specified
-/// matcher, or is a pointer to a type that matches the InnerMatcher.
+/// Matches if the type of the expression's implicit object argument either
+/// matches the InnerMatcher, or is a pointer to a type that matches the
+/// InnerMatcher.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   class X : public Y { void g(); };
+///   void z() { Y y; y.m(); Y *p; p->m(); X x; x.m(); x.g(); }
+/// \endcode
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `p->m()` and `x.m()`.
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("X")
+///   matches `x.g()`.
 AST_MATCHER_P_OVERLOAD(CXXMemberCallExpr, thisPointerType,
internal::Matcher, InnerMatcher, 0) {
   return onImplicitObjectArgument(
@@ -4964,18 +5003,22 @@
   return InnerMatcher.matches(*Node.getMemberDecl(), Finder, Builder);
 }
 
-/// Matches a member expression where the object expression is
-/// matched by a given matcher.
+/// Matches a member expression where the object expression is matched by a
+/// given matcher. Implicit object expressions are included; that is, it matches
+/// use of implicit `this`.
 ///
 /// Given
 /// \code
-///   struct X { int m; };
-///   void f(X x) { x.m; m; }
+///   struct X {
+/// int m;
+/// int f(X x) { x.m; return m; }
+///   };
 /// \endcode
-/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
-///   matches "x.m" and "m"
-/// with hasObjectExpression(...)
-///   matching "x" and the implicit object expression of "m" which has type X*.
+/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+///   matches "x.m", but not "m"; however,
+/// memberExpr(hasObjectExpression(hasType(pointsTo(
+//  cxxRecordDecl(hasName("X"))
+///   matches "m" (aka. "this->m"), but not "x.m".
 AST_POLYMORPHIC_MATCHER_P(
 hasObjectExpression,
 AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4814,12 +4814,15 @@
 matched by a given matcher.
 
 Given
-  struct X { int m; };
-  void f(X x) { x.m; m; }
-mem

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 182301.
ymandel marked 2 inline comments as done.
ymandel added a comment.

Update HTML docs.


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

https://reviews.llvm.org/D56849

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h

Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2887,14 +2887,22 @@
  InnerMatcher.matches(*UnderlyingDecl, Finder, Builder);
 }
 
-/// Matches on the implicit object argument of a member call expression.
+/// Matches on the implicit object argument of a member call expression, after
+/// stripping off any parentheses or implicit casts.
 ///
-/// Example matches y.x()
-///   (matcher = cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y"))
+/// Given
 /// \code
-///   class Y { public: void x(); };
-///   void z() { Y y; y.x(); }
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y {};
+///   void z(Y y, X x) { y.m(); (g()).m(); x.m(); }
 /// \endcode
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("Y")
+///   matches `y.m()` and `(g()).m()`.
+/// cxxMemberCallExpr(on(hasType(cxxRecordDecl(hasName("X")
+///   matches `x.m()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   matches `(g()).m()`.
 ///
 /// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, on, internal::Matcher,
@@ -3254,6 +3262,23 @@
   .matches(Node, Finder, Builder);
 }
 
+/// Matches on the implicit object argument of a member call expression. Unlike
+/// `on`, matches the argument directly without stripping away anything.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   Y g();
+///   class X : public Y { void g(); };
+///   void z(Y y, X x) { y.m(); x.m(); x.g(); (g()).m(); }
+/// \endcode
+/// cxxMemberCallExpr(onImplicitObjectArgument(hasType(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `x.m()` and (g()).m(), but not `x.g()`.
+/// cxxMemberCallExpr(on(callExpr()))
+///   does not match `(g()).m()`, because the parens are not ignored.
+///
+/// FIXME: Overload to allow directly matching types?
 AST_MATCHER_P(CXXMemberCallExpr, onImplicitObjectArgument,
   internal::Matcher, InnerMatcher) {
   const Expr *ExprNode = Node.getImplicitObjectArgument();
@@ -3261,8 +3286,22 @@
   InnerMatcher.matches(*ExprNode, Finder, Builder));
 }
 
-/// Matches if the expression's type either matches the specified
-/// matcher, or is a pointer to a type that matches the InnerMatcher.
+/// Matches if the type of the expression's implicit object argument either
+/// matches the InnerMatcher, or is a pointer to a type that matches the
+/// InnerMatcher.
+///
+/// Given
+/// \code
+///   class Y { public: void m(); };
+///   class X : public Y { void g(); };
+///   void z() { Y y; y.m(); Y *p; p->m(); X x; x.m(); x.g(); }
+/// \endcode
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("Y")
+///   matches `y.m()`, `p->m()` and `x.m()`.
+/// cxxMemberCallExpr(thisPointerType(hasDeclaration(
+/// cxxRecordDecl(hasName("X")
+///   matches `x.g()`.
 AST_MATCHER_P_OVERLOAD(CXXMemberCallExpr, thisPointerType,
internal::Matcher, InnerMatcher, 0) {
   return onImplicitObjectArgument(
@@ -4964,18 +5003,22 @@
   return InnerMatcher.matches(*Node.getMemberDecl(), Finder, Builder);
 }
 
-/// Matches a member expression where the object expression is
-/// matched by a given matcher.
+/// Matches a member expression where the object expression is matched by a
+/// given matcher. Implicit object expressions are included; that is, it matches
+/// use of implicit `this`.
 ///
 /// Given
 /// \code
-///   struct X { int m; };
-///   void f(X x) { x.m; m; }
+///   struct X {
+/// int m;
+/// int f(X x) { x.m; return m; }
+///   };
 /// \endcode
-/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")))
-///   matches "x.m" and "m"
-/// with hasObjectExpression(...)
-///   matching "x" and the implicit object expression of "m" which has type X*.
+/// memberExpr(hasObjectExpression(hasType(cxxRecordDecl(hasName("X")
+///   matches "x.m", but not "m"; however,
+/// memberExpr(hasObjectExpression(hasType(pointsTo(
+//  cxxRecordDecl(hasName("X"))
+///   matches "m" (aka. "this->m"), but not "x.m".
 AST_POLYMORPHIC_MATCHER_P(
 hasObjectExpression,
 AST_POLYMORPHIC_SUPPORTED_TYPES(MemberExpr, UnresolvedMemberExpr,
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4810,16 +4810,20 @@
 
 
 Matcher

[PATCH] D56849: [ASTMatchers][NFC] Update comments on assorted `CXXMemberCallExpr` matchers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2905
+/// cxxMemberCallExpr(on(callExpr()))
+///   matches `(g()).m()`
 ///

aaron.ballman wrote:
> Missing a full stop.
also fixed mistake in first example (turned up by actually testing the claim. 
:) )


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

https://reviews.llvm.org/D56849



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

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Tooling/ArgumentsAdjusters.h:69
 
+/// Gets an argument adjuster which strips plugin related command line
+/// arguments.

Maybe put the function before `combineAdjusters`?
The latter is a function that works on two adjusters, therefore a bit different 
from the rest, which are adjusters themselves.



Comment at: include/clang/Tooling/ArgumentsAdjusters.h:71
+/// arguments.
+ArgumentsAdjuster getClangStripPluginsAdjuster();
+

Maybe drop "clang" from the name? `getStripPluginsAdjuster()?



Comment at: lib/Tooling/ArgumentsAdjusters.cpp:111
 
+// Strips plugin related command line arguments.
+ArgumentsAdjuster getClangStripPluginsAdjuster() {

remove the comment? we already have one in the header


Repository:
  rC Clang

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

https://reviews.llvm.org/D56856



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

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Just a few NITs from my side, LG when fixed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D56856



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

[PATCH] D56860: [clangd] NFC: Use buildCompilerInvocation in CodeComplete

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D56860

Files:
  clangd/CodeComplete.cpp


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -1017,25 +1017,11 @@
   const SemaCompleteInput &Input,
   IncludeStructure *Includes = nullptr) {
   trace::Span Tracer("Sema completion");
-  std::vector ArgStrs;
-  for (const auto &S : Input.Command.CommandLine)
-ArgStrs.push_back(S.c_str());
-
-  if (Input.VFS->setCurrentWorkingDirectory(Input.Command.Directory)) {
-log("Couldn't set working directory");
-// We run parsing anyway, our lit-tests rely on results for non-existing
-// working dirs.
-  }
-
   llvm::IntrusiveRefCntPtr VFS = Input.VFS;
   if (Input.Preamble && Input.Preamble->StatCache)
 VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS));
-  IgnoreDiagnostics DummyDiagsConsumer;
-  auto CI = createInvocationFromCommandLine(
-  ArgStrs,
-  CompilerInstance::createDiagnostics(new DiagnosticOptions,
-  &DummyDiagsConsumer, false),
-  VFS);
+  auto CI =
+  buildCompilerInvocation(ParseInputs{Input.Command, VFS, Input.Contents});
   if (!CI) {
 elog("Couldn't create CompilerInvocation");
 return false;
@@ -1073,6 +1059,7 @@
   *Offset;
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. 
Otherwise
   // the remapped buffers do not get freed.
+  IgnoreDiagnostics DummyDiagsConsumer;
   auto Clang = prepareCompilerInstance(
   std::move(CI),
   (Input.Preamble && !CompletingInPreamble) ? &Input.Preamble->Preamble


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -1017,25 +1017,11 @@
   const SemaCompleteInput &Input,
   IncludeStructure *Includes = nullptr) {
   trace::Span Tracer("Sema completion");
-  std::vector ArgStrs;
-  for (const auto &S : Input.Command.CommandLine)
-ArgStrs.push_back(S.c_str());
-
-  if (Input.VFS->setCurrentWorkingDirectory(Input.Command.Directory)) {
-log("Couldn't set working directory");
-// We run parsing anyway, our lit-tests rely on results for non-existing
-// working dirs.
-  }
-
   llvm::IntrusiveRefCntPtr VFS = Input.VFS;
   if (Input.Preamble && Input.Preamble->StatCache)
 VFS = Input.Preamble->StatCache->getConsumingFS(std::move(VFS));
-  IgnoreDiagnostics DummyDiagsConsumer;
-  auto CI = createInvocationFromCommandLine(
-  ArgStrs,
-  CompilerInstance::createDiagnostics(new DiagnosticOptions,
-  &DummyDiagsConsumer, false),
-  VFS);
+  auto CI =
+  buildCompilerInvocation(ParseInputs{Input.Command, VFS, Input.Contents});
   if (!CI) {
 elog("Couldn't create CompilerInvocation");
 return false;
@@ -1073,6 +1059,7 @@
   *Offset;
   // NOTE: we must call BeginSourceFile after prepareCompilerInstance. Otherwise
   // the remapped buffers do not get freed.
+  IgnoreDiagnostics DummyDiagsConsumer;
   auto Clang = prepareCompilerInstance(
   std::move(CI),
   (Input.Preamble && !CompletingInPreamble) ? &Input.Preamble->Preamble
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182315.
ilya-biryukov marked 9 inline comments as done.
ilya-biryukov added a comment.

- Remove intermediate StringMap, use registry directly
- Rename codeAction to codeTweak in ClangdServer
- Rename a helper to get position to sourceLocationInMainFile
- Move the Registry typedef into the .cpp file
- Make toCodeAction a helper function
- Update comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267

Files:
  clangd/CMakeLists.txt
  clangd/ClangdLSPServer.cpp
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/Tweak.cpp
  clangd/refactor/Tweak.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/tool/CMakeLists.txt
  test/clangd/initialize-params.test

Index: test/clangd/initialize-params.test
===
--- test/clangd/initialize-params.test
+++ test/clangd/initialize-params.test
@@ -25,7 +25,8 @@
 # CHECK-NEXT:  "documentSymbolProvider": true,
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
-# CHECK-NEXT:  "clangd.applyFix"
+# CHECK-NEXT:  "clangd.applyFix",
+# CHECK-NEXT:  "clangd.applyCodeAction"
 # CHECK-NEXT:]
 # CHECK-NEXT:  },
 # CHECK-NEXT:  "hoverProvider": true,
Index: clangd/tool/CMakeLists.txt
===
--- clangd/tool/CMakeLists.txt
+++ clangd/tool/CMakeLists.txt
@@ -3,6 +3,7 @@
 
 add_clang_tool(clangd
   ClangdMain.cpp
+  $
   )
 
 set(LLVM_LINK_COMPONENTS
Index: clangd/refactor/tweaks/CMakeLists.txt
===
--- /dev/null
+++ clangd/refactor/tweaks/CMakeLists.txt
@@ -0,0 +1,11 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../..)
+
+# A target containing all code tweaks (i.e. mini-refactorings) provided by
+# clangd.
+# Built as an object library to make sure linker does not remove global
+# constructors that register individual tweaks in a global registry.
+# To enable these tweaks in exectubales or shared libraries, add
+# $ to a list of sources, see
+# clangd/tool/CMakeLists.txt for an example.
+add_clang_library(clangDaemonTweaks OBJECT
+  )
Index: clangd/refactor/Tweak.h
===
--- /dev/null
+++ clangd/refactor/Tweak.h
@@ -0,0 +1,96 @@
+//===--- Tweak.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Tweaks are small refactoring-like actions that run over the AST and produce
+// the set of edits as a result. They are local, i.e. they should take the
+// current editor context, e.g. the cursor position and selection into account.
+// The actions are executed in two stages:
+//   - Stage 1 should check whether the action is available in a current
+// context. It should be cheap and fast to compute as it is executed for all
+// available actions on every client request, which happen quite frequently.
+//   - Stage 2 is performed after stage 1 and can be more expensive to compute.
+// It is performed when the user actually chooses the action.
+// To avoid extra round-trips and AST reads, actions can also produce results on
+// stage 1 in cases when the actions are fast to compute.
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_ACTIONS_TWEAK_H
+
+#include "ClangdUnit.h"
+#include "Protocol.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+namespace clang {
+namespace clangd {
+
+using TweakID = llvm::StringRef;
+
+/// An interface base for small context-sensitive refactoring actions.
+class Tweak {
+public:
+  /// Input to prepare and apply tweaks.
+  struct Selection {
+static llvm::Optional create(llvm::StringRef File,
+llvm::StringRef Code,
+ParsedAST &AST, Range Selection);
+
+/// The path of an active document the action was invoked in.
+llvm::StringRef File;
+/// The text of the active document.
+llvm::StringRef Code;
+/// Parsed AST of the active file.
+ParsedAST *
+/// A location of the cursor in the editor.
+SourceLocation Cursor;
+// FIXME: add selection when there are checks relying on it.
+// FIXME: provide a way to get sources and ASTs for other files.
+// FIXME: cache some commonly require

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182316.
ilya-biryukov added a comment.

- Update to reflect changes in parent revision


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp
  clangd/refactor/tweaks/QualifyName.h

Index: clangd/refactor/tweaks/QualifyName.h
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.h
@@ -0,0 +1,42 @@
+//===--- QualifyName.h ---*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Fully qualifies a name under a cursor.
+// Before:
+//   using namespace std;
+//   ^vector foo;
+// After:
+//   std::vector foo;
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_QUALIFYNAME_H
+
+#include "refactor/Tweak.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,163 @@
+//===--- QualifyName.cpp -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "QualifyName.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(QualifyName);
+
+namespace {
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+return nullptr; // FIXME: handle this situation, retrieve the thing
+// referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+   // TranslationUnitDecl.
+return nullptr;
+return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor {
+public:
+  LocateInsertLoc(ASTContext &Ctx, SourceLocation CursorLoc,
+  Reference &UnqualRef)
+  : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+  return false;
+return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (NNS.getPrefix())
+  return true; // we want only unqualified names.
+auto &SM = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+if (!Target)
+  return true; // continue traversal to recurse into types, if any.
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Target;
+return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182317.
ilya-biryukov added a comment.

- Update to reflect changes in parent revision


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56611

Files:
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/SwapIfBranches.cpp
  clangd/refactor/tweaks/SwapIfBranches.h

Index: clangd/refactor/tweaks/SwapIfBranches.h
===
--- /dev/null
+++ clangd/refactor/tweaks/SwapIfBranches.h
@@ -0,0 +1,41 @@
+//===--- SwapIfBrances.h -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Swaps the 'then' and 'else' branch of the if statement.
+// Before:
+//   if (foo) { return 10; } else { continue; }
+// After:
+//   if (foo) { continue; } else { return 10; }
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_SWAPIFBRANCHES_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_SWAPIFBRANCHES_H
+
+#include "refactor/Tweak.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/StringRef.h"
+
+namespace clang {
+namespace clangd {
+
+class SwapIfBranches : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("swap-if-branches"); }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  tooling::Replacements Result;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -0,0 +1,102 @@
+//===--- SwapIfBranches.cpp --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "SwapIfBranches.h"
+#include "ClangdUnit.h"
+#include "Logger.h"
+#include "SourceCode.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(SwapIfBranches);
+
+namespace {
+class FindIfUnderCursor : public RecursiveASTVisitor {
+public:
+  FindIfUnderCursor(ASTContext &Ctx, SourceLocation CursorLoc, IfStmt *&Result)
+  : Ctx(Ctx), CursorLoc(CursorLoc), Result(Result) {}
+
+  bool VisitIfStmt(IfStmt *If) {
+auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(),
+ SourceRange(If->getIfLoc()));
+if (!R)
+  return true;
+if (!halfOpenRangeContains(Ctx.getSourceManager(), *R, CursorLoc))
+  return true;
+Result = If;
+return false;
+  }
+
+private:
+  ASTContext &Ctx;
+  SourceLocation CursorLoc;
+  IfStmt *&Result;
+};
+} // namespace
+
+bool SwapIfBranches::prepare(const Selection &Inputs) {
+
+  auto &Ctx = Inputs.AST.getASTContext();
+  auto &SrcMgr = Ctx.getSourceManager();
+  IfStmt *If = nullptr;
+  FindIfUnderCursor(Ctx, Inputs.Cursor, If).TraverseAST(Ctx);
+  if (!If)
+return false;
+
+  // avoid dealing with single-statement brances, they require careful handling
+  // to avoid changing semantics of the code (i.e. dangling else).
+  if (!llvm::dyn_cast_or_null(If->getThen()) ||
+  !llvm::dyn_cast_or_null(If->getElse()))
+return false;
+
+  auto ThenRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getThen()->getSourceRange());
+  if (!ThenRng)
+return false;
+  auto ElseRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getElse()->getSourceRange());
+  if (!ElseRng)
+return false;
+
+  llvm::StringRef ThenCode = toSourceCode(SrcMgr, *ThenRng);
+  llvm::StringRef ElseCode = toSourceCode(SrcMgr, *ElseRng);
+
+  if (auto Err = Result.add(tooling::Replacement(SrcMgr, ThenRng->getBegin(),
+ ThenCode.size(), ElseCode))) {
+llvm::consumeError(std::move(Err));
+return false;
+  }
+  if (auto Err = Result.add(tool

[PATCH] D56612: [clangd] A code action to remove 'using namespace'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182318.
ilya-biryukov added a comment.

- Update to reflect changes in parent revision


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56612

Files:
  clangd/AST.cpp
  clangd/AST.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clangd/refactor/tweaks/RemoveUsingNamespace.h

Index: clangd/refactor/tweaks/RemoveUsingNamespace.h
===
--- /dev/null
+++ clangd/refactor/tweaks/RemoveUsingNamespace.h
@@ -0,0 +1,42 @@
+//===--- RemoveUsingNamespace.h --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+// Removes the 'using namespace' under the cursor and qualifies all accesses in
+// the current file. E.g.,
+//   using namespace std;
+//   vector foo(std::map);
+// Would become:
+//   std::vector foo(std::map);
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_REMOVEUSINGNAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_REFACTOR_TWEAKS_REMOVEUSINGNAMESPACE_H
+
+#include "refactor/Tweak.h"
+
+namespace clang {
+namespace clangd {
+
+class RemoveUsingNamespace : public Tweak {
+public:
+  TweakID id() const override {
+return llvm::StringLiteral("remove-using-namespace");
+  }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  UsingDirectiveDecl *TargetDirective = nullptr;
+};
+
+} // namespace clangd
+} // namespace clang
+
+#endif
Index: clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -0,0 +1,188 @@
+//===--- RemoveUsingNamespace.cpp *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "RemoveUsingNamespace.h"
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
+
+namespace clang {
+namespace clangd {
+
+REGISTER_TWEAK(RemoveUsingNamespace);
+
+namespace {
+class FindNodeUnderCursor : public RecursiveASTVisitor {
+public:
+  FindNodeUnderCursor(SourceLocation SearchedLoc, UsingDirectiveDecl *&Result)
+  : SearchedLoc(SearchedLoc), Result(Result) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getUsingLoc() != SearchedLoc)
+  return true;
+
+Result = D;
+return false;
+  }
+
+private:
+  SourceLocation SearchedLoc;
+  UsingDirectiveDecl *&Result;
+};
+
+class FindSameUsings : public RecursiveASTVisitor {
+public:
+  FindSameUsings(UsingDirectiveDecl &Target,
+ std::vector &Results)
+  : TargetNS(Target.getNominatedNamespace()),
+TargetCtx(Target.getDeclContext()), Results(Results) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getNominatedNamespace() != TargetNS ||
+D->getDeclContext() != TargetCtx)
+  return true;
+
+Results.push_back(D);
+return true;
+  }
+
+private:
+  NamespaceDecl *TargetNS;
+  DeclContext *TargetCtx;
+  std::vector &Results;
+};
+
+class FindIdentsToQualify
+: public tooling::RecursiveSymbolVisitor {
+public:
+  FindIdentsToQualify(ASTContext &Ctx, NamespaceDecl &TargetNS,
+  std::vector &ResultIdents)
+  : RecursiveSymbolVisitor(Ctx.getSourceManager(), Ctx.getLangOpts()),
+Ctx(Ctx), TargetNS(TargetNS), ResultIdents(ResultIdents) {}
+
+  bool visitSymbolOccurrence(const NamedDecl *D,
+ llvm::ArrayRef NameRanges) {
+if (!D || D->getCanonicalDecl() == TargetNS.getCanonicalDecl())
+  return true;
+if (!D->getDeclName().isIdentifier() ||
+D->getDeclName().getNameKind() == DeclarationName::CXXOperatorName)
+  return true; // do not add qualifiers for non-idents, e.g. 'operator+'.
+// Check the symbol is unqualified and references something inside our
+// namespace.
+// FIXME: add a check it's unqualified.
+if (!TargetNS.InEnclosingNamespaceSetOf(D->getDeclContext()))
+  return true;
+// FIXME: handle more tricky cases, e.g. we don't need the qualifier if we
+//have the usi

[PATCH] D56802: [CodeGenObjC] Treat ivar offsets variables as constant if they refer to ivars of a direct subclass of NSObject with an @implementation

[John McCall via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D56802



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

[PATCH] D56610: [clangd] A code action to qualify an unqualified name

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182320.
ilya-biryukov added a comment.

- Remove the header file


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56610

Files:
  clangd/SourceCode.cpp
  clangd/SourceCode.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/QualifyName.cpp

Index: clangd/refactor/tweaks/QualifyName.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/QualifyName.cpp
@@ -0,0 +1,180 @@
+//===--- QualifyName.cpp -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TypeLoc.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Fully qualifies a name under a cursor.
+/// Before:
+///   using namespace std;
+///   ^vector foo;
+/// After:
+///   std::vector foo;
+class QualifyName : public Tweak {
+public:
+  TweakID id() const override { return llvm::StringLiteral("qualify-name"); }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  SourceLocation InsertLoc;
+  std::string Qualifier;
+};
+
+REGISTER_TWEAK(QualifyName);
+
+struct Reference {
+  SourceLocation Begin;
+  NamedDecl *Target = nullptr;
+
+  operator bool() const { return Target != nullptr; }
+};
+
+NamedDecl *toReferencedDecl(NestedNameSpecifier *NNS) {
+  switch (NNS->getKind()) {
+  case clang::NestedNameSpecifier::Namespace:
+return NNS->getAsNamespace();
+  case clang::NestedNameSpecifier::NamespaceAlias:
+return NNS->getAsNamespaceAlias();
+  case clang::NestedNameSpecifier::TypeSpec:
+  case clang::NestedNameSpecifier::TypeSpecWithTemplate:
+return nullptr; // FIXME: handle this situation, retrieve the thing
+// referenced inside a type.
+  case clang::NestedNameSpecifier::Identifier:
+  case clang::NestedNameSpecifier::Super:
+  case clang::NestedNameSpecifier::Global: // FIXME: could return
+   // TranslationUnitDecl.
+return nullptr;
+return nullptr;
+  }
+  llvm_unreachable("unhandled NestedNameSpecifier kind.");
+}
+
+class LocateInsertLoc : public RecursiveASTVisitor {
+public:
+  LocateInsertLoc(ASTContext &Ctx, SourceLocation CursorLoc,
+  Reference &UnqualRef)
+  : Ctx(Ctx), CursorLoc(CursorLoc), UnqualRef(UnqualRef) {}
+
+  bool shouldWalkTypesOfTypeLocs() const { return false; }
+
+  // FIXME: RAT does not have VisitNestedNameSpecifierLoc. Should we add that?
+  bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (!RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(NNS))
+  return false;
+return VisitNestedNameSpecifierLoc(NNS);
+  }
+
+  bool VisitNestedNameSpecifierLoc(NestedNameSpecifierLoc NNS) {
+if (NNS.getPrefix())
+  return true; // we want only unqualified names.
+auto &SM = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), NNS.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+auto *Target = toReferencedDecl(NNS.getNestedNameSpecifier());
+if (!Target)
+  return true; // continue traversal to recurse into types, if any.
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Target;
+return false; // we found the insertion point.
+  }
+
+  bool VisitDeclRefExpr(DeclRefExpr *E) {
+if (E->hasQualifier())
+  return true; // we want only unqualified names.
+auto &SM = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), E->getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = E->getFoundDecl();
+return false;
+  }
+
+  bool VisitTagTypeLoc(TagTypeLoc Loc) {
+auto &SM = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRange());
+if (!Rng)
+  return true;
+if (!halfOpenRangeContains(SM, *Rng, CursorLoc))
+  return true;
+UnqualRef.Begin = Rng->getBegin();
+UnqualRef.Target = Loc.getDecl();
+return false;
+  }
+
+  bool VisitTypedefTypeLoc(TypedefTypeLoc Loc) {
+auto &SM = Ctx.getSourceManager();
+auto Rng = toHalfOpenFileRange(SM, Ctx.getLangOpts(), Loc.getSourceRa

[PATCH] D56612: [clangd] A code action to remove 'using namespace'

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182322.
ilya-biryukov added a comment.

- Remove the header file


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56612

Files:
  clangd/AST.cpp
  clangd/AST.h
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/RemoveUsingNamespace.cpp

Index: clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -0,0 +1,206 @@
+//===--- RemoveUsingNamespace.cpp *- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "AST.h"
+#include "ClangdUnit.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Refactoring/RecursiveSymbolVisitor.h"
+#include "llvm/ADT/ScopeExit.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Removes the 'using namespace' under the cursor and qualifies all accesses in
+/// the current file. E.g.,
+///   using namespace std;
+///   vector foo(std::map);
+/// Would become:
+///   std::vector foo(std::map);
+class RemoveUsingNamespace : public Tweak {
+public:
+  TweakID id() const override {
+return llvm::StringLiteral("remove-using-namespace");
+  }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  UsingDirectiveDecl *TargetDirective = nullptr;
+};
+REGISTER_TWEAK(RemoveUsingNamespace);
+
+class FindNodeUnderCursor : public RecursiveASTVisitor {
+public:
+  FindNodeUnderCursor(SourceLocation SearchedLoc, UsingDirectiveDecl *&Result)
+  : SearchedLoc(SearchedLoc), Result(Result) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getUsingLoc() != SearchedLoc)
+  return true;
+
+Result = D;
+return false;
+  }
+
+private:
+  SourceLocation SearchedLoc;
+  UsingDirectiveDecl *&Result;
+};
+
+class FindSameUsings : public RecursiveASTVisitor {
+public:
+  FindSameUsings(UsingDirectiveDecl &Target,
+ std::vector &Results)
+  : TargetNS(Target.getNominatedNamespace()),
+TargetCtx(Target.getDeclContext()), Results(Results) {}
+
+  bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) {
+if (D->getNominatedNamespace() != TargetNS ||
+D->getDeclContext() != TargetCtx)
+  return true;
+
+Results.push_back(D);
+return true;
+  }
+
+private:
+  NamespaceDecl *TargetNS;
+  DeclContext *TargetCtx;
+  std::vector &Results;
+};
+
+class FindIdentsToQualify
+: public tooling::RecursiveSymbolVisitor {
+public:
+  FindIdentsToQualify(ASTContext &Ctx, NamespaceDecl &TargetNS,
+  std::vector &ResultIdents)
+  : RecursiveSymbolVisitor(Ctx.getSourceManager(), Ctx.getLangOpts()),
+Ctx(Ctx), TargetNS(TargetNS), ResultIdents(ResultIdents) {}
+
+  bool visitSymbolOccurrence(const NamedDecl *D,
+ llvm::ArrayRef NameRanges) {
+if (!D || D->getCanonicalDecl() == TargetNS.getCanonicalDecl())
+  return true;
+if (!D->getDeclName().isIdentifier() ||
+D->getDeclName().getNameKind() == DeclarationName::CXXOperatorName)
+  return true; // do not add qualifiers for non-idents, e.g. 'operator+'.
+// Check the symbol is unqualified and references something inside our
+// namespace.
+// FIXME: add a check it's unqualified.
+if (!TargetNS.InEnclosingNamespaceSetOf(D->getDeclContext()))
+  return true;
+// FIXME: handle more tricky cases, e.g. we don't need the qualifier if we
+//have the using decls for some entities, we might have qualified
+//references that need updating too.
+for (auto R : NameRanges) {
+  if (!Ctx.getSourceManager().isWrittenInMainFile(R.getBegin()))
+continue; // we can't fix refences outside our file.
+  // FIXME: this might be a conflict that we need to report.
+  ResultIdents.push_back(R.getBegin());
+}
+return true;
+  }
+
+  bool TraverseDecl(Decl *D) {
+if (!Ctx.getSourceManager().isWrittenInMainFile(D->getLocation()) &&
+!isa(D))
+  return true; // skip decls outside main file.
+return RecursiveSymbolVisitor::TraverseDecl(D);
+  }
+
+private:
+  ASTContext &Ctx;
+  NamespaceDecl &TargetNS;
+  std::vector &ResultIdents;
+};
+
+// Produces an edit to remove 'using namespace xxx::yyy' and the trailing
+// semicolon.
+llvm::Expected
+removeUsingDirective(ASTContext &Ctx, UsingDirectiveDecl *D) {
+  auto &SrcMgr = Ctx.getSourceManager();
+  auto R =
+  Lexer::get

[PATCH] D56611: [clangd] A code action to swap branches of an if statement

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 182321.
ilya-biryukov added a comment.

- Remove the header file


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56611

Files:
  clangd/refactor/tweaks/CMakeLists.txt
  clangd/refactor/tweaks/SwapIfBranches.cpp

Index: clangd/refactor/tweaks/SwapIfBranches.cpp
===
--- /dev/null
+++ clangd/refactor/tweaks/SwapIfBranches.cpp
@@ -0,0 +1,119 @@
+//===--- SwapIfBranches.cpp --*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+#include "ClangdUnit.h"
+#include "Logger.h"
+#include "SourceCode.h"
+#include "refactor/Tweak.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Stmt.h"
+#include "clang/Basic/LangOptions.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Lex/Lexer.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+/// Swaps the 'then' and 'else' branch of the if statement.
+/// Before:
+///   if (foo) { return 10; } else { continue; }
+/// After:
+///   if (foo) { continue; } else { return 10; }
+class SwapIfBranches : public Tweak {
+public:
+  TweakID id() const override {
+return llvm::StringLiteral("swap-if-branches");
+  }
+
+  bool prepare(const Selection &Inputs) override;
+  Expected apply(const Selection &Inputs) override;
+  std::string title() const override;
+
+private:
+  tooling::Replacements Result;
+};
+REGISTER_TWEAK(SwapIfBranches);
+
+class FindIfUnderCursor : public RecursiveASTVisitor {
+public:
+  FindIfUnderCursor(ASTContext &Ctx, SourceLocation CursorLoc, IfStmt *&Result)
+  : Ctx(Ctx), CursorLoc(CursorLoc), Result(Result) {}
+
+  bool VisitIfStmt(IfStmt *If) {
+auto R = toHalfOpenFileRange(Ctx.getSourceManager(), Ctx.getLangOpts(),
+ SourceRange(If->getIfLoc()));
+if (!R)
+  return true;
+if (!halfOpenRangeContains(Ctx.getSourceManager(), *R, CursorLoc))
+  return true;
+Result = If;
+return false;
+  }
+
+private:
+  ASTContext &Ctx;
+  SourceLocation CursorLoc;
+  IfStmt *&Result;
+};
+} // namespace
+
+bool SwapIfBranches::prepare(const Selection &Inputs) {
+
+  auto &Ctx = Inputs.AST.getASTContext();
+  auto &SrcMgr = Ctx.getSourceManager();
+  IfStmt *If = nullptr;
+  FindIfUnderCursor(Ctx, Inputs.Cursor, If).TraverseAST(Ctx);
+  if (!If)
+return false;
+
+  // avoid dealing with single-statement brances, they require careful handling
+  // to avoid changing semantics of the code (i.e. dangling else).
+  if (!llvm::dyn_cast_or_null(If->getThen()) ||
+  !llvm::dyn_cast_or_null(If->getElse()))
+return false;
+
+  auto ThenRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getThen()->getSourceRange());
+  if (!ThenRng)
+return false;
+  auto ElseRng = toHalfOpenFileRange(SrcMgr, Ctx.getLangOpts(),
+ If->getElse()->getSourceRange());
+  if (!ElseRng)
+return false;
+
+  llvm::StringRef ThenCode = toSourceCode(SrcMgr, *ThenRng);
+  llvm::StringRef ElseCode = toSourceCode(SrcMgr, *ElseRng);
+
+  if (auto Err = Result.add(tooling::Replacement(SrcMgr, ThenRng->getBegin(),
+ ThenCode.size(), ElseCode))) {
+llvm::consumeError(std::move(Err));
+return false;
+  }
+  if (auto Err = Result.add(tooling::Replacement(SrcMgr, ElseRng->getBegin(),
+ ElseCode.size(), ThenCode))) {
+llvm::consumeError(std::move(Err));
+return false;
+  }
+  return true;
+}
+
+Expected SwapIfBranches::apply(const Selection &Inputs) {
+  return Result;
+}
+
+std::string SwapIfBranches::title() const { return "Swap if branches"; }
+} // namespace clangd
+} // namespace clang
Index: clangd/refactor/tweaks/CMakeLists.txt
===
--- clangd/refactor/tweaks/CMakeLists.txt
+++ clangd/refactor/tweaks/CMakeLists.txt
@@ -9,4 +9,5 @@
 # clangd/tool/CMakeLists.txt for an example.
 add_clang_library(clangDaemonTweaks OBJECT
   QualifyName.cpp
+  SwapIfBranches.cpp
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdLSPServer.cpp:346
+  {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
+   ExecuteCommandParams::CLANGD_APPLY_CODE_ACTION}},
  }},

sammccall wrote:
> Seems a bit more direct to give each one its own command name? Maybe under a 
> namespace like refactor/swapIfBranches
I'm not sure what we're winning here.

Currently we have: `command:{ name: "applyCodeAction", args: 
["swap-if-branches", "…"] }`
In the proposed approach we'd have: `command: { name: "swap-if-branches", args: 
[…] }`

The latter would definitely mean more fiddling at least to communicate the list 
of supported actions. Both approaches don't seem to require any changes on the 
client side, even though they affect the messages we send over the protocol.



Comment at: clangd/ClangdLSPServer.cpp:662
+std::vector Actions = std::move(FixIts);
+auto toCodeAction = [&](PreparedAction &&A) -> CodeAction {
+  CodeAction CA;

sammccall wrote:
> This seems like it could just be a helper function... what does it capture?
> I think it might belong next to PreparedActions just like we have 
> `toLSPDiags` in `Diag.h` - it's not ideal dependency-wise, but inlining 
> everything into ClangdLSPServer seems dubious too. Happy to discuss offline...
I've moved it into a helper function, moved away from using a lambda.
However, kept it in `ClangdLSPServer.cpp`, `Diag.h` does feel like the wrong 
place for it, it would be nice to find a better place for both functions.



Comment at: clangd/refactor/Tweak.cpp:62
+  }
+  // Ensure deterministic order of the results.
+  llvm::sort(Available,

sammccall wrote:
> (nit: I'd make this `operator<` of `Tweak`? e.g. if priority is added, it 
> should be private)
My personal preference would be to keep it in .cpp file to avoid complicating 
the public interface with a function and a friend declaration for 
`prepareTweaks` (let me know if I misinterpreted your proposal).
Let me know if you have strong preference wrt to this one



Comment at: clangd/refactor/Tweak.h:78
+/// Contains all actions supported by clangd.
+typedef llvm::Registry TweakRegistry;
+

sammccall wrote:
> nit: can we move this to the cpp file and out of the public interface?
> (or keep the registry public, but drop prepareTweak[s] and just make callers 
> do it)
Done.
I don't think that makes a big difference, though: hiding the typedef would 
hide the fact we're using the `Registry`, though. We still have 
`REGISTER_TWEAK` that mentions it in the public interface.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267



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

[PATCH] D56856: [tooling] Add a new argument adjuster for deleting plugin related command line args

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM after the nits are fixed


Repository:
  rC Clang

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

https://reviews.llvm.org/D56856



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

[PATCH] D53928: Enable builtins necessary for SLEEF [AArch64] vectorized trigonometry libm functions

[Hal Finkel via Phabricator via cfe-commits]

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

LGTM

Please, in the future, make sure you post full-context patches.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53928



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

[PATCH] D56267: [clangd] Interfaces for writing code actions

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

This should be ready for another round of review. Let me know if I missed any 
of the older comments.
There are two more things that I'd like to do before this lands:

- go through the docs again and update/simplify them to make sure they're in 
sync with the new interface,
- publish the tests I have, they're not 100% ready yet, so I'm keeping the, 
local for now. This could be a separate change, though I don't think this 
should matter much if the rest is dealt with.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D56267



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

[PATCH] D53153: [OpenCL] Mark kernel functions with default visibility

[Scott Linder via Phabricator via cfe-commits]

scott.linder added a comment.

I think we need the option to be in the driver, not just -cc1. We are moving 
towards the clang driver being the authority on how we do offline compilation 
for AMDGPU, and hiding this in cc1 doesn't help us.

I also don't think this is unreasonable to expose in general. As you note the 
behavior of `-fvisibility` wrt. declarations is just a practical measure, and 
even in the world of e.g. x86 it seems like there are cases where the option is 
useful.


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

https://reviews.llvm.org/D53153



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

r351457 - TLS: Respect visibility for thread_local variables on Darwin (PR40327)

[Vlad Tsyrklevich via cfe-commits]

Author: vlad.tsyrklevich
Date: Thu Jan 17 09:53:45 2019
New Revision: 351457

URL: http://llvm.org/viewvc/llvm-project?rev=351457&view=rev
Log:
TLS: Respect visibility for thread_local variables on Darwin (PR40327)

Summary:
Teach clang to mark thread wrappers for thread_local variables with
hidden visibility when the original variable is marked with hidden
visibility. This is necessary on Darwin which exposes the thread wrapper
instead of the thread variable. The thread wrapper would previously
always be created with default visibility unless it had
linkonce*/weak_odr linkage.

Reviewers: rjmccall

Reviewed By: rjmccall

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

Added:
cfe/trunk/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
Modified:
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp

Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=351457&r1=351456&r2=351457&view=diff
==
--- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
+++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Thu Jan 17 09:53:45 2019
@@ -2463,10 +2463,12 @@ ItaniumCXXABI::getOrCreateThreadLocalWra
 CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Wrapper);
 
   // Always resolve references to the wrapper at link time.
-  if (!Wrapper->hasLocalLinkage() && !(isThreadWrapperReplaceable(VD, CGM) &&
-  !llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) &&
-  !llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage(
-Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
+  if (!Wrapper->hasLocalLinkage())
+if (!isThreadWrapperReplaceable(VD, CGM) ||
+llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) ||
+llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage()) ||
+VD->getVisibility() == HiddenVisibility)
+  Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
 
   if (isThreadWrapperReplaceable(VD, CGM)) {
 Wrapper->setCallingConv(llvm::CallingConv::CXX_FAST_TLS);

Added: cfe/trunk/test/CodeGenCXX/cxx11-thread-local-visibility.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-thread-local-visibility.cpp?rev=351457&view=auto
==
--- cfe/trunk/test/CodeGenCXX/cxx11-thread-local-visibility.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/cxx11-thread-local-visibility.cpp Thu Jan 17 
09:53:45 2019
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | 
FileCheck --check-prefix=LINUX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 
| FileCheck --check-prefix=DARWIN %s
+
+// Regression test for PR40327
+
+// LINUX: @default_tls = thread_local global i32
+// LINUX: @hidden_tls = hidden thread_local global i32
+// LINUX: define weak_odr hidden i32* @_ZTW11default_tls()
+// LINUX: define weak_odr hidden i32* @_ZTW10hidden_tls()
+//
+// DARWIN: @default_tls = internal thread_local global i32
+// DARWIN: @hidden_tls = internal thread_local global i32
+// DARWIN: define cxx_fast_tlscc i32* @_ZTW11default_tls()
+// DARWIN: define hidden cxx_fast_tlscc i32* @_ZTW10hidden_tls()
+
+__attribute__((visibility("default"))) thread_local int default_tls;
+__attribute__((visibility("hidden"))) thread_local int hidden_tls;

Modified: cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp?rev=351457&r1=351456&r2=351457&view=diff
==
--- cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx11-thread-local.cpp Thu Jan 17 09:53:45 2019
@@ -318,7 +318,7 @@ void set_anon_i() {
 // CHECK-NOT: call void @[[V_M_INIT]]()
 
 
-// LIUNX: define weak_odr hidden i32* @_ZTW1a() {
+// LINUX: define weak_odr hidden i32* @_ZTW1a()
 // DARWIN: define cxx_fast_tlscc i32* @_ZTW1a()
 // LINUX:   call void @_ZTH1a()
 // DARWIN: call cxx_fast_tlscc void @_ZTH1a()


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

[PATCH] D56818: TLS: Respect visibility for thread_local variables on Darwin (PR40327)

[Vlad Tsyrklevich via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC351457: TLS: Respect visibility for thread_local variables 
on Darwin (PR40327) (authored by vlad.tsyrklevich, committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56818?vs=182171&id=182327#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56818

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/cxx11-thread-local-visibility.cpp
  test/CodeGenCXX/cxx11-thread-local.cpp


Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2463,10 +2463,12 @@
 CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Wrapper);
 
   // Always resolve references to the wrapper at link time.
-  if (!Wrapper->hasLocalLinkage() && !(isThreadWrapperReplaceable(VD, CGM) &&
-  !llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) &&
-  !llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage(
-Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
+  if (!Wrapper->hasLocalLinkage())
+if (!isThreadWrapperReplaceable(VD, CGM) ||
+llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) ||
+llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage()) ||
+VD->getVisibility() == HiddenVisibility)
+  Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
 
   if (isThreadWrapperReplaceable(VD, CGM)) {
 Wrapper->setCallingConv(llvm::CallingConv::CXX_FAST_TLS);
Index: test/CodeGenCXX/cxx11-thread-local.cpp
===
--- test/CodeGenCXX/cxx11-thread-local.cpp
+++ test/CodeGenCXX/cxx11-thread-local.cpp
@@ -318,7 +318,7 @@
 // CHECK-NOT: call void @[[V_M_INIT]]()
 
 
-// LIUNX: define weak_odr hidden i32* @_ZTW1a() {
+// LINUX: define weak_odr hidden i32* @_ZTW1a()
 // DARWIN: define cxx_fast_tlscc i32* @_ZTW1a()
 // LINUX:   call void @_ZTH1a()
 // DARWIN: call cxx_fast_tlscc void @_ZTH1a()
Index: test/CodeGenCXX/cxx11-thread-local-visibility.cpp
===
--- test/CodeGenCXX/cxx11-thread-local-visibility.cpp
+++ test/CodeGenCXX/cxx11-thread-local-visibility.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-linux-gnu | 
FileCheck --check-prefix=LINUX %s
+// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -triple x86_64-apple-darwin12 
| FileCheck --check-prefix=DARWIN %s
+
+// Regression test for PR40327
+
+// LINUX: @default_tls = thread_local global i32
+// LINUX: @hidden_tls = hidden thread_local global i32
+// LINUX: define weak_odr hidden i32* @_ZTW11default_tls()
+// LINUX: define weak_odr hidden i32* @_ZTW10hidden_tls()
+//
+// DARWIN: @default_tls = internal thread_local global i32
+// DARWIN: @hidden_tls = internal thread_local global i32
+// DARWIN: define cxx_fast_tlscc i32* @_ZTW11default_tls()
+// DARWIN: define hidden cxx_fast_tlscc i32* @_ZTW10hidden_tls()
+
+__attribute__((visibility("default"))) thread_local int default_tls;
+__attribute__((visibility("hidden"))) thread_local int hidden_tls;


Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2463,10 +2463,12 @@
 CGM.SetLLVMFunctionAttributesForDefinition(nullptr, Wrapper);
 
   // Always resolve references to the wrapper at link time.
-  if (!Wrapper->hasLocalLinkage() && !(isThreadWrapperReplaceable(VD, CGM) &&
-  !llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) &&
-  !llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage(
-Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
+  if (!Wrapper->hasLocalLinkage())
+if (!isThreadWrapperReplaceable(VD, CGM) ||
+llvm::GlobalVariable::isLinkOnceLinkage(Wrapper->getLinkage()) ||
+llvm::GlobalVariable::isWeakODRLinkage(Wrapper->getLinkage()) ||
+VD->getVisibility() == HiddenVisibility)
+  Wrapper->setVisibility(llvm::GlobalValue::HiddenVisibility);
 
   if (isThreadWrapperReplaceable(VD, CGM)) {
 Wrapper->setCallingConv(llvm::CallingConv::CXX_FAST_TLS);
Index: test/CodeGenCXX/cxx11-thread-local.cpp
===
--- test/CodeGenCXX/cxx11-thread-local.cpp
+++ test/CodeGenCXX/cxx11-thread-local.cpp
@@ -318,7 +318,7 @@
 // CHECK-NOT: call void @[[V_M_INIT]]()
 
 
-// LIUNX: define weak_odr hidden i32* @_ZTW1a() {
+// LINUX: define weak_odr hidden i32* @_ZTW1a()
 // DARWIN: define cxx_fast_tlscc i32* @_ZTW1a()
 // LINUX:   call void @_ZTH1a()
 // DARWIN: call cxx_fast_tlscc void @_ZTH1a()
Index: test/CodeGenCXX/cxx11-thread-local-visibility.cpp
==

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

Hi @vit9696 ,

This looks to be caused by using 128-bit long double on the platform.  Does 
linux really use 128-bit long doubles on ppc32?  FreeBSD uses 64-bit long 
double, so compiling that with '-target powerpc-gnu-freebsd' works fine.  I'm 
not sure how to handle the 128-bit values.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

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

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

Szelethus accepted this revision.
Szelethus added a comment.

Overall I think this looks great, thanks! I left some inlines that would be 
nice to fix before commiting, but all of them are minor nits.

Would it be possible for you to commit the clang-formatting and the actual 
logic separately?




Comment at: include/clang/StaticAnalyzer/Checkers/Checkers.td:393
+  def DeprecatedOrUnsafeBufferHandling : 
Checker<"DeprecatedOrUnsafeBufferHandling">,
+HelpText<"Warn on uses of unsecure or deprecated buffer manipulating 
functions">,
+DescFile<"CheckSecuritySyntaxOnly.cpp">;

Lately we started paying attention to the 80 column limit in `Checkers.td`. 
Could you please break this line?



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:583
+// Use of 'sprintf', 'vsprintf', 'scanf', 'wscanf',
+//'fscanf',
+//'fwscanf', 'vscanf', 'vwscanf', 'vfscanf', 'vfwscanf', 'sscanf',

This is out of place.



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:591
+//===--===//
+void WalkAST::checkDeprecatedOrUnsafeBufferHandling(const CallExpr *CE,
+const FunctionDecl *FD) {

Could you please add an extra newline?



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:598
 
//===--===//
-
 void WalkAST::checkCall_rand(const CallExpr *CE, const FunctionDecl *FD) {

I think this newline should stay.



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:603-610
+  int ArgIndex =
+  llvm::StringSwitch(Name)
+  .Cases("scanf", "wscanf", "vscanf", "vwscanf", 0)
+  .Cases("sprintf", "vsprintf", "fscanf", "fwscanf", "vfscanf",
+ "vfwscanf", "sscanf", "swscanf", "vsscanf", "vswscanf", 1)
+  .Cases("swprintf", "snprintf", "vswprintf", "vsnprintf", "memcpy",
+ "memmove", "memset", "strncpy", "strncat", DEPR_ONLY)

I wonder whether using `CallDescription` would be (way) more efficient here. 
Comparing `IndentifierInfo`s would also be better I guess.

I'm a little unsure about performance implications here, it might not be worth 
the chore to refactor this.



Comment at: lib/StaticAnalyzer/Checkers/CheckSecuritySyntaxOnly.cpp:627-630
+  SmallString<128> buf1;
+  SmallString<512> buf2;
+  llvm::raw_svector_ostream out1(buf1);
+  llvm::raw_svector_ostream out2(buf2);

Could you please start these variable names with a capital letter?



Comment at: test/Analysis/security-syntax-checks.m:253
+  FILE *file;
+  sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is 
insecure as it does not provide security checks introduced in the C11 standard. 
Replace with analogous functions that support length arguments or provides 
boundary checks such as 'sprintf_s' in case of C11}}
+  scanf("%d", &a); // expected-warning{{Call to function 'scanf' is insecure 
as it does not provide security checks introduced in the C11 standard. Replace 
with analogous functions that support length arguments or provides boundary 
checks such as 'scanf_s' in case of C11}}

When using `{{}}`, you actually supply a regex as an argument, and the output 
of the analyzer is matched against it. My point is, could you instead just write
```
// expected-warning{{Call to function 'sprintf' is insecure}}
```
to improve readability?


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

https://reviews.llvm.org/D35068



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

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

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

Szelethus added inline comments.



Comment at: test/Analysis/security-syntax-checks.m:253
+  FILE *file;
+  sprintf(buf, "a"); // expected-warning{{Call to function 'sprintf' is 
insecure as it does not provide security checks introduced in the C11 standard. 
Replace with analogous functions that support length arguments or provides 
boundary checks such as 'sprintf_s' in case of C11}}
+  scanf("%d", &a); // expected-warning{{Call to function 'scanf' is insecure 
as it does not provide security checks introduced in the C11 standard. Replace 
with analogous functions that support length arguments or provides boundary 
checks such as 'scanf_s' in case of C11}}

Szelethus wrote:
> When using `{{}}`, you actually supply a regex as an argument, and the output 
> of the analyzer is matched against it. My point is, could you instead just 
> write
> ```
> // expected-warning{{Call to function 'sprintf' is insecure}}
> ```
> to improve readability?
Or whatever the shortest string is needed to know whether the expected output 
it there.


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

https://reviews.llvm.org/D35068



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

Actually I am not sure about Linux, since this is bare metal, and I just used 
what fited. However, it does not look like 128-bit or 64-bit long doubles are 
related.
I retried to compile the test case with powerpc-gnu-freebsd target (and even 
made a compile-time assertion to ensure that long double is now 64-bit), yet it 
still crashed in a similar way.
Might it be another LLVM 7 vs LLVM 8 difference? Does it crash for you with 
Linux target?


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[Justin Hibbits via Phabricator via cfe-commits]

jhibbits added a comment.

Hi @vit9696 it does crash with the linux target (powerpc-gnu-linux), but is 
fine with powerpc-gnu-freebsd.


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[PATCH] D56816: [ObjC] Follow-up r350768 and allow the use of unavailable methods that are declared in a parent class from within the @implementation context

[Alex Lorenz via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC351459: [ObjC] Follow-up r350768 and allow the use of 
unavailable methods that are (authored by arphaman, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56816?vs=182191&id=182334#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56816

Files:
  lib/Sema/SemaDeclAttr.cpp
  test/SemaObjC/call-unavailable-init-in-self.m
  test/SemaObjC/infer-availability-from-init.m


Index: test/SemaObjC/infer-availability-from-init.m
===
--- test/SemaObjC/infer-availability-from-init.m
+++ test/SemaObjC/infer-availability-from-init.m
@@ -47,12 +47,12 @@
 }
 
 @interface FromSelf : NSObject
--(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+-(instancetype)init __attribute__((unavailable));
 +(FromSelf*)another_one;
 @end
 
 @implementation FromSelf
 +(FromSelf*)another_one {
-  [self new]; // expected-error{{'new' is unavailable}}
+  [self new];
 }
 @end
Index: test/SemaObjC/call-unavailable-init-in-self.m
===
--- test/SemaObjC/call-unavailable-init-in-self.m
+++ test/SemaObjC/call-unavailable-init-in-self.m
@@ -5,13 +5,24 @@
 + (instancetype)new;
 + (instancetype)alloc;
 
+- (void)declaredInSuper;
+
+@end
+
+@interface NSObject (Category)
+
+- (void)declaredInSuperCategory;
+
 @end
 
 @interface Sub: NSObject
 
 - (instancetype)init __attribute__((unavailable)); // expected-note 4 {{'init' 
has been explicitly marked unavailable here}}
 
-- (void)notImplemented __attribute__((unavailable)); // expected-note 
{{'notImplemented' has been explicitly marked unavailable here}}
+- (void)notImplemented __attribute__((unavailable));
+
+- (void)declaredInSuper __attribute__((unavailable));
+- (void)declaredInSuperCategory __attribute__((unavailable));
 
 @end
 
@@ -34,7 +45,14 @@
 }
 
 - (void)reportUseOfUnimplemented {
-  [self notImplemented]; // expected-error {{'notImplemented' is unavailable}}
+  [self notImplemented];
+}
+
+- (void)allowSuperCallUsingSelf {
+  [self declaredInSuper];
+  [[Sub alloc] declaredInSuper];
+  [self declaredInSuperCategory];
+  [[Sub alloc] declaredInSuperCategory];
 }
 
 @end
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -7365,13 +7365,11 @@
 return true;
 } else if (K == AR_Unavailable) {
   // It is perfectly fine to refer to an 'unavailable' Objective-C method
-  // when it's actually defined and is referenced from within the
-  // @implementation itself. In this context, we interpret unavailable as a
-  // form of access control.
+  // when it is referenced from within the @implementation itself. In this
+  // context, we interpret unavailable as a form of access control.
   if (const auto *MD = dyn_cast(OffendingDecl)) {
 if (const auto *Impl = dyn_cast(C)) {
-  if (MD->getClassInterface() == Impl->getClassInterface() &&
-  MD->isDefined())
+  if (MD->getClassInterface() == Impl->getClassInterface())
 return true;
 }
   }


Index: test/SemaObjC/infer-availability-from-init.m
===
--- test/SemaObjC/infer-availability-from-init.m
+++ test/SemaObjC/infer-availability-from-init.m
@@ -47,12 +47,12 @@
 }
 
 @interface FromSelf : NSObject
--(instancetype)init __attribute__((unavailable)); // expected-note {{'init' has been explicitly marked unavailable here}}
+-(instancetype)init __attribute__((unavailable));
 +(FromSelf*)another_one;
 @end
 
 @implementation FromSelf
 +(FromSelf*)another_one {
-  [self new]; // expected-error{{'new' is unavailable}}
+  [self new];
 }
 @end
Index: test/SemaObjC/call-unavailable-init-in-self.m
===
--- test/SemaObjC/call-unavailable-init-in-self.m
+++ test/SemaObjC/call-unavailable-init-in-self.m
@@ -5,13 +5,24 @@
 + (instancetype)new;
 + (instancetype)alloc;
 
+- (void)declaredInSuper;
+
+@end
+
+@interface NSObject (Category)
+
+- (void)declaredInSuperCategory;
+
 @end
 
 @interface Sub: NSObject
 
 - (instancetype)init __attribute__((unavailable)); // expected-note 4 {{'init' has been explicitly marked unavailable here}}
 
-- (void)notImplemented __attribute__((unavailable)); // expected-note {{'notImplemented' has been explicitly marked unavailable here}}
+- (void)notImplemented __attribute__((unavailable));
+
+- (void)declaredInSuper __attribute__((unavailable));
+- (void)declaredInSuperCategory __attribute__((unavailable));
 
 @end
 
@@ -34,7 +45,14 @@
 }
 
 - (void)reportUseOfUnimplemented {
-  [self notImplemented]; // expected-error {{'notI

r351459 - [ObjC] Follow-up r350768 and allow the use of unavailable methods that are

[Alex Lorenz via cfe-commits]

Author: arphaman
Date: Thu Jan 17 10:12:45 2019
New Revision: 351459

URL: http://llvm.org/viewvc/llvm-project?rev=351459&view=rev
Log:
[ObjC] Follow-up r350768 and allow the use of unavailable methods that are
declared in a parent class from within the @implementation context

This commit extends r350768 and allows the use of methods marked as unavailable
that are declared in a parent class/category from within the @implementation of
the class where the method is marked as unavailable.
This allows users to call init that's marked as unavailable even if they don't
define it.

rdar://47134898

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

Modified:
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
cfe/trunk/test/SemaObjC/infer-availability-from-init.m

Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=351459&r1=351458&r2=351459&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Jan 17 10:12:45 2019
@@ -7365,13 +7365,11 @@ ShouldDiagnoseAvailabilityInContext(Sema
 return true;
 } else if (K == AR_Unavailable) {
   // It is perfectly fine to refer to an 'unavailable' Objective-C method
-  // when it's actually defined and is referenced from within the
-  // @implementation itself. In this context, we interpret unavailable as a
-  // form of access control.
+  // when it is referenced from within the @implementation itself. In this
+  // context, we interpret unavailable as a form of access control.
   if (const auto *MD = dyn_cast(OffendingDecl)) {
 if (const auto *Impl = dyn_cast(C)) {
-  if (MD->getClassInterface() == Impl->getClassInterface() &&
-  MD->isDefined())
+  if (MD->getClassInterface() == Impl->getClassInterface())
 return true;
 }
   }

Modified: cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m?rev=351459&r1=351458&r2=351459&view=diff
==
--- cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m (original)
+++ cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m Thu Jan 17 10:12:45 
2019
@@ -5,13 +5,24 @@
 + (instancetype)new;
 + (instancetype)alloc;
 
+- (void)declaredInSuper;
+
+@end
+
+@interface NSObject (Category)
+
+- (void)declaredInSuperCategory;
+
 @end
 
 @interface Sub: NSObject
 
 - (instancetype)init __attribute__((unavailable)); // expected-note 4 {{'init' 
has been explicitly marked unavailable here}}
 
-- (void)notImplemented __attribute__((unavailable)); // expected-note 
{{'notImplemented' has been explicitly marked unavailable here}}
+- (void)notImplemented __attribute__((unavailable));
+
+- (void)declaredInSuper __attribute__((unavailable));
+- (void)declaredInSuperCategory __attribute__((unavailable));
 
 @end
 
@@ -34,7 +45,14 @@
 }
 
 - (void)reportUseOfUnimplemented {
-  [self notImplemented]; // expected-error {{'notImplemented' is unavailable}}
+  [self notImplemented];
+}
+
+- (void)allowSuperCallUsingSelf {
+  [self declaredInSuper];
+  [[Sub alloc] declaredInSuper];
+  [self declaredInSuperCategory];
+  [[Sub alloc] declaredInSuperCategory];
 }
 
 @end

Modified: cfe/trunk/test/SemaObjC/infer-availability-from-init.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/infer-availability-from-init.m?rev=351459&r1=351458&r2=351459&view=diff
==
--- cfe/trunk/test/SemaObjC/infer-availability-from-init.m (original)
+++ cfe/trunk/test/SemaObjC/infer-availability-from-init.m Thu Jan 17 10:12:45 
2019
@@ -47,12 +47,12 @@ void usenotmyobject() {
 }
 
 @interface FromSelf : NSObject
--(instancetype)init __attribute__((unavailable)); // expected-note {{'init' 
has been explicitly marked unavailable here}}
+-(instancetype)init __attribute__((unavailable));
 +(FromSelf*)another_one;
 @end
 
 @implementation FromSelf
 +(FromSelf*)another_one {
-  [self new]; // expected-error{{'new' is unavailable}}
+  [self new];
 }
 @end


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

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

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

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

In D35068#811436 , @NoQ wrote:

> I wonder how noisy this check is - did you test it on large codebases? 
> Because these functions are popular, and in many cases it'd be fine to use 
> insecure functions, i wonder if it's worth it to have this check on by 
> default. Like, if it's relatively quiet - it's fine, but if it'd constitute 
> 90% of the analyzer's warnings on popular projects, that'd probably not be 
> fine.




In D35068#1049530 , @george.karpenkov 
wrote:

> @koldaniel Have you evaluated this checker? On which codebases? Were the 
> warnings real security issues, or were they mostly spurious? The code seems 
> fine, but I'm not sure whether it should be in `security` or in `alpha`.


Sorry, didn't read the discussion, there are some fair points in the quoted 
comments.

In D35068#1069880 , @koldaniel wrote:

> I've evaluated this checker on LLVM+Clang, there were only a few (about 15) 
> warnings,  because of the C11 flag check at the beginning of the checker 
> body. However, if this check was removed, number of the warnings would be 
> increased significantly. I wouldn't say the findings were real security 
> issues, most of the warnings were about usages of deprecated functions, which 
> has not been considered unsecure (but which may cause problems if the code is 
> modified in an improper way in the future).


My problem is that LLVM+Clang isn't really a C (neither a C11) project, and I 
think judging this checker on it is a little misleading. Could you please test 
it on some C11 projects? I think tmux uses C11.

In D35068#1361195 , @xazax.hun wrote:

> I think this is quiet coding guideline specific check which is useful for a 
> set of  security critical projects. As this is an opt in kind of check, I 
> think it does no harm to have it upstream.


I do generally agree with this statement, but I'd be more comfortable either 
landing it in alpha or seeing some other results.


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

https://reviews.llvm.org/D35068



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

Re: r351459 - [ObjC] Follow-up r350768 and allow the use of unavailable methods that are

[Alex L via cfe-commits]

Hi Hans,

Could you please cherry-pick this change into the release branch?

Cheers,
Alex

On Thu, 17 Jan 2019 at 10:16, Alex Lorenz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: arphaman
> Date: Thu Jan 17 10:12:45 2019
> New Revision: 351459
>
> URL: http://llvm.org/viewvc/llvm-project?rev=351459&view=rev
> Log:
> [ObjC] Follow-up r350768 and allow the use of unavailable methods that are
> declared in a parent class from within the @implementation context
>
> This commit extends r350768 and allows the use of methods marked as
> unavailable
> that are declared in a parent class/category from within the
> @implementation of
> the class where the method is marked as unavailable.
> This allows users to call init that's marked as unavailable even if they
> don't
> define it.
>
> rdar://47134898
>
> Differential Revision: https://reviews.llvm.org/D56816
>
> Modified:
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
> cfe/trunk/test/SemaObjC/infer-availability-from-init.m
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=351459&r1=351458&r2=351459&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Jan 17 10:12:45 2019
> @@ -7365,13 +7365,11 @@ ShouldDiagnoseAvailabilityInContext(Sema
>  return true;
>  } else if (K == AR_Unavailable) {
>// It is perfectly fine to refer to an 'unavailable' Objective-C
> method
> -  // when it's actually defined and is referenced from within the
> -  // @implementation itself. In this context, we interpret
> unavailable as a
> -  // form of access control.
> +  // when it is referenced from within the @implementation itself. In
> this
> +  // context, we interpret unavailable as a form of access control.
>if (const auto *MD = dyn_cast(OffendingDecl)) {
>  if (const auto *Impl = dyn_cast(C)) {
> -  if (MD->getClassInterface() == Impl->getClassInterface() &&
> -  MD->isDefined())
> +  if (MD->getClassInterface() == Impl->getClassInterface())
>  return true;
>  }
>}
>
> Modified: cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m?rev=351459&r1=351458&r2=351459&view=diff
>
> ==
> --- cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m (original)
> +++ cfe/trunk/test/SemaObjC/call-unavailable-init-in-self.m Thu Jan 17
> 10:12:45 2019
> @@ -5,13 +5,24 @@
>  + (instancetype)new;
>  + (instancetype)alloc;
>
> +- (void)declaredInSuper;
> +
> +@end
> +
> +@interface NSObject (Category)
> +
> +- (void)declaredInSuperCategory;
> +
>  @end
>
>  @interface Sub: NSObject
>
>  - (instancetype)init __attribute__((unavailable)); // expected-note 4
> {{'init' has been explicitly marked unavailable here}}
>
> -- (void)notImplemented __attribute__((unavailable)); // expected-note
> {{'notImplemented' has been explicitly marked unavailable here}}
> +- (void)notImplemented __attribute__((unavailable));
> +
> +- (void)declaredInSuper __attribute__((unavailable));
> +- (void)declaredInSuperCategory __attribute__((unavailable));
>
>  @end
>
> @@ -34,7 +45,14 @@
>  }
>
>  - (void)reportUseOfUnimplemented {
> -  [self notImplemented]; // expected-error {{'notImplemented' is
> unavailable}}
> +  [self notImplemented];
> +}
> +
> +- (void)allowSuperCallUsingSelf {
> +  [self declaredInSuper];
> +  [[Sub alloc] declaredInSuper];
> +  [self declaredInSuperCategory];
> +  [[Sub alloc] declaredInSuperCategory];
>  }
>
>  @end
>
> Modified: cfe/trunk/test/SemaObjC/infer-availability-from-init.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/infer-availability-from-init.m?rev=351459&r1=351458&r2=351459&view=diff
>
> ==
> --- cfe/trunk/test/SemaObjC/infer-availability-from-init.m (original)
> +++ cfe/trunk/test/SemaObjC/infer-availability-from-init.m Thu Jan 17
> 10:12:45 2019
> @@ -47,12 +47,12 @@ void usenotmyobject() {
>  }
>
>  @interface FromSelf : NSObject
> --(instancetype)init __attribute__((unavailable)); // expected-note
> {{'init' has been explicitly marked unavailable here}}
> +-(instancetype)init __attribute__((unavailable));
>  +(FromSelf*)another_one;
>  @end
>
>  @implementation FromSelf
>  +(FromSelf*)another_one {
> -  [self new]; // expected-error{{'new' is unavailable}}
> +  [self new];
>  }
>  @end
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
__

r351461 - [CodeGenObjC] Use a constant value for non-fragile ivar offsets when possible

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Jan 17 10:18:53 2019
New Revision: 351461

URL: http://llvm.org/viewvc/llvm-project?rev=351461&view=rev
Log:
[CodeGenObjC] Use a constant value for non-fragile ivar offsets when possible

If a class inherits from NSObject and has an implementation, then we
can assume that ivar offsets won't need to be updated by the runtime.
This allows us to index into the object using a constant value and
avoid loading from the ivar offset variable.

This patch was adapted from one written by Pete Cooper.

rdar://problem/10132568

Differential revision: https://reviews.llvm.org/D56802

Added:
cfe/trunk/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
Modified:
cfe/trunk/lib/CodeGen/CGObjCMac.cpp
cfe/trunk/test/CodeGenObjC/optimize-ivar-offset-load.m
cfe/trunk/test/CodeGenObjC/reorder-synthesized-ivars.m

Modified: cfe/trunk/lib/CodeGen/CGObjCMac.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjCMac.cpp?rev=351461&r1=351460&r2=351461&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjCMac.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjCMac.cpp Thu Jan 17 10:18:53 2019
@@ -1550,6 +1550,15 @@ private:
 return false;
   }
 
+  bool isClassLayoutKnownStatically(const ObjCInterfaceDecl *ID) {
+// NSObject is a fixed size. If we can see the @implementation of a class
+// which inherits from NSObject then we know that all it's offsets also 
must
+// be fixed. FIXME: Can we do this if see a chain of super classes with
+// implementations leading to NSObject?
+return ID->getImplementation() && ID->getSuperClass() &&
+   ID->getSuperClass()->getName() == "NSObject";
+  }
+
 public:
   CGObjCNonFragileABIMac(CodeGen::CodeGenModule &cgm);
 
@@ -6702,6 +6711,12 @@ CGObjCNonFragileABIMac::EmitIvarOffsetVa
   IvarOffsetGV->setVisibility(llvm::GlobalValue::DefaultVisibility);
   }
 
+  // If ID's layout is known, then make the global constant. This serves as a
+  // useful assertion: we'll never use this variable to calculate ivar offsets,
+  // so if the runtime tries to patch it then we should crash.
+  if (isClassLayoutKnownStatically(ID))
+IvarOffsetGV->setConstant(true);
+
   if (CGM.getTriple().isOSBinFormatMachO())
 IvarOffsetGV->setSection("__DATA, __objc_ivar");
   return IvarOffsetGV;
@@ -6990,17 +7005,24 @@ LValue CGObjCNonFragileABIMac::EmitObjCV
   Offset);
 }
 
-llvm::Value *CGObjCNonFragileABIMac::EmitIvarOffset(
-  CodeGen::CodeGenFunction &CGF,
-  const ObjCInterfaceDecl *Interface,
-  const ObjCIvarDecl *Ivar) {
-  llvm::Value *IvarOffsetValue = ObjCIvarOffsetVariable(Interface, Ivar);
-  IvarOffsetValue = CGF.Builder.CreateAlignedLoad(IvarOffsetValue,
-  CGF.getSizeAlign(), "ivar");
-  if (IsIvarOffsetKnownIdempotent(CGF, Ivar))
-cast(IvarOffsetValue)
-->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
-  llvm::MDNode::get(VMContext, None));
+llvm::Value *
+CGObjCNonFragileABIMac::EmitIvarOffset(CodeGen::CodeGenFunction &CGF,
+   const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar) {
+  llvm::Value *IvarOffsetValue;
+  if (isClassLayoutKnownStatically(Interface)) {
+IvarOffsetValue = llvm::ConstantInt::get(
+ObjCTypes.IvarOffsetVarTy,
+ComputeIvarBaseOffset(CGM, Interface->getImplementation(), Ivar));
+  } else {
+llvm::GlobalVariable *GV = ObjCIvarOffsetVariable(Interface, Ivar);
+IvarOffsetValue =
+CGF.Builder.CreateAlignedLoad(GV, CGF.getSizeAlign(), "ivar");
+if (IsIvarOffsetKnownIdempotent(CGF, Ivar))
+  cast(IvarOffsetValue)
+  ->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
+llvm::MDNode::get(VMContext, None));
+  }
 
   // This could be 32bit int or 64bit integer depending on the architecture.
   // Cast it to 64bit integer value, if it is a 32bit integer ivar offset value

Added: cfe/trunk/test/CodeGenObjC/constant-non-fragile-ivar-offset.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/constant-non-fragile-ivar-offset.m?rev=351461&view=auto
==
--- cfe/trunk/test/CodeGenObjC/constant-non-fragile-ivar-offset.m (added)
+++ cfe/trunk/test/CodeGenObjC/constant-non-fragile-ivar-offset.m Thu Jan 17 
10:18:53 2019
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -emit-llvm %s -o - | 
FileCheck %s
+
+// CHECK: @"OBJC_IVAR_$_StaticLayout.static_layout_ivar" = hidden constant i64 
20
+// CHECK: @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar" = hidden 
global i64 12
+
+@interface NSObject {
+  int these, will, never, change, ever;
+}
+@end
+
+@interface StaticLayout : NSObject
+@end
+
+@implementation StaticLayout {
+  int static_

[PATCH] D56802: [CodeGenObjC] Treat ivar offsets variables as constant if they refer to ivars of a direct subclass of NSObject with an @implementation

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC351461: [CodeGenObjC] Use a constant value for non-fragile 
ivar offsets when possible (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56802?vs=182214&id=182336#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56802

Files:
  lib/CodeGen/CGObjCMac.cpp
  test/CodeGenObjC/constant-non-fragile-ivar-offset.m
  test/CodeGenObjC/optimize-ivar-offset-load.m
  test/CodeGenObjC/reorder-synthesized-ivars.m

Index: lib/CodeGen/CGObjCMac.cpp
===
--- lib/CodeGen/CGObjCMac.cpp
+++ lib/CodeGen/CGObjCMac.cpp
@@ -1550,6 +1550,15 @@
 return false;
   }
 
+  bool isClassLayoutKnownStatically(const ObjCInterfaceDecl *ID) {
+// NSObject is a fixed size. If we can see the @implementation of a class
+// which inherits from NSObject then we know that all it's offsets also must
+// be fixed. FIXME: Can we do this if see a chain of super classes with
+// implementations leading to NSObject?
+return ID->getImplementation() && ID->getSuperClass() &&
+   ID->getSuperClass()->getName() == "NSObject";
+  }
+
 public:
   CGObjCNonFragileABIMac(CodeGen::CodeGenModule &cgm);
 
@@ -6702,6 +6711,12 @@
   IvarOffsetGV->setVisibility(llvm::GlobalValue::DefaultVisibility);
   }
 
+  // If ID's layout is known, then make the global constant. This serves as a
+  // useful assertion: we'll never use this variable to calculate ivar offsets,
+  // so if the runtime tries to patch it then we should crash.
+  if (isClassLayoutKnownStatically(ID))
+IvarOffsetGV->setConstant(true);
+
   if (CGM.getTriple().isOSBinFormatMachO())
 IvarOffsetGV->setSection("__DATA, __objc_ivar");
   return IvarOffsetGV;
@@ -6990,17 +7005,24 @@
   Offset);
 }
 
-llvm::Value *CGObjCNonFragileABIMac::EmitIvarOffset(
-  CodeGen::CodeGenFunction &CGF,
-  const ObjCInterfaceDecl *Interface,
-  const ObjCIvarDecl *Ivar) {
-  llvm::Value *IvarOffsetValue = ObjCIvarOffsetVariable(Interface, Ivar);
-  IvarOffsetValue = CGF.Builder.CreateAlignedLoad(IvarOffsetValue,
-  CGF.getSizeAlign(), "ivar");
-  if (IsIvarOffsetKnownIdempotent(CGF, Ivar))
-cast(IvarOffsetValue)
-->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
-  llvm::MDNode::get(VMContext, None));
+llvm::Value *
+CGObjCNonFragileABIMac::EmitIvarOffset(CodeGen::CodeGenFunction &CGF,
+   const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar) {
+  llvm::Value *IvarOffsetValue;
+  if (isClassLayoutKnownStatically(Interface)) {
+IvarOffsetValue = llvm::ConstantInt::get(
+ObjCTypes.IvarOffsetVarTy,
+ComputeIvarBaseOffset(CGM, Interface->getImplementation(), Ivar));
+  } else {
+llvm::GlobalVariable *GV = ObjCIvarOffsetVariable(Interface, Ivar);
+IvarOffsetValue =
+CGF.Builder.CreateAlignedLoad(GV, CGF.getSizeAlign(), "ivar");
+if (IsIvarOffsetKnownIdempotent(CGF, Ivar))
+  cast(IvarOffsetValue)
+  ->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
+llvm::MDNode::get(VMContext, None));
+  }
 
   // This could be 32bit int or 64bit integer depending on the architecture.
   // Cast it to 64bit integer value, if it is a 32bit integer ivar offset value
Index: test/CodeGenObjC/constant-non-fragile-ivar-offset.m
===
--- test/CodeGenObjC/constant-non-fragile-ivar-offset.m
+++ test/CodeGenObjC/constant-non-fragile-ivar-offset.m
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: @"OBJC_IVAR_$_StaticLayout.static_layout_ivar" = hidden constant i64 20
+// CHECK: @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar" = hidden global i64 12
+
+@interface NSObject {
+  int these, will, never, change, ever;
+}
+@end
+
+@interface StaticLayout : NSObject
+@end
+
+@implementation StaticLayout {
+  int static_layout_ivar;
+}
+-(void)meth {
+  static_layout_ivar = 0;
+  // CHECK-NOT: load i64, i64* @"OBJC_IVAR_$_StaticLayout
+}
+@end
+
+@interface NotNSObject {
+  int these, might, change;
+}
+@end
+
+@interface NotStaticLayout : NotNSObject
+@end
+
+@implementation NotStaticLayout {
+  int not_static_layout_ivar;
+}
+-(void)meth {
+  not_static_layout_ivar = 0;
+  // CHECK: load i64, i64* @"OBJC_IVAR_$_NotStaticLayout.not_static_layout_ivar
+}
+@end
Index: test/CodeGenObjC/optimize-ivar-offset-load.m
===
--- test/CodeGenObjC/optimize-ivar-offset-load.m
+++ test/CodeGenObjC/optimize-ivar-offset-load.m
@@ -1,17 +1,17 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwi

[PATCH] D49754: Add -m(no-)spe, and e500 CPU definitions and support to clang

[vit9696 via Phabricator via cfe-commits]

vit9696 added a comment.

You are right, had to modify it like this to get the crash with FreeBSD triple:

  void f1(long double v, void *a)
  {
  }
  
  void f2(void* v, __builtin_va_list arg)
  {
f1(__builtin_va_arg(arg, long double), 0);
  }


Repository:
  rC Clang

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

https://reviews.llvm.org/D49754



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

[clang-tools-extra] r351463 - [Documentation] Add a chapter about Clang-tidy integrations.

[Eugene Zelenko via cfe-commits]

Author: eugenezelenko
Date: Thu Jan 17 10:31:34 2019
New Revision: 351463

URL: http://llvm.org/viewvc/llvm-project?rev=351463&view=rev
Log:
[Documentation] Add a chapter about Clang-tidy integrations.

Patch by Marina Kalashina.

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

Added:
clang-tools-extra/trunk/docs/clang-tidy/Contributing.rst
clang-tools-extra/trunk/docs/clang-tidy/Integrations.rst
Modified:
clang-tools-extra/trunk/docs/clang-tidy/index.rst

Added: clang-tools-extra/trunk/docs/clang-tidy/Contributing.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/Contributing.rst?rev=351463&view=auto
==
--- clang-tools-extra/trunk/docs/clang-tidy/Contributing.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/Contributing.rst Thu Jan 17 
10:31:34 2019
@@ -0,0 +1,507 @@
+
+Getting Involved
+
+
+:program:`clang-tidy` has several own checks and can run Clang static analyzer
+checks, but its power is in the ability to easily write custom checks.
+
+Checks are organized in modules, which can be linked into :program:`clang-tidy`
+with minimal or no code changes in :program:`clang-tidy`.
+
+Checks can plug into the analysis on the preprocessor level using 
`PPCallbacks`_
+or on the AST level using `AST Matchers`_. When an error is found, checks can
+report them in a way similar to how Clang diagnostics work. A fix-it hint can 
be
+attached to a diagnostic message.
+
+The interface provided by :program:`clang-tidy` makes it easy to write useful
+and precise checks in just a few lines of code. If you have an idea for a good
+check, the rest of this document explains how to do this.
+
+There are a few tools particularly useful when developing clang-tidy checks:
+  * ``add_new_check.py`` is a script to automate the process of adding a new
+check, it will create the check, update the CMake file and create a test;
+  * ``rename_check.py`` does what the script name suggests, renames an existing
+check;
+  * :program:`clang-query` is invaluable for interactive prototyping of AST
+matchers and exploration of the Clang AST;
+  * `clang-check`_ with the ``-ast-dump`` (and optionally ``-ast-dump-filter``)
+provides a convenient way to dump AST of a C++ program.
+
+If CMake is configured with ``CLANG_ENABLE_STATIC_ANALYZER``,
+:program:`clang-tidy` will not be built with support for the
+``clang-analyzer-*`` checks or the ``mpi-*`` checks.
+
+
+.. _AST Matchers: http://clang.llvm.org/docs/LibASTMatchers.html
+.. _PPCallbacks: http://clang.llvm.org/doxygen/classclang_1_1PPCallbacks.html
+.. _clang-check: http://clang.llvm.org/docs/ClangCheck.html
+
+
+Choosing the Right Place for your Check
+---
+
+If you have an idea of a check, you should decide whether it should be
+implemented as a:
+
++ *Clang diagnostic*: if the check is generic enough, targets code patterns 
that
+  most probably are bugs (rather than style or readability issues), can be
+  implemented effectively and with extremely low false positive rate, it may
+  make a good Clang diagnostic.
+
++ *Clang static analyzer check*: if the check requires some sort of control 
flow
+  analysis, it should probably be implemented as a static analyzer check.
+
++ *clang-tidy check* is a good choice for linter-style checks, checks that are
+  related to a certain coding style, checks that address code readability, etc.
+
+
+Preparing your Workspace
+
+
+If you are new to LLVM development, you should read the `Getting Started with
+the LLVM System`_, `Using Clang Tools`_ and `How To Setup Tooling For LLVM`_
+documents to check out and build LLVM, Clang and Clang Extra Tools with CMake.
+
+Once you are done, change to the ``llvm/tools/clang/tools/extra`` directory, 
and
+let's start!
+
+.. _Getting Started with the LLVM System: 
http://llvm.org/docs/GettingStarted.html
+.. _Using Clang Tools: http://clang.llvm.org/docs/ClangTools.html
+
+
+The Directory Structure
+---
+
+:program:`clang-tidy` source code resides in the
+``llvm/tools/clang/tools/extra`` directory and is structured as follows:
+
+::
+
+  clang-tidy/   # Clang-tidy core.
+  |-- ClangTidy.h   # Interfaces for users and checks.
+  |-- ClangTidyModule.h # Interface for clang-tidy modules.
+  |-- ClangTidyModuleRegistry.h # Interface for registering of modules.
+ ...
+  |-- google/   # Google clang-tidy module.
+  |-+
+|-- GoogleTidyModule.cpp
+|-- GoogleTidyModule.h
+  ...
+  |-- llvm/ # LLVM clang-tidy module.
+  |-+
+|-- LLVMTidyModule.cpp
+|-- LLVMTidyModule.h
+  ...
+  |-- objc/ # Objective-C clang-tidy module.
+  |-+
+|-- ObjCTidyModule.cpp
+|-- ObjCTidyModule.h
+  ...
+  |-- tool/ 

[PATCH] D54945: This commit adds a chapter about clang-tidy integrations

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL351463: [Documentation] Add a chapter about Clang-tidy 
integrations. (authored by eugenezelenko, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54945?vs=181528&id=182341#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54945

Files:
  clang-tools-extra/trunk/docs/clang-tidy/Contributing.rst
  clang-tools-extra/trunk/docs/clang-tidy/Integrations.rst
  clang-tools-extra/trunk/docs/clang-tidy/index.rst

Index: clang-tools-extra/trunk/docs/clang-tidy/index.rst
===
--- clang-tools-extra/trunk/docs/clang-tidy/index.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/index.rst
@@ -10,6 +10,8 @@
:maxdepth: 1
 
The list of clang-tidy checks 
+   Clang-tidy IDE/Editor Integrations 
+   Getting Involved 
 
 :program:`clang-tidy` is a clang-based C++ "linter" tool. Its purpose is to
 provide an extensible framework for diagnosing and fixing typical programming
@@ -310,511 +312,3 @@
 
 .. _LibTooling: http://clang.llvm.org/docs/LibTooling.html
 .. _How To Setup Tooling For LLVM: http://clang.llvm.org/docs/HowToSetupToolingForLLVM.html
-
-
-Getting Involved
-
-
-:program:`clang-tidy` has several own checks and can run Clang static analyzer
-checks, but its power is in the ability to easily write custom checks.
-
-Checks are organized in modules, which can be linked into :program:`clang-tidy`
-with minimal or no code changes in :program:`clang-tidy`.
-
-Checks can plug into the analysis on the preprocessor level using `PPCallbacks`_
-or on the AST level using `AST Matchers`_. When an error is found, checks can
-report them in a way similar to how Clang diagnostics work. A fix-it hint can be
-attached to a diagnostic message.
-
-The interface provided by :program:`clang-tidy` makes it easy to write useful
-and precise checks in just a few lines of code. If you have an idea for a good
-check, the rest of this document explains how to do this.
-
-There are a few tools particularly useful when developing clang-tidy checks:
-  * ``add_new_check.py`` is a script to automate the process of adding a new
-check, it will create the check, update the CMake file and create a test;
-  * ``rename_check.py`` does what the script name suggests, renames an existing
-check;
-  * :program:`clang-query` is invaluable for interactive prototyping of AST
-matchers and exploration of the Clang AST;
-  * `clang-check`_ with the ``-ast-dump`` (and optionally ``-ast-dump-filter``)
-provides a convenient way to dump AST of a C++ program.
-
-If CMake is configured with ``CLANG_ENABLE_STATIC_ANALYZER``,
-:program:`clang-tidy` will not be built with support for the 
-``clang-analyzer-*`` checks or the ``mpi-*`` checks.
-
-
-.. _AST Matchers: http://clang.llvm.org/docs/LibASTMatchers.html
-.. _PPCallbacks: http://clang.llvm.org/doxygen/classclang_1_1PPCallbacks.html
-.. _clang-check: http://clang.llvm.org/docs/ClangCheck.html
-
-
-Choosing the Right Place for your Check

-
-If you have an idea of a check, you should decide whether it should be
-implemented as a:
-
-+ *Clang diagnostic*: if the check is generic enough, targets code patterns that
-  most probably are bugs (rather than style or readability issues), can be
-  implemented effectively and with extremely low false positive rate, it may
-  make a good Clang diagnostic.
-
-+ *Clang static analyzer check*: if the check requires some sort of control flow
-  analysis, it should probably be implemented as a static analyzer check.
-
-+ *clang-tidy check* is a good choice for linter-style checks, checks that are
-  related to a certain coding style, checks that address code readability, etc.
-
-
-Preparing your Workspace
-
-
-If you are new to LLVM development, you should read the `Getting Started with
-the LLVM System`_, `Using Clang Tools`_ and `How To Setup Tooling For LLVM`_
-documents to check out and build LLVM, Clang and Clang Extra Tools with CMake.
-
-Once you are done, change to the ``llvm/tools/clang/tools/extra`` directory, and
-let's start!
-
-.. _Getting Started with the LLVM System: http://llvm.org/docs/GettingStarted.html
-.. _Using Clang Tools: http://clang.llvm.org/docs/ClangTools.html
-
-
-The Directory Structure

-
-:program:`clang-tidy` source code resides in the
-``llvm/tools/clang/tools/extra`` directory and is structured as follows:
-
-::
-
-  clang-tidy/   # Clang-tidy core.
-  |-- ClangTidy.h   # Interfaces for users and checks.
-  |-- ClangTidyModule.h # Interface for clang-tidy modules.
-  |-- ClangTidyModuleRegistry.h # Interface for registering of modules.
- ...
-  |-- google/   # Google