[PATCH] D71400: [RFC] [MinGW] Implicitly add .exe suffix if not provided

2019-12-17 Thread Martin Storsjö via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGee0a3b5c776c: [MinGW] Implicitly add .exe suffix if not 
provided (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71400

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/test/Driver/mingw-implicit-extension-cross.c
  clang/test/Driver/mingw-implicit-extension-windows.c


Index: clang/test/Driver/mingw-implicit-extension-windows.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-windows.c
@@ -0,0 +1,14 @@
+// Test how an implicit .exe extension is added. If running the compiler
+// on windows, an implicit extension is added if none is provided in the
+// given name. (Therefore, this test is skipped when not running on windows.)
+
+// REQUIRES: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck 
%s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.exe 2>&1 | 
FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.q 2>&1 | 
FileCheck %s --check-prefix=CHECK-OUTPUTNAME-Q
+
+// CHECK-OUTPUTNAME-EXE: "-o" "outputname.exe"
+// CHECK-OUTPUTNAME-Q: "-o" "outputname.q"
Index: clang/test/Driver/mingw-implicit-extension-cross.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-cross.c
@@ -0,0 +1,9 @@
+// Test how an implicit .exe extension is added. If not running the compiler
+// on windows, no implicit extension is added. (Therefore, this test is skipped
+// when running on windows.)
+
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### 
--sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck 
%s
+
+// CHECK: "-o" "outputname"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,19 @@
   }
 
   CmdArgs.push_back("-o");
-  CmdArgs.push_back(Output.getFilename());
+  const char *OutputFile = Output.getFilename();
+  // GCC implicitly adds an .exe extension if it is given an output file name
+  // that lacks an extension. However, GCC only does this when actually
+  // running on windows, not when operating as a cross compiler. As some users
+  // have come to rely on this behaviour, try to replicate it.
+#ifdef _WIN32
+  if (!llvm::sys::path::has_extension(OutputFile))
+CmdArgs.push_back(Args.MakeArgString(Twine(OutputFile) + ".exe"));
+  else
+CmdArgs.push_back(OutputFile);
+#else
+  CmdArgs.push_back(OutputFile);
+#endif
 
   Args.AddAllArgs(CmdArgs, options::OPT_e);
   // FIXME: add -N, -n flags


Index: clang/test/Driver/mingw-implicit-extension-windows.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-windows.c
@@ -0,0 +1,14 @@
+// Test how an implicit .exe extension is added. If running the compiler
+// on windows, an implicit extension is added if none is provided in the
+// given name. (Therefore, this test is skipped when not running on windows.)
+
+// REQUIRES: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.exe 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-EXE
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname.q 2>&1 | FileCheck %s --check-prefix=CHECK-OUTPUTNAME-Q
+
+// CHECK-OUTPUTNAME-EXE: "-o" "outputname.exe"
+// CHECK-OUTPUTNAME-Q: "-o" "outputname.q"
Index: clang/test/Driver/mingw-implicit-extension-cross.c
===
--- /dev/null
+++ clang/test/Driver/mingw-implicit-extension-cross.c
@@ -0,0 +1,9 @@
+// Test how an implicit .exe extension is added. If not running the compiler
+// on windows, no implicit extension is added. (Therefore, this test is skipped
+// when running on windows.)
+
+// UNSUPPORTED: system-windows
+
+// RUN: %clang -target i686-windows-gnu -### --sysroot=%S/Inputs/mingw_clang_tree/mingw32 %s -o outputname 2>&1 | FileCheck %s
+
+// CHECK: "-o" "outputname"
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -160,7 +160,19 @@
   }
 
   

[PATCH] D71455: [NFC] Fix typos in Clangd and Clang

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment.

In D71455#1787266 , @MaskRay wrote:

> Renaming `handleDeclOccurence`, `handleMacroOccurence` and  
> `handleModuleOccurence` are definitely not NFC because they are public APIs 
> that can be used by downstream projects.


I'm sorry if it broke some donwstream buildbots for the projects. I thought NFC 
is something that does not change behavior of the code (which is the case in 
this patch?) so is there something else that can make a patch NFC?

Also, if there is anything I could help with (e.g. submitting patches to some 
projects you know are affected by this change) please let me know.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71455



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


[PATCH] D70157: Align branches within 32-Byte boundary

2019-12-17 Thread Kan Shengchen via Phabricator via cfe-commits
skan added a comment.

In D70157#1786901 , @reames wrote:

> Noting another issue we found in local testing (with an older version of this 
> patch).  This interacts badly with the implicit exception mechanism in LLVM.  
> For that mechanism, we end up generating assembly which looks more or less 
> like this:
> Ltmp:
>
>   cmp %rsi, (%rdi)
>   jcc 
>   
>
> And a side table which maps TLmp to another label so that a fault at Ltmp can 
> be interpreted as an extremely expensive branch via signal handler.
>
> The problem is that the auto-alignment of the fused branch causes padding to 
> be introduced which separate the label and the faulting exception, breaking 
> the mapping.
>
> Essentially, this comes down to an implicit assumption that the label stays 
> tightly bundled with the following instruction.
>
> This can happen with either nop or prefix padding.


How about insert NOP before the label `Ltmp`?


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

https://reviews.llvm.org/D70157



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


[PATCH] D71476: [OpenCL] Add builtin function extension handling

2019-12-17 Thread Nicola Zaghen via Phabricator via cfe-commits
Nicola accepted this revision.
Nicola added a comment.

Just a minor comment, can you address it before you submit? Thanks!




Comment at: clang/lib/Sema/OpenCLBuiltins.td:1095
 // OpenCL v2.0 s9.17.3: Additions to section 6.13.1: Work-Item Functions
+def FuncExtKhrSubgroups : FunctionExtension<"cl_khr_subgroups">;
 let MinVersion = CL20 in {

Should this be moved together with the other extensions defined above? It might 
make the file easier to navigate if all the extensions are in the same place 
(or all close to where they are used)


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

https://reviews.llvm.org/D71476



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


[clang] ccfab8e - [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as __attribute__((objc_direct))

2019-12-17 Thread Raphael Isemann via cfe-commits

Author: Raphael Isemann
Date: 2019-12-17T09:40:36+01:00
New Revision: ccfab8e4596e59c8eea6b3610cd163c5d0312193

URL: 
https://github.com/llvm/llvm-project/commit/ccfab8e4596e59c8eea6b3610cd163c5d0312193
DIFF: 
https://github.com/llvm/llvm-project/commit/ccfab8e4596e59c8eea6b3610cd163c5d0312193.diff

LOG: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as 
__attribute__((objc_direct))

Summary:
With DWARF5 it is no longer possible to distinguish normal methods and methods 
with `__attribute__((objc_direct))` by just looking at the debug information
as they are both now children of the of the DW_TAG_structure_type that defines 
them (before only the `__attribute__((objc_direct))` methods were children).

This means that in LLDB we are no longer able to create a correct Clang AST of 
a module by just looking at the debug information. Instead we would
need to call the Objective-C runtime to see which of the methods have a 
`__attribute__((objc_direct))` and then add the attribute to our own Clang AST
depending on what the runtime returns. This would mean that we either let the 
module AST be dependent on the Objective-C runtime (which doesn't
seem right) or we retroactively add the missing attribute to the imported AST 
in our expressions.

A third option is to annotate methods with `__attribute__((objc_direct))` as 
`DW_AT_APPLE_objc_direct` which is what this patch implements. This way
LLDB doesn't have to call the runtime for any `__attribute__((objc_direct))` 
method and the AST in our module will already be correct when we create it.

Reviewers: aprantl, SouraVX

Reviewed By: aprantl

Subscribers: hiraditya, cfe-commits, llvm-commits

Tags: #clang, #llvm, #debug-info

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

Added: 
llvm/test/DebugInfo/X86/objc_direct.ll

Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGenObjC/debug-info-direct-method.m
llvm/include/llvm/BinaryFormat/Dwarf.def
llvm/include/llvm/IR/DebugInfoFlags.def
llvm/include/llvm/IR/DebugInfoMetadata.h
llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index 06204a860091..675df309e3f0 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3495,6 +3495,9 @@ llvm::DISubprogram *CGDebugInfo::getObjCMethodDeclaration(
   if (CGM.getCodeGenOpts().DwarfVersion < 5 && !OMD->isDirectMethod())
 return nullptr;
 
+  if (OMD->isDirectMethod())
+SPFlags |= llvm::DISubprogram::SPFlagObjCDirect;
+
   // Starting with DWARF V5 method declarations are emitted as children of
   // the interface type.
   auto *ID = dyn_cast_or_null(D->getDeclContext());

diff  --git a/clang/test/CodeGenObjC/debug-info-direct-method.m 
b/clang/test/CodeGenObjC/debug-info-direct-method.m
index f822088f946c..e5e2939a8c81 100644
--- a/clang/test/CodeGenObjC/debug-info-direct-method.m
+++ b/clang/test/CodeGenObjC/debug-info-direct-method.m
@@ -1,12 +1,17 @@
 // RUN: %clang_cc1 -dwarf-version=5 -emit-llvm -debug-info-kind=limited -w 
-triple x86_64-apple-darwin10 %s -o - | FileCheck %s
 // RUN: %clang_cc1 -dwarf-version=4 -emit-llvm -debug-info-kind=limited -w 
-triple x86_64-apple-darwin10 %s -o - | FileCheck %s
+// RUN: %clang_cc1 -dwarf-version=5 -emit-llvm -debug-info-kind=limited -w 
-triple x86_64-apple-darwin10 %s -o - -DDISABLE_DIRECT | FileCheck 
--check-prefix=CHECK-DISABLED %s
 
 __attribute__((objc_root_class))
 @interface Root
 @end
 
 @implementation Root
-- (int)getInt __attribute__((objc_direct)) {
+- (int)getInt
+#ifndef DISABLE_DIRECT
+ __attribute__((objc_direct))
+#endif
+{
   return 42;
 }
 @end
@@ -19,3 +24,6 @@ - (int)getInt __attribute__((objc_direct)) {
 // CHECK-SAME: runtimeLang: DW_LANG_ObjC)
 // CHECK: ![[MEMBERS]] = !{![[GETTER:[0-9]+]]}
 // CHECK: ![[GETTER]] = !DISubprogram(name: "-[Root getInt]",
+// CHECK-SAME: spFlags: DISPFlagObjCDirect
+
+// CHECK-DISABLED-NOT: DISPFlagObjCDirect

diff  --git a/llvm/include/llvm/BinaryFormat/Dwarf.def 
b/llvm/include/llvm/BinaryFormat/Dwarf.def
index 34a7410f7474..8b1b14de6f09 100644
--- a/llvm/include/llvm/BinaryFormat/Dwarf.def
+++ b/llvm/include/llvm/BinaryFormat/Dwarf.def
@@ -421,6 +421,7 @@ HANDLE_DW_AT(0x3fea, APPLE_property_setter, 0, APPLE)
 HANDLE_DW_AT(0x3feb, APPLE_property_attribute, 0, APPLE)
 HANDLE_DW_AT(0x3fec, APPLE_objc_complete_type, 0, APPLE)
 HANDLE_DW_AT(0x3fed, APPLE_property, 0, APPLE)
+HANDLE_DW_AT(0x3fee, APPLE_objc_direct, 0, APPLE)
 
 // Attribute form encodings.
 HANDLE_DW_FORM(0x01, addr, 2, DWARF)

diff  --git a/llvm/include/llvm/IR/DebugInfoFlags.def 
b/llvm/include/llvm/IR/DebugInfoFlags.def
index 587df8bec79c..df375b6c68e8 100644
--- a/llvm/include/llvm/IR/DebugInfoFlags.def
+++ b/llvm/include/llvm/IR/DebugInfoFlags.def
@@ -90,11 +90,12 @@ HANDLE_DISP_FLAG((1u << 8), MainSubprogram)
 // May al

[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

2019-12-17 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo marked 2 inline comments as done.
mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2519-2520
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())

rnk wrote:
> mstorsjo wrote:
> > rnk wrote:
> > > I would say that this is inaccurate. It greatly affects what the 
> > > optimizer is allowed to do.
> > > 
> > > It looks like we forgot to put a comdat on these things, is that not the 
> > > correct fix?
> > Oh, ok.
> > 
> > The full case I was trying to fix (but forgot to recheck after changing 
> > this bit) is that when used with `-ffunction-sections`, the tls wrapper 
> > function ends up as comdat `one_only`, which then gives multiple definition 
> > errors. So perhaps the issue is in the handling of `-ffunction-sections` 
> > wrt weak_odr?
> Ah, that is an interesting wrinkle. I'm surprised that things worked without 
> -ffunction-sections, though. I would've expected the two plain external 
> definitions to produce multiple definition errors.
Without `-ffunction-sections`, there's no separate thread wrapper produced at 
all, so everything else is generated with working linkage. I just haven't 
happened to build code that forces generation of a tls wrapper without 
`-ffunction-sections` yet.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2522
+  if (CGM.getTriple().isOSWindows())
+return llvm::GlobalValue::LinkOnceODRLinkage;
   return llvm::GlobalValue::WeakODRLinkage;

rsmith wrote:
> I think the thread wrapper should probably be `linkonce_odr` across all 
> platforms, at least in all TUs that don't contain a definition of the 
> variable. Every such TU is supposed to provide its own copy regardless, so 
> making it non-discardable seems to serve no purpose.
> 
> That said, I suspect this is only hiding the real problem (by discarding the 
> symbol before it creates a link error), and you'd still get link errors if 
> you have two TUs that both use the same thread-local variable and happen to 
> not inline / discard their thread wrappers.
There's actually a case already where these are made linkonce for other TUs at 
https://github.com/llvm/llvm-project/blob/master/clang/lib/CodeGen/ItaniumCXXABI.cpp#L2639-L2642:

```
  // If this isn't a TU in which this variable is defined, the thread
  // wrapper is discardable.
  if (Wrapper->getLinkage() == llvm::Function::WeakODRLinkage)
Wrapper->setLinkage(llvm::Function::LinkOnceODRLinkage);
```

In what cases would two TUs create two non-inline/discardable wrappers - and 
wouldn't that cause similar linking errors on other platforms as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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


[PATCH] D71201: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for methods marked as __attribute__((objc_direct))

2019-12-17 Thread Raphael Isemann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGccfab8e4596e: [ObjC][DWARF] Emit DW_AT_APPLE_objc_direct for 
methods marked as __attribute__… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71201

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenObjC/debug-info-direct-method.m
  llvm/include/llvm/BinaryFormat/Dwarf.def
  llvm/include/llvm/IR/DebugInfoFlags.def
  llvm/include/llvm/IR/DebugInfoMetadata.h
  llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  llvm/test/DebugInfo/X86/objc_direct.ll

Index: llvm/test/DebugInfo/X86/objc_direct.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/X86/objc_direct.ll
@@ -0,0 +1,54 @@
+; RUN: llc < %s -filetype=obj -o %t
+; RUN: llvm-dwarfdump -v %t | FileCheck %s
+
+; Source code to regenerate:
+; __attribute__((objc_root_class))
+; @interface Root
+; - (int)direct_method __attribute__((objc_direct));
+; @end
+;
+; @implementation Root
+; - (int)direct_method __attribute__((objc_direct)) {
+;   return 42;
+; }
+; @end
+;
+; clang -O0 -g -gdwarf-5 direct.m -c
+
+; CHECK: DW_TAG_subprogram [3]
+; CHECK: DW_AT_APPLE_objc_direct
+; CHECK-SAME: DW_FORM_flag_present
+; CHECK: DW_TAG_formal_parameter [4]
+
+; ModuleID = 'direct.bc'
+source_filename = "direct.m"
+
+%0 = type opaque
+
+define hidden i32 @"\01-[Root direct_method]"(%0* %self, i8* %_cmd) {
+entry:
+  %retval = alloca i32, align 4
+  %0 = load i32, i32* %retval, align 4
+  ret i32 %0
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!19, !20}
+!llvm.ident = !{}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_ObjC, file: !1, producer: "clang version 10.0.0 (https://github.com/llvm/llvm-project d6b2f33e2b6338d24cf756ba220939aecc81210d)", isOptimized: false, runtimeVersion: 2, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None)
+!1 = !DIFile(filename: "direct.m", directory: "/", checksumkind: CSK_MD5, checksum: "6b49fad130344b0011fc0eef65949390")
+!2 = !{}
+!3 = !{!4}
+!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "Root", scope: !1, file: !1, line: 2, flags: DIFlagObjcClassComplete, elements: !5, runtimeLang: DW_LANG_ObjC)
+!5 = !{!6}
+!6 = !DISubprogram(name: "-[Root direct_method]", scope: !4, file: !1, line: 7, type: !7, scopeLine: 7, flags: DIFlagPrototyped, spFlags: DISPFlagObjCDirect, retainedNodes: !2)
+!7 = !DISubroutineType(types: !8)
+!8 = !{!9, !10, !11}
+!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!10 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !4, size: 64, flags: DIFlagArtificial | DIFlagObjectPointer)
+!11 = !DIDerivedType(tag: DW_TAG_typedef, name: "SEL", file: !1, baseType: !12, flags: DIFlagArtificial)
+!12 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !13, size: 64)
+!13 = !DICompositeType(tag: DW_TAG_structure_type, name: "objc_selector", file: !1, flags: DIFlagFwdDecl)
+!19 = !{i32 7, !"Dwarf Version", i32 5}
+!20 = !{i32 2, !"Debug Info Version", i32 3}
Index: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
===
--- llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1233,6 +1233,9 @@
Language == dwarf::DW_LANG_ObjC))
 addFlag(SPDie, dwarf::DW_AT_prototyped);
 
+  if (SP->isObjCDirect())
+addFlag(SPDie, dwarf::DW_AT_APPLE_objc_direct);
+
   unsigned CC = 0;
   DITypeRefArray Args;
   if (const DISubroutineType *SPTy = SP->getType()) {
Index: llvm/include/llvm/IR/DebugInfoMetadata.h
===
--- llvm/include/llvm/IR/DebugInfoMetadata.h
+++ llvm/include/llvm/IR/DebugInfoMetadata.h
@@ -1758,6 +1758,7 @@
   bool isPure() const { return getSPFlags() & SPFlagPure; }
   bool isElemental() const { return getSPFlags() & SPFlagElemental; }
   bool isRecursive() const { return getSPFlags() & SPFlagRecursive; }
+  bool isObjCDirect() const { return getSPFlags() & SPFlagObjCDirect; }
 
   /// Check if this is deleted member function.
   ///
Index: llvm/include/llvm/IR/DebugInfoFlags.def
===
--- llvm/include/llvm/IR/DebugInfoFlags.def
+++ llvm/include/llvm/IR/DebugInfoFlags.def
@@ -90,11 +90,12 @@
 // May also utilize this Flag in future, when adding support
 // for defaulted functions
 HANDLE_DISP_FLAG((1u << 9), Deleted)
+HANDLE_DISP_FLAG((1u << 11), ObjCDirect)
 
 #ifdef DISP_FLAG_LARGEST_NEEDED
 // Intended to be used with ADT/BitmaskEnum.h.
 // NOTE: Always must be equal to largest flag, check this when adding new flags.
-HANDLE_DISP_FLAG((1 << 9), Largest)
+HANDLE_DISP_FLAG((1 << 11), Largest)
 #undef DISP_FLAG_LARGEST_NEEDED
 #endif
 
Index: llvm/include/llvm/BinaryFormat/Dwarf.def
===
--- llvm/include/llvm/BinaryForm

[PATCH] D70157: Align branches within 32-Byte boundary

2019-12-17 Thread annita.zhang via Phabricator via cfe-commits
annita.zhang added a comment.

In D70157#1787160 , @MaskRay wrote:

>




> There is a precedant: .pushsection/.popsection (MCStreamer::SectionStack). 
> With .push_align_branch/.pop_align_branch, we probably don't need the 
> 'switch-to-default' action.
> 
> I don't know how likely we may ever need nested states (e.g. an `.include` 
> directive inside an .align_branch region where the included file has own idea 
> about branch alignment), but .push/.pop does not seem to be more complex than 
> disable/enable/default.

I rethink about the directives and prefer the .push/.pop pair as @MaskRay 
suggested. To be specified, I'd suggest to use .push_align_branch_boundary and 
.pop_align_branch_boundary to align with MC command line options. They will 
cowork with the command line options and overwrite the options if both are 
existing.

To be clarified, I described the behavior of the directives from my 
understanding. Feel free to speak if you have difference opinion.

.push_align_branch_boundary [N,] [instruction,]*

  This directive specifies the beginning of a region which will overwrite the 
value set by the command line or by the previous directive. It can represent 
either an enabling or disabling directive controlled by parameter N. 
  N indicates to align the branches within N byte boundary. The default value 
is 32. If N is 0, it means the branch alignment is off within this region. 
  Instruction specifies types of branches to align. The value is one or 
multiple values from fused, jcc, jmp, call, ret and indirect. The default value 
is fused, jcc and jmp. (may change later)

.pop_align_branch_boundary

  This directive specifies the end of a region to align branch boundary. The 
status will be back to which was set by the previous directive or the one set 
by the command line if there's no previous directive existing.

I will coordinate with GNU binutils community once we discuss and have 
agreement with the directives.


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

https://reviews.llvm.org/D70157



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


[PATCH] D71586: [clangd] Add a test case that verifies -fimplicit-modules work in Clangd when building the AST

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added a comment.

Can we add a unit test instead of(or in addition to) this one?

I suppose it should be easy to right one using `TestTU` with `ExtraArgs` and 
`AdditionalFiles`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71586



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


[PATCH] D71455: [NFC] Fix typos in Clangd and Clang

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

In addition to that, NFC is also often used as a way to tag commits that should 
not cause any problems.
wrt to this change, I don't think anything was done wrong. It even went through 
review, although often we don't send reviews for NFC changes.

Downstream projects should be ready to update their code when public API 
changes like this happen, LLVM does not promise to keep a stable public 
interface in the C++ APIs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71455



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


[PATCH] D71586: [clangd] Add a test case that verifies -fimplicit-modules work in Clangd when building the AST

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov requested changes to this revision.
ilya-biryukov added a comment.
This revision now requires changes to proceed.

+1, please add a unit test instead.
We prefer to keep the number of lit tests in clangd minimal. They're mostly 
testing the LSP layer and other things which are hard to unit-test. 
`-fimplicit-modules` does not seem to be one of these things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71586



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


[PATCH] D71545: [clangd] Improve hover for auto on template instantiations

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:556
+// FIXME: Drop default arguments.
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;

NIT: Maybe remove default argument from this test for now and file a bug for 
fixing this instead?
Tracking this in the issue tracker seems to be better than in code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71545



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

In D71543#1785842 , @kadircet wrote:

> I've got D71545  to reduce that regression.


