[PATCH] D52034: [Clang] Add options -fprofile-filter-files and -fprofile-exclude-files to filter the files to instrument with gcov

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Can be done in a new commit but I think this should be mentioned in the clang 
release notes.


Repository:
  rC Clang

https://reviews.llvm.org/D52034



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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In https://reviews.llvm.org/D53153#1289380, @scott.linder wrote:

> I don't believe that is currently the case (the unrestricted linking of OCL 
> code to OCL code via a dynamic linker), but we do have the notion of a static 
> link step, followed by dynamic linking at runtime. The static link step is 
> currently via IR, but we plan to support linking object files. Maybe I 
> misunderstand the distinction between linkage and visibility, but it seems 
> reasonable that a user would want to have e.g. a non-kernel function 
> participate in static linking, but not be preemptible in the final shared 
> object. The intention with this patch is to allow this with something like 
> `-fvisibility hidden` without disrupting kernel symbols, which must appear in 
> the dynsym for the reasons mentioned earlier in the thread.


Okay, this still doesn't need user-controlled symbol visibility.  The basic 
question here is whether there is any need for anything other than kernel 
functions to have dynamic linkage.  If not, then you really just need to mark 
everything as hidden except for kernels.  You *could* do that marking in Sema, 
but frankly that seems like an implementation detail escaping into the AST, and 
it's just going to cause you unwanted pain and sub-optimality.  (For example: 
`-fvisibility=hidden` doesn't actually change the visibility of external 
declarations for reasons that make a lot of sense for general-purpose 
environments but probably don't matter to you at all.  If you're doing this to 
get better code-generation for references to external entities that are going 
to end up within the image, you actually need to add a visibility attribute to 
every single top-level declaration to get that effect.  It's much easier to 
just do it in a pass on the module.)


https://reviews.llvm.org/D53153



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

[PATCH] D53675: [WebAssembly] Lower select for vectors

[Thomas Lively via Phabricator via cfe-commits]

tlively updated this revision to Diff 173120.
tlively added a comment.

- Update comment


Repository:
  rL LLVM

https://reviews.llvm.org/D53675

Files:
  lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  test/CodeGen/WebAssembly/simd-select.ll
  test/CodeGen/WebAssembly/simd-vselect.ll

Index: test/CodeGen/WebAssembly/simd-vselect.ll
===
--- test/CodeGen/WebAssembly/simd-vselect.ll
+++ /dev/null
@@ -1,90 +0,0 @@
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -wasm-enable-unimplemented-simd -mattr=+simd128,+sign-ext | FileCheck %s
-
-; Test that lanewise vector selects lower correctly to bitselects
-
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
-
-; CHECK-LABEL: vselect_v16i8:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 7{{$}}
-; CHECK-NEXT: i8x16.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 7{{$}}
-; CHECK-NEXT: i8x16.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <16 x i8> @vselect_v16i8(<16 x i1> %c, <16 x i8> %x, <16 x i8> %y) {
-  %res = select <16 x i1> %c, <16 x i8> %x, <16 x i8> %y
-  ret <16 x i8> %res
-}
-
-; CHECK-LABEL: vselect_v8i16:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 15{{$}}
-; CHECK-NEXT: i16x8.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 15{{$}}
-; CHECK-NEXT: i16x8.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <8 x i16> @vselect_v8i16(<8 x i1> %c, <8 x i16> %x, <8 x i16> %y) {
-  %res = select <8 x i1> %c, <8 x i16> %x, <8 x i16> %y
-  ret <8 x i16> %res
-}
-
-; CHECK-LABEL: vselect_v4i32:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <4 x i32> @vselect_v4i32(<4 x i1> %c, <4 x i32> %x, <4 x i32> %y) {
-  %res = select <4 x i1> %c, <4 x i32> %x, <4 x i32> %y
-  ret <4 x i32> %res
-}
-
-; CHECK-LABEL: vselect_v2i64:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <2 x i64> @vselect_v2i64(<2 x i1> %c, <2 x i64> %x, <2 x i64> %y) {
-  %res = select <2 x i1> %c, <2 x i64> %x, <2 x i64> %y
-  ret <2 x i64> %res
-}
-
-; CHECK-LABEL: vselect_v4f32:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <4 x float> @vselect_v4f32(<4 x i1> %c, <4 x float> %x, <4 x float> %y) {
-  %res = select <4 x i1> %c, <4 x float> %x, <4 x float> %y
-  ret <4 x float> %res
-}
-
-; CHECK-LABEL: vselect_v2f64:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <2 x double> @vselect_v2f64(<2 x i1> %c, <2 x double> %x, <2 x double> %y) {
-  %res = select <2 x i1> %c, <2 x double> %x, <2 x double> %y
-  ret <2 x double> %res
-}
Index: test/CodeGen/WebAssembly/simd-select.ll
===
--- /dev/null
+++ test/CodeGen/WebAssembly/simd-select.ll
@@ -0,0 +1,463 @@
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers 

[PATCH] D53675: [WebAssembly] Lower select for vectors

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

In https://reviews.llvm.org/D53675#1290660, @aheejin wrote:

> I tried to add `ISD::SELECT` in the expansion list, like
>
>   for (auto Op : {ISD::VSELECT, ISD::SELECT_CC, ISD::SELECT}) {
> ...
>   }
>   
>
> And the code generation apparently works for current test `simd-select.ll`. 
> I'm not sure if it's optimal though. Could you check again?


I'm not sure what I did before, but you're totally right. Letting the expander 
take care of selects yields the same code in most places and actually allows 
some DAGCombine optimizations that weren't possible before.


Repository:
  rL LLVM

https://reviews.llvm.org/D53675



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

[PATCH] D53675: [WebAssembly] Lower select for vectors

[Thomas Lively via Phabricator via cfe-commits]

tlively updated this revision to Diff 173119.
tlively added a comment.

- Use automatic expansion instead of patterns


Repository:
  rL LLVM

https://reviews.llvm.org/D53675

Files:
  lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  test/CodeGen/WebAssembly/simd-select.ll
  test/CodeGen/WebAssembly/simd-vselect.ll

Index: test/CodeGen/WebAssembly/simd-vselect.ll
===
--- test/CodeGen/WebAssembly/simd-vselect.ll
+++ /dev/null
@@ -1,90 +0,0 @@
-; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers -wasm-enable-unimplemented-simd -mattr=+simd128,+sign-ext | FileCheck %s
-
-; Test that lanewise vector selects lower correctly to bitselects
-
-target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
-target triple = "wasm32-unknown-unknown"
-
-; CHECK-LABEL: vselect_v16i8:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 7{{$}}
-; CHECK-NEXT: i8x16.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 7{{$}}
-; CHECK-NEXT: i8x16.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <16 x i8> @vselect_v16i8(<16 x i1> %c, <16 x i8> %x, <16 x i8> %y) {
-  %res = select <16 x i1> %c, <16 x i8> %x, <16 x i8> %y
-  ret <16 x i8> %res
-}
-
-; CHECK-LABEL: vselect_v8i16:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 15{{$}}
-; CHECK-NEXT: i16x8.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 15{{$}}
-; CHECK-NEXT: i16x8.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <8 x i16> @vselect_v8i16(<8 x i1> %c, <8 x i16> %x, <8 x i16> %y) {
-  %res = select <8 x i1> %c, <8 x i16> %x, <8 x i16> %y
-  ret <8 x i16> %res
-}
-
-; CHECK-LABEL: vselect_v4i32:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <4 x i32> @vselect_v4i32(<4 x i1> %c, <4 x i32> %x, <4 x i32> %y) {
-  %res = select <4 x i1> %c, <4 x i32> %x, <4 x i32> %y
-  ret <4 x i32> %res
-}
-
-; CHECK-LABEL: vselect_v2i64:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <2 x i64> @vselect_v2i64(<2 x i1> %c, <2 x i64> %x, <2 x i64> %y) {
-  %res = select <2 x i1> %c, <2 x i64> %x, <2 x i64> %y
-  ret <2 x i64> %res
-}
-
-; CHECK-LABEL: vselect_v4f32:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 31{{$}}
-; CHECK-NEXT: i32x4.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <4 x float> @vselect_v4f32(<4 x i1> %c, <4 x float> %x, <4 x float> %y) {
-  %res = select <4 x i1> %c, <4 x float> %x, <4 x float> %y
-  ret <4 x float> %res
-}
-
-; CHECK-LABEL: vselect_v2f64:
-; CHECK-NEXT: .param v128, v128, v128{{$}}
-; CHECK-NEXT: .result v128{{$}}
-; CHECK-NEXT: i32.const $push[[L0:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shl $push[[L1:[0-9]+]]=, $0, $pop[[L0]]{{$}}
-; CHECK-NEXT: i32.const $push[[L2:[0-9]+]]=, 63{{$}}
-; CHECK-NEXT: i64x2.shr_s $push[[L3:[0-9]+]]=, $pop[[L1]], $pop[[L2]]{{$}}
-; CHECK-NEXT: v128.bitselect $push[[R:[0-9]+]]=, $1, $2, $pop[[L3]]{{$}}
-; CHECK-NEXT: return $pop[[R]]{{$}}
-define <2 x double> @vselect_v2f64(<2 x i1> %c, <2 x double> %x, <2 x double> %y) {
-  %res = select <2 x i1> %c, <2 x double> %x, <2 x double> %y
-  ret <2 x double> %res
-}
Index: test/CodeGen/WebAssembly/simd-select.ll
===
--- /dev/null
+++ test/CodeGen/WebAssembly/simd-select.ll
@@ -0,0 +1,463 @@
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt -wasm-disable-explicit-locals -wasm-keep-registers 

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+// If the header in the module map refers to a symlink, Header.Entry
+// refers to the actual file. The callback should be notified of both.
+if (!FullPathAsWritten.empty() &&
+!FullPathAsWritten.equals(Header.Entry->getName())) {
+  Cb->moduleMapAddHeader(FullPathAsWritten, Mod->IsSystem, Imported);
+}

vsapsai wrote:
> It is strange but after removing this part the tests are still passing. I 
> suspect the code is correct but the test allows some roundabout way to add 
> symlink to dependencies. In my experiments only 
> `DFGMMCallback::moduleMapAddHeader` affects the tests. Is it expected?
Looks like you have 3 cases:

1. Add all files in module map to dependencies, even if a file isn't 
`#include`d anywhere (this turned out to be `link.h`).
2. For `-fmodule-file` replace header files in dependencies with .pcm file 
(that's what `Imported` achieves).
3. Some scenario with symlinks. Here I haven't found a representative test case.


https://reviews.llvm.org/D53522



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Sorry, had a leftover draft which I forgot to clean up. Edited in Phabricator 
now.


Repository:
  rC Clang

https://reviews.llvm.org/D52674



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

[PATCH] D52674: [AST] Add Obj-C discriminator to MS ABI RTTI

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote:

> In https://reviews.llvm.org/D52674#1271814, @smeenai wrote:
>
> > @rjmccall I prototyped the ForRTTI parameter approach in 
> > https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, 
> > but it demonstrates the problems I saw with the added parameter. Namely, 
> > `mangleType(QualType, SourceRange, QualifierMangleMode)` has a bunch of 
> > additional logic which is needed for correctness, so we need to thread the 
> > parameter through the entire chain, and the only way I could think of doing 
> > that without adding the parameter to every single `mangleType` overload was 
> > to have an additional switch to dispatch to the overloads with the added 
> > ForRTTI parameter, which is pretty ugly.
> >
> > As I see it, there are a few ways to proceed here:
> >
> > - The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I 
> > think it's pretty ugly, but you may have suggestions on how to do it better.
> > - In the `mangleType` overload for `ObjCObjectPointerType`, skipping the 
> > generic `mangleType` dispatch and going directly to either the 
> > `ObjCObjectType` or `ObjCInterfaceType` overloads. That avoids the ugliness 
> > in the generic `mangleType`, but it also requires us to duplicate some 
> > logic from it in the `ObjCObjectPointerType` overload, which doesn't seem 
> > great either.
> > - Maintaining `ForRTTI` state in the mangler instead of threading a 
> > parameter through (I'm generally not a fan of state like that, but it might 
> > be cleaner than the threading in this case?)
> > - Just sticking with what I'm doing in this patch.
> >
> >   What do you think?
>
>
> Sorry for the delay.  I think duplicating the dispatch logic for 
> `ObjCObjectPointerType` is probably the best approach; the pointee type will 
> always an `ObjCObjectType` of some sort, and there are only two kinds of 
> those.


To be clear, we would need to duplicate (or factor out into a common function) 
some mangling logic as well, because e.g. adding the `E`

In https://reviews.llvm.org/D52674#1281747, @rjmccall wrote:

> In https://reviews.llvm.org/D52674#1271814, @smeenai wrote:
>
> > @rjmccall I prototyped the ForRTTI parameter approach in 
> > https://reviews.llvm.org/D53546. It could definitely be cleaned up a bit, 
> > but it demonstrates the problems I saw with the added parameter. Namely, 
> > `mangleType(QualType, SourceRange, QualifierMangleMode)` has a bunch of 
> > additional logic which is needed for correctness, so we need to thread the 
> > parameter through the entire chain, and the only way I could think of doing 
> > that without adding the parameter to every single `mangleType` overload was 
> > to have an additional switch to dispatch to the overloads with the added 
> > ForRTTI parameter, which is pretty ugly.
> >
> > As I see it, there are a few ways to proceed here:
> >
> > - The approach in https://reviews.llvm.org/D53546 (cleaned up a bit). I 
> > think it's pretty ugly, but you may have suggestions on how to do it better.
> > - In the `mangleType` overload for `ObjCObjectPointerType`, skipping the 
> > generic `mangleType` dispatch and going directly to either the 
> > `ObjCObjectType` or `ObjCInterfaceType` overloads. That avoids the ugliness 
> > in the generic `mangleType`, but it also requires us to duplicate some 
> > logic from it in the `ObjCObjectPointerType` overload, which doesn't seem 
> > great either.
> > - Maintaining `ForRTTI` state in the mangler instead of threading a 
> > parameter through (I'm generally not a fan of state like that, but it might 
> > be cleaner than the threading in this case?)
> > - Just sticking with what I'm doing in this patch.
> >
> >   What do you think?
>
>
> Sorry for the delay.  I think duplicating the dispatch logic for 
> `ObjCObjectPointerType` is probably the best approach; the pointee type will 
> always an `ObjCObjectType` of some sort, and there are only two kinds of 
> those.


Sorry, I'm still not sure how this will work.

Duplicating the dispatch logic for `ObjCObjectPointerType` ends up looking like 
https://reviews.llvm.org/P8114, which is fine. However, when we're actually 
mangling RTTI or RTTI names, we're still going through the main 
`mangleType(QualType, SourceRange, QualifierMangleMode)` overload, which still 
requires us to thread `ForRTTI` through that function, which still requires us 
to duplicate the switch in that function (to handle the `ForRTTI` case, since 
the other switch is generated via TypeNodes.def and not easily modifiable), 
which is the main ugliness in my opinion. Do you also want me to add special 
dispatching to `mangleCXXRTTI` and `mangleCXXRTTIName` to just call the 
`ObjCObjectPointerType` function directly when they're dealing with that type? 
That's certainly doable, but at that point just keeping some state around in 
the demangler starts to feel cleaner, at least to me.


Repository:
  

[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added inline comments.



Comment at: clang/lib/Frontend/DependencyFile.cpp:100
+
+DepCollector.maybeAddDependency(HeaderPath, /*FromModule=*/false, IsSystem,
+/*IsModuleFile*/ false,

This `/*FromModule=*/false` looks confusing and suspicious but seems like we 
don't use this argument, so it doesn't really matter and worth cleaning up (out 
of scope for this change).



Comment at: clang/lib/Frontend/DependencyFile.cpp:105-109
+  void moduleMapAddUmbrellaHeader(FileManager *FileMgr, const FileEntry 
*Header,
+  bool IsSystem, bool Imported) override {
+DepCollectorMMCallbacks::moduleMapAddHeader(Header->getName(), IsSystem,
+Imported);
+  }

What is the purpose of this method and the one in `DFGMMCallback`? It looks 
correct-ish but after removing this callback, the tests are still passing.



Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+// If the header in the module map refers to a symlink, Header.Entry
+// refers to the actual file. The callback should be notified of both.
+if (!FullPathAsWritten.empty() &&
+!FullPathAsWritten.equals(Header.Entry->getName())) {
+  Cb->moduleMapAddHeader(FullPathAsWritten, Mod->IsSystem, Imported);
+}

It is strange but after removing this part the tests are still passing. I 
suspect the code is correct but the test allows some roundabout way to add 
symlink to dependencies. In my experiments only 
`DFGMMCallback::moduleMapAddHeader` affects the tests. Is it expected?


https://reviews.llvm.org/D53522



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

[PATCH] D54246: [clang-tidy] Add the abseil-duration-factory-scale check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:36
+GetScaleForFactory(llvm::StringRef FactoryName) {
+  static const auto *ScaleMap =
+  new std::unordered_map(

This will cause memory leaks, so may be unique_ptr should be used to hold 
pointer? May be LLVM ADT has better container?



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:59
+return IntLit->getValue().getLimitedValue();
+  } else {
+assert(FloatLit != nullptr);

else after return. Same in other places.



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:164
+  }
+  return "unreachable";
+}

Should be llvm_unreachable().



Comment at: clang-tidy/abseil/DurationFactoryScaleCheck.cpp:288
+  // We can't do anything with this expression, so return.
+  return;
+}

Please remove trailing return and comment above. See [[ 
http://clang.llvm.org/extra/clang-tidy/checks/readability-redundant-control-flow.html
 | readability-redundant-control-flow ]].



Comment at: docs/ReleaseNotes.rst:88
+  Checks for cases where arguments to ``absl::Duration`` factory functions are
+  scaled internally and could be changed to a different factory function.  This
+  check also looks for arguements with a zero value and suggests using

Second statement belongs to documentation.



Comment at: docs/clang-tidy/checks/abseil-duration-factory-scale.rst:7
+Checks for cases where arguments to ``absl::Duration`` factory functions are
+scaled internally and could be changed to a different factory function.  This
+check also looks for arguements with a zero value and suggests using

Please fix double space.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54246



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

[PATCH] D54245: [VFS] Implement `RedirectingFileSystem::getRealPath`.

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai created this revision.
vsapsai added reviewers: bruno, benlangmuir.
Herald added subscribers: dexonsmith, hiraditya.

It fixes the case when Objective-C framework is added as a subframework
through a symlink. When parent framework infers a module map and fails
to detect a symlink, it would add a subframework as a submodule. And
when we parse module map for the subframework, we would encounter an
error like

> error: umbrella for module 'WithSubframework.Foo' already covers this 
> directory

By implementing `getRealPath` "an egregious but useful hack" in
`ModuleMap::inferFrameworkModule` works as expected.

rdar://problem/45821279


https://reviews.llvm.org/D54245

Files:
  clang/test/VFS/subframework-symlink.m
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1775,3 +1775,49 @@
   checkContents(FS->dir_begin("//root/foo", EC),
 {"//root/foo/a", "//root/foo/b"});
 }
+
+TEST_F(VFSFromYAMLTest, GetRealPath) {
+  IntrusiveRefCntPtr Lower(new DummyFileSystem());
+  Lower->addDirectory("/dir/");
+  Lower->addRegularFile("/foo");
+  Lower->addSymlink("/link");
+  IntrusiveRefCntPtr FS = getFromYAMLString(
+  "{ 'use-external-names': false,\n"
+  "  'roots': [\n"
+  "{\n"
+  "  'type': 'directory',\n"
+  "  'name': '/root/',\n"
+  "  'contents': [ {\n"
+  "  'type': 'file',\n"
+  "  'name': 'bar',\n"
+  "  'external-contents': '/link'\n"
+  "}\n"
+  "  ]\n"
+  "},\n"
+  "{\n"
+  "  'type': 'directory',\n"
+  "  'name': '/dir/',\n"
+  "  'contents': []\n"
+  "}\n"
+  "]\n"
+  "}",
+  Lower);
+  ASSERT_TRUE(FS.get() != nullptr);
+
+  // Regular file present in underlying file system.
+  SmallString<16> RealPath;
+  EXPECT_FALSE(FS->getRealPath("/foo", RealPath));
+  EXPECT_EQ(RealPath.str(), "/foo");
+
+  // File present in YAML pointing to symlink in underlying file system.
+  EXPECT_FALSE(FS->getRealPath("/root/bar", RealPath));
+  EXPECT_EQ(RealPath.str(), "/symlink");
+
+  // Directories should fall back to the underlying file system is possible.
+  EXPECT_FALSE(FS->getRealPath("/dir/", RealPath));
+  EXPECT_EQ(RealPath.str(), "/dir/");
+
+  // Try a non-existing file.
+  EXPECT_EQ(FS->getRealPath("/non_existing", RealPath),
+errc::no_such_file_or_directory);
+}
Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1164,14 +1164,14 @@
   /// Looks up the path [Start, End) in \p From, possibly
   /// recursing into the contents of \p From if it is a directory.
   ErrorOr lookupPath(sys::path::const_iterator Start,
-  sys::path::const_iterator End, Entry *From);
+  sys::path::const_iterator End, Entry *From) const;
 
   /// Get the status of a given an \c Entry.
   ErrorOr status(const Twine , Entry *E);
 
 public:
   /// Looks up \p Path in \c Roots.
-  ErrorOr lookupPath(const Twine );
+  ErrorOr lookupPath(const Twine ) const;
 
   /// Parses \p Buffer, which is expected to be in YAML format and
   /// returns a virtual file system representing its contents.
@@ -1183,6 +1183,9 @@
   ErrorOr status(const Twine ) override;
   ErrorOr> openFileForRead(const Twine ) override;
 
+  std::error_code getRealPath(const Twine ,
+  SmallVectorImpl ) const override;
+
   llvm::ErrorOr getCurrentWorkingDirectory() const override {
 return ExternalFS->getCurrentWorkingDirectory();
   }
@@ -1726,7 +1729,7 @@
   return FS.release();
 }
 
-ErrorOr RedirectingFileSystem::lookupPath(const Twine _) {
+ErrorOr RedirectingFileSystem::lookupPath(const Twine _) const {
   SmallString<256> Path;
   Path_.toVector(Path);
 
@@ -1757,7 +1760,8 @@
 
 ErrorOr
 RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
-  sys::path::const_iterator End, Entry *From) {
+  sys::path::const_iterator End,
+  Entry *From) const {
 #ifndef _WIN32
   assert(!isTraversalComponent(*Start) &&
  !isTraversalComponent(From->getName()) &&
@@ -1890,6 +1894,27 @@
   llvm::make_unique(std::move(*Result), S));
 }
 
+std::error_code
+RedirectingFileSystem::getRealPath(const Twine ,
+   SmallVectorImpl ) const {
+  ErrorOr Result = lookupPath(Path);
+  if (!Result) {
+if (IsFallthrough &&
+Result.getError() == llvm::errc::no_such_file_or_directory) {
+  return ExternalFS->getRealPath(Path, Output);
+}

[PATCH] D54243: DebugInfo: Add a driver flag for DWARF debug_ranges base address specifier use.

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added reviewers: JDevlieghere, aprantl, probinson.
Herald added a subscriber: cfe-commits.

This saves a lot of relocations in optimized object files (at the cost
of some cost/increase in linked executable bytes), but gold's 32 bit
gdb-index support has a bug (
https://sourceware.org/bugzilla/show_bug.cgi?id=21894 ) so we can't
switch to this unconditionally. (& even if it weren't for that bug, one
might argue that some users would want to optimize in one direction or
the other - prioritizing object size or linked executable size)

Mostly sending this for review to bikeshed the flag name. (let's
bikeshed the IR names in the LLVM code review)

-fdwarf-base-address is my first prototype. Issues:

- Some existing flags (like -fdebug-types-section) use 'debug' rather than 
'dwarf', but Clang supports formats other than dwarf, so perhaps names related 
to dwarf-specific features should have dwarf in the name, though it's unlikely 
GCC would ever be consistent with that goal?

- base addresses are part of DWARF even beyond this change (eg: range lists are 
relative to the CU's base address - which is somewhat unrelated to the base 
address specifiers used in the range list). So should this specifically call 
out that it's related to ranges? that it's base address /specifiers/ that are 
in use?


Repository:
  rC Clang

https://reviews.llvm.org/D54243

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -651,6 +651,7 @@
   : Args.hasArg(OPT_gpubnames)
 ? llvm::DICompileUnit::DebugNameTableKind::Default
 : llvm::DICompileUnit::DebugNameTableKind::None);
+  Opts.DebugBaseAddress = Args.hasArg(OPT_fdwarf_base_address);
 
   setPGOInstrumentor(Opts, Args, Diags);
   Opts.InstrProfileOutput =
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3188,6 +3188,11 @@
 ? "-gpubnames"
 : "-ggnu-pubnames");
 
+  if (Args.hasFlag(options::OPT_fdwarf_base_address,
+   options::OPT_fno_dwarf_base_address, false)) {
+CmdArgs.push_back("-fdwarf-base-address");
+  }
+
   // -gdwarf-aranges turns on the emission of the aranges section in the
   // backend.
   // Always enabled for SCE tuning.
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -331,6 +331,9 @@
 /// Whether to emit .debug_gnu_pubnames section instead of .debug_pubnames.
 CODEGENOPT(DebugNameTable, 2, 0)
 
+/// Whether to use DWARF base address specifiers in .debug_ranges.
+CODEGENOPT(DebugBaseAddress, 1, 0)
+
 CODEGENOPT(NoPLT, 1, 0)
 
 /// Whether to embed source in DWARF debug line section.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1780,6 +1780,10 @@
   Flags<[CC1Option]>, HelpText<"Place debug types in their own section (ELF 
Only)">;
 def fno_debug_types_section: Flag<["-"], "fno-debug-types-section">, 
Group,
   Flags<[CC1Option]>;
+def fdwarf_base_address: Flag <["-"], "fdwarf-base-address">, Group,
+  Flags<[CC1Option]>, HelpText<"Use DWARF base address selection entries in 
debug_ranges">;
+def fno_dwarf_base_address: Flag <["-"], "fno-dwarf_base_address">, 
Group,
+  Flags<[CC1Option]>;
 def fsplit_dwarf_inlining: Flag <["-"], "fsplit-dwarf-inlining">, 
Group,
   Flags<[CC1Option]>, HelpText<"Provide minimal debug info in the 
object/executable to facilitate online symbolication/stack traces in the 
absence of .dwo/.dwp files when using Split DWARF">;
 def fno_split_dwarf_inlining: Flag<["-"], "fno-split-dwarf-inlining">, 
Group,


Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -651,6 +651,7 @@
   : Args.hasArg(OPT_gpubnames)
 ? llvm::DICompileUnit::DebugNameTableKind::Default
 : llvm::DICompileUnit::DebugNameTableKind::None);
+  Opts.DebugBaseAddress = Args.hasArg(OPT_fdwarf_base_address);
 
   setPGOInstrumentor(Opts, Args, Diags);
   Opts.InstrProfileOutput =
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3188,6 +3188,11 @@
 ? 

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/lib/AST/TypePrinter.cpp:958-964
+  // Remove the address_space qualifier so it does not get printed. We
+  // instead want to print the macro only.
+  SplitQualType SplitTy = AttrTy->getModifiedType().split();
+  Qualifiers Quals = SplitTy.Quals;
+  if (Quals.getAddressSpace() >= LangAS::FirstTargetAddressSpace)
+Quals.removeAddressSpace();
+  return printBefore(SplitTy.Ty, Quals, OS);

This has not been addressed yet in this patch, but will still make another 
patch moving the address_space from the qualifier to the AttributedType (in 
reference to r340765).


Repository:
  rC Clang

https://reviews.llvm.org/D51329



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: lib/Sema/SemaType.cpp:5817-5822
+  IdentifierInfo *AddressSpaceMacroII = nullptr;
+  auto FoundMacro = S.PP.AddressSpaceMacros.find(
+  S.SourceMgr.getSpellingLoc(Attr.getLoc()));
+  if (FoundMacro != S.PP.AddressSpaceMacros.end())
+AddressSpaceMacroII = FoundMacro->second;
+

rsmith wrote:
> This is not the right way to find the macro.
> 
> What you should do is this: look at the source range of the attribute, and 
> find the object-like macro expansion(s) common to both the start and end 
> location. Search through those for the outermost macro expansion whose range 
> is that of the attribute plus the enclosing syntax (`__attribute__((` ... 
> `))` or `[[` ... `]]` or similar). That macro name is the name that was used 
> to write this type, and that's the name we should preserve.
> 
> Plus, please do this all from `getAttributedType` itself so that it applies 
> to all type attributes, not just address space attributes.
Moved the saved identifier into the new type sugar.


Repository:
  rC Clang

https://reviews.llvm.org/D51329



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

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 173090.
leonardchan marked an inline comment as done.
leonardchan added a comment.
Herald added a subscriber: arphaman.

- Added a new type sugar node `MacroDefinedType` which wraps `AttributedType`s 
for the purpose of determining if an attribute was defined in a macro (this 
took much longer than I would've expected it to).


Repository:
  rC Clang

https://reviews.llvm.org/D51329

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/include/clang/AST/TypeNodes.def
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/ARCMigrate/TransGCAttrs.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Sema/address_space_print_macro.c
  clang/test/Sema/address_spaces.c
  clang/test/SemaCUDA/qualifiers.cu
  clang/test/SemaCXX/attr-require-constant-initialization.cpp
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -1615,6 +1615,10 @@
   return Visit(TL.getInnerLoc());
 }
 
+bool CursorVisitor::VisitMacroDefinedTypeLoc(MacroDefinedTypeLoc TL) {
+  return Visit(TL.getInnerLoc());
+}
+
 bool CursorVisitor::VisitPointerTypeLoc(PointerTypeLoc TL) {
   return Visit(TL.getPointeeLoc());
 }
Index: clang/test/SemaCXX/attr-require-constant-initialization.cpp
===
--- clang/test/SemaCXX/attr-require-constant-initialization.cpp
+++ clang/test/SemaCXX/attr-require-constant-initialization.cpp
@@ -103,7 +103,7 @@
 ATTR const NonLit _temp_ref = 42; // expected-error {{variable does not have a constant initializer}}
 // expected-note@-1 {{required by 'require_constant_initialization' attribute here}}
 #if __cplusplus >= 201103L
-// expected-note@-3 {{non-literal type 'const NonLit' cannot be used in a constant expression}}
+// expected-note@-3 {{non-literal type 'NonLit const' cannot be used in a constant expression}}
 #else
 // expected-note@-5 {{subexpression not valid in a constant expression}}
 #endif
Index: clang/test/SemaCUDA/qualifiers.cu
===
--- clang/test/SemaCUDA/qualifiers.cu
+++ clang/test/SemaCUDA/qualifiers.cu
@@ -20,16 +20,16 @@
 #if defined(__CUDA_ARCH__)
 // NVPTX does not support TLS
 __device__ int __thread device_tls_var; // expected-error {{thread-local storage is not supported for the current target}}
-// CHECK-DEVICE: device_tls_var 'int' tls
+// CHECK-DEVICE: device_tls_var 'int':'int' tls
 __shared__ int __thread shared_tls_var; // expected-error {{thread-local storage is not supported for the current target}}
-// CHECK-DEVICE: shared_tls_var 'int' tls
+// CHECK-DEVICE: shared_tls_var 'int':'int' tls
 #else
 // Device-side vars should not produce any errors during host-side
 // compilation.
 __device__ int __thread device_tls_var;
-// CHECK-HOST: device_tls_var 'int' tls
+// CHECK-HOST: device_tls_var 'int':'int' tls
 __shared__ int __thread shared_tls_var;
-// CHECK-HOST: shared_tls_var 'int' tls
+// CHECK-HOST: shared_tls_var 'int':'int' tls
 #endif
 
 __global__ void g1(int x) {}
Index: clang/test/Sema/address_spaces.c
===
--- clang/test/Sema/address_spaces.c
+++ clang/test/Sema/address_spaces.c
@@ -71,7 +71,7 @@
 
 // Clang extension doesn't forbid operations on pointers to different address spaces.
 char* cmp(_AS1 char *x,  _AS2 char *y) {
-  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
+  return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('_AS1 char *' and '_AS2 char *') which are pointers to non-overlapping address spaces}}
 }
 
 struct SomeStruct {
Index: clang/test/Sema/address_space_print_macro.c
===
--- /dev/null
+++ clang/test/Sema/address_space_print_macro.c
@@ -0,0 +1,39 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify
+
+#define AS1 __attribute__((address_space(1)))
+#define AS2 

r346378 - [clang] Set CMP0075 to new

[Shoaib Meenai via cfe-commits]

Author: smeenai
Date: Wed Nov  7 16:29:33 2018
New Revision: 346378

URL: http://llvm.org/viewvc/llvm-project?rev=346378=rev
Log:
[clang] Set CMP0075 to new

Make the check_include_file* macros honor CMAKE_REQUIRED_LIBRARIES. This
shouldn't cause any of the configuration checks to give different
results, and I verified that a clean configure before and after this
change resulted in identical CMake caches on my machine. The same change
was done for LLVM in r346377.

Modified:
cfe/trunk/CMakeLists.txt

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=346378=346377=346378=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Wed Nov  7 16:29:33 2018
@@ -1,5 +1,9 @@
 cmake_minimum_required(VERSION 3.4.3)
 
+if(POLICY CMP0075)
+  cmake_policy(SET CMP0075 NEW)
+endif()
+
 # If we are not building as a part of LLVM, build Clang as an
 # standalone project, using LLVM as an external library:
 if( CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR )


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

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

[Neeraj K. Singh via Phabricator via cfe-commits]

neerajksingh added a comment.

Reid, Hans, or someone else with commit access. If the revision looks good, 
could you please submit to SVN?

Any particular testing I should run beforehand? I ran the clang tests locally 
on Windows.


https://reviews.llvm.org/D53457



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

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

[Neeraj K. Singh via Phabricator via cfe-commits]

neerajksingh updated this revision to Diff 173072.
neerajksingh marked an inline comment as done.

https://reviews.llvm.org/D53457

Files:
  docs/UsersManual.rst
  include/clang/Driver/CLCompatOptions.td
  include/clang/Driver/Driver.h
  lib/Driver/Driver.cpp
  test/Driver/cl-options.c

Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -614,5 +614,19 @@
 // RUN: --version \
 // RUN: -Werror /Zs -- %s 2>&1
 
+// Accept clang options under the /clang: flag.
+// The first test case ensures that the SLP vectorizer is on by default and that
+// it's being turned off by the /clang:-fno-slp-vectorize flag.
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"
+// NOCLANG-SAME: "-vectorize-slp"
+// NOCLANG-NOT: "--dependent-lib=msvcrt"
+
+// RUN: %clang_cl -O2 -MD /clang:-fno-slp-vectorize /clang:-MD /clang:-MF /clang:my_dependency_file.dep -### -- %s 2>&1 | FileCheck -check-prefix=CLANG %s
+// CLANG: "--dependent-lib=msvcrt"
+// CLANG-SAME: "-dependency-file" "my_dependency_file.dep"
+// CLANG-NOT: "--dependent-lib=libcmt"
+// CLANG-NOT: "-vectorize-slp"
 
 void f() { }
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -166,14 +166,15 @@
 }
 
 InputArgList Driver::ParseArgStrings(ArrayRef ArgStrings,
+ bool IsClCompatMode,
  bool ) {
   llvm::PrettyStackTraceString CrashInfo("Command line argument parsing");
   ContainsError = false;
 
   unsigned IncludedFlagsBitmask;
   unsigned ExcludedFlagsBitmask;
   std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
-  getIncludeExcludeOptionFlagMasks();
+  getIncludeExcludeOptionFlagMasks(IsClCompatMode);
 
   unsigned MissingArgIndex, MissingArgCount;
   InputArgList Args =
@@ -730,7 +731,7 @@
   ConfigFile = CfgFileName.str();
   bool ContainErrors;
   CfgOptions = llvm::make_unique(
-  ParseArgStrings(NewCfgArgs, ContainErrors));
+  ParseArgStrings(NewCfgArgs, IsCLMode(), ContainErrors));
   if (ContainErrors) {
 CfgOptions.reset();
 return true;
@@ -924,7 +925,7 @@
   // Arguments specified in command line.
   bool ContainsError;
   CLOptions = llvm::make_unique(
-  ParseArgStrings(ArgList.slice(1), ContainsError));
+  ParseArgStrings(ArgList.slice(1), IsCLMode(), ContainsError));
 
   // Try parsing configuration file.
   if (!ContainsError)
