[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Is there a test missing here?


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

https://reviews.llvm.org/D58743



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExprObjC.cpp:534
+NSStringPointer, NSStringPointer);
+return new (Context) ObjCBoxedExpr(SL, BoxedType, nullptr, SR);
+  }

ahatanak wrote:
> rjmccall wrote:
> > You're implicitly dropping sugar from the source expression here; I really 
> > think you should preserve that, and if it means you end up having to look 
> > through array-decay expressions, that's not the end of the world.
> > 
> > The need for a special case here is just to allow us to compile these even 
> > if there isn't a boxing method available?
> > 
> > Do you need to restrict the type of the string literal so that it doesn't 
> > apply to e.g. wide strings?
> Line 516 checks that the pointee type has the same unqualified type as 
> 'char'. If the string literal inside the parentheses is of a wider type 
> (e.g., `@(u"abc")`), this piece of code won't be executed, and a diagnostic 
> is emitted later ("illegal type 'const char16_t *' used in a boxed 
> expression").
> 
> The original motivation for special-casing string literals inside boxed 
> expressions was to silence the `-Wnullable-to-nonnull-conversion` warning 
> mentioned here: https://oleb.net/2018/@keypath/. The warning is issued 
> because the return type of `stringWithUTF8String` is nullable.
> 
> ```
> ...
> + (nullable instancetype)stringWithUTF8String:(const char 
> *)nullTerminatedCString;
> ...
> 
> NSString * _Nonnull ptr = @("abc");  // expected-error {{implicit conversion 
> from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 
> 'NSString * _Nonnull'}}
> ```
I see.  So in addition to guaranteeing to skip the boxing call, you'd like to 
adjust the type to report the result as non-null, which we can do because we're 
unconditionally making a constant string.  Makes sense.

However, I think you need to check that the string is valid UTF-8 or else this 
is semantics-changing, since such strings are rejected by 
`+stringWithUTF8String:` (which is why it returns a nullable pointer at all).  
It should be alright to warn about strings that are statically known to be 
invalid UTF-8, but you'll then need to emit them using the normal boxing method.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Heejin Ahn via Phabricator via cfe-commits]

aheejin added inline comments.



Comment at: clang/include/clang/Driver/ToolChain.h:456
   /// getThreadModel() - Which thread model does this target use?
-  virtual std::string getThreadModel(const llvm::opt::ArgList &) const {
-return "posix";
-  }
+  virtual std::string getThreadModel() const { return "posix"; }
 

tlively wrote:
> aheejin wrote:
> > I think now we can delete this overridden method, because the default value 
> > for this is "posix". See Dan's [[ 
> > https://github.com/llvm/llvm-project/commit/1384ee936e46816f348bc3756704aeaff92e86fe
> >  | commit ]].
> I did delete the overridden method below in 
> clang/lib/Driver/ToolChains/WebAssembly.cpp. This is restoring the base class 
> implementation to the way it was before your CL added the ArgList argument, 
> since we no longer need it.
Oh sorry, I thought this file was WebAssembly.cpp... 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58757: Add a version of the pass_object_size attribute that works with builtin_dynamic_object_size

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: aaron.ballman, george.burgess.iv, rsmith.
Herald added subscribers: jdoerfert, kristina, dexonsmith, jkorous.
Herald added a project: clang.

This attribute, named `pass_dynamic_object_size` has the same semantics as 
pass_object_size, except that it calls `__builtin_dynamic_object_size` at the 
caller instead of `__builtin_object_size`. You can read more about 
`__builtin_dynamic_object_size` here: 
https://clang.llvm.org/docs/LanguageExtensions.html#evaluating-object-size-dynamically,
 it was introduced in D56760 .

rdar://48208787

Thanks for taking a look!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D58757

Files:
  clang/include/clang/AST/Attr.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/pass-object-size.c
  clang/test/CodeGenCXX/mangle-ms.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/Sema/pass-object-size.c