D71545  seems to be pretty small, yet depends 
on this change. Maybe add it right here?
Otherwise clangd is in a kinda broken state between those two commits, not a 
very good state to be at.
Would also make it simpler to revert the change in case something bad happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:353
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {

kadircet wrote:
> ilya-biryukov wrote:
> > Not related to this patch, but what is `D` here? Is this getting hover 
> > contents for a type or for a decl?
> it represents the deduced decl for Type, if any.
What is a "deduced decl for Type"?



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:365
[](HoverInfo &HI) {
- HI.Name = "class (lambda)";
+ // FIXME: Special case lambdas.
+ HI.Name = "(anonymous class)";

kadircet wrote:
> ilya-biryukov wrote:
> > NIT: could you give an example how you want the output to look like?
> See D71544
D71544 is trivial, could you put it into this patch?
Having a FIXME that is fixed in the follow-up with a one-line change seems to 
potentially complicate things (reverts, reading change history).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71508: [DebugInfo] Duplicate file names in debug info

2019-12-17 Thread Pavel Labath via Phabricator via cfe-commits
labath added a comment.

In D71508#1785767 , @probinson wrote:

> Do we have a similar problem if the filespec has an embedded ./ or ../ in it? 
>  I'm thinking some broader canonicalization ought to be done here.
>  $ clang ./dir1/dir2/../dir3/file.c
>  should resolve to dir1/dir3/file.c shouldn't it?


I would be very careful about aggressive canonicalization. Once you throw 
symlinks into the mix, there's very little things you can safely do. If `dir2` 
is a symlink then `dir2/..` can point pretty much anywhere. And if you use 
something like `realpath` there's no telling whether the final result actually 
be the thing you actually want to put into the debug info (your whole source 
tree could be a "symlink farm" with individual files pointing to random 
unpredictable locations).

It seems to me that the underlying problem here is that there are different 
kinds of canonicalization applied to the "main" and other files, which is 
something that the comment in CDDebugInfo::CreateCompileUnit seems to 
acknowledge. However, I don't know anything about this code to say how/if it is 
possible to fix that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71508



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


[PATCH] D71594: testing clang-tidy

2019-12-17 Thread Mikhail Goncharov via Phabricator via cfe-commits
goncharov created this revision.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
goncharov removed subscribers: jkorous, arphaman, kadircet, usaxena95, 
cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71594

Files:
  clang-tools-extra/clangd/Function.h
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/tools/clang-check/ClangCheck.cpp
  clang/tools/clang-diff/ClangDiff.cpp
  clang/tools/clang-fuzzer/ClangFuzzer.cpp
  clang/tools/clang-refactor/ClangRefactor.cpp

Index: clang/tools/clang-refactor/ClangRefactor.cpp
===
--- clang/tools/clang-refactor/ClangRefactor.cpp
+++ clang/tools/clang-refactor/ClangRefactor.cpp
@@ -90,7 +90,6 @@
   TestSourceSelectionArgument(TestSelectionRangesInFile TestSelections)
   : TestSelections(std::move(TestSelections)) {}
 
-  void print(raw_ostream &OS) override { TestSelections.dump(OS); }
 
   std::unique_ptr
   createCustomConsumer() override {
@@ -104,6 +103,9 @@
 return TestSelections.foreachRange(SM, Callback);
   }
 
+  void print(raw_ostream &OS) override {
+  TestSelections.dump(OS);
+  }
 private:
   TestSelectionRangesInFile TestSelections;
 };
Index: clang/tools/clang-fuzzer/ClangFuzzer.cpp
===
--- clang/tools/clang-fuzzer/ClangFuzzer.cpp
+++ clang/tools/clang-fuzzer/ClangFuzzer.cpp
@@ -15,11 +15,11 @@
 #include "handle-cxx/handle_cxx.h"
 
 using namespace clang_fuzzer;
-
-extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
-
 extern "C" int LLVMFuzzerTestOneInput(uint8_t *data, size_t size) {
   std::string s((const char *)data, size);
   HandleCXX(s, "./test.cc", {"-O2"});
   return 0;
 }
+
+extern "C" int LLVMFuzzerInitialize(   int *argc,
+   char ***argv) { return 0 ; }
Index: clang/tools/clang-diff/ClangDiff.cpp
===
--- clang/tools/clang-diff/ClangDiff.cpp
+++ clang/tools/clang-diff/ClangDiff.cpp
@@ -32,12 +32,10 @@
 cl::desc("Print the internal representation of the AST as JSON."),
 cl::init(false), cl::cat(ClangDiffCategory));
 
-static cl::opt PrintMatches("dump-matches",
-  cl::desc("Print the matched nodes."),
-  cl::init(false), cl::cat(ClangDiffCategory));
+static cl::opt PrintMatches("dump-matches", cl::desc("Print the matched nodes."), cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt HtmlDiff("html",
-  cl::desc("Output a side-by-side diff in HTML."),
+ cl::desc("Output a side-by-side diff in HTML."),
   cl::init(false), cl::cat(ClangDiffCategory));
 
 static cl::opt SourcePath(cl::Positional, cl::desc(""),
@@ -77,7 +75,10 @@
   std::make_unique(
   std::move(Compilations));
   AdjustingCompilations->appendArgumentsAdjuster(
-  getInsertArgumentAdjuster(ArgsBefore, ArgumentInsertPosition::BEGIN));
+
+  getInsertArgumentAdjuster(ArgsBefore   ,
+
+  ArgumentInsertPosition::BEGIN));
   AdjustingCompilations->appendArgumentsAdjuster(
   getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
   Compilations = std::move(AdjustingCompilations);
Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -17,10 +17,10 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/CodeGen/ObjectFilePCHContainerOperations.h"
-#include "clang/Driver/Options.h"
 #include "clang/Frontend/ASTConsumers.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Rewrite/Frontend/FixItRewriter.h"
+#include "clang/Driver/Options.h"
 #include "clang/Rewrite/Frontend/FrontendActions.h"
 #include "clang/StaticAnalyzer/Frontend/FrontendActions.h"
 #include "clang/Tooling/CommonOptionsParser.h"
@@ -28,8 +28,8 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Support/Path.h"
-#include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h"
+#include "llvm/Support/Signals.h"
 
 using namespace clang::driver;
 using namespace clang::tooling;
@@ -52,7 +52,7 @@
 );
 
 static cl::OptionCategory ClangCheckCategory("clang-check options");
-static const opt::OptTable &Options = getDriverOptTable();
+staticconst opt::OptTable &Options = getDriverOptTable();
 static cl::opt
 ASTDump("ast-dump",
 cl::desc(Options.getOptionHelpText(options::OPT_ast_dump)),
@@ -148,7 +148,7 @@
   }
 };
 
-} // namespace
+}
 
 int main(int argc, const char **argv) {
   llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
Index: clang/include/clang/Analysis/AnalysisDeclContext.h

[PATCH] D71545: [clangd] Improve hover for auto on template instantiations

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 2 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:556
+// FIXME: Drop default arguments.
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;

ilya-biryukov wrote:
> NIT: Maybe remove default argument from this test for now and file a bug for 
> fixing this instead?
> Tracking this in the issue tracker seems to be better than in code.
sure filed https://github.com/clangd/clangd/issues/229


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71545



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


[clang] ddd0bb8 - [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Thomas Preud'homme via cfe-commits

Author: Thomas Preud'homme
Date: 2019-12-17T10:36:36Z
New Revision: ddd0bb8dba2a367c6aa8a25e98915509847745ce

URL: 
https://github.com/llvm/llvm-project/commit/ddd0bb8dba2a367c6aa8a25e98915509847745ce
DIFF: 
https://github.com/llvm/llvm-project/commit/ddd0bb8dba2a367c6aa8a25e98915509847745ce.diff

LOG: [lit] Remove lit's REQUIRES-ANY directive

Summary:
Remove REQUIRES-ANY alias lit directive since it is hardly used and can
be easily implemented using an OR expression using REQUIRES. Fixup
remaining testcases still using REQUIRES-ANY.

Reviewers: probinson, jdenny, gparker42

Reviewed By: gparker42

Subscribers: eugenis, asb, rbar, johnrusso, simoncook, sabuasal, niosHD, 
delcypher, jrtc27, zzheng, edward-jones, rogfer01, MartinMosbeck, brucehoult, 
the_o, PkmX, jocewei, lenary, s.egerton, pzheng, sameer.abuasal, apazos, 
luismarques, cfe-commits, #sanitizers, llvm-commits

Tags: #llvm, #clang, #sanitizers

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

Added: 


Modified: 
clang/test/Driver/XRay/xray-instrument-macos.c
clang/test/Driver/XRay/xray-instrument-os.c
clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
clang/test/Driver/XRay/xray-mode-flags.cpp
clang/test/Driver/XRay/xray-nolinkdeps.cpp
compiler-rt/test/builtins/Unit/arm/aeabi_cdcmpeq_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_cdcmple_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_cfcmpeq_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_cfcmple_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_drsub_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_frsub_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_idivmod_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c
compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
llvm/utils/lit/lit/TestRunner.py

Removed: 
llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt



diff  --git a/clang/test/Driver/XRay/xray-instrument-macos.c 
b/clang/test/Driver/XRay/xray-instrument-macos.c
index afccc625832b..ce68345ed019 100644
--- a/clang/test/Driver/XRay/xray-instrument-macos.c
+++ b/clang/test/Driver/XRay/xray-instrument-macos.c
@@ -1,4 +1,4 @@
 // RUN: %clang -o /dev/null -v -fxray-instrument -target 
x86_64-apple-macos10.11 -c %s
 // RUN: %clang -o /dev/null -v -fxray-instrument -target x86_64-apple-darwin15 
-c %s
-// REQUIRES-ANY: x86_64, x86_64h
+// REQUIRES: x86_64 || x86_64h
 typedef int a;

diff  --git a/clang/test/Driver/XRay/xray-instrument-os.c 
b/clang/test/Driver/XRay/xray-instrument-os.c
index 3a0c428ce19c..ba97328b54a6 100644
--- a/clang/test/Driver/XRay/xray-instrument-os.c
+++ b/clang/test/Driver/XRay/xray-instrument-os.c
@@ -1,4 +1,4 @@
 // RUN: not %clang -o /dev/null -v -fxray-instrument -c %s
 // XFAIL: -linux-, -freebsd, -darwin, -macos
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
+// REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64
 typedef int a;

diff  --git a/clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp 
b/clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
index da2535509b99..b68dca235525 100644
--- a/clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
+++ b/clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
@@ -7,5 +7,5 @@
 // RUN: | FileCheck %s
 // CHECK:  -fxray-instrumentation-bundle=function
 //
-// REQUIRES-ANY: linux, freebsd
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
+// REQUIRES: linux || freebsd
+// REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64

diff  --git a/clang/test/Driver/XRay/xray-mode-flags.cpp 
b/clang/test/Driver/XRay/xray-mode-flags.cpp
index 281cf0b547fa..e95053a4c684 100644
--- a/clang/test/Driver/XRay/xray-mode-flags.cpp
+++ b/clang/test/Driver/XRay/xray-mode-flags.cpp
@@ -45,5 +45,5 @@
 // FDR: libclang_rt.xray-fdr
 // NONE-NOT: libclang_rt.xray-basic
 // NONE-NOT: libclang_rt.xray-fdr
-// REQUIRES-ANY: linux, freebsd
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
+// REQUIRES: linux || freebsd
+// REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64

diff  --git a/clang/test/Driver/XRay/xray-nolinkdeps.cpp 
b/clang/test/Driver/XRay/xray-nolinkdeps.cpp
index 5a79e362e356..5461fc325a24 100644
--- a/clang/test/Driver/XRay/xray-nolinkdeps.cpp
+++ b/clang/test/Driver/XRay/xray-nolinkdeps.cpp
@@ -4,5 +4,5 @@
 // RUN: 2>&1 | FileCheck --check-prefix ENABLE %s
 // ENABLE: clang_rt.xray
 // DISABLE-NOT: clang_rt.xray
-// REQUIRES-ANY: linux, freebsd
-// REQUIRES-ANY: amd64, x86_64, x86_64h, arm, aarch64, arm64
+// REQUIRES: linux || freebsd
+// REQUIRES: amd64 || x86_64 || x86_64h || arm || aarch64 || arm64

diff  --git a/compiler-rt/test/builtins/Unit/arm/aeabi_cdcmpeq_test.c 
b/compiler-rt/test/buil

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234249.
kadircet added a comment.

- Squash follow-ups


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -104,7 +104,7 @@
   }}
   )cpp",
[](HoverInfo &HI) {
- HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.NamespaceScope = "ns1::";
  HI.LocalScope = "(anonymous struct)::";
  HI.Name = "bar";
  HI.Kind = index::SymbolKind::Field;
@@ -362,7 +362,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class (lambda)";
+ HI.Name = "(lambda)";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on template instantiation
@@ -373,7 +373,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on specialized template
@@ -385,7 +385,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
 
@@ -524,6 +524,44 @@
  HI.NamespaceScope = "";
  HI.LocalScope = "boom::";
}},
+  {
+  R"cpp(// Should not print inline or anon namespaces.
+  namespace ns {
+inline namespace in_ns {
+  namespace a {
+namespace {
+  namespace b {
+inline namespace in_ns2 {
+  class Foo {};
+} // in_ns2
+  } // b
+} // anon
+  } // a
+} // in_ns
+  } // ns
+  void foo() {
+ns::a::b::[[F^oo]] x;
+(void)x;
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns::a::b::";
+HI.Definition = "class Foo {}";
+  }},
+  {
+  R"cpp(
+  template  class Foo {};
+  class X;
+  void foo() {
+[[^auto]] x = Foo();
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+  }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
@@ -895,7 +933,7 @@
   [](HoverInfo &HI) {
 HI.Name = "foo";
 HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "ns::(anonymous)::";
+HI.NamespaceScope = "ns::";
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
@@ -1173,7 +1211,8 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "class std::initializer_list";
+// FIXME: Print template instantiation parameters.
+HI.Name = "initializer_list";
 HI.Kind = index::SymbolKind::Class;
   }},
   {
@@ -1231,7 +1270,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1242,7 +1281,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1253,7 +1292,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1265,7 +1304,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1277,7 +1316,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1289,7 +1328,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1300,7 +1339,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = i

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked 3 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:353
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > Not related to this patch, but what is `D` here? Is this getting hover 
> > > contents for a type or for a decl?
> > it represents the deduced decl for Type, if any.
> What is a "deduced decl for Type"?
it was referring to the tagdecl referred by `decltype`s, i am not sure if there 
are cases in which this can be different than `T->getAsTagDecl`.
Always making use of `T->getAsTagDecl` doesn't seem to be causing any test 
failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234250.
kadircet added a comment.

- Delete fixme


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -104,7 +104,7 @@
   }}
   )cpp",