@@ -934,21 +935,47 @@
   // All arguments, from both config file and command line.
   InputArgList Args = std::move(HasConfigFile ? std::move(*CfgOptions)
   : std::move(*CLOptions));
-  if (HasConfigFile)
-for (auto *Opt : *CLOptions) {
-  if (Opt->getOption().matches(options::OPT_config))
-continue;
+
+  auto appendOneArg = [](const Arg *Opt, const Arg *BaseArg) {
   unsigned Index = Args.MakeIndex(Opt->getSpelling());
-  const Arg *BaseArg = >getBaseArg();
-  if (BaseArg == Opt)
-BaseArg = nullptr;
   Arg *Copy = new llvm::opt::Arg(Opt->getOption(), Opt->getSpelling(),
  Index, BaseArg);
   Copy->getValues() = Opt->getValues();
   if (Opt->isClaimed())
 Copy->claim();
   Args.append(Copy);
+  };
+
+  if (HasConfigFile)
+for (auto *Opt : *CLOptions) {
+  if (Opt->getOption().matches(options::OPT_config))
+continue;
+  const Arg *BaseArg = >getBaseArg();
+  if (BaseArg == Opt)
+BaseArg = nullptr;
+  appendOneArg(Opt, BaseArg);
+}
+
+  // In CL mode, look for any pass-through arguments
+  if (IsCLMode() && !ContainsError) {
+SmallVector CLModePassThroughArgList;
+for (const auto *A : Args.filtered(options::OPT__SLASH_clang)) {
+  A->claim();
+  CLModePassThroughArgList.push_back(A->getValue());
+}
+
+if (!CLModePassThroughArgList.empty()) {
+  // Parse any pass through args using default clang processing rather
+  // than clang-cl processing.
+  auto CLModePassThroughOptions = llvm::make_unique(
+  ParseArgStrings(CLModePassThroughArgList, false, ContainsError));
+
+  if (!ContainsError)
+for (auto *Opt : *CLModePassThroughOptions) {
+  appendOneArg(Opt, nullptr);
+}
 }
+  }
 
   // FIXME: This stuff needs to go into the Compilation, not the driver.
   bool CCCPrintPhases;
@@ -1452,7 +1479,7 @@
   unsigned IncludedFlagsBitmask;
   unsigned ExcludedFlagsBitmask;
   std::tie(IncludedFlagsBitmask, ExcludedFlagsBitmask) =
-  getIncludeExcludeOptionFlagMasks();
+  getIncludeExcludeOptionFlagMasks(IsCLMode());
 
   ExcludedFlagsBitmask |= options::NoDriverOption;
   if (!ShowHidden)
@@ -4661,11 +4688,11 @@
   return false;
 }
 
-std::pair Driver::getIncludeExcludeOptionFlagMasks() const {

[PATCH] D53457: clang-cl: Add "/clang:" pass-through arg support.

[Neeraj K. Singh via Phabricator via cfe-commits]

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

Will update revision...




Comment at: test/Driver/cl-options.c:619
+
+// RUN: %clang_cl -O2 -### -- %s 2>&1 | FileCheck -check-prefix=NOCLANG %s
+// NOCLANG: "--dependent-lib=libcmt"

hans wrote:
> I'm not sure this test adds much value..
This test ensures that the next test is actually testing something (i.e. that 
the settings change between the two runs).


https://reviews.llvm.org/D53457



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete marked 9 inline comments as done.
Rakete added a comment.

I also found another diagnostic regression:

  template 
  T::type();

The previous error message was something with "nested name specifier does not 
refer to class ...". Now, the T::type part is interpreted as type and so we get 
a "expected unqualified-id" error message.




Comment at: include/clang/Parse/Parser.h:2119
   DeclSpecContext DSC = DeclSpecContext::DSC_normal,
-  LateParsedAttrList *LateAttrs = nullptr);
+  LateParsedAttrList *LateAttrs = nullptr) {
+return ParseDeclarationSpecifiers(DS, TemplateInfo, AS, DSC, LateAttrs,

This overload is unnecessary; I accidentally included it. I'll change it later 
so that Phab doesn't lose the review comments.



Comment at: include/clang/Parse/Parser.h:2054
+case DeclSpecContext::DSC_template_param:
+case DeclSpecContext::DSC_template_type_arg:
+case DeclSpecContext::DSC_normal:

rsmith wrote:
> This doesn't seem right to me. Doesn't this mean that `X` will change 
> from being a non-type template argument to being a type template argument?
> 
> Maybe that's a bug in the wording paper, though, since that context *is* a 
> /type-id/.
AFAIK no it won't, but I can't really tell if that's the right behavior or not. 
This patch doesn't touch the `X` case.



Comment at: test/SemaCXX/unknown-type-name.cpp:121-122
 template
-A::g() { } // expected-error{{requires a type specifier}}
+A::g() { } // expected-error{{expected unqualified-id}}
+// expected-warning@-1{{implicit 'typename' is a C++2a extension}}

rsmith wrote:
> rsmith wrote:
> > This is a diagnostic quality regression. Perhaps that's an inevitable 
> > consequence of P0634, but we should at least try to do better.
> This is marked "done" but doesn't seem to be done?
Oops, don't know why I did that. I can't figure out a way to fix this. I can 
implement a basic heuristic to detect some very basic cases like this one, but 
I don't think it's worth it.


Repository:
  rC Clang

https://reviews.llvm.org/D53847



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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Please mention new check in Release Notes (in alphabetical order).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54222



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

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete updated this revision to Diff 173056.
Rakete added a comment.

Addressed review comments! :)


Repository:
  rC Clang

https://reviews.llvm.org/D53847

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseTentative.cpp
  lib/Parse/Parser.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaDecl.cpp
  lib/Sema/SemaTemplate.cpp
  test/CXX/drs/dr1xx.cpp
  test/CXX/drs/dr2xx.cpp
  test/CXX/drs/dr4xx.cpp
  test/CXX/drs/dr5xx.cpp
  test/CXX/temp/temp.res/temp.dep/temp.dep.type/p1.cpp
  test/FixIt/fixit.cpp
  test/Parser/editor-placeholder-recovery.cpp
  test/SemaCXX/MicrosoftCompatibility.cpp
  test/SemaCXX/MicrosoftExtensions.cpp
  test/SemaCXX/MicrosoftSuper.cpp
  test/SemaCXX/unknown-type-name.cpp

Index: test/SemaCXX/unknown-type-name.cpp
===
--- test/SemaCXX/unknown-type-name.cpp
+++ test/SemaCXX/unknown-type-name.cpp
@@ -36,15 +36,15 @@
 
   static int n;
   static type m;
-  static int h(T::type, int); // expected-error{{missing 'typename'}}
-  static int h(T::type x, char); // expected-error{{missing 'typename'}}
+  static int h(T::type, int); // expected-warning{{implicit 'typename' is a C++2a extension}}
+  static int h(T::type x, char); // expected-warning{{implicit 'typename' is a C++2a extension}}
 };
 
 template
-A::type g(T t) { return t; } // expected-error{{missing 'typename'}}
+A::type g(T t) { return t; } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
-A::type A::f() { return type(); } // expected-error{{missing 'typename'}}
+A::type A::f() { return type(); } // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template
 void f(T::type) { } // expected-error{{missing 'typename'}}
@@ -72,9 +72,7 @@
 
 int *p;
 
-// FIXME: We should assume that 'undeclared' is a type, not a parameter name
-//here, and produce an 'unknown type name' diagnostic instead.
-int f1(undeclared, int); // expected-error{{requires a type specifier}}
+int f1(undeclared, int); // expected-error{{unknown type name 'undeclared'}}
 
 int f2(undeclared, 0); // expected-error{{undeclared identifier}}
 
@@ -86,11 +84,11 @@
 
 template int A::n(T::value); // ok
 template
-A::type // expected-error{{missing 'typename'}}
+A::type // expected-warning {{implicit 'typename' is a C++2a extension}}
 A::m(T::value, 0); // ok
 
-template int A::h(T::type, int) {} // expected-error{{missing 'typename'}}
-template int A::h(T::type x, char) {} // expected-error{{missing 'typename'}}
+template int A::h(T::type, int) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
+template int A::h(T::type x, char) {} // expected-warning{{implicit 'typename' is a C++2a extension}}
 
 template int h(T::type, int); // expected-error{{missing 'typename'}}
 template int h(T::type x, char); // expected-error{{missing 'typename'}}
@@ -118,4 +116,5 @@
 // FIXME: We know which type specifier should have been specified here. Provide
 //a fix-it to add 'typename A::type'
 template
-A::g() { } // expected-error{{requires a type specifier}}
+A::g() { } // expected-error{{expected unqualified-id}}
+// expected-warning@-1{{implicit 'typename' is a C++2a extension}}
Index: test/SemaCXX/MicrosoftSuper.cpp
===
--- test/SemaCXX/MicrosoftSuper.cpp
+++ test/SemaCXX/MicrosoftSuper.cpp
@@ -108,8 +108,8 @@
   typename __super::XXX a;
   typedef typename __super::XXX b;
 