Index: clang/test/Sema/pass-object-size.c
===
--- clang/test/Sema/pass-object-size.c
+++ clang/test/Sema/pass-object-size.c
@@ -17,6 +17,9 @@
 void i(char *p __attribute__((pass_object_size(0; // OK -- const is only necessary on definitions, not decls.
 void j(char *p __attribute__((pass_object_size(0), pass_object_size(1; //expected-error{{'pass_object_size' attribute can only be applied once per parameter}}
 
+void k(char *p __attribute__((pass_dynamic_object_size))); // expected-error {{'pass_dynamic_object_size' attribute takes one argument}}
+void l(int p __attribute__((pass_dynamic_object_size(0; // expected-error {{'pass_dynamic_object_size' attribute only applies to constant pointer arguments}}
+
 #define PS(N) __attribute__((pass_object_size(N)))
 #define overloaded __attribute__((overloadable))
 void Overloaded(void *p PS(0)) overloaded; //expected-note{{previous declaration is here}}
@@ -32,14 +35,17 @@
 void TakeFnOvl(void (*)(int *)) overloaded;
 
 void NotOverloaded(void *p PS(0));
-void IsOverloaded(void *p PS(0)) overloaded;
-void IsOverloaded(char *p) overloaded; // char* inestead of void* is intentional
+void IsOverloaded(void *p PS(0)) overloaded; // expected-note 2 {{candidate address cannot be taken because parameter 1 has pass_object_size attribute}}
+
+// char* inestead of void* is intentional
+void IsOverloaded(char *p) overloaded; // expected-note{{passing argument to parameter 'p' here}} expected-note 2 {{type mismatch}}
+
 void FunctionPtrs() {
   void (*p)(void *) = NotOverloaded; //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}}
   void (*p2)(void *) = &NotOverloaded; //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}}
 
-  void (*p3)(void *) = IsOverloaded; //expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type ''}} expected-note@-6{{candidate address cannot be taken because parameter 1 has pass_object_size attribute}} expected-note@-5{{type mismatch}}
-  void (*p4)(void *) = &IsOverloaded; //expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type ''}} expected-note@-7{{candidate address cannot be taken because parameter 1 has pass_object_size attribute}} expected-note@-6{{type mismatch}}
+  void (*p3)(void *) = IsOverloaded; //expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type ''}}
+  void (*p4)(void *) = &IsOverloaded; //expected-warning{{incompatible pointer types initializing 'void (*)(void *)' with an expression of type ''}}
 
   void (*p5)(char *) = IsOverloaded;
   void (*p6)(char *) = &IsOverloaded;
@@ -52,5 +58,11 @@
 
   int P;
   (&NotOverloaded)(&P); //expected-error{{cannot take address of function 'NotOverloaded' because parameter 1 has pass_object_size attribute}}
-  (&IsOverloaded)(&P); //expected-warning{{incompatible pointer types passing 'int *' to parameter of type 'char *'}} expected-note@36{{passing argument to parameter 'p' here}}
+  (&IsOverloaded)(&P); //expected-warning{{incompatible pointer types passing 'int *' to parameter of type 'char *'}}
 }
+
+void mismatch(void *p __attribute__((pass_object_size(0; // expected-note {{previous declaration is here}}
+void mismatch(void *p __attribute__((pass_dynamic_object_size(0; // expected-error {{conflicti

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaExprObjC.cpp:534
+NSStringPointer, NSStringPointer);
+return new (Context) ObjCBoxedExpr(SL, BoxedType, nullptr, SR);
+  }

rjmccall wrote:
> You're implicitly dropping sugar from the source expression here; I really 
> think you should preserve that, and if it means you end up having to look 
> through array-decay expressions, that's not the end of the world.
> 
> The need for a special case here is just to allow us to compile these even if 
> there isn't a boxing method available?
> 
> Do you need to restrict the type of the string literal so that it doesn't 
> apply to e.g. wide strings?
Line 516 checks that the pointee type has the same unqualified type as 'char'. 
If the string literal inside the parentheses is of a wider type (e.g., 
`@(u"abc")`), this piece of code won't be executed, and a diagnostic is emitted 
later ("illegal type 'const char16_t *' used in a boxed expression").

The original motivation for special-casing string literals inside boxed 
expressions was to silence the `-Wnullable-to-nonnull-conversion` warning 
mentioned here: https://oleb.net/2018/@keypath/. The warning is issued because 
the return type of `stringWithUTF8String` is nullable.

```
...
+ (nullable instancetype)stringWithUTF8String:(const char 
*)nullTerminatedCString;
...

NSString * _Nonnull ptr = @("abc");  // expected-error {{implicit conversion 
from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 
'NSString * _Nonnull'}}
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Thomas Lively via Phabricator via cfe-commits]

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

In D58742#1413077 , @sunfish wrote:

> This is still a little confusing to me. -matomic is supposed to be a 
> subtarget flag, stating that the wasm implementation we will run on supports 
> atomic instructions. -mthread-model posix is about the C++ interpretation -- 
> what style implementation of memory model do we want? In the future, -matomic 
> may become enabled by default, when enough wasm engines generally support it. 
> However, -mthread-model single/posix may still be useful to control 
> independently, because even with wasm engines supporting atomic, there are 
> reasons users might still want to compile their apps single-threaded: access 
> to linear memory with no declared max, lower overall code size, or other 
> things.


The only backend besides us that uses the ThreadModel at all is ARM, so we were 
questioning its generality. I originally shared your opinion that the available 
target features should be controlled separately from whether or not the user 
wants threads, but in reality we get to use the atomics feature if and only if 
the user opts in to threads. Because of this, ignoring the thread model 
entirely like most other backends lets us have a much simpler user interface 
that is not invalid by default.

I expect with this model we would never turn on atomics by default (since we 
don't want multithreaded by default), and most users would get them by using 
-pthread. This is very close in UI to native targets. The -matomics option is 
only there for users who are doing weird threaded things but don't want 
libpthread for some reason.




Comment at: clang/include/clang/Driver/ToolChain.h:456
   /// getThreadModel() - Which thread model does this target use?
-  virtual std::string getThreadModel(const llvm::opt::ArgList &) const {
-return "posix";
-  }
+  virtual std::string getThreadModel() const { return "posix"; }
 

aheejin wrote:
> I think now we can delete this overridden method, because the default value 
> for this is "posix". See Dan's [[ 
> https://github.com/llvm/llvm-project/commit/1384ee936e46816f348bc3756704aeaff92e86fe
>  | commit ]].
I did delete the overridden method below in 
clang/lib/Driver/ToolChains/WebAssembly.cpp. This is restoring the base class 
implementation to the way it was before your CL added the ArgList argument, 
since we no longer need it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: lib/Sema/SemaDecl.cpp:5971
 auto *VD = dyn_cast(&ND);
-if (!ND.isExternallyVisible() || (VD && VD->isStaticLocal())) {
+NamespaceDecl *NS = NULL;
+if (VD)

s/NULL/nullptr

Also, I think this should be `const NamespaceDecl *`.



Comment at: lib/Sema/SemaDecl.cpp:5973
+if (VD)
+  NS = dyn_cast(VD->getDeclContext());
+int isAnonymousNS = NS && NS->getDeclName().isEmpty();

Does only the immediate declaration context matter? How is this handled?
```
namespace {
namespace named {
  __declspec(dllexport) int const x = 3; 
}
}
```



Comment at: lib/Sema/SemaDecl.cpp:5975-5977
+if ((!ND.isExternallyVisible() &&
+  (!isAnonymousNS || !(VD && VD->hasInit( ||
+  (VD && VD->isStaticLocal())) {

This used to unconditionally warn, but now has predicates -- should this still 
warn when not in MSVC mode?


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

https://reviews.llvm.org/D45978



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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Dan Gohman via Phabricator via cfe-commits]

sunfish added a comment.

This is still a little confusing to me. -matomic is supposed to be a subtarget 
flag, stating that the wasm implementation we will run on supports atomic 
instructions. -mthread-model posix is about the C++ interpretation -- what 
style implementation of memory model do we want? In the future, -matomic may 
become enabled by default, when enough wasm engines generally support it. 
However, -mthread-model single/posix may still be useful to control 
independently, because even with wasm engines supporting atomic, there are 
reasons users might still want to compile their apps single-threaded: access to 
linear memory with no declared max, lower overall code size, or other things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

@xbolva00 I prefer to keep it here for now. I suggest we wait with upstreaming 
until an actual need for use in some other project arises. This is what we 
basically did with VFS.


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

https://reviews.llvm.org/D58418



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

[PATCH] D58751: Order File Instrumentation: add clang support for -forder-file-instrumentation

[Manman Ren via Phabricator via cfe-commits]

manmanren created this revision.
manmanren added reviewers: davidxl, beanz, dexonsmith.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

When -forder-file-instrumentation is on, we pass llvm flag to enable the order 
file instrumentation pass.


Repository:
  rC Clang

https://reviews.llvm.org/D58751

Files:
  include/clang/Driver/Options.td
  lib/Driver/ToolChain.cpp
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/clang_f_opts.c


Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -121,6 +121,7 @@
 // RUN: %clang -### -S -fcoverage-mapping -fno-coverage-mapping %s 2>&1 | 
FileCheck -check-prefix=CHECK-DISABLE-COVERAGE %s
 // RUN: %clang -### -S -fprofile-instr-generate -fcoverage-mapping 
-fno-coverage-mapping %s 2>&1 | FileCheck -check-prefix=CHECK-DISABLE-COVERAGE 
%s
 // RUN: %clang -### -S -fprofile-remapping-file foo/bar.txt %s 2>&1 | 
FileCheck -check-prefix=CHECK-PROFILE-REMAP %s
+// RUN: %clang -### -S -forder-file-instrumentation %s 2>&1 | FileCheck 
-check-prefix=CHECK-ORDERFILE-INSTR %s
 // CHECK-PROFILE-GENERATE: "-fprofile-instrument=clang"
 // CHECK-PROFILE-GENERATE-LLVM: "-fprofile-instrument=llvm"
 // CHECK-PROFILE-GENERATE-DIR: 
"-fprofile-instrument-path=/some/dir{{/|}}{{.*}}"
@@ -132,6 +133,8 @@
 // CHECK-COVERAGE-AND-GEN: '-fcoverage-mapping' only allowed with 
'-fprofile-instr-generate'
 // CHECK-DISABLE-COVERAGE-NOT: "-fcoverage-mapping"
 // CHECK-PROFILE-REMAP: "-fprofile-remapping-file=foo/bar.txt"
+// CHECK-ORDERFILE-INSTR: "-forder-file-instrumentation"
+// CHECK-ORDERFILE-INSTR: "-enable-order-file-instrumentation"
 
 // RUN: %clang -### -S -fprofile-use %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-USE %s
 // RUN: %clang -### -S -fprofile-instr-use %s 2>&1 | FileCheck 
-check-prefix=CHECK-PROFILE-USE %s
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -5307,6 +5307,17 @@
 }
   }
 
+  if (Args.hasArg(options::OPT_forder_file_instrumentation)) {
+ CmdArgs.push_back("-forder-file-instrumentation");
+ // Enable order file instrumentation when ThinLTO is not on. When ThinLTO 
is
+ // on, we need to pass these flags as linker flags and that will be 
handled
+ // outside of the compiler.
+ if (D.getLTOMode() != LTOK_Thin) {
+   CmdArgs.push_back("-mllvm");
+   CmdArgs.push_back("-enable-order-file-instrumentation");
+ }
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_fforce_enable_int128,
options::OPT_fno_force_enable_int128)) {
 if (A->getOption().matches(options::OPT_fforce_enable_int128))
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -407,7 +407,8 @@
   Args.hasArg(options::OPT_fprofile_generate_EQ) ||
   Args.hasArg(options::OPT_fprofile_instr_generate) ||
   Args.hasArg(options::OPT_fprofile_instr_generate_EQ) ||
-  Args.hasArg(options::OPT_fcreate_profile))
+  Args.hasArg(options::OPT_fcreate_profile) ||
+  Args.hasArg(options::OPT_forder_file_instrumentation)) 
 return true;
 
   return false;
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -769,6 +769,9 @@
 def fprofile_exclude_files_EQ : Joined<["-"], "fprofile-exclude-files=">,
 Group, Flags<[CC1Option, CoreOption]>,
 HelpText<"Instrument only functions from files where names don't match all 
the regexes separated by a semi-colon">;
+def forder_file_instrumentation : Flag<["-"], "forder-file-instrumentation">,
+Group, Flags<[CoreOption]>,
+HelpText<"Generate instrumented code to collect order file into 
default.profraw file (overridden by '=' form of option or LLVM_PROFILE_FILE env 
var)">;
 
 def faddrsig : Flag<["-"], "faddrsig">, Group, Flags<[CoreOption, 
CC1Option]>,
   HelpText<"Emit an address-significance table">;


Index: test/Driver/clang_f_opts.c
===
--- test/Driver/clang_f_opts.c
+++ test/Driver/clang_f_opts.c
@@ -121,6 +121,7 @@
 // RUN: %clang -### -S -fcoverage-mapping -fno-coverage-mapping %s 2>&1 | FileCheck -check-prefix=CHECK-DISABLE-COVERAGE %s
 // RUN: %clang -### -S -fprofile-instr-generate -fcoverage-mapping -fno-coverage-mapping %s 2>&1 | FileCheck -check-prefix=CHECK-DISABLE-COVERAGE %s
 // RUN: %clang -### -S -fprofile-remapping-file foo/bar.txt %s 2>&1 | FileCheck -check-prefix=CHECK-PROFILE-REMAP %s
+// RUN: %clang -### -S -forder-file-instrumentation %s 2>&1 | FileCheck -check-prefix=CHECK-ORDERFILE-INSTR %s
 // CHECK-PROFILE-GENERATE: "-fprofile-instrument=clang"
 // CHECK-PROFILE-GENERA

r355061 - [clang][index-while-building][NFC] Comment about implementation detail in FileIndexRecord

[Jan Korous via cfe-commits]

Author: jkorous
Date: Wed Feb 27 17:12:27 2019
New Revision: 355061

URL: http://llvm.org/viewvc/llvm-project?rev=355061&view=rev
Log:
[clang][index-while-building][NFC] Comment about implementation detail in 
FileIndexRecord

Modified:
cfe/trunk/lib/Index/FileIndexRecord.cpp

Modified: cfe/trunk/lib/Index/FileIndexRecord.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/FileIndexRecord.cpp?rev=355061&r1=355060&r2=355061&view=diff
==
--- cfe/trunk/lib/Index/FileIndexRecord.cpp (original)
+++ cfe/trunk/lib/Index/FileIndexRecord.cpp Wed Feb 27 17:12:27 2019
@@ -35,6 +35,7 @@ void FileIndexRecord::addDeclOccurence(S
   }
 
   DeclOccurrence NewInfo(Roles, Offset, D, Relations);
+  // We keep Decls in order as we need to access them in this order in all 
cases.
   auto It = std::upper_bound(Decls.begin(), Decls.end(), NewInfo);
   Decls.insert(It, std::move(NewInfo));
 }


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

r355058 - Ensure that set constrained asm operands are not affected by truncation.

[Joerg Sonnenberger via cfe-commits]

Author: joerg
Date: Wed Feb 27 16:55:09 2019
New Revision: 355058

URL: http://llvm.org/viewvc/llvm-project?rev=355058&view=rev
Log:
Ensure that set constrained asm operands are not affected by truncation.

Modified:
cfe/trunk/include/clang/Basic/TargetInfo.h
cfe/trunk/test/Sema/inline-asm-validate-x86.c

Modified: cfe/trunk/include/clang/Basic/TargetInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=355058&r1=355057&r2=355058&view=diff
==
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Wed Feb 27 16:55:09 2019
@@ -857,8 +857,10 @@ public:
 }
 bool isValidAsmImmediate(const llvm::APInt &Value) const {
   if (!ImmSet.empty())
-return ImmSet.count(Value.getZExtValue()) != 0;
-  return !ImmRange.isConstrained || (Value.sge(ImmRange.Min) && 
Value.sle(ImmRange.Max));
+return Value.isSignedIntN(32) &&
+   ImmSet.count(Value.getZExtValue()) != 0;
+  return !ImmRange.isConstrained ||
+ (Value.sge(ImmRange.Min) && Value.sle(ImmRange.Max));
 }
 
 void setIsReadWrite() { Flags |= CI_ReadWrite; }

Modified: cfe/trunk/test/Sema/inline-asm-validate-x86.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/inline-asm-validate-x86.c?rev=355058&r1=355057&r2=355058&view=diff
==
--- cfe/trunk/test/Sema/inline-asm-validate-x86.c (original)
+++ cfe/trunk/test/Sema/inline-asm-validate-x86.c Wed Feb 27 16:55:09 2019
@@ -56,6 +56,7 @@ void L(int i, int j) {
   static const int Invalid1 = 1;
   static const int Invalid2 = 42;
   static const int Invalid3 = 0;
+  static const long long Invalid4 = 0x100ff;
   static const int Valid1 = 0xff;
   static const int Valid2 = 0x;
   static const int Valid3 = 0x;
@@ -73,6 +74,9 @@ void L(int i, int j) {
   : "0"(i), "L"(Invalid3)); // expected-error{{value '0' out of range 
for constraint 'L'}}
   __asm__("xorl %0,%2"
   : "=r"(i)
+  : "0"(i), "L"(Invalid4)); // expected-error{{value '4294967551' out 
of range for constraint 'L'}}
+  __asm__("xorl %0,%2"
+  : "=r"(i)
   : "0"(i), "L"(Valid1)); // expected-no-error
   __asm__("xorl %0,%2"
   : "=r"(i)


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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Heejin Ahn via Phabricator via cfe-commits]

aheejin accepted this revision.
aheejin added a comment.

This looks a nice improvement! Thank you.




Comment at: clang/include/clang/Driver/ToolChain.h:456
   /// getThreadModel() - Which thread model does this target use?
-  virtual std::string getThreadModel(const llvm::opt::ArgList &) const {
-return "posix";
-  }
+  virtual std::string getThreadModel() const { return "posix"; }
 

I think now we can delete this overridden method, because the default value for 
this is "posix". See Dan's [[ 
https://github.com/llvm/llvm-project/commit/1384ee936e46816f348bc3756704aeaff92e86fe
 | commit ]].



Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:135
+  << "-pthread"
+  << "-mno-atomics";
+if (DriverArgs.hasFlag(options::OPT_mno_bulk_memory,

This is nicer! Given that the default values for both options were false, we 
didn't need to care whether a user explicitly gave it or not after all.



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp:150
? FSAttr.getValueAsString().str()
: TargetFS;
 

Aha, this is the info.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355054: [CodeGen] Fix some broken IR generated by 
-fsanitize=unsigned-integer-overflow (authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58744?vs=188645&id=188648#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58744

Files:
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c


Index: cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
===
--- cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
+++ cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 
-fsanitize=unsigned-integer-overflow %s -emit-llvm -o - | FileCheck %s
+
+_Atomic(unsigned) atomic;
+
+// CHECK-LABEL: define void @cmpd_assign
+void cmpd_assign() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic += 1;
+}
+
+// CHECK-LABEL: define void @inc
+void inc() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic++;
+}
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -2555,14 +2555,14 @@
   }
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LV, RValue::get(atomicPHI), RValue::get(value), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), type);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicPHI->getParent());
 Builder.SetInsertPoint(contBB);
 return isPre ? value : input;
   }
@@ -2909,14 +2909,14 @@
 Loc, ScalarConversionOpts(CGF.SanOpts));
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LHSLV, RValue::get(atomicPHI), RValue::get(Result), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), LHSTy);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicPHI->getParent());
 Builder.SetInsertPoint(contBB);
 return LHSLV;
   }


Index: cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
===
--- cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
+++ cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsanitize=unsigned-integer-overflow %s -emit-llvm -o - | FileCheck %s
+
+_Atomic(unsigned) atomic;
+
+// CHECK-LABEL: define void @cmpd_assign
+void cmpd_assign() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic += 1;
+}
+
+// CHECK-LABEL: define void @inc
+void inc() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic++;
+}
Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cp

r355054 - [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Wed Feb 27 16:47:55 2019
New Revision: 355054

URL: http://llvm.org/viewvc/llvm-project?rev=355054&view=rev
Log:
[CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

I think the author of the function assumed that `GetInsertBlock()`
wouldn't change from where `atomicPHI` was created, but this isn't
true when `-fsanitize=unsigned-integer-overflow` is enabled (we
generate an overflow/continuation label). Fix by keeping track of the
block we want to return to to complete the cmpxchg loop.

rdar://48406558

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

Added:
cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=355054&r1=355053&r2=355054&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Wed Feb 27 16:47:55 2019
@@ -2555,14 +2555,14 @@ ScalarExprEmitter::EmitScalarPrePostIncD
   }
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LV, RValue::get(atomicPHI), RValue::get(value), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), type);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicPHI->getParent());
 Builder.SetInsertPoint(contBB);
 return isPre ? value : input;
   }
@@ -2909,14 +2909,14 @@ LValue ScalarExprEmitter::EmitCompoundAs
 Loc, ScalarConversionOpts(CGF.SanOpts));
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LHSLV, RValue::get(atomicPHI), RValue::get(Result), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), LHSTy);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicPHI->getParent());
 Builder.SetInsertPoint(contBB);
 return LHSLV;
   }

Added: cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c?rev=355054&view=auto
==
--- cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c (added)
+++ cfe/trunk/test/CodeGen/sanitize-atomic-int-overflow.c Wed Feb 27 16:47:55 
2019
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 
-fsanitize=unsigned-integer-overflow %s -emit-llvm -o - | FileCheck %s
+
+_Atomic(unsigned) atomic;
+
+// CHECK-LABEL: define void @cmpd_assign
+void cmpd_assign() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic += 1;
+}
+
+// CHECK-LABEL: define void @inc
+void inc() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic++;
+}


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

[PATCH] D58749: [index-while-building] IndexRecordHasher

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: nathawes, akyrtzi, arphaman, dexonsmith, ioeric, 
malaperle.
Herald added subscribers: cfe-commits, jdoerfert, mgorny.
Herald added a project: clang.

Another piece of index-while-building functionality.
RFC: http://lists.llvm.org/pipermail/cfe-dev/2019-February/061432.html

Originally part of review https://reviews.llvm.org/D39050

This implementation is covered by lit tests using it through c-index-test in 
upcoming patch. It's just split off to make the patch smaller and easier to 
review.


Repository:
  rC Clang

https://reviews.llvm.org/D58749

Files:
  clang/lib/Index/CMakeLists.txt
  clang/lib/Index/IndexRecordHasher.cpp
  clang/lib/Index/IndexRecordHasher.h

Index: clang/lib/Index/IndexRecordHasher.h
===
--- /dev/null
+++ clang/lib/Index/IndexRecordHasher.h
@@ -0,0 +1,61 @@
+//===--- IndexRecordHasher.h - Index record hashing -*- C++ -*-===//
+
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_INDEX_INDEXRECORDHASHER_H
+#define LLVM_CLANG_LIB_INDEX_INDEXRECORDHASHER_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/Hashing.h"
+
+namespace clang {
+  class ASTContext;
+  class Decl;
+  class DeclarationName;
+  class NestedNameSpecifier;
+  class QualType;
+  class Type;
+  template  class CanQual;
+  typedef CanQual CanQualType;
+
+namespace index {
+  class FileIndexRecord;
+
+/// Implements hashing of AST nodes suitable for the index.
+/// Caching all produced hashes.
+class IndexRecordHasher {
+  ASTContext &Ctx;
+  llvm::DenseMap HashByPtr;
+
+public:
+  explicit IndexRecordHasher(ASTContext &Ctx) : Ctx(Ctx) {}
+  ASTContext &getASTContext() { return Ctx; }
+
+  /// Returns hash for all declaration occurences in \c Record.
+  llvm::hash_code hashRecord(const FileIndexRecord &Record);
+  llvm::hash_code hash(const Decl *D);
+  llvm::hash_code hash(QualType Ty);
+  llvm::hash_code hash(CanQualType Ty);
+  llvm::hash_code hash(DeclarationName Name);
+  llvm::hash_code hash(const NestedNameSpecifier *NNS);
+
+private:
+  template 
+  llvm::hash_code tryCache(const void *Ptr, T Obj);
+
+  llvm::hash_code hashImpl(const Decl *D);
+  llvm::hash_code hashImpl(CanQualType Ty);
+  llvm::hash_code hashImpl(DeclarationName Name);
+  llvm::hash_code hashImpl(const NestedNameSpecifier *NNS);
+};
+
+} // end namespace index
+} // end namespace clang
+
+#endif
Index: clang/lib/Index/IndexRecordHasher.cpp
===
--- /dev/null
+++ clang/lib/Index/IndexRecordHasher.cpp
@@ -0,0 +1,484 @@
+//===--- IndexRecordHasher.cpp - Index record hashing ---*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "IndexRecordHasher.h"
+#include "FileIndexRecord.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclVisitor.h"
+#include "llvm/Support/Path.h"
+
+constexpr size_t InitialHash = 5381;
+
+using namespace clang;
+using namespace clang::index;
+using namespace llvm;
+
+static hash_code computeHash(const TemplateArgument &Arg,
+ IndexRecordHasher &Hasher);
+
+namespace {
+class DeclHashVisitor : public ConstDeclVisitor {
+  IndexRecordHasher &Hasher;
+
+public:
+  DeclHashVisitor(IndexRecordHasher &Hasher) : Hasher(Hasher) {}
+
+  hash_code VisitDecl(const Decl *D) {
+return VisitDeclContext(D->getDeclContext());
+  }
+
+  hash_code VisitNamedDecl(const NamedDecl *D) {
+hash_code Hash = VisitDecl(D);
+if (auto *attr = D->getExternalSourceSymbolAttr()) {
+  Hash = hash_combine(Hash, hash_value(attr->getDefinedIn()));
+}
+return hash_combine(Hash, Hasher.hash(D->getDeclName()));
+  }
+
+  hash_code VisitTagDecl(const TagDecl *D) {
+if (D->getDeclName().isEmpty()) {
+  if (const TypedefNameDecl *TD = D->getTypedefNameForAnonDecl())
+return Visit(TD);
+
+  hash_code Hash = VisitDeclContext(D->getDeclContext());
+  if (D->isEmbeddedInDeclarator() && !D->isFreeStanding()) {
+Hash = hash_combine(Hash,
+hashLoc(D->getLocation(), /*IncludeOffset=*/true));
+  } else
+Hash = hash_combine(Hash, 'a');
+  return Hash;
+}
+
+hash_code Hash = VisitTypeDecl(D);
+return hash_combine(Hash, 'T');
+  }
+
+  hash_code VisitClassTemplateSpecializationDecl(const ClassTemplateSpecializationDecl *D) {
+hash_

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Akira Hatanaka via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D58744



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

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 188645.
erik.pilkington added a comment.

Use `atomicPHI->getParent()` instead of tracking the block.


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

https://reviews.llvm.org/D58744

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/sanitize-atomic-int-overflow.c


Index: clang/test/CodeGen/sanitize-atomic-int-overflow.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 
-fsanitize=unsigned-integer-overflow %s -emit-llvm -o - | FileCheck %s
+
+_Atomic(unsigned) atomic;
+
+// CHECK-LABEL: define void @cmpd_assign
+void cmpd_assign() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic += 1;
+}
+
+// CHECK-LABEL: define void @inc
+void inc() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic++;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2555,14 +2555,14 @@
   }
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LV, RValue::get(atomicPHI), RValue::get(value), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), type);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicPHI->getParent());
 Builder.SetInsertPoint(contBB);
 return isPre ? value : input;
   }
@@ -2909,14 +2909,14 @@
 Loc, ScalarConversionOpts(CGF.SanOpts));
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LHSLV, RValue::get(atomicPHI), RValue::get(Result), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), LHSTy);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicPHI->getParent());
 Builder.SetInsertPoint(contBB);
 return LHSLV;
   }


Index: clang/test/CodeGen/sanitize-atomic-int-overflow.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsanitize=unsigned-integer-overflow %s -emit-llvm -o - | FileCheck %s
+
+_Atomic(unsigned) atomic;
+
+// CHECK-LABEL: define void @cmpd_assign
+void cmpd_assign() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic += 1;
+}
+
+// CHECK-LABEL: define void @inc
+void inc() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic++;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2555,14 +2555,14 @@
   }
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LV, RValue::get(atomicPHI), RValu

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2921
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicOpBB);
 Builder.SetInsertPoint(contBB);

ahatanak wrote:
> Would passing `atomicPHI->getParent()` instead of `opBB` to 
> `Builder.CreateCondBr` fix the crash?
Yeah, `atomicOpBB` and `atomicPHI->getParent()` should always be equal here. 
I'll update the patch to use this, since there is one less variable to keep 
track of.


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

https://reviews.llvm.org/D58744



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

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/test/CodeGen/sanitize-atomic-int-overflow.c:3
+
+_Atomic(unsigned) atomic;
+

ahatanak wrote:
> It's probably better to add some check strings here.
I see this has been fixed in the updated patch.


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

https://reviews.llvm.org/D58744



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

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:2921
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicOpBB);
 Builder.SetInsertPoint(contBB);

Would passing `atomicPHI->getParent()` instead of `opBB` to 
`Builder.CreateCondBr` fix the crash?



Comment at: clang/test/CodeGen/sanitize-atomic-int-overflow.c:3
+
+_Atomic(unsigned) atomic;
+

It's probably better to add some check strings here.


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

https://reviews.llvm.org/D58744



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk updated this revision to Diff 188642.
lewmpk added a comment.

cleaned up documentation


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

https://reviews.llvm.org/D58731

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/modernize/UseOverrideCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
  docs/clang-tidy/checks/modernize-use-override.rst
  test/clang-tidy/modernize-use-override-no-destructors.cpp

Index: test/clang-tidy/modernize-use-override-no-destructors.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-no-destructors.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.IgnoreDestructors, value: 1}]}" \
+// RUN: -- -std=c++11
+
+struct Base {
+  virtual ~Base();
+  virtual void f();
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES-NOT: warning:
+  virtual void f();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f() override;
+};
Index: docs/clang-tidy/checks/modernize-use-override.rst
===
--- docs/clang-tidy/checks/modernize-use-override.rst
+++ docs/clang-tidy/checks/modernize-use-override.rst
@@ -3,5 +3,11 @@
 modernize-use-override
 ==
 
-
 Use C++11's ``override`` and remove ``virtual`` where applicable.
+
+Options
+---
+
+.. option:: IgnoreDestructors
+
+   If set to non-zero, this check will not diagnose destructors. Default is `0`.
Index: docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cppcoreguidelines-explicit-virtual-functions
+.. meta::
+   :http-equiv=refresh: 5;URL=cppcoreguidelines-explicit-virtual-functions.html
+
+cppcoreguidelines-explicit-virtual-functions
+
+
+The cppcoreguidelines-explicit-virtual-functions check is an alias, please see
+`modernize-use-override `_
+for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,11 @@
 Improvements to clang-tidy
 --
 
+- New alias :doc:`cppcoreguidelines-explicit-virtual-functions
+  ` to 
+  :doc:`modernize-use-override
+  ` was added.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,14 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -17,9 +17,20 @@
 namespace tidy {
 namespace modernize {
 
+void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+}
+
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
+  if (!getLangOpts().CPlusPlus11)
+return;
+
+  if (IgnoreDestructors)
+Finder->addMatcher(
+cxxMethodDecl(isOverride(), unless(cxxDestructorDecl())).bind("method"),
+this);
+  else
 Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
 }
 
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: docs/clang-tidy/checks/modernize-use-override.rst:5
 ==
 
 

Will be good idea to remove duplicated empty line.



Comment at: docs/clang-tidy/checks/modernize-use-override.rst:14
+
+   If set to non-zero, this check will not diagnose destructors. Default is 
'0'.

Please use ` to highlight value.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington updated this revision to Diff 188641.
erik.pilkington added a comment.

Use FileCheck in the test, NFC.


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

https://reviews.llvm.org/D58744

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/sanitize-atomic-int-overflow.c

Index: clang/test/CodeGen/sanitize-atomic-int-overflow.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsanitize=unsigned-integer-overflow %s -emit-llvm -o - | FileCheck %s
+
+_Atomic(unsigned) atomic;
+
+// CHECK-LABEL: define void @cmpd_assign
+void cmpd_assign() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic += 1;
+}
+
+// CHECK-LABEL: define void @inc
+void inc() {
+  // CHECK: br label %[[LOOP_START:.*]]
+
+  // CHECK: [[LOOP_START]]:
+  // CHECK-NEXT: phi i32 {{.*}}, [ {{.*}}, %[[INCOMING_BLOCK:.*]] ]
+
+  // CHECK: [[INCOMING_BLOCK]]:
+  // CHECK-NEXT: cmpxchg
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: extractvalue
+  // CHECK-NEXT: br i1 %8, label %{{.*}}, label %[[LOOP_START]]
+  atomic++;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2350,6 +2350,7 @@
 
   QualType type = E->getSubExpr()->getType();
   llvm::PHINode *atomicPHI = nullptr;
+  llvm::BasicBlock *atomicOpBB = nullptr;
   llvm::Value *value;
   llvm::Value *input;
 
@@ -2393,10 +2394,10 @@
 input = value;
 // For every other atomic operation, we need to emit a load-op-cmpxchg loop
 llvm::BasicBlock *startBB = Builder.GetInsertBlock();
-llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+atomicOpBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
 value = CGF.EmitToMemory(value, type);
-Builder.CreateBr(opBB);
-Builder.SetInsertPoint(opBB);
+Builder.CreateBr(atomicOpBB);
+Builder.SetInsertPoint(atomicOpBB);
 atomicPHI = Builder.CreatePHI(value->getType(), 2);
 atomicPHI->addIncoming(value, startBB);
 value = atomicPHI;
@@ -2555,14 +2556,14 @@
   }
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LV, RValue::get(atomicPHI), RValue::get(value), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), type);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicOpBB);
 Builder.SetInsertPoint(contBB);
 return isPre ? value : input;
   }
@@ -2838,6 +2839,7 @@
   LValue LHSLV = EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
 
   llvm::PHINode *atomicPHI = nullptr;
+  llvm::BasicBlock *atomicOpBB = nullptr;
   if (const AtomicType *atomicTy = LHSTy->getAs()) {
 QualType type = atomicTy->getValueType();
 if (!type->isBooleanType() && type->isIntegerType() &&
@@ -2884,11 +2886,11 @@
 // FIXME: For floating point types, we should be saving and restoring the
 // floating point environment in the loop.
 llvm::BasicBlock *startBB = Builder.GetInsertBlock();
-llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+atomicOpBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
 OpInfo.LHS = EmitLoadOfLValue(LHSLV, E->getExprLoc());
 OpInfo.LHS = CGF.EmitToMemory(OpInfo.LHS, type);
-Builder.CreateBr(opBB);
-Builder.SetInsertPoint(opBB);
+Builder.CreateBr(atomicOpBB);
+Builder.SetInsertPoint(atomicOpBB);
 atomicPHI = Builder.CreatePHI(OpInfo.LHS->getType(), 2);
 atomicPHI->addIncoming(OpInfo.LHS, startBB);
 OpInfo.LHS = atomicPHI;
@@ -2909,14 +2911,14 @@
 Loc, ScalarConversionOpts(CGF.SanOpts));
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LHSLV, RValue::get(atomicPHI), RValue::get(Result), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), LHSTy);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

[Raphael Isemann via Phabricator via cfe-commits]

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

I think the idea of the patch is right. Not sure tough if having Import take a 
second parameter is consistent with the other Import functions (and if that 
even matters).




Comment at: include/clang/AST/ASTImporter.h:340
 /// context, or the import error.
 llvm::Expected Import_New(FileID);
 // FIXME: Remove this version.

Import_New should probably also get the new paramater.



Comment at: include/clang/AST/ASTImporter.h:342
 // FIXME: Remove this version.
-FileID Import(FileID);
+FileID Import(FileID, bool isBuiltin=false);
 

`IsBuiltin`, not `isBuiltin`


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

https://reviews.llvm.org/D58743



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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Thomas Lively via Phabricator via cfe-commits]

tlively added a comment.

I'll wait and see if @aheejin has any concerns before landing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Thomas Lively via Phabricator via cfe-commits]

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



Comment at: llvm/test/CodeGen/WebAssembly/atomic-mem-consistency.ll:1
-; RUN: not llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt
 ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt 
-wasm-disable-explicit-locals -wasm-keep-registers -mattr=+atomics,+sign-ext | 
FileCheck %s

sbc100 wrote:
> What did this start passing?   Is it because llc now has the correct default 
> and will lower atomics to non-atomics?  I guess there is no longer a way to 
> make this fail in the way that it did before.. which is good?
Precisely :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk updated this revision to Diff 188638.
lewmpk added a comment.

- fixed tests
- fixed typo in documentation




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

https://reviews.llvm.org/D58731

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/modernize/UseOverrideCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
  docs/clang-tidy/checks/modernize-use-override.rst
  test/clang-tidy/modernize-use-override-no-destructors.cpp

Index: test/clang-tidy/modernize-use-override-no-destructors.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-no-destructors.cpp
@@ -0,0 +1,16 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.IgnoreDestructors, value: 1}]}" \
+// RUN: -- -std=c++11
+
+struct Base {
+  virtual ~Base();
+  virtual void f();
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES-NOT: warning:
+  virtual void f();
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: prefer using
+  // CHECK-FIXES: {{^}}  void f() override;
+};
Index: docs/clang-tidy/checks/modernize-use-override.rst
===
--- docs/clang-tidy/checks/modernize-use-override.rst
+++ docs/clang-tidy/checks/modernize-use-override.rst
@@ -5,3 +5,10 @@
 
 
 Use C++11's ``override`` and remove ``virtual`` where applicable.
+
+Options
+---
+
+.. option:: IgnoreDestructors
+
+   If set to non-zero, this check will not diagnose destructors. Default is '0'.
Index: docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cppcoreguidelines-explicit-virtual-functions
+.. meta::
+   :http-equiv=refresh: 5;URL=cppcoreguidelines-explicit-virtual-functions.html
+
+cppcoreguidelines-explicit-virtual-functions
+
+
+The cppcoreguidelines-explicit-virtual-functions check is an alias, please see
+`modernize-use-override `_
+for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,11 @@
 Improvements to clang-tidy
 --
 
+- New alias :doc:`cppcoreguidelines-explicit-virtual-functions
+  ` to 
+  :doc:`modernize-use-override
+  ` was added.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,14 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -17,9 +17,20 @@
 namespace tidy {
 namespace modernize {
 
+void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+}
+
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
+  if (!getLangOpts().CPlusPlus11)
+return;
+
+  if (IgnoreDestructors)
+Finder->addMatcher(
+cxxMethodDecl(isOverride(), unless(cxxDestructorDecl())).bind("method"),
+this);
+  else
 Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
 }
 
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlo

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355044: [InstrProf] Use separate comdat group for data and 
counters (authored by rnk, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D58737?vs=188635&id=188637#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58737

Files:
  cfe/trunk/test/Profile/cxx-templates.cpp
  compiler-rt/trunk/test/profile/coverage-inline.cpp
  llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/trunk/test/Instrumentation/InstrProfiling/PR23499.ll
  llvm/trunk/test/Instrumentation/InstrProfiling/comdat.ll

Index: llvm/trunk/test/Instrumentation/InstrProfiling/PR23499.ll
===
--- llvm/trunk/test/Instrumentation/InstrProfiling/PR23499.ll
+++ llvm/trunk/test/Instrumentation/InstrProfiling/PR23499.ll
@@ -20,8 +20,8 @@
 
 
 ; COFF-NOT: __profn__Z3barIvEvv
-; COFF: @__profc__Z3barIvEvv = internal global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat($_Z3barIvEvv), align 8
-; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($_Z3barIvEvv), align 8
+; COFF: @__profc__Z3barIvEvv = linkonce_odr dso_local global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat, align 8
+; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($__profc__Z3barIvEvv), align 8
 
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1
Index: llvm/trunk/test/Instrumentation/InstrProfiling/comdat.ll
===
--- llvm/trunk/test/Instrumentation/InstrProfiling/comdat.ll
+++ llvm/trunk/test/Instrumentation/InstrProfiling/comdat.ll
@@ -17,8 +17,8 @@
 
 ; ELF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat($__profv_foo_inline), align 8
 ; ELF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat($__profv_foo_inline), align 8
-; COFF: @__profc_foo_inline = internal global{{.*}}, section ".lprfc$M", comdat($foo_inline), align 8
-; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($foo_inline), align 8
+; COFF: @__profc_foo_inline = linkonce_odr dso_local global{{.*}}, section ".lprfc$M", comdat, align 8
+; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($__profc_foo_inline), align 8
 define weak_odr void @foo_inline() comdat {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
Index: llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/trunk/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -743,30 +743,23 @@
 
   // Move the name variable to the right section. Place them in a COMDAT group
   // if the associated function is a COMDAT. This will make sure that only one
-  // copy of counters of the COMDAT function will be emitted after linking.
+  // copy of counters of the COMDAT function will be emitted after linking. Keep
+  // in mind that this pass may run before the inliner, so we need to create a
+  // new comdat group for the counters and profiling data. If we use the comdat
+  // of the parent function, that will result in relocations against discarded
+  // sections.
   Comdat *Cmdt = nullptr;
   GlobalValue::LinkageTypes CounterLinkage = Linkage;
   if (needsComdatForCounter(*Fn, *M)) {
+StringRef CmdtPrefix = getInstrProfComdatPrefix();
 if (TT.isOSBinFormatCOFF()) {
-  // There are two cases that need a comdat on COFF:
-  // 1. Functions that already have comdats (standard case)
-  // 2. available_externally functions (dllimport and C99 inline)
-  // In the first case, put all the data in the original function comdat. In
-  // the second case, create a new comdat group using the counter as the
-  // leader. It's linkage must be external, so use linkonce_odr linkage in
-  // that case.
-  if (Comdat *C = Fn->getComdat()) {
-Cmdt = C;
-  } else {
-Cmdt = M->getOrInsertComdat(
-getVarName(Inc, getInstrProfCountersVarPrefix()));
-CounterLinkage = GlobalValue::LinkOnceODRLinkage;
-  }
-} else {
-  // For oth

r355044 - [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via cfe-commits]

Author: rnk
Date: Wed Feb 27 15:38:44 2019
New Revision: 355044

URL: http://llvm.org/viewvc/llvm-project?rev=355044&view=rev
Log:
[InstrProf] Use separate comdat group for data and counters

Summary:
I hadn't realized that instrumentation runs before inlining, so we can't
use the function as the comdat group. Doing so can create relocations
against discarded sections when references to discarded __profc_
variables are inlined into functions outside the function's comdat
group.

In the future, perhaps we should consider standardizing the comdat group
names that ELF and COFF use. It will save object file size, since
__profv_$sym won't appear in the symbol table again.

Reviewers: xur, vsk

Subscribers: eraman, hiraditya, cfe-commits, #sanitizers, llvm-commits

Tags: #clang, #sanitizers, #llvm

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

Modified:
cfe/trunk/test/Profile/cxx-templates.cpp

Modified: cfe/trunk/test/Profile/cxx-templates.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/cxx-templates.cpp?rev=355044&r1=355043&r2=355044&view=diff
==
--- cfe/trunk/test/Profile/cxx-templates.cpp (original)
+++ cfe/trunk/test/Profile/cxx-templates.cpp Wed Feb 27 15:38:44 2019
@@ -10,8 +10,8 @@
 // RUN: FileCheck --input-file=%tuse -check-prefix=T0USE -check-prefix=ALL %s
 // RUN: FileCheck --input-file=%tuse -check-prefix=T100USE -check-prefix=ALL %s
 
-// T0GEN: @[[T0C:__profc__Z4loopILj0EEvv]] = {{(linkonce_odr 
hidden|internal)}} global [2 x i64] zeroinitializer
-// T100GEN: @[[T100C:__profc__Z4loopILj100EEvv]] = {{(linkonce_odr 
hidden|internal)}} global [2 x i64] zeroinitializer
+// T0GEN: @[[T0C:__profc__Z4loopILj0EEvv]] = linkonce_odr 
{{(hidden|dso_local)}} global [2 x i64] zeroinitializer
+// T100GEN: @[[T100C:__profc__Z4loopILj100EEvv]] = linkonce_odr 
{{(hidden|dso_local)}} global [2 x i64] zeroinitializer
 
 // T0GEN-LABEL: define linkonce_odr {{.*}}void @_Z4loopILj0EEvv()
 // T0USE-LABEL: define linkonce_odr {{.*}}void @_Z4loopILj0EEvv()


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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

I certainly like that llc now does the right thing by default!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

I think I like this.

We may find a reason down the line to allow passive to be set per-segment, but 
now keeping it simple and lettings the linker decide makes sense.




Comment at: llvm/test/CodeGen/WebAssembly/atomic-mem-consistency.ll:1
-; RUN: not llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt
+; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt
 ; RUN: llc < %s -asm-verbose=false -disable-wasm-fallthrough-return-opt 
-wasm-disable-explicit-locals -wasm-keep-registers -mattr=+atomics,+sign-ext | 
FileCheck %s

What did this start passing?   Is it because llc now has the correct default 
and will lower atomics to non-atomics?  I guess there is no longer a way to 
make this fail in the way that it did before.. which is good?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58742



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D58731#1412843 , @alexfh wrote:

> In D58731#1412831 , @lewmpk wrote:
>
> > I'm trying to find a way to run the test. If someone else has already 
> > tested it then yes please.
>
>
> If you've set up the build with ninja, `ninja check-clang-tools` should do 
> the right thing. But I'm running the test anyway.


The new test fails even after I fixed the YAML config in the RUN lines. Please 
fix and re-upload the patch.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D58737#1412847 , @xur wrote:

> lgtm.
>
> BTW, I'm in the process of committing D54175 
> . After that patch, PGO instrumentation can 
> be called after the main inlining. I don't think it will conflict anything in 
> this patch


I see, I guess I'll update the comment to say "may run before the inliner".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Micah S. via Phabricator via cfe-commits]

micah-s added a comment.

In D28462#1412556 , @MyDeveloperDay 
wrote:

> For those wishing to test the effectiveness of unlanded revisions like this 
> and to reduce the amount of time between Windows snapshot builds 
> (https://llvm.org/builds/), I have forked llvm-project and using AppVeyor  to 
> build a new x64 Windows clang-format-experimental.exe binary, on a 
> semi-automatic basis. (master branch clang-format with selected unlanded 
> revisions)
>
> https://github.com/mydeveloperday/clang-experimental/releases


Thanks @MyDeveloperDay! That is very helpful, and significantly reduced the 
overhead for testing.

I removed the `// clang-format off`/`on` markers around all of our aligned 
macros, applied the `AlignConsecurityMacros: true` setting to our 
.clang-format, and reformatted the affected files using the 
clang-format-experimental.exe binary.  My own experience was flawless.  The 
only issue we'll have to work around is that we have some longer macro sets 
that include unused values in a series.

For example:

  ...
  #define CL_SCAN_ALGORITHMIC   0x200
  //#define UNUSED  0x400
  #define CL_SCAN_PHISHING_BLOCKSSL 0x800
  ...

becomes

  ...
  #define CL_SCAN_ALGORITHMIC 0x200
  //#define UNUSED  0x400
  #define CL_SCAN_PHISHING_BLOCKSSL 0x800
  ...

I can't really complain though, because it's working as intended -- we'll just 
have to find a different way to indicate reserved/unused values in a series.  
I'll probably just do something like `CL_SCAN_UNUSED_0`/`1`/`2`/etc.
Big thumbs up here from an end user experience.


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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D28462#1407239 , @micah-s wrote:

> ClamAV recently started using clang-format.  We published this blog post 
> about how we're using it: 
> https://blog.clamav.net/2019/02/clamav-adopts-clang-format.html  One of the 
> things I called out in the blog post is that we really want this feature, and 
> presently we have to use the "// clang-format on/off" markers throughout our 
> code mostly to keep our consecutive macros aligned.
>
> I haven't found time to build clang-format with this patch to test it on our 
> code base with the markers disabled.  If a review of my experience doing so 
> will help get traction for this review in any way, let me know and I'll make 
> the effort.


For those wishing to test the effectiveness of unlanded revisions like this and 
to reduce the amount of time between Windows snapshot builds 
(https://llvm.org/builds/), I have forked llvm-project and using AppVeyor  to 
build a new x64 Windows clang-format-experimental.exe binary, on a 
semi-automatic basis. (master branch clang-format with selected unlanded 
revisions)

https://github.com/mydeveloperday/clang-experimental/releases

When a revision like this meets excessive delay in the review cycle, feel free 
to log a github issue, Early testing of unlanded features may help to prove the 
usefulness of a revision, provide useful review comments and/or help find bugs 
earlier in the development cycle.


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

https://reviews.llvm.org/D28462



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

r355008 - Support framework import/include auto-completion

[David Goldman via cfe-commits]

Author: dgoldman
Date: Wed Feb 27 09:40:33 2019
New Revision: 355008

URL: http://llvm.org/viewvc/llvm-project?rev=355008&view=rev
Log:
Support framework import/include auto-completion

Frameworks filesystem representations:
  UIKit.framework/Headers/%header%

Framework import format:
  #import 

Thus the completion code must map the input format of  to
the path of UIKit.framework/Headers as well as strip the
".framework" suffix when auto-completing the framework name.

Added:
cfe/trunk/test/CodeCompletion/included-frameworks.m
Modified:
cfe/trunk/lib/Sema/SemaCodeComplete.cpp

Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=355008&r1=355007&r2=355008&view=diff
==
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Wed Feb 27 09:40:33 2019
@@ -8404,10 +8404,23 @@ void Sema::CodeCompleteIncludedFile(llvm
   };
 
   // Helper: scans IncludeDir for nice files, and adds results for each.
-  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir,
+bool IsSystem,
+DirectoryLookup::LookupType_t LookupType) {
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
-  llvm::sys::path::append(Dir, NativeRelDir);
+if (!NativeRelDir.empty()) {
+  if (LookupType == DirectoryLookup::LT_Framework) {
+// For a framework dir, #include  actually maps to
+// a path of Foo.framework/Headers/Bar/.
+auto Begin = llvm::sys::path::begin(NativeRelDir);
+auto End = llvm::sys::path::end(NativeRelDir);
+
+llvm::sys::path::append(Dir, *Begin + ".framework", "Headers");
+llvm::sys::path::append(Dir, ++Begin, End);
+  } else {
+llvm::sys::path::append(Dir, NativeRelDir);
+  }
+}
 
 std::error_code EC;
 unsigned Count = 0;
@@ -8418,6 +8431,12 @@ void Sema::CodeCompleteIncludedFile(llvm
   StringRef Filename = llvm::sys::path::filename(It->path());
   switch (It->type()) {
   case llvm::sys::fs::file_type::directory_file:
+// All entries in a framework directory must have a ".framework" 
suffix,
+// but the suffix does not appear in the source code's include/import.
+if (LookupType == DirectoryLookup::LT_Framework &&
+NativeRelDir.empty() && !Filename.consume_back(".framework"))
+  break;
+
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
   case llvm::sys::fs::file_type::regular_file:
@@ -8446,10 +8465,12 @@ void Sema::CodeCompleteIncludedFile(llvm
   // header maps are not (currently) enumerable.
   break;
 case DirectoryLookup::LT_NormalDir:
-  AddFilesFromIncludeDir(IncludeDir.getDir()->getName(), IsSystem);
+  AddFilesFromIncludeDir(IncludeDir.getDir()->getName(), IsSystem,
+ DirectoryLookup::LT_NormalDir);
   break;
 case DirectoryLookup::LT_Framework:
-  AddFilesFromIncludeDir(IncludeDir.getFrameworkDir()->getName(), 
IsSystem);
+  AddFilesFromIncludeDir(IncludeDir.getFrameworkDir()->getName(), IsSystem,
+ DirectoryLookup::LT_Framework);
   break;
 }
   };
@@ -8463,7 +8484,8 @@ void Sema::CodeCompleteIncludedFile(llvm
 // The current directory is on the include path for "quoted" includes.
 auto *CurFile = PP.getCurrentFileLexer()->getFileEntry();
 if (CurFile && CurFile->getDir())
-  AddFilesFromIncludeDir(CurFile->getDir()->getName(), false);
+  AddFilesFromIncludeDir(CurFile->getDir()->getName(), false,
+ DirectoryLookup::LT_NormalDir);
 for (const auto &D : make_range(S.quoted_dir_begin(), S.quoted_dir_end()))
   AddFilesFromDirLookup(D, false);
   }

Added: cfe/trunk/test/CodeCompletion/included-frameworks.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/included-frameworks.m?rev=355008&view=auto
==
--- cfe/trunk/test/CodeCompletion/included-frameworks.m (added)
+++ cfe/trunk/test/CodeCompletion/included-frameworks.m Wed Feb 27 09:40:33 2019
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t && mkdir -p %t/Foo.framework/Headers/SubFolder && mkdir 
%t/NotAFramework/
+// RUN: touch %t/Foo.framework/Headers/Foo.h && touch 
%t/Foo.framework/Headers/FOOClass.h
+// RUN: touch %t/Foo.framework/Headers/SubFolder/FOOInternal.h
+
+#import 
+
+#import 
+
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+// Autocomplete frameworks without the ".framework" extension.
+//
+// RUN: %clang -fsyntax-only -F %t -Xclang -code-completion-at=%s:5:10 %s -o - 
| FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: 

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Rong Xu via Phabricator via cfe-commits]

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

lgtm.

BTW, I'm in the process of committing D54175 . 
After that patch, PGO instrumentation can be called after the main inlining. I 
don't think it will conflict anything in this patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737



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

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Oops, forgot to respond to this...

In D58737#1412734 , @vsk wrote:

> I'm confused by this wording re: comdats in the LangRef: "All global objects 
> that specify this key will only end up in the final object file if the linker 
> chooses that key over some other key". Why can multiple global objects with 
> the same comdat key end up in the final object file?


I agree, that wording seems very confusing. I probably even wrote it. I'll try 
editing it and send you the result later.

These words are trying to say that, some number of global objects can use a 
comdat key. If the object file they are contained in has the prevailing 
definition of that comdat key, then they will be included, and they will be 
discarded if not. The prevailing object is typically just the first object file 
that uses the comdat.

So, multiple objects from *one TU* can use the key and end up in the final 
binary, but objects from different TUs will never end up mixed together in the 
final binary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

In D58731#1412831 , @lewmpk wrote:

> I'm trying to find a way to run the test. If someone else has already tested 
> it then yes please.


If you've set up the build with ninja, `ninja check-clang-tools` should do the 
right thing. But I'm running the test anyway.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58744: [CodeGen] Fix some broken IR generated by -fsanitize=unsigned-integer-overflow

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, arphaman, ahatanak.
Herald added subscribers: jdoerfert, jfb, dexonsmith, jkorous.
Herald added a project: clang.

I think the author of the function assumed that `GetInsertBlock()` wouldn't 
change from where `atomicPHI` was created, but this isn't true when 
`-fsanitize=unsigned-integer-overflow` is enabled (we generate an 
overflow/continuation label). Fix by keeping track of the block we want to 
return to to complete the cmpxchg loop.

rdar://48406558

Thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D58744

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/sanitize-atomic-int-overflow.c

Index: clang/test/CodeGen/sanitize-atomic-int-overflow.c
===
--- /dev/null
+++ clang/test/CodeGen/sanitize-atomic-int-overflow.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-apple-macosx10.14.0 -fsanitize=unsigned-integer-overflow %s -emit-llvm -o -
+
+_Atomic(unsigned) atomic;
+
+void cmpd_assign() {
+  atomic += 1;
+}
+
+void inc() {
+  atomic++;
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -2350,6 +2350,7 @@
 
   QualType type = E->getSubExpr()->getType();
   llvm::PHINode *atomicPHI = nullptr;
+  llvm::BasicBlock *atomicOpBB = nullptr;
   llvm::Value *value;
   llvm::Value *input;
 
@@ -2393,10 +2394,10 @@
 input = value;
 // For every other atomic operation, we need to emit a load-op-cmpxchg loop
 llvm::BasicBlock *startBB = Builder.GetInsertBlock();
-llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+atomicOpBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
 value = CGF.EmitToMemory(value, type);
-Builder.CreateBr(opBB);
-Builder.SetInsertPoint(opBB);
+Builder.CreateBr(atomicOpBB);
+Builder.SetInsertPoint(atomicOpBB);
 atomicPHI = Builder.CreatePHI(value->getType(), 2);
 atomicPHI->addIncoming(value, startBB);
 value = atomicPHI;
@@ -2555,14 +2556,14 @@
   }
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LV, RValue::get(atomicPHI), RValue::get(value), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), type);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicOpBB);
 Builder.SetInsertPoint(contBB);
 return isPre ? value : input;
   }
@@ -2838,6 +2839,7 @@
   LValue LHSLV = EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
 
   llvm::PHINode *atomicPHI = nullptr;
+  llvm::BasicBlock *atomicOpBB = nullptr;
   if (const AtomicType *atomicTy = LHSTy->getAs()) {
 QualType type = atomicTy->getValueType();
 if (!type->isBooleanType() && type->isIntegerType() &&
@@ -2884,11 +2886,11 @@
 // FIXME: For floating point types, we should be saving and restoring the
 // floating point environment in the loop.
 llvm::BasicBlock *startBB = Builder.GetInsertBlock();
-llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+atomicOpBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
 OpInfo.LHS = EmitLoadOfLValue(LHSLV, E->getExprLoc());
 OpInfo.LHS = CGF.EmitToMemory(OpInfo.LHS, type);
-Builder.CreateBr(opBB);
-Builder.SetInsertPoint(opBB);
+Builder.CreateBr(atomicOpBB);
+Builder.SetInsertPoint(atomicOpBB);
 atomicPHI = Builder.CreatePHI(OpInfo.LHS->getType(), 2);
 atomicPHI->addIncoming(OpInfo.LHS, startBB);
 OpInfo.LHS = atomicPHI;
@@ -2909,14 +2911,14 @@
 Loc, ScalarConversionOpts(CGF.SanOpts));
 
   if (atomicPHI) {
-llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+llvm::BasicBlock *curBlock = Builder.GetInsertBlock();
 llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
 auto Pair = CGF.EmitAtomicCompareExchange(
 LHSLV, RValue::get(atomicPHI), RValue::get(Result), E->getExprLoc());
 llvm::Value *old = CGF.EmitToMemory(Pair.first.getScalarVal(), LHSTy);
 llvm::Value *success = Pair.second;
-atomicPHI->addIncoming(old, opBB);
-Builder.CreateCondBr(success, contBB, opBB);
+atomicPHI->addIncoming(old, curBlock);
+Builder.CreateCondBr(success, contBB, atomicOpBB);
 Builder.SetInsertPoint(contBB);
 return LHSLV;
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk added a comment.

I'm trying to find a way to run the test. If someone else has already tested it 
then yes please.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58743: Handle built-in when importing SourceLocation and FileID

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: martong, a.sidorin, teemperor, aaron.ballman.
Herald added subscribers: jdoerfert, rnkovacs.

Currently when we see a built-in we try and import the include location. 
Instead what we do now is find the buffer like we do for the invalid case and 
copy that over to the to context.


https://reviews.llvm.org/D58743

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


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8229,9 +8229,10 @@
 return {};
 
   SourceManager &FromSM = FromContext.getSourceManager();
+  bool IsBuiltin =  FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  FileID ToFileID = Import(Decomposed.first);
+  FileID ToFileID = Import(Decomposed.first, IsBuiltin);
   if (ToFileID.isInvalid())
 return {};
   SourceManager &ToSM = ToContext.getSourceManager();
@@ -8252,7 +8253,7 @@
 return llvm::make_error();
   return ToID;
 }
-FileID ASTImporter::Import(FileID FromID) {
+FileID ASTImporter::Import(FileID FromID, bool isBuiltin) {
   llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
@@ -8278,25 +8279,30 @@
 }
 ToID = ToSM.getFileID(MLoc);
   } else {
-// Include location of this file.
-SourceLocation ToIncludeLoc = Import(FromSLoc.getFile().getIncludeLoc());
-
 const SrcMgr::ContentCache *Cache = FromSLoc.getFile().getContentCache();
-if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
-  // FIXME: We probably want to use getVirtualFile(), so we don't hit the
-  // disk again
-  // FIXME: We definitely want to re-use the existing MemoryBuffer, rather
-  // than mmap the files several times.
-  const FileEntry *Entry =
-  ToFileManager.getFile(Cache->OrigEntry->getName());
-  // FIXME: The filename may be a virtual name that does probably not
-  // point to a valid file and we get no Entry here. In this case try with
-  // the memory buffer below.
-  if (Entry)
-ToID = ToSM.createFileID(Entry, ToIncludeLoc,
- FromSLoc.getFile().getFileCharacteristic());
+
+if (!isBuiltin) {
+   // Include location of this file.
+   SourceLocation ToIncludeLoc = 
Import(FromSLoc.getFile().getIncludeLoc());
+
+
+if (Cache->OrigEntry && Cache->OrigEntry->getDir()) {
+  // FIXME: We probably want to use getVirtualFile(), so we don't hit 
the
+  // disk again
+  // FIXME: We definitely want to re-use the existing MemoryBuffer, 
rather
+  // than mmap the files several times.
+  const FileEntry *Entry =
+  ToFileManager.getFile(Cache->OrigEntry->getName());
+  // FIXME: The filename may be a virtual name that does probably not
+  // point to a valid file and we get no Entry here. In this case try 
with
+  // the memory buffer below.
+  if (Entry)
+ToID = ToSM.createFileID(Entry, ToIncludeLoc,
+ 
FromSLoc.getFile().getFileCharacteristic());
+}
 }
-if (ToID.isInvalid()) {
+
+if (ToID.isInvalid() || isBuiltin) {
   // FIXME: We want to re-use the existing MemoryBuffer!
   bool Invalid = true;
   const llvm::MemoryBuffer *FromBuf = Cache->getBuffer(
Index: include/clang/AST/ASTImporter.h
===
--- include/clang/AST/ASTImporter.h
+++ include/clang/AST/ASTImporter.h
@@ -339,7 +339,7 @@
 /// context, or the import error.
 llvm::Expected Import_New(FileID);
 // FIXME: Remove this version.
-FileID Import(FileID);
+FileID Import(FileID, bool isBuiltin=false);
 
 /// Import the given C++ constructor initializer from the "from"
 /// context into the "to" context.


Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -8229,9 +8229,10 @@
 return {};
 
   SourceManager &FromSM = FromContext.getSourceManager();
+  bool IsBuiltin =  FromSM.isWrittenInBuiltinFile(FromLoc);
 
   std::pair Decomposed = FromSM.getDecomposedLoc(FromLoc);
-  FileID ToFileID = Import(Decomposed.first);
+  FileID ToFileID = Import(Decomposed.first, IsBuiltin);
   if (ToFileID.isInvalid())
 return {};
   SourceManager &ToSM = ToContext.getSourceManager();
@@ -8252,7 +8253,7 @@
 return llvm::make_error();
   return ToID;
 }
-FileID ASTImporter::Import(FileID FromID) {
+FileID ASTImporter::Import(FileID FromID, bool isBuiltin) {
   llvm::DenseMap::iterator Pos = ImportedFileIDs.find(FromID);
   if (Pos != ImportedFileIDs.end())
 return Pos->second;
@@ -8278,25 +8279,30 @@
 }
 ToID = ToSM.getFileID(MLoc);
   } else {
-// Include location of

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via Phabricator via cfe-commits]

rnk updated this revision to Diff 188635.
rnk marked an inline comment as done.
rnk added a comment.

- share code


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737

Files:
  clang/test/Profile/cxx-templates.cpp
  compiler-rt/test/profile/coverage-inline.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/PR23499.ll
  llvm/test/Instrumentation/InstrProfiling/comdat.ll

Index: llvm/test/Instrumentation/InstrProfiling/comdat.ll
===
--- llvm/test/Instrumentation/InstrProfiling/comdat.ll
+++ llvm/test/Instrumentation/InstrProfiling/comdat.ll
@@ -17,8 +17,8 @@
 
 ; ELF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat($__profv_foo_inline), align 8
 ; ELF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat($__profv_foo_inline), align 8
-; COFF: @__profc_foo_inline = internal global{{.*}}, section ".lprfc$M", comdat($foo_inline), align 8
-; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($foo_inline), align 8
+; COFF: @__profc_foo_inline = linkonce_odr dso_local global{{.*}}, section ".lprfc$M", comdat, align 8
+; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($__profc_foo_inline), align 8
 define weak_odr void @foo_inline() comdat {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
Index: llvm/test/Instrumentation/InstrProfiling/PR23499.ll
===
--- llvm/test/Instrumentation/InstrProfiling/PR23499.ll
+++ llvm/test/Instrumentation/InstrProfiling/PR23499.ll
@@ -20,8 +20,8 @@
 
 
 ; COFF-NOT: __profn__Z3barIvEvv
-; COFF: @__profc__Z3barIvEvv = internal global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat($_Z3barIvEvv), align 8
-; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($_Z3barIvEvv), align 8
+; COFF: @__profc__Z3barIvEvv = linkonce_odr dso_local global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat, align 8
+; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($__profc__Z3barIvEvv), align 8
 
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -743,30 +743,23 @@
 
   // Move the name variable to the right section. Place them in a COMDAT group
   // if the associated function is a COMDAT. This will make sure that only one
-  // copy of counters of the COMDAT function will be emitted after linking.
+  // copy of counters of the COMDAT function will be emitted after linking. Keep
+  // in mind that this pass runs before the inliner, so we need to create a new
+  // comdat group for the counters and profiling data. If we use the comdat of
+  // the parent function, that will result in relocations against discarded
+  // sections.
   Comdat *Cmdt = nullptr;
   GlobalValue::LinkageTypes CounterLinkage = Linkage;
   if (needsComdatForCounter(*Fn, *M)) {
+StringRef CmdtPrefix = getInstrProfComdatPrefix();
 if (TT.isOSBinFormatCOFF()) {
-  // There are two cases that need a comdat on COFF:
-  // 1. Functions that already have comdats (standard case)
-  // 2. available_externally functions (dllimport and C99 inline)
-  // In the first case, put all the data in the original function comdat. In
-  // the second case, create a new comdat group using the counter as the
-  // leader. It's linkage must be external, so use linkonce_odr linkage in
-  // that case.
-  if (Comdat *C = Fn->getComdat()) {
-Cmdt = C;
-  } else {
-Cmdt = M->getOrInsertComdat(
-getVarName(Inc, getInstrProfCountersVarPrefix()));
-CounterLinkage = GlobalValue::LinkOnceODRLinkage;
-  }
-} else {
-  // For other platforms that use comdats (ELF), make a new comdat group for
-  // all the profile data. It will be deduplicated within the current DSO.
-  Cmdt = M->getOrInsertComdat(getVarName(Inc, getInstrProfComdatPrefix()));
+  // For COFF, the comdat group name mus

[PATCH] D58742: [WebAssembly] Remove uses of ThreadModel

[Thomas Lively via Phabricator via cfe-commits]

tlively created this revision.
tlively added reviewers: aheejin, sbc100, dschuff.
Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, jfb, rupprecht, 
dexonsmith, steven_wu, sunfish, hiraditya, jgravelle-google, mehdi_amini.
Herald added projects: clang, LLVM.

In the clang UI, replaces -mthread-model posix with -matomics as the
source of truth on threading. In the backend, replaces
-thread-model=posix with the atomics target feature, which is now
collected on the WebAssemblyTargetMachine along with all other used
features. These collected features will also be used to emit the
target features section in the future.

The default configuration for the backend is thread-model=posix and no
atomics, which was previously an invalid configuration. This change
makes the default valid because the thread model is ignored.

A side effect of this change is that objects are never emitted with
passive segments. It will instead be up to the linker to decide
whether sections should be active or passive based on whether atomics
are used in the final link.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58742

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/ToolChain.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/WebAssembly.cpp
  clang/lib/Driver/ToolChains/WebAssembly.h
  clang/test/Driver/wasm-toolchain.c
  clang/test/Preprocessor/wasm-target-features.c
  lld/test/wasm/init-fini.ll
  lld/test/wasm/lto/atomics.ll
  lld/wasm/LTO.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.h
  llvm/test/CodeGen/WebAssembly/atomic-mem-consistency.ll
  llvm/test/CodeGen/WebAssembly/atomic-rmw.ll
  llvm/test/CodeGen/WebAssembly/global.ll
  llvm/test/CodeGen/WebAssembly/tls.ll
  llvm/test/CodeGen/WebAssembly/vtable.ll
  llvm/test/MC/WebAssembly/bss.ll
  llvm/test/MC/WebAssembly/comdat.ll
  llvm/test/MC/WebAssembly/debug-info.ll
  llvm/test/MC/WebAssembly/explicit-sections.ll
  llvm/test/MC/WebAssembly/external-data.ll
  llvm/test/MC/WebAssembly/external-func-address.ll
  llvm/test/MC/WebAssembly/global-ctor-dtor.ll
  llvm/test/MC/WebAssembly/init-flags.ll
  llvm/test/MC/WebAssembly/reloc-data.ll
  llvm/test/MC/WebAssembly/unnamed-data.ll
  llvm/test/MC/WebAssembly/weak-alias.ll
  llvm/test/tools/llvm-nm/wasm/local-symbols.ll
  llvm/test/tools/llvm-objdump/WebAssembly/relocations.test

Index: llvm/test/tools/llvm-objdump/WebAssembly/relocations.test
===
--- llvm/test/tools/llvm-objdump/WebAssembly/relocations.test
+++ llvm/test/tools/llvm-objdump/WebAssembly/relocations.test
@@ -1,4 +1,4 @@
-; RUN: llc -thread-model=single -mtriple=wasm32-unknown-unknown -filetype=obj %s -o - | llvm-objdump -r - | FileCheck %s
+; RUN: llc -mtriple=wasm32-unknown-unknown -filetype=obj %s -o - | llvm-objdump -r - | FileCheck %s
 
 @foo = external global i32, align 4
 @bar = global i32* @foo, align 4
Index: llvm/test/tools/llvm-nm/wasm/local-symbols.ll
===
--- llvm/test/tools/llvm-nm/wasm/local-symbols.ll
+++ llvm/test/tools/llvm-nm/wasm/local-symbols.ll
@@ -1,4 +1,4 @@
-; RUN: llc -filetype=obj -thread-model=single -mtriple=wasm32-unknown-unknown -o %t.o %s
+; RUN: llc -filetype=obj -mtriple=wasm32-unknown-unknown -o %t.o %s
 ; RUN: llvm-nm %t.o | FileCheck %s
 
 @foo = internal global i32 1, align 4
Index: llvm/test/MC/WebAssembly/weak-alias.ll
===
--- llvm/test/MC/WebAssembly/weak-alias.ll
+++ llvm/test/MC/WebAssembly/weak-alias.ll
@@ -1,4 +1,4 @@
-; RUN: llc -filetype=obj -thread-model=single -wasm-keep-registers %s -o %t.o
+; RUN: llc -filetype=obj -wasm-keep-registers %s -o %t.o
 ; RUN: obj2yaml %t.o | FileCheck %s
 ; RUN: llvm-objdump -t %t.o | FileCheck --check-prefix=CHECK-SYMS %s
 
Index: llvm/test/MC/WebAssembly/unnamed-data.ll
===
--- llvm/test/MC/WebAssembly/unnamed-data.ll
+++ llvm/test/MC/WebAssembly/unnamed-data.ll
@@ -1,4 +1,4 @@
-; RUN: llc -filetype=obj -thread-model=single %s -o - | obj2yaml | FileCheck %s
+; RUN: llc -filetype=obj %s -o - | obj2yaml | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
Index: llvm/test/MC/WebAssembly/reloc-data.ll
===
--- llvm/test/MC/WebAssembly/reloc-data.ll
+++ llvm/test/MC/WebAssembly/reloc-data.ll
@@ -1,4 +1,4 @@
-; RUN: llc -O0 -filetype=obj -thread-model=single %s -o - | llvm-readobj -r -expand-relocs | FileCheck %s
+; RUN: llc -O0 -filetype=obj %s -o - | llvm-readobj -r -expand-relocs | FileCheck %s
 
 target triple = "wasm32-unknown-unknown"
 
Index: llvm/test/MC/WebAssembly/init-flags.ll
=

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

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

Looks good! Do you need someone to commit the patch for you?


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

https://reviews.llvm.org/D58731



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

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via Phabricator via cfe-commits]

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

In D58737#1412734 , @vsk wrote:

> I'm confused by this wording re: comdats in the LangRef: "All global objects 
> that specify this key will only end up in the final object file if the linker 
> chooses that key over some other key". Why can multiple global objects with 
> the same comdat key end up in the final object file?
>
> Also, why is the selection kind here 'any' instead of 'exactmatch'? If the 
> counter array sizes legitimately can vary (why?), then wouldn't we want 
> 'largestsize'?


I think if the data size varies, then we are out of luck. Every TU will produce 
the same data if the profile intrinsics are inserted at the same point in the 
pipeline. Also, both `largestsize` and `exactmatch` are COFF-only.

For code coverage, the instrprof.increment calls are inserted very early, and 
it is source-directed, so we should be pretty confident that all profile data 
for a given inline function is identical across the whole program. Of course, 
different versions of clang may insert counters differently, so if you link 
together object files produced by two versions of clang, you run the risk of 
having corrupt profile data.

For PGO, I have less confidence that that is true, but I believe InstrProf is 
carefully placed in the pipeline (before the main inliner pass, preinlining 
only) at such a point to reduce the likelihood that this happens. Of course, 
PGO already locks in the compiler version of the whole program. You can't 
optimize with instrumentation inserted by the last release of clang.




Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:756
+  // For COFF, the comdat group name must be the name of a symbol in the
+  // group. Use the counter variable name.
+  Cmdt = M->getOrInsertComdat(

xur wrote:
> So with the patch, COFF and ELF have the same way of naming. Can we simplify 
> the code by hosting the common code?
It's not identical yet, though, but maybe it should be. For COFF, the comdat is 
`__profc_$sym`, but for ELF, it is `__profv_$sym`. However, they are very 
similar, and I can factor out the common code now without changing ELF to match 
COFF.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737



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

[PATCH] D54395: [clang-tidy] implement utility-function to add 'const' to variables

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

A few more comments.




Comment at: clang-tidy/utils/FixItHintUtils.cpp:83
+return (llvm::Twine(' ') +
+llvm::Twine(DeclSpec::getSpecifierName(Qualifier)))
+.str();

The first operand being llvm::Twine is enough for the whole chained 
concatenation expression to be llvm::Twine.



Comment at: unittests/clang-tidy/AddConstTest.cpp:15
+
+template 

What's the point of default values of template arguments here?



Comment at: unittests/clang-tidy/AddConstTest.cpp:53
+
+// TODO: Template-code
+

Is this still needed? If yes, please use "FIXME" instead and expand on what 
exactly is missing or needs to be done.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54395



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk added a comment.

thanks for the feedback everyone (and the warm welcome)


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

https://reviews.llvm.org/D58731



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

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Rong Xu via Phabricator via cfe-commits]

xur added inline comments.



Comment at: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp:756
+  // For COFF, the comdat group name must be the name of a symbol in the
+  // group. Use the counter variable name.
+  Cmdt = M->getOrInsertComdat(

So with the patch, COFF and ELF have the same way of naming. Can we simplify 
the code by hosting the common code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk updated this revision to Diff 188628.
lewmpk added a comment.

addressed comments


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

https://reviews.llvm.org/D58731

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/modernize/UseOverrideCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
  docs/clang-tidy/checks/modernize-use-override.rst
  test/clang-tidy/modernize-use-override-no-destructors.cpp

Index: test/clang-tidy/modernize-use-override-no-destructors.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-no-destructors.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.IgnoreDestructors, value: '1'" \
+// RUN: -- -std=c++11
+
+struct Base {
+  virtual ~Base() {}
+  virtual void f() {}
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  void b() override;
+  virtual void f() {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void f() override;
+};
Index: docs/clang-tidy/checks/modernize-use-override.rst
===
--- docs/clang-tidy/checks/modernize-use-override.rst
+++ docs/clang-tidy/checks/modernize-use-override.rst
@@ -5,3 +5,11 @@
 
 
 Use C++11's ``override`` and remove ``virtual`` where applicable.
+
+Options
+---
+
+.. option:: IgnoreDestructors
+
+   If set to non-zero, this check with not diagnose destructors. Default is '0'.
+   
\ No newline at end of file
Index: docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - cppcoreguidelines-explicit-virtual-functions
+.. meta::
+   :http-equiv=refresh: 5;URL=cppcoreguidelines-explicit-virtual-functions.html
+
+cppcoreguidelines-explicit-virtual-functions
+
+
+The cppcoreguidelines-explicit-virtual-functions check is an alias, please see
+`modernize-use-override `_
+for more information.
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,11 @@
 Improvements to clang-tidy
 --
 
+- New alias :doc:`cppcoreguidelines-explicit-virtual-functions
+  ` to 
+  :doc:`modernize-use-override
+  ` was added.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,14 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -17,9 +17,20 @@
 namespace tidy {
 namespace modernize {
 
+void UseOverrideCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreDestructors", IgnoreDestructors);
+}
+
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
+  if (!getLangOpts().CPlusPlus11)
+return;
+
+  if (IgnoreDestructors)
+Finder->addMatcher(
+cxxMethodDecl(isOverride(), unless(cxxDestructorDecl())).bind("method"),
+this);
+  else
 Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
 }
 
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG. Sorry for the delay.


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

https://reviews.llvm.org/D57087



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

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

I'm confused by this wording re: comdats in the LangRef: "All global objects 
that specify this key will only end up in the final object file if the linker 
chooses that key over some other key". Why can multiple global objects with the 
same comdat key end up in the final object file?

Also, why is the selection kind here 'any' instead of 'exactmatch'? If the 
counter array sizes legitimately can vary (why?), then wouldn't we want 
'largestsize'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58737



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

[PATCH] D58606: [clang-tidy] misc-string-integer-assignment: fix false positive

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.

LG. Thanks for improving this check!


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58606



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: test/clang-tidy/modernize-use-override-no-destructors.cpp:6
+struct Base {
+  virtual ~Base(){};
+  virtual void f(){};

Remove the semicolons after methods. Clang-format the test (but ensure 
clang-format doesn't break comments at the 80 character bound).


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

https://reviews.llvm.org/D58731



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: clang-tidy/modernize/UseOverrideCheck.h:23
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;

lewmpk wrote:
> JonasToth wrote:
> > That requires additional methods for the function for the configuration to 
> > function properly. Please see other checks and implement that the same way.
> Could you elaborate? I looked at other checks and they seem to implement it 
> the same way.
> 
> /llvm/tools/clang/tools/extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:74
> 
> ```
> CheckFunctionCalls(Options.get("CheckFunctionCalls", false)),
> ```
>   
You need to override the `storeOptions()` method.


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

https://reviews.llvm.org/D58731



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

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D56160#1412701 , @bernhardmgruber 
wrote:

> - renamed the check to modernize-use-trailing-return-type


Thanks!


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

https://reviews.llvm.org/D56160



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thank you for the contribution! Please see the comments inline.




Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:22
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
-Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+  if (getLangOpts().CPlusPlus11) {
+if (IgnoreDestructors)

Please prefer early exit:
```
if (!getLangOpts().CPlusPlus11)
  return;

```



Comment at: docs/ReleaseNotes.rst:71
+- New :doc:`cppcoreguidelines-explicit-virtual-functions
+  ` check.
+

1. Please mention that this is an alias for the modernize-use-override check.
2. Please add a document for the new alias. See 
docs/clang-tidy/checks/cppcoreguidelines-avoid-c-arrays.rst for an example.


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

https://reviews.llvm.org/D58731



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk marked 2 inline comments as done and an inline comment as not done.
lewmpk added inline comments.



Comment at: clang-tidy/modernize/UseOverrideCheck.h:23
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;

JonasToth wrote:
> That requires additional methods for the function for the configuration to 
> function properly. Please see other checks and implement that the same way.
Could you elaborate? I looked at other checks and they seem to implement it the 
same way.

/llvm/tools/clang/tools/extra/clang-tidy/bugprone/AssertSideEffectCheck.cpp:74

```
CheckFunctionCalls(Options.get("CheckFunctionCalls", false)),
```
  


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

https://reviews.llvm.org/D58731



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk updated this revision to Diff 188622.
lewmpk marked an inline comment as done.
lewmpk added a comment.

- addressed comments
- provided tests
- updated documentation
- updated release notes




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

https://reviews.llvm.org/D58731

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/modernize/UseOverrideCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/modernize-use-override.rst
  test/clang-tidy/modernize-use-override-no-destructors.cpp

Index: test/clang-tidy/modernize-use-override-no-destructors.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-override-no-destructors.cpp
@@ -0,0 +1,17 @@
+// RUN: %check_clang_tidy %s modernize-use-override %t -- \
+// RUN:   -config="{CheckOptions: [{key: modernize-use-override.IgnoreDestructors, value: '1'" \
+// RUN: -- -std=c++11
+
+struct Base {
+  virtual ~Base(){};
+  virtual void f(){};
+};
+
+struct Simple : public Base {
+  virtual ~Simple();
+  // CHECK-MESSAGES-NOT: warning:
+  // CHECK-FIXES: {{^}}  void b() override;
+  virtual void f(){};
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: prefer using 'override' or (rarely) 'final' instead of 'virtual' [modernize-use-override]
+  // CHECK-FIXES: {{^}}  void f() override;
+};
Index: docs/clang-tidy/checks/modernize-use-override.rst
===
--- docs/clang-tidy/checks/modernize-use-override.rst
+++ docs/clang-tidy/checks/modernize-use-override.rst
@@ -5,3 +5,11 @@
 
 
 Use C++11's ``override`` and remove ``virtual`` where applicable.
+
+Options
+---
+
+.. option:: IgnoreDestructors
+
+   If set to non-zero, this check with not diagnose destructors. Default is '0'.
+   
\ No newline at end of file
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -67,6 +67,11 @@
 Improvements to clang-tidy
 --
 
+- New :doc:`cppcoreguidelines-explicit-virtual-functions
+  ` check.
+
+  Checks whether virtual functions other than destructors can specify 'override' instead.
+
 - New :doc:`abseil-duration-addition
   ` check.
 
Index: clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,13 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -19,8 +19,15 @@
 
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
-Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+  if (getLangOpts().CPlusPlus11) {
+if (IgnoreDestructors)
+  Finder->addMatcher(
+  cxxMethodDecl(isOverride(), unless(cxxDestructorDecl()))
+  .bind("method"),
+  this);
+else
+  Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+  }
 }
 
 // Re-lex the tokens to get precise locations to insert 'override' and remove
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
@@ -46,6 +47,8 @@
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-magic-numbers");
+CheckFactories.registerCheck(
+"cppcoreguidelines-explicit-virtual-functions");
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
 CheckFactories.registerCheck(
@@ -91,6 +94,9 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVari

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

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber added inline comments.



Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:335
+  StringRef ReturnType = tooling::fixit::getText(ReturnTypeCVRange, Ctx);
+  StringRef Auto = std::isspace(*ReturnType.end()) // FIXME (dereferencing end)
+   ? "auto"

JonasToth wrote:
> bernhardmgruber wrote:
> > FIXME: this feels wrong when I wrote it, but it works. I tried to fiddle 
> > with the ReturnTypeCVRange to include the next charakter, but I could not 
> > get it working without writing `tooling::fixit::getText` myself. Any ideas?
> That only works, because `StringRef` points within the orignal code buffer. 
> IMHO just adding the space always and letting clang-format do its job is 
> better then this potential out-of-bounds read.
> Maybe you could increase the `ReturnTypeCVRange` by one at the end, then its 
> fine.
I now retrieve the space after the return type explicitely. Using clang-format 
afterwards is not always possible on the code bases I work with, so I would 
like to not rely on it.


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

https://reviews.llvm.org/D56160



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

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

[Bernhard Manfred Gruber via Phabricator via cfe-commits]

bernhardmgruber updated this revision to Diff 188621.
bernhardmgruber marked 4 inline comments as done.
bernhardmgruber retitled this revision from "[clang-tidy] 
modernize-use-trailing-return check" to "[clang-tidy] 
modernize-use-trailing-return-type check".
bernhardmgruber added a comment.

- renamed the check to modernize-use-trailing-return-type
- fixed the out-of-bounds access to check whether there is a space after the 
return type


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

https://reviews.llvm.org/D56160

Files:
  clang-tidy/modernize/CMakeLists.txt
  clang-tidy/modernize/ModernizeTidyModule.cpp
  clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tidy/modernize/UseTrailingReturnTypeCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/modernize-use-trailing-return-type.rst
  test/clang-tidy/modernize-use-trailing-return-type.cpp

Index: test/clang-tidy/modernize-use-trailing-return-type.cpp
===
--- /dev/null
+++ test/clang-tidy/modernize-use-trailing-return-type.cpp
@@ -0,0 +1,437 @@
+// RUN: %check_clang_tidy %s modernize-use-trailing-return-type %t -- -- --std=c++14 -fdeclspec
+
+namespace std {
+template 
+class vector;
+
+template 
+class array;
+
+class string;
+
+template 
+auto declval() -> T;
+}
+
+//
+// Functions
+//
+
+int f();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f() -> int;{{$}}
+int ((f))();
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto ((f))() -> int;{{$}}
+int f(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int) -> int;{{$}}
+int f(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int arg) -> int;{{$}}
+int f(int arg1, int arg2, int arg3);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3) -> int;{{$}}
+int f(int arg1, int arg2, int arg3, ...);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto f(int arg1, int arg2, int arg3, ...) -> int;{{$}}
+template  int f(T t);
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}template  auto f(T t) -> int;{{$}}
+
+//
+// Functions with formatting
+//
+
+int a1() { return 42; }
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a1() -> int { return 42; }{{$}}
+int a2() {
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a2() -> int {{{$}}
+int a3()
+{
+return 42;
+}
+// CHECK-MESSAGES: :[[@LINE-4]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a3() -> int{{$}}
+int a4(int   arg   )   ;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a4(int   arg   ) -> int   ;{{$}}
+int a5
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a5{{$}}
+(int arg);
+// CHECK-FIXES: {{^}}(int arg) -> int;{{$}}
+const
+int
+*
+a7
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+()
+// CHECK-FIXES: {{^}}() -> const{{$}}
+// CHECK-FIXES: {{^}}int{{$}}
+// CHECK-FIXES: {{^}}*{{$}}
+;
+
+int*a7(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a7(int arg) -> int*;{{$}}
+template class C>
+Ca8(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}auto a8(int arg) -> C;{{$}}
+
+
+//
+// Functions with qualifiers and specifiers
+//
+
+inline int d1(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use a trailing return type for this function [modernize-use-trailing-return-type]
+// CHECK-FIXES: {{^}}inline auto d1(int arg) -> int;{{$}}
+extern "C" int d2(int arg);
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use a trailing return typ

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

[MyDeveloperDay via Phabricator via cfe-commits]

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

I think I've seen you need this for another patch, so in the absence of the 
code owners this LGTM, and thank you for removing all the changes to the tests 
in the previous diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56943



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

[PATCH] D58724: [gnustep-objc] Make the GNUstep v2 ABI work for Windows DLLs.

[Dustin L. Howett via Phabricator via cfe-commits]

DHowett-MSFT added a comment.

This looks great, and takes up the parts of my patch that I cared about. Thank 
you for doing this.
My primary concern is that the patch includes my "early init" changes -- while 
I support it and think it's the right solution, especially where it reduces 
double indirection on class pointers, there may be issues left to iron out.




Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:188
 
+  StringRef SymbolPrefix() {
+return CGM.getTriple().isOSBinFormatCOFF() ? "_" : "._";

Should this be `SymbolPrefix()` or something more like `MangleSymbol(StringRef 
sym)`? I know that right now we only need to prepend `_` or `._`, but is it 
more future-proof to make it generic?



Comment at: clang/lib/CodeGen/CGObjCGNU.cpp:1528
 InitStructBuilder.addInt(Int64Ty, 0);
-for (auto *s : SectionsBaseNames) {
-  auto bounds = GetSectionBounds(s);
-  InitStructBuilder.add(bounds.first);
-  InitStructBuilder.add(bounds.second);
-};
+if (CGM.getTriple().isOSBinFormatCOFF()) {
+  for (auto *s : PECOFFSectionsBaseNames) {

We may be able to reduce the duplication here (aside: it is very cool that 
Phabricator shows duplicated lines) by capturing `auto sectionVec = 
&SectionsBaseNames;` and switching which is in use based on bin format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58724



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

[PATCH] D58537: lib/Header: Simplify CMakeLists.txt

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added a comment.

LGTM, thanks!




Comment at: clang/lib/Headers/CMakeLists.txt:136
   list(APPEND out_files ${dst})
+  # The list function only updates out_files in the current scope.  We need
+  # call set in order to also update the variable for the parent's scope.

I don't think this comment is necessary – this is a pretty common CMake 
pattern, and I think the PARENT_SCOPE is enough of a search hint for people who 
aren't familiar with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58537



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

[PATCH] D58478: [index-while-building] FileIndexRecord

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355035: [clang][index-while-building] FileIndexRecord 
(authored by jkorous, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58478?vs=187863&id=188614#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58478

Files:
  include/clang/Index/DeclOccurrence.h
  lib/Index/CMakeLists.txt
  lib/Index/FileIndexRecord.cpp
  lib/Index/FileIndexRecord.h

Index: include/clang/Index/DeclOccurrence.h
===
--- include/clang/Index/DeclOccurrence.h
+++ include/clang/Index/DeclOccurrence.h
@@ -0,0 +1,42 @@
+//===--- DeclOccurrence.h - An occurrence of a decl within a file -===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_INDEX_DECLOCCURRENCE_H
+#define LLVM_CLANG_INDEX_DECLOCCURRENCE_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+class Decl;
+
+namespace index {
+
+struct DeclOccurrence {
+  SymbolRoleSet Roles;
+  unsigned Offset;
+  const Decl *Dcl;
+  SmallVector Relations;
+
+  DeclOccurrence(SymbolRoleSet R, unsigned Offset, const Decl *D,
+ ArrayRef Relations)
+  : Roles(R), Offset(Offset), Dcl(D),
+Relations(Relations.begin(), Relations.end()) {}
+
+  friend bool operator<(const DeclOccurrence &LHS, const DeclOccurrence &RHS) {
+return LHS.Offset < RHS.Offset;
+  }
+};
+
+} // namespace index
+} // namespace clang
+
+#endif
Index: lib/Index/CMakeLists.txt
===
--- lib/Index/CMakeLists.txt
+++ lib/Index/CMakeLists.txt
@@ -6,6 +6,7 @@
 add_clang_library(clangIndex
   CodegenNameGenerator.cpp
   CommentToXML.cpp
+  FileIndexRecord.cpp
   IndexBody.cpp
   IndexDecl.cpp
   IndexingAction.cpp
Index: lib/Index/FileIndexRecord.h
===
--- lib/Index/FileIndexRecord.h
+++ lib/Index/FileIndexRecord.h
@@ -0,0 +1,58 @@
+//===--- FileIndexRecord.h - Index data per file --===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_INDEX_FILEINDEXRECORD_H
+#define LLVM_CLANG_LIB_INDEX_FILEINDEXRECORD_H
+
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Index/DeclOccurrence.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+#include 
+
+namespace clang {
+class IdentifierInfo;
+
+namespace index {
+
+/// Stores the declaration occurrences seen in a particular source or header
+/// file of a translation unit
+class FileIndexRecord {
+private:
+  FileID FID;
+  bool IsSystem;
+  std::vector Decls;
+
+public:
+  FileIndexRecord(FileID FID, bool IsSystem) : FID(FID), IsSystem(IsSystem) {}
+
+  ArrayRef getDeclOccurrencesSortedByOffset() const {
+return Decls;
+  }
+
+  FileID getFileID() const { return FID; }
+  bool isSystem() const { return IsSystem; }
+
+  /// Adds an occurrence of the canonical declaration \c D at the supplied
+  /// \c Offset
+  ///
+  /// \param Roles the roles the occurrence fulfills in this position.
+  /// \param Offset the offset in the file of this occurrence.
+  /// \param D the canonical declaration this is an occurrence of.
+  /// \param Relations the set of symbols related to this occurrence.
+  void addDeclOccurence(SymbolRoleSet Roles, unsigned Offset, const Decl *D,
+ArrayRef Relations);
+  void print(llvm::raw_ostream &OS) const;
+};
+
+} // end namespace index
+} // end namespace clang
+
+#endif
Index: lib/Index/FileIndexRecord.cpp
===
--- lib/Index/FileIndexRecord.cpp
+++ lib/Index/FileIndexRecord.cpp
@@ -0,0 +1,59 @@
+//===--- FileIndexRecord.cpp - Index data per file ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FileIndexRecord.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang;
+using namespace clang::index;
+
+void FileIndexRecord::addDeclOccurence(Sym

r355035 - [clang][index-while-building] FileIndexRecord

[Jan Korous via cfe-commits]

Author: jkorous
Date: Wed Feb 27 13:47:40 2019
New Revision: 355035

URL: http://llvm.org/viewvc/llvm-project?rev=355035&view=rev
Log:
[clang][index-while-building] FileIndexRecord

Basic data structures for index

Tests are missing from this patch - will be covered properly by tests for the 
whole feature.
I'm just trying to split into smaller patches to make it easier for reviewers.

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

Added:
cfe/trunk/include/clang/Index/DeclOccurrence.h
cfe/trunk/lib/Index/FileIndexRecord.cpp
cfe/trunk/lib/Index/FileIndexRecord.h
Modified:
cfe/trunk/lib/Index/CMakeLists.txt

Added: cfe/trunk/include/clang/Index/DeclOccurrence.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/DeclOccurrence.h?rev=355035&view=auto
==
--- cfe/trunk/include/clang/Index/DeclOccurrence.h (added)
+++ cfe/trunk/include/clang/Index/DeclOccurrence.h Wed Feb 27 13:47:40 2019
@@ -0,0 +1,42 @@
+//===--- DeclOccurrence.h - An occurrence of a decl within a file 
-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_INDEX_DECLOCCURRENCE_H
+#define LLVM_CLANG_INDEX_DECLOCCURRENCE_H
+
+#include "clang/Basic/LLVM.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+class Decl;
+
+namespace index {
+
+struct DeclOccurrence {
+  SymbolRoleSet Roles;
+  unsigned Offset;
+  const Decl *Dcl;
+  SmallVector Relations;
+
+  DeclOccurrence(SymbolRoleSet R, unsigned Offset, const Decl *D,
+ ArrayRef Relations)
+  : Roles(R), Offset(Offset), Dcl(D),
+Relations(Relations.begin(), Relations.end()) {}
+
+  friend bool operator<(const DeclOccurrence &LHS, const DeclOccurrence &RHS) {
+return LHS.Offset < RHS.Offset;
+  }
+};
+
+} // namespace index
+} // namespace clang
+
+#endif

Modified: cfe/trunk/lib/Index/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/CMakeLists.txt?rev=355035&r1=355034&r2=355035&view=diff
==
--- cfe/trunk/lib/Index/CMakeLists.txt (original)
+++ cfe/trunk/lib/Index/CMakeLists.txt Wed Feb 27 13:47:40 2019
@@ -6,6 +6,7 @@ set(LLVM_LINK_COMPONENTS
 add_clang_library(clangIndex
   CodegenNameGenerator.cpp
   CommentToXML.cpp
+  FileIndexRecord.cpp
   IndexBody.cpp
   IndexDecl.cpp
   IndexingAction.cpp

Added: cfe/trunk/lib/Index/FileIndexRecord.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/FileIndexRecord.cpp?rev=355035&view=auto
==
--- cfe/trunk/lib/Index/FileIndexRecord.cpp (added)
+++ cfe/trunk/lib/Index/FileIndexRecord.cpp Wed Feb 27 13:47:40 2019
@@ -0,0 +1,59 @@
+//===--- FileIndexRecord.cpp - Index data per file 
===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "FileIndexRecord.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclTemplate.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/Path.h"
+
+using namespace clang;
+using namespace clang::index;
+
+void FileIndexRecord::addDeclOccurence(SymbolRoleSet Roles, unsigned Offset,
+   const Decl *D,
+   ArrayRef Relations) {
+  assert(D->isCanonicalDecl() &&
+ "Occurrences should be associated with their canonical decl");
+
+  auto IsNextOccurence = [&]() -> bool {
+if (Decls.empty())
+  return true;
+auto &Last = Decls.back();
+return Last.Offset < Offset;
+  };
+
+  if (IsNextOccurence()) {
+Decls.emplace_back(Roles, Offset, D, Relations);
+return;
+  }
+
+  DeclOccurrence NewInfo(Roles, Offset, D, Relations);
+  auto It = std::upper_bound(Decls.begin(), Decls.end(), NewInfo);
+  Decls.insert(It, std::move(NewInfo));
+}
+
+void FileIndexRecord::print(llvm::raw_ostream &OS) const {
+  OS << "DECLS BEGIN ---\n";
+  for (auto &DclInfo : Decls) {
+auto D = DclInfo.Dcl;
+SourceManager &SM = D->getASTContext().getSourceManager();
+SourceLocation Loc = SM.getFileLoc(D->getLocation());
+PresumedLoc PLoc = SM.getPresumedLoc(Loc);
+OS << llvm::sys::path::filename(PLoc.getFilename()) << ':' << 
PLoc.getLine()
+   << ':' << PLoc.getColumn();
+
+if (auto ND = dyn_cast(D)) {
+  OS << ' ' << ND->getNameAsString();
+}
+
+OS << '\n';
+  }
+  OS << "DECLS END ---\n";
+}

Add

r355036 - [clang][index-while-building][NFC] FileIndexRecord - Comments, replace auto with type

[Jan Korous via cfe-commits]

Author: jkorous
Date: Wed Feb 27 13:48:02 2019
New Revision: 355036

URL: http://llvm.org/viewvc/llvm-project?rev=355036&view=rev
Log:
[clang][index-while-building][NFC] FileIndexRecord - Comments, replace auto 
with type

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

Modified:
cfe/trunk/include/clang/Index/DeclOccurrence.h
cfe/trunk/lib/Index/FileIndexRecord.cpp
cfe/trunk/lib/Index/FileIndexRecord.h

Modified: cfe/trunk/include/clang/Index/DeclOccurrence.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Index/DeclOccurrence.h?rev=355036&r1=355035&r2=355036&view=diff
==
--- cfe/trunk/include/clang/Index/DeclOccurrence.h (original)
+++ cfe/trunk/include/clang/Index/DeclOccurrence.h Wed Feb 27 13:48:02 2019
@@ -1,9 +1,8 @@
-//===--- DeclOccurrence.h - An occurrence of a decl within a file 
-===//
+//===- DeclOccurrence.h - An occurrence of a decl within a file -*- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
 
@@ -39,4 +38,4 @@ struct DeclOccurrence {
 } // namespace index
 } // namespace clang
 
-#endif
+#endif // LLVM_CLANG_INDEX_DECLOCCURRENCE_H

Modified: cfe/trunk/lib/Index/FileIndexRecord.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/FileIndexRecord.cpp?rev=355036&r1=355035&r2=355036&view=diff
==
--- cfe/trunk/lib/Index/FileIndexRecord.cpp (original)
+++ cfe/trunk/lib/Index/FileIndexRecord.cpp Wed Feb 27 13:48:02 2019
@@ -1,4 +1,4 @@
-//===--- FileIndexRecord.cpp - Index data per file 
===//
+//===--- FileIndexRecord.cpp - Index data per file --*- C++ 
-*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -42,7 +42,7 @@ void FileIndexRecord::addDeclOccurence(S
 void FileIndexRecord::print(llvm::raw_ostream &OS) const {
   OS << "DECLS BEGIN ---\n";
   for (auto &DclInfo : Decls) {
-auto D = DclInfo.Dcl;
+const Decl *D = DclInfo.Dcl;
 SourceManager &SM = D->getASTContext().getSourceManager();
 SourceLocation Loc = SM.getFileLoc(D->getLocation());
 PresumedLoc PLoc = SM.getPresumedLoc(Loc);

Modified: cfe/trunk/lib/Index/FileIndexRecord.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Index/FileIndexRecord.h?rev=355036&r1=355035&r2=355036&view=diff
==
--- cfe/trunk/lib/Index/FileIndexRecord.h (original)
+++ cfe/trunk/lib/Index/FileIndexRecord.h Wed Feb 27 13:48:02 2019
@@ -1,9 +1,8 @@
-//===--- FileIndexRecord.h - Index data per file 
--===//
+//===--- FileIndexRecord.h - Index data per file *- C++ 
-*-===//
 //
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
 
@@ -55,4 +54,4 @@ public:
 } // end namespace index
 } // end namespace clang
 
-#endif
+#endif // LLVM_CLANG_LIB_INDEX_FILEINDEXRECORD_H


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

r355033 - Use Secure PLT as default on NetBSD/PowerPC.

[Joerg Sonnenberger via cfe-commits]

Author: joerg
Date: Wed Feb 27 13:46:01 2019
New Revision: 355033

URL: http://llvm.org/viewvc/llvm-project?rev=355033&view=rev
Log:
Use Secure PLT as default on NetBSD/PowerPC.

Modified:
cfe/trunk/lib/Driver/ToolChains/Arch/PPC.cpp
cfe/trunk/test/Driver/netbsd.c

Modified: cfe/trunk/lib/Driver/ToolChains/Arch/PPC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Arch/PPC.cpp?rev=355033&r1=355032&r2=355033&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Arch/PPC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Arch/PPC.cpp Wed Feb 27 13:46:01 2019
@@ -115,7 +115,7 @@ ppc::ReadGOTPtrMode ppc::getPPCReadGOTPt
   const ArgList &Args) {
   if (Args.getLastArg(options::OPT_msecure_plt))
 return ppc::ReadGOTPtrMode::SecurePlt;
-  if (Triple.isOSOpenBSD())
+  if (Triple.isOSNetBSD() || Triple.isOSOpenBSD())
 return ppc::ReadGOTPtrMode::SecurePlt;
   else
 return ppc::ReadGOTPtrMode::Bss;

Modified: cfe/trunk/test/Driver/netbsd.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/netbsd.c?rev=355033&r1=355032&r2=355033&view=diff
==
--- cfe/trunk/test/Driver/netbsd.c (original)
+++ cfe/trunk/test/Driver/netbsd.c Wed Feb 27 13:46:01 2019
@@ -446,3 +446,8 @@
 // PTHREAD-NOT: _POSIX_THREADS
 // PTHREAD: _REENTRANT
 // PTHREAD-NOT: _POSIX_THREADS
+
+// Check PowerPC for Secure PLT
+// RUN: %clang -target powerpc-unknown-netbsd -### -c %s 2>&1 \
+// RUN:   | FileCheck -check-prefix=POWERPC-SECUREPLT %s
+// POWERPC-SECUREPLT: "-target-feature" "+secure-plt"


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

[PATCH] D58737: [InstrProf] Use separate comdat group for data and counters

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added reviewers: xur, vsk.
Herald added subscribers: Sanitizers, cfe-commits, hiraditya, eraman.
Herald added projects: clang, Sanitizers, LLVM.

I hadn't realized that instrumentation runs before inlining, so we can't
use the function as the comdat group. Doing so can create relocations
against discarded sections when references to discarded __profc_
variables are inlined into functions outside the function's comdat
group.

In the future, perhaps we should consider standardizing the comdat group
names that ELF and COFF use. It will save object file size, since
__profv_$sym won't appear in the symbol table again.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58737

Files:
  clang/test/Profile/cxx-templates.cpp
  compiler-rt/test/profile/coverage-inline.cpp
  llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
  llvm/test/Instrumentation/InstrProfiling/PR23499.ll
  llvm/test/Instrumentation/InstrProfiling/comdat.ll

Index: llvm/test/Instrumentation/InstrProfiling/comdat.ll
===
--- llvm/test/Instrumentation/InstrProfiling/comdat.ll
+++ llvm/test/Instrumentation/InstrProfiling/comdat.ll
@@ -17,8 +17,8 @@
 
 ; ELF: @__profc_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_cnts", comdat($__profv_foo_inline), align 8
 ; ELF: @__profd_foo_inline = linkonce_odr hidden global{{.*}}, section "__llvm_prf_data", comdat($__profv_foo_inline), align 8
-; COFF: @__profc_foo_inline = internal global{{.*}}, section ".lprfc$M", comdat($foo_inline), align 8
-; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($foo_inline), align 8
+; COFF: @__profc_foo_inline = linkonce_odr dso_local global{{.*}}, section ".lprfc$M", comdat, align 8
+; COFF: @__profd_foo_inline = internal global{{.*}}, section ".lprfd$M", comdat($__profc_foo_inline), align 8
 define weak_odr void @foo_inline() comdat {
   call void @llvm.instrprof.increment(i8* getelementptr inbounds ([10 x i8], [10 x i8]* @__profn_foo_inline, i32 0, i32 0), i64 0, i32 1, i32 0)
   ret void
Index: llvm/test/Instrumentation/InstrProfiling/PR23499.ll
===
--- llvm/test/Instrumentation/InstrProfiling/PR23499.ll
+++ llvm/test/Instrumentation/InstrProfiling/PR23499.ll
@@ -20,8 +20,8 @@
 
 
 ; COFF-NOT: __profn__Z3barIvEvv
-; COFF: @__profc__Z3barIvEvv = internal global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat($_Z3barIvEvv), align 8
-; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($_Z3barIvEvv), align 8
+; COFF: @__profc__Z3barIvEvv = linkonce_odr dso_local global [1 x i64] zeroinitializer, section "{{.*}}prfc$M", comdat, align 8
+; COFF: @__profd__Z3barIvEvv = internal global { i64, i64, i64*, i8*, i8*, i32, [2 x i16] } { i64 4947693190065689389, i64 0, i64* getelementptr inbounds ([1 x i64], [1 x i64]* @__profc__Z3barIvEvv, i32 0, i32 0), i8*{{.*}}, i8* null, i32 1, [2 x i16] zeroinitializer }, section "{{.*}}prfd{{.*}}", comdat($__profc__Z3barIvEvv), align 8
 
 
 declare void @llvm.instrprof.increment(i8*, i64, i32, i32) #1
Index: llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
===
--- llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
+++ llvm/lib/Transforms/Instrumentation/InstrProfiling.cpp
@@ -743,25 +743,20 @@
 
   // Move the name variable to the right section. Place them in a COMDAT group
   // if the associated function is a COMDAT. This will make sure that only one
-  // copy of counters of the COMDAT function will be emitted after linking.
+  // copy of counters of the COMDAT function will be emitted after linking. Keep
+  // in mind that this pass runs before the inliner, so we need to create a new
+  // comdat group for the counters and profiling data. If we use the comdat of
+  // the parent function, that will result in relocations against discarded
+  // sections.
   Comdat *Cmdt = nullptr;
   GlobalValue::LinkageTypes CounterLinkage = Linkage;
   if (needsComdatForCounter(*Fn, *M)) {
 if (TT.isOSBinFormatCOFF()) {
-  // There are two cases that need a comdat on COFF:
-  // 1. Functions that already have comdats (standard case)
-  // 2. available_externally functions (dllimport and C99 inline)
-  // In the first case, put all the data in the original function comdat. In
-  // the second case, create a new comdat group using the counter as the
-  // leader. It's linkage must be external, so use linkonce_odr linkage in
-  // that case.
-  if (Comdat *C = Fn->getComdat()) {
-Cmdt = C;
-  } else {
-Cmdt = M->getOrInsertComdat(
-getVar

[PATCH] D58478: [index-while-building] FileIndexRecord

[Nathan Hawes via Phabricator via cfe-commits]

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

Looks good to me.


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

https://reviews.llvm.org/D58478



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Please resubmit this patch with full context

  git diff --cached -U99 > patch_to_submitt.diff

You need to add some documention describing the new option into the check in

docs/clang-tidy/checks/modernize .rst


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58731



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExprConstant.cpp:1793
+ "this boxed expression can't be emitted as a compile-time constant");
+  return emitConstantObjCStringLiteral(cast(E->getSubExpr()),
+   E->getType(), CGM);

`IgnoreParens`.



Comment at: lib/Sema/SemaExprObjC.cpp:534
+NSStringPointer, NSStringPointer);
+return new (Context) ObjCBoxedExpr(SL, BoxedType, nullptr, SR);
+  }

You're implicitly dropping sugar from the source expression here; I really 
think you should preserve that, and if it means you end up having to look 
through array-decay expressions, that's not the end of the world.

The need for a special case here is just to allow us to compile these even if 
there isn't a boxing method available?

Do you need to restrict the type of the string literal so that it doesn't apply 
to e.g. wide strings?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

r355027 - [OPENMP]Delay emission of the error for unsupported types.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Wed Feb 27 12:29:45 2019
New Revision: 355027

URL: http://llvm.org/viewvc/llvm-project?rev=355027&view=rev
Log:
[OPENMP]Delay emission of the error for unsupported types.

If the type is unsupported on the device side, it still must be emitted,
but we should emit errors for operations with such types.

Added:
cfe/trunk/test/OpenMP/nvptx_unsupported_type_codegen.cpp
cfe/trunk/test/OpenMP/nvptx_unsupported_type_messages.cpp
Modified:
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/ASTContext.cpp
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/lib/Sema/SemaType.cpp

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=355027&r1=355026&r2=355027&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Wed Feb 27 12:29:45 2019
@@ -8805,6 +8805,10 @@ private:
   /// Check whether we're allowed to call Callee from the current function.
   void checkOpenMPDeviceFunction(SourceLocation Loc, FunctionDecl *Callee);
 
+  /// Check if the expression is allowed to be used in expressions for the
+  /// OpenMP devices.
+  void checkOpenMPDeviceExpr(const Expr *E);
+
   /// Checks if a type or a declaration is disabled due to the owning extension
   /// being disabled, and emits diagnostic messages if it is disabled.
   /// \param D type or declaration to be checked.

Modified: cfe/trunk/lib/AST/ASTContext.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=355027&r1=355026&r2=355027&view=diff
==
--- cfe/trunk/lib/AST/ASTContext.cpp (original)
+++ cfe/trunk/lib/AST/ASTContext.cpp Wed Feb 27 12:29:45 2019
@@ -1902,8 +1902,16 @@ TypeInfo ASTContext::getTypeInfoImpl(con
   break;
 case BuiltinType::Float16:
 case BuiltinType::Half:
-  Width = Target->getHalfWidth();
-  Align = Target->getHalfAlign();
+  if (Target->hasFloat16Type() || !getLangOpts().OpenMP ||
+  !getLangOpts().OpenMPIsDevice) {
+Width = Target->getHalfWidth();
+Align = Target->getHalfAlign();
+  } else {
+assert(getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice &&
+   "Expected OpenMP device compilation.");
+Width = AuxTarget->getHalfWidth();
+Align = AuxTarget->getHalfAlign();
+  }
   break;
 case BuiltinType::Float:
   Width = Target->getFloatWidth();
@@ -1918,8 +1926,16 @@ TypeInfo ASTContext::getTypeInfoImpl(con
   Align = Target->getLongDoubleAlign();
   break;
 case BuiltinType::Float128:
-  Width = Target->getFloat128Width();
-  Align = Target->getFloat128Align();
+  if (Target->hasFloat128Type() || !getLangOpts().OpenMP ||
+  !getLangOpts().OpenMPIsDevice) {
+Width = Target->getFloat128Width();
+Align = Target->getFloat128Align();
+  } else {
+assert(getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice &&
+   "Expected OpenMP device compilation.");
+Width = AuxTarget->getFloat128Width();
+Align = AuxTarget->getFloat128Align();
+  }
   break;
 case BuiltinType::NullPtr:
   Width = Target->getPointerWidth(0); // C++ 3.9.1p11: sizeof(nullptr_t)

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=355027&r1=355026&r2=355027&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Wed Feb 27 12:29:45 2019
@@ -6268,10 +6268,56 @@ private:
   static void addNVVMMetadata(llvm::Function *F, StringRef Name, int Operand);
 };
 
+/// Checks if the type is unsupported directly by the current target.
+static bool isUnsupportedType(ASTContext &Context, QualType T) {
+  if (!Context.getTargetInfo().hasFloat16Type() && T->isFloat16Type())
+return true;
+  if (!Context.getTargetInfo().hasFloat128Type() && T->isFloat128Type())
+return true;
+  if (!Context.getTargetInfo().hasInt128Type() && T->isIntegerType() &&
+  Context.getTypeSize(T) > 64)
+return true;
+  if (const auto *AT = T->getAsArrayTypeUnsafe())
+return isUnsupportedType(Context, AT->getElementType());
+  const auto *RT = T->getAs();
+  if (!RT)
+return false;
+  const RecordDecl *RD = RT->getDecl();
+
+  // If this is a C++ record, check the bases first.
+  if (const CXXRecordDecl *CXXRD = dyn_cast(RD))
+for (const CXXBaseSpecifier &I : CXXRD->bases())
+  if (isUnsupportedType(Context, I.getType()))
+return true;
+
+  for (const FieldDecl *I : RD->fields())
+if (isUnsupportedType(Context, I->getType()))
+  return true;
+  return fa

[PATCH] D58478: [index-while-building] FileIndexRecord

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

ping -c 1


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

https://reviews.llvm.org/D58478



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

[PATCH] D58137: [clang-tidy] Add the abseil-time-subtraction check

[Hyrum Wright via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE355024: [clang-tidy] Add the abseil-time-subtraction check 
(authored by hwright, committed by ).

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58137

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/DurationRewriter.cpp
  clang-tidy/abseil/DurationRewriter.h
  clang-tidy/abseil/TimeSubtractionCheck.cpp
  clang-tidy/abseil/TimeSubtractionCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-time-subtraction.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/time/time.h
  test/clang-tidy/abseil-time-subtraction.cpp

Index: test/clang-tidy/abseil-time-subtraction.cpp
===
--- test/clang-tidy/abseil-time-subtraction.cpp
+++ test/clang-tidy/abseil-time-subtraction.cpp
@@ -0,0 +1,117 @@
+// RUN: %check_clang_tidy %s abseil-time-subtraction %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void g(absl::Duration d);
+
+void f() {
+  absl::Time t;
+  int x, y;
+  absl::Duration d;
+
+  d = absl::Hours(absl::ToUnixHours(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixHours(x));
+  d = absl::Minutes(absl::ToUnixMinutes(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixMinutes(x));
+  d = absl::Seconds(absl::ToUnixSeconds(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixSeconds(x));
+  d = absl::Milliseconds(absl::ToUnixMillis(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixMillis(x));
+  d = absl::Microseconds(absl::ToUnixMicros(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixMicros(x));
+  d = absl::Nanoseconds(absl::ToUnixNanos(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixNanos(x));
+
+  y = x - absl::ToUnixHours(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Hours(absl::FromUnixHours(x) - t);
+  y = x - absl::ToUnixMinutes(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Minutes(absl::FromUnixMinutes(x) - t);
+  y = x - absl::ToUnixSeconds(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Seconds(absl::FromUnixSeconds(x) - t);
+  y = x - absl::ToUnixMillis(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Milliseconds(absl::FromUnixMillis(x) - t);
+  y = x - absl::ToUnixMicros(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Microseconds(absl::FromUnixMicros(x) - t);
+  y = x - absl::ToUnixNanos(t);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: y = absl::ToInt64Nanoseconds(absl::FromUnixNanos(x) - t);
+
+  // Check parenthesis placement
+  d = 5 * absl::Seconds(absl::ToUnixSeconds(t) - x);
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = 5 * (t - absl::FromUnixSeconds(x));
+  d = absl::Seconds(absl::ToUnixSeconds(t) - x) / 5;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: d = (t - absl::FromUnixSeconds(x)) / 5;
+
+  // No extra parens around arguments
+  g(absl::Seconds(absl::ToUnixSeconds(t) - x));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: g(t - absl::FromUnixSeconds(x));
+  g(absl::Seconds(x - absl::ToUnixSeconds(t)));
+  // CHECK-MESSAGES: [[@LINE-1]]:5: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK-FIXES: g(absl::FromUnixSeconds(x) - t);
+
+  // More complex subexpressions
+  d = absl::Hours(absl::ToUnixHours(t) - 5 * x);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform subtraction in the time domain [abseil-time-subtraction]
+  // CHECK

[clang-tools-extra] r355024 - [clang-tidy] Add the abseil-time-subtraction check

[Hyrum Wright via cfe-commits]

Author: hwright
Date: Wed Feb 27 12:08:50 2019
New Revision: 355024

URL: http://llvm.org/viewvc/llvm-project?rev=355024&view=rev
Log:
[clang-tidy] Add the abseil-time-subtraction check

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

Added:
clang-tools-extra/trunk/clang-tidy/abseil/TimeSubtractionCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/TimeSubtractionCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-time-subtraction.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-time-subtraction.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.h
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/time/time.h

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=355024&r1=355023&r2=355024&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Wed Feb 27 
12:08:50 2019
@@ -23,6 +23,7 @@
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
+#include "TimeSubtractionCheck.h"
 #include "UpgradeDurationConversionsCheck.h"
 
 namespace clang {
@@ -59,6 +60,8 @@ public:
 "abseil-str-cat-append");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
+CheckFactories.registerCheck(
+"abseil-time-subtraction");
 CheckFactories.registerCheck(
 "abseil-upgrade-duration-conversions");
   }

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=355024&r1=355023&r2=355024&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Wed Feb 27 
12:08:50 2019
@@ -17,6 +17,7 @@ add_clang_library(clangTidyAbseilModule
   RedundantStrcatCallsCheck.cpp
   StrCatAppendCheck.cpp
   StringFindStartswithCheck.cpp
+  TimeSubtractionCheck.cpp
   UpgradeDurationConversionsCheck.cpp
 
   LINK_LIBS

Modified: clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp?rev=355024&r1=355023&r2=355024&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/DurationRewriter.cpp Wed Feb 27 
12:08:50 2019
@@ -84,6 +84,22 @@ rewriteInverseDurationCall(const MatchFi
   return llvm::None;
 }
 
+/// If `Node` is a call to the inverse of `Scale`, return that inverse's
+/// argument, otherwise None.
+static llvm::Optional
+rewriteInverseTimeCall(const MatchFinder::MatchResult &Result,
+   DurationScale Scale, const Expr &Node) {
+  llvm::StringRef InverseFunction = getTimeInverseForScale(Scale);
+  if (const auto *MaybeCallArg = selectFirst(
+  "e", match(callExpr(callee(functionDecl(hasName(InverseFunction))),
+  hasArgument(0, expr().bind("e"))),
+ Node, *Result.Context))) {
+return tooling::fixit::getText(*MaybeCallArg, *Result.Context).str();
+  }
+
+  return llvm::None;
+}
+
 /// Returns the factory function name for a given `Scale`.
 llvm::StringRef getDurationFactoryForScale(DurationScale Scale) {
   switch (Scale) {
@@ -103,6 +119,24 @@ llvm::StringRef getDurationFactoryForSca
   llvm_unreachable("unknown scaling factor");
 }
 
+llvm::StringRef getTimeFactoryForScale(DurationScale Scale) {
+  switch (Scale) {
+  case DurationScale::Hours:
+return "absl::FromUnixHours";
+  case DurationScale::Minutes:
+return "absl::FromUnixMinutes";
+  case DurationScale::Seconds:
+return "absl::FromUnixSeconds";
+  case DurationScale::Milliseconds:
+return "absl::FromUnixMillis";
+  case DurationScale::Microseconds:
+return "absl::FromUnixMicros";
+  case DurationScale::Nanoseconds:
+return "absl::FromUnixNanos";
+  }
+  llvm_unreachable("unknown scaling factor");
+}
+
 /// Returns the Time factory function name for a given `Scale`.
 llvm::StringRef getTimeInverseForScale(DurationScale scale) {
   switch (scale) {
@@ -250,6 +284,24 @@ std::string rewriteExprFromNumberToDurat
   .str();
 }
 
+std::string rewriteExprFromNumberToTime(
+c

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

I think this change is worth mentioning in the release notes as well 
(cte/docs/ReleaseNotes.rst)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58731



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Welcome to the LLVM community and thank you for the patch lewmpk!

Please add unit tests for the changes you made to the check. They live in 
`clang-tools-extra/test/clang-tidy/modernize-`.
It is common to add a additional test-file to ensure the configuration options 
do work as expected.
You can get inspiration from other checks that provide configuration capability 
(e.g. 
https://clang.llvm.org/extra/clang-tidy/checks/modernize-use-bool-literals.html#options)
for reference.

If you have any questions don't hesitate to ask!

P.S: Usually we upload the patches with full context 
(https://llvm.org/docs/DeveloperPolicy.html#making-and-submitting-a-patch) for 
the reviewer to see the change in context.




Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:23
+  if (getLangOpts().CPlusPlus11) {
+if (IgnoreDestructors) {
+  Finder->addMatcher(

Please ellide the braces for the single-statements (this `if` and the `else` 
branch)



Comment at: clang-tidy/modernize/UseOverrideCheck.h:23
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;

That requires additional methods for the function for the configuration to 
function properly. Please see other checks and implement that the same way.



Comment at: clang-tidy/modernize/UseOverrideCheck.h:27
+
+protected:
+  const bool IgnoreDestructors;

please use private instead


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58731



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

[PATCH] D58731: [clang-tidy] added cppcoreguidelines-explicit-virtual-functions

[Lewis Clark via Phabricator via cfe-commits]

lewmpk created this revision.
lewmpk added reviewers: alexfh, alexfh_.
Herald added subscribers: cfe-commits, kbarton, xazax.hun, nemanjai.
Herald added a project: clang.

Addresses the bugzilla bug #30397.
modernize-use-override suggests that destructors require the override specifier 
and the CPP core guidelines do not recommend this.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58731

Files:
  clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tidy/modernize/UseOverrideCheck.cpp
  clang-tidy/modernize/UseOverrideCheck.h


Index: clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,13 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+protected:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -19,8 +19,16 @@
 
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
-Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+  if (getLangOpts().CPlusPlus11) {
+if (IgnoreDestructors) {
+  Finder->addMatcher(
+  cxxMethodDecl(isOverride(), unless(cxxDestructorDecl()))
+  .bind("method"),
+  this);
+} else {
+  Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+}
+  }
 }
 
 // Re-lex the tokens to get precise locations to insert 'override' and remove
Index: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
===
--- clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
+++ clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../misc/NonPrivateMemberVariablesInClassesCheck.h"
 #include "../misc/UnconventionalAssignOperatorCheck.h"
 #include "../modernize/AvoidCArraysCheck.h"
+#include "../modernize/UseOverrideCheck.h"
 #include "../readability/MagicNumbersCheck.h"
 #include "AvoidGotoCheck.h"
 #include "InterfacesGlobalInitCheck.h"
@@ -46,6 +47,8 @@
 "cppcoreguidelines-avoid-goto");
 CheckFactories.registerCheck(
 "cppcoreguidelines-avoid-magic-numbers");
+CheckFactories.registerCheck(
+"cppcoreguidelines-explicit-virtual-functions");
 CheckFactories.registerCheck(
 "cppcoreguidelines-interfaces-global-init");
 CheckFactories.registerCheck(
@@ -91,6 +94,9 @@
 Opts["cppcoreguidelines-non-private-member-variables-in-classes."
  "IgnoreClassesWithAllMemberVariablesBeingPublic"] = "1";
 
+Opts["cppcoreguidelines-explicit-virtual-functions."
+ "IgnoreDestructors"] = "1";
+
 return Options;
   }
 };


Index: clang-tidy/modernize/UseOverrideCheck.h
===
--- clang-tidy/modernize/UseOverrideCheck.h
+++ clang-tidy/modernize/UseOverrideCheck.h
@@ -19,9 +19,13 @@
 class UseOverrideCheck : public ClangTidyCheck {
 public:
   UseOverrideCheck(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreDestructors(Options.get("IgnoreDestructors", false)) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+protected:
+  const bool IgnoreDestructors;
 };
 
 } // namespace modernize
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -19,8 +19,16 @@
 
 void UseOverrideCheck::registerMatchers(MatchFinder *Finder) {
   // Only register the matcher for C++11.
-  if (getLangOpts().CPlusPlus11)
-Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+  if (getLangOpts().CPlusPlus11) {
+if (IgnoreDestructors) {
+  Finder->addMatcher(
+  cxxMethodDecl(isOverride(), unless(cxxDestructorDecl()))
+  .bind("method"),
+  this);
+} else {
+  Finder->addMatcher(cxxMethodDecl(isOverride()).bind("method"), this);
+}
+  }
 }
 
 // Re-lex the tokens to get precise locations to insert 'override' and remove
Index: clang-tidy/cppcoreguidelines

[PATCH] D58666: [OpenCL] Undefine cl_intel_planar_yuv extension

[Dmitry Sidorov via Phabricator via cfe-commits]

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



Comment at: test/SemaOpenCL/extension-begin.cl:26
+
 #ifndef IMPLICIT_INCLUDE
 #include "extension-begin.h"

Anastasia wrote:
> sidorovd wrote:
> > Anastasia wrote:
> > > Can we also test that macro `my_ext` is not defined here but defined 
> > > above?
> > > 
> > > It seems we are not testing anything like this...
> > #pragma OPENCL EXTENSION my_ext : begin doesn't define an appropriate 
> > macro. And so cl-ext=+my_ext.
> But don't you need to expose the definition of it?
Certainly I need, but now the only proper way to do so is by adding an 
extension via adding it in OpenCLExtensions.def. Previously we decided to avoid 
adding an extension directly into clang, so with a new approach I'd prefer not 
to add a definition of the macro in the header but define it somewhere else, 
otherwise the macro becomes defined  where it's not supposed to be (even for 
ARM and AMD =) ). 


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

https://reviews.llvm.org/D58666



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

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

xbolva00 added a comment.

Maybe move it to LLVM? As this watcher may be useful for other projects in the 
future?


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

https://reviews.llvm.org/D58418



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

[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

[Jan Korous via Phabricator via cfe-commits]

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

- moved checks for CoreServices/inotify to cmake


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

https://reviews.llvm.org/D58418

Files:
  clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
  clang/lib/CMakeLists.txt
  clang/lib/DirectoryWatcher/CMakeLists.txt
  clang/lib/DirectoryWatcher/Config.inc.in
  clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h
  clang/lib/DirectoryWatcher/DirectoryWatcher-mac.inc.h
  clang/lib/DirectoryWatcher/DirectoryWatcher.cpp
  clang/unittests/CMakeLists.txt
  clang/unittests/DirectoryWatcher/CMakeLists.txt
  clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp

Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===
--- /dev/null
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -0,0 +1,327 @@
+//===- unittests/DirectoryWatcher/DirectoryWatcherTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/DirectoryWatcher/DirectoryWatcher.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+
+using namespace llvm;
+using namespace llvm::sys;
+using namespace llvm::sys::fs;
+using namespace clang;
+
+namespace {
+
+class EventCollection {
+  SmallVector Events;
+
+public:
+  EventCollection() = default;
+  explicit EventCollection(ArrayRef events) {
+append(events);
+  }
+
+  void append(ArrayRef events) {
+Events.append(events.begin(), events.end());
+  }
+
+  bool empty() const { return Events.empty(); }
+  size_t size() const { return Events.size(); }
+  void clear() { Events.clear(); }
+
+  bool hasEvents(ArrayRef filenames,
+ ArrayRef kinds,
+ ArrayRef stats) const {
+assert(filenames.size() == kinds.size());
+assert(filenames.size() == stats.size());
+SmallVector evts = Events;
+bool hadError = false;
+for (unsigned i = 0, e = filenames.size(); i < e; ++i) {
+  StringRef fname = filenames[i];
+  DirectoryWatcher::EventKind kind = kinds[i];
+  auto it = std::find_if(evts.begin(), evts.end(),
+ [&](const DirectoryWatcher::Event &evt) -> bool {
+   return path::filename(evt.Filename) == fname;
+ });
+  if (it == evts.end()) {
+hadError = err(Twine("expected filename '" + fname + "' not found"));
+continue;
+  }
+  if (it->Kind != kind) {
+hadError = err(Twine("filename '" + fname + "' has event kind " +
+ std::to_string((int)it->Kind) + ", expected ") +
+   std::to_string((int)kind));
+  }
+  evts.erase(it);
+}
+for (const auto &evt : evts) {
+  hadError = err(Twine("unexpected filename '" +
+   path::filename(evt.Filename) + "' found"));
+}
+return !hadError;
+  }
+
+  bool hasAdded(ArrayRef filenames,
+ArrayRef stats) const {
+std::vector kinds{
+filenames.size(), DirectoryWatcher::EventKind::Added};
+return hasEvents(filenames, kinds, stats);
+  }
+
+  bool hasRemoved(ArrayRef filenames) const {
+std::vector kinds{
+filenames.size(), DirectoryWatcher::EventKind::Removed};
+std::vector stats{filenames.size(), file_status{}};
+return hasEvents(filenames, kinds, stats);
+  }
+
+private:
+  bool err(Twine msg) const {
+SmallString<128> buf;
+llvm::errs() << msg.toStringRef(buf) << '\n';
+return true;
+  }
+};
+
+struct EventOccurrence {
+  std::vector Events;
+  bool IsInitial;
+};
+
+class DirectoryWatcherTest
+: public std::enable_shared_from_this {
+  std::string WatchedDir;
+  std::string TempDir;
+  std::unique_ptr DirWatcher;
+
+  std::condition_variable Condition;
+  std::mutex Mutex;
+  std::deque EvtOccurs;
+
+public:
+  void init() {
+SmallString<128> pathBuf;
+std::error_code EC = createUniqueDirectory("dirwatcher", pathBuf);
+ASSERT_FALSE(EC);
+TempDir = pathBuf.str();
+path::append(pathBuf, "watch");
+WatchedDir = pathBuf.str();
+EC = create_directory(WatchedDir);
+ASSERT_FALSE(EC);
+  }
+
+  ~DirectoryWatcherTest() {
+stopWatching();
+remove_directories(TempDir);
+  }
+
+public:
+  StringRef getWatchedDir() const { return WatchedDir; }
+
+  void addFile(StringRef filename, file_status &stat) {
+SmallString<128> pathBuf;
+pathBuf = TempDir;
+path::append(pathBuf, filename);
+Expected ft =
+openNativeFileForWrite(pathBuf, CD_CreateNew, OF_None);
+ASSERT_TRUE((bool)ft);
+closeFile(*ft);
+

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:193
   RawStringFormat.Language, &PredefinedStyle)) {
-PredefinedStyle = getLLVMStyle();
+PredefinedStyle = getLLVMStyle(FormatStyle::LK_Cpp);
 PredefinedStyle.Language = RawStringFormat.Language;

MyDeveloperDay wrote:
> if you've set up a default argument of LK_Cpp do you need to keep supplying 
> it? if your going to supply it everywhere don't make it a default argument or 
> vice versa
> 
> This might make a much smaller patch, a little more digestable and all the 
> tests can remain as they are.
My goal was to fix all the in-tree callers but leave it as an optional arg (at 
least initially) so as to not break out of tree callers. I reduced it to what I 
actually need for this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56943



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

[PATCH] D56943: [clang-format][NFC] Allow getLLVMStyle() to take a language

[Jordan Rupprecht via Phabricator via cfe-commits]

rupprecht updated this revision to Diff 188584.
rupprecht marked 2 inline comments as done.
rupprecht added a comment.

Revert getLLVMStyle(LK_Cpp) fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56943

Files:
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -120,6 +120,15 @@
   EXPECT_EQ("a\n#b c d\ne", test::messUp("a\n#b\\\nc\\\nd\ne"));
 }
 
+TEST_F(FormatTest, DefaultLLVMStyleIsCpp) {
+  EXPECT_EQ(FormatStyle::LK_Cpp, getLLVMStyle().Language);
+}
+
+TEST_F(FormatTest, LLVMStyleOverride) {
+  EXPECT_EQ(FormatStyle::LK_Proto,
+getLLVMStyle(FormatStyle::LK_Proto).Language);
+}
+
 
//===--===//
 // Basic function tests.
 
//===--===//
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -618,9 +618,9 @@
   return Expanded;
 }
 
-FormatStyle getLLVMStyle() {
+FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   FormatStyle LLVMStyle;
-  LLVMStyle.Language = FormatStyle::LK_Cpp;
+  LLVMStyle.Language = Language;
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
@@ -729,8 +729,7 @@
 return GoogleStyle;
   }
 
-  FormatStyle GoogleStyle = getLLVMStyle();
-  GoogleStyle.Language = Language;
+  FormatStyle GoogleStyle = getLLVMStyle(Language);
 
   GoogleStyle.AccessModifierOffset = -1;
   GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
@@ -2344,8 +2343,7 @@
   if (!FS) {
 FS = llvm::vfs::getRealFileSystem().get();
   }
-  FormatStyle Style = getLLVMStyle();
-  Style.Language = guessLanguage(FileName, Code);
+  FormatStyle Style = getLLVMStyle(guessLanguage(FileName, Code));
 
   FormatStyle FallbackStyle = getNoStyle();
   if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1849,7 +1849,8 @@
 
 /// Returns a format style complying with the LLVM coding standards:
 /// http://llvm.org/docs/CodingStandards.html.
-FormatStyle getLLVMStyle();
+FormatStyle getLLVMStyle(
+FormatStyle::LanguageKind Language = FormatStyle::LanguageKind::LK_Cpp);
 
 /// Returns a format style complying with one of Google's style guides:
 /// http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml.


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -120,6 +120,15 @@
   EXPECT_EQ("a\n#b c d\ne", test::messUp("a\n#b\\\nc\\\nd\ne"));
 }
 
+TEST_F(FormatTest, DefaultLLVMStyleIsCpp) {
+  EXPECT_EQ(FormatStyle::LK_Cpp, getLLVMStyle().Language);
+}
+
+TEST_F(FormatTest, LLVMStyleOverride) {
+  EXPECT_EQ(FormatStyle::LK_Proto,
+getLLVMStyle(FormatStyle::LK_Proto).Language);
+}
+
 //===--===//
 // Basic function tests.
 //===--===//
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -618,9 +618,9 @@
   return Expanded;
 }
 
-FormatStyle getLLVMStyle() {
+FormatStyle getLLVMStyle(FormatStyle::LanguageKind Language) {
   FormatStyle LLVMStyle;
-  LLVMStyle.Language = FormatStyle::LK_Cpp;
+  LLVMStyle.Language = Language;
   LLVMStyle.AccessModifierOffset = -2;
   LLVMStyle.AlignEscapedNewlines = FormatStyle::ENAS_Right;
   LLVMStyle.AlignAfterOpenBracket = FormatStyle::BAS_Align;
@@ -729,8 +729,7 @@
 return GoogleStyle;
   }
 
-  FormatStyle GoogleStyle = getLLVMStyle();
-  GoogleStyle.Language = Language;
+  FormatStyle GoogleStyle = getLLVMStyle(Language);
 
   GoogleStyle.AccessModifierOffset = -1;
   GoogleStyle.AlignEscapedNewlines = FormatStyle::ENAS_Left;
@@ -2344,8 +2343,7 @@
   if (!FS) {
 FS = llvm::vfs::getRealFileSystem().get();
   }
-  FormatStyle Style = getLLVMStyle();
-  Style.Language = guessLanguage(FileName, Code);
+  FormatStyle Style = getLLVMStyle(guessLanguage(FileName, Code));
 
   FormatStyle FallbackStyle = getNoStyle();
   if (!getPredefinedStyle(FallbackStyleName, Style.Language, &FallbackStyle))
Index: clang/include/clang/Format/F

[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 does it make sense to warn for this case only in C mode? Since 
this case in C++ is handled by -Wnarrowing, as we see from tests.


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

https://reviews.llvm.org/D52835



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: rjmccall, arphaman.
ahatanak added a project: clang.
Herald added subscribers: jdoerfert, dexonsmith, jkorous.

clang currently emits an expression like `@("abc")` as a message send to 
`stringWithUTF8String`. This patch makes changes so that a compile-time 
constant is emitted instead when the expression inside the parentheses is a 
string literal. I think it's possible to do the same optimization for constant 
expressions that evaluate to zero-terminated strings (see the example below), 
but I'm leaving that for future work.

  constexpr const char *foo() { return "abc"; }
  
  void test() {
NSString *s = @(foo());
  }

The original motivation for the patch is to silence the 
`-Wnullable-to-nonnull-conversion` warning that clang started emitting after 
r317727:
The https://oleb.net/2018/@keypath/.

rdar://problem/42684601


Repository:
  rC Clang

https://reviews.llvm.org/D58729

Files:
  include/clang/AST/ExprObjC.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/boxing.m
  test/SemaObjC/objc-literal-sig.m
  test/SemaObjC/transfer-boxed-string-nullability.m
  test/SemaObjCXX/literals.mm

Index: test/SemaObjCXX/literals.mm
===
--- test/SemaObjCXX/literals.mm
+++ test/SemaObjCXX/literals.mm
@@ -50,6 +50,9 @@
 + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt;
 @end
 
+@interface NSString
+@end
+
 template
 struct ConvertibleTo {
   operator T();
@@ -185,3 +188,8 @@
 void test_dictionary_colon() {
   id dict = @{ key : value };
 }
+
+void testConstExpr() {
+  constexpr NSString *t0 = @"abc";
+  constexpr NSString *t1 = @("abc");
+}
Index: test/SemaObjC/transfer-boxed-string-nullability.m
===
--- test/SemaObjC/transfer-boxed-string-nullability.m
+++ test/SemaObjC/transfer-boxed-string-nullability.m
@@ -16,6 +16,10 @@
 void takesNonNull(NSString * _Nonnull ptr);
 
 void testBoxedString() {
+  // No diagnostic emitted as this doesn't need a stringWithUTF8String message
+  // send.
+  takesNonNull(@("hey"));
+
   const char *str = "hey";
   takesNonNull([NSString stringWithUTF8String:str]);
   takesNonNull(@(str));
Index: test/SemaObjC/objc-literal-sig.m
===
--- test/SemaObjC/objc-literal-sig.m
+++ test/SemaObjC/objc-literal-sig.m
@@ -39,6 +39,8 @@
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
@@ -46,6 +48,6 @@
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
-  id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
-  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }
Index: test/CodeGenObjC/boxing.m
===
--- test/CodeGenObjC/boxing.m
+++ test/CodeGenObjC/boxing.m
@@ -53,6 +53,9 @@
 + (id)stringWithUTF8String:(const char *)nullTerminatedCString;
 @end
 
+// CHECK: [[V0:%.*]] = type opaque
+// CHECK: [[STRUCT_NSCONSTANT_STRING_TAG:%.*]] = type { i32*, i32, i8*, i64 }
+
 // CHECK: [[WithIntMeth:@.*]] = private unnamed_addr constant [15 x i8] c"numberWithInt:\00"
 // CHECK: [[WithIntSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([15 x i8], [15 x i8]* [[WithIntMeth]]
 // CHECK: [[WithCharMeth:@.*]] = private unnamed_addr constant [16 x i8] c"numberWithChar:\00"
@@ -65,8 +68,12 @@
 // CHECK: [[WithUnsignedIntegerSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds ([27 x i8], [27 x i8]* [[WithUnsignedIntegerMeth]]
 // CHECK: [[stringWithUTF8StringMeth:@.*]] = private unnamed_addr constant [22 x i8] c"stringWithUTF8String:\00"
 // CHECK: [[stringWithUTF8StringSEL:@.*]] = private externally_initialized global i8* getelementptr inbounds (

[PATCH] D57898: CodeGen: Fix PR40605

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

Can you change the title to be more descriptive?

Small test fixes, otherwise this LGTM, will check box after update.




Comment at: clang/test/CodeGenCXX/auto-var-init.cpp:123
+// PATTERN-NOT-O1: @__const.test_int1_custom.custom
+// ZERO-NOT-O1: @__const.test_int1_custom.custom
+


`-NOT` is in the wrong place above.



Comment at: clang/test/CodeGenCXX/auto-var-init.cpp:130
+// PATTERN-NOT-O1: @__const.test_bool4_custom.custom
+// ZERO-NOT-O1: @__const.test_bool4_custom.custom
+

`-NOT` is in the wrong place above.


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

https://reviews.llvm.org/D57898



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

r355017 - Add triples to the test I committed in r355012 to fix windows bots.

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Wed Feb 27 10:59:52 2019
New Revision: 355017

URL: http://llvm.org/viewvc/llvm-project?rev=355017&view=rev
Log:
Add triples to the test I committed in r355012 to fix windows bots.

Modified:
cfe/trunk/test/PCH/arc-blocks.mm

Modified: cfe/trunk/test/PCH/arc-blocks.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/arc-blocks.mm?rev=355017&r1=355016&r2=355017&view=diff
==
--- cfe/trunk/test/PCH/arc-blocks.mm (original)
+++ cfe/trunk/test/PCH/arc-blocks.mm Wed Feb 27 10:59:52 2019
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fobjc-arc -fblocks -std=c++1y -emit-pch %s -o %t
-// RUN: %clang_cc1 -fobjc-arc -fblocks -std=c++1y -include-pch %t -emit-llvm 
-o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-arc -fblocks 
-std=c++1y -emit-pch %s -o %t
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-arc -fblocks 
-std=c++1y -include-pch %t -emit-llvm -o - %s | FileCheck %s
 
 #ifndef HEADER_INCLUDED
 #define HEADER_INCLUDED


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

[libclc] r355016 - Creating release candidate rc3 from release_800 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Feb 27 10:49:24 2019
New Revision: 355016

URL: http://llvm.org/viewvc/llvm-project?rev=355016&view=rev
Log:
Creating release candidate rc3 from release_800 branch

Added:
libclc/tags/RELEASE_800/rc3/
  - copied from r355015, libclc/branches/release_80/

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

[libunwind] r355016 - Creating release candidate rc3 from release_800 branch

[Hans Wennborg via cfe-commits]

Author: hans
Date: Wed Feb 27 10:49:24 2019
New Revision: 355016

URL: http://llvm.org/viewvc/llvm-project?rev=355016&view=rev
Log:
Creating release candidate rc3 from release_800 branch

Added:
libunwind/tags/RELEASE_800/rc3/
  - copied from r355015, libunwind/branches/release_80/

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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: test/SemaOpenCL/address-spaces.cl:87
+
+  // FIXME: This doesn't seem right. This should be an error, not a warning.
+  __local int * __global * __private * lll;

Anastasia wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > Are we sure it has to be an error? May be we can change this wording to 
> > > something like - unknown behavior that needs clarifying? Some similar 
> > > text to your comment in the code.
> > Well... If `__private int **` to `__generic int **` is an error but 
> > `__private int ***` to `__generic int **` isn't, that seems a bit odd to 
> > me...
> > 
> Ok, I am still confused about this case because it's converting number of 
> pointers. It's not exactly the same to me as converting address spaces of 
> pointers. Since `__generic` is default addr space in OpenCL, in your example 
> following 2 inner address spaces seem to match:
> `__private int * __generic* __generic*` -> `__generic int * __generic*`
> 
> But in any case keeping FIXME is definitely the right thing to do here until 
> we come up with some sort of rules.
For reasons that have always been obscure to me, Clang is very permissive about 
pointer-type changes in C, far more than the C standard actually requires.  
It's hard to change that in general, but I think it's fine to harden specific 
new cases like nested address-space conversions.


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

https://reviews.llvm.org/D58236



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

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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Generally, with an explicit cast, C allows any pointer cast with a reasonable 
interpretation, even if the underlying operation is suspicious.  For example, 
you can cast an "long*" to a "int*" (as in "(int*)(long*)p") without any 
complaint, even though dereferencing the result is likely undefined behavior.  
(C doesn't allow explicitly writing a reinterpret_cast, but that's basically 
the operation in question.)

Along those lines, in general, the normal C rules should allow casting `foo*` 
to `bar*` for any object types foo and bar, even if foo and bar are pointers 
with address spaces, like `__local int *` and `__global int *`.  I don't see 
anything in the OpenCL standard that would contradict this.  It looks like this 
patch changes that?

I agree we shouldn't allow implicit conversions, though...


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

https://reviews.llvm.org/D58236



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

[PATCH] D58719: [PR40778][Sema] Adjust address space of operands in builtin operators

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think the right model is that a builtin operator candidate exists for all 
possible address-space qualifiers on the pointer/reference pointee, and it 
should be straightforward to only actually add candidates for the qualifier 
used by the corresponding operand.


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

https://reviews.llvm.org/D58719



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak closed this revision.
ahatanak added a comment.

Fixed in r355012.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaCast.cpp:2309
+auto DestPointeeTypeWithoutAS = Self.Context.removeAddrSpaceQualType(
+DestPointeeType.getCanonicalType());
+return Self.Context.hasSameType(SrcPointeeTypeWithoutAS,

rjmccall wrote:
> ebevhan wrote:
> > Anastasia wrote:
> > > Anastasia wrote:
> > > > ebevhan wrote:
> > > > > Maybe I'm mistaken, but won't getting the canonical type here drop 
> > > > > qualifiers (like cv) in nested pointers? If so, an addrspace_cast 
> > > > > might strip qualifiers by mistake.
> > > > Yes, indeed I will need to extend this to nested pointers when we are 
> > > > ready. But for now I can try to change this bit... however I am not 
> > > > sure it will work w/o canonical types when we have typedef. I will try 
> > > > to create an example and see.
> > > I checked the canonical type does preserve the qualifiers correctly.
> > > 
> > > Here is the AST dump of the following C type `mytype const __generic*` 
> > > where  `typedef  __generic int* mytype;`.
> > > 
> > > 
> > > ```
> > > PointerType 0x204d3b0 'const __generic mytype *'
> > > `-QualType 0x204d369 'const __generic mytype' const __generic
> > >   `-TypedefType 0x204d320 'mytype' sugar
> > > |-Typedef 0x204d1b0 'mytype'
> > > `-PointerType 0x204d170 '__generic int *'
> > >   `-QualType 0x204d158 '__generic int' __generic
> > > `-BuiltinType 0x2009750 'int'
> > > ```
> > > 
> > > and it's canonical representation in AST is:
> > > 
> > > ```
> > > PointerType 0x204d380 '__generic int *const __generic *'
> > > `-QualType 0x204d349 '__generic int *const __generic' const __generic
> > >   `-PointerType 0x204d170 '__generic int *'
> > > `-QualType 0x204d158 '__generic int' __generic
> > >   `-BuiltinType 0x2009750 'int'
> > > ```
> > > 
> > > So using canonical type will just simply handling of nested pointer chain 
> > > by avoiding special casing typedefs. We won't loose any qualifiers.
> > Okay, seems good then!
> It seems to me that the rules in this function are the reasonable 
> cross-language rules.  If you want to check for OpenCL at the top as a 
> fast-path check (taking advantage of that fact that no other language modes 
> currently have overlapping address spaces), that might be reasonable, but the 
> comment and indentation should reflect that.
Btw, I attempted to remove the OpenCL check and unfortunately found failure in 
the following test: **test/CodeGenCXX/address-space-cast.cpp**


```
error: C-style cast from 'char *' to '__attribute__((address_space(5))) char *' 
converts between mismatching address spaces
  __private__ char *priv_char_ptr = (__private__ char *)gen_char_ptr;
```

I am not sure what such conversion does but I feel if I want to update it I 
might need to get wider agreement. I think sending an RFC might be a good way 
forward.



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

https://reviews.llvm.org/D58346



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

r355012 - Avoid needlessly copying a block to the heap when a block literal

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Wed Feb 27 10:17:16 2019
New Revision: 355012

URL: http://llvm.org/viewvc/llvm-project?rev=355012&view=rev
Log:
Avoid needlessly copying a block to the heap when a block literal
initializes a local auto variable or is assigned to a local auto
variable that is declared in the scope that introduced the block
literal.

rdar://problem/13289333

https://reviews.llvm.org/D58514

Added:
cfe/trunk/test/PCH/arc-blocks.mm
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/AST/DeclBase.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/CodeGenObjC/arc-block-copy-escape.m
cfe/trunk/test/CodeGenObjC/arc-blocks.m
cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm

Modified: cfe/trunk/include/clang/AST/Decl.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=355012&r1=355011&r2=355012&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Wed Feb 27 10:17:16 2019
@@ -4008,6 +4008,13 @@ public:
   bool doesNotEscape() const { return BlockDeclBits.DoesNotEscape; }
   void setDoesNotEscape(bool B = true) { BlockDeclBits.DoesNotEscape = B; }
 
+  bool canAvoidCopyToHeap() const {
+return BlockDeclBits.CanAvoidCopyToHeap;
+  }
+  void setCanAvoidCopyToHeap(bool B = true) {
+BlockDeclBits.CanAvoidCopyToHeap = B;
+  }
+
   bool capturesVariable(const VarDecl *var) const;
 
   void setCaptures(ASTContext &Context, ArrayRef Captures,

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=355012&r1=355011&r2=355012&view=diff
==
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Wed Feb 27 10:17:16 2019
@@ -1665,6 +1665,11 @@ class DeclContext {
 /// A bit that indicates this block is passed directly to a function as a
 /// non-escaping parameter.
 uint64_t DoesNotEscape : 1;
+
+/// A bit that indicates whether it's possible to avoid coying this block 
to
+/// the heap when it initializes or is assigned to a local variable with
+/// automatic storage.
+uint64_t CanAvoidCopyToHeap : 1;
   };
 
   /// Number of non-inherited bits in BlockDeclBitfields.

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=355012&r1=355011&r2=355012&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Wed Feb 27 10:17:16 2019
@@ -4265,6 +4265,7 @@ BlockDecl::BlockDecl(DeclContext *DC, So
   setBlockMissingReturnType(true);
   setIsConversionFromLambda(false);
   setDoesNotEscape(false);
+  setCanAvoidCopyToHeap(false);
 }
 
 void BlockDecl::setParams(ArrayRef NewParamInfo) {

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=355012&r1=355011&r2=355012&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Wed Feb 27 10:17:16 2019
@@ -2871,6 +2871,7 @@ public:
   Result visit(const Expr *e);
   Result visitCastExpr(const CastExpr *e);
   Result visitPseudoObjectExpr(const PseudoObjectExpr *e);
+  Result visitBlockExpr(const BlockExpr *e);
   Result visitBinaryOperator(const BinaryOperator *e);
   Result visitBinAssign(const BinaryOperator *e);
   Result visitBinAssignUnsafeUnretained(const BinaryOperator *e);
@@ -2947,6 +2948,12 @@ ARCExprEmitter::visitPseudo
 }
 
 template 
+Result ARCExprEmitter::visitBlockExpr(const BlockExpr *e) {
+  // The default implementation just forwards the expression to visitExpr.
+  return asImpl().visitExpr(e);
+}
+
+template 
 Result ARCExprEmitter::visitCastExpr(const CastExpr *e) {
   switch (e->getCastKind()) {
 
@@ -3089,7 +3096,8 @@ Result ARCExprEmitter::visi
   // Look through pseudo-object expressions.
   } else if (const PseudoObjectExpr *pseudo = dyn_cast(e)) {
 return asImpl().visitPseudoObjectExpr(pseudo);
-  }
+  } else if (auto *be = dyn_cast(e))
+return asImpl().visitBlockExpr(be);
 
   return asImpl().visitExpr(e);
 }
@@ -3124,6 +3132,15 @@ struct ARCRetainExprEmitter :
 return TryEmitResult(result, true);
   }
 
+  TryEmitResult visitBlockExpr(const BlockExpr *e) {
+TryEmitResult result = visitExpr(e);
+// Avoid the block-retain if this is a block literal that doesn't need to 
be
+// copied to the heap.
+if (e->getBlockDecl()->canAvoidCopyToHeap())
+  result.setInt(true);
+re

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

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

- Added a comment to explain OpenCL check.


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

https://reviews.llvm.org/D58346

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaCast.cpp
  test/CodeGenOpenCLCXX/address-space-castoperators.cpp
  test/SemaCXX/address-space-conversion.cpp
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl

Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -26,24 +26,96 @@
 }
 
 void explicit_cast(__global int *g, __local int *l, __constant int *c, __private int *p, const __constant int *cc) {
-  g = (__global int *)l;  // expected-error {{casting '__local int *' to type '__global int *' changes address space of pointer}}
-  g = (__global int *)c;  // expected-error {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
-  g = (__global int *)cc; // expected-error {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
-  g = (__global int *)p;  // expected-error {{casting 'int *' to type '__global int *' changes address space of pointer}}
-
-  l = (__local int *)g;  // expected-error {{casting '__global int *' to type '__local int *' changes address space of pointer}}
-  l = (__local int *)c;  // expected-error {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
-  l = (__local int *)cc; // expected-error {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
-  l = (__local int *)p;  // expected-error {{casting 'int *' to type '__local int *' changes address space of pointer}}
-
-  c = (__constant int *)g; // expected-error {{casting '__global int *' to type '__constant int *' changes address space of pointer}}
-  c = (__constant int *)l; // expected-error {{casting '__local int *' to type '__constant int *' changes address space of pointer}}
-  c = (__constant int *)p; // expected-error {{casting 'int *' to type '__constant int *' changes address space of pointer}}
-
-  p = (__private int *)g;  // expected-error {{casting '__global int *' to type 'int *' changes address space of pointer}}
-  p = (__private int *)l;  // expected-error {{casting '__local int *' to type 'int *' changes address space of pointer}}
-  p = (__private int *)c;  // expected-error {{casting '__constant int *' to type 'int *' changes address space of pointer}}
-  p = (__private int *)cc; // expected-error {{casting 'const __constant int *' to type 'int *' changes address space of pointer}}
+  g = (__global int *)l;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__local int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__local int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  g = (__global int *)c;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__constant int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  g = (__global int *)cc;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from 'const __constant int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  g = (__global int *)p;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from 'int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)g;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__global int *' to type '__local int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__global int *' to '__local int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)c;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__constant int *' to '__local int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)cc;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from 'const __constant int *' to '__local int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)p;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'i

[PATCH] D57914: [Driver] Allow enum SanitizerOrdinal to represent more than 64 different sanitizer checks, NFC.

[pierre gousseau via Phabricator via cfe-commits]

pgousseau updated this revision to Diff 188576.
pgousseau added a comment.

move hash_value declaration to clang's namespace to solve lldb cmake bot build 
error.


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

https://reviews.llvm.org/D57914

Files:
  include/clang/Basic/Attr.td
  include/clang/Basic/Sanitizers.def
  include/clang/Basic/Sanitizers.h
  include/clang/Driver/ToolChain.h
  lib/Basic/SanitizerSpecialCaseList.cpp
  lib/Basic/Sanitizers.cpp
  lib/CodeGen/CGExpr.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChain.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Sema/SemaDeclAttr.cpp

Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -6211,7 +6211,8 @@
 if (!S.checkStringLiteralArgumentAttr(AL, I, SanitizerName, &LiteralLoc))
   return;
 
-if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) == 0)
+if (parseSanitizerValue(SanitizerName, /*AllowGroups=*/true) ==
+SanitizerMask())
   S.Diag(LiteralLoc, diag::warn_unknown_sanitizer_ignored) << SanitizerName;
 else if (isGlobalVar(D) && SanitizerName != "address")
   S.Diag(D->getLocation(), diag::err_attribute_wrong_decl_type)
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -551,7 +551,7 @@
 DiagnosticsEngine &Diags, SanitizerSet &S) {
   for (const auto &Sanitizer : Sanitizers) {
 SanitizerMask K = parseSanitizerValue(Sanitizer, /*AllowGroups=*/false);
-if (K == 0)
+if (K == SanitizerMask())
   Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
 else
   S.set(K, true);
Index: lib/Driver/ToolChain.cpp
===
--- lib/Driver/ToolChain.cpp
+++ lib/Driver/ToolChain.cpp
@@ -818,21 +818,23 @@
   // Return sanitizers which don't require runtime support and are not
   // platform dependent.
 
-  using namespace SanitizerKind;
-
-  SanitizerMask Res = (Undefined & ~Vptr & ~Function) | (CFI & ~CFIICall) |
-  CFICastStrict | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds;
+  SanitizerMask Res = (SanitizerKind::Undefined & ~SanitizerKind::Vptr &
+   ~SanitizerKind::Function) |
+  (SanitizerKind::CFI & ~SanitizerKind::CFIICall) |
+  SanitizerKind::CFICastStrict |
+  SanitizerKind::UnsignedIntegerOverflow |
+  SanitizerKind::ImplicitConversion |
+  SanitizerKind::Nullability | SanitizerKind::LocalBounds;
   if (getTriple().getArch() == llvm::Triple::x86 ||
   getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().getArch() == llvm::Triple::arm ||
   getTriple().getArch() == llvm::Triple::aarch64 ||
   getTriple().getArch() == llvm::Triple::wasm32 ||
   getTriple().getArch() == llvm::Triple::wasm64)
-Res |= CFIICall;
+Res |= SanitizerKind::CFIICall;
   if (getTriple().getArch() == llvm::Triple::x86_64 ||
   getTriple().getArch() == llvm::Triple::aarch64)
-Res |= ShadowCallStack;
+Res |= SanitizerKind::ShadowCallStack;
   return Res;
 }
 
Index: lib/Driver/SanitizerArgs.cpp
===
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -21,33 +21,52 @@
 #include 
 
 using namespace clang;
-using namespace clang::SanitizerKind;
 using namespace clang::driver;
 using namespace llvm::opt;
 
-enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer | ImplicitConversion | Nullability | CFI,
-  NeedsUbsanCxxRt = Vptr | CFI,
-  NotAllowedWithTrap = Vptr,
-  NotAllowedWithMinimalRuntime = Vptr,
-  RequiresPIE = DataFlow | HWAddress | Scudo,
-  NeedsUnwindTables = Address | HWAddress | Thread | Memory | DataFlow,
-  SupportsCoverage = Address | HWAddress | KernelAddress | KernelHWAddress |
- Memory | KernelMemory | Leak | Undefined | Integer |
- ImplicitConversion | Nullability | DataFlow | Fuzzer |
- FuzzerNoLink,
-  RecoverableByDefault = Undefined | Integer | ImplicitConversion | Nullability,
-  Unrecoverable = Unreachable | Return,
-  AlwaysRecoverable = KernelAddress | KernelHWAddress,
-  LegacyFsanitizeRecoverMask = Undefined | Integer,
-  NeedsLTO = CFI,
-  TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
-  ImplicitConversion | Nullability | LocalBounds | CFI,
-  TrappingDefault = CFI,
-  CFIClasses =
-  CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
-  CompatibleWithMinimalRuntime = TrappingSupported | Scudo | ShadowCallStack,
-};
+static const SanitizerMask NeedsUbsanRt =
+

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:2670
 } else {
   DestType = DestRecordType;
   FromRecordType = FromType;

This path (when the object is a gl-value) also needs an address-space 
qualifier, so you should probably add it above and then just add a pointer in 
the pointer case.


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

https://reviews.llvm.org/D58708



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