[](HoverInfo &HI) {
- HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.NamespaceScope = "ns1::";
  HI.LocalScope = "(anonymous struct)::";
  HI.Name = "bar";
  HI.Kind = index::SymbolKind::Field;
@@ -362,7 +362,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class (lambda)";
+ HI.Name = "(lambda)";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on template instantiation
@@ -373,7 +373,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on specialized template
@@ -385,7 +385,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
 
@@ -524,6 +524,44 @@
  HI.NamespaceScope = "";
  HI.LocalScope = "boom::";
}},
+  {
+  R"cpp(// Should not print inline or anon namespaces.
+  namespace ns {
+inline namespace in_ns {
+  namespace a {
+namespace {
+  namespace b {
+inline namespace in_ns2 {
+  class Foo {};
+} // in_ns2
+  } // b
+} // anon
+  } // a
+} // in_ns
+  } // ns
+  void foo() {
+ns::a::b::[[F^oo]] x;
+(void)x;
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns::a::b::";
+HI.Definition = "class Foo {}";
+  }},
+  {
+  R"cpp(
+  template  class Foo {};
+  class X;
+  void foo() {
+[[^auto]] x = Foo();
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+  }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
@@ -895,7 +933,7 @@
   [](HoverInfo &HI) {
 HI.Name = "foo";
 HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "ns::(anonymous)::";
+HI.NamespaceScope = "ns::";
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
@@ -1173,7 +1211,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "class std::initializer_list";
+HI.Name = "initializer_list";
 HI.Kind = index::SymbolKind::Class;
   }},
   {
@@ -1231,7 +1269,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1242,7 +1280,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1253,7 +1291,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1265,7 +1303,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1277,7 +1315,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1289,7 +1327,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1300,7 +1338,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1364,7 +1402,7 

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60932 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60932 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71556: [AArch64][SVE] Implement intrinsic for non-faulting loads

2019-12-17 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:329
+
+def non_faulting_load :
+   PatFrag<(ops node:$ptr, node:$pred, node:$def),

This duplicates a lot of code, maybe it makes sense to combine this into a 
multiclass. I'm thinking of something along the lines of:
```
multiclass load_store_fragments {
  def : PatFrag<(ops node:$ptr, node:$pred, node:$def),
(masked_ld node:$ptr, undef, node:$pred, node:$def),
pred>;

  def _i8 : PatFrag<(ops ...),
((cast(NAME) ...),
[{ return 
cast(N)->getMemoryVT().getScalarType() == MVT::i8; }]>;
  def _i16 : PatFrag<(ops ...),
((cast(NAME) ),
[{ return 
cast(N)->getMemoryVT().getScalarType() == MVT::i16; }]>;
  def _i32 : PatFrag<(ops ...),
((cast(NAME) ),
[{ return 
cast(N)->getMemoryVT().getScalarType() == MVT::i32; }]>;
  def _i64 : PatFrag<(ops ...),
((cast(NAME) ),
[{ return 
cast(N)->getMemoryVT().getScalarType() == MVT::i64; }]>;
}

defm non_temporal_load : load_store_fragments<[{
   return cast(N)->getExtensionType() == ISD::NON_EXTLOAD &&
  cast(N)->isUnindexed() &&
  cast(N)->isNonTemporal() &&
  !cast(N)->isNonFaulting();
}]>;

defm sext_non_temporal_load : load_store_fragments<[{
   return cast(N)->getExtensionType() == ISD::SEXTLOAD &&
  cast(N)->isUnindexed() &&
  cast(N)->isNonTemporal() &&
  !cast(N)->isNonFaulting();
}]>;

defm zext_non_faulting_load : load_store_fragments<[{
   return cast(N)->getExtensionType() == ISD::ZEXTLOAD &&
  cast(N)->isUnindexed() &&
  !cast(N)->isNonTemporal() &&
  cast(N)->isNonFaulting();
}]>;
```



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:338
+
+def sext_non_faulting_load :
+  PatFrag<(ops node:$ptr, node:$pred, node:$def),

nit: we should probably add `_masked_` to the name (although I realise that I 
forgot to spot that on the non-temporal patch)



Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:341
+  (masked_ld node:$ptr, undef, node:$pred, node:$def), [{
+  return cast(N)->getExtensionType() == ISD::SEXTLOAD &&
+ cast(N)->isUnindexed() &&

Shall we support any-extend as well? (thus making `asext_non_faulting_load`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71556



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


[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

2019-12-17 Thread Sourabh Singh Tomar via Phabricator via cfe-commits
SouraVX added a comment.

> It looks like this implementation is a bit buggy in one way and incomplete in 
> another:
> 
> 1. even if the auto-returning function is defined, that function definition 
> doesn't describe the concrete return type. Compare GCC and Clang's output for 
> your example and note that... oh.

I think that's correct behavior, consider this for a moment --

  struct foo {
  auto foo_func();
  };
  int foo::foo_func(){return 0;}
  clang error: ->
  error: return type of out-of-line definition of 'foo::foo_func' differs from 
that in the declaration

So this seems fair to me, regardless of the concrete return type{assuming this 
is what this patch is doing}. We should be emitting `auto` in declaration. AKA 
in unspecified_type. GCC(trunk) also seems fair here.

> Hmm, maybe this feature/suggestion is broken or at least not exactly awesome 
> when it comes to auto-returning functions that are eventually void-returning 
> functions? Now the function definition has no DW_AT_type to override the 
> unspecified_type in the declaration... :/ that's unfortunate (@probinson - 
> thoughts?)

I'm a bit confused here, regardless of the concrete return type{void/int/float} 
GCC(trunk) is emitting 
`DW_TAG_unspecified_type`
`DW_AT_name "auto"` 
Are you trying to say we should be emitting `DW_AT_type void/int/float` ?? 
That's what functionality of clang is right now, and if we entertain that, then 
their is no point of emitting `DW_TAG_unspecifed_type auto` at first place.

> GCC does use the unspecified type "auto" even back in DWARFv4 and it leaves 
> the subprogram definition DIE without a DW_AT_type if the auto type ends up 
> as void (what else could it do?) so I guess we can do this for consistency & 
> consumers have to know that a definition can't really have an auto return 
> type and that it must be really void.
> 
> In any case - change the test case to use a non-void return type in the 
> definition ("return 3;" for instance, to get an int return type instead) and 
> check that the DISubprogram for the definition has a concrete return type of 
> "int" while the DISubprogram for the declaration has the "auto" 
> unspecified_type return type. (contrast/test against GCC's behavior)



> 2. Presumably in a follow-up patch, make sure that the declaration for the 
> DISubprogram declaration for an "auto" return type function appears in the 
> member list of the DICompositeType even if the function is not called (same 
> as other normal (non-implicit/non-template) functions) since that's the value 
> of being able to describe the return type as "auto" (the function can be 
> described even when the definition isn't available/emitted) - it doesn't 
> currently. (contrast/test against with GCC's behavior)

Agreed testing for this must be exhaustive, but I think for all test cases 
behavior should be same -- "DW_TAG_unspecified_type auto" should be emitted for 
the function declared/defined as auto returnning. Do you have other test cases 
in mind, where above points diverges ??


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

https://reviews.llvm.org/D70524



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


[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D71241#1787265 , @hfinkel wrote:

> In D71241#1786959 , @jdoerfert wrote:
>
> > In D71241#1786530 , @ABataev wrote:
> >
> > > Most probably, we can use this solution without adding a new expression. 
> > > `DeclRefExpr` class can contain 2 decls: FoundDecl and the Decl being 
> > > used. You can use FoundDecl to point to the original function and used 
> > > decl to point to the function being called in this context. But at first, 
> > > we need to be sure that we can handle all corner cases correctly.
> >
> >
> > What new expression are you talking about?
>
>
> To be clear, I believe he's talking about the new expression that I proposed 
> we add in order to represent this kind of call. If that's not needed, and we 
> can use the FoundDecl/Decl pair for that purpose, that should also be 
> considered.
>
> > This solution already does point to both declarations, as shown here: 
> > https://reviews.llvm.org/D71241#1782504


Exactly. We need to check if the `MemberRefExpr` can do this too to handle 
member functions correctly.
And need to wait for opinion from Richard Smith about the design. We need to be 
sure that it won't break compatibility with C/C++ in some corner cases. Design 
bugs are very hard to solve and I want to be sure that we don't miss anything. 
And we provide full compatibility with both C and C++.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[clang] df5a905 - [OpenCL] Add ExtVectorElementExpr constant evaluation (PR42387)

2019-12-17 Thread Sven van Haastregt via cfe-commits

Author: Sven van Haastregt
Date: 2019-12-17T11:10:06Z
New Revision: df5a905aa8a868bdb700d88e427491ee56243e30

URL: 
https://github.com/llvm/llvm-project/commit/df5a905aa8a868bdb700d88e427491ee56243e30
DIFF: 
https://github.com/llvm/llvm-project/commit/df5a905aa8a868bdb700d88e427491ee56243e30.diff

LOG: [OpenCL] Add ExtVectorElementExpr constant evaluation (PR42387)

Add constexpr evaluation for ExtVectorElementExpr nodes by evaluating
the underlying vector expression.  Add basic folding for the case that
Evaluate does not return an LValue.

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

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/CodeGenOpenCLCXX/constexpr.cl

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 0b658cbb712f..7e33b9d354b0 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -7080,6 +7080,31 @@ class ExprEvaluatorBase
DerivedSuccess(Result, E);
   }
 
+  bool VisitExtVectorElementExpr(const ExtVectorElementExpr *E) {
+APValue Val;
+if (!Evaluate(Val, Info, E->getBase()))
+  return false;
+
+if (Val.isVector()) {
+  SmallVector Indices;
+  E->getEncodedElementAccess(Indices);
+  if (Indices.size() == 1) {
+// Return scalar.
+return DerivedSuccess(Val.getVectorElt(Indices[0]), E);
+  } else {
+// Construct new APValue vector.
+SmallVector Elts;
+for (unsigned I = 0; I < Indices.size(); ++I) {
+  Elts.push_back(Val.getVectorElt(Indices[I]));
+}
+APValue VecResult(Elts.data(), Indices.size());
+return DerivedSuccess(VecResult, E);
+  }
+}
+
+return false;
+  }
+
   bool VisitCastExpr(const CastExpr *E) {
 switch (E->getCastKind()) {
 default:

diff  --git a/clang/test/CodeGenOpenCLCXX/constexpr.cl 
b/clang/test/CodeGenOpenCLCXX/constexpr.cl
index b7175020b814..8c3fad08ea76 100644
--- a/clang/test/CodeGenOpenCLCXX/constexpr.cl
+++ b/clang/test/CodeGenOpenCLCXX/constexpr.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -O0 
-emit-llvm -o - | FileCheck %s
 
+typedef int int2 __attribute__((ext_vector_type(2)));
+typedef int int4 __attribute__((ext_vector_type(4)));
+
 struct Storage final {
   constexpr const float& operator[](const int index) const noexcept {
 return InternalStorage[index];
@@ -24,3 +27,28 @@ constexpr float FloatConstant = compute();
 kernel void foo(global float *x) {
   *x = FloatConstant;
 }
+
+// Test evaluation of constant vectors.
+// CHECK-LABEL: define spir_kernel void @vecEval
+// CHECK: store i32 3
+// CHECK: store <2 x i32> , <2 x i32>
+
+const int oneElt = int4(3).x;
+const int2 twoElts = (int4)(11, 22, 33, 44).yz;
+
+kernel void vecEval(global int *x, global int2 *y) {
+  *x = oneElt;
+  *y = twoElts;
+}
+
+// Test evaluation of vectors initialized through a constexpr function.
+// CHECK-LABEL: define spir_kernel void @vecEval2
+// CHECK: store <2 x i32>
+constexpr int2 addOne(int2 x) {
+  return (int2)(x.x + 1, x.y + 1);
+}
+const int2 fromConstexprFunc = addOne(int2(2));
+
+kernel void vecEval2(global int2 *x) {
+  *x = fromConstexprFunc;
+}



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


[PATCH] D71133: [OpenCL] Add ExtVectorElementExpr constant evaluation (PR42387)

2019-12-17 Thread Sven van Haastregt via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGdf5a905aa8a8: [OpenCL] Add ExtVectorElementExpr constant 
evaluation (PR42387) (authored by svenvh).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71133

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGenOpenCLCXX/constexpr.cl


Index: clang/test/CodeGenOpenCLCXX/constexpr.cl
===
--- clang/test/CodeGenOpenCLCXX/constexpr.cl
+++ clang/test/CodeGenOpenCLCXX/constexpr.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -O0 
-emit-llvm -o - | FileCheck %s
 
+typedef int int2 __attribute__((ext_vector_type(2)));
+typedef int int4 __attribute__((ext_vector_type(4)));
+
 struct Storage final {
   constexpr const float& operator[](const int index) const noexcept {
 return InternalStorage[index];
@@ -24,3 +27,28 @@
 kernel void foo(global float *x) {
   *x = FloatConstant;
 }
+
+// Test evaluation of constant vectors.
+// CHECK-LABEL: define spir_kernel void @vecEval
+// CHECK: store i32 3
+// CHECK: store <2 x i32> , <2 x i32>
+
+const int oneElt = int4(3).x;
+const int2 twoElts = (int4)(11, 22, 33, 44).yz;
+
+kernel void vecEval(global int *x, global int2 *y) {
+  *x = oneElt;
+  *y = twoElts;
+}
+
+// Test evaluation of vectors initialized through a constexpr function.
+// CHECK-LABEL: define spir_kernel void @vecEval2
+// CHECK: store <2 x i32>
+constexpr int2 addOne(int2 x) {
+  return (int2)(x.x + 1, x.y + 1);
+}
+const int2 fromConstexprFunc = addOne(int2(2));
+
+kernel void vecEval2(global int2 *x) {
+  *x = fromConstexprFunc;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7080,6 +7080,31 @@
DerivedSuccess(Result, E);
   }
 
+  bool VisitExtVectorElementExpr(const ExtVectorElementExpr *E) {
+APValue Val;
+if (!Evaluate(Val, Info, E->getBase()))
+  return false;
+
+if (Val.isVector()) {
+  SmallVector Indices;
+  E->getEncodedElementAccess(Indices);
+  if (Indices.size() == 1) {
+// Return scalar.
+return DerivedSuccess(Val.getVectorElt(Indices[0]), E);
+  } else {
+// Construct new APValue vector.
+SmallVector Elts;
+for (unsigned I = 0; I < Indices.size(); ++I) {
+  Elts.push_back(Val.getVectorElt(Indices[I]));
+}
+APValue VecResult(Elts.data(), Indices.size());
+return DerivedSuccess(VecResult, E);
+  }
+}
+
+return false;
+  }
+
   bool VisitCastExpr(const CastExpr *E) {
 switch (E->getCastKind()) {
 default:


Index: clang/test/CodeGenOpenCLCXX/constexpr.cl
===
--- clang/test/CodeGenOpenCLCXX/constexpr.cl
+++ clang/test/CodeGenOpenCLCXX/constexpr.cl
@@ -1,5 +1,8 @@
 // RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=clc++ -O0 -emit-llvm -o - | FileCheck %s
 
+typedef int int2 __attribute__((ext_vector_type(2)));
+typedef int int4 __attribute__((ext_vector_type(4)));
+
 struct Storage final {
   constexpr const float& operator[](const int index) const noexcept {
 return InternalStorage[index];
@@ -24,3 +27,28 @@
 kernel void foo(global float *x) {
   *x = FloatConstant;
 }
+
+// Test evaluation of constant vectors.
+// CHECK-LABEL: define spir_kernel void @vecEval
+// CHECK: store i32 3
+// CHECK: store <2 x i32> , <2 x i32>
+
+const int oneElt = int4(3).x;
+const int2 twoElts = (int4)(11, 22, 33, 44).yz;
+
+kernel void vecEval(global int *x, global int2 *y) {
+  *x = oneElt;
+  *y = twoElts;
+}
+
+// Test evaluation of vectors initialized through a constexpr function.
+// CHECK-LABEL: define spir_kernel void @vecEval2
+// CHECK: store <2 x i32>
+constexpr int2 addOne(int2 x) {
+  return (int2)(x.x + 1, x.y + 1);
+}
+const int2 fromConstexprFunc = addOne(int2(2));
+
+kernel void vecEval2(global int2 *x) {
+  *x = fromConstexprFunc;
+}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7080,6 +7080,31 @@
DerivedSuccess(Result, E);
   }
 
+  bool VisitExtVectorElementExpr(const ExtVectorElementExpr *E) {
+APValue Val;
+if (!Evaluate(Val, Info, E->getBase()))
+  return false;
+
+if (Val.isVector()) {
+  SmallVector Indices;
+  E->getEncodedElementAccess(Indices);
+  if (Indices.size() == 1) {
+// Return scalar.
+return DerivedSuccess(Val.getVectorElt(Indices[0]), E);
+  } else {
+// Construct new APValue vector.
+SmallVector Elts;
+for (unsigned I = 0; I < Indices.size(); ++I) {
+  Elts.push_back(Val.getVectorElt(Indices[I]));
+  

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

Clangd didn't fill documentation for `auto` when it wasn't available in
index. Also it wasn't showing any documentations for implicit instantiations.

This patch ensures auto and normal decl case behaves in the same way and also
makes use of the explicit template specialization while fetching comments for
implicit instantiations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71596

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "AST.h"
 #include "Annotations.h"
 #include "Hover.h"
 #include "TestIndex.h"
@@ -1271,6 +1272,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto function return with trailing type";
   }},
   {
   R"cpp(// trailing return type
@@ -1282,6 +1284,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "trailing return type";
   }},
   {
   R"cpp(// auto in function return
@@ -1293,6 +1296,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto in function return";
   }},
   {
   R"cpp(// auto& in function return
@@ -1305,6 +1309,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto& in function return";
   }},
   {
   R"cpp(// auto* in function return
@@ -1317,6 +1322,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto* in function return";
   }},
   {
   R"cpp(// const auto& in function return
@@ -1329,6 +1335,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "const auto& in function return";
   }},
   {
   R"cpp(// decltype(auto) in function return
@@ -1340,6 +1347,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "decltype(auto) in function return";
   }},
   {
   R"cpp(// decltype(auto) reference in function return
@@ -1404,6 +1412,8 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation =
+"decltype of function with trailing return type.";
   }},
   {
   R"cpp(// decltype of var with decltype.
@@ -1477,6 +1487,87 @@
   }
 }
 
+TEST(Hover, DocsFromIndex) {
+  Annotations T(R"cpp(
+  template  class X {};
+  void foo() {
+au^to t = X();
+X^ w;
+(void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  Symbol IndexSym;
+  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.Documentation = "comment from index";
+  SymbolSlab::Builder Symbols;
+  Symbols.insert(IndexSym);
+  auto Index =
+  MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
+
+  for (const auto &P : T.points()) {
+auto H = getHover(AST, P, format::getLLVMStyle(), Index.get());
+ASSERT_TRUE(H);
+EXPECT_EQ(H->Documentation, IndexSym.Documentation);
+  }
+}
+
+TEST(Hover, DocsFromAST) {
+  Annotations T(R"cpp(
+  // doc
+  template  class X {};
+  void foo() {
+au^to t = X();
+X^ w;
+(void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (const auto &P : T.points()) {
+auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
+ASSERT_TRUE(H);
+EXPECT_EQ(H->Documentation, "doc");
+  }
+}
+
+TEST(Hover, DocsFromMostSpecial) {
+  Annotations T(R"cpp(
+  // doc1
+  template  class X {};
+  // doc2
+  template <> class X {};
+  // doc3
+  template  class X {};
+  void foo() {
+X$doc1^();
+X$doc2^();
+X$doc3^();
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build()

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon question-circle color=gray} Unit tests: unknown.

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 console-log.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:353
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {

kadircet wrote:
> ilya-biryukov wrote:
> > kadircet wrote:
> > > ilya-biryukov wrote:
> > > > Not related to this patch, but what is `D` here? Is this getting hover 
> > > > contents for a type or for a decl?
> > > it represents the deduced decl for Type, if any.
> > What is a "deduced decl for Type"?
> it was referring to the tagdecl referred by `decltype`s, i am not sure if 
> there are cases in which this can be different than `T->getAsTagDecl`.
> Always making use of `T->getAsTagDecl` doesn't seem to be causing any test 
> failures.
if `T->getAsTagDecl()` is non-null, why does `D` being passed to the function 
is null?
How can they be different?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71566: New checks for fortified sprintf

2019-12-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 234262.
serge-sans-paille added a comment.

Take remarks into account:

- support sprintf when the buffer has a known static size
- use llvm::Optional
- some extra minor tweaks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-fortify-source.c

Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -11,6 +11,8 @@
 extern "C" {
 #endif
 
+extern int sprintf(char *str, const char *format, ...);
+
 #if defined(USE_PASS_OBJECT_SIZE)
 void *memcpy(void *dst, const void *src, size_t c);
 static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
@@ -96,6 +98,37 @@
   __builtin_vsnprintf(buf, 11, "merp", list); // expected-warning {{'vsnprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
 }
 
+void call_sprintf_chk(char *buf) {
+  __builtin___sprintf_chk(buf, 1, 2, "%d", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 4, "%#x", 9);
+  __builtin___sprintf_chk(buf, 1, 3, "%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+  __builtin___sprintf_chk(buf, 1, 4, "%p", (void *)9);
+  __builtin___sprintf_chk(buf, 1, 3, "%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+  __builtin___sprintf_chk(buf, 1, 3, "%+d", 9);
+  __builtin___sprintf_chk(buf, 1, 2, "%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 3}}
+  __builtin___sprintf_chk(buf, 1, 6, "%5d", 9);
+  __builtin___sprintf_chk(buf, 1, 5, "%5d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
+  __builtin___sprintf_chk(buf, 1, 2, "%f", 9.f);
+  __builtin___sprintf_chk(buf, 1, 1, "%f", 9.f); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+}
+
+void call_sprintf() {
+  char buf[6];
+  sprintf(buf, "1234%d", 9);
+  sprintf(buf, "12345%d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "12%#x", 9);
+  sprintf(buf, "123%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "12%p", (void *)9);
+  sprintf(buf, "123%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "123%+d", 9);
+  sprintf(buf, "1234%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "%5d", 9);
+  sprintf(buf, "1%5d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "1234%f", 9.f);
+  sprintf(buf, "12345%f", 9.f); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+}
+
 #ifdef __cplusplus
 template  struct S {
   void mf() const {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -309,13 +309,70 @@
   return false;
 }
 
+namespace {
+
+class EstimateSizeFormatHandler
+: public analyze_format_string::FormatStringHandler {
+  size_t Size;
+
+public:
+  EstimateSizeFormatHandler(const StringLiteral *fexpr)
+  : Size(fexpr->getLength() + 1 /* null byte always written by sprintf */) {
+  }
+
+  bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
+ const char *startSpecifier,
+ unsigned specifierLen) override {
+const analyze_format_string::OptionalAmount &FW = FS.getFieldWidth();
+if (FW.getHowSpecified() == analyze_format_string::OptionalAmount::Constant)
+  Size += FW.getConstantAmount();
+else
+  Size += FS.getConversionSpecifier().isAnyIntArg() ||
+  FS.getConversionSpecifier().isDoubleArg();
+Size += FS.hasPlusPrefix() || FS.hasSpacePrefix();
+if (FS.hasAlternativeForm()) {
+  switch (FS.

[PATCH] D71566: New checks for fortified sprintf

2019-12-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille marked 7 inline comments as done.
serge-sans-paille added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:370
   // FIXME: There are some more useful checks we could be doing here:
   //  - Analyze the format string of sprintf to see how much of buffer is used.
   //  - Evaluate strlen of strcpy arguments, use as object size.

erik.pilkington wrote:
> Can you delete this comment now?
I only deleted the one related to sprintf



Comment at: clang/lib/Sema/SemaChecking.cpp:392
+  EstimateSizeFormatHandler H(StrE);
+  StringRef StrRef = StrE->getString();
+  const char *Str = StrRef.data();

erik.pilkington wrote:
> Will this assert on: `sprintf(buf, L"foo");`? Not that that makes any sense, 
> but we shouldn't crash.
Still need to check that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:372
+T.print(OS, Policy);
+OS.flush();
+  }

NIT: is flush redundant? I believe it's called in destructor



Comment at: clang-tools-extra/clangd/Hover.cpp:432
+}();
+HI = getHoverContents(*Deduced, D, AST.getASTContext(), Index);
   } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) 
{

Is this the only callsite of `getHoverContents` that we are changin?
We could just move the logic that computes `D` into the `getHoverContents`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D70527: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths

2019-12-17 Thread Karl-Johan Karlsson via Phabricator via cfe-commits
Ka-Ka added a comment.

In D70527#1786718 , @rnk wrote:

> In D70527#1785552 , @ikudrin wrote:
>
> > Personally, I would prefer to see the file name and path to be changed as 
> > little as possible because that would help to recognize the files better. 
> > We cannot use `remove_dots()` on POSIX OSes to simplify paths, because it 
> > may return an invalid path; thus we have to use `getRealPath()`. If I 
> > understand it right, there is no similar problem with the file name itself.
> >
> > So, which issues this patch is going to solve?
>
>
> It seems clear to me, the filename could be an absolute symlink to a real 
> file somewhere far removed from the realpath of the parent directory. It 
> seems reasonable that -fdiagnostics-absolute-paths would look through 
> symlinks in this case.


This fix is important when building with Bazel (google build system) as Bazel 
set up a temporary sandbox (with a series of symbolic links) to be be able to 
control the dependencies and keep a valid build cache. Currently when using the 
clang option -fdiagnostics-absolute-paths it will only resolve the sybolic 
names in the dir-path of the filename which end up with warning- and 
error-messages to files inside the sandbox instead of the correct source inside 
the repo you are working in.

In D70527#1786698 , @rnk wrote:

> From auditing the call sites, it seems that almost all of them could be 
> simplified by using the new API:
>  
> http://llvm-cs.pcc.me.uk/tools/clang/lib/Basic/FileManager.cpp/rgetCanonicalName


I agree, that they can be simplified, but I guess it is easier to do those 
changes in separate patches.




Comment at: clang/include/clang/Basic/FileManager.h:226
   /// The canonical names of directories.
   llvm::DenseMap CanonicalDirNames;
 

rnk wrote:
> You could make the key `void*` and save a DenseMap here. Nobody ever iterates 
> CanonicalDirNames to look at the keys.
Sure, I will update the patch ...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70527



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


[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet created this revision.
kadircet added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay.
Herald added a project: clang.

We were traversing AST twice to get the Decl in case of sugared
types(auto, decltype). They seem to be same in practice, so this patch gets rid
of the second traversal and makes use of TagDecl inside QualType instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71597

Files:
  clang-tools-extra/clangd/Hover.cpp


Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
   OS.flush();
 
-  if (D) {
+  if (const auto *D = T->getAsTagDecl()) {
 HI.Kind = index::getSymbolInfo(D).Kind;
 enhanceFromIndex(HI, D, Index);
   }
@@ -396,12 +396,7 @@
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
 
   if (auto Deduced = getDeducedType(AST.getASTContext(), SourceLocationBeg)) {
-// Find the corresponding decl to populate kind and fetch documentation.
-DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
-auto Decls =
-targetDecl(ast_type_traits::DynTypedNode::create(*Deduced), Rel);
-HI = getHoverContents(*Deduced, Decls.empty() ? nullptr : Decls.front(),
-  AST.getASTContext(), Index);
+HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
   } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) 
{
 HI = getHoverContents(*M, AST);
   } else {


Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
   OS.flush();
 
-  if (D) {
+  if (const auto *D = T->getAsTagDecl()) {
 HI.Kind = index::getSymbolInfo(D).Kind;
 enhanceFromIndex(HI, D, Index);
   }
@@ -396,12 +396,7 @@
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
 
   if (auto Deduced = getDeducedType(AST.getASTContext(), SourceLocationBeg)) {
-// Find the corresponding decl to populate kind and fetch documentation.
-DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
-auto Decls =
-targetDecl(ast_type_traits::DynTypedNode::create(*Deduced), Rel);
-HI = getHoverContents(*Deduced, Decls.empty() ? nullptr : Decls.front(),
-  AST.getASTContext(), Index);
+HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
   } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) {
 HI = getHoverContents(*M, AST);
   } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev created this revision.
kbobyrev added reviewers: ilya-biryukov, kadircet.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman, mgrang, jkorous, MaskRay.

When asked for references during cross-file rename, index might return implicit 
references to the renamed symbol (such as those in macro expansions). To fix 
the incorrect behavior, this patch introduces basic filtering machinery which 
ensures that all ranges where renaming is about to be applied actually contains 
the identifier the user asked to rename.


https://reviews.llvm.org/D71598

Files:
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -728,6 +728,22 @@
 llvm::StringRef FooH;
 llvm::StringRef FooCC;
   } Cases[] = {
+  {
+  // Macros and implicit references.
+  R"cpp(
+class [[Fo^o]] {};
+#define FooFoo Foo
+#define FOO Foo
+  )cpp",
+  R"cpp(
+#include "foo.h"
+void bar() {
+  [[Foo]] x;
+  FOO y;
+  FooFoo z;
+}
+  )cpp",
+  },
   {
   // classes.
   R"cpp(
Index: clang-tools-extra/clangd/refactor/Rename.cpp
===
--- clang-tools-extra/clangd/refactor/Rename.cpp
+++ clang-tools-extra/clangd/refactor/Rename.cpp
@@ -14,17 +14,22 @@
 #include "Selection.h"
 #include "SourceCode.h"
 #include "index/SymbolCollector.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Tooling/Refactoring/Rename/USRFindingAction.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FormatVariadic.h"
 #include 
+#include 
+#include 
 
 namespace clang {
 namespace clangd {
@@ -340,12 +345,11 @@
 //
 // FIXME: Add range patching heuristics to detect staleness of the index, and
 // report to users.
-// FIXME: Our index may return implicit references, which are not eligible for
-// rename, we should filter out these references.
 llvm::Expected renameOutsideFile(
 const NamedDecl &RenameDecl, llvm::StringRef MainFilePath,
 llvm::StringRef NewName, const SymbolIndex &Index,
-llvm::function_ref(PathRef)> GetFileContent) {
+llvm::function_ref(PathRef)> GetFileContent,
+const SourceManager &SM) {
   auto AffectedFiles =
   findOccurrencesOutsideFile(RenameDecl, MainFilePath, Index);
   if (!AffectedFiles)
@@ -359,10 +363,16 @@
   elog("Fail to read file content: {0}", AffectedFileCode.takeError());
   continue;
 }
-auto RenameRanges =
-adjustRenameRanges(*AffectedFileCode, RenameDecl.getNameAsString(),
-   std::move(FileAndOccurrences.second),
-   RenameDecl.getASTContext().getLangOpts());
+
+// Filter out possible implicit references returned from the index.
+const auto FilteredRanges = filterRenameRanges(
+SM, FileAndOccurrences.first(),
+RenameDecl.getASTContext().getLangOpts(), *AffectedFileCode,
+std::move(FileAndOccurrences.second), RenameDecl.getNameAsString());
+
+auto RenameRanges = adjustRenameRanges(
+*AffectedFileCode, RenameDecl.getNameAsString(),
+std::move(FilteredRanges), RenameDecl.getASTContext().getLangOpts());
 if (!RenameRanges) {
   // Our heuristics fails to adjust rename ranges to the current state of
   // the file, it is most likely the index is stale, so we give up the
@@ -508,7 +518,7 @@
   if (RInputs.Index) {
 auto OtherFilesEdits =
 renameOutsideFile(*RenameDecl, RInputs.MainFilePath, RInputs.NewName,
-  *RInputs.Index, GetFileContent);
+  *RInputs.Index, GetFileContent, SM);
 if (!OtherFilesEdits)
   return OtherFilesEdits.takeError();
 Results = std::move(*OtherFilesEdits);
Index: clang-tools-extra/clangd/SourceCode.h
===
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -298,6 +298,14 @@
 bool isHeaderFile(llvm::StringRef FileName,
   llvm::Optional LangOpts = llvm::None);
 
+/// Removes ranges with implicit references to the renamed symbol (e.g. in macro
+/// expansions).
+std::vector filterRenameRanges(const SourceManager &SM,
+  Stri

[PATCH] D70527: [clang] Fix the canonicalization of paths in -fdiagnostics-absolute-paths

2019-12-17 Thread Karl-Johan Karlsson via Phabricator via cfe-commits
Ka-Ka updated this revision to Diff 234268.
Ka-Ka marked 2 inline comments as done.
Ka-Ka added a comment.

Updated patch according to review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70527

Files:
  clang/include/clang/Basic/FileManager.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Frontend/absolute-paths-symlinks.c

Index: clang/test/Frontend/absolute-paths-symlinks.c
===
--- /dev/null
+++ clang/test/Frontend/absolute-paths-symlinks.c
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: cp "%s" "test.c"
+// RUN: ln -sf "test.c" "link.c"
+// RUN: not %clang_cc1 -fsyntax-only -fdiagnostics-absolute-paths "link.c" 2>&1|FileCheck %s
+
+// Verify that -fdiagnostics-absolute-paths resolve symbolic links in
+// diagnostics messages.
+
+// CHECK: test.c
+// CHECK-SAME: error: unknown type name
+This do not compile
+
+// REQUIRES: shell
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -761,11 +761,12 @@
 }
 
 void TextDiagnostic::emitFilename(StringRef Filename, const SourceManager &SM) {
-  SmallVector AbsoluteFilename;
+#ifdef _WIN32
+  SmallString<4096> TmpFilename;
+#endif
   if (DiagOpts->AbsolutePath) {
-auto Dir = SM.getFileManager().getDirectory(
-llvm::sys::path::parent_path(Filename));
-if (Dir) {
+auto File = SM.getFileManager().getFile(Filename);
+if (File) {
   // We want to print a simplified absolute path, i. e. without "dots".
   //
   // The hardest part here are the paths like "//../".
@@ -781,16 +782,14 @@
   // on Windows we can just use llvm::sys::path::remove_dots(), because,
   // on that system, both aforementioned paths point to the same place.
 #ifdef _WIN32
-  SmallString<4096> DirName = (*Dir)->getName();
-  llvm::sys::fs::make_absolute(DirName);
-  llvm::sys::path::native(DirName);
-  llvm::sys::path::remove_dots(DirName, /* remove_dot_dot */ true);
+  TmpFilename = (*File)->getName();
+  llvm::sys::fs::make_absolute(TmpFilename);
+  llvm::sys::path::native(TmpFilename);
+  llvm::sys::path::remove_dots(TmpFilename, /* remove_dot_dot */ true);
+  Filename = StringRef(TmpFilename.data(), TmpFilename.size());
 #else
-  StringRef DirName = SM.getFileManager().getCanonicalName(*Dir);
+  Filename = SM.getFileManager().getCanonicalName(*File);
 #endif
-  llvm::sys::path::append(AbsoluteFilename, DirName,
-  llvm::sys::path::filename(Filename));
-  Filename = StringRef(AbsoluteFilename.data(), AbsoluteFilename.size());
 }
   }
 
Index: clang/lib/Basic/FileManager.cpp
===
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -549,9 +549,9 @@
 
 StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
   // FIXME: use llvm::sys::fs::canonical() when it gets implemented
-  llvm::DenseMap::iterator Known
-= CanonicalDirNames.find(Dir);
-  if (Known != CanonicalDirNames.end())
+  llvm::DenseMap::iterator Known
+= CanonicalNames.find(Dir);
+  if (Known != CanonicalNames.end())
 return Known->second;
 
   StringRef CanonicalName(Dir->getName());
@@ -560,7 +560,24 @@
   if (!FS->getRealPath(Dir->getName(), CanonicalNameBuf))
 CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
 
-  CanonicalDirNames.insert({Dir, CanonicalName});
+  CanonicalNames.insert({Dir, CanonicalName});
+  return CanonicalName;
+}
+
+StringRef FileManager::getCanonicalName(const FileEntry *File) {
+  // FIXME: use llvm::sys::fs::canonical() when it gets implemented
+  llvm::DenseMap::iterator Known
+= CanonicalNames.find(File);
+  if (Known != CanonicalNames.end())
+return Known->second;
+
+  StringRef CanonicalName(File->getName());
+
+  SmallString<4096> CanonicalNameBuf;
+  if (!FS->getRealPath(File->getName(), CanonicalNameBuf))
+CanonicalName = StringRef(CanonicalNameBuf).copy(CanonicalNameStorage);
+
+  CanonicalNames.insert({File, CanonicalName});
   return CanonicalName;
 }
 
Index: clang/include/clang/Basic/FileManager.h
===
--- clang/include/clang/Basic/FileManager.h
+++ clang/include/clang/Basic/FileManager.h
@@ -222,8 +222,8 @@
   llvm::BumpPtrAllocator>
   SeenFileEntries;
 
-  /// The canonical names of directories.
-  llvm::DenseMap CanonicalDirNames;
+  /// The canonical names of files and directories .
+  llvm::DenseMap CanonicalNames;
 
   /// Storage for canonical names that we have computed.
   llvm::BumpPtrAllocator CanonicalNameStorage;
@@ -421,6 +421,13 @@
   /// required, which

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

In D71241#1787571 , @ABataev wrote:

> In D71241#1787265 , @hfinkel wrote:
>
> > In D71241#1786959 , @jdoerfert 
> > wrote:
> >
> > > In D71241#1786530 , @ABataev 
> > > wrote:
> > >
> > > > Most probably, we can use this solution without adding a new 
> > > > expression. `DeclRefExpr` class can contain 2 decls: FoundDecl and the 
> > > > Decl being used. You can use FoundDecl to point to the original 
> > > > function and used decl to point to the function being called in this 
> > > > context. But at first, we need to be sure that we can handle all corner 
> > > > cases correctly.
> > >
> > >
> > > What new expression are you talking about?
> >
> >
> > To be clear, I believe he's talking about the new expression that I 
> > proposed we add in order to represent this kind of call. If that's not 
> > needed, and we can use the FoundDecl/Decl pair for that purpose, that 
> > should also be considered.
> >
> > > This solution already does point to both declarations, as shown here: 
> > > https://reviews.llvm.org/D71241#1782504
>
>
> Exactly. We need to check if the `MemberRefExpr` can do this too to handle 
> member functions correctly.
>  And need to wait for opinion from Richard Smith about the design. We need to 
> be sure that it won't break compatibility with C/C++ in some corner cases. 
> Design bugs are very hard to solve and I want to be sure that we don't miss 
> anything. And we provide full compatibility with both C and C++.


We do need to be careful here. For cases with FoundDecl != Decl, I think that 
the typo-correction cases look like they probably work, but there are a few 
places where we make semantic decisions based on the mismatch, such as:

In SemaTemplate.cpp below line 512, we have (this is in C++03-specific code):

  } else if (!Found.isSuppressingDiagnostics()) {
//   - if the name found is a class template, it must refer to the same
// entity as the one found in the class of the object expression,
// otherwise the program is ill-formed.
if (!Found.isSingleResult() ||
getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() !=
OuterTemplate->getCanonicalDecl()) {
  Diag(Found.getNameLoc(),
   diag::ext_nested_name_member_ref_lookup_ambiguous)

and in SemaExpr.cpp near line 2783, we have:

  // If we actually found the member through a using declaration, cast
  // down to the using declaration's type.
  //
  // Pointer equality is fine here because only one declaration of a
  // class ever has member declarations.
  if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
assert(isa(FoundDecl));
QualType URecordType = Context.getTypeDeclType(
   cast(FoundDecl->getDeclContext()));


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71572: [ItaniumCXXABI] Use linkonce_odr instead of weak_odr for tls wrappers on Windows

2019-12-17 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo marked an inline comment as done.
mstorsjo added inline comments.



Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:2519-2520
   return VarLinkage;
+  // On Windows, WeakODR is a no-op, boiling down to the same as normal 
external
+  // linkage.
+  if (CGM.getTriple().isOSWindows())

mstorsjo wrote:
> rnk wrote:
> > mstorsjo wrote:
> > > rnk wrote:
> > > > I would say that this is inaccurate. It greatly affects what the 
> > > > optimizer is allowed to do.
> > > > 
> > > > It looks like we forgot to put a comdat on these things, is that not 
> > > > the correct fix?
> > > Oh, ok.
> > > 
> > > The full case I was trying to fix (but forgot to recheck after changing 
> > > this bit) is that when used with `-ffunction-sections`, the tls wrapper 
> > > function ends up as comdat `one_only`, which then gives multiple 
> > > definition errors. So perhaps the issue is in the handling of 
> > > `-ffunction-sections` wrt weak_odr?
> > Ah, that is an interesting wrinkle. I'm surprised that things worked 
> > without -ffunction-sections, though. I would've expected the two plain 
> > external definitions to produce multiple definition errors.
> Without `-ffunction-sections`, there's no separate thread wrapper produced at 
> all, so everything else is generated with working linkage. I just haven't 
> happened to build code that forces generation of a tls wrapper without 
> `-ffunction-sections` yet.
Actually, sorry, I misremembered. Yes, I think the same issue would be present 
even without `-ffunction-sections`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71572



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


[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60932 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71597



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


[PATCH] D71572: [ItaniumCXXABI] Make tls wrappers comdat on Windows

2019-12-17 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo updated this revision to Diff 234269.
mstorsjo retitled this revision from "[ItaniumCXXABI] Use linkonce_odr instead 
of weak_odr for tls wrappers on Windows" to "[ItaniumCXXABI] Make tls wrappers 
comdat on Windows".
mstorsjo edited the summary of this revision.
mstorsjo added a reviewer: rsmith.
mstorsjo added a comment.

Changed to make it a comdat; unless I'm mistaken, neither linkonce_odr nor 
weak_odr actually make it possible to link multiple copies of the symbol on 
windows, unless it's made a comdat. With that in place, the previous change 
(weak_odr to linkonce_odr) actually isn't needed.


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

https://reviews.llvm.org/D71572

Files:
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/tls-init-funcs.cpp


Index: clang/test/CodeGenCXX/tls-init-funcs.cpp
===
--- clang/test/CodeGenCXX/tls-init-funcs.cpp
+++ clang/test/CodeGenCXX/tls-init-funcs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.8 -std=c++1y -S -emit-llvm %s 
-o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -std=c++1y -S -emit-llvm %s -o - 
| FileCheck %s --check-prefix=MINGW
 
 // CHECK: @a = internal thread_local global
 // CHECK: @_Z2vtIiE = linkonce_odr thread_local global i32 5
@@ -10,6 +11,9 @@
 // CHECK-DAG: define weak_odr hidden cxx_fast_tlscc i32* @_ZTW2vtIvE()
 // CHECK-DAG: define {{.*}} @_ZTW1a
 
+// MINGW-DAG: define weak_odr hidden i32* @_ZTW2vtIiE() {{.*}} comdat
+// MINGW-DAG: define weak_odr hidden i32* @_ZTW2vtIvE() {{.*}} comdat
+
 struct A {
   ~A();
 };
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2546,6 +2546,12 @@
   llvm::Function::Create(FnTy, getThreadLocalWrapperLinkage(VD, CGM),
  WrapperName.str(), &CGM.getModule());
 
+  // On Windows, we need to make the weak/linkonce wrapper comdat to
+  // actually handle multiple TUs defining the same wrapper.
+  if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() &&
+  Wrapper->isWeakForLinker())
+Wrapper->setComdat(CGM.getModule().getOrInsertComdat(Wrapper->getName()));
+
   CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, Wrapper);
 
   // Always resolve references to the wrapper at link time.


Index: clang/test/CodeGenCXX/tls-init-funcs.cpp
===
--- clang/test/CodeGenCXX/tls-init-funcs.cpp
+++ clang/test/CodeGenCXX/tls-init-funcs.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.8 -std=c++1y -S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-windows-gnu -std=c++1y -S -emit-llvm %s -o - | FileCheck %s --check-prefix=MINGW
 
 // CHECK: @a = internal thread_local global
 // CHECK: @_Z2vtIiE = linkonce_odr thread_local global i32 5
@@ -10,6 +11,9 @@
 // CHECK-DAG: define weak_odr hidden cxx_fast_tlscc i32* @_ZTW2vtIvE()
 // CHECK-DAG: define {{.*}} @_ZTW1a
 
+// MINGW-DAG: define weak_odr hidden i32* @_ZTW2vtIiE() {{.*}} comdat
+// MINGW-DAG: define weak_odr hidden i32* @_ZTW2vtIvE() {{.*}} comdat
+
 struct A {
   ~A();
 };
Index: clang/lib/CodeGen/ItaniumCXXABI.cpp
===
--- clang/lib/CodeGen/ItaniumCXXABI.cpp
+++ clang/lib/CodeGen/ItaniumCXXABI.cpp
@@ -2546,6 +2546,12 @@
   llvm::Function::Create(FnTy, getThreadLocalWrapperLinkage(VD, CGM),
  WrapperName.str(), &CGM.getModule());
 
+  // On Windows, we need to make the weak/linkonce wrapper comdat to
+  // actually handle multiple TUs defining the same wrapper.
+  if (CGM.getTriple().isOSWindows() && CGM.supportsCOMDAT() &&
+  Wrapper->isWeakForLinker())
+Wrapper->setComdat(CGM.getModule().getOrInsertComdat(Wrapper->getName()));
+
   CGM.SetLLVMFunctionAttributes(GlobalDecl(), FI, Wrapper);
 
   // Always resolve references to the wrapper at link time.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

2019-12-17 Thread Jaydeep Chauhan via Phabricator via cfe-commits
Jac1494 updated this revision to Diff 234273.
Jac1494 added a comment.

Separate clang patch with test cases .


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

https://reviews.llvm.org/D71451

Files:
  clang/include/clang/Basic/TargetInfo.h
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGen/debug-info-extern-variable-basic.c
  clang/test/CodeGen/debug-info-extern-variable-unused.c


Index: clang/test/CodeGen/debug-info-extern-variable-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-unused.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "i"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "j"
Index: clang/test/CodeGen/debug-info-extern-variable-basic.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-basic.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return i+j;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "i"
+// CHECK: distinct !DIGlobalVariable(name: "j"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (D->hasAttr())
 return;
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1390,7 +1390,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.


Index: clang/test/CodeGen/debug-info-extern-variable-unused.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-unused.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return 0;
+}
+
+// CHECK-NOT: distinct !DIGlobalVariable(name: "i"
+// CHECK-NOT: distinct !DIGlobalVariable(name: "j"
Index: clang/test/CodeGen/debug-info-extern-variable-basic.c
===
--- /dev/null
+++ clang/test/CodeGen/debug-info-extern-variable-basic.c
@@ -0,0 +1,10 @@
+// RUN: %clang -emit-llvm -S -g -fstandalone-debug %s -o - | FileCheck %s
+
+extern int i;
+int foo() {
+  extern int j;
+  return i+j;
+}
+
+// CHECK: distinct !DIGlobalVariable(name: "i"
+// CHECK: distinct !DIGlobalVariable(name: "j"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -4594,6 +4594,8 @@
   assert(DebugKind >= codegenoptions::LimitedDebugInfo);
   if (D->hasAttr())
 return;
+  if (!(DebugKind == clang::codegenoptions::FullDebugInfo))
+return;
 
   auto Align = getDeclAlignIfRequired(D, CGM.getContext());
   llvm::DIFile *Unit = getOrCreateFile(D->getLocation());
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -1390,7 +1390,7 @@
   virtual void setAuxTarget(const TargetInfo *Aux) {}
 
   /// Whether target allows debuginfo types for decl only variables.
-  virtual bool allowDebugInfoForExternalVar() const { return false; }
+  virtual bool allowDebugInfoForExternalVar() const { return true; }
 
 protected:
   /// Copy type and layout related info.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71600: PowerPC 32-bit - forces 8 byte lock/lock_free decisions at compiled time

2019-12-17 Thread Alfredo Dal'Ava Júnior via Phabricator via cfe-commits
adalava created this revision.
Herald added subscribers: llvm-commits, Sanitizers, cfe-commits, steven.zhang, 
shchenz, jsji, jfb, krytarowski, arichardson, nemanjai, emaste.
Herald added a reviewer: jfb.
Herald added projects: clang, Sanitizers, LLVM.
adalava added a reviewer: dim.
Herald added subscribers: wuzish, dexonsmith.

FreeBSD on powerpc 32 bit is moving to LLVM, so  I enabled atomic.c on this 
platform and found it was emiting calls to make decisions about lock/lock free 
at runtime. 
We know in advance 8 byte (64 bits) operations need lock and there's no plans 
to create something like a "libatomic", so I creates this patch. This is being 
currently in use on our experimental builds/ISOs, so I ask for your inputs. 
Thanks!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71600

Files:
  clang/lib/AST/ExprConstant.cpp
  compiler-rt/lib/builtins/atomic.c


Index: compiler-rt/lib/builtins/atomic.c
===
--- compiler-rt/lib/builtins/atomic.c
+++ compiler-rt/lib/builtins/atomic.c
@@ -119,13 +119,20 @@
   return locks + (hash & SPINLOCK_MASK);
 }
 
-/// Macros for determining whether a size is lock free.  Clang can not yet
-/// codegen __atomic_is_lock_free(16), so for now we assume 16-byte values are
-/// not lock free.
+/// Macros for determining whether a size is lock free.
 #define IS_LOCK_FREE_1 __c11_atomic_is_lock_free(1)
 #define IS_LOCK_FREE_2 __c11_atomic_is_lock_free(2)
 #define IS_LOCK_FREE_4 __c11_atomic_is_lock_free(4)
+
+/// 32 bit PowerPC doesn't support 8-byte lock_free atomics
+#if !defined(__powerpc64__) && defined(__powerpc__)
+#define IS_LOCK_FREE_8 0
+#else
 #define IS_LOCK_FREE_8 __c11_atomic_is_lock_free(8)
+#endif
+
+/// Clang can not yet codegen __atomic_is_lock_free(16), so for now we assume
+/// 16-byte values are not lock free.
 #define IS_LOCK_FREE_16 0
 
 /// Macro that calls the compiler-generated lock-free versions of functions
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11023,6 +11023,13 @@
   }
 }
 
+// Avoid emiting call for runtime decision on PowerPC 32-bit
+// The lock free possibilities on this platform are covered by the lines 
+// above and we know in advance other cases require lock
+if (Info.Ctx.getTargetInfo().getTriple().getArch() == llvm::Triple::ppc) {
+return Success(0, E);
+}
+
 return BuiltinOp == Builtin::BI__atomic_always_lock_free ?
 Success(0, E) : Error(E);
   }


Index: compiler-rt/lib/builtins/atomic.c
===
--- compiler-rt/lib/builtins/atomic.c
+++ compiler-rt/lib/builtins/atomic.c
@@ -119,13 +119,20 @@
   return locks + (hash & SPINLOCK_MASK);
 }
 
-/// Macros for determining whether a size is lock free.  Clang can not yet
-/// codegen __atomic_is_lock_free(16), so for now we assume 16-byte values are
-/// not lock free.
+/// Macros for determining whether a size is lock free.
 #define IS_LOCK_FREE_1 __c11_atomic_is_lock_free(1)
 #define IS_LOCK_FREE_2 __c11_atomic_is_lock_free(2)
 #define IS_LOCK_FREE_4 __c11_atomic_is_lock_free(4)
+
+/// 32 bit PowerPC doesn't support 8-byte lock_free atomics
+#if !defined(__powerpc64__) && defined(__powerpc__)
+#define IS_LOCK_FREE_8 0
+#else
 #define IS_LOCK_FREE_8 __c11_atomic_is_lock_free(8)
+#endif
+
+/// Clang can not yet codegen __atomic_is_lock_free(16), so for now we assume
+/// 16-byte values are not lock free.
 #define IS_LOCK_FREE_16 0
 
 /// Macro that calls the compiler-generated lock-free versions of functions
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11023,6 +11023,13 @@
   }
 }
 
+// Avoid emiting call for runtime decision on PowerPC 32-bit
+// The lock free possibilities on this platform are covered by the lines 
+// above and we know in advance other cases require lock
+if (Info.Ctx.getTargetInfo().getTriple().getArch() == llvm::Triple::ppc) {
+return Success(0, E);
+}
+
 return BuiltinOp == Builtin::BI__atomic_always_lock_free ?
 Success(0, E) : Error(E);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71476: [OpenCL] Add builtin function extension handling

2019-12-17 Thread Sven van Haastregt via Phabricator via cfe-commits
svenvh marked 2 inline comments as done.
svenvh added inline comments.



Comment at: clang/lib/Sema/OpenCLBuiltins.td:51
+// Extension associated to a builtin function.
+class FunctionExtension : AbstractExtension<_Ext>;
+

Anastasia wrote:
> Are we planning to add type extensions too? If not it might not be worth 
> creating extra abstractions. :)
Yes, type extensions are planned.



Comment at: clang/lib/Sema/OpenCLBuiltins.td:1095
 // OpenCL v2.0 s9.17.3: Additions to section 6.13.1: Work-Item Functions
+def FuncExtKhrSubgroups : FunctionExtension<"cl_khr_subgroups">;
 let MinVersion = CL20 in {

Nicola wrote:
> Should this be moved together with the other extensions defined above? It 
> might make the file easier to navigate if all the extensions are in the same 
> place (or all close to where they are used)
I'll move it to the top for now.


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

https://reviews.llvm.org/D71476



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


[PATCH] D71451: Support to emit extern variables debuginfo with "-fstandalone-debug"

2019-12-17 Thread Jaydeep Chauhan via Phabricator via cfe-commits
Jac1494 added a comment.

In D71451#1786517 , @dblaikie wrote:

> Do you have any particular users/use case for this?


A case when shared library built without debug  info 
and executable with debug info. And while debugging we want to know the types 
of extern.


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

https://reviews.llvm.org/D71451



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


[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Thomas Preud'homme via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGddd0bb8dba2a: [lit] Remove lit's REQUIRES-ANY directive 
(authored by thopre).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408

Files:
  clang/test/Driver/XRay/xray-instrument-macos.c
  clang/test/Driver/XRay/xray-instrument-os.c
  clang/test/Driver/XRay/xray-instrumentation-bundles-flags.cpp
  clang/test/Driver/XRay/xray-mode-flags.cpp
  clang/test/Driver/XRay/xray-nolinkdeps.cpp
  compiler-rt/test/builtins/Unit/arm/aeabi_cdcmpeq_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_cdcmple_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_cfcmpeq_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_cfcmple_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_drsub_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_frsub_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_idivmod_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c
  compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
  compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
  llvm/utils/lit/lit/TestRunner.py
  llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
  llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt

Index: llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt
===
--- llvm/utils/lit/tests/Inputs/shtest-format/requires-any-present.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-RUN: true
-REQUIRES-ANY: a-missing-feature, a-present-feature
Index: llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
===
--- llvm/utils/lit/tests/Inputs/shtest-format/requires-any-missing.txt
+++ /dev/null
@@ -1,2 +0,0 @@
-RUN: true
-REQUIRES-ANY: a-missing-feature, a-missing-feature-2
Index: llvm/utils/lit/lit/TestRunner.py
===
--- llvm/utils/lit/lit/TestRunner.py
+++ llvm/utils/lit/lit/TestRunner.py
@@ -1304,20 +1304,6 @@
 BooleanExpression.evaluate(s, [])
 return output
 
-@staticmethod
-def _handleRequiresAny(line_number, line, output):
-"""A custom parser to transform REQUIRES-ANY: into REQUIRES:"""
-
-# Extract the conditions specified in REQUIRES-ANY: as written.
-conditions = []
-IntegratedTestKeywordParser._handleList(line_number, line, conditions)
-
-# Output a `REQUIRES: a || b || c` expression in its place.
-expression = ' || '.join(conditions)
-IntegratedTestKeywordParser._handleBooleanExpr(line_number,
-   expression, output)
-return output
-
 def parseIntegratedTestScript(test, additional_parsers=[],
   require_script=True):
 """parseIntegratedTestScript - Scan an LLVM/Clang style integrated test
@@ -1341,9 +1327,6 @@
 initial_value=test.xfails),
 IntegratedTestKeywordParser('REQUIRES:', ParserKind.BOOLEAN_EXPR,
 initial_value=test.requires),
-IntegratedTestKeywordParser('REQUIRES-ANY:', ParserKind.CUSTOM,
-IntegratedTestKeywordParser._handleRequiresAny, 
-initial_value=test.requires), 
 IntegratedTestKeywordParser('UNSUPPORTED:', ParserKind.BOOLEAN_EXPR,
 initial_value=test.unsupported),
 IntegratedTestKeywordParser('END.', ParserKind.TAG)
Index: compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
===
--- compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
+++ compiler-rt/test/builtins/Unit/riscv/mulsi3_test.c
@@ -1,4 +1,4 @@
-// REQUIRES-ANY: riscv32-target-arch
+// REQUIRES: riscv32-target-arch
 // RUN: %clang_builtins %s %librt -o %t && %run %t
 //===-- mulsi3_test.c - Test __mulsi3 -===//
 //
Index: compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
===
--- compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
+++ compiler-rt/test/builtins/Unit/arm/aeabi_uldivmod_test.c
@@ -1,4 +1,4 @@
-// REQUIRES-ANY: arm-target-arch,armv6m-target-arch
+// REQUIRES: arm-target-arch || armv6m-target-arch
 // RUN: %clang_builtins %s %librt -o %t && %run %t
 //===-- aeabi_uldivmod_test.c - Test aeabi_uldivmod ---===//
 //
Index: compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c
===
--- compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c
+++ compiler-rt/test/builtins/Unit/arm/aeabi_uidivmod_test.c
@@ -1,4 +1,4 @@
-// REQUIRES-ANY: arm-target-arch,armv6m-target-arch
+// REQUIRES: arm-targ

[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

2019-12-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

@ilya-biryukov thanks for taking it during my OOO, just a drive-by comment :)




Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:86
+// Add macro references.
+for (const auto &IDToRefs : MacroRefsToIndex->MacroRefs) {
+  for (const auto &Range : IDToRefs.second) {

ilya-biryukov wrote:
> This is trying to emulate existing logic in `SymbolCollector::finish`. Is 
> there a way we could share this?
> Would avoid creating extra copies of reference slabs and allow to keep the 
> code in one place, rather than scattering it between `FileIndex.cpp` and 
> `SymbolCollector.cpp`. Would also allow to keep `toURI` private, meaning we 
> don't have to worry about naming it and the fact it's exposed in the public 
> interface.
> 
> One potential way to do this is to have an alternative version of 
> `handleMacroOccurence`, which would fill `SymbolCollector::MacroRefs` 
> directly and call this right after `indexTopLevelDecls`.
> Would that work?
+1 on the concern about performance (this is a hot function, we are paying an 
extra cost of copying all refs, which should be avoided), and the layering of 
`toURI`.

> One potential way to do this is to have an alternative version of 
> handleMacroOccurence, which would fill SymbolCollector::MacroRefs directly 
> and call this right after indexTopLevelDecls.
> Would that work?

the main problem is that at this point (parsing is finished), preprocessor 
callback is not available, so we won't see macro references 
(`SymbolCollector::handleMacroOccurence` will only receive macro definitions).

I think two options:
- as mentioned above, add alternative version of `handleMacroOccurence` in 
SymbolCollector, calling it **before** `indexTopLevelDecls` (because `finish` 
is called in `indexTopLevelDecls` which we have no way to customize);
- or we could add a finish callback to the `SymbolCollector` which is called in 
`SymbolCollector::finish`, and put this logic to the callback.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71406



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


[PATCH] D71566: New checks for fortified sprintf

2019-12-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille marked 2 inline comments as done.
serge-sans-paille added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:392
+  EstimateSizeFormatHandler H(StrE);
+  StringRef StrRef = StrE->getString();
+  const char *Str = StrRef.data();

serge-sans-paille wrote:
> erik.pilkington wrote:
> > Will this assert on: `sprintf(buf, L"foo");`? Not that that makes any 
> > sense, but we shouldn't crash.
> Still need to check that.
Checked and fixed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566



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


[PATCH] D71566: New checks for fortified sprintf

2019-12-17 Thread serge via Phabricator via cfe-commits
serge-sans-paille updated this revision to Diff 234282.
serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

- Prevent assert upon wide string format
- More test cases


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71566

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/warn-fortify-source.c

Index: clang/test/Sema/warn-fortify-source.c
===
--- clang/test/Sema/warn-fortify-source.c
+++ clang/test/Sema/warn-fortify-source.c
@@ -11,6 +11,8 @@
 extern "C" {
 #endif
 
+extern int sprintf(char *str, const char *format, ...);
+
 #if defined(USE_PASS_OBJECT_SIZE)
 void *memcpy(void *dst, const void *src, size_t c);
 static void *memcpy(void *dst __attribute__((pass_object_size(1))), const void *src, size_t c) __attribute__((overloadable)) __asm__("merp");
@@ -96,6 +98,41 @@
   __builtin_vsnprintf(buf, 11, "merp", list); // expected-warning {{'vsnprintf' size argument is too large; destination buffer has size 10, but size argument is 11}}
 }
 
+void call_sprintf_chk(char *buf) {
+  __builtin___sprintf_chk(buf, 1, 6, "hello");
+  __builtin___sprintf_chk(buf, 1, 5, "hello"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
+  __builtin___sprintf_chk(buf, 1, 2, "%d", 9);
+  __builtin___sprintf_chk(buf, 1, 1, "%d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+  __builtin___sprintf_chk(buf, 1, 4, "%#x", 9);
+  __builtin___sprintf_chk(buf, 1, 3, "%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+  __builtin___sprintf_chk(buf, 1, 4, "%p", (void *)9);
+  __builtin___sprintf_chk(buf, 1, 3, "%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 3, but format string expands to at least 4}}
+  __builtin___sprintf_chk(buf, 1, 3, "%+d", 9);
+  __builtin___sprintf_chk(buf, 1, 2, "%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 2, but format string expands to at least 3}}
+  __builtin___sprintf_chk(buf, 1, 6, "%5d", 9);
+  __builtin___sprintf_chk(buf, 1, 5, "%5d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 5, but format string expands to at least 6}}
+  __builtin___sprintf_chk(buf, 1, 2, "%f", 9.f);
+  __builtin___sprintf_chk(buf, 1, 1, "%f", 9.f); // expected-warning {{'sprintf' will always overflow; destination buffer has size 1, but format string expands to at least 2}}
+}
+
+void call_sprintf() {
+  char buf[6];
+  sprintf(buf, "hello");
+  sprintf(buf, "hello!"); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "1234%d", 9);
+  sprintf(buf, "12345%d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "12%#x", 9);
+  sprintf(buf, "123%#x", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "12%p", (void *)9);
+  sprintf(buf, "123%p", (void *)9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "123%+d", 9);
+  sprintf(buf, "1234%+d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "%5d", 9);
+  sprintf(buf, "1%5d", 9); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+  sprintf(buf, "1234%f", 9.f);
+  sprintf(buf, "12345%f", 9.f); // expected-warning {{'sprintf' will always overflow; destination buffer has size 6, but format string expands to at least 7}}
+}
+
 #ifdef __cplusplus
 template  struct S {
   void mf() const {
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -309,13 +309,70 @@
   return false;
 }
 
+namespace {
+
+class EstimateSizeFormatHandler
+: public analyze_format_string::FormatStringHandler {
+  size_t Size;
+
+public:
+  EstimateSizeFormatHandler(const StringLiteral *fexpr)
+  : Size(fexpr->getLength() + 1 /* null byte always written by sprintf */) {
+  }
+
+  bool HandlePrintfSpecifier(const analyze_printf::PrintfSpecifier &FS,
+ const char *startSpecifier,
+ unsigned specifierLen) override {
+const analyze_format_string::OptionalAmount &FW =

[clang] 4becf68 - [ASTImporter] Friend class decl should not be visible in its context

2019-12-17 Thread Gabor Marton via cfe-commits

Author: Gabor Marton
Date: 2019-12-17T14:48:55+01:00
New Revision: 4becf68c6f17fe143539ceac954b21175914e1c1

URL: 
https://github.com/llvm/llvm-project/commit/4becf68c6f17fe143539ceac954b21175914e1c1
DIFF: 
https://github.com/llvm/llvm-project/commit/4becf68c6f17fe143539ceac954b21175914e1c1.diff

LOG: [ASTImporter] Friend class decl should not be visible in its context

Summary:
In the past we had to use DeclContext::makeDeclVisibleInContext to make
friend declarations available for subsequent lookup calls and this way
we could chain (redecl) the structurally equivalent decls.
By doing this we created an AST that improperly made declarations
visible in some contexts, so the AST was malformed.
Since we use the importer specific lookup this is no longer necessary,
because with that we can find every previous nodes.

Reviewers: balazske, a_sidorin, a.sidorin, shafik

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, teemperor, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 414092f33c47..336ee88f994a 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -298,6 +298,45 @@ namespace clang {
   return nullptr;
 }
 
+void addDeclToContexts(Decl *FromD, Decl *ToD) {
+  if (Importer.isMinimalImport()) {
+// In minimal import case the decl must be added even if it is not
+// contained in original context, for LLDB compatibility.
+// FIXME: Check if a better solution is possible.
+if (!FromD->getDescribedTemplate() &&
+FromD->getFriendObjectKind() == Decl::FOK_None)
+  ToD->getLexicalDeclContext()->addDeclInternal(ToD);
+return;
+  }
+
+  DeclContext *FromDC = FromD->getDeclContext();
+  DeclContext *FromLexicalDC = FromD->getLexicalDeclContext();
+  DeclContext *ToDC = ToD->getDeclContext();
+  DeclContext *ToLexicalDC = ToD->getLexicalDeclContext();
+
+  bool Visible = false;
+  if (FromDC->containsDeclAndLoad(FromD)) {
+ToDC->addDeclInternal(ToD);
+Visible = true;
+  }
+  if (ToDC != ToLexicalDC && FromLexicalDC->containsDeclAndLoad(FromD)) {
+ToLexicalDC->addDeclInternal(ToD);
+Visible = true;
+  }
+
+  // If the Decl was added to any context, it was made already visible.
+  // Otherwise it is still possible that it should be visible.
+  if (!Visible) {
+if (auto *FromNamed = dyn_cast(FromD)) {
+  auto *ToNamed = cast(ToD);
+  DeclContextLookupResult FromLookup =
+  FromDC->lookup(FromNamed->getDeclName());
+  if (llvm::is_contained(FromLookup, FromNamed))
+ToDC->makeDeclVisibleInContext(ToNamed);
+}
+  }
+}
+
   public:
 explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
@@ -2737,11 +2776,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl 
*D) {
 D2 = D2CXX;
 D2->setAccess(D->getAccess());
 D2->setLexicalDeclContext(LexicalDC);
-if (!DCXX->getDescribedClassTemplate() || DCXX->isImplicit())
-  LexicalDC->addDeclInternal(D2);
-
-if (LexicalDC != DC && D->isInIdentifierNamespace(Decl::IDNS_TagFriend))
-  DC->makeDeclVisibleInContext(D2);
+addDeclToContexts(D, D2);
 
 if (ClassTemplateDecl *FromDescribed =
 DCXX->getDescribedClassTemplate()) {
@@ -2807,7 +2842,7 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl 
*D) {
 Name.getAsIdentifierInfo(), PrevDecl))
   return D2;
 D2->setLexicalDeclContext(LexicalDC);
-LexicalDC->addDeclInternal(D2);
+addDeclToContexts(D, D2);
   }
 
   if (auto BraceRangeOrErr = import(D->getBraceRange()))
@@ -3386,23 +3421,7 @@ ExpectedDecl 
ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
   if (Error Err = ImportTemplateInformation(D, ToFunction))
 return std::move(Err);
 
-  bool IsFriend = D->isInIdentifierNamespace(Decl::IDNS_OrdinaryFriend);
-
-  // TODO Can we generalize this approach to other AST nodes as well?
-  if (D->getDeclContext()->containsDeclAndLoad(D))
-DC->addDeclInternal(ToFunction);
-  if (DC != LexicalDC && D->getLexicalDeclContext()->containsDeclAndLoad(D))
-LexicalDC->addDeclInternal(ToFunction);
-
-  // Friend declaration's lexical context is the befriending class, but the
-  // semantic context is the enclosing scope of the befriending class.
-  // We want the friend functions to be found in the semantic context by 
lookup.
-  // FIXME should we handle this generically in VisitFriendDecl?
-  // In Other cases when LexicalDC != DC we don't want it to be added,
-  // e.g out-of-class definitions like void B::f() {} .
-  if (LexicalDC != DC 

[PATCH] D71020: [ASTImporter] Friend class decl should not be visible in its context

2019-12-17 Thread Gabor Marton via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4becf68c6f17: [ASTImporter] Friend class decl should not be 
visible in its context (authored by martong).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71020

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -247,6 +247,11 @@
   return cast(ET->getNamedType().getTypePtr())->getDecl();
 }
 
+static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
+  QualType Ty = FD->getFriendType()->getType().getCanonicalType();
+  return cast(Ty)->getDecl();
+}
+
 struct ImportExpr : TestImportBase {};
 struct ImportType : TestImportBase {};
 struct ImportDecl : TestImportBase {};
@@ -2707,7 +2712,7 @@
   EXPECT_FALSE(To0->isInIdentifierNamespace(Decl::IDNS_Ordinary));
 }
 
-TEST_P(ImportFriendFunctions, DISABLED_LookupWithProtoAfter) {
+TEST_P(ImportFriendFunctions, LookupWithProtoAfter) {
   auto FunctionPattern = functionDecl(hasName("f"));
   auto ClassPattern = cxxRecordDecl(hasName("X"));
 
@@ -3778,6 +3783,44 @@
   EXPECT_TRUE(MatchVerifier{}.match(ToD, Pattern));
 }
 
+TEST_P(ImportFriendClasses, UndeclaredFriendClassShouldNotBeVisible) {
+  Decl *FromTu = getTuDecl("class X { friend class Y; };", Lang_CXX, "from.cc");
+  auto *FromX = FirstDeclMatcher().match(
+  FromTu, cxxRecordDecl(hasName("X")));
+  auto *FromFriend = FirstDeclMatcher().match(FromTu, friendDecl());
+  RecordDecl *FromRecordOfFriend =
+  const_cast(getRecordDeclOfFriend(FromFriend));
+
+  ASSERT_EQ(FromRecordOfFriend->getDeclContext(), cast(FromTu));
+  ASSERT_EQ(FromRecordOfFriend->getLexicalDeclContext(),
+cast(FromX));
+  ASSERT_FALSE(
+  FromRecordOfFriend->getDeclContext()->containsDecl(FromRecordOfFriend));
+  ASSERT_FALSE(FromRecordOfFriend->getLexicalDeclContext()->containsDecl(
+  FromRecordOfFriend));
+  ASSERT_FALSE(FromRecordOfFriend->getLookupParent()
+   ->lookup(FromRecordOfFriend->getDeclName())
+   .empty());
+
+  auto *ToX = Import(FromX, Lang_CXX);
+  ASSERT_TRUE(ToX);
+
+  Decl *ToTu = ToX->getTranslationUnitDecl();
+  auto *ToFriend = FirstDeclMatcher().match(ToTu, friendDecl());
+  RecordDecl *ToRecordOfFriend =
+  const_cast(getRecordDeclOfFriend(ToFriend));
+
+  ASSERT_EQ(ToRecordOfFriend->getDeclContext(), cast(ToTu));
+  ASSERT_EQ(ToRecordOfFriend->getLexicalDeclContext(), cast(ToX));
+  EXPECT_FALSE(
+  ToRecordOfFriend->getDeclContext()->containsDecl(ToRecordOfFriend));
+  EXPECT_FALSE(ToRecordOfFriend->getLexicalDeclContext()->containsDecl(
+  ToRecordOfFriend));
+  EXPECT_FALSE(ToRecordOfFriend->getLookupParent()
+   ->lookup(ToRecordOfFriend->getDeclName())
+   .empty());
+}
+
 TEST_P(ImportFriendClasses, ImportOfRecursiveFriendClassTemplate) {
   Decl *FromTu = getTuDecl(
   R"(
@@ -4477,11 +4520,6 @@
   ASSERT_EQ(Res.size(), 0u);
 }
 
-static const RecordDecl *getRecordDeclOfFriend(FriendDecl *FD) {
-  QualType Ty = FD->getFriendType()->getType().getCanonicalType();
-  return cast(Ty)->getDecl();
-}
-
 TEST_P(ASTImporterLookupTableTest,
LookupFindsFwdFriendClassDeclWithElaboratedType) {
   TranslationUnitDecl *ToTU = getToTuDecl(
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -298,6 +298,45 @@
   return nullptr;
 }
 
+void addDeclToContexts(Decl *FromD, Decl *ToD) {
+  if (Importer.isMinimalImport()) {
+// In minimal import case the decl must be added even if it is not
+// contained in original context, for LLDB compatibility.
+// FIXME: Check if a better solution is possible.
+if (!FromD->getDescribedTemplate() &&
+FromD->getFriendObjectKind() == Decl::FOK_None)
+  ToD->getLexicalDeclContext()->addDeclInternal(ToD);
+return;
+  }
+
+  DeclContext *FromDC = FromD->getDeclContext();
+  DeclContext *FromLexicalDC = FromD->getLexicalDeclContext();
+  DeclContext *ToDC = ToD->getDeclContext();
+  DeclContext *ToLexicalDC = ToD->getLexicalDeclContext();
+
+  bool Visible = false;
+  if (FromDC->containsDeclAndLoad(FromD)) {
+ToDC->addDeclInternal(ToD);
+Visible = true;
+  }
+  if (ToDC != ToLexicalDC && FromLexicalDC->containsDeclAndLoad(FromD)) {
+ToLexicalDC->addDeclInternal(ToD);
+Visible = true;
+  }
+
+  // If the Decl was added to any context, it was made already visible.
+  // Otherwise it is still possible that it should be visible.
+  if (!Visible) {
+if (auto *FromNamed = dyn_cast(

[PATCH] D68913: Adds fixit hints to the Wrange-loop-analysis

2019-12-17 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Mordante, do you need someone to land this for you?


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

https://reviews.llvm.org/D68913



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


[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Haojian Wu via Phabricator via cfe-commits
hokein added a comment.

(apologies, the FIXME may imply this approach...)

this approach is based on an assumption: the index results are matched to the 
latest file content, but this is not always true in practice, our index maybe 
stale (index results came from an old snapshot of the file), then this approach 
will fail.

I think we should do it in another direction:

- add a new `RefKind` (something like implicit references, or named references) 
to `clangd::Ref`
- when querying the index for rename, we set a corresponding `Filter` in the 
query request (or filter out non-interesting references based on the `RefKind` 
afterwards)


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

https://reviews.llvm.org/D71598



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


[PATCH] D71556: [AArch64][SVE] Implement intrinsic for non-faulting loads

2019-12-17 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added a comment.

In D71556#1786465 , @efriedma wrote:

> I'm not sure it's legal to transform a non-faulting load to a load with a 
> non-faulting flag?  At least, we'd need to consider the implications of that 
> very carefully.  In particular, I'm concerned about the interaction with 
> intrinsics that read/write FFR.  I mean, you could specify that loads marked 
> MONonFaulting actually write to the FFR register, but that seems confusing.
>
> It seems simpler to preserve the intrinsic until isel, at least for now.


I missed this comment earlier, but that's a valid point. For SVE having 
side-effects is assumed from the non-faulting flag. We hoped to latch on to the 
MLOAD here to reuse code and benefit from legalization in case we want to add a 
more generic mechanism in the future to use such loads directly in the 
loop-vectorizer.

Perhaps we can clarify the intent that the non-faulting mode may have 
side-effects by renaming the flag to something like 
`NonFaultingWithSideEffects`? Otherwise we can stick with the intrinsics as you 
suggest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71556



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


[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment.

In D71598#1787806 , @hokein wrote:

> (apologies, the FIXME may imply this approach...)
>
> this approach is based on an assumption: the index results are matched to the 
> latest file content, but this is not always true in practice, our index maybe 
> stale (index results came from an old snapshot of the file), then this 
> approach will fail.
>
> I think we should do it in another direction:
>
> - add a new `RefKind` (something like implicit references, or named 
> references) to `clangd::Ref`
> - when querying the index for rename, we set a corresponding `Filter` in the 
> query request (or filter out non-interesting references based on the 
> `RefKind` aftjerwards)


I think this approach would also fail for stale index, wouldn't it?

I can totally understand why that might be slightly better for performance, but 
if we have no guarantees that our index is aware of implicit references and 
would not be able to mark those in the first place, this implementation would 
shield us from that.

Anyway, I am happy to learn more about why the proposed approach might be 
better, but I do not fully understand the concern here.


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

https://reviews.llvm.org/D71598



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


[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Kirill Bobyrev via Phabricator via cfe-commits
kbobyrev added a comment.

(also, this should eliminate unwanted changes caused by the index being stale, 
wouldn't it?)


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

https://reviews.llvm.org/D71598



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:353
 /// Generate a \p Hover object given the type \p T.
 HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > kadircet wrote:
> > > > ilya-biryukov wrote:
> > > > > Not related to this patch, but what is `D` here? Is this getting 
> > > > > hover contents for a type or for a decl?
> > > > it represents the deduced decl for Type, if any.
> > > What is a "deduced decl for Type"?
> > it was referring to the tagdecl referred by `decltype`s, i am not sure if 
> > there are cases in which this can be different than `T->getAsTagDecl`.
> > Always making use of `T->getAsTagDecl` doesn't seem to be causing any test 
> > failures.
> if `T->getAsTagDecl()` is non-null, why does `D` being passed to the function 
> is null?
> How can they be different?
see D71597


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234287.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Address comments
- Get rid of deduced decl in decltype/auto case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -104,7 +104,7 @@
   }}
   )cpp",
[](HoverInfo &HI) {
- HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.NamespaceScope = "ns1::";
  HI.LocalScope = "(anonymous struct)::";
  HI.Name = "bar";
  HI.Kind = index::SymbolKind::Field;
@@ -362,7 +362,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class (lambda)";
+ HI.Name = "(lambda)";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on template instantiation
@@ -373,7 +373,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on specialized template
@@ -385,7 +385,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
 
@@ -524,6 +524,44 @@
  HI.NamespaceScope = "";
  HI.LocalScope = "boom::";
}},
+  {
+  R"cpp(// Should not print inline or anon namespaces.
+  namespace ns {
+inline namespace in_ns {
+  namespace a {
+namespace {
+  namespace b {
+inline namespace in_ns2 {
+  class Foo {};
+} // in_ns2
+  } // b
+} // anon
+  } // a
+} // in_ns
+  } // ns
+  void foo() {
+ns::a::b::[[F^oo]] x;
+(void)x;
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns::a::b::";
+HI.Definition = "class Foo {}";
+  }},
+  {
+  R"cpp(
+  template  class Foo {};
+  class X;
+  void foo() {
+[[^auto]] x = Foo();
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+  }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
@@ -895,7 +933,7 @@
   [](HoverInfo &HI) {
 HI.Name = "foo";
 HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "ns::(anonymous)::";
+HI.NamespaceScope = "ns::";
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
@@ -1173,7 +1211,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "class std::initializer_list";
+HI.Name = "initializer_list";
 HI.Kind = index::SymbolKind::Class;
   }},
   {
@@ -1231,7 +1269,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1242,7 +1280,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1253,7 +1291,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1265,7 +1303,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1277,7 +1315,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1289,7 +1327,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1300,7 +1338,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar"

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234288.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "AST.h"
 #include "Annotations.h"
 #include "Hover.h"
 #include "TestIndex.h"
@@ -1271,6 +1272,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto function return with trailing type";
   }},
   {
   R"cpp(// trailing return type
@@ -1282,6 +1284,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "trailing return type";
   }},
   {
   R"cpp(// auto in function return
@@ -1293,6 +1296,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto in function return";
   }},
   {
   R"cpp(// auto& in function return
@@ -1305,6 +1309,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto& in function return";
   }},
   {
   R"cpp(// auto* in function return
@@ -1317,6 +1322,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto* in function return";
   }},
   {
   R"cpp(// const auto& in function return
@@ -1329,6 +1335,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "const auto& in function return";
   }},
   {
   R"cpp(// decltype(auto) in function return
@@ -1340,6 +1347,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "decltype(auto) in function return";
   }},
   {
   R"cpp(// decltype(auto) reference in function return
@@ -1404,6 +1412,8 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation =
+"decltype of function with trailing return type.";
   }},
   {
   R"cpp(// decltype of var with decltype.
@@ -1477,6 +1487,87 @@
   }
 }
 
+TEST(Hover, DocsFromIndex) {
+  Annotations T(R"cpp(
+  template  class X {};
+  void foo() {
+au^to t = X();
+X^ w;
+(void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  Symbol IndexSym;
+  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.Documentation = "comment from index";
+  SymbolSlab::Builder Symbols;
+  Symbols.insert(IndexSym);
+  auto Index =
+  MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
+
+  for (const auto &P : T.points()) {
+auto H = getHover(AST, P, format::getLLVMStyle(), Index.get());
+ASSERT_TRUE(H);
+EXPECT_EQ(H->Documentation, IndexSym.Documentation);
+  }
+}
+
+TEST(Hover, DocsFromAST) {
+  Annotations T(R"cpp(
+  // doc
+  template  class X {};
+  void foo() {
+au^to t = X();
+X^ w;
+(void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (const auto &P : T.points()) {
+auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
+ASSERT_TRUE(H);
+EXPECT_EQ(H->Documentation, "doc");
+  }
+}
+
+TEST(Hover, DocsFromMostSpecial) {
+  Annotations T(R"cpp(
+  // doc1
+  template  class X {};
+  // doc2
+  template <> class X {};
+  // doc3
+  template  class X {};
+  void foo() {
+X$doc1^();
+X$doc2^();
+X$doc3^();
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (auto Comment : {"doc1", "doc2", "doc3"}) {
+for (const auto &P : T.points(Comment)) {
+  auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
+  ASSERT_TRUE(H);
+  EXPECT_EQ(H->Documentation, Comment);
+}
+  }
+

[PATCH] D71607: Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

2019-12-17 Thread Jeffrey Sorensen via Phabricator via cfe-commits
sorenj created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Many C++ programmers are unaware that an expression of unsigned - signed will
promote the signed argument to unsigned, and the resulting underflow produces
a large positive rather than negative result. Hence the frequent errors
related to the test x.size() - 1 <= 0 when the container x is empty.

This clang tidy detects signed values being subtracted from unsigned values
and warns the user about the potential error. It is not perfect as it is
not always possible at compile time to reason about code when this comparison
is made.

The warning also suggests a fixit change that will append a "u" to numerical
constants - this makes the implicit cast explicit and signals that the
developer knew what they were doing in a subtraction. In other cases it
suggests the rather abhorrent static_cast<>().

The easiest fix is to not do subtraction at all, just move the operation
to the other side of the comparison where it becomes an addition - which
has none of these surprising properties.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71607

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s bugprone-unsigned-subtraction %t
+
+template 
+class vector {
+ public:
+  unsigned size();
+  bool empty();
+};
+
+#define MACRO_MINUS(x) x - 5
+#define MACRO_INT 20
+
+void signedSubtracted() {
+  unsigned x = 5;
+  int  = 2;
+  if (x -  == 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+// CHECK-FIXES: if (x - static_cast() == 0) {
+return;
+  }
+  if (0 >= x - ) {
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: signed value subtracted from
+// CHECK-FIXES: if (0 >= x - static_cast()) {
+return;
+  }
+  unsigned z = MACRO_MINUS(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: signed value subtracted from
+  z = x - MACRO_INT;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+  // CHECK-FIXES: z = x - static_cast(MACRO_INT);
+}
+
+void signedConstantSubtracted() {
+  unsigned x = 5;
+  if (x - 2 > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+// CHECK-FIXES: if (x - 2u > 0) {
+return;
+  }
+}
+
+void casesThatShouldBeIgnored() {
+  unsigned x = 5;
+  // If the constant used in subtraction is already explicitly unsigned, do not
+  // warn. This is not safe, but the user presumably knows what they are doing.
+  if (x - 2u > 0u) {
+return;
+  }
+  if (x - 2u > 0) {
+return;
+  }
+  // sizeof operators are strictly positive for all types, and a constexpr, so
+  // any underflow would happen at compile time, so do not warn.
+  x = sizeof(long double) - 1;
+  // If both sides of the subtraction are compile time constants, don't warn.
+  if (5u - 2 > 0) {
+return;
+  }
+  constexpr long y = 4;  // NOLINT(runtime/int)
+  if (y - 4 > 0) {
+return;
+  }
+}
+
+// If the first argument of the subtraction is an expression that was previously
+// used in a comparison, the user is presumed to have done something smart.
+// This isn't perfect, but it greatly reduces false alarms.
+void contextMatters() {
+  unsigned x = 5;
+  if (x < 5) return;
+  if (x - 2 > 0u) {
+return;
+  }
+}
+
+// For loops with a compartor meet the previously described case, and therefore
+// do not warn if the variable is used in a subtraction. Again not perfect, but
+// this code is complex to reason about and it's best not to generate false
+// alarms.
+unsigned forLoops() {
+  unsigned x;
+  for (unsigned i = 1; i < 5; ++i) {
+x += i - 1;
+  }
+  return x;
+}
+
+// Testing for an empty container before subtracting from size() is considered
+// to be the same as comparing against the size - it's still possible to fail
+// but reduces the number of false positives.
+void containersEmpty() {
+  vector x;
+  if (!x.empty()) {
+if (x.size() - 1 > 0) {
+  return;
+}
+  }
+  vector y;
+  if (y.size() - 1 > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: signed value subtracted from
+// CHECK-FIXES: if (y.size() - 1u > 0) {
+return;
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.h
@@ -0,0 +1,38 @@
+//===--- UnsignedSubtractionCheck.h - clang-tidy ---

[PATCH] D71545: [clangd] Improve hover for auto on template instantiations

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet abandoned this revision.
kadircet marked an inline comment as done.
kadircet added a comment.

integrated into D71543 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71545



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


[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:353
 
-  if (D) {
+  if (const auto *D = T->getAsTagDecl()) {
 HI.Kind = index::getSymbolInfo(D).Kind;

This might be a functional change in case of typedefs.

Could you check on these cases (not sure if we already have those in the tests):
```
typedef int int_type;
^auto x = int_type();

struct cls {};
typedef cls cls_type;
^auto y = cls_type();

template 
struct templ {};
^auto z = templ();
```

Note that I don't necessarily think they **will** change behavior, but they 
**might**.

Also happy to LGTM as is if you remove "NFC" from the description. It seems 
we're potentially changing behvior, but it's not tested currently and we can 
land this, discover and fix any regreessions later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71597



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


[PATCH] D71544: [clangd] Improve printing of lambda names

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet abandoned this revision.
kadircet added a comment.

Integrated into D71543 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71544



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


[PATCH] D71406: [clangd] Add xref for macros to FileIndex.

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:86
+// Add macro references.
+for (const auto &IDToRefs : MacroRefsToIndex->MacroRefs) {
+  for (const auto &Range : IDToRefs.second) {

hokein wrote:
> ilya-biryukov wrote:
> > This is trying to emulate existing logic in `SymbolCollector::finish`. Is 
> > there a way we could share this?
> > Would avoid creating extra copies of reference slabs and allow to keep the 
> > code in one place, rather than scattering it between `FileIndex.cpp` and 
> > `SymbolCollector.cpp`. Would also allow to keep `toURI` private, meaning we 
> > don't have to worry about naming it and the fact it's exposed in the public 
> > interface.
> > 
> > One potential way to do this is to have an alternative version of 
> > `handleMacroOccurence`, which would fill `SymbolCollector::MacroRefs` 
> > directly and call this right after `indexTopLevelDecls`.
> > Would that work?
> +1 on the concern about performance (this is a hot function, we are paying an 
> extra cost of copying all refs, which should be avoided), and the layering of 
> `toURI`.
> 
> > One potential way to do this is to have an alternative version of 
> > handleMacroOccurence, which would fill SymbolCollector::MacroRefs directly 
> > and call this right after indexTopLevelDecls.
> > Would that work?
> 
> the main problem is that at this point (parsing is finished), preprocessor 
> callback is not available, so we won't see macro references 
> (`SymbolCollector::handleMacroOccurence` will only receive macro definitions).
> 
> I think two options:
> - as mentioned above, add alternative version of `handleMacroOccurence` in 
> SymbolCollector, calling it **before** `indexTopLevelDecls` (because `finish` 
> is called in `indexTopLevelDecls` which we have no way to customize);
> - or we could add a finish callback to the `SymbolCollector` which is called 
> in `SymbolCollector::finish`, and put this logic to the callback.
> 
Thanks, good point, in my proposal we should populate macro references before 
calling `indexTopLevelDecls`.

I would still suggest going with the first option, it seems simpler to me. But 
up to you, @usaxena95.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71406



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


[PATCH] D71598: [clangd] Filter implicit references from index while renaming

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

If we go with the solution proposed by @hokein, it looks like using the current 
patch is an improvement of what we have now.
One big issue with the adding a new ref kind/ref modifier is that it requires 
modifications to Kythe-based index implementation, something that cannot be 
done as easily as landing this patch.

WDYT about landing something similar to this patch for now and discussing the 
possibilities of fixing it by storing enough information in the references 
later?


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

https://reviews.llvm.org/D71598



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


[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234291.
kadircet marked an inline comment as done.
kadircet added a comment.

- Added alias tests, behavior stayed the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71597

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1396,6 +1396,32 @@
 HI.Definition = "struct Test &&test = {}";
 HI.Value = "{}";
   }},
+  {
+  R"cpp(// auto on alias
+  typedef int int_type;
+  ^[[auto]] x = int_type();
+  )cpp",
+  [](HoverInfo &HI) { HI.Name = "int"; }},
+  {
+  R"cpp(// auto on alias
+  struct cls {};
+  typedef cls cls_type;
+  ^[[auto]] y = cls_type();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct cls";
+HI.Kind = index::SymbolKind::Struct;
+  }},
+  {
+  R"cpp(// auto on alias
+  template 
+  struct templ {};
+  ^[[auto]] z = templ();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct templ";
+HI.Kind = index::SymbolKind::Struct;
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
   OS.flush();
 
-  if (D) {
+  if (const auto *D = T->getAsTagDecl()) {
 HI.Kind = index::getSymbolInfo(D).Kind;
 enhanceFromIndex(HI, D, Index);
   }
@@ -396,12 +396,7 @@
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
 
   if (auto Deduced = getDeducedType(AST.getASTContext(), SourceLocationBeg)) {
-// Find the corresponding decl to populate kind and fetch documentation.
-DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
-auto Decls =
-targetDecl(ast_type_traits::DynTypedNode::create(*Deduced), Rel);
-HI = getHoverContents(*Deduced, Decls.empty() ? nullptr : Decls.front(),
-  AST.getASTContext(), Index);
+HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
   } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) 
{
 HI = getHoverContents(*M, AST);
   } else {


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1396,6 +1396,32 @@
 HI.Definition = "struct Test &&test = {}";
 HI.Value = "{}";
   }},
+  {
+  R"cpp(// auto on alias
+  typedef int int_type;
+  ^[[auto]] x = int_type();
+  )cpp",
+  [](HoverInfo &HI) { HI.Name = "int"; }},
+  {
+  R"cpp(// auto on alias
+  struct cls {};
+  typedef cls cls_type;
+  ^[[auto]] y = cls_type();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct cls";
+HI.Kind = index::SymbolKind::Struct;
+  }},
+  {
+  R"cpp(// auto on alias
+  template 
+  struct templ {};
+  ^[[auto]] z = templ();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct templ";
+HI.Kind = index::SymbolKind::Struct;
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = print

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60980 tests passed, 1 failed 
and 727 were skipped.

  failed: lit.lit/shtest-format.py

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596



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


[PATCH] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

2019-12-17 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added a comment.

Please add documentation and mention new check in Release Notes.




Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.cpp:10
+#include "UnsignedSubtractionCheck.h"
+
+#include "../utils/Matchers.h"

Unnecessary empty line.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.cpp:137
+}
+}  // namespace bugprone
+}  // namespace tidy

Please separate with empty line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71607



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


[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234293.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -6,6 +6,7 @@
 //
 //===--===//
 
+#include "AST.h"
 #include "Annotations.h"
 #include "Hover.h"
 #include "TestIndex.h"
@@ -1271,6 +1272,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto function return with trailing type";
   }},
   {
   R"cpp(// trailing return type
@@ -1282,6 +1284,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "trailing return type";
   }},
   {
   R"cpp(// auto in function return
@@ -1293,6 +1296,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto in function return";
   }},
   {
   R"cpp(// auto& in function return
@@ -1305,6 +1309,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto& in function return";
   }},
   {
   R"cpp(// auto* in function return
@@ -1317,6 +1322,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto* in function return";
   }},
   {
   R"cpp(// const auto& in function return
@@ -1329,6 +1335,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "const auto& in function return";
   }},
   {
   R"cpp(// decltype(auto) in function return
@@ -1340,6 +1347,7 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "decltype(auto) in function return";
   }},
   {
   R"cpp(// decltype(auto) reference in function return
@@ -1404,6 +1412,8 @@
   [](HoverInfo &HI) {
 HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation =
+"decltype of function with trailing return type.";
   }},
   {
   R"cpp(// decltype of var with decltype.
@@ -1449,6 +1459,7 @@
   [](HoverInfo &HI) {
 HI.Name = "cls";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto on alias";
   }},
   {
   R"cpp(// auto on alias
@@ -1459,6 +1470,7 @@
   [](HoverInfo &HI) {
 HI.Name = "templ";
 HI.Kind = index::SymbolKind::Struct;
+HI.Documentation = "auto on alias";
   }},
   };
 
@@ -1503,6 +1515,87 @@
   }
 }
 
+TEST(Hover, DocsFromIndex) {
+  Annotations T(R"cpp(
+  template  class X {};
+  void foo() {
+au^to t = X();
+X^ w;
+(void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  Symbol IndexSym;
+  IndexSym.ID = *getSymbolID(&findDecl(AST, "X"));
+  IndexSym.Documentation = "comment from index";
+  SymbolSlab::Builder Symbols;
+  Symbols.insert(IndexSym);
+  auto Index =
+  MemIndex::build(std::move(Symbols).build(), RefSlab(), RelationSlab());
+
+  for (const auto &P : T.points()) {
+auto H = getHover(AST, P, format::getLLVMStyle(), Index.get());
+ASSERT_TRUE(H);
+EXPECT_EQ(H->Documentation, IndexSym.Documentation);
+  }
+}
+
+TEST(Hover, DocsFromAST) {
+  Annotations T(R"cpp(
+  // doc
+  template  class X {};
+  void foo() {
+au^to t = X();
+X^ w;
+(void)w;
+  })cpp");
+
+  TestTU TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  for (const auto &D : AST.getDiagnostics())
+ADD_FAILURE() << D;
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (const auto &P : T.points()) {
+auto H = getHover(AST, P, format::getLLVMStyle(), nullptr);
+ASSERT_TRUE(H);
+EXPECT_EQ(H->Documentation, "doc");
+  }
+}
+
+TEST(Hover, DocsFromMostSpecial) {
+  Annotations T(R"cpp(
+  // doc1
+  template  class X {};
+  // doc2
+  template <> class X {};
+  // doc3
+  template  class X {};
+  void foo() {
+X$doc1^();
+X$doc2^();
+X$doc3^();
+

[clang] 1ed832e - Reland [NFC-I] Remove hack for fp-classification builtins

2019-12-17 Thread Erich Keane via cfe-commits

Author: Erich Keane
Date: 2019-12-17T06:58:29-08:00
New Revision: 1ed832e42446ef8c4afe08f980db2e54ac316bf3

URL: 
https://github.com/llvm/llvm-project/commit/1ed832e42446ef8c4afe08f980db2e54ac316bf3
DIFF: 
https://github.com/llvm/llvm-project/commit/1ed832e42446ef8c4afe08f980db2e54ac316bf3.diff

LOG: Reland [NFC-I] Remove hack for fp-classification builtins

The FP-classification builtins (__builtin_isfinite, etc) use variadic
packs in the definition file to mean an overload set.  Because of that,
floats were converted to doubles, which is incorrect. There WAS a patch
to remove the cast after the fact.

THis patch switches these builtins to just be custom type checking,
calls the implicit conversions for the integer members, and makes sure
the correct L->R casts are put into place, then does type checking like
normal.

A future direction (that wouldn't be NFC) would consider making
conversions for the floating point parameter legal.

Note: The initial patch for this missed that certain systems need to
still convert half to float, since they dont' support that type.

Added: 
clang/test/Sema/builtin-fpclassification.c

Modified: 
clang/include/clang/Basic/Builtins.def
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/builtin_float.c
clang/test/Sema/crash-invalid-builtin.c

Removed: 




diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 29dec78e4b96..51d3500df8ae 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -399,15 +399,15 @@ BUILTIN(__builtin_islessgreater , "i.", "Fnct")
 BUILTIN(__builtin_isunordered   , "i.", "Fnct")
 
 // Unary FP classification
-BUILTIN(__builtin_fpclassify, "ii.", "Fnc")
-BUILTIN(__builtin_isfinite,   "i.", "Fnc")
-BUILTIN(__builtin_isinf,  "i.", "Fnc")
-BUILTIN(__builtin_isinf_sign, "i.", "Fnc")
-BUILTIN(__builtin_isnan,  "i.", "Fnc")
-BUILTIN(__builtin_isnormal,   "i.", "Fnc")
+BUILTIN(__builtin_fpclassify, "ii.", "Fnct")
+BUILTIN(__builtin_isfinite,   "i.", "Fnct")
+BUILTIN(__builtin_isinf,  "i.", "Fnct")
+BUILTIN(__builtin_isinf_sign, "i.", "Fnct")
+BUILTIN(__builtin_isnan,  "i.", "Fnct")
+BUILTIN(__builtin_isnormal,   "i.", "Fnct")
 
 // FP signbit builtins
-BUILTIN(__builtin_signbit, "i.", "Fnc")
+BUILTIN(__builtin_signbit, "i.", "Fnct")
 BUILTIN(__builtin_signbitf, "if", "Fnc")
 BUILTIN(__builtin_signbitl, "iLd", "Fnc")
 

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 910afefffb6d..cc091b27fe56 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5797,51 +5797,41 @@ bool Sema::SemaBuiltinFPClassification(CallExpr 
*TheCall, unsigned NumArgs) {
<< SourceRange(TheCall->getArg(NumArgs)->getBeginLoc(),
   (*(TheCall->arg_end() - 1))->getEndLoc());
 
+  // __builtin_fpclassify is the only case where NumArgs != 1, so we can count
+  // on all preceding parameters just being int.  Try all of those.
+  for (unsigned i = 0; i < NumArgs - 1; ++i) {
+Expr *Arg = TheCall->getArg(i);
+
+if (Arg->isTypeDependent())
+  return false;
+
+ExprResult Res = PerformImplicitConversion(Arg, Context.IntTy, AA_Passing);
+
+if (Res.isInvalid())
+  return true;
+TheCall->setArg(i, Res.get());
+  }
+
   Expr *OrigArg = TheCall->getArg(NumArgs-1);
 
   if (OrigArg->isTypeDependent())
 return false;
 
+  // Usual Unary Conversions will convert half to float, which we want for
+  // machines that use fp16 conversion intrinsics. Else, we wnat to leave the
+  // type how it is, but do normal L->Rvalue conversions.
+  if (Context.getTargetInfo().useFP16ConversionIntrinsics())
+OrigArg = UsualUnaryConversions(OrigArg).get();
+  else
+OrigArg = DefaultFunctionArrayLvalueConversion(OrigArg).get();
+  TheCall->setArg(NumArgs - 1, OrigArg);
+
   // This operation requires a non-_Complex floating-point number.
   if (!OrigArg->getType()->isRealFloatingType())
 return Diag(OrigArg->getBeginLoc(),
 diag::err_typecheck_call_invalid_unary_fp)
<< OrigArg->getType() << OrigArg->getSourceRange();
 
-  // If this is an implicit conversion from float -> float, double, or
-  // long double, or half -> half, float, double, or long double, remove it.
-  if (ImplicitCastExpr *Cast = dyn_cast(OrigArg)) {
-// Only remove standard FloatCasts, leaving other casts inplace
-if (Cast->getCastKind() == CK_FloatingCast) {
-  bool IgnoreCast = false;
-  Expr *CastArg = Cast->getSubExpr();
-  if (CastArg->getType()->isSpecificBuiltinType(BuiltinType::Float)) {
-assert(
-(Cast->getType()->isSpecificBuiltinType(BuiltinType::Double) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::Float) ||
- Cast->getType()->isSpecificBuiltinType(BuiltinType::LongDouble)) 
&&
-  

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 234292.
kadircet added a comment.

- Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -104,7 +104,7 @@
   }}
   )cpp",
[](HoverInfo &HI) {
- HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.NamespaceScope = "ns1::";
  HI.LocalScope = "(anonymous struct)::";
  HI.Name = "bar";
  HI.Kind = index::SymbolKind::Field;
@@ -362,7 +362,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class (lambda)";
+ HI.Name = "(lambda)";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on template instantiation
@@ -373,7 +373,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on specialized template
@@ -385,7 +385,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
 
@@ -524,6 +524,44 @@
  HI.NamespaceScope = "";
  HI.LocalScope = "boom::";
}},
+  {
+  R"cpp(// Should not print inline or anon namespaces.
+  namespace ns {
+inline namespace in_ns {
+  namespace a {
+namespace {
+  namespace b {
+inline namespace in_ns2 {
+  class Foo {};
+} // in_ns2
+  } // b
+} // anon
+  } // a
+} // in_ns
+  } // ns
+  void foo() {
+ns::a::b::[[F^oo]] x;
+(void)x;
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns::a::b::";
+HI.Definition = "class Foo {}";
+  }},
+  {
+  R"cpp(
+  template  class Foo {};
+  class X;
+  void foo() {
+[[^auto]] x = Foo();
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+  }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
@@ -895,7 +933,7 @@
   [](HoverInfo &HI) {
 HI.Name = "foo";
 HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "ns::(anonymous)::";
+HI.NamespaceScope = "ns::";
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
@@ -1173,7 +1211,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "class std::initializer_list";
+HI.Name = "initializer_list";
 HI.Kind = index::SymbolKind::Class;
   }},
   {
@@ -1231,7 +1269,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1242,7 +1280,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1253,7 +1291,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1265,7 +1303,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1277,7 +1315,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1289,7 +1327,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1300,7 +1338,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1364,7 +1402,7 @@
   

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

Hal, are we going to support something like this?

  void cpu() { asm("nop"); }
  
  #pragma omp declare variant(cpu) match(device = {kind(cpu)})
  void wrong_asm() {
asm ("xxx");
  }

Currently, there is an error when we try to emit the assembler output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon check-circle color=green} Unit tests: pass. 60932 tests passed, 0 failed 
and 726 were skipped.

{icon check-circle color=green} clang-tidy: pass.

{icon check-circle color=green} clang-format: pass.

Build artifacts 
: 
diff.json 
,
 clang-tidy.txt 
,
 clang-format.patch 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71597



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


[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71597



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


[PATCH] D71607: [clang-tidy] Add unsigned subtraction warning, with suggestion to convert to unsigned literals.

2019-12-17 Thread Jeffrey Sorensen via Phabricator via cfe-commits
sorenj updated this revision to Diff 234294.
sorenj added a comment.

Address requested whitespace changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71607

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.h
  clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-unsigned-subtraction.cpp
@@ -0,0 +1,104 @@
+// RUN: %check_clang_tidy %s bugprone-unsigned-subtraction %t
+
+template 
+class vector {
+ public:
+  unsigned size();
+  bool empty();
+};
+
+#define MACRO_MINUS(x) x - 5
+#define MACRO_INT 20
+
+void signedSubtracted() {
+  unsigned x = 5;
+  int  = 2;
+  if (x -  == 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+// CHECK-FIXES: if (x - static_cast() == 0) {
+return;
+  }
+  if (0 >= x - ) {
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: signed value subtracted from
+// CHECK-FIXES: if (0 >= x - static_cast()) {
+return;
+  }
+  unsigned z = MACRO_MINUS(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: signed value subtracted from
+  z = x - MACRO_INT;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+  // CHECK-FIXES: z = x - static_cast(MACRO_INT);
+}
+
+void signedConstantSubtracted() {
+  unsigned x = 5;
+  if (x - 2 > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: signed value subtracted from
+// CHECK-FIXES: if (x - 2u > 0) {
+return;
+  }
+}
+
+void casesThatShouldBeIgnored() {
+  unsigned x = 5;
+  // If the constant used in subtraction is already explicitly unsigned, do not
+  // warn. This is not safe, but the user presumably knows what they are doing.
+  if (x - 2u > 0u) {
+return;
+  }
+  if (x - 2u > 0) {
+return;
+  }
+  // sizeof operators are strictly positive for all types, and a constexpr, so
+  // any underflow would happen at compile time, so do not warn.
+  x = sizeof(long double) - 1;
+  // If both sides of the subtraction are compile time constants, don't warn.
+  if (5u - 2 > 0) {
+return;
+  }
+  constexpr long y = 4;  // NOLINT(runtime/int)
+  if (y - 4 > 0) {
+return;
+  }
+}
+
+// If the first argument of the subtraction is an expression that was previously
+// used in a comparison, the user is presumed to have done something smart.
+// This isn't perfect, but it greatly reduces false alarms.
+void contextMatters() {
+  unsigned x = 5;
+  if (x < 5) return;
+  if (x - 2 > 0u) {
+return;
+  }
+}
+
+// For loops with a compartor meet the previously described case, and therefore
+// do not warn if the variable is used in a subtraction. Again not perfect, but
+// this code is complex to reason about and it's best not to generate false
+// alarms.
+unsigned forLoops() {
+  unsigned x;
+  for (unsigned i = 1; i < 5; ++i) {
+x += i - 1;
+  }
+  return x;
+}
+
+// Testing for an empty container before subtracting from size() is considered
+// to be the same as comparing against the size - it's still possible to fail
+// but reduces the number of false positives.
+void containersEmpty() {
+  vector x;
+  if (!x.empty()) {
+if (x.size() - 1 > 0) {
+  return;
+}
+  }
+  vector y;
+  if (y.size() - 1 > 0) {
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: signed value subtracted from
+// CHECK-FIXES: if (y.size() - 1u > 0) {
+return;
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/UnsignedSubtractionCheck.h
@@ -0,0 +1,38 @@
+//===--- UnsignedSubtractionCheck.h - clang-tidy *- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSIGNED_SUBTRACTION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSIGNED_SUBTRACTION_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Finds cases where a signed value is substracted from an unsigned value,
+/// a likely cause of unexpected underflow.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-unsigned-subtraction.html
+class UnsignedSubtractionCheck : public ClangTidyCheck {
+ public:
+  UnsignedSubtractionCheck(String

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60977 tests passed, 1 failed 
and 727 were skipped.

  failed: lit.lit/shtest-format.py

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60978 tests passed, 1 failed 
and 727 were skipped.

  failed: lit.lit/shtest-format.py

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543



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


[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread pre-merge checks [bot] via Phabricator via cfe-commits
merge_guards_bot added a comment.

{icon times-circle color=red} Unit tests: fail. 60981 tests passed, 1 failed 
and 727 were skipped.

  failed: lit.lit/shtest-format.py

{icon question-circle color=gray} clang-tidy: unknown.

{icon question-circle color=gray} clang-format: unknown.

Build artifacts 
: 
diff.json 
,
 CMakeCache.txt 
,
 console-log.txt 
,
 test-results.xml 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596



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


[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

Here is more proper fix. We don't need to capture just `k` here, instead, we 
need to capture the whole expression.
Linear clause has a little bit different processing rather than all other 
clauses caused by a non-perfect design. Add codegen tests for all combined 
constructs that may require capturing of the linear step expression. Probably, 
some additional work in codegen may be required.

  diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
  index afe0f1a..cb55ace 100644
  --- a/clang/lib/Sema/SemaOpenMP.cpp
  +++ b/clang/lib/Sema/SemaOpenMP.cpp
  @@ -3830,6 +3830,9 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S,
 MarkDeclarationsReferencedInExpr(E);
   }
 }
  +  if (auto *LC = dyn_cast(Clause))
  +if (Expr *E = LC->getStep())
  +  MarkDeclarationsReferencedInExpr(E);
 DSAStack->setForceVarCapturing(/*V=*/false);
   } else if (CaptureRegions.size() > 1 ||
  CaptureRegions.back() != OMPD_unknown) {
  @@ -11396,12 +11399,87 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
 llvm_unreachable("Unknown OpenMP directive");
   }
   break;
  +  case OMPC_linear:
  +switch (DKind) {
  +case OMPD_taskloop_simd:
  +case OMPD_master_taskloop_simd:
  +case OMPD_parallel_master_taskloop_simd:
  +  CaptureRegion = OMPD_taskloop;
  +  break;
  +case OMPD_target_simd:
  +  CaptureRegion = OMPD_target;
  +  break;
  +case OMPD_target_teams_distribute_simd:
  +case OMPD_teams_distribute_simd:
  +  CaptureRegion = OMPD_teams;
  +  break;
  +case OMPD_target_parallel_for:
  +case OMPD_target_parallel_for_simd:
  +case OMPD_target_teams_distribute_parallel_for:
  +case OMPD_target_teams_distribute_parallel_for_simd:
  +case OMPD_teams_distribute_parallel_for:
  +case OMPD_teams_distribute_parallel_for_simd:
  +case OMPD_distribute_parallel_for:
  +case OMPD_distribute_parallel_for_simd:
  +case OMPD_parallel_for:
  +case OMPD_parallel_for_simd:
  +  CaptureRegion = OMPD_parallel;
  +  break;
  +case OMPD_parallel_master_taskloop:
  +case OMPD_task:
  +case OMPD_taskloop:
  +case OMPD_master_taskloop:
  +case OMPD_target_update:
  +case OMPD_target_enter_data:
  +case OMPD_target_exit_data:
  +case OMPD_target:
  +case OMPD_target_teams:
  +case OMPD_target_parallel:
  +case OMPD_target_teams_distribute:
  +case OMPD_target_data:
  +case OMPD_teams:
  +case OMPD_teams_distribute:
  +case OMPD_cancel:
  +case OMPD_parallel:
  +case OMPD_parallel_master:
  +case OMPD_parallel_sections:
  +case OMPD_threadprivate:
  +case OMPD_allocate:
  +case OMPD_taskyield:
  +case OMPD_barrier:
  +case OMPD_taskwait:
  +case OMPD_cancellation_point:
  +case OMPD_flush:
  +case OMPD_declare_reduction:
  +case OMPD_declare_mapper:
  +case OMPD_declare_simd:
  +case OMPD_declare_variant:
  +case OMPD_declare_target:
  +case OMPD_end_declare_target:
  +case OMPD_simd:
  +case OMPD_for:
  +case OMPD_for_simd:
  +case OMPD_sections:
  +case OMPD_section:
  +case OMPD_single:
  +case OMPD_master:
  +case OMPD_critical:
  +case OMPD_taskgroup:
  +case OMPD_distribute:
  +case OMPD_ordered:
  +case OMPD_atomic:
  +case OMPD_distribute_simd:
  +case OMPD_requires:
  +  llvm_unreachable("Unexpected OpenMP directive with linear-clause");
  +case OMPD_unknown:
  +  llvm_unreachable("Unknown OpenMP directive");
  +}
  +break;
 case OMPC_firstprivate:
 case OMPC_lastprivate:
 case OMPC_reduction:
 case OMPC_task_reduction:
 case OMPC_in_reduction:
  -  case OMPC_linear:
 case OMPC_default:
 case OMPC_proc_bind:
 case OMPC_safelen:
  @@ -14377,6 +14455,16 @@ OMPClause *Sema::ActOnOpenMPLinearClause(
   if (Val.isInvalid())
 return nullptr;
   StepExpr = Val.get();
  +OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
  +OpenMPDirectiveKind CaptureRegion =
  +getOpenMPCaptureRegionForClause(DKind, OMPC_linear, LangOpts.OpenMP);
  +if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
  +  StepExpr = MakeFullExpr(StepExpr).get();
  +  llvm::MapVector Captures;
  +  StepExpr = tryBuildCapture(*this, StepExpr, Captures).get();
  +  for (const auto &Pair : Captures)
  +ExprCaptures.push_back(Pair.second->getDecl());
  +}
  
   // Build var to save the step value.
   VarDecl *SaveVar =




Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128
+   (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate ||
+   Data.Attributes == OMPC_linear)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstpriva

[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:4918
 const OMPMappableExprListSizeTy &Sizes)
-  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, &MapperQualifierLoc,
-  &MapperIdInfo),
+  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+  &MapperQualifierLoc, &MapperIdInfo),

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > Do we really need to set `HasMapper` to `true` unconditionally here and 
> > > > in other places?
> > > For `map`, `to`, and `from` clauses, they can have mappers, so it's set 
> > > to `true`. For `is_device_ptr` and `use_device_ptr`, it's set to `false`.
> > So, it completely depends on the clause kind, right? If so, can we just 
> > remove it and rely on the clause kind?
> This is used in `OMPMappableExprListClause` which is inherited by all these 
> clauses. Do you mean to check the template type there to get the clause kind?
I see. I think it is better to rename it to `ClauseSupportsMapper` (or 
something like this) and make it `const`. Currently, it is confusing. I thought 
that this flag shows the use of the mapper in the clause.


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

https://reviews.llvm.org/D67833



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


[clang-tools-extra] 3d15605 - [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2019-12-17T16:33:22+01:00
New Revision: 3d15605358ebadcbd7740a61c92b4fea4d6241e5

URL: 
https://github.com/llvm/llvm-project/commit/3d15605358ebadcbd7740a61c92b4fea4d6241e5
DIFF: 
https://github.com/llvm/llvm-project/commit/3d15605358ebadcbd7740a61c92b4fea4d6241e5.diff

LOG: [clangd][NFC] Make use of TagDecl inside type for hover on auto

Summary:
We were traversing AST twice to get the Decl in case of sugared
types(auto, decltype). They seem to be same in practice, so this patch gets rid
of the second traversal and makes use of TagDecl inside QualType instead.

Reviewers: ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index f55a9c3d1f67..af43af8fcc32 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@ HoverInfo getHoverContents(const Decl *D, const 
SymbolIndex *Index) {
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
   OS.flush();
 
-  if (D) {
+  if (const auto *D = T->getAsTagDecl()) {
 HI.Kind = index::getSymbolInfo(D).Kind;
 enhanceFromIndex(HI, D, Index);
   }
@@ -396,12 +396,7 @@ llvm::Optional getHover(ParsedAST &AST, 
Position Pos,
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
 
   if (auto Deduced = getDeducedType(AST.getASTContext(), SourceLocationBeg)) {
-// Find the corresponding decl to populate kind and fetch documentation.
-DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
-auto Decls =
-targetDecl(ast_type_traits::DynTypedNode::create(*Deduced), Rel);
-HI = getHoverContents(*Deduced, Decls.empty() ? nullptr : Decls.front(),
-  AST.getASTContext(), Index);
+HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
   } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) 
{
 HI = getHoverContents(*M, AST);
   } else {

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index 783599ad7ac0..aa44d3bda72f 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1396,6 +1396,32 @@ TEST(Hover, All) {
 HI.Definition = "struct Test &&test = {}";
 HI.Value = "{}";
   }},
+  {
+  R"cpp(// auto on alias
+  typedef int int_type;
+  ^[[auto]] x = int_type();
+  )cpp",
+  [](HoverInfo &HI) { HI.Name = "int"; }},
+  {
+  R"cpp(// auto on alias
+  struct cls {};
+  typedef cls cls_type;
+  ^[[auto]] y = cls_type();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct cls";
+HI.Kind = index::SymbolKind::Struct;
+  }},
+  {
+  R"cpp(// auto on alias
+  template 
+  struct templ {};
+  ^[[auto]] z = templ();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct templ";
+HI.Kind = index::SymbolKind::Struct;
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.



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


[clang-tools-extra] 9ab15f3 - [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2019-12-17T16:33:22+01:00
New Revision: 9ab15f303efdfc841f475918535ab4e13960491b

URL: 
https://github.com/llvm/llvm-project/commit/9ab15f303efdfc841f475918535ab4e13960491b
DIFF: 
https://github.com/llvm/llvm-project/commit/9ab15f303efdfc841f475918535ab4e13960491b.diff

LOG: [clangd] Fix handling of inline/anon namespaces and names of deduced types 
in hover

Summary:
Clangd normally skips inline and anon namespaces while printing nested name
specifiers. It also drops any tag specifiers since we make use of 
`HoverInfo::Kind`
instead of some text in `HoverInfo::Name`

There was a bug causing us to print innermost inline/anon namespace, this patch
fixes that by skipping those.
Also changes printing and kind detection of deduced types to be similar to decl
case.

Also improves printing for lambdas, currently clangd prints lambdas as
`(anonymous class)`, we can improve it by at least printing `(lambda)`
instead.

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

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/AST.cpp 
b/clang-tools-extra/clangd/AST.cpp
index 6aa02a7454dc..d06245a53977 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -12,6 +12,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/NestedNameSpecifier.h"
@@ -222,8 +223,11 @@ std::string printName(const ASTContext &Ctx, const 
NamedDecl &ND) {
 // Come up with a presentation for an anonymous entity.
 if (isa(ND))
   return "(anonymous namespace)";
-if (auto *Cls = llvm::dyn_cast(&ND))
+if (auto *Cls = llvm::dyn_cast(&ND)) {
+  if (Cls->isLambda())
+return "(lambda)";
   return ("(anonymous " + Cls->getKindName() + ")").str();
+}
 if (isa(ND))
   return "(anonymous enum)";
 return "(anonymous)";

diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index af43af8fcc32..623aa962b85a 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -19,9 +19,13 @@
 
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/Support/Casting.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace clang {
@@ -72,12 +76,17 @@ std::string getLocalScope(const Decl *D) {
 std::string getNamespaceScope(const Decl *D) {
   const DeclContext *DC = D->getDeclContext();
 
-  if (const TypeDecl *TD = dyn_cast(DC))
+  if (const TagDecl *TD = dyn_cast(DC))
 return getNamespaceScope(TD);
   if (const FunctionDecl *FD = dyn_cast(DC))
 return getNamespaceScope(FD);
+  if (const NamespaceDecl *NSD = dyn_cast(DC)) {
+// Skip inline/anon namespaces.
+if (NSD->isInline() || NSD->isAnonymousNamespace())
+  return getNamespaceScope(NSD);
+  }
   if (const NamedDecl *ND = dyn_cast(DC))
-return ND->getQualifiedNameAsString();
+return printQualifiedName(*ND);
 
   return "";
 }
@@ -345,15 +354,19 @@ HoverInfo getHoverContents(const Decl *D, const 
SymbolIndex *Index) {
 HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
const SymbolIndex *Index) {
   HoverInfo HI;
-  llvm::raw_string_ostream OS(HI.Name);
-  PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
-  T.print(OS, Policy);
-  OS.flush();
 
   if (const auto *D = T->getAsTagDecl()) {
+HI.Name = printName(ASTCtx, *D);
 HI.Kind = index::getSymbolInfo(D).Kind;
 enhanceFromIndex(HI, D, Index);
   }
+
+  if (HI.Name.empty()) {
+// Builtin types
+llvm::raw_string_ostream OS(HI.Name);
+PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
+T.print(OS, Policy);
+  }
   return HI;
 }
 

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index aa44d3bda72f..8435544f37c5 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -104,7 +104,7 @@ TEST(Hover, Structured) {
   }}
   )cpp",
[](HoverInfo &HI) {
- HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.NamespaceScope = "ns1::";
  HI.LocalScope = "(anonymous struct)::";
  HI.Name = "bar";
  HI.Kind = index::SymbolKind::Field;
@@ -

[PATCH] D71597: [clangd][NFC] Make use of TagDecl inside type for hover on auto

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3d15605358eb: [clangd][NFC] Make use of TagDecl inside type 
for hover on auto (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71597

Files:
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1396,6 +1396,32 @@
 HI.Definition = "struct Test &&test = {}";
 HI.Value = "{}";
   }},
+  {
+  R"cpp(// auto on alias
+  typedef int int_type;
+  ^[[auto]] x = int_type();
+  )cpp",
+  [](HoverInfo &HI) { HI.Name = "int"; }},
+  {
+  R"cpp(// auto on alias
+  struct cls {};
+  typedef cls cls_type;
+  ^[[auto]] y = cls_type();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct cls";
+HI.Kind = index::SymbolKind::Struct;
+  }},
+  {
+  R"cpp(// auto on alias
+  template 
+  struct templ {};
+  ^[[auto]] z = templ();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct templ";
+HI.Kind = index::SymbolKind::Struct;
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
   PrintingPolicy Policy = printingPolicyForDecls(ASTCtx.getPrintingPolicy());
   T.print(OS, Policy);
   OS.flush();
 
-  if (D) {
+  if (const auto *D = T->getAsTagDecl()) {
 HI.Kind = index::getSymbolInfo(D).Kind;
 enhanceFromIndex(HI, D, Index);
   }
@@ -396,12 +396,7 @@
   getBeginningOfIdentifier(Pos, SM, AST.getLangOpts()));
 
   if (auto Deduced = getDeducedType(AST.getASTContext(), SourceLocationBeg)) {
-// Find the corresponding decl to populate kind and fetch documentation.
-DeclRelationSet Rel = DeclRelation::TemplatePattern | DeclRelation::Alias;
-auto Decls =
-targetDecl(ast_type_traits::DynTypedNode::create(*Deduced), Rel);
-HI = getHoverContents(*Deduced, Decls.empty() ? nullptr : Decls.front(),
-  AST.getASTContext(), Index);
+HI = getHoverContents(*Deduced, AST.getASTContext(), Index);
   } else if (auto M = locateMacroAt(SourceLocationBeg, AST.getPreprocessor())) 
{
 HI = getHoverContents(*M, AST);
   } else {


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1396,6 +1396,32 @@
 HI.Definition = "struct Test &&test = {}";
 HI.Value = "{}";
   }},
+  {
+  R"cpp(// auto on alias
+  typedef int int_type;
+  ^[[auto]] x = int_type();
+  )cpp",
+  [](HoverInfo &HI) { HI.Name = "int"; }},
+  {
+  R"cpp(// auto on alias
+  struct cls {};
+  typedef cls cls_type;
+  ^[[auto]] y = cls_type();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct cls";
+HI.Kind = index::SymbolKind::Struct;
+  }},
+  {
+  R"cpp(// auto on alias
+  template 
+  struct templ {};
+  ^[[auto]] z = templ();
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "struct templ";
+HI.Kind = index::SymbolKind::Struct;
+  }},
   };
 
   // Create a tiny index, so tests above can verify documentation is fetched.
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -342,15 +342,15 @@
 }
 
 /// Generate a \p Hover object given the type \p T.
-HoverInfo getHoverContents(QualType T, const Decl *D, ASTContext &ASTCtx,
-  const SymbolIndex *Index) {
+HoverInfo getHoverContents(QualType T, ASTContext &ASTCtx,
+   const SymbolIndex *Index) {
   HoverInfo HI;
   llvm::raw_string_ostream OS(HI.Name);
 

[PATCH] D71543: [clangd] Fix handling of inline/anon namespaces and names of deduced types in hover

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ab15f303efd: [clangd] Fix handling of inline/anon 
namespaces and names of deduced types in… (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71543

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -104,7 +104,7 @@
   }}
   )cpp",
[](HoverInfo &HI) {
- HI.NamespaceScope = "ns1::(anonymous)::";
+ HI.NamespaceScope = "ns1::";
  HI.LocalScope = "(anonymous struct)::";
  HI.Name = "bar";
  HI.Kind = index::SymbolKind::Field;
@@ -362,7 +362,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class (lambda)";
+ HI.Name = "(lambda)";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on template instantiation
@@ -373,7 +373,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
   // auto on specialized template
@@ -385,7 +385,7 @@
 }
 )cpp",
[](HoverInfo &HI) {
- HI.Name = "class Foo";
+ HI.Name = "Foo";
  HI.Kind = index::SymbolKind::Class;
}},
 
@@ -524,6 +524,44 @@
  HI.NamespaceScope = "";
  HI.LocalScope = "boom::";
}},
+  {
+  R"cpp(// Should not print inline or anon namespaces.
+  namespace ns {
+inline namespace in_ns {
+  namespace a {
+namespace {
+  namespace b {
+inline namespace in_ns2 {
+  class Foo {};
+} // in_ns2
+  } // b
+} // anon
+  } // a
+} // in_ns
+  } // ns
+  void foo() {
+ns::a::b::[[F^oo]] x;
+(void)x;
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+HI.NamespaceScope = "ns::a::b::";
+HI.Definition = "class Foo {}";
+  }},
+  {
+  R"cpp(
+  template  class Foo {};
+  class X;
+  void foo() {
+[[^auto]] x = Foo();
+  }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "Foo";
+HI.Kind = index::SymbolKind::Class;
+  }},
   };
   for (const auto &Case : Cases) {
 SCOPED_TRACE(Case.Code);
@@ -895,7 +933,7 @@
   [](HoverInfo &HI) {
 HI.Name = "foo";
 HI.Kind = index::SymbolKind::Variable;
-HI.NamespaceScope = "ns::(anonymous)::";
+HI.NamespaceScope = "ns::";
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
@@ -1173,7 +1211,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "class std::initializer_list";
+HI.Name = "initializer_list";
 HI.Kind = index::SymbolKind::Class;
   }},
   {
@@ -1231,7 +1269,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1242,7 +1280,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1253,7 +1291,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1265,7 +1303,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1277,7 +1315,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1289,7 +1327,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+HI.Name = "Bar";
 HI.Kind = index::SymbolKind::Struct;
   }},
   {
@@ -1300,7 +1338,7 @@
 }
   )cpp",
   [](HoverInfo &HI) {
-HI.Name = "struct Bar";
+ 

[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:188
+// returns D.
+const NamedDecl *getExplicitSpec(const NamedDecl *D) {
+  if (auto *CTSD = llvm::dyn_cast(D)) {

What's the purpose of this function?
I don't think its description has semantic meaning in C++, "implicit 
instantiations" do not have an "explicit specialization"...

It seems to be doing something to change a decl into something that could be 
used to query the index. If that's the case, we could probably have a name 
that's closer to the described goal.



Comment at: clang-tools-extra/clangd/Hover.cpp:200
+  // We only add documentation, so don't bother if we already have some, or
+  // Index isn't supplied.
+  if (!Hover.Documentation.empty() || !Index)

NIT: maybe remove `or Index isn't supplied`.
It's clear from the code, no need to duplicate it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596



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


[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Ulrich Weigand via Phabricator via cfe-commits
uweigand added a comment.

It looks like this caused build bot failures:
http://lab.llvm.org:8011/builders/clang-s390x-linux/builds/28928/steps/ninja%20check%201/logs/FAIL%3A%20lit%3A%3A%20shtest-format.py

  
/home/uweigand/sandbox/buildbot/clang-s390x-linux/stage1/utils/lit/tests/shtest-format.py:52:10:
 error: CHECK: expected string not found in input
  # CHECK: UNSUPPORTED: shtest-format :: requires-any-missing.txt
   ^
  :71:33: note: scanning from here
  PASS: shtest-format :: pass.txt (8 of 22)
  ^
  :72:1: note: possible intended match here
  UNSUPPORTED: shtest-format :: requires-missing.txt (9 of 22)
  ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408



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


[PATCH] D71408: [lit] Remove lit's REQUIRES-ANY directive

2019-12-17 Thread Joel E. Denny via Phabricator via cfe-commits
jdenny added a comment.

This breaks the lit test suite.  `llvm/utils/lit/tests/shtest-format.py` needs 
to be updated for the removed tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71408



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


[PATCH] D67833: [OpenMP 5.0] Codegen support to pass user-defined mapper functions to runtime

2019-12-17 Thread Lingda Li via Phabricator via cfe-commits
lildmh marked an inline comment as done.
lildmh added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:4918
 const OMPMappableExprListSizeTy &Sizes)
-  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, &MapperQualifierLoc,
-  &MapperIdInfo),
+  : OMPMappableExprListClause(OMPC_map, Locs, Sizes, /*HasMapper=*/true,
+  &MapperQualifierLoc, &MapperIdInfo),

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > lildmh wrote:
> > > > ABataev wrote:
> > > > > Do we really need to set `HasMapper` to `true` unconditionally here 
> > > > > and in other places?
> > > > For `map`, `to`, and `from` clauses, they can have mappers, so it's set 
> > > > to `true`. For `is_device_ptr` and `use_device_ptr`, it's set to 
> > > > `false`.
> > > So, it completely depends on the clause kind, right? If so, can we just 
> > > remove it and rely on the clause kind?
> > This is used in `OMPMappableExprListClause` which is inherited by all these 
> > clauses. Do you mean to check the template type there to get the clause 
> > kind?
> I see. I think it is better to rename it to `ClauseSupportsMapper` (or 
> something like this) and make it `const`. Currently, it is confusing. I 
> thought that this flag shows the use of the mapper in the clause.
Ok, will do that


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

https://reviews.llvm.org/D67833



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


[PATCH] D71596: [clangd] Improve documentation for auto and implicit specs

2019-12-17 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Hover.cpp:188
+// returns D.
+const NamedDecl *getExplicitSpec(const NamedDecl *D) {
+  if (auto *CTSD = llvm::dyn_cast(D)) {

ilya-biryukov wrote:
> What's the purpose of this function?
> I don't think its description has semantic meaning in C++, "implicit 
> instantiations" do not have an "explicit specialization"...
> 
> It seems to be doing something to change a decl into something that could be 
> used to query the index. If that's the case, we could probably have a name 
> that's closer to the described goal.
yeah naming is hard :/

it is not just something that can be used to query index, but also for AST, as 
the decl of instantiation doesn't contain comments attached to specialization.
this is basically returning the explicit specialization used to instantiate `D`.

any suggestions for the name ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71596



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


[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

2019-12-17 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen marked an inline comment as done.
cchen added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128
+   (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate ||
+   Data.Attributes == OMPC_linear)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) ||

ABataev wrote:
> What is this change for?
Since now the variable in step expression has been set as implicit firstprivate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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


[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128
+   (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate ||
+   Data.Attributes == OMPC_linear)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) ||

cchen wrote:
> ABataev wrote:
> > What is this change for?
> Since now the variable in step expression has been set as implicit 
> firstprivate.
No need to do this, the whole expression must be evaluated before entering the 
parallel region.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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


[PATCH] D71553: [RISCV] Add Clang frontend support for Bitmanip extension

2019-12-17 Thread Scott Egerton via Phabricator via cfe-commits
s.egerton updated this revision to Diff 234308.
s.egerton added a comment.

Add test that march 'b' flag enables __riscv_bitmanip.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71553

Files:
  clang/lib/Basic/Targets/RISCV.cpp
  clang/lib/Basic/Targets/RISCV.h
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/test/Preprocessor/riscv-target-features.c


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -7,6 +7,7 @@
 // CHECK-NOT: __riscv_mul
 // CHECK-NOT: __riscv_muldiv
 // CHECK-NOT: __riscv_compressed
+// CHECK-NOT: __riscv_bitmanip
 // CHECK-NOT: __riscv_flen
 // CHECK-NOT: __riscv_fdiv
 // CHECK-NOT: __riscv_fsqrt
@@ -48,6 +49,12 @@
 // RUN: -o - | FileCheck --check-prefix=CHECK-C-EXT %s
 // CHECK-C-EXT: __riscv_compressed 1
 
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ib -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-B-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ib -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-B-EXT %s
+// CHECK-B-EXT: __riscv_bitmanip 1
+
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32 -x 
c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x 
c -E -dM %s \
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -331,6 +331,9 @@
 case 'c':
   Features.push_back("+c");
   break;
+case 'b':
+  Features.push_back("+b");
+  break;
 }
   }
 
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -30,11 +30,12 @@
   bool HasF;
   bool HasD;
   bool HasC;
+  bool HasB;
 
 public:
   RISCVTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
   : TargetInfo(Triple), HasM(false), HasA(false), HasF(false),
-HasD(false), HasC(false) {
+HasD(false), HasC(false), HasB(false) {
 LongDoubleWidth = 128;
 LongDoubleAlign = 128;
 LongDoubleFormat = &llvm::APFloat::IEEEquad();
Index: clang/lib/Basic/Targets/RISCV.cpp
===
--- clang/lib/Basic/Targets/RISCV.cpp
+++ clang/lib/Basic/Targets/RISCV.cpp
@@ -125,6 +125,10 @@
 
   if (HasC)
 Builder.defineMacro("__riscv_compressed");
+
+  if (HasB) {
+Builder.defineMacro("__riscv_bitmanip");
+  }
 }
 
 /// Return true if has this feature, need to sync with handleTargetFeatures.
@@ -139,6 +143,7 @@
   .Case("f", HasF)
   .Case("d", HasD)
   .Case("c", HasC)
+  .Case("b", HasB)
   .Default(false);
 }
 
@@ -156,6 +161,8 @@
   HasD = true;
 else if (Feature == "+c")
   HasC = true;
+else if (Feature == "+b")
+  HasB = true;
   }
 
   return true;


Index: clang/test/Preprocessor/riscv-target-features.c
===
--- clang/test/Preprocessor/riscv-target-features.c
+++ clang/test/Preprocessor/riscv-target-features.c
@@ -7,6 +7,7 @@
 // CHECK-NOT: __riscv_mul
 // CHECK-NOT: __riscv_muldiv
 // CHECK-NOT: __riscv_compressed
+// CHECK-NOT: __riscv_bitmanip
 // CHECK-NOT: __riscv_flen
 // CHECK-NOT: __riscv_fdiv
 // CHECK-NOT: __riscv_fsqrt
@@ -48,6 +49,12 @@
 // RUN: -o - | FileCheck --check-prefix=CHECK-C-EXT %s
 // CHECK-C-EXT: __riscv_compressed 1
 
+// RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ib -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-B-EXT %s
+// RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ib -x c -E -dM %s \
+// RUN: -o - | FileCheck --check-prefix=CHECK-B-EXT %s
+// CHECK-B-EXT: __riscv_bitmanip 1
+
 // RUN: %clang -target riscv32-unknown-linux-gnu -march=rv32ifd -mabi=ilp32 -x c -E -dM %s \
 // RUN: -o - | FileCheck --check-prefix=CHECK-SOFT %s
 // RUN: %clang -target riscv64-unknown-linux-gnu -march=rv64ifd -mabi=lp64 -x c -E -dM %s \
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -331,6 +331,9 @@
 case 'c':
   Features.push_back("+c");
   break;
+case 'b':
+  Features.push_back("+b");
+  break;
 }
   }
 
Index: clang/lib/Basic/Targets/RISCV.h
===
--- clang/lib/Basic/Targets/RISCV.h
+++ clang/lib/Basic/Targets/RISCV.h
@@ -30,11 +30,12 @@
   bool HasF;
   bool HasD;
   bool HasC;
+ 

[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D71241#1787888 , @ABataev wrote:

> Hal, are we going to support something like this?


I'm not Hal but I will answer anyway.

>   void cpu() { asm("nop"); }
>   
>   #pragma omp declare variant(cpu) match(device = {kind(cpu)})
>   void wrong_asm() {
> asm ("xxx");
>   }
> 
> 
> Currently, there is an error when we try to emit the assembler output.

While it is unclear to me what you think should happen, an error pointing at 
`"xxx"` is to be expected without further information on how this is compiled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D71241#1787998 , @jdoerfert wrote:

> In D71241#1787888 , @ABataev wrote:
>
> > Hal, are we going to support something like this?
>
>
> I'm not Hal but I will answer anyway.
>
> >   void cpu() { asm("nop"); }
> >   
> >   #pragma omp declare variant(cpu) match(device = {kind(cpu)})
> >   void wrong_asm() {
> > asm ("xxx");
> >   }
> > 
> > 
> > Currently, there is an error when we try to emit the assembler output.
>
> While it is unclear to me what you think should happen, an error pointing at 
> `"xxx"` is to be expected without further information on how this is compiled.


Shall we emit function `wrong_asm` or not? If we're not going to use it in our 
program, only `cpu` must be used, `wrong_asm` is not required and must not be 
emitted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment.

In D71241#1787652 , @hfinkel wrote:

> In D71241#1787571 , @ABataev wrote:
>
> > In D71241#1787265 , @hfinkel wrote:
> >
> > > In D71241#1786959 , @jdoerfert 
> > > wrote:
> > >
> > > > In D71241#1786530 , @ABataev 
> > > > wrote:
> > > >
> > > > > Most probably, we can use this solution without adding a new 
> > > > > expression. `DeclRefExpr` class can contain 2 decls: FoundDecl and 
> > > > > the Decl being used. You can use FoundDecl to point to the original 
> > > > > function and used decl to point to the function being called in this 
> > > > > context. But at first, we need to be sure that we can handle all 
> > > > > corner cases correctly.
> > > >
> > > >
> > > > What new expression are you talking about?
> > >
> > >
> > > To be clear, I believe he's talking about the new expression that I 
> > > proposed we add in order to represent this kind of call. If that's not 
> > > needed, and we can use the FoundDecl/Decl pair for that purpose, that 
> > > should also be considered.
> > >
> > > > This solution already does point to both declarations, as shown here: 
> > > > https://reviews.llvm.org/D71241#1782504
> >
> >
> > Exactly. We need to check if the `MemberRefExpr` can do this too to handle 
> > member functions correctly.
> >  And need to wait for opinion from Richard Smith about the design. We need 
> > to be sure that it won't break compatibility with C/C++ in some corner 
> > cases. Design bugs are very hard to solve and I want to be sure that we 
> > don't miss anything. And we provide full compatibility with both C and C++.
>
>
> We do need to be careful here. For cases with FoundDecl != Decl, I think that 
> the typo-correction cases look like they probably work, but there are a few 
> places where we make semantic decisions based on the mismatch, such as:
>
> In SemaTemplate.cpp below line 512, we have (this is in C++03-specific code):
>
>   } else if (!Found.isSuppressingDiagnostics()) {
> //   - if the name found is a class template, it must refer to the same
> // entity as the one found in the class of the object expression,
> // otherwise the program is ill-formed.
> if (!Found.isSingleResult() ||
> getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() !=
> OuterTemplate->getCanonicalDecl()) {
>   Diag(Found.getNameLoc(),
>diag::ext_nested_name_member_ref_lookup_ambiguous)
>   
>
> and in SemaExpr.cpp near line 2783, we have:
>
>   // If we actually found the member through a using declaration, cast
>   // down to the using declaration's type.
>   //
>   // Pointer equality is fine here because only one declaration of a
>   // class ever has member declarations.
>   if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
> assert(isa(FoundDecl));
> QualType URecordType = Context.getTypeDeclType(
>cast(FoundDecl->getDeclContext()));


Could you specify what behavior you expect, or what the test casees would look 
like?

For the record:
OpenMP basically says, if you have a call to a (base)function that has variants 
with contexts that match at the call site, call the variant with the highest 
score. The variants are specified by a //variant-func-id//, which is a base 
language identifier or C++ //template-id//. For C++, the variant declaration is 
identified by *performing the base language lookup rules on the 
//variant-func-id// with arguments that correspond to the base function 
argument types*.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71475: [WIP][OPENMP] Try to fix linear clause crash by emitting alloca for step

2019-12-17 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:1128
+   (A == OMPC_firstprivate && (Data.Attributes == OMPC_lastprivate ||
+   Data.Attributes == OMPC_linear)) ||
(A == OMPC_lastprivate && Data.Attributes == OMPC_firstprivate) ||

ABataev wrote:
> cchen wrote:
> > ABataev wrote:
> > > What is this change for?
> > Since now the variable in step expression has been set as implicit 
> > firstprivate.
> No need to do this, the whole expression must be evaluated before entering 
> the parallel region.
The standard is not clear on this, e.g., what if the expression has a 
side-effect and the loop has 0 iterations. However, evaluating it once in the 
beginning seems fine to me, assuming we do not evaluate it later again.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71475



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


[PATCH] D71241: [OpenMP][WIP] Use overload centric declare variants

2019-12-17 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment.

In D71241#1788003 , @jdoerfert wrote:

> In D71241#1787652 , @hfinkel wrote:
>
> > In D71241#1787571 , @ABataev wrote:
> >
> > > In D71241#1787265 , @hfinkel 
> > > wrote:
> > >
> > > > In D71241#1786959 , @jdoerfert 
> > > > wrote:
> > > >
> > > > > In D71241#1786530 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > Most probably, we can use this solution without adding a new 
> > > > > > expression. `DeclRefExpr` class can contain 2 decls: FoundDecl and 
> > > > > > the Decl being used. You can use FoundDecl to point to the original 
> > > > > > function and used decl to point to the function being called in 
> > > > > > this context. But at first, we need to be sure that we can handle 
> > > > > > all corner cases correctly.
> > > > >
> > > > >
> > > > > What new expression are you talking about?
> > > >
> > > >
> > > > To be clear, I believe he's talking about the new expression that I 
> > > > proposed we add in order to represent this kind of call. If that's not 
> > > > needed, and we can use the FoundDecl/Decl pair for that purpose, that 
> > > > should also be considered.
> > > >
> > > > > This solution already does point to both declarations, as shown here: 
> > > > > https://reviews.llvm.org/D71241#1782504
> > >
> > >
> > > Exactly. We need to check if the `MemberRefExpr` can do this too to 
> > > handle member functions correctly.
> > >  And need to wait for opinion from Richard Smith about the design. We 
> > > need to be sure that it won't break compatibility with C/C++ in some 
> > > corner cases. Design bugs are very hard to solve and I want to be sure 
> > > that we don't miss anything. And we provide full compatibility with both 
> > > C and C++.
> >
> >
> > We do need to be careful here. For cases with FoundDecl != Decl, I think 
> > that the typo-correction cases look like they probably work, but there are 
> > a few places where we make semantic decisions based on the mismatch, such 
> > as:
> >
> > In SemaTemplate.cpp below line 512, we have (this is in C++03-specific 
> > code):
> >
> >   } else if (!Found.isSuppressingDiagnostics()) {
> > //   - if the name found is a class template, it must refer to the same
> > // entity as the one found in the class of the object expression,
> > // otherwise the program is ill-formed.
> > if (!Found.isSingleResult() ||
> > getAsTemplateNameDecl(Found.getFoundDecl())->getCanonicalDecl() !=
> > OuterTemplate->getCanonicalDecl()) {
> >   Diag(Found.getNameLoc(),
> >diag::ext_nested_name_member_ref_lookup_ambiguous)
> >   
> >
> > and in SemaExpr.cpp near line 2783, we have:
> >
> >   // If we actually found the member through a using declaration, cast
> >   // down to the using declaration's type.
> >   //
> >   // Pointer equality is fine here because only one declaration of a
> >   // class ever has member declarations.
> >   if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
> > assert(isa(FoundDecl));
> > QualType URecordType = Context.getTypeDeclType(
> >
> > cast(FoundDecl->getDeclContext()));
>
>
> Could you specify what behavior you expect, or what the test casees would 
> look like?
>
> For the record:
>  OpenMP basically says, if you have a call to a (base)function that has 
> variants with contexts that match at the call site, call the variant with the 
> highest score. The variants are specified by a //variant-func-id//, which is 
> a base language identifier or C++ //template-id//. For C++, the variant 
> declaration is identified by *performing the base language lookup rules on 
> the //variant-func-id// with arguments that correspond to the base function 
> argument types*.


No need to worry about lookup in C++, ADL lookup is implemented already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71612: [analyzer] Add PlacementNewChecker

2019-12-17 Thread Gabor Marton via Phabricator via cfe-commits
martong created this revision.
martong added reviewers: NoQ, Szelethus.
Herald added subscribers: cfe-commits, Charusso, gamesh411, dkrupp, donat.nagy, 
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, 
whisperity, mgorny.
Herald added a project: clang.

This checker verifies if default placement new is provided with pointers
to sufficient storage capacity.

Noncompliant Code Example:

  #include 
  void f() {
short s;
long *lp = ::new (&s) long;
  }

Based on SEI CERT rule MEM54-CPP
https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM54-CPP.+Provide+placement+new+with+properly+aligned+pointers+to+sufficient+storage+capacity
This patch does not implement checking of the alignment.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71612

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CheckPlacementNew.cpp
  clang/test/Analysis/placement-new-user-defined.cpp
  clang/test/Analysis/placement-new.cpp

Index: clang/test/Analysis/placement-new.cpp
===
--- /dev/null
+++ clang/test/Analysis/placement-new.cpp
@@ -0,0 +1,94 @@
+// RUN: %clang_analyze_cc1 -std=c++11 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=cplusplus.NewDelete \
+// RUN:   -analyzer-checker=cplusplus.PlacementNew \
+// RUN:   -analyzer-output=text -verify \
+// RUN:   -triple x86_64-unknown-linux-gnu
+
+#include "Inputs/system-header-simulator-cxx.h"
+
+void f() {
+  short s; // expected-note-re {{'s' declared{{.*
+  long *lp = ::new (&s) long; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 3 {{}}
+  (void)lp;
+}
+
+namespace testArrayNew {
+void f() {
+  short s; // expected-note-re {{'s' declared{{.*
+  char *buf = ::new (&s) char[8]; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 3 {{}}
+  (void)buf;
+}
+} // namespace testArrayNew
+
+namespace testBufferInOtherFun {
+void f(void *place) {
+  long *lp = ::new (place) long; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 1 {{}}
+  (void)lp;
+}
+void g() {
+  short buf; // expected-note-re {{'buf' declared{{.*
+  f(&buf); // expected-note 2 {{}}
+}
+} // namespace testBufferInOtherFun
+
+namespace testArrayBuffer {
+void f(void *place) {
+  long *lp = ::new (place) long; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 1 {{}}
+  (void)lp;
+}
+void g() {
+  char buf[2]; // expected-note-re {{'buf' initialized{{.*
+  f(&buf); // expected-note 2 {{}}
+}
+} // namespace testArrayBuffer
+
+namespace testGlobalPtrAsPlace {
+void *gptr = nullptr;
+short gs;
+void f() {
+  gptr = &gs; // expected-note {{Value assigned to 'gptr'}}
+}
+void g() {
+  f(); // expected-note 2 {{}}
+  long *lp = ::new (gptr) long; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 1 {{}}
+  (void)lp;
+}
+} // namespace testGlobalPtrAsPlace
+
+namespace testRvalue {
+short gs;
+void *f() {
+  return &gs;
+}
+void g() {
+  long *lp = ::new (f()) long; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 1 {{}}
+  (void)lp;
+}
+} // namespace testRvalue
+
+namespace testNoWarning {
+void *f();
+void g() {
+  long *lp = ::new (f()) long;
+  (void)lp;
+}
+} // namespace testNoWarning
+
+namespace testPtrToArrayAsPlace {
+void f() {
+  //char *st = new char [8];
+  char buf[3]; // expected-note-re {{'buf' initialized{{.*
+  void *st = buf; // expected-note-re {{'st' initialized{{.*
+  long *lp = ::new (st) long; // expected-warning{{Argument of default placement new provides storage capacity of 3 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 1 {{}}
+  (void)lp;
+}
+} // namespace testPtrToArrayAsPlace
+
+namespace testHeapAllocatedBuffer {
+void g2() {
+  char *buf = new char[2]; // expected-note-re {{'buf' initialized{{.*
+  long *lp = ::new (buf) long; // expected-warning{{Argument of default placement new provides storage capacity of 2 bytes, but the allocated type requires storage capacity of 8 bytes}} expected-note 1 {{}}
+  (void)lp;
+}
+} // namespace testHeapAllocatedBuffer
Index: clang/test/Analysis/placement-new-user-defined.cpp
===
--- /dev/nul

  1   2   3   >