-  __super::XXX c; // expected-error {{missing 'typename'}}
-  typedef __super::XXX d; // expected-error {{missing 'typename'}}
+  __super::XXX c; // expected-warning {{implicit 'typename' is a C++2a extension}}
+  typedef __super::XXX d; // expected-warning {{implicit 'typename' is a C++2a extension}}
 
   void foo() {
 typename __super::XXX e;
@@ -127,8 +127,8 @@
   typename __super::XXX a;
   typedef typename __super::XXX b;
 
-  __super::XXX c; // expected-error {{missing 'typename'}}
-  typedef __super::XXX d; // expected-error {{missing 'typename'}}
+  __super::XXX c; // expected-warning {{implicit 'typename' is a C++2a extension}}
+  typedef __super::XXX d; // expected-warning {{implicit 'typename' is a C++2a extension}}
 
   void foo() {
 typename __super::XXX e;
Index: test/SemaCXX/MicrosoftExtensions.cpp
===
--- test/SemaCXX/MicrosoftExtensions.cpp
+++ test/SemaCXX/MicrosoftExtensions.cpp
@@ -526,7 +526,7 @@
 
 namespace PR32750 {
 template struct A {};
-template struct B : A> { A::C::D d; }; // expected-error {{missing 'typename' prior to dependent type name 'A::C::D'}}
+template struct B : A> { A::C::D d; }; // expected-warning {{implicit 'typename' is a C++2a extension}}
 }
 
 #else
Index: test/SemaCXX/MicrosoftCompatibility.cpp

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

[Nikita Astafev via Phabricator via cfe-commits]

nastafev added a comment.

>   can trigger arbitrary floating-point exceptions anywhere in your code

I believe this statement reflects the current state of many compilers on the 
market, I guess I just don't see the reason why making things worse. It seems 
the original intent of the commit was to add support for masked compares, and 
that could have been achieved without breaking what already worked.

I hope the patch is ultimately helping some performance optimization, but it is 
breaking the original intent of some legitimate programs that worked before, 
and introduces correctness regression. So to me it must be at least guarded by 
a flip-switch.

The reference to constrained floating-point intrinsics work is relevant, but it 
will obviously take time and extra effort to enable and then to unbreak all the 
cases that are made broken here. Instead one could postpone lowering of the 
particular instructions until it was possible without violation of the 
semantics...


Repository:
  rC Clang

https://reviews.llvm.org/D45616



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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:33
+  const auto *VD = Result.Nodes.getNodeAs("var");
+  const auto *Return = Result.Nodes.getNodeAs("return");
+  diag(Return->getBeginLoc(), "address of static local variable %0 may not "

JonasToth wrote:
> Please `assert` on `VD` and `Return`
> Please assert on VD and Return
Why?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54222



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

This is https://bugs.llvm.org//show_bug.cgi?id=32739 . I think checks not 
relevant to a general audience (ie including the boost directory) should be in 
external plugins. It's not really clear to me what has to happen for that, but 
it seems increasingly relevant.


https://reviews.llvm.org/D54169



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

[PATCH] D54222: [clang-tidy] Add a check to detect returning static locals in public headers

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I agree with @Eugene.Zelenko that this check should rather live in `bugprone-` 
as the issue its diagnosing is not LLVM specific. It is ok to add an alias into 
the llvm module.




Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:22
+void ProblematicStaticsCheck::registerMatchers(MatchFinder *Finder) {
+  auto Matcher = returnStmt(hasReturnValue(ignoringImplicit(unaryOperator(
+hasOperatorName("&"),

Nit: you can inline this variable, having it does not improve readability 
(IMHO) nor does it reduce duplication.



Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:26
+varDecl(isStaticLocal()).bind("var"))),
+isExpansionInFileMatching("/include/.*\\.h"))
+ .bind("return");

I think the matching on `include` should not be done. Maybe it's ok to filter 
for header files, but we should not assume they live in `include/...` (as i 
think this check is generally useful and not too LLVM specific).

More filters for false positives could be checking if the function itself is 
`static` or in an anonymous namespace.



Comment at: clang-tidy/llvm/ProblematicStaticsCheck.cpp:33
+  const auto *VD = Result.Nodes.getNodeAs("var");
+  const auto *Return = Result.Nodes.getNodeAs("return");
+  diag(Return->getBeginLoc(), "address of static local variable %0 may not "

Please `assert` on `VD` and `Return`



Comment at: test/clang-tidy/llvm-problematic-statics.cpp:3
+
+#include "include/ProblematicStatics.h"
+// CHECK-MESSAGES: ProblematicStatics.h:7:5: warning: address of static local 
variable 'index' may not be identical across library boundaries 
[llvm-problematic-statics]

I think the testing should include more cases. At least normal functions, and a 
false case as well.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54222



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

[PATCH] D54222: [clang-tidy/checks] Add a checker to detect returning static locals in public headers

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

C++ Core Guidelines also mentioned similar patterns, so may be this check 
belongs to bugprone module?




Comment at: docs/clang-tidy/checks/llvm-problematic-statics.rst:7
+
+Looks for functions defined in "public" headers that return the address of a 
static local. These are not guaranteed to have the addresss across shared 
libraries and could be problematic for plugins.
+

static local variable? Probably //Looks for// should be replaced with 
//Detects//. Please also wrap with 80 characters.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54222



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

[PATCH] D53334: [Frontend/Modules] Show diagnostics on prebuilt module configuration mismatch too

[David Blaikie via Phabricator via cfe-commits]

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

While I'm not 100% sure about the actual fix - I'm confident enough that 
@rsmith can provide any further clarification in post-commit.

the test case can probably be simplified before commit - I'd suggest removing 
the "positive" case (the case without any diagnostics) as that's likely already 
covered in other tests that added this functionality to begin with, so I'd only 
have the case which produces the diagnostics - that should simplify/reduce the 
RUN lines, remove some of the #ifdefs, etc. (& potentially, you could roll the 
use and definition of the module into the same file as the RUN lines - so it's 
easier to eyeball all the relevant stuff in one place) & remove the function 
from the module - as this seems to produce a diagnostic even if the module is 
empty? Ultimately something like:

  RUN: compile -DMOD_DEF to %t/module_mismatch.pcm
  RUN: compile -DMOD_USE with prebuilt-module-path=%t
  
  #ifdef MOD_DEF
  export module module_mismatch
  #endif
  #ifdef MOD_USE
  import module_mismatch
  #endif

(also removing the extra "prebuilt_modules" directory name to reduce the length 
of command line arguments, etc)


Repository:
  rC Clang

https://reviews.llvm.org/D53334



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

[PATCH] D53675: [WebAssembly] Lower select for vectors

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added a comment.

I tried to add `ISD::SELECT` in the expansion list, like

  for (auto Op : {ISD::VSELECT, ISD::SELECT_CC, ISD::SELECT}) {
...
  }

And the code generation apparently works for current test `simd-select.ll`. I'm 
not sure if it's optimal though. Could you check again?


Repository:
  rL LLVM

https://reviews.llvm.org/D53675



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

[PATCH] D54222: [clang-tidy/checks] Add a checker to detect returning static locals in public headers

[Joe Ranieri via Phabricator via cfe-commits]

jranieri-grammatech created this revision.
jranieri-grammatech added reviewers: aaron.ballman, alexfh, NoQ.
Herald added subscribers: cfe-commits, mgorny.

This adds a checker, llvm-problematic-statics, that detects functions in header 
files that return the address of a local static variable. These cause problems 
if the resulting pointer is expected to be unique because it may have different 
values across shared library boundaries.

It's has been a problem in the static analyzer that I've had to deal with in 
the past (https://reviews.llvm.org/D52905 and https://reviews.llvm.org/D52906).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54222

Files:
  clang-tidy/llvm/CMakeLists.txt
  clang-tidy/llvm/LLVMTidyModule.cpp
  clang-tidy/llvm/ProblematicStaticsCheck.cpp
  clang-tidy/llvm/ProblematicStaticsCheck.h
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/llvm-problematic-statics.rst
  test/clang-tidy/Inputs/Headers/include/ProblematicStatics.h
  test/clang-tidy/llvm-problematic-statics.cpp

Index: test/clang-tidy/llvm-problematic-statics.cpp
===
--- test/clang-tidy/llvm-problematic-statics.cpp
+++ test/clang-tidy/llvm-problematic-statics.cpp
@@ -0,0 +1,4 @@
+// RUN: %check_clang_tidy %s llvm-problematic-statics %t -- -header-filter='.*' -- -I %S/Inputs/Headers
+
+#include "include/ProblematicStatics.h"
+// CHECK-MESSAGES: ProblematicStatics.h:7:5: warning: address of static local variable 'index' may not be identical across library boundaries [llvm-problematic-statics]
Index: test/clang-tidy/Inputs/Headers/include/ProblematicStatics.h
===
--- test/clang-tidy/Inputs/Headers/include/ProblematicStatics.h
+++ test/clang-tidy/Inputs/Headers/include/ProblematicStatics.h
@@ -0,0 +1,11 @@
+#ifndef PROBLEMATIC_STATICS_H
+#define PROBLEMATIC_STATICS_H
+
+struct ProgramStateTrait {
+  static void *GDMIndex() {
+static int index = 0;
+return 
+  }
+};
+
+#endif
Index: docs/clang-tidy/checks/llvm-problematic-statics.rst
===
--- docs/clang-tidy/checks/llvm-problematic-statics.rst
+++ docs/clang-tidy/checks/llvm-problematic-statics.rst
@@ -0,0 +1,16 @@
+.. title:: clang-tidy - llvm-problematic-statics
+
+llvm-problematic-statics
+
+
+
+Looks for functions defined in "public" headers that return the address of a static local. These are not guaranteed to have the addresss across shared libraries and could be problematic for plugins.
+
+.. code-block:: c++
+
+  struct ProgramStateTrait {
+static void *GDMIndex() {
+static int index = 0;
+return 
+}
+  };
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -163,6 +163,7 @@
llvm-header-guard
llvm-include-order
llvm-namespace-comment
+   llvm-problematic-statics
llvm-twine-local
misc-definitions-in-headers
misc-misplaced-const
Index: clang-tidy/llvm/ProblematicStaticsCheck.h
===
--- clang-tidy/llvm/ProblematicStaticsCheck.h
+++ clang-tidy/llvm/ProblematicStaticsCheck.h
@@ -0,0 +1,33 @@
+//===--- ProblematicStaticsCheck.h - clang-tidy *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PROBLEMATIC_STATICS_CHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PROBLEMATIC_STATICS_CHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace llvm {
+
+/// Looks for static variables in LLVM headers that may pose problems for
+/// users in other shared libraries.
+class ProblematicStaticsCheck : public ClangTidyCheck {
+public:
+  using ClangTidyCheck::ClangTidyCheck;
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace llvm
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_PROBLEMATIC_STATICS_CHECK_H
Index: clang-tidy/llvm/ProblematicStaticsCheck.cpp
===
--- clang-tidy/llvm/ProblematicStaticsCheck.cpp
+++ clang-tidy/llvm/ProblematicStaticsCheck.cpp
@@ -0,0 +1,41 @@
+//===--- ProblematicStaticsCheck.cpp - clang-tidy -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: docs/clang-tidy/checks/google-objc-function-naming.rst:20
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+

This is not actually handled by the check, right? (You even have a test which 
confirms this.)

As far as I can tell, this check essentially looks for at least one capital 
letter and no `_` in function names, right?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> It seems you were well aware that you are changing the semantics of FP 
> operation here by ignoring the signaling/quiet portion of the immediate.

There's ongoing work to support code that accesses the floating point 
environment (see 
http://llvm.org/docs/LangRef.html#constrained-floating-point-intrinsics).  
There isn't any way to turn it on in clang yet, though.  And until that's 
implemented, LLVM can trigger arbitrary floating-point exceptions anywhere in 
your code, so there's no point to trying to generate a "quiet" compare.


Repository:
  rC Clang

https://reviews.llvm.org/D45616



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+  if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;

juliehockett wrote:
> aaron.ballman wrote:
> > juliehockett wrote:
> > > aaron.ballman wrote:
> > > > juliehockett wrote:
> > > > > aaron.ballman wrote:
> > > > > > Does this also work on platforms where the path separator is `\` 
> > > > > > instead of `/`? What about case insensitive file systems where it 
> > > > > > may be spelled `LiMiTs.H`? Does this properly handle a case like:
> > > > > > ```
> > > > > > #define LIMITS "fbl/limits.h"
> > > > > > #include LIMITS
> > > > > > ```
> > > > > > (Should add test cases for all of these scenarios.)
> > > > > Since this is a migration for our own codebase, we know we don't have 
> > > > > any code that uses any variation other than  and so 
> > > > > hardcoding that is acceptable to us here.
> > > > Then why should this check be a public one, rather than an internal 
> > > > check?
> > > I explained this on the other one, but for completeness: 
> > > 
> > > 
> > > So yes, this check is for a migration, and we would delete it once 
> > > regressions weren't possible. We would like the suite to be in upstream, 
> > > however, because we use the ToT llvm/clang/tools/etc, and don't want to 
> > > have to fork just to use clang-tidy for this sort of thing. Since 
> > > clang-tidy doesn't provide any way to have external checks to the tool 
> > > itself, upstreaming is the most ideal option.
> > > 
> > > Orthogonal to our particular build setup, it'd also be nice to have an 
> > > example of this sort of migration done by clang-tidy in-tree. There has 
> > > been a lot of discussion recently about doing migrations with clang-tidy, 
> > > but it's always describing an internal migration that uses a forked tree 
> > > and a private suite of checks that can't be released.
> > I don't think it's reasonable to expect the community to bear the 
> > maintenance burden for internal-only checks for an organization. I 
> > definitely understand the desire not to carry around a fork of clang-tidy 
> > for this, but that doesn't seem like a good reason for us to spend cycles 
> > reviewing these patches, maintaining them, and then eventually removing 
> > them.
> > 
> > We have some precedent in that we have clang-tidy checks for llvm coding 
> > standards or google coding standards, etc but those are also used outside 
> > of the particular community for which they're named. In this case, I don't 
> > think the functionality is useful to anyone except Google. Is that correct?
> That makes sense in terms of investment from you and others (though I do 
> appreciate the effort you've put in for reviewing it thus far), but I (and my 
> team) are willing to bear the burden of reviewing/maintaining/removing them. 
> If clang-tidy provided a way to tack on a set of external checks without 
> forking, we'd happily do that, but that's not really possible right now.
> 
> Would it be reasonable to do the review/maintenance/removal of these 
> migration checks from within our team, such that we're not asking you to 
> spend cycles on them?
> 
> 
> Would it be reasonable to do the review/maintenance/removal of these 
> migration checks from within our team, such that we're not asking you to 
> spend cycles on them?

Personally, I don't think so. It still impose burdens on users of clang-tidy by 
having checks that are not going to be useful to anyone except people from one 
company. These checks appears in clang-tidy list checks, contribute to binary 
sizes, are documented in user-facing places, users may enable them needlessly 
and report bugs against them, etc.

I think that we shouldn't have any checks that are only useful for 
(effectively) a single company -- I don't think that scales well, and I don't 
think it benefits the community of users. I think a better approach is to 
introduce plugins for clang-tidy checks. It's definitely a larger project than 
what you're proposing and I understand if you don't want to tackle it, but it 
benefits the entire community and solves the problem you want to solve.


https://reviews.llvm.org/D54169



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

[PATCH] D45616: [X86] Lower _mm[256|512]_cmp[.]_mask intrinsics to native llvm IR

[Nikita Astafev via Phabricator via cfe-commits]

nastafev added a comment.

Hello. It seems you were well aware that you are changing the semantics of FP 
operation here by ignoring the signaling/quiet portion of the immediate. But 
what shall the user do now? There was no way to force quiet FP comparison 
behavior in C language, so intrinsics and reliance on quiet compare (and SAE 
bit in AVX512) were natural way of forcing it. And now you are taking them out. 
Is there a switch that could prevent this optimization? I think it could be 
more tolerable if you only did this under fast-math.


Repository:
  rC Clang

https://reviews.llvm.org/D45616



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

r346343 - [OPENMP]Fix handling of the globals during compilation for the device.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Nov  7 11:11:14 2018
New Revision: 346343

URL: http://llvm.org/viewvc/llvm-project?rev=346343=rev
Log:
[OPENMP]Fix handling of the globals during compilation for the device.

Fixed lookup for the target regions in unused virtual functions + fixed
processing of the global variables not marked as declare target but
emitted during debug info emission.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/test/OpenMP/declare_target_codegen.cpp
cfe/trunk/test/OpenMP/target_messages.cpp
cfe/trunk/test/OpenMP/target_parallel_debug_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=346343=346342=346343=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Nov  7 11:11:14 2018
@@ -1223,6 +1223,17 @@ CGOpenMPRuntime::CGOpenMPRuntime(CodeGen
 
 void CGOpenMPRuntime::clear() {
   InternalVars.clear();
+  // Clean non-target variable declarations possibly used only in debug info.
+  for (const auto  : EmittedNonTargetVariables) {
+if (!Data.getValue().pointsToAliveValue())
+  continue;
+auto *GV = dyn_cast(Data.getValue());
+if (!GV)
+  continue;
+if (!GV->isDeclaration() || GV->getNumUses() > 0)
+  continue;
+GV->eraseFromParent();
+  }
 }
 
 std::string CGOpenMPRuntime::getName(ArrayRef Parts) const {
@@ -2501,8 +2512,7 @@ llvm::Function *CGOpenMPRuntime::emitThr
 return nullptr;
 
   VD = VD->getDefinition(CGM.getContext());
-  if (VD && ThreadPrivateWithDefinition.count(VD) == 0) {
-ThreadPrivateWithDefinition.insert(VD);
+  if (VD && ThreadPrivateWithDefinition.insert(CGM.getMangledName(VD)).second) 
{
 QualType ASTTy = VD->getType();
 
 llvm::Value *Ctor = nullptr, *CopyCtor = nullptr, *Dtor = nullptr;
@@ -2648,7 +2658,7 @@ bool CGOpenMPRuntime::emitDeclareTargetV
   if (!Res || *Res == OMPDeclareTargetDeclAttr::MT_Link)
 return CGM.getLangOpts().OpenMPIsDevice;
   VD = VD->getDefinition(CGM.getContext());
-  if (VD && !DeclareTargetWithDefinition.insert(VD).second)
+  if (VD && !DeclareTargetWithDefinition.insert(CGM.getMangledName(VD)).second)
 return CGM.getLangOpts().OpenMPIsDevice;
 
   QualType ASTTy = VD->getType();
@@ -3924,6 +3934,8 @@ void CGOpenMPRuntime::createOffloadEntri
   llvm::LLVMContext  = M.getContext();
   SmallVector
   OrderedEntries(OffloadEntriesInfoManager.size());
+  llvm::SmallVector ParentFunctions(
+  OffloadEntriesInfoManager.size());
 
   // Auxiliary methods to create metadata values and strings.
   auto & = [this](unsigned V) {
@@ -3938,7 +3950,7 @@ void CGOpenMPRuntime::createOffloadEntri
 
   // Create function that emits metadata for each target region entry;
   auto & =
-  [, MD, , , ](
+  [, MD, , , , ](
   unsigned DeviceID, unsigned FileID, StringRef ParentName,
   unsigned Line,
   const OffloadEntriesInfoManagerTy::OffloadEntryInfoTargetRegion ) {
@@ -3958,6 +3970,7 @@ void CGOpenMPRuntime::createOffloadEntri
 
 // Save this entry in the right position of the ordered entries array.
 OrderedEntries[E.getOrder()] = 
+ParentFunctions[E.getOrder()] = ParentName;
 
 // Add metadata to the named metadata node.
 MD->addOperand(llvm::MDNode::get(C, Ops));
@@ -3999,6 +4012,10 @@ void CGOpenMPRuntime::createOffloadEntri
 
dyn_cast(
 E)) {
   if (!CE->getID() || !CE->getAddress()) {
+// Do not blame the entry if the parent funtion is not emitted.
+StringRef FnName = ParentFunctions[CE->getOrder()];
+if (!CGM.GetGlobalValue(FnName))
+  continue;
 unsigned DiagID = CGM.getDiags().getCustomDiagID(
 DiagnosticsEngine::Error,
 "Offloading entry for target region is incorrect: either the "
@@ -8425,14 +8442,15 @@ bool CGOpenMPRuntime::emitTargetFunction
   if (!CGM.getLangOpts().OpenMPIsDevice)
 return false;
 
-  // Try to detect target regions in the function.
   const ValueDecl *VD = cast(GD.getDecl());
+  StringRef Name = CGM.getMangledName(GD);
+  // Try to detect target regions in the function.
   if (const auto *FD = dyn_cast(VD))
-scanForTargetRegionsFunctions(FD->getBody(), CGM.getMangledName(GD));
+scanForTargetRegionsFunctions(FD->getBody(), Name);
 
   // Do not to emit function if it is not marked as declare target.
   return !OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD) &&
- AlreadyEmittedTargetFunctions.count(VD->getCanonicalDecl()) == 0;
+ AlreadyEmittedTargetFunctions.count(Name) == 0;
 }
 
 bool CGOpenMPRuntime::emitTargetGlobalVariable(GlobalDecl GD) {
@@ -8469,54 +8487,62 @@ bool CGOpenMPRuntime::emitTargetGlobalVa
 
 void 

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D54187#1290432, @rnk wrote:

> I hadn't realized that Dexter knew how to drive VS tools. I'll have to go 
> read more and get back to you all.
>
> I think it would be more promising than attempting to come up with a new 
> llgdb.py-like abstraction for cdb. Specifically, abstracting over setting 
> breakpoints and reformatting output is what makes that difficult. Everything 
> can of course be done with enough effort, but especially in testing, it often 
> makes sense to trade off duplication between test cases for ease of use.


Note that WinDbg (specifically) is an important use case, and uses a different 
debug engine than VS.  So Dexter would (at the very least) need to be extended 
to support WinDbg (which has the same debugging engine as cdb).  But I agree 
it's worth trying out and seeing what kind of test cases we can and can't fit 
into it.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D54187#1290340, @gbedwell wrote:

> (Dexter) will step through every line of the program it can, collecting info 
> about each step until it reaches the program exit.  It won't currently 
> produce a pass/fail, but rather a score.  That is, if it observes that 'x' is 
> visible and has the value 42 it'll get a perfect score.


Seems like it would easy enough to base pass/fail on getting a perfect score, 
for the purpose of debuginfo-tests.


https://reviews.llvm.org/D54187



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

[PATCH] D53952: [clang-format] Support breaking consecutive string literals for TableGen

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added a comment.

@djasper -- ping


Repository:
  rC Clang

https://reviews.llvm.org/D53952



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I hadn't realized that Dexter knew how to drive VS tools. I'll have to go read 
more and get back to you all.

I think it would be more promising than attempting to come up with a new 
llgdb.py-like abstraction for cdb. Specifically, abstracting over setting 
breakpoints and reformatting output is what makes that difficult. Everything 
can of course be done with enough effort, but especially in testing, it often 
makes sense to trade off duplication between test cases for ease of use.


https://reviews.llvm.org/D54187



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

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added a comment.

I reverted this change in r346338, as it breaks Index/opencl-types.cl LIT test. 
See 
http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/21503/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Aopencl-types.cl


Repository:
  rL LLVM

https://reviews.llvm.org/D51484



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

[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clangd/index/Index.h:84
 inline bool operator==(const SymbolLocation , const SymbolLocation ) {
-  return std::tie(L.FileURI, L.Start, L.End) ==
- std::tie(R.FileURI, R.Start, R.End);
+  return std::make_tuple(llvm::StringRef(L.FileURI), L.Start, L.End) ==
+ std::make_tuple(llvm::StringRef(R.FileURI), R.Start, R.End);

The `StringRef(const char*)` constructor calls `strlen`. Do you believe this 
does not have noticeable performance issue?

(A minor point: `tuple::operator<` calls `operator<` of its components in a not 
very efficient way. The compiler does not know `compare(a, b) < 0` implies 
`compare(b, a) > 0` and `strcmp` may end up being called twice.)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53427



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

r346338 - Revert r346326 [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation

[Andrew Savonichev via cfe-commits]

Author: asavonic
Date: Wed Nov  7 10:34:19 2018
New Revision: 346338

URL: http://llvm.org/viewvc/llvm-project?rev=346338=rev
Log:
Revert r346326 [OpenCL] Add support of 
cl_intel_device_side_avc_motion_estimation

This patch breaks Index/opencl-types.cl LIT test:

Script:
--
: 'RUN: at line 1';   stage1/bin/c-index-test -test-print-type 
llvm/tools/clang/test/Index/opencl-types.cl -cl-std=CL2.0 | 
stage1/bin/FileCheck llvm/tools/clang/test/Index/opencl-types.cl
--
Command Output (stderr):
--
llvm/tools/clang/test/Index/opencl-types.cl:3:26: warning: unsupported OpenCL 
extension 'cl_khr_fp16' - ignoring [-Wignored-pragmas]
llvm/tools/clang/test/Index/opencl-types.cl:4:26: warning: unsupported OpenCL 
extension 'cl_khr_fp64' - ignoring [-Wignored-pragmas]
llvm/tools/clang/test/Index/opencl-types.cl:8:9: error: use of type 'double' 
requires cl_khr_fp64 extension to be enabled
llvm/tools/clang/test/Index/opencl-types.cl:11:8: error: declaring variable of 
type 'half' is not allowed
llvm/tools/clang/test/Index/opencl-types.cl:15:3: error: use of type 'double' 
requires cl_khr_fp64 extension to be enabled
llvm/tools/clang/test/Index/opencl-types.cl:16:3: error: use of type 'double4' 
(vector of 4 'double' values) requires cl_khr_fp64 extension to be enabled
llvm/tools/clang/test/Index/opencl-types.cl:26:26: warning: unsupported OpenCL 
extension 'cl_khr_gl_msaa_sharing' - ignoring [-Wignored-pragmas]
llvm/tools/clang/test/Index/opencl-types.cl:35:44: error: use of type 
'__read_only image2d_msaa_t' requires cl_khr_gl_msaa_sharing extension to be 
enabled
llvm/tools/clang/test/Index/opencl-types.cl:36:49: error: use of type 
'__read_only image2d_array_msaa_t' requires cl_khr_gl_msaa_sharing extension to 
be enabled
llvm/tools/clang/test/Index/opencl-types.cl:37:49: error: use of type 
'__read_only image2d_msaa_depth_t' requires cl_khr_gl_msaa_sharing extension to 
be enabled
llvm/tools/clang/test/Index/opencl-types.cl:38:54: error: use of type 
'__read_only image2d_array_msaa_depth_t' requires cl_khr_gl_msaa_sharing 
extension to be enabled


Removed:
cfe/trunk/include/clang/Basic/OpenCLExtensionTypes.def
cfe/trunk/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
cfe/trunk/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/OpenCLExtensions.def
cfe/trunk/include/clang/Sema/Initialization.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/module.modulemap
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/NSAPI.cpp
cfe/trunk/lib/AST/PrintfFormatString.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypeLoc.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Headers/opencl-c.h
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Headers/opencl-c-header.cl
cfe/trunk/test/Index/opencl-types.cl
cfe/trunk/test/SemaOpenCL/extension-version.cl
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=346338=346337=346338=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Wed Nov  7 10:34:19 2018
@@ -3297,21 +3297,7 @@ enum CXTypeKind {
 
   CXType_ObjCObject = 161,
   CXType_ObjCTypeParam = 162,
-  CXType_Attributed = 163,
-
-  CXType_OCLIntelSubgroupAVCMcePayload = 164,
-  CXType_OCLIntelSubgroupAVCImePayload = 165,
-  CXType_OCLIntelSubgroupAVCRefPayload = 166,
-  CXType_OCLIntelSubgroupAVCSicPayload = 167,
-  CXType_OCLIntelSubgroupAVCMceResult = 168,
-  CXType_OCLIntelSubgroupAVCImeResult = 169,
-  CXType_OCLIntelSubgroupAVCRefResult = 170,
-  CXType_OCLIntelSubgroupAVCSicResult = 171,
-  CXType_OCLIntelSubgroupAVCImeResultSingleRefStreamout = 172,
-  CXType_OCLIntelSubgroupAVCImeResultDualRefStreamout = 173,
-  CXType_OCLIntelSubgroupAVCImeSingleRefStreamin = 174,
-
-  CXType_OCLIntelSubgroupAVCImeDualRefStreamin = 175
+  CXType_Attributed = 163
 };
 
 /**

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=346338=346337=346338=diff

[PATCH] D54214: [RISCV] Set triple based on -march flag

[Simon Cook via Phabricator via cfe-commits]

simoncook created this revision.
simoncook added a reviewer: asb.
Herald added subscribers: cfe-commits, jocewei, PkmX, rkruppe, the_o, 
brucehoult, MartinMosbeck, rogfer01, mgrang, edward-jones, zzheng, jrtc27, 
shiva0217, kito-cheng, niosHD, sabuasal, apazos, johnrusso, rbar.

For RISC-V the value provided to -march should determine whether to compile for 
32- or 64-bit RISC-V irrespective of the target provided to the Clang driver. 
This adds a test for this flag for RISC-V and sets the Target architecture 
correctly in these cases.


Repository:
  rC Clang

https://reviews.llvm.org/D54214

Files:
  lib/Driver/Driver.cpp
  test/Driver/riscv-arch.c


Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -506,6 +506,17 @@
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 


Index: test/Driver/riscv-arch.c
===
--- test/Driver/riscv-arch.c
+++ test/Driver/riscv-arch.c
@@ -315,3 +315,15 @@
 // RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-X-S-SX-INVAL %s
 // RV32-X-S-SX-INVAL: error: invalid arch name 'rv32ixabc_sdef_sxghi',
 // RV32-X-S-SX-INVAL: unsupported non-standard user-level extension 'xabc'
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv32i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV32-TARGET %s
+// RV32-TARGET: "-triple" "riscv32-unknown-unknown-elf"
+
+// RUN: %clang -target riscv32-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RUN: %clang -target riscv64-unknown-elf -march=rv64i -### %s \
+// RUN: -fsyntax-only 2>&1 | FileCheck -check-prefix=RV64-TARGET %s
+// RV64-TARGET: "-triple" "riscv64-unknown-unknown-elf"
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -506,6 +506,17 @@
 }
   }
 
+  // If target is RISC-V adjust the target triple according to
+  // provided architecture name
+  A = Args.getLastArg(options::OPT_march_EQ);
+  if (A && Target.isRISCV()) {
+StringRef ArchName = A->getValue();
+if (ArchName.startswith_lower("rv32"))
+  Target.setArch(llvm::Triple::riscv32);
+else if (ArchName.startswith_lower("rv64"))
+  Target.setArch(llvm::Triple::riscv64);
+  }
+
   return Target;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

@aaron.ballman


https://reviews.llvm.org/D52835



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Greg Bedwell via Phabricator via cfe-commits]

gbedwell added a comment.

In https://reviews.llvm.org/D54187#1290282, @probinson wrote:

> +gbedwell
>
> Just to throw the idea out there, why not abandon debuginfo-tests entirely 
> and try using Dexter for this. Dexter's design center is debug-info quality, 
> not correctness, but it already knows how to drive several different 
> debuggers on multiple platforms.


I initially looked at the possibility of expanding debuginfo-tests rather than 
reinventing the wheel with dexter, but in the end decided that the aims were 
different enough that it would be messy to shoehorn one into the other.  I'm 
open to it though.  I think there's a case to be made for potentially sharing 
some of the low-level "this is my generalized abstraction layer for how I can 
talk to lots of different debuggers" code as a library with different client 
tools, but I wouldn't want to bog down this discussion with that.

I'm wary of going too far off-topic for this review as this is more related to 
the 'in the long run' aside rather than the review itself, but as I was asked 
for a few more details, for reference, dexter drives LLDB via the Python API ( 
https://lldb.llvm.org/python-reference.html ) and Visual Studio via the DTE 
interface ( 
https://docs.microsoft.com/en-us/dotnet/api/envdte.dte?view=visualstudiosdk-2017
 ).

For something like the hello.c example in this review, and equivalent dexter 
test would look like:

  #include 
  int main() {
printf("hello world\n");
int x = 42;  // DexWatch('x')
  }
  
  // DexExpectWatchValue('x', '42')

It will step through every line of the program it can, collecting info about 
each step until it reaches the program exit.  It won't currently produce a 
pass/fail, but rather a score.  That is, if it observes that 'x' is visible and 
has the value 42 it'll get a perfect score.  If it's not visible it'll get a 
worse score.  If it's visible, but tells me that the value is 43, it'll get a 
worse score still (on the assumption that actively lying to the user is a much 
worse sin than shrugging and saying "I don't know").
The best place to look for real examples is probably Jeremy and Tom's test 
corpus which they've been using to raise bugs against LLVM: 
https://github.com/SNSystems/dexter/pull/4/commits/825c2c9aaccc0fb38ede2c8732152d6500c22ac5
 
(see also 
http://llvm.org/devmtg/2018-04/slides/Bedwell-Measuring_the_User_Debugging_Experience_poster.png
 )


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54187#1290317, @zturner wrote:

> In https://reviews.llvm.org/D54187#1290297, @aprantl wrote:
>
> > In https://reviews.llvm.org/D54187#1290293, @zturner wrote:
> >
> > > Especially since as far as I can tell, nobody has even run 
> > > debuginfo-tests since late August, because it was actually broken by 
> > > r341135 on August 30 (fixed in r346060 yesterday)
> >
> >
> > Can you please refrain from making such general statements? They distract 
> > from the discussion.
> >
> > http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/ runs 
> > the debuginfo-tests continuously. There is a configuration issue that 
> > prevents it from running the ASAN subset of the tests at the moment.
>
>
> The issue introduced by r341135 doesn't seem related to ASAN unless I'm 
> misunderstanding the issue.  Were debuginfo-tests passing on that bot even 
> before r346060 landed?


r341135 is a fix to `lit.site.cfg.py.in` my best guess is that because 
clang-stage1-cmake-RA-expensive runs the debuginfo-tests inside the 
llvm/tools/clang/test/debuginfo-tests subdirectory that doesn't get used in 
that configuration?


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D54187#1290297, @aprantl wrote:

> In https://reviews.llvm.org/D54187#1290293, @zturner wrote:
>
> > Especially since as far as I can tell, nobody has even run debuginfo-tests 
> > since late August, because it was actually broken by r341135 on August 30 
> > (fixed in r346060 yesterday)
>
>
> Can you please refrain from making such general statements? They distract 
> from the discussion.
>
> http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/ runs the 
> debuginfo-tests continuously. There is a configuration issue that prevents it 
> from running the ASAN subset of the tests at the moment.


The issue introduced by r341135 doesn't seem related to ASAN unless I'm 
misunderstanding the issue.  Were debuginfo-tests passing on that bot even 
before r346060 landed?


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54187#1290298, @probinson wrote:

> In https://reviews.llvm.org/D54187#1290294, @zturner wrote:
>
> > In https://reviews.llvm.org/D54187#1290282, @probinson wrote:
> >
> > > +gbedwell
> > >
> > > Just to throw the idea out there, why not abandon debuginfo-tests 
> > > entirely and try using Dexter for this. Dexter's design center is 
> > > debug-info quality, not correctness, but it already knows how to drive 
> > > several different debuggers on multiple platforms.
> > >  Dexter would have to become an LLVM project tool, and I'm not sure how 
> > > Sony management feels about that, but I think this would be an awesome 
> > > use-case.
> >
> >
> > Depends where we draw the distinction between quality and correctness.  We 
> > specifically want a way to test that when we fix a correctness bug, it's 
> > actually fixed against Microsoft debuggers.
>
>
> Dexter knows how to drive Visual Studio tools already, as well as gdb and 
> (maybe) lldb.  I have never looked inside it but I'd expect Greg to have made 
> it straightforward to add new tools.


If it fits our use-cases, that sounds like a great idea. Are there some 
examples of dexter input/outputs we could take a look at to see how good of a 
fit it would be?


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In https://reviews.llvm.org/D54187#1290293, @zturner wrote:

> Especially since as far as I can tell, nobody has even run debuginfo-tests 
> since late August, because it was actually broken by r341135 on August 30 
> (fixed in r346060 yesterday)


Can you please refrain from making such general statements? They distract from 
the discussion.

http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-expensive/ runs the 
debuginfo-tests continuously. There is a configuration issue that prevents it 
from running the ASAN subset of the tests at the moment.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In https://reviews.llvm.org/D54187#1290294, @zturner wrote:

> In https://reviews.llvm.org/D54187#1290282, @probinson wrote:
>
> > +gbedwell
> >
> > Just to throw the idea out there, why not abandon debuginfo-tests entirely 
> > and try using Dexter for this. Dexter's design center is debug-info 
> > quality, not correctness, but it already knows how to drive several 
> > different debuggers on multiple platforms.
> >  Dexter would have to become an LLVM project tool, and I'm not sure how 
> > Sony management feels about that, but I think this would be an awesome 
> > use-case.
>
>
> Depends where we draw the distinction between quality and correctness.  We 
> specifically want a way to test that when we fix a correctness bug, it's 
> actually fixed against Microsoft debuggers.


Dexter knows how to drive Visual Studio tools already, as well as gdb and 
(maybe) lldb.  I have never looked inside it but I'd expect Greg to have made 
it straightforward to add new tools.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D54187#1290282, @probinson wrote:

> +gbedwell
>
> Just to throw the idea out there, why not abandon debuginfo-tests entirely 
> and try using Dexter for this. Dexter's design center is debug-info quality, 
> not correctness, but it already knows how to drive several different 
> debuggers on multiple platforms.
>  Dexter would have to become an LLVM project tool, and I'm not sure how Sony 
> management feels about that, but I think this would be an awesome use-case.


Depends where we draw the distinction between quality and correctness.  We 
specifically want a way to test that when we fix a correctness bug, it's 
actually fixed against Microsoft debuggers.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Zachary Turner via Phabricator via cfe-commits]

zturner added a comment.

In https://reviews.llvm.org/D54187#1290247, @aprantl wrote:

> > I think the only way to realistically make this work for all platforms 
> > would be to separate the source file from the input/output. The source file 
> > would be the test case, and if you wanted to support a different debugger 
> > you would need to supply a different input/output file..
>
> I don't necessarily agree with that statement. The debuginfo-tests use only a 
> very small subset of debugger functionality that includes
>
> - setting breakpoints
> - printing the value of integer variables
> - continuing to the next breakpoint
>
>   I'm genuinely curious what is so different about the Windows debugger that 
> it couldn't be wrapped in a translation layer like `llgdb.py` that abstracts 
> these three operations. This would cover a large set of the existing tests. 
> I'm fine with having a few extra tests that test something that only works on 
> a specific platform here and there, but I really don't want us to grow 
> separate platform-specific testsuites. Inevitably, someone working on 
> platform A will fix a general bug in LLVM and then only add a test for 
> platform A and that's bad for everyone. What I'm trying to avoid is a 
> situation like the debug info tests in LLVM that are almost entirely 
> x86-specific, even if they are testing stuff that happens at the IR level.


I don't think we want tests that are limited to that amount of simplicity.  We 
don't just want to print integers, we'd like to also print callstacks.  And 
objects.  And other things that aren't integers.  A cdb call stack looks like 
this:

00c4`aa35f730 7ffc`8944bc72 : 0294`3b6e0ae8 0294`3b6e0a98 
` ` : MSVCP140D!_Cnd_wait+0x20 
[f:\dd\vctools\crt\crtw32\stdcpp\thr\cond.c @ 106] 
08 00c4`aa35f760 7ffc`89450a54 : 0294`3b6e0ae8 0294`3b6e0a98 
` ` : liblldb!std::_Cnd_waitX+0x32 [c:\program 
files (x86)\microsoft visual 
studio\2017\professional\vc\tools\msvc\14.15.26726\include\thr\xthread @ 97] 
09 00c4`aa35f790 7ffc`8944122d : 0294`3b6e0ae8 00c4`aa35f828 
` ` : 
liblldb!std::condition_variable::wait+0x54 [c:\program files (x86)\microsoft 
visual studio\2017\professional\vc\tools\msvc\14.15.26726\include\mutex @ 714] 
0a 00c4`aa35f7d0 7ffc`89440897 : 0294`3b6e09e0 00c4`aa35fb78 
` ` : 
liblldb!lldb_private::Listener::GetEventInternal+0x20d 
[d:\src\llvm-mono\lldb\source\core\listener.cpp @ 367] 
0b 00c4`aa35f8e0 7ffc`8939b70e : 0294`3b6e09e0 00c4`aa35fa78 
00c4`aa35fb78 ` : 
liblldb!lldb_private::Listener::GetEvent+0x57 
[d:\src\llvm-mono\lldb\source\core\listener.cpp @ 404] 
0c 00c4`aa35f930 7ffc`8939b118 : 0294`3b6de700 ` 
` ` : 
liblldb!lldb_private::Debugger::DefaultEventHandler+0x27e 
[d:\src\llvm-mono\lldb\source\core\debugger.cpp @ 1546] 
0d 00c4`aa35fc30 7ffc`8960cf62 : 0294`3b6de700 0294`0001 
` ` : 
liblldb!lldb_private::Debugger::EventHandlerThread+0x28 
[d:\src\llvm-mono\lldb\source\core\debugger.cpp @ 1600]

Then you need a way to abstract over the command lines needed to generate the 
executables (`clang-cl` and `lld-link` use different flag syntax than `clang++` 
etc).  Then there's the issue of when a test is using Microsoft specific 
extensions.  At the end of all of this, it's going to take a lot of effort to 
implement this layer of abstraction that is ultimately going to be subverted 
for a large number of tests anyway when something doesn't fit cleanly into the 
abstraction.  I think there is also the issue of how much actual overlap there 
is between the set of interesting test cases for DWARF / Itanium ABI and PDB / 
Microsoft ABI.  Many issues that we would want to add tests for would be 
surrounding fixes specific to the way we emit the PDB or that are constructed 
due to knowledge of how we emit CodeView in certain situations.  And none of 
those test cases will be interesting to abstract over.

Finally, by strictly limiting the amount of output we're checking against, we 
can be too permissive.  For example, we have a command up there that checks 
against the line `g`.  But this will match any line that has the letter `g` in 
it, which is extremely permissive.  It would be more useful if the line said 
`DEBUGGER: 0:000> g`

The line that says `CHECK: a = 0n2` will also match if the variable happens to 
be 23 or any number of other values.  So we'd really like to be able to be more 
precise here.

So I'm not convinced there is a lot of added value, or at the very least, I 
don't believe it to be worth the cost.  Especially since as far as I can tell, 
nobody has even run debuginfo-tests since late August, because it was actually 
broken by r341135 on August 30 (fixed in 

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Paul Robinson via Phabricator via cfe-commits]

probinson added subscribers: gbedwell, probinson.
probinson added a comment.

+gbedwell

Just to throw the idea out there, why not abandon debuginfo-tests entirely and 
try using Dexter for this. Dexter's design center is debug-info quality, not 
correctness, but it already knows how to drive several different debuggers on 
multiple platforms.
Dexter would have to become an LLVM project tool, and I'm not sure how Sony 
management feels about that, but I think this would be an awesome use-case.


https://reviews.llvm.org/D54187



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

[PATCH] D54187: Add debuginfo-tests that use cdb on Windows

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

> I think the only way to realistically make this work for all platforms would 
> be to separate the source file from the input/output. The source file would 
> be the test case, and if you wanted to support a different debugger you would 
> need to supply a different input/output file..

I don't necessarily agree with that statement. The debuginfo-tests use only a 
very small subset of debugger functionality that includes

- setting breakpoints
- printing the value of integer variables
- continuing to the next breakpoint

I'm genuinely curious what is so different about the Windows debugger that it 
couldn't be wrapped in a translation layer like `llgdb.py` that abstracts these 
three operations. This would cover a large set of the existing tests. I'm fine 
with having a few extra tests that test something that only works on a specific 
platform here and there, but I really don't want us to grow separate 
platform-specific testsuites. Inevitably, someone working on platform A will 
fix a general bug in LLVM and then only add a test for platform A and that's 
bad for everyone. What I'm trying to avoid is a situation like the debug info 
tests in LLVM that are almost entirely x86-specific, even if they are testing 
stuff that happens at the IR level.


https://reviews.llvm.org/D54187



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

[PATCH] D54169: [clang-tidy] Zircon ->

[Julie Hockett via Phabricator via cfe-commits]

juliehockett added inline comments.



Comment at: clang-tools-extra/clang-tidy/zircon/FblLimitsCheck.cpp:47
+SrcMgr::CharacteristicKind FileType) {
+  if (FileName == "fbl/limits.h") {
+unsigned End = std::strcspn(SM.getCharacterData(HashLoc), "\n") + 1;

aaron.ballman wrote:
> juliehockett wrote:
> > aaron.ballman wrote:
> > > juliehockett wrote:
> > > > aaron.ballman wrote:
> > > > > Does this also work on platforms where the path separator is `\` 
> > > > > instead of `/`? What about case insensitive file systems where it may 
> > > > > be spelled `LiMiTs.H`? Does this properly handle a case like:
> > > > > ```
> > > > > #define LIMITS "fbl/limits.h"
> > > > > #include LIMITS
> > > > > ```
> > > > > (Should add test cases for all of these scenarios.)
> > > > Since this is a migration for our own codebase, we know we don't have 
> > > > any code that uses any variation other than  and so 
> > > > hardcoding that is acceptable to us here.
> > > Then why should this check be a public one, rather than an internal check?
> > I explained this on the other one, but for completeness: 
> > 
> > 
> > So yes, this check is for a migration, and we would delete it once 
> > regressions weren't possible. We would like the suite to be in upstream, 
> > however, because we use the ToT llvm/clang/tools/etc, and don't want to 
> > have to fork just to use clang-tidy for this sort of thing. Since 
> > clang-tidy doesn't provide any way to have external checks to the tool 
> > itself, upstreaming is the most ideal option.
> > 
> > Orthogonal to our particular build setup, it'd also be nice to have an 
> > example of this sort of migration done by clang-tidy in-tree. There has 
> > been a lot of discussion recently about doing migrations with clang-tidy, 
> > but it's always describing an internal migration that uses a forked tree 
> > and a private suite of checks that can't be released.
> I don't think it's reasonable to expect the community to bear the maintenance 
> burden for internal-only checks for an organization. I definitely understand 
> the desire not to carry around a fork of clang-tidy for this, but that 
> doesn't seem like a good reason for us to spend cycles reviewing these 
> patches, maintaining them, and then eventually removing them.
> 
> We have some precedent in that we have clang-tidy checks for llvm coding 
> standards or google coding standards, etc but those are also used outside of 
> the particular community for which they're named. In this case, I don't think 
> the functionality is useful to anyone except Google. Is that correct?
That makes sense in terms of investment from you and others (though I do 
appreciate the effort you've put in for reviewing it thus far), but I (and my 
team) are willing to bear the burden of reviewing/maintaining/removing them. If 
clang-tidy provided a way to tack on a set of external checks without forking, 
we'd happily do that, but that's not really possible right now.

Would it be reasonable to do the review/maintenance/removal of these migration 
checks from within our team, such that we're not asking you to spend cycles on 
them?




https://reviews.llvm.org/D54169



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

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I think this is in a good shape as initial patch!




Comment at: clangd/ClangdUnit.cpp:168
+// The placeholder check here does not use hasAncestor() so is unaffected.
+CTOpts.Checks = "bugprone-sizeof-expression";
+CTContext.emplace(llvm::make_unique(

It seems we also support clang-tidy checks that analyze preprocessor-dependent 
properties. I think we can add a clang-tidy check to make sure `PPCallback` 
actually work, `google-readability-todo` is a good candidate. 

Ah just realized limitations (truncated `PPCallback` events) you wrote in the 
patch description, maybe mention them in the source comment, so that we won't 
forget in the future when reading the code.



Comment at: clangd/ClangdUnit.cpp:175
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(*Clang);

Maybe add the check names to the `Trace`?



Comment at: clangd/ClangdUnit.cpp:468
+  X##ModuleAnchorSource
+LINK_TIDY_MODULE(CERT);
+LINK_TIDY_MODULE(Abseil);

I'm curious how much does clangd binary size get increased.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204



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

[PATCH] D52296: [Clang] - Add -fdwarf-fission=split,single option.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Thanks! - though on reflection I'm going to invoke @echristo again about the 
naming. It's unfortunately a bit backwards that the pre-standard flag is 
-gsplit-dwarf and what we're proposing as the standard flag is -fdwarf-fission, 
when the DWARF standard doesn't use the word "Fission" at all (only "split 
DWARF").

Maybe it'd be easy enough/better to add the "=single" suffix to the existing 
"-gsplit-dwarf"? (as much as I'm still a bit confused by which debug options 
use a '-g' and which ones use a '-f', etc).

Really sorry to go back/forth/around about on this, George.




Comment at: lib/Driver/ToolChains/Clang.cpp:2999-3001
+StringRef Value = A->getOption().matches(options::OPT_fdwarf_fission_EQ)
+  ? A->getValue()
+  : "split";

Rather than creating a string in the case where there's no need for string 
matching/parsing, perhaps that could be avoided?

  if (!A->getOption().matches(options::OPT_fdwarf_fission_EQ))
return DwarfFissionKind::Split;

  StringRef Value = A->getValue();
  if (Value == "split")
  ...





Comment at: lib/Driver/ToolChains/Clang.cpp:3010-3011
+  }
+  if (ArgIndex)
+*ArgIndex = 0;
+  return DwarfFissionKind::None;

I'd probably either accept this as a precondition (assert(!ArgIndex || ArgIndex 
== 0)) or do this bit at the start of the function rather than the end here, to 
make it simpler/clearer that all paths assign some value to ArgIndex before 
exit of the function (even though that does mean checking/assigning ArgIndex 
unnecessarily in the case where the argument is given, etc)



Comment at: lib/Driver/ToolChains/Clang.cpp:5889
   const llvm::Triple  = getToolChain().getTriple();
-  if (Args.hasArg(options::OPT_gsplit_dwarf) &&
+  if ((getDebugFissionKind(D, Args) == DwarfFissionKind::Split) &&
   (T.isOSLinux() || T.isOSFuchsia())) {

Could having more than one call to getDebugFissionKind lead to the error 
message (for -fdwarf-fission=garbage) being printed multiple times? Or is this 
call on a distinct/mutually exclusive codepath to the other?


https://reviews.llvm.org/D52296



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

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Andrew Savonichev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346326: [OpenCL] Add support of 
cl_intel_device_side_avc_motion_estimation extension (authored by asavonic, 
committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51484?vs=172946=172956#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51484

Files:
  cfe/trunk/include/clang-c/Index.h
  cfe/trunk/include/clang/AST/ASTContext.h
  cfe/trunk/include/clang/AST/Type.h
  cfe/trunk/include/clang/Basic/OpenCLExtensionTypes.def
  cfe/trunk/include/clang/Basic/OpenCLExtensions.def
  cfe/trunk/include/clang/Sema/Initialization.h
  cfe/trunk/include/clang/Serialization/ASTBitCodes.h
  cfe/trunk/include/clang/module.modulemap
  cfe/trunk/lib/AST/ASTContext.cpp
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/lib/AST/ExprConstant.cpp
  cfe/trunk/lib/AST/ItaniumMangle.cpp
  cfe/trunk/lib/AST/MicrosoftMangle.cpp
  cfe/trunk/lib/AST/NSAPI.cpp
  cfe/trunk/lib/AST/PrintfFormatString.cpp
  cfe/trunk/lib/AST/Type.cpp
  cfe/trunk/lib/AST/TypeLoc.cpp
  cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
  cfe/trunk/lib/CodeGen/CGDebugInfo.h
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
  cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
  cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
  cfe/trunk/lib/Headers/opencl-c.h
  cfe/trunk/lib/Index/USRGeneration.cpp
  cfe/trunk/lib/Sema/Sema.cpp
  cfe/trunk/lib/Sema/SemaExpr.cpp
  cfe/trunk/lib/Sema/SemaInit.cpp
  cfe/trunk/lib/Serialization/ASTCommon.cpp
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  cfe/trunk/test/Headers/opencl-c-header.cl
  cfe/trunk/test/Index/opencl-types.cl
  cfe/trunk/test/SemaOpenCL/extension-version.cl
  cfe/trunk/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
  cfe/trunk/tools/libclang/CIndex.cpp
  cfe/trunk/tools/libclang/CXType.cpp

Index: cfe/trunk/include/clang-c/Index.h
===
--- cfe/trunk/include/clang-c/Index.h
+++ cfe/trunk/include/clang-c/Index.h
@@ -3297,7 +3297,21 @@
 
   CXType_ObjCObject = 161,
   CXType_ObjCTypeParam = 162,
-  CXType_Attributed = 163
+  CXType_Attributed = 163,
+
+  CXType_OCLIntelSubgroupAVCMcePayload = 164,
+  CXType_OCLIntelSubgroupAVCImePayload = 165,
+  CXType_OCLIntelSubgroupAVCRefPayload = 166,
+  CXType_OCLIntelSubgroupAVCSicPayload = 167,
+  CXType_OCLIntelSubgroupAVCMceResult = 168,
+  CXType_OCLIntelSubgroupAVCImeResult = 169,
+  CXType_OCLIntelSubgroupAVCRefResult = 170,
+  CXType_OCLIntelSubgroupAVCSicResult = 171,
+  CXType_OCLIntelSubgroupAVCImeResultSingleRefStreamout = 172,
+  CXType_OCLIntelSubgroupAVCImeResultDualRefStreamout = 173,
+  CXType_OCLIntelSubgroupAVCImeSingleRefStreamin = 174,
+
+  CXType_OCLIntelSubgroupAVCImeDualRefStreamin = 175
 };
 
 /**
Index: cfe/trunk/include/clang/AST/ASTContext.h
===
--- cfe/trunk/include/clang/AST/ASTContext.h
+++ cfe/trunk/include/clang/AST/ASTContext.h
@@ -1062,6 +1062,9 @@
   CanQualType OCLSamplerTy, OCLEventTy, OCLClkEventTy;
   CanQualType OCLQueueTy, OCLReserveIDTy;
   CanQualType OMPArraySectionTy;
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+  CanQualType Id##Ty;
+#include "clang/Basic/OpenCLExtensionTypes.def"
 
   // Types for deductions in C++0x [stmt.ranged]'s desugaring. Built on demand.
   mutable QualType AutoDeductTy; // Deduction against 'auto'.
Index: cfe/trunk/include/clang/AST/Type.h
===
--- cfe/trunk/include/clang/AST/Type.h
+++ cfe/trunk/include/clang/AST/Type.h
@@ -2046,6 +2046,13 @@
   bool isQueueT() const;// OpenCL queue_t
   bool isReserveIDT() const;// OpenCL reserve_id_t
 
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+  bool is##Id##Type() const;
+#include "clang/Basic/OpenCLExtensionTypes.def"
+  // Type defined in cl_intel_device_side_avc_motion_estimation OpenCL extension
+  bool isOCLIntelSubgroupAVCType() const;
+  bool isOCLExtOpaqueType() const;  // Any OpenCL extension type
+
   bool isPipeType() const;  // OpenCL pipe type
   bool isOpenCLSpecificType() const;// Any OpenCL specific type
 
@@ -2393,6 +2400,9 @@
 // OpenCL image types
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) Id,
 #include "clang/Basic/OpenCLImageTypes.def"
+// OpenCL extension types
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) Id,
+#include "clang/Basic/OpenCLExtensionTypes.def"
 // All other builtin types
 #define BUILTIN_TYPE(Id, SingletonId) Id,
 #define LAST_BUILTIN_TYPE(Id) LastKind = Id
@@ -6454,9 +6464,30 @@
   return isa(CanonicalType);
 }
 
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+  inline bool Type::is##Id##Type() const { \
+return isSpecificBuiltinType(BuiltinType::Id); \
+  }
+#include "clang/Basic/OpenCLExtensionTypes.def"
+

r346326 - [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Andrew Savonichev via cfe-commits]

Author: asavonic
Date: Wed Nov  7 07:44:01 2018
New Revision: 346326

URL: http://llvm.org/viewvc/llvm-project?rev=346326=rev
Log:
[OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

Summary:
Documentation can be found at 
https://www.khronos.org/registry/OpenCL/extensions/intel/cl_intel_device_side_avc_motion_estimation.txt

Patch by Kristina Bessonova


Reviewers: Anastasia, yaxunl, shafik

Reviewed By: Anastasia

Subscribers: arphaman, sidorovd, AlexeySotkin, krisb, bader, asavonic, 
cfe-commits

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

Added:
cfe/trunk/include/clang/Basic/OpenCLExtensionTypes.def
cfe/trunk/test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
cfe/trunk/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
Modified:
cfe/trunk/include/clang-c/Index.h
cfe/trunk/include/clang/AST/ASTContext.h
cfe/trunk/include/clang/AST/Type.h
cfe/trunk/include/clang/Basic/OpenCLExtensions.def
cfe/trunk/include/clang/Sema/Initialization.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/module.modulemap
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/AST/ItaniumMangle.cpp
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/lib/AST/NSAPI.cpp
cfe/trunk/lib/AST/PrintfFormatString.cpp
cfe/trunk/lib/AST/Type.cpp
cfe/trunk/lib/AST/TypeLoc.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/CodeGen/CGDebugInfo.h
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/CodeGen/CGOpenCLRuntime.cpp
cfe/trunk/lib/CodeGen/CodeGenTypes.cpp
cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
cfe/trunk/lib/Headers/opencl-c.h
cfe/trunk/lib/Index/USRGeneration.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/lib/Serialization/ASTCommon.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Headers/opencl-c-header.cl
cfe/trunk/test/Index/opencl-types.cl
cfe/trunk/test/SemaOpenCL/extension-version.cl
cfe/trunk/tools/libclang/CIndex.cpp
cfe/trunk/tools/libclang/CXType.cpp

Modified: cfe/trunk/include/clang-c/Index.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang-c/Index.h?rev=346326=346325=346326=diff
==
--- cfe/trunk/include/clang-c/Index.h (original)
+++ cfe/trunk/include/clang-c/Index.h Wed Nov  7 07:44:01 2018
@@ -3297,7 +3297,21 @@ enum CXTypeKind {
 
   CXType_ObjCObject = 161,
   CXType_ObjCTypeParam = 162,
-  CXType_Attributed = 163
+  CXType_Attributed = 163,
+
+  CXType_OCLIntelSubgroupAVCMcePayload = 164,
+  CXType_OCLIntelSubgroupAVCImePayload = 165,
+  CXType_OCLIntelSubgroupAVCRefPayload = 166,
+  CXType_OCLIntelSubgroupAVCSicPayload = 167,
+  CXType_OCLIntelSubgroupAVCMceResult = 168,
+  CXType_OCLIntelSubgroupAVCImeResult = 169,
+  CXType_OCLIntelSubgroupAVCRefResult = 170,
+  CXType_OCLIntelSubgroupAVCSicResult = 171,
+  CXType_OCLIntelSubgroupAVCImeResultSingleRefStreamout = 172,
+  CXType_OCLIntelSubgroupAVCImeResultDualRefStreamout = 173,
+  CXType_OCLIntelSubgroupAVCImeSingleRefStreamin = 174,
+
+  CXType_OCLIntelSubgroupAVCImeDualRefStreamin = 175
 };
 
 /**

Modified: cfe/trunk/include/clang/AST/ASTContext.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=346326=346325=346326=diff
==
--- cfe/trunk/include/clang/AST/ASTContext.h (original)
+++ cfe/trunk/include/clang/AST/ASTContext.h Wed Nov  7 07:44:01 2018
@@ -1062,6 +1062,9 @@ public:
   CanQualType OCLSamplerTy, OCLEventTy, OCLClkEventTy;
   CanQualType OCLQueueTy, OCLReserveIDTy;
   CanQualType OMPArraySectionTy;
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+  CanQualType Id##Ty;
+#include "clang/Basic/OpenCLExtensionTypes.def"
 
   // Types for deductions in C++0x [stmt.ranged]'s desugaring. Built on demand.
   mutable QualType AutoDeductTy; // Deduction against 'auto'.

Modified: cfe/trunk/include/clang/AST/Type.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Type.h?rev=346326=346325=346326=diff
==
--- cfe/trunk/include/clang/AST/Type.h (original)
+++ cfe/trunk/include/clang/AST/Type.h Wed Nov  7 07:44:01 2018
@@ -2046,6 +2046,13 @@ public:
   bool isQueueT() const;// OpenCL queue_t
   bool isReserveIDT() const;// OpenCL reserve_id_t
 
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+  bool is##Id##Type() const;
+#include "clang/Basic/OpenCLExtensionTypes.def"
+  // Type defined in cl_intel_device_side_avc_motion_estimation OpenCL 
extension
+  bool isOCLIntelSubgroupAVCType() const;
+  bool isOCLExtOpaqueType() const;  // Any OpenCL extension type
+
   bool 

[PATCH] D52296: [Clang] - Add -fdwarf-fission=split,single option.

[George Rimar via Phabricator via cfe-commits]

grimar updated this revision to Diff 172953.
grimar retitled this revision from "[Clang] - Add -gsingle-file-split-dwarf 
option." to "[Clang] - Add -fdwarf-fission=split,single option.".
grimar added a comment.

Reimplemented option as `-fdwarf-fission[=split,single]`.


https://reviews.llvm.org/D52296

Files:
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/BackendUtil.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/CommonArgs.cpp
  lib/Driver/ToolChains/CommonArgs.h
  lib/Driver/ToolChains/Gnu.cpp
  lib/Driver/ToolChains/MinGW.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/split-debug-single-file.c
  test/Driver/split-debug.c
  test/Driver/split-debug.s

Index: test/Driver/split-debug.s
===
--- test/Driver/split-debug.s
+++ test/Driver/split-debug.s
@@ -5,6 +5,13 @@
 //
 // CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo"
 
+// Check we pass -split-dwarf-file to `as` if -fdwarf-fission=split.
+// RUN: %clang -target x86_64-unknown-linux-gnu -fdwarf-fission=split -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
+
+// Check we do not pass any -split-dwarf* commands to `as` if -fdwarf-fission=single.
+// RUN: %clang -target x86_64-unknown-linux-gnu -fdwarf-fission=single -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
 
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
Index: test/Driver/split-debug.c
===
--- test/Driver/split-debug.c
+++ test/Driver/split-debug.c
@@ -5,6 +5,21 @@
 //
 // CHECK-ACTIONS: "-split-dwarf-file" "split-debug.dwo"
 
+// RUN: %clang -target x86_64-unknown-linux-gnu -fdwarf-fission -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
+// RUN: %clang -target x86_64-unknown-linux-gnu -fdwarf-fission=split -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-ACTIONS < %t %s
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -fdwarf-fission=single -c -### %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-ACTIONS-SINGLE-SPLIT < %t %s
+//
+// CHECK-ACTIONS-SINGLE-SPLIT: "-enable-split-dwarf=single"
+// CHECK-ACTIONS-SINGLE-SPLIT: "-split-dwarf-file" "split-debug.o"
+
+// RUN: %clang -target x86_64-unknown-linux-gnu -fdwarf-fission=single -c -### -o %tfoo.o %s 2> %t
+// RUN: FileCheck -check-prefix=CHECK-SINGLE-SPLIT-FILENAME < %t %s
+//
+// CHECK-SINGLE-SPLIT-FILENAME: "-split-dwarf-file" "{{.*}}foo.o"
 
 // RUN: %clang -target x86_64-macosx -gsplit-dwarf -c -### %s 2> %t
 // RUN: FileCheck -check-prefix=CHECK-NO-ACTIONS < %t %s
Index: test/CodeGen/split-debug-single-file.c
===
--- test/CodeGen/split-debug-single-file.c
+++ test/CodeGen/split-debug-single-file.c
@@ -0,0 +1,17 @@
+// REQUIRES: x86-registered-target
+
+// Testing to ensure -enable-split-dwarf=single allows to place .dwo sections into regular output object.
+//  RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux \
+//  RUN:   -enable-split-dwarf=single -split-dwarf-file %t.o -emit-obj -o %t.o %s
+//  RUN: llvm-objdump -section-headers %t.o | FileCheck --check-prefix=MODE-SINGLE %s
+//  MODE-SINGLE: .dwo
+
+// Testing to ensure -enable-split-dwarf=single allows to place .dwo sections into regular output object.
+//  RUN: %clang_cc1 -debug-info-kind=limited -triple x86_64-unknown-linux \
+//  RUN:   -enable-split-dwarf=split -split-dwarf-file %t.o -emit-obj -o %t.o %s
+//  RUN: llvm-objdump -section-headers %t.o | FileCheck --check-prefix=MODE-SPLIT %s
+//  MODE-SPLIT-NOT: .dwo
+
+int main (void) {
+  return 0;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -596,9 +596,25 @@
   Opts.MacroDebugInfo = Args.hasArg(OPT_debug_info_macro);
   Opts.WholeProgramVTables = Args.hasArg(OPT_fwhole_program_vtables);
   Opts.LTOVisibilityPublicStd = Args.hasArg(OPT_flto_visibility_public_std);
-  Opts.EnableSplitDwarf = Args.hasArg(OPT_enable_split_dwarf);
   Opts.SplitDwarfFile = Args.getLastArgValue(OPT_split_dwarf_file);
   Opts.SplitDwarfInlining = !Args.hasArg(OPT_fno_split_dwarf_inlining);
+ 
+  if (Arg *A =
+  Args.getLastArg(OPT_enable_split_dwarf, OPT_enable_split_dwarf_EQ)) {
+if (A->getOption().matches(options::OPT_enable_split_dwarf)) {
+  Opts.setSplitDwarfMode(CodeGenOptions::SplitFileFission);
+} else {
+  StringRef Name = A->getValue();
+  if (Name == "single")
+Opts.setSplitDwarfMode(CodeGenOptions::SingleFileFission);
+  else if (Name == "split")
+

[PATCH] D54202: [clangd] Drop namespace references in the index.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346319: [clangd] Drop namespace references in the index. 
(authored by hokein, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D54202

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


Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h
@@ -60,6 +60,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, and they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -367,7 +367,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.


Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h
@@ -60,6 +60,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, and they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
===
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
@@ -367,7 +367,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders 

[PATCH] D52554: [WIP] [clangd] Tests for special methods code-completion

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 172951.
jkorous marked 4 inline comments as done.
jkorous added a comment.

Rewritten tests to shared implementation different cases.


https://reviews.llvm.org/D52554

Files:
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -2204,6 +2204,115 @@
  {cls("naber"), cls("nx::naber")}, Opts);
   EXPECT_THAT(Results.Completions, UnorderedElementsAre());
 }
+
+TEST(CompletionTest, NoCompletionForSpecialMembers) {
+  clangd::CodeCompleteOptions Opts;
+  for( auto& testcase : { 
+R"cpp(
+  struct foo {};
+  void bar() {
+foo a;
+a.^
+)cpp",
+R"cpp(
+  template struct foo {};
+  void bar() {
+foo a;
+a.^
+)cpp",
+R"cpp(
+template struct foo {}; template<> struct foo {};
+void bar() {
+  foo a;
+  a.^
+)cpp",
+R"cpp(
+  struct foo {};
+  void bar() {
+foo* b;
+b->^
+)cpp",
+R"cpp(
+  template struct foo {};
+  void bar() {
+foo* b;
+b->^
+)cpp",
+R"cpp(
+  template struct foo {}; template<> struct foo {};
+  void bar() {
+foo* b;
+b->^
+)cpp",
+R"cpp(
+  struct foo {};
+  foo::^
+)cpp",
+R"cpp(
+  template struct foo {};
+  foo::^
+)cpp",
+R"cpp(
+  template struct foo {}; template<> struct foo {};
+  foo::^
+)cpp",
+R"cpp(
+  struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  void bar() {
+foo a;
+a.^
+)cpp",
+R"cpp(
+  struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  void bar() {
+foo* b;
+b->^
+)cpp",
+R"cpp(
+  struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  foo::^
+)cpp",
+R"cpp(
+  template struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  void bar() {
+foo a;
+a.^
+)cpp",
+R"cpp(
+  template struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  void bar() {
+foo* b;
+b->^
+)cpp",
+R"cpp(
+  template struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  foo::^
+)cpp",
+R"cpp(
+  template struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  template<> struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  void bar() {
+foo a;
+a.^
+)cpp",
+R"cpp(
+  template struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  template<> struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  void bar() {
+foo* b;
+b->^
+)cpp",
+R"cpp(
+  template struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  template<> struct foo { foo(); ~foo(); foo& operator=(const foo&); foo& operator=(foo&&); };
+  foo::^
+)cpp"
+  }) {
+auto Results = completions(testcase, /*IndexSymbols=*/{}, Opts);
+EXPECT_THAT(Results.Completions, IsEmpty()) << testcase;
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52554: [WIP] [clangd] Tests for special methods code-completion

[Jan Korous via Phabricator via cfe-commits]

jkorous marked 6 inline comments as done.
jkorous added inline comments.



Comment at: clangd/CodeCompleteTests.cpp:2043
 
+TEST(CompletionTestNoExplicitMembers, Struct) {
+  clangd::CodeCompleteOptions Opts;

sammccall wrote:
> sammccall wrote:
> > (Should this be ImplicitMembers?)
> nit: these cases should probably be moved up with the other code completion 
> ones, and called something like `TEST(CompletionTest, NoExplicitMembers)` or 
> so for consistency.
> 
> It's OK to have related tests in one test case.
> 
> In fact, this large set of closely-related cases seems like a good place for 
> Go-style table-driven tests.
Ultimately got rid of the name completely.



Comment at: clangd/CodeCompleteTests.cpp:2043
 
+TEST(CompletionTestNoExplicitMembers, Struct) {
+  clangd::CodeCompleteOptions Opts;

jkorous wrote:
> sammccall wrote:
> > sammccall wrote:
> > > (Should this be ImplicitMembers?)
> > nit: these cases should probably be moved up with the other code completion 
> > ones, and called something like `TEST(CompletionTest, NoExplicitMembers)` 
> > or so for consistency.
> > 
> > It's OK to have related tests in one test case.
> > 
> > In fact, this large set of closely-related cases seems like a good place 
> > for Go-style table-driven tests.
> Ultimately got rid of the name completely.
I reconsidered the simple yet verbatim approach.



Comment at: clangd/CodeCompleteTests.cpp:2054
+
+  EXPECT_TRUE(ResultsDot.Completions.empty());
+

sammccall wrote:
> EXPECT_THAT(ResultsDot.Completions, IsEmpty())
> 
> (when the assertion fails, the failure message will print the contents of the 
> container)
I didn't know this. It's pretty neat. Thanks!



Comment at: clangd/CodeCompleteTests.cpp:2139
+
+TEST(CompletionTestMethodDeclared, Struct) {
+  clangd::CodeCompleteOptions Opts;

sammccall wrote:
> doesn't this test a strict superset of what CompletionTestNoTestMembers tests?
> i.e. this also asserts that the implicit members are not included.
> 
> ISTM we could combine many of these tests (and that in fact many of them are 
> covered by TestAfterDotCompletion.
You are right, I pruned the list of testcases a bit.



Comment at: clangd/CodeCompleteTests.cpp:2338
+
+TEST(CompletionTestSpecialMethodsDeclared, 
ExplicitStructTemplateSpecialization) {
+  clangd::CodeCompleteOptions Opts;

sammccall wrote:
> (I think we could get away with a representative set of cases, rather than 
> testing the intersection of every feature. e.g. test an explicitly declared 
> operator= on a struct, but combining that with an explicitly specialized 
> struct template seems unneccesary)
I simplified the whole test and removed some cases that were not really unique. 
Do you think the rest is still too exhaustive? (Asking before deleting.)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52554



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

[clang-tools-extra] r346319 - [clangd] Drop namespace references in the index.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Wed Nov  7 06:59:24 2018
New Revision: 346319

URL: http://llvm.org/viewvc/llvm-project?rev=346319=rev
Log:
[clangd] Drop namespace references in the index.

Summary:
Namespace references is less useful compared with other symbols, and
they contribute large part of the index. This patch drops them.
The number of refs is reduced from 5.4 million to 4.7 million.

|   |  Before | After |
|file size  |  78 MB  |  71MB |
|memory |  330MB  |  300MB|

Reviewers: sammccall

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

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

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

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=346319=346318=346319=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Nov  7 
06:59:24 2018
@@ -367,7 +367,7 @@ bool SymbolCollector::handleDeclOccurenc
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.

Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=346319=346318=346319=diff
==
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Wed Nov  7 06:59:24 
2018
@@ -60,6 +60,9 @@ public:
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, and they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be

Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=346319=346318=346319=diff
==
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed Nov  
7 06:59:24 2018
@@ -443,6 +443,8 @@ TEST_F(SymbolCollectorTest, Refs) {
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@ TEST_F(SymbolCollectorTest, Refs) {
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();


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

[PATCH] D54202: [clangd] Drop namespace references in the index.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 172950.
hokein marked an inline comment as done.
hokein added a comment.

Fix another typo.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54202

Files:
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -56,6 +56,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, and they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -356,7 +356,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -56,6 +56,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, and they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -356,7 +356,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54202: [clangd] Drop namespace references in the index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:359
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))

kadircet wrote:
> Why not handle in `shouldCollectSymbol` ?
`shouldCollectSymbol` is shared between the code collecting symbols and the 
code collecting references, we still want to collect namespace symbols.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54202



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

[PATCH] D53427: [clangd] Replace StringRef in SymbolLocation with a char pointer.

[Haojian Wu via Phabricator via cfe-commits]

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

Move forward the patch based on the offline discussion.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53427

Files:
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/Serialization.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/YAMLSerialization.cpp
  clangd/index/dex/Dex.cpp
  unittests/clangd/BackgroundIndexTests.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/DexTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SerializationTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,8 +53,10 @@
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
-MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
+MATCHER_P(DeclURI, P, "") {
+  return StringRef(arg.CanonicalDeclaration.FileURI) == P;
+}
+MATCHER_P(DefURI, P, "") { return StringRef(arg.Definition.FileURI) == P; }
 MATCHER_P(IncludeHeader, P, "") {
   return (arg.IncludeHeaders.size() == 1) &&
  (arg.IncludeHeaders.begin()->IncludeHeader == P);
Index: unittests/clangd/SerializationTests.cpp
===
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -111,7 +111,7 @@
   EXPECT_EQ(Sym1.Signature, "");
   EXPECT_EQ(Sym1.Documentation, "Foo doc");
   EXPECT_EQ(Sym1.ReturnType, "int");
-  EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
+  EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), "file:///path/foo.h");
   EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
   EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
@@ -122,7 +122,8 @@
   EXPECT_THAT(Sym2, QName("clang::Foo2"));
   EXPECT_EQ(Sym2.Signature, "-sig");
   EXPECT_EQ(Sym2.ReturnType, "");
-  EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
+  EXPECT_EQ(llvm::StringRef(Sym2.CanonicalDeclaration.FileURI),
+"file:///path/bar.h");
   EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
   EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
 
@@ -134,7 +135,7 @@
testing::SizeIs(1;
   auto Ref1 = ParsedYAML->Refs->begin()->second.front();
   EXPECT_EQ(Ref1.Kind, RefKind::Reference);
-  EXPECT_EQ(Ref1.Location.FileURI, "file:///path/foo.cc");
+  EXPECT_EQ(StringRef(Ref1.Location.FileURI), "file:///path/foo.cc");
 }
 
 std::vector YAMLFromSymbols(const SymbolSlab ) {
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -36,7 +36,7 @@
  std::make_tuple(Range.start.line, Range.start.character,
  Range.end.line, Range.end.character);
 }
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
 
 TEST(SymbolLocation, Position) {
   using Position = SymbolLocation::Position;
@@ -222,7 +222,7 @@
 
   Symbol M = mergeSymbol(L, R);
   EXPECT_EQ(M.Name, "Foo");
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h");
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:///left.h");
   EXPECT_EQ(M.References, 3u);
   EXPECT_EQ(M.Signature, "()");
   EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}");
@@ -241,14 +241,14 @@
   R.Name = "right";
 
   Symbol M = mergeSymbol(L, R);
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h");
-  EXPECT_EQ(M.Definition.FileURI, "");
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/left.h");
+  EXPECT_EQ(StringRef(M.Definition.FileURI), "");
   EXPECT_EQ(M.Name, "left");
 
   R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored.
   M = mergeSymbol(L, R);
-  EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h");
-  EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp");
+  EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/right.h");
+  EXPECT_EQ(StringRef(M.Definition.FileURI), "file:/right.cpp");
   EXPECT_EQ(M.Name, "right");
 }
 
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -31,19 +31,21 @@
 using testing::IsEmpty;
 using testing::Pair;
 using testing::UnorderedElementsAre;
+using namespace llvm;
 
 MATCHER_P(RefRange, Range, "") {
   return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov updated this revision to Diff 172946.
AlexeySachkov added a comment.

Rebased to ToT.


https://reviews.llvm.org/D51484

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Basic/OpenCLExtensionTypes.def
  include/clang/Basic/OpenCLExtensions.def
  include/clang/Sema/Initialization.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/module.modulemap
  lib/AST/ASTContext.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/PrintfFormatString.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Headers/opencl-c.h
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  test/Headers/opencl-c-header.cl
  test/Index/opencl-types.cl
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
  tools/libclang/CIndex.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -70,6 +70,8 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) BTCASE(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) BTCASE(Id);
+#include "clang/Basic/OpenCLExtensionTypes.def"
 BTCASE(OCLSampler);
 BTCASE(OCLEvent);
 BTCASE(OCLQueue);
@@ -605,6 +607,8 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
+#define EXT_OPAQUE_TYPE(ExtTYpe, Id, Ext) TKIND(Id);
+#include "clang/Basic/OpenCLExtensionTypes.def"
 TKIND(OCLSampler);
 TKIND(OCLEvent);
 TKIND(OCLQueue);
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1519,6 +1519,9 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   case BuiltinType::Id:
 #include "clang/Basic/OpenCLImageTypes.def"
+#define EXT_OPAQUE_TYPE(ExtTYpe, Id, Ext) \
+  case BuiltinType::Id:
+#include "clang/Basic/OpenCLExtensionTypes.def"
   case BuiltinType::OCLSampler:
   case BuiltinType::OCLEvent:
   case BuiltinType::OCLClkEvent:
Index: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
===
--- /dev/null
+++ test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
@@ -0,0 +1,105 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify %s
+
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+
+// All intel_sub_group_avc_* types can only be used as argument or return value
+// of built-in functions defined in the extension.
+// But there are also additional initialization rules:
+//   * All types except intel_sub_group_avc_mce_* types can be initialized with
+// the corresponding initializer macro defined in opencl-c.h
+// Currently all these macroses are defined as 0x0
+//   * In previous versions of the extension these macroses was defined as {0},
+// so initialization with initializer list containing one integer equal to
+// zero should also work
+
+struct st{};
+// negative test cases for initializers
+void foo(char c, float f, void* v, struct st ss) {
+  intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer for mce types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ime_payload_t payload_ime = 1; // No literal initializer for *payload_t types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_ime_payload_t' with an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ref_payload_t payload_ref = f;
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_ref_payload_t' with an expression of incompatible type 'float'}}
+  intel_sub_group_avc_sic_payload_t payload_sic = ss;
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_sic_payload_t' with an expression of incompatible type 'struct st'}}
+
+  intel_sub_group_avc_mce_result_t result_mce = 0; // No zero initializer for mce types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_result_t' with an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ime_result_t result_ime = 1; // No literal initializer for *result_t types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_ime_result_t' with an expression of 

[PATCH] D53974: [clang-tidy] new check: bugprone-too-small-loop-variable

[Tamás Zolnai via Phabricator via cfe-commits]

ztamas updated this revision to Diff 172938.
ztamas added a comment.

- no `else` after `return`
- `static constexpr llvm::StringLiteral`
- CamelCase variable names
- Remove unneeded isIntegerType() check
- Better terminology: not terminating condition, but iteration's upper bound.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D53974

Files:
  clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tidy/bugprone/CMakeLists.txt
  clang-tidy/bugprone/TooSmallLoopVariableCheck.cpp
  clang-tidy/bugprone/TooSmallLoopVariableCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/bugprone-too-small-loop-variable.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/bugprone-too-small-loop-variable.cpp

Index: test/clang-tidy/bugprone-too-small-loop-variable.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-too-small-loop-variable.cpp
@@ -0,0 +1,227 @@
+// RUN: %check_clang_tidy %s bugprone-too-small-loop-variable %t
+
+long size() { return 294967296l; }
+
+
+/// Test cases correctly caught by bugprone-too-small-loop-variable.
+
+void voidBadForLoop() {
+  for (int i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop2() {
+  for (int i = 0; i < size() + 10; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop3() {
+  for (int i = 0; i <= size() - 1; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop4() {
+  for (int i = 0; size() > i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:28: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop5() {
+  for (int i = 0; size() - 1 >= i; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:33: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidBadForLoop6() {
+  int i = 0;
+  for (; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: loop variable has narrower type 'int' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidForLoopUnsignedBound() {
+  unsigned size = 3147483647;
+  for (int i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: loop variable has narrower type 'int' than iteration's upper bound 'unsigned int' [bugprone-too-small-loop-variable]
+  }
+}
+
+// The iteration's upper bound has a template dependent value.
+template 
+void doSomething() {
+  for (short i = 0; i < size; ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+// The iteration's upper bound has a template dependent type.
+template 
+void doSomething() {
+  for (T i = 0; i < size(); ++i) {
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: loop variable has narrower type 'short' than iteration's upper bound 'long' [bugprone-too-small-loop-variable]
+  }
+}
+
+void voidForLoopInstantiation() {
+  // This line does not trigger the warning.
+  doSomething();
+  // This one triggers the warning.
+  doSomething();
+}
+
+
+/// Correct loops: we should not warn here.
+
+// A simple use case when both expressions have the same type.
+void voidGoodForLoop() {
+  for (long i = 0; i < size(); ++i) { // no warning
+  }
+}
+
+// Other use case where both expressions have the same type,
+// but short expressions are converted to int by the compare operator.
+void voidGoodForLoop2() {
+  short loopCond = 10;
+  for (short i = 0; i < loopCond; ++i) { // no warning
+  }
+}
+
+// Because of the integer literal, the iteration's upper bound is int, but we suppress the warning here.
+void voidForLoopShortPlusLiteral() {
+  short size = 3;
+  for (short i = 0; i <= (size - 1); ++i) { // no warning
+  }
+}
+
+// Addition of two short variables results in an int value, but we suppress this to avoid false positives.
+void voidForLoopShortPlusShort() {
+  short size = 256;
+  short increment = 14;
+  for (short i = 0; i < size + increment; ++i) { // no warning
+  }
+}
+
+// In this test case we have different integer types, but here the loop variable has the bigger type.
+// The iteration's bound is cast implicitly, not the loop variable.
+void voidForLoopBoundImplicitCast() {
+  short start = 256;
+  short end = 14;
+  for (int i = start; i >= end; --i) { // no 

[PATCH] D54204: [clangd] Initial clang-tidy diagnostics support.

[Sam McCall via Phabricator via cfe-commits]

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

This runs checks over a restricted subset of the TU:

- preprocessor callbacks just receive the truncated PP events that occur when a 
preamble is used.
- ASTMatchers run only over the top-level decls in the main-file

This patch just turns on one simple check (bugprone-sizeof-expression)
with no configuration.

- configuration is complex enough to warrant a separate patch
- arbitrary checks don't work well yet - there are various ways that checks can 
access the whole AST (and thus be incredibly slow). Most notably: the 
hasAncestor matcher, and using the ASTContext from check().

This depends on a small patch to ASTMatchers to run a MatchFinder on a
certain set of top-level declarations.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54204

Files:
  clangd/CMakeLists.txt
  clangd/ClangdUnit.cpp
  unittests/clangd/ClangdUnitTests.cpp

Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -128,6 +128,15 @@
   WithFix(Fix(Test.range(), "int", "change return type to 'int'");
 }
 
+TEST(DiagnosticsTest, ClangTidy) {
+  Annotations Test("int main() { return [[sizeof]](sizeof(int)); }");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(),
+   "suspicious usage of 'sizeof(sizeof(...))' "
+   "[bugprone-sizeof-expression]")));
+}
+
 TEST(DiagnosticsTest, Preprocessor) {
   // This looks like a preamble, but there's an #else in the middle!
   // Check that:
Index: clangd/ClangdUnit.cpp
===
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -8,6 +8,8 @@
 //===--===//
 
 #include "ClangdUnit.h"
+#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
+#include "../clang-tidy/ClangTidyModuleRegistry.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Logger.h"
@@ -148,6 +150,34 @@
 return None;
   }
 
+  // Set up ClangTidy. Must happen after BeginSourceFile() so ASTContext exists.
+  std::vector> CTChecks;
+  ast_matchers::MatchFinder CTFinder;
+  llvm::Optional CTContext;
+  {
+trace::Span Tracer("ClangTidyInit");
+tidy::ClangTidyCheckFactories CTFactories;
+for (const auto  : tidy::ClangTidyModuleRegistry::entries())
+  E.instantiate()->addCheckFactories(CTFactories);
+auto CTOpts = tidy::ClangTidyOptions::getDefaults();
+// FIXME: this needs to be configurable, and we need to support .clang-tidy
+// files and other options providers.
+// There is an important bug to fix before turning on arbitrary checks:
+// we must restrict the AST parent map to the current file for performance.
+// The placeholder check here does not use hasAncestor() so is unaffected.
+CTOpts.Checks = "bugprone-sizeof-expression";
+CTContext.emplace(llvm::make_unique(
+tidy::ClangTidyGlobalOptions(), CTOpts));
+CTContext->setDiagnosticsEngine(>getDiagnostics());
+CTContext->setASTContext(>getASTContext());
+CTContext->setCurrentFile(MainInput.getFile());
+CTFactories.createChecks(CTContext.getPointer(), CTChecks);
+for (const auto  : CTChecks) {
+  Check->registerPPCallbacks(*Clang);
+  Check->registerMatchers();
+}
+  }
+
   // Copy over the includes from the preamble, then combine with the
   // non-preamble includes below.
   auto Includes = Preamble ? Preamble->Includes : IncludeStructure{};
@@ -157,13 +187,24 @@
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
+  std::vector ParsedDecls = Action->takeTopLevelDecls();
+  {
+// Run the AST-dependent part of the clang-tidy checks.
+// (The preprocessor part ran already, via PPCallbacks).
+trace::Span Tracer("ClangTidyMatch");
+CTFinder.matchAST(Clang->getASTContext(), ParsedDecls);
+  }
+
   // UnitDiagsConsumer is local, we can not store it in CompilerInstance that
   // has a longer lifetime.
   Clang->getDiagnostics().setClient(new IgnoreDiagnostics);
   // CompilerInstance won't run this callback, do it directly.
   ASTDiags.EndSourceFile();
+  // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.
+  // However Action->EndSourceFile() would destroy the ASTContext!
+  // So just inform the preprocessor of EOF, while keeping everything alive.
+  Clang->getPreprocessor().EndSourceFile();
 
-  std::vector ParsedDecls = Action->takeTopLevelDecls();
   std::vector Diags = ASTDiags.take();
   // Add diagnostics from the preamble, if any.
   if (Preamble)
@@ -179,7 +220,12 @@
 
 

[PATCH] D52984: [analyzer] Checker reviewer's checklist

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

Szelethus added a comment.

In https://reviews.llvm.org/D52984#1269850, @NoQ wrote:

> - Warning and note messages should be clear and easy to understand, even if a 
> bit long.
>   - Messages should start with a capital letter (unlike Clang warnings!) and 
> should not end with `.`.
>   - Articles are usually omitted, eg. `Dereference of a null pointer` -> 
> `Dereference of null pointer`.


I don't see this point in the list.

Coding standard related items should probably in a different list, alongside 
the out-of-alpha items.




Comment at: www/analyzer/checker_dev_manual.html:719
+
+User facing documentation is important for adoption! Make sure the check 
list updated
+at the homepage of the analyzer. Also ensure that the description is good 
quality in

Make sure the **checker** list **is** updated



Comment at: www/analyzer/checker_dev_manual.html:720
+User facing documentation is important for adoption! Make sure the check 
list updated
+at the homepage of the analyzer. Also ensure that the description is good 
quality in
+Checkers.td.

ensure the description is clear even to non-developers


https://reviews.llvm.org/D52984



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

[PATCH] D50211: [analyzer] Fix displayed checker name for InnerPointerChecker

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

Szelethus added a comment.

I personally find the registration process very error-prone, the current 
infrastructure around it makes it super easy to mess up in a way that won't be 
detected for months, so we probably need a better approach altogether. It 
shouldn't be possible to do any harm within a registry function, especially not 
in a way that affects other checkers.


https://reviews.llvm.org/D50211



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

[PATCH] D50211: [analyzer] Fix displayed checker name for InnerPointerChecker

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

Szelethus added a comment.
Herald added a subscriber: donat.nagy.

Bad news, this approach doesn't work too well, and here's why: 
https://github.com/llvm-mirror/clang/blob/master/lib/StaticAnalyzer/Core/CheckerRegistry.cpp#L115

  void CheckerRegistry::initializeManager(CheckerManager ,
SmallVectorImpl ) 
const {
// Sort checkers for efficient collection.
llvm::sort(Checkers, checkerNameLT);
  
// Collect checkers enabled by the options.
CheckerInfoSet enabledCheckers;
for (auto  : opts)
  collectCheckers(Checkers, Packages, i, enabledCheckers);
  
// Initialize the CheckerManager with all enabled checkers.
for (const auto *i :enabledCheckers) {
  checkerMgr.setCurrentCheckName(CheckName(i->FullName));
  i->Initialize(checkerMgr);
}
  }

Note that `Initialize` is a function pointer that points to 
`register##CHECKERNAME`, so a single registry function should only register one 
checker, because `setCurrentCheckName` is only called once, resulting in 
`MallocChecker`'s checker object receiving the `cpluscplus.InnetPointer` name. 
It's very not-obvious how to fix this gracefully :/ You could call 
`CheckerManager::setCurrentCheckName` within the registry function, but you 
have to supply a full name, which adds unnecessary maintenance cost (for 
example, if someone moves a checker out of alpha, or puts one back).

I found this bug while trying to fix checker options, and noticed that the 
`Optimistic` flag of `MallocChecker` is acquired as 
`cplusplus.InnerPointer:Optimistic`, instead of `unix.Malloc:Optimistic`. Let's 
keep this in for now, and I'll try to look for ways to express dependencies 
while avoiding this issue.


https://reviews.llvm.org/D50211



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

[clang-tools-extra] r346312 - [clangd] Fix mainfile filtering for diagnostics: don't emit diagnostics for builtin headers. (Relevant for clang-tidy)

[Sam McCall via cfe-commits]

Author: sammccall
Date: Wed Nov  7 05:21:09 2018
New Revision: 346312

URL: http://llvm.org/viewvc/llvm-project?rev=346312=rev
Log:
[clangd] Fix mainfile filtering for diagnostics: don't emit diagnostics for 
builtin headers. (Relevant for clang-tidy)

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

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=346312=346311=346312=diff
==
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Wed Nov  7 05:21:09 2018
@@ -79,7 +79,7 @@ Range diagnosticRange(const clang::Diagn
 }
 
 bool isInsideMainFile(const SourceLocation Loc, const SourceManager ) {
-  return Loc.isValid() && M.isInMainFile(Loc);
+  return Loc.isValid() && M.isWrittenInMainFile(Loc);
 }
 
 bool isInsideMainFile(const clang::Diagnostic ) {


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

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

@hokein you and I seem to be making the same proposal :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54141



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

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

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

Szelethus added inline comments.



Comment at: test/Analysis/conversion.c:195
+return r;
+  } else if (b>1<<25) {
+float f = b; // expected-warning {{Loss of precision}}

NoQ wrote:
> Szelethus wrote:
> > This too -- how about running clang-format on this file?
> Tests are often unformatted and we usually don't care. Additionally, they are 
> almost always violating LLVM's camelCase vs. under_score conventions.
Hmm, okay :)


Repository:
  rC Clang

https://reviews.llvm.org/D52730



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

[PATCH] D54152: [OpenCL] Fix diagnostic message about overload candidates

[Andrew Savonichev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346311: [OpenCL] Fix diagnostic message about overload 
candidates (authored by asavonic, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D54152

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  test/SemaOpenCL/extension-begin.cl


Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3681,7 +3681,7 @@
 def note_ovl_candidate_disabled_by_function_cond_attr : Note<
 "candidate disabled: %0">;
 def note_ovl_candidate_disabled_by_extension : Note<
-"candidate unavailable as it requires OpenCL extension '%0' to be 
disabled">;
+"candidate unavailable as it requires OpenCL extension '%0' to be 
enabled">;
 def err_addrof_function_disabled_by_enable_if_attr : Error<
 "cannot take address of function %0 because it has one or more "
 "non-tautological enable_if conditions">;
Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -40,7 +40,7 @@
   PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 
'const struct A *') requires my_ext extension to be enabled}}
   f(); // expected-error {{use of declaration 'f' requires my_ext extension to 
be enabled}}
   g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@extension-begin.h:18 {{candidate unavailable as it 
requires OpenCL extension 'my_ext' to be disabled}}
+// expected-note@extension-begin.h:18 {{candidate unavailable as it 
requires OpenCL extension 'my_ext' to be enabled}}
 // expected-note@extension-begin.h:23 {{candidate function not viable: 
requires 0 arguments, but 1 was provided}}
 }
 


Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3681,7 +3681,7 @@
 def note_ovl_candidate_disabled_by_function_cond_attr : Note<
 "candidate disabled: %0">;
 def note_ovl_candidate_disabled_by_extension : Note<
-"candidate unavailable as it requires OpenCL extension '%0' to be disabled">;
+"candidate unavailable as it requires OpenCL extension '%0' to be enabled">;
 def err_addrof_function_disabled_by_enable_if_attr : Error<
 "cannot take address of function %0 because it has one or more "
 "non-tautological enable_if conditions">;
Index: test/SemaOpenCL/extension-begin.cl
===
--- test/SemaOpenCL/extension-begin.cl
+++ test/SemaOpenCL/extension-begin.cl
@@ -40,7 +40,7 @@
   PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 'const struct A *') requires my_ext extension to be enabled}}
   f(); // expected-error {{use of declaration 'f' requires my_ext extension to be enabled}}
   g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@extension-begin.h:18 {{candidate unavailable as it requires OpenCL extension 'my_ext' to be disabled}}
+// expected-note@extension-begin.h:18 {{candidate unavailable as it requires OpenCL extension 'my_ext' to be enabled}}
 // expected-note@extension-begin.h:23 {{candidate function not viable: requires 0 arguments, but 1 was provided}}
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r346311 - [OpenCL] Fix diagnostic message about overload candidates

[Andrew Savonichev via cfe-commits]

Author: asavonic
Date: Wed Nov  7 05:07:18 2018
New Revision: 346311

URL: http://llvm.org/viewvc/llvm-project?rev=346311=rev
Log:
[OpenCL] Fix diagnostic message about overload candidates

Summary:
I wonder if there are some extension which need to be disabled to get
overloadable candidate available.


Reviewers: asavonic, Anastasia

Reviewed By: Anastasia

Subscribers: yaxunl, sidorovd, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/test/SemaOpenCL/extension-begin.cl

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=346311=346310=346311=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Nov  7 05:07:18 
2018
@@ -3681,7 +3681,7 @@ def warn_diagnose_if_succeeded : Warning
 def note_ovl_candidate_disabled_by_function_cond_attr : Note<
 "candidate disabled: %0">;
 def note_ovl_candidate_disabled_by_extension : Note<
-"candidate unavailable as it requires OpenCL extension '%0' to be 
disabled">;
+"candidate unavailable as it requires OpenCL extension '%0' to be 
enabled">;
 def err_addrof_function_disabled_by_enable_if_attr : Error<
 "cannot take address of function %0 because it has one or more "
 "non-tautological enable_if conditions">;

Modified: cfe/trunk/test/SemaOpenCL/extension-begin.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/extension-begin.cl?rev=346311=346310=346311=diff
==
--- cfe/trunk/test/SemaOpenCL/extension-begin.cl (original)
+++ cfe/trunk/test/SemaOpenCL/extension-begin.cl Wed Nov  7 05:07:18 2018
@@ -40,7 +40,7 @@ void test_f2(void) {
   PointerOfA test_A_pointer; // expected-error {{use of type 'PointerOfA' (aka 
'const struct A *') requires my_ext extension to be enabled}}
   f(); // expected-error {{use of declaration 'f' requires my_ext extension to 
be enabled}}
   g(0); // expected-error {{no matching function for call to 'g'}}
-// expected-note@extension-begin.h:18 {{candidate unavailable as it 
requires OpenCL extension 'my_ext' to be disabled}}
+// expected-note@extension-begin.h:18 {{candidate unavailable as it 
requires OpenCL extension 'my_ext' to be enabled}}
 // expected-note@extension-begin.h:23 {{candidate function not viable: 
requires 0 arguments, but 1 was provided}}
 }
 


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

[PATCH] D54202: [clangd] Drop namespace references in the index.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/index/SymbolCollector.cpp:359
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))

Why not handle in `shouldCollectSymbol` ?



Comment at: clangd/index/SymbolCollector.h:60
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, anbd they are less useful compared
+/// with other decls.

another typo anbd -> and


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54202



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

[PATCH] D54202: [clangd] Drop namespace references in the index.

[Haojian Wu via Phabricator via cfe-commits]

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

Fix a typo.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54202

Files:
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -56,6 +56,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, anbd they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -356,7 +356,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -56,6 +56,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, anbd they are less useful compared
+/// with other decls.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -356,7 +356,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54202: [clangd] Drop namespace references in the index.

[Haojian Wu via Phabricator via cfe-commits]

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

Namespace references is less useful compared with other symbols, and
they contribute large part of the index. This patch drops them.
The number of refs is reduced from 5.4 million to 4.7 million.

|   | Before | After |
| file size | 78 MB  | 71MB  |
| memory| 330MB  | 300MB |


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54202

Files:
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/SymbolCollectorTests.cpp


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -56,6 +56,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, anbd they are less useful compared
+/// with other delcarations.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -356,7 +356,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.


Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -443,6 +443,8 @@
   };
   class $bar[[Bar]];
   void $func[[func]]();
+
+  namespace $ns[[NS]] {} // namespace ref is ignored
   )");
   Annotations Main(R"(
   class $bar[[Bar]] {};
@@ -474,6 +476,7 @@
   HaveRanges(Main.ranges("bar");
   EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID,
   HaveRanges(Main.ranges("func");
+  EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _;
   // Symbols *only* in the main file (a, b, c) had no refs collected.
   auto MainSymbols =
   TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -56,6 +56,9 @@
 bool CountReferences = false;
 /// The symbol ref kinds that will be collected.
 /// If not set, SymbolCollector will not collect refs.
+/// Note that references of namespace decls are not collected, as they
+/// contribute large part of the index, anbd they are less useful compared
+/// with other delcarations.
 RefKind RefFilter = RefKind::Unknown;
 /// If set to true, SymbolCollector will collect all refs (from main file
 /// and included headers); otherwise, only refs from main file will be
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -356,7 +356,7 @@
 return true;
   if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
 return true;
-  if (CollectRef &&
+  if (CollectRef && !isa(ND) &&
   (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
 DeclRefs[ND].emplace_back(SpellingLoc, Roles);
   // Don't continue indexing if this is a mere reference.
___
cfe-commits mailing list

[PATCH] D54157: [clangd] [NFC] Fix clang-tidy warnings.

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346308: [clangd] [NFC] Fix clang-tidy warnings. (authored by 
kadircet, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D54157

Files:
  clang-tools-extra/trunk/clangd/index/FileIndex.cpp
  clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp


Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -126,7 +126,7 @@
   for (const auto  : *Slab) {
 auto I = Merged.try_emplace(Sym.ID, Sym);
 if (!I.second)
-  I.first->second = mergeSymbol(std::move(I.first->second), Sym);
+  I.first->second = mergeSymbol(I.first->second, Sym);
   }
 }
 SymsStorage.reserve(Merged.size());
Index: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
@@ -43,7 +43,7 @@
   void f_b() {
 (void)common;
   })cpp";
-  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchmes=*/{"unittest"});
+  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"});
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");


Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
===
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp
@@ -126,7 +126,7 @@
   for (const auto  : *Slab) {
 auto I = Merged.try_emplace(Sym.ID, Sym);
 if (!I.second)
-  I.first->second = mergeSymbol(std::move(I.first->second), Sym);
+  I.first->second = mergeSymbol(I.first->second, Sym);
   }
 }
 SymsStorage.reserve(Merged.size());
Index: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
===
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
@@ -43,7 +43,7 @@
   void f_b() {
 (void)common;
   })cpp";
-  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchmes=*/{"unittest"});
+  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"});
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy added a comment.

Could someone with commit rights commit this patch (if it is acceptable)? I 
don't have commit rights myself.




Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:158-162
+unsigned FloatingSize = AC.getTypeSize(DestType);
+// getAllOneValues returns an APFloat with semantics corresponding to the
+// bit size given as the first argument; this is the only function in
+// APFloat.h that maps bit width to semantics.
+llvm::APFloat Tmp = llvm::APFloat::getAllOnesValue(FloatingSize, true);

NoQ wrote:
> Hmm, so the remaining problem is how to extract float semantics from a float 
> `QualType`? Would `ASTContext::getFloatTypeSemantics(DestType)` make sense?
Thank you, that is the method I was looking for!


Repository:
  rC Clang

https://reviews.llvm.org/D52730



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

[clang-tools-extra] r346308 - [clangd] [NFC] Fix clang-tidy warnings.

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Wed Nov  7 04:25:27 2018
New Revision: 346308

URL: http://llvm.org/viewvc/llvm-project?rev=346308=rev
Log:
[clangd] [NFC] Fix clang-tidy warnings.

Reviewers: ioeric, sammccall, ilya-biryukov, hokein

Subscribers: MaskRay, jkorous, arphaman, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/index/FileIndex.cpp
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp

Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=346308=346307=346308=diff
==
--- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Wed Nov  7 04:25:27 2018
@@ -126,7 +126,7 @@ FileSymbols::buildIndex(IndexType Type,
   for (const auto  : *Slab) {
 auto I = Merged.try_emplace(Sym.ID, Sym);
 if (!I.second)
-  I.first->second = mergeSymbol(std::move(I.first->second), Sym);
+  I.first->second = mergeSymbol(I.first->second, Sym);
   }
 }
 SymsStorage.reserve(Merged.size());

Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=346308=346307=346308=diff
==
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Wed Nov  
7 04:25:27 2018
@@ -43,7 +43,7 @@ TEST(BackgroundIndexTest, IndexTwoFiles)
   void f_b() {
 (void)common;
   })cpp";
-  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchmes=*/{"unittest"});
+  BackgroundIndex Idx(Context::empty(), "", FS, /*URISchemes=*/{"unittest"});
 
   tooling::CompileCommand Cmd;
   Cmd.Filename = testPath("root/A.cc");


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

[PATCH] D52730: [analyzer] ConversionChecker: handle floating point

[Donát Nagy via Phabricator via cfe-commits]

donat.nagy updated this revision to Diff 172922.
donat.nagy marked an inline comment as done.
donat.nagy added a comment.

Use ASTContext::getFloatTypeSemantics()


Repository:
  rC Clang

https://reviews.llvm.org/D52730

Files:
  lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
  test/Analysis/conversion.c

Index: test/Analysis/conversion.c
===
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -137,6 +137,12 @@
   U8 = S + 10;
 }
 
+char dontwarn6(long long x) {
+  long long y = 42;
+  y += x;
+  return y == 42;
+}
+
 
 // C library functions, handled via apiModeling.StdCLibraryFunctions
 
@@ -154,7 +160,7 @@
 # define EOF (-1)
 char reply_string[8192];
 FILE *cin;
-extern int dostuff (void);
+extern int dostuff(void);
 int libraryFunction2() {
   int c, n;
   int dig;
@@ -179,3 +185,26 @@
   }
 }
 
+double floating_point(long long a, int b) {
+  if (a > 1LL << 55) {
+double r = a; // expected-warning {{Loss of precision}}
+return r;
+  } else if (b > 1 << 25) {
+float f = b; // expected-warning {{Loss of precision}}
+return f;
+  }
+  return 137;
+}
+
+double floating_point2() {
+  int a = 1 << 24;
+  long long b = 1LL << 53;
+  float f = a; // no-warning
+  double d = b; // no-warning
+  return d - f;
+}
+
+int floating_point_3(unsigned long long a) {
+  double b = a; // no-warning
+  return 42;
+}
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -14,8 +14,10 @@
 // of expressions. A warning is reported when:
 // * a negative value is implicitly converted to an unsigned value in an
 //   assignment, comparison or multiplication.
-// * assignment / initialization when source value is greater than the max
-//   value of target
+// * assignment / initialization when the source value is greater than the max
+//   value of the target integer type
+// * assignment / initialization when the source integer is above the range
+//   where the target floating point type can represent all integers
 //
 // Many compilers and tools have similar checks that are based on semantic
 // analysis. Those checks are sound but have poor precision. ConversionChecker
@@ -28,6 +30,9 @@
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "llvm/ADT/APFloat.h"
+
+#include 
 
 using namespace clang;
 using namespace ento;
@@ -40,11 +45,9 @@
 private:
   mutable std::unique_ptr BT;
 
-  // Is there loss of precision
   bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType,
  CheckerContext ) const;
 
-  // Is there loss of sign
   bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext ) const;
 
   void reportBug(ExplodedNode *N, CheckerContext , const char Msg[]) const;
@@ -132,19 +135,51 @@
 
   QualType SubType = Cast->IgnoreParenImpCasts()->getType();
 
-  if (!DestType->isIntegerType() || !SubType->isIntegerType())
+  if (!DestType->isRealType() || !SubType->isIntegerType())
 return false;
 
-  if (C.getASTContext().getIntWidth(DestType) >=
-  C.getASTContext().getIntWidth(SubType))
+  const bool isFloat = DestType->isFloatingType();
+
+  const auto  = C.getASTContext();
+
+  // We will find the largest RepresentsUntilExp value such that the DestType
+  // can exactly represent all nonnegative integers below 2^RepresentsUntilExp.
+  unsigned RepresentsUntilExp;
+
+  if (isFloat) {
+const llvm::fltSemantics  = AC.getFloatTypeSemantics(DestType);
+RepresentsUntilExp = llvm::APFloat::semanticsPrecision(Sema);
+  } else {
+RepresentsUntilExp = AC.getIntWidth(DestType);
+if (RepresentsUntilExp == 1) {
+  // This is just casting a number to bool, probably not a bug.
+  return false;
+}
+if (DestType->isSignedIntegerType())
+  RepresentsUntilExp--;
+  }
+
+  if (RepresentsUntilExp >= sizeof(unsigned long long) * CHAR_BIT) {
+// Avoid overflow in our later calculations.
 return false;
+  }
+
+  unsigned CorrectedSrcWidth = AC.getIntWidth(SubType);
+  if (SubType->isSignedIntegerType())
+CorrectedSrcWidth--;
 
-  unsigned W = C.getASTContext().getIntWidth(DestType);
-  if (W == 1 || W >= 64U)
+  if (RepresentsUntilExp >= CorrectedSrcWidth) {
+// Simple case: the destination can store all values of the source type.
 return false;
+  }
 
-  unsigned long long MaxVal = 1ULL << W;
+  unsigned long long MaxVal = 1ULL << RepresentsUntilExp;
+  if (isFloat) {
+// If this is a floating point type, it can also represent MaxVal exactly.
+MaxVal++;
+  }
   return C.isGreaterOrEqual(Cast->getSubExpr(), MaxVal);
+  // TODO: maybe also check negative values with too large magnitude.
 }
 
 bool 

[PATCH] D54141: [clang-tidy] add deduplication support for run-clang-tidy.py

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

>> If you're suggesting proceeding with this regex based solution, I
> 
> don't think that's a good idea. Why commit a hack which people will object to 
> ever removing? Just see if we can do the right thing instead.

+1, my main concern is the complexity of the patch and maintenance burden of 
the python script.

In https://reviews.llvm.org/D54141#1288811, @JonasToth wrote:

> > - The output of clang-tidy diagnostic is YAML, and YAML is not an 
> > space-efficient format (just for human readability). If you want to save 
> > space further, we might consider using some compressed formats, e.g. 
> > llvm::bitcode. Given the reduced YAML result (5.4MB) is promising, this 
> > might not matter.
>
> The output were normal diagnostics written to stdout, deduplication happens 
> from there (see the test-cases). The files i created were just through piping 
> to filter some of the noise.
>  Without de-duplication its very hard to get something useful out of a run 
> with many checks activated for bigger projects (e.g. Blender and OpenCV are 
> useless to try, because they have some commonly used macros with a 
> check-violation. The buildbot filled 30GB of RAM before it crashed and 
> couldn't even finish the analysis of the project. Similar for LLVM).
>
> I would like to try the simple deduplication first and see if space is still 
> an issue. After all I want to just read the diagnostic and see whats 
> happening instantly and a more compressed format might not help there.


I misthought that the output was the `-export-fixes`, but what you mean is the 
stdout of clang-tidy.

Could you please explain your motivation of catching clang-tidy stdout? 
`--export-fixes` emits everything of `diagnostic` to YAML even the `diagnostic` 
doesn't have fixes. I guess the reason is that you want code snippets that you 
could show to users? If so, I think this is a separate UX problem, since we 
have everything in the emitted YAML, and we could construct whatever messages 
we want from it. A simpler approach maybe:

1. run clang-tidy in parallel on whole project, and emits a deduplicated result 
(`fixes.yaml`).
2. run a postprocessing in your buildbot that constructs diagnostic messages 
from `fixes.yaml`, and store it somewhere.
3. do whatever you want with output from 1) and 2).

Step 1 could be done in upstream, probably via `AllTUsExecutor`, and 
deduplication can be done on the fly based on 
``; we still need 
`clang-apply-replacement` to deduplicate replacements; I'm happy to help with 
this. Step 2 could be done by your own, just a simple script.

> At the moment clang-apply-replacements is called at the end of an clang-tidy 
> run in run-clang-tidy.py That means we produce ~GBs of Yaml first, to then 
> emit ~10MBs worth of it.

That's why I suggest using some sort of other space-efficient formats to store 
the fixes. My intuition is that the final deduplicated result shouldn't be too 
large (even for YAML), because 1) no duplication 2) these are **actual 
diagnostics** in code, a healthy codebase shouldn't contain lots of problem 3) 
you have mentioned that you use it for small projects :)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54141



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

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov updated this revision to Diff 172917.
AlexeySachkov added a comment.

Updated opencl-c.h header: fixed typos in built-in declarations. Applied 
comment from Alexey


https://reviews.llvm.org/D51484

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenCLExtensionTypes.def
  include/clang/Basic/OpenCLExtensions.def
  include/clang/Sema/Initialization.h
  include/clang/Serialization/ASTBitCodes.h
  include/clang/module.modulemap
  lib/AST/ASTContext.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/PrintfFormatString.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGExprScalar.cpp
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Headers/opencl-c.h
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaInit.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  test/Headers/opencl-c-header.cl
  test/Index/opencl-types.cl
  test/SemaOpenCL/extension-begin.cl
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
  tools/libclang/CIndex.cpp
  tools/libclang/CXType.cpp

Index: tools/libclang/CXType.cpp
===
--- tools/libclang/CXType.cpp
+++ tools/libclang/CXType.cpp
@@ -70,6 +70,8 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) BTCASE(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) BTCASE(Id);
+#include "clang/Basic/OpenCLExtensionTypes.def"
 BTCASE(OCLSampler);
 BTCASE(OCLEvent);
 BTCASE(OCLQueue);
@@ -605,6 +607,8 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) TKIND(Id);
 #include "clang/Basic/OpenCLImageTypes.def"
 #undef IMAGE_TYPE
+#define EXT_OPAQUE_TYPE(ExtTYpe, Id, Ext) TKIND(Id);
+#include "clang/Basic/OpenCLExtensionTypes.def"
 TKIND(OCLSampler);
 TKIND(OCLEvent);
 TKIND(OCLQueue);
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -1519,6 +1519,9 @@
 #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
   case BuiltinType::Id:
 #include "clang/Basic/OpenCLImageTypes.def"
+#define EXT_OPAQUE_TYPE(ExtTYpe, Id, Ext) \
+  case BuiltinType::Id:
+#include "clang/Basic/OpenCLExtensionTypes.def"
   case BuiltinType::OCLSampler:
   case BuiltinType::OCLEvent:
   case BuiltinType::OCLClkEvent:
Index: test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
===
--- /dev/null
+++ test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
@@ -0,0 +1,105 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=CL1.2 -cl-ext=+cl_intel_device_side_avc_motion_estimation -fsyntax-only -verify %s
+
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+
+// All intel_sub_group_avc_* types can only be used as argument or return value
+// of built-in functions defined in the extension.
+// But there are also additional initialization rules:
+//   * All types except intel_sub_group_avc_mce_* types can be initialized with
+// the corresponding initializer macro defined in opencl-c.h
+// Currently all these macroses are defined as 0x0
+//   * In previous versions of the extension these macroses was defined as {0},
+// so initialization with initializer list containing one integer equal to
+// zero should also work
+
+struct st{};
+// negative test cases for initializers
+void foo(char c, float f, void* v, struct st ss) {
+  intel_sub_group_avc_mce_payload_t payload_mce = 0; // No zero initializer for mce types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_payload_t' with an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ime_payload_t payload_ime = 1; // No literal initializer for *payload_t types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_ime_payload_t' with an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ref_payload_t payload_ref = f;
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_ref_payload_t' with an expression of incompatible type 'float'}}
+  intel_sub_group_avc_sic_payload_t payload_sic = ss;
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_sic_payload_t' with an expression of incompatible type 'struct st'}}
+
+  intel_sub_group_avc_mce_result_t result_mce = 0; // No zero initializer for mce types
+  // expected-error@-1 {{initializing 'intel_sub_group_avc_mce_result_t' with an expression of incompatible type 'int'}}
+  intel_sub_group_avc_ime_result_t 

[PATCH] D54148: [NFC][Clang][Aarch64] Add missing test file

[Diogo N. Sampaio via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346303: [NFC][Clang][Aarch64] Add missing test file 
(authored by dnsampaio, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D54148?vs=172743=172918#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D54148

Files:
  cfe/trunk/test/Driver/aarch64-security-options.c


Index: cfe/trunk/test/Driver/aarch64-security-options.c
===
--- cfe/trunk/test/Driver/aarch64-security-options.c
+++ cfe/trunk/test/Driver/aarch64-security-options.c
@@ -0,0 +1,54 @@
+// Check the -msign-return-address= option, which has a required argument to
+// select scope.
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-OFF  --check-prefix=KEY-A 
--check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A 
--check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all 
 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-A 
--check-prefix=BTE-OFF
+
+// Check that the -msign-return-address= option can also accept the signing key
+// to use.
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf   2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-B 
--check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all 
   2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-B 
--check-prefix=BTE-OFF
+
+// -mbranch-protection with standard
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-mbranch-protection=standard2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A 
--check-prefix=BTE-ON
+
+// If the -msign-return-address and -mbranch-protection are both used, the
+// right-most one controls return address signing.
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf -mbranch-protection=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CONFLICT
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-mbranch-protection=pac-ret -msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CONFLICT
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=foo 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-RA-PROTECTION
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mbranch-protection=bar   
  2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-BP-PROTECTION
+
+// RA-OFF: "-msign-return-address=none"
+// RA-NON-LEAF: "-msign-return-address=non-leaf"
+// RA-ALL: "-msign-return-address=all"
+
+// KEY-A: "-msign-return-address-key=a_key"
+
+// BTE-OFF-NOT: "-mbranch-target-enforce"
+// BTE-ON: "-mbranch-target-enforce"
+
+// CONFLICT: "-msign-return-address=none"
+
+// BAD-RA-PROTECTION: invalid branch protection option 'foo' in 
'-msign-return-address={{.*}}'
+// BAD-BP-PROTECTION: invalid branch protection option 'bar' in 
'-mbranch-protection={{.*}}'
+
+// BAD-B-KEY-COMBINATION: invalid branch protection option 'b-key' in 
'-mbranch-protection={{.*}}'
+// BAD-LEAF-COMBINATION: invalid branch protection option 'leaf' in 
'-mbranch-protection={{.*}}'


Index: cfe/trunk/test/Driver/aarch64-security-options.c
===
--- cfe/trunk/test/Driver/aarch64-security-options.c
+++ cfe/trunk/test/Driver/aarch64-security-options.c
@@ -0,0 +1,54 @@
+// Check the -msign-return-address= option, which has a required argument to
+// select scope.
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-OFF  --check-prefix=KEY-A --check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=non-leaf 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A --check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all  2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-A --check-prefix=BTE-OFF
+
+// Check that the -msign-return-address= option can also accept the signing key
+// to use.
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=non-leaf   2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-B --check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 

r346303 - [NFC][Clang][Aarch64] Add missing test file

[Diogo N. Sampaio via cfe-commits]

Author: dnsampaio
Date: Wed Nov  7 03:42:02 2018
New Revision: 346303

URL: http://llvm.org/viewvc/llvm-project?rev=346303=rev
Log:
[NFC][Clang][Aarch64] Add missing test file

The commit rL345273 by @LukeCheeseman has a missing test file,
see https://reviews.llvm.org/D51429
This patch adds the missing test file.

Patch by Luke Cheeseman

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


Added:
cfe/trunk/test/Driver/aarch64-security-options.c

Added: cfe/trunk/test/Driver/aarch64-security-options.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/aarch64-security-options.c?rev=346303=auto
==
--- cfe/trunk/test/Driver/aarch64-security-options.c (added)
+++ cfe/trunk/test/Driver/aarch64-security-options.c Wed Nov  7 03:42:02 2018
@@ -0,0 +1,54 @@
+// Check the -msign-return-address= option, which has a required argument to
+// select scope.
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-OFF  --check-prefix=KEY-A 
--check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A 
--check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all 
 2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-A 
--check-prefix=BTE-OFF
+
+// Check that the -msign-return-address= option can also accept the signing key
+// to use.
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf   2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-B 
--check-prefix=BTE-OFF
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=all 
   2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-ALL  --check-prefix=KEY-B 
--check-prefix=BTE-OFF
+
+// -mbranch-protection with standard
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-mbranch-protection=standard2>&1 | \
+// RUN: FileCheck %s --check-prefix=RA-NON-LEAF --check-prefix=KEY-A 
--check-prefix=BTE-ON
+
+// If the -msign-return-address and -mbranch-protection are both used, the
+// right-most one controls return address signing.
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-msign-return-address=non-leaf -mbranch-protection=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CONFLICT
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### 
-mbranch-protection=pac-ret -msign-return-address=none 2>&1 | \
+// RUN: FileCheck %s --check-prefix=CONFLICT
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -msign-return-address=foo 
2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-RA-PROTECTION
+
+// RUN: %clang -target aarch64--none-eabi -c %s -### -mbranch-protection=bar   
  2>&1 | \
+// RUN: FileCheck %s --check-prefix=BAD-BP-PROTECTION
+
+// RA-OFF: "-msign-return-address=none"
+// RA-NON-LEAF: "-msign-return-address=non-leaf"
+// RA-ALL: "-msign-return-address=all"
+
+// KEY-A: "-msign-return-address-key=a_key"
+
+// BTE-OFF-NOT: "-mbranch-target-enforce"
+// BTE-ON: "-mbranch-target-enforce"
+
+// CONFLICT: "-msign-return-address=none"
+
+// BAD-RA-PROTECTION: invalid branch protection option 'foo' in 
'-msign-return-address={{.*}}'
+// BAD-BP-PROTECTION: invalid branch protection option 'bar' in 
'-mbranch-protection={{.*}}'
+
+// BAD-B-KEY-COMBINATION: invalid branch protection option 'b-key' in 
'-mbranch-protection={{.*}}'
+// BAD-LEAF-COMBINATION: invalid branch protection option 'leaf' in 
'-mbranch-protection={{.*}}'


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

[PATCH] D54038: [mips][msa] Fix msa_[st/ld] offset check

[Aleksandar Beserminji via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346302: [mips][msa] Fix msa_[st/ld] offset check (authored 
by abeserminji, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D54038

Files:
  lib/Sema/SemaChecking.cpp
  test/CodeGen/builtins-mips-msa-error.c
  test/CodeGen/builtins-mips-msa.c


Index: test/CodeGen/builtins-mips-msa.c
===
--- test/CodeGen/builtins-mips-msa.c
+++ test/CodeGen/builtins-mips-msa.c
@@ -520,10 +520,10 @@
   v4i32_r = __msa_insve_w(v4i32_r, 1, v4i32_a); // CHECK: call <4  x i32> 
@llvm.mips.insve.w(
   v2i64_r = __msa_insve_d(v2i64_r, 1, v2i64_a); // CHECK: call <2  x i64> 
@llvm.mips.insve.d(
 
-  v16i8_r = __msa_ld_b(_a, 16); // CHECK: call <16 x i8>  
@llvm.mips.ld.b(
-  v8i16_r = __msa_ld_h(_a, 32); // CHECK: call <8  x i16> 
@llvm.mips.ld.h(
-  v4i32_r = __msa_ld_w(_a, 48); // CHECK: call <4  x i32> 
@llvm.mips.ld.w(
-  v2i64_r = __msa_ld_d(_a, 96); // CHECK: call <2  x i64> 
@llvm.mips.ld.d(
+  v16i8_r = __msa_ld_b(_a, 1); // CHECK: call <16 x i8>  @llvm.mips.ld.b(
+  v8i16_r = __msa_ld_h(_a, 2); // CHECK: call <8  x i16> @llvm.mips.ld.h(
+  v4i32_r = __msa_ld_w(_a, 4); // CHECK: call <4  x i32> @llvm.mips.ld.w(
+  v2i64_r = __msa_ld_d(_a, 8); // CHECK: call <2  x i64> @llvm.mips.ld.d(
 
   v16i8_r = __msa_ldi_b(3); // CHECK: call <16 x i8>  @llvm.mips.ldi.b(
   v16i8_r = __msa_ldi_b(-128); // CHECK: call <16 x i8>  @llvm.mips.ldi.b(
@@ -771,10 +771,10 @@
   v4i32_r = __msa_srlri_w(v4i32_a, 3); // CHECK: call <4  x i32> 
@llvm.mips.srlri.w(
   v2i64_r = __msa_srlri_d(v2i64_a, 3); // CHECK: call <2  x i64> 
@llvm.mips.srlri.d(
 
-  __msa_st_b(v16i8_b, _a, 16); // CHECK: call void @llvm.mips.st.b(
-  __msa_st_h(v8i16_b, _a, 32); // CHECK: call void @llvm.mips.st.h(
-  __msa_st_w(v4i32_b, _a, 48); // CHECK: call void @llvm.mips.st.w(
-  __msa_st_d(v2i64_b, _a, 96); // CHECK: call void @llvm.mips.st.d(
+  __msa_st_b(v16i8_b, _a, 1); // CHECK: call void @llvm.mips.st.b(
+  __msa_st_h(v8i16_b, _a, 2); // CHECK: call void @llvm.mips.st.h(
+  __msa_st_w(v4i32_b, _a, 4); // CHECK: call void @llvm.mips.st.w(
+  __msa_st_d(v2i64_b, _a, 8); // CHECK: call void @llvm.mips.st.d(
 
   v16i8_r = __msa_subs_s_b(v16i8_a, v16i8_b); // CHECK: call <16 x i8>  
@llvm.mips.subs.s.b(
   v8i16_r = __msa_subs_s_h(v8i16_a, v8i16_b); // CHECK: call <8  x i16> 
@llvm.mips.subs.s.h(
Index: test/CodeGen/builtins-mips-msa-error.c
===
--- test/CodeGen/builtins-mips-msa-error.c
+++ test/CodeGen/builtins-mips-msa-error.c
@@ -112,9 +112,9 @@
   v4i32_r = __msa_insve_w(v4i32_r, 4, v4i32_a);  // expected-error 
{{argument value 4 is outside the valid range [0, 3]}}
   v2i64_r = __msa_insve_d(v2i64_r, 2, v2i64_a);  // expected-error 
{{argument value 2 is outside the valid range [0, 1]}}
 
-  v8i16_r = __msa_ld_h(_a, 77);// expected-error 
{{argument should be a multiple of 16}}
-  v4i32_r = __msa_ld_w(_a, 14);// expected-error 
{{argument should be a multiple of 16}}
-  v2i64_r = __msa_ld_d(_a, 23);// expected-error 
{{argument should be a multiple of 16}}
+  v8i16_r = __msa_ld_h(_a, 77);// expected-error 
{{argument should be a multiple of 2}}
+  v4i32_r = __msa_ld_w(_a, 14);// expected-error 
{{argument should be a multiple of 4}}
+  v2i64_r = __msa_ld_d(_a, 23);// expected-error 
{{argument should be a multiple of 8}}
 
   v16i8_r = __msa_ld_b(_a, 512);   // expected-error 
{{argument value 512 is outside the valid range [-512, 511]}}
   v8i16_r = __msa_ld_h(_a, 1024);  // expected-error 
{{argument value 1024 is outside the valid range [-1024, 1022]}}
@@ -199,9 +199,9 @@
   v4i32_r = __msa_srlri_w(v4i32_a, 32);  // expected-error 
{{argument value 32 is outside the valid range [0, 31]}}
   v2i64_r = __msa_srlri_d(v2i64_a, 64);  // expected-error 
{{argument value 64 is outside the valid range [0, 63]}}
 
-  __msa_st_h(v8i16_b, _a, 51);// expected-error 
{{argument should be a multiple of 16}}
-  __msa_st_w(v4i32_b, _a, 51);// expected-error 
{{argument should be a multiple of 16}}
-  __msa_st_d(v2i64_b, _a, 12);// expected-error 
{{argument should be a multiple of 16}}
+  __msa_st_h(v8i16_b, _a, 51);// expected-error 
{{argument should be a multiple of 2}}
+  __msa_st_w(v4i32_b, _a, 51);// expected-error 
{{argument should be a multiple of 4}}
+  __msa_st_d(v2i64_b, _a, 12);// expected-error 
{{argument should be a multiple of 8}}
 
   __msa_st_b(v16i8_b, _a, 512);// expected-error 
{{argument value 512 is outside the valid range [-512, 511]}}
   __msa_st_h(v8i16_b, _a, 1024);// expected-error 
{{argument 

r346302 - [mips][msa] Fix msa_[st/ld] offset check

[Aleksandar Beserminji via cfe-commits]

Author: abeserminji
Date: Wed Nov  7 03:37:05 2018
New Revision: 346302

URL: http://llvm.org/viewvc/llvm-project?rev=346302=rev
Log:
[mips][msa] Fix msa_[st/ld] offset check

This patch fixes a minimum divider for offset in intrinsics
msa_[st/ld]_[b/h/w/d], when value is known in compile time.

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

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/CodeGen/builtins-mips-msa-error.c
cfe/trunk/test/CodeGen/builtins-mips-msa.c

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=346302=346301=346302=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Nov  7 03:37:05 2018
@@ -2992,14 +2992,14 @@ bool Sema::CheckMipsBuiltinFunctionCall(
   case Mips::BI__builtin_msa_ldi_h:
   case Mips::BI__builtin_msa_ldi_w:
   case Mips::BI__builtin_msa_ldi_d: i = 0; l = -512; u = 511; break;
-  case Mips::BI__builtin_msa_ld_b: i = 1; l = -512; u = 511; m = 16; break;
-  case Mips::BI__builtin_msa_ld_h: i = 1; l = -1024; u = 1022; m = 16; break;
-  case Mips::BI__builtin_msa_ld_w: i = 1; l = -2048; u = 2044; m = 16; break;
-  case Mips::BI__builtin_msa_ld_d: i = 1; l = -4096; u = 4088; m = 16; break;
-  case Mips::BI__builtin_msa_st_b: i = 2; l = -512; u = 511; m = 16; break;
-  case Mips::BI__builtin_msa_st_h: i = 2; l = -1024; u = 1022; m = 16; break;
-  case Mips::BI__builtin_msa_st_w: i = 2; l = -2048; u = 2044; m = 16; break;
-  case Mips::BI__builtin_msa_st_d: i = 2; l = -4096; u = 4088; m = 16; break;
+  case Mips::BI__builtin_msa_ld_b: i = 1; l = -512; u = 511; m = 1; break;
+  case Mips::BI__builtin_msa_ld_h: i = 1; l = -1024; u = 1022; m = 2; break;
+  case Mips::BI__builtin_msa_ld_w: i = 1; l = -2048; u = 2044; m = 4; break;
+  case Mips::BI__builtin_msa_ld_d: i = 1; l = -4096; u = 4088; m = 8; break;
+  case Mips::BI__builtin_msa_st_b: i = 2; l = -512; u = 511; m = 1; break;
+  case Mips::BI__builtin_msa_st_h: i = 2; l = -1024; u = 1022; m = 2; break;
+  case Mips::BI__builtin_msa_st_w: i = 2; l = -2048; u = 2044; m = 4; break;
+  case Mips::BI__builtin_msa_st_d: i = 2; l = -4096; u = 4088; m = 8; break;
   }
 
   if (!m)

Modified: cfe/trunk/test/CodeGen/builtins-mips-msa-error.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/builtins-mips-msa-error.c?rev=346302=346301=346302=diff
==
--- cfe/trunk/test/CodeGen/builtins-mips-msa-error.c (original)
+++ cfe/trunk/test/CodeGen/builtins-mips-msa-error.c Wed Nov  7 03:37:05 2018
@@ -112,9 +112,9 @@ void test(void) {
   v4i32_r = __msa_insve_w(v4i32_r, 4, v4i32_a);  // expected-error 
{{argument value 4 is outside the valid range [0, 3]}}
   v2i64_r = __msa_insve_d(v2i64_r, 2, v2i64_a);  // expected-error 
{{argument value 2 is outside the valid range [0, 1]}}
 
-  v8i16_r = __msa_ld_h(_a, 77);// expected-error 
{{argument should be a multiple of 16}}
-  v4i32_r = __msa_ld_w(_a, 14);// expected-error 
{{argument should be a multiple of 16}}
-  v2i64_r = __msa_ld_d(_a, 23);// expected-error 
{{argument should be a multiple of 16}}
+  v8i16_r = __msa_ld_h(_a, 77);// expected-error 
{{argument should be a multiple of 2}}
+  v4i32_r = __msa_ld_w(_a, 14);// expected-error 
{{argument should be a multiple of 4}}
+  v2i64_r = __msa_ld_d(_a, 23);// expected-error 
{{argument should be a multiple of 8}}
 
   v16i8_r = __msa_ld_b(_a, 512);   // expected-error 
{{argument value 512 is outside the valid range [-512, 511]}}
   v8i16_r = __msa_ld_h(_a, 1024);  // expected-error 
{{argument value 1024 is outside the valid range [-1024, 1022]}}
@@ -199,9 +199,9 @@ void test(void) {
   v4i32_r = __msa_srlri_w(v4i32_a, 32);  // expected-error 
{{argument value 32 is outside the valid range [0, 31]}}
   v2i64_r = __msa_srlri_d(v2i64_a, 64);  // expected-error 
{{argument value 64 is outside the valid range [0, 63]}}
 
-  __msa_st_h(v8i16_b, _a, 51);// expected-error 
{{argument should be a multiple of 16}}
-  __msa_st_w(v4i32_b, _a, 51);// expected-error 
{{argument should be a multiple of 16}}
-  __msa_st_d(v2i64_b, _a, 12);// expected-error 
{{argument should be a multiple of 16}}
+  __msa_st_h(v8i16_b, _a, 51);// expected-error 
{{argument should be a multiple of 2}}
+  __msa_st_w(v4i32_b, _a, 51);// expected-error 
{{argument should be a multiple of 4}}
+  __msa_st_d(v2i64_b, _a, 12);// expected-error 
{{argument should be a multiple of 8}}
 
   __msa_st_b(v16i8_b, _a, 512);// expected-error 
{{argument value 512 is outside the valid range [-512, 511]}}
   

[PATCH] D54148: [NFC][Clang][Aarch64] Add missing test file

[Oliver Stannard via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D54148



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

[PATCH] D50801: [rename] Use isPointWithin when looking for a declaration at location

[Jan Korous via Phabricator via cfe-commits]

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

LGTM Thanks for fixing this!


Repository:
  rC Clang

https://reviews.llvm.org/D50801



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

[PATCH] D50926: [SourceManager] Extract 'findFileIDsForFile' from 'translateFile' to allow mapping from one file entry to multiple FileIDs

[Jan Korous via Phabricator via cfe-commits]

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

LGTM and it seems like all of Eric's comments were answered too.


Repository:
  rC Clang

https://reviews.llvm.org/D50926



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

Re: [PATCH] D53651: [clangd] Use thread pool for background indexing.

[Mikael Holmén via cfe-commits]

On 11/7/18 11:31 AM, Sam McCall wrote:
> I'll send a patch shortly to unbreak this.
> 
> Is emitting a warning a problem for you? (i.e. do you build with -Werror)

Yes "unfortunately" we use -Werror.

> I'd like to do something like #ifdef SCHED_IDLE ... #else #warning "old 
> libc?" #endif
> That way if this isn't actually working we'll break in linux 
> configurations covered by -Werror bots, we'll notice.
> 

I can always remove the #warning locally if you want to give a warning 
about it but nicest for me would of course be if it was silent. :)

/Mikael

> On Wed, Nov 7, 2018 at 11:22 AM Mikael Holmén 
> mailto:mikael.hol...@ericsson.com>> wrote:
> 
> Hi,
> 
> On 11/7/18 11:03 AM, Sam McCall wrote:
>  > On Wed, Nov 7, 2018 at 10:32 AM Mikael Holmén via Phabricator
>  > mailto:revi...@reviews.llvm.org>
> >>
> wrote:
>  >
>  >     uabelho added a comment.
>  >
>  >     Hi,
>  >
>  >     I've got a post-review comment about the use of SCHED_IDLE vs the
>  >     needed gcc version.
>  >
>  >
>  >
>  >     
>  >     Comment at: clang-tools-extra/trunk/clangd/Threading.cpp:110
>  >     +      T.native_handle(),
>  >     +      Priority == ThreadPriority::Low ? SCHED_IDLE :
> SCHED_OTHER,
>  >     );
>  >     +#endif
>  >     
>  >     I noticed that when I compile this with gcc 5.4.0, I get a
>  >     compilation error about SCHED_IDLE not being defined.
>  >     Indeed, if I look in sched.h used with gcc 5.4.0, these are the
>  >     SCHED_* definitions:
>  >
>  > Sorry for the breakage here!
>  >
>  > sched.h comes from glibc, rather than GCC.
>  > SCHED_IDLE was added in glibc 2.12, which is considerably older
> than any
>  > supported compiler (2010, vs 2013 for gcc 4.8 and 2016 for gcc 5.4).
>  >
>  > I don't think there's a documented policy on libc versions...
> what does
>  > this system look like? (e.g. glibc version, distribution?)
> 
> Ok, we're apparently using glibc 2.11.3. A bit too old then :/
> 
> We're cross-compiling llvm using rather old gcc and glibc versions so
> that it will work on a number of different machines with different OS
> versions that our users have.
> 
> /Mikael
> 
>  >
>  > (It seems like it would be OK to #ifdef SCHED_IDLE here and do
> nothing
>  > the libc is just too old, but it may mask other problems we'd
> want to
>  > know about).
>  >
>  >     ```
>  >     /* Scheduling algorithms.  */
>  >     #define SCHED_OTHER     0
>  >     #define SCHED_FIFO      1
>  >     #define SCHED_RR        2
>  >     #ifdef __USE_GNU
>  >     # define SCHED_BATCH    3
>  >     #endif
>  >     ```
>  >     I suppose SCHED_IDLE was added in some later version...
>  >
>  >     On
>  >
> 
> https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library
>  >     it says that gcc 4.8 should work, but it doesn't now then?
>  >
>  >
>  >
>  >     Repository:
>  >        rL LLVM
>  >
>  > https://reviews.llvm.org/D53651
>  >
>  >
>  >
> 

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

Re: [PATCH] D53651: [clangd] Use thread pool for background indexing.

[Sam McCall via cfe-commits]

I'll send a patch shortly to unbreak this.

Is emitting a warning a problem for you? (i.e. do you build with -Werror)
I'd like to do something like #ifdef SCHED_IDLE ... #else #warning "old
libc?" #endif
That way if this isn't actually working we'll break in linux configurations
covered by -Werror bots, we'll notice.

On Wed, Nov 7, 2018 at 11:22 AM Mikael Holmén 
wrote:

> Hi,
>
> On 11/7/18 11:03 AM, Sam McCall wrote:
> > On Wed, Nov 7, 2018 at 10:32 AM Mikael Holmén via Phabricator
> > mailto:revi...@reviews.llvm.org>> wrote:
> >
> > uabelho added a comment.
> >
> > Hi,
> >
> > I've got a post-review comment about the use of SCHED_IDLE vs the
> > needed gcc version.
> >
> >
> >
> > 
> > Comment at: clang-tools-extra/trunk/clangd/Threading.cpp:110
> > +  T.native_handle(),
> > +  Priority == ThreadPriority::Low ? SCHED_IDLE : SCHED_OTHER,
> > );
> > +#endif
> > 
> > I noticed that when I compile this with gcc 5.4.0, I get a
> > compilation error about SCHED_IDLE not being defined.
> > Indeed, if I look in sched.h used with gcc 5.4.0, these are the
> > SCHED_* definitions:
> >
> > Sorry for the breakage here!
> >
> > sched.h comes from glibc, rather than GCC.
> > SCHED_IDLE was added in glibc 2.12, which is considerably older than any
> > supported compiler (2010, vs 2013 for gcc 4.8 and 2016 for gcc 5.4).
> >
> > I don't think there's a documented policy on libc versions... what does
> > this system look like? (e.g. glibc version, distribution?)
>
> Ok, we're apparently using glibc 2.11.3. A bit too old then :/
>
> We're cross-compiling llvm using rather old gcc and glibc versions so
> that it will work on a number of different machines with different OS
> versions that our users have.
>
> /Mikael
>
> >
> > (It seems like it would be OK to #ifdef SCHED_IDLE here and do nothing
> > the libc is just too old, but it may mask other problems we'd want to
> > know about).
> >
> > ```
> > /* Scheduling algorithms.  */
> > #define SCHED_OTHER 0
> > #define SCHED_FIFO  1
> > #define SCHED_RR2
> > #ifdef __USE_GNU
> > # define SCHED_BATCH3
> > #endif
> > ```
> > I suppose SCHED_IDLE was added in some later version...
> >
> > On
> >
> https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library
> > it says that gcc 4.8 should work, but it doesn't now then?
> >
> >
> >
> > Repository:
> >rL LLVM
> >
> > https://reviews.llvm.org/D53651
> >
> >
> >
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D53651: [clangd] Use thread pool for background indexing.

[Mikael Holmén via cfe-commits]

Hi,

On 11/7/18 11:03 AM, Sam McCall wrote:
> On Wed, Nov 7, 2018 at 10:32 AM Mikael Holmén via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> 
> uabelho added a comment.
> 
> Hi,
> 
> I've got a post-review comment about the use of SCHED_IDLE vs the
> needed gcc version.
> 
> 
> 
> 
> Comment at: clang-tools-extra/trunk/clangd/Threading.cpp:110
> +      T.native_handle(),
> +      Priority == ThreadPriority::Low ? SCHED_IDLE : SCHED_OTHER,
> );
> +#endif
> 
> I noticed that when I compile this with gcc 5.4.0, I get a
> compilation error about SCHED_IDLE not being defined.
> Indeed, if I look in sched.h used with gcc 5.4.0, these are the
> SCHED_* definitions:
> 
> Sorry for the breakage here!
> 
> sched.h comes from glibc, rather than GCC.
> SCHED_IDLE was added in glibc 2.12, which is considerably older than any 
> supported compiler (2010, vs 2013 for gcc 4.8 and 2016 for gcc 5.4).
> 
> I don't think there's a documented policy on libc versions... what does 
> this system look like? (e.g. glibc version, distribution?)

Ok, we're apparently using glibc 2.11.3. A bit too old then :/

We're cross-compiling llvm using rather old gcc and glibc versions so 
that it will work on a number of different machines with different OS 
versions that our users have.

/Mikael

> 
> (It seems like it would be OK to #ifdef SCHED_IDLE here and do nothing 
> the libc is just too old, but it may mask other problems we'd want to 
> know about).
> 
> ```
> /* Scheduling algorithms.  */
> #define SCHED_OTHER     0
> #define SCHED_FIFO      1
> #define SCHED_RR        2
> #ifdef __USE_GNU
> # define SCHED_BATCH    3
> #endif
> ```
> I suppose SCHED_IDLE was added in some later version...
> 
> On
> 
> https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library
> it says that gcc 4.8 should work, but it doesn't now then?
> 
> 
> 
> Repository:
>    rL LLVM
> 
> https://reviews.llvm.org/D53651
> 
> 
> 

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

[PATCH] D54156: [CodeComplete] Do not complete self-initializations

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL346301: [CodeComplete] Do not complete self-initializations 
(authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D54156

Files:
  cfe/trunk/lib/Sema/SemaCodeComplete.cpp
  cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp
  cfe/trunk/test/CodeCompletion/ordinary-name.cpp
  cfe/trunk/test/CodeCompletion/self-inits.cpp
  cfe/trunk/test/Index/complete-type-factors.m


Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -4747,7 +4747,12 @@
 return;
   }
 
-  CodeCompleteExpression(S, VD->getType());
+  CodeCompleteExpressionData Data;
+  Data.PreferredType = VD->getType();
+  // Ignore VD to avoid completing the variable itself, e.g. in 'int foo = ^'.
+  Data.IgnoreDecls.push_back(VD);
+
+  CodeCompleteExpression(S, Data);
 }
 
 void Sema::CodeCompleteReturn(Scope *S) {
Index: cfe/trunk/test/Index/complete-type-factors.m
===
--- cfe/trunk/test/Index/complete-type-factors.m
+++ cfe/trunk/test/Index/complete-type-factors.m
@@ -39,16 +39,14 @@
 // CHECK-CC1: FunctionDecl:{ResultType enum Priority}{TypedText 
func2}{LeftParen (}{Placeholder int}{RightParen )} (25)
 // CHECK-CC1: EnumConstantDecl:{ResultType enum Color}{TypedText Green} (32)
 // CHECK-CC1: EnumConstantDecl:{ResultType enum Priority}{TypedText High} (32)
-// CHECK-CC1: VarDecl:{ResultType int}{TypedText i} (8)
 // CHECK-CC1: ParmDecl:{ResultType int}{TypedText integer} (8)
 // CHECK-CC1: EnumConstantDecl:{ResultType enum Priority}{TypedText Low} (32)
 // CHECK-CC1: ParmDecl:{ResultType enum Priority}{TypedText priority} (17)
 // CHECK-CC1: EnumConstantDecl:{ResultType enum Color}{TypedText Red} (32)
 // CHECK-CC1: NotImplemented:{ResultType size_t}{TypedText sizeof}{LeftParen 
(}{Placeholder expression-or-type}{RightParen )} (40)
 // CHECK-CC1: FunctionDecl:{ResultType enum Priority}{TypedText 
test1}{LeftParen (}{Placeholder enum Priority priority}{Comma , }{Placeholder 
enum Color color}{Comma , }{Placeholder int integer}{RightParen )} (25)
 // RUN: c-index-test -code-completion-at=%s:17:18 -Xclang 
-code-completion-patterns %s | FileCheck -check-prefix=CHECK-CC2 %s
 // CHECK-CC2: EnumConstantDecl:{ResultType enum Color}{TypedText Blue} (16)
-// CHECK-CC2: VarDecl:{ResultType enum Color}{TypedText c} (8)
 // CHECK-CC2: ParmDecl:{ResultType enum Color}{TypedText color} (8)
 // CHECK-CC2: FunctionDecl:{ResultType int}{TypedText func1}{LeftParen 
(}{Placeholder enum Color}{RightParen )} (25)
 // CHECK-CC2: FunctionDecl:{ResultType enum Priority}{TypedText 
func2}{LeftParen (}{Placeholder int}{RightParen )} (50)
Index: cfe/trunk/test/CodeCompletion/self-inits.cpp
===
--- cfe/trunk/test/CodeCompletion/self-inits.cpp
+++ cfe/trunk/test/CodeCompletion/self-inits.cpp
@@ -0,0 +1,3 @@
+int foo = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:1:11 %s -o - | 
FileCheck --check-prefix=CC1 %s
+// CC1-NOT: foo
Index: cfe/trunk/test/CodeCompletion/ordinary-name.cpp
===
--- cfe/trunk/test/CodeCompletion/ordinary-name.cpp
+++ cfe/trunk/test/CodeCompletion/ordinary-name.cpp
@@ -171,7 +171,6 @@
   // CHECK-CC4-NEXT: COMPLETION: volatile
   // CHECK-CC4-NEXT: COMPLETION: wchar_t
   // CHECK-CC4-NEXT: COMPLETION: X : X
-  // CHECK-CC4-NEXT: COMPLETION: y : [#int#]y
   // CHECK-CC4-NEXT: COMPLETION: z : [#void#]z(<#int#>)
 
   // RUN: %clang_cc1 -fsyntax-only -fno-rtti -code-completion-patterns 
-code-completion-at=%s:6:14 -std=gnu++98 %s -o - | FileCheck 
-check-prefix=CHECK-NO-RTTI %s
Index: cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp
===
--- cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp
+++ cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp
@@ -197,7 +197,6 @@
   // CHECK-CC4-NEXT: COMPLETION: volatile
   // CHECK-CC4-NEXT: COMPLETION: wchar_t
   // CHECK-CC4-NEXT: COMPLETION: X : X
-  // CHECK-CC4-NEXT: COMPLETION: y : [#int#]y
   // CHECK-CC4-NEXT: COMPLETION: z : [#void#]z(<#int#>)
 
   // RUN: %clang_cc1 -fsyntax-only -fno-rtti -code-completion-patterns 
-code-completion-at=%s:6:14 -std=gnu++11 %s -o - | FileCheck 
-check-prefix=CHECK-NO-RTTI %s


Index: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
===
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp
@@ -4747,7 +4747,12 @@
 return;
   }
 
-  CodeCompleteExpression(S, VD->getType());
+  CodeCompleteExpressionData Data;
+  Data.PreferredType = VD->getType();
+  // Ignore VD to avoid completing the 

r346301 - [CodeComplete] Do not complete self-initializations

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Nov  7 02:02:31 2018
New Revision: 346301

URL: http://llvm.org/viewvc/llvm-project?rev=346301=rev
Log:
[CodeComplete] Do not complete self-initializations

Summary:
Removes references to initialized variable from the following completions:

int x = ^;

Handles only the trivial cases where the variable name is completed
immediately at the start of initializer or assignment, more complicated
cases aren't covered, e.g. these completions still contain 'x':

// More complicated expressions.
int x = foo(^);
int x = 10 + ^;
// Other kinds of initialization.
int x{^};
int x(^);
// Constructor initializers.
struct Foo {
  Foo() : x(^) {}
  int x;
};

We should address those in the future, but they are outside of the scope of
this initial change.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: arphaman, cfe-commits

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

Added:
cfe/trunk/test/CodeCompletion/self-inits.cpp
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp
cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp
cfe/trunk/test/CodeCompletion/ordinary-name.cpp
cfe/trunk/test/Index/complete-type-factors.m

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=346301=346300=346301=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Nov  7 02:02:31 2018
@@ -4747,7 +4747,12 @@ void Sema::CodeCompleteInitializer(Scope
 return;
   }
 
-  CodeCompleteExpression(S, VD->getType());
+  CodeCompleteExpressionData Data;
+  Data.PreferredType = VD->getType();
+  // Ignore VD to avoid completing the variable itself, e.g. in 'int foo = ^'.
+  Data.IgnoreDecls.push_back(VD);
+
+  CodeCompleteExpression(S, Data);
 }
 
 void Sema::CodeCompleteReturn(Scope *S) {

Modified: cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp?rev=346301=346300=346301=diff
==
--- cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp (original)
+++ cfe/trunk/test/CodeCompletion/ordinary-name-cxx11.cpp Wed Nov  7 02:02:31 
2018
@@ -197,7 +197,6 @@ void foo() {
   // CHECK-CC4-NEXT: COMPLETION: volatile
   // CHECK-CC4-NEXT: COMPLETION: wchar_t
   // CHECK-CC4-NEXT: COMPLETION: X : X
-  // CHECK-CC4-NEXT: COMPLETION: y : [#int#]y
   // CHECK-CC4-NEXT: COMPLETION: z : [#void#]z(<#int#>)
 
   // RUN: %clang_cc1 -fsyntax-only -fno-rtti -code-completion-patterns 
-code-completion-at=%s:6:14 -std=gnu++11 %s -o - | FileCheck 
-check-prefix=CHECK-NO-RTTI %s

Modified: cfe/trunk/test/CodeCompletion/ordinary-name.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/ordinary-name.cpp?rev=346301=346300=346301=diff
==
--- cfe/trunk/test/CodeCompletion/ordinary-name.cpp (original)
+++ cfe/trunk/test/CodeCompletion/ordinary-name.cpp Wed Nov  7 02:02:31 2018
@@ -171,7 +171,6 @@ void foo() {
   // CHECK-CC4-NEXT: COMPLETION: volatile
   // CHECK-CC4-NEXT: COMPLETION: wchar_t
   // CHECK-CC4-NEXT: COMPLETION: X : X
-  // CHECK-CC4-NEXT: COMPLETION: y : [#int#]y
   // CHECK-CC4-NEXT: COMPLETION: z : [#void#]z(<#int#>)
 
   // RUN: %clang_cc1 -fsyntax-only -fno-rtti -code-completion-patterns 
-code-completion-at=%s:6:14 -std=gnu++98 %s -o - | FileCheck 
-check-prefix=CHECK-NO-RTTI %s

Added: cfe/trunk/test/CodeCompletion/self-inits.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/self-inits.cpp?rev=346301=auto
==
--- cfe/trunk/test/CodeCompletion/self-inits.cpp (added)
+++ cfe/trunk/test/CodeCompletion/self-inits.cpp Wed Nov  7 02:02:31 2018
@@ -0,0 +1,3 @@
+int foo = 10;
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:1:11 %s -o - | 
FileCheck --check-prefix=CC1 %s
+// CC1-NOT: foo

Modified: cfe/trunk/test/Index/complete-type-factors.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/complete-type-factors.m?rev=346301=346300=346301=diff
==
--- cfe/trunk/test/Index/complete-type-factors.m (original)
+++ cfe/trunk/test/Index/complete-type-factors.m Wed Nov  7 02:02:31 2018
@@ -39,7 +39,6 @@ void test2(A *a) {
 // CHECK-CC1: FunctionDecl:{ResultType enum Priority}{TypedText 
func2}{LeftParen (}{Placeholder int}{RightParen )} (25)
 // CHECK-CC1: EnumConstantDecl:{ResultType enum Color}{TypedText Green} (32)
 // CHECK-CC1: EnumConstantDecl:{ResultType enum Priority}{TypedText High} (32)
-// CHECK-CC1: VarDecl:{ResultType int}{TypedText i} (8)
 // CHECK-CC1: ParmDecl:{ResultType 

Re: [PATCH] D53651: [clangd] Use thread pool for background indexing.

[Sam McCall via cfe-commits]

On Wed, Nov 7, 2018 at 10:32 AM Mikael Holmén via Phabricator <
revi...@reviews.llvm.org> wrote:

> uabelho added a comment.
>
> Hi,
>
> I've got a post-review comment about the use of SCHED_IDLE vs the needed
> gcc version.
>
>
>
> 
> Comment at: clang-tools-extra/trunk/clangd/Threading.cpp:110
> +  T.native_handle(),
> +  Priority == ThreadPriority::Low ? SCHED_IDLE : SCHED_OTHER,
> );
> +#endif
> 
> I noticed that when I compile this with gcc 5.4.0, I get a compilation
> error about SCHED_IDLE not being defined.
> Indeed, if I look in sched.h used with gcc 5.4.0, these are the SCHED_*
> definitions:
>
Sorry for the breakage here!

sched.h comes from glibc, rather than GCC.
SCHED_IDLE was added in glibc 2.12, which is considerably older than any
supported compiler (2010, vs 2013 for gcc 4.8 and 2016 for gcc 5.4).

I don't think there's a documented policy on libc versions... what does
this system look like? (e.g. glibc version, distribution?)

(It seems like it would be OK to #ifdef SCHED_IDLE here and do nothing the
libc is just too old, but it may mask other problems we'd want to know
about).

```
> /* Scheduling algorithms.  */
> #define SCHED_OTHER 0
> #define SCHED_FIFO  1
> #define SCHED_RR2
> #ifdef __USE_GNU
> # define SCHED_BATCH3
> #endif
> ```
> I suppose SCHED_IDLE was added in some later version...
>
> On
>
> https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library
> it says that gcc 4.8 should work, but it doesn't now then?
>
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D53651
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits