[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

In D61147#1479940 , @erik.pilkington 
wrote:

> In D61147#1479932 , @rjmccall wrote:
>
> > I do not think the ObjC version of this diagnostic is useful as it's been 
> > explained here, and I would strongly recommend just not including such a 
> > warning for the time being.
>
>
> Why? It seems to me like the vast majority of methods only declared in an 
> `@implementation` are used as local helpers (even if people probably should 
> be using functions for this), where this warning would be useful. Maybe I'm 
> missing something? Still trying to learn more about Objective-C.


What rule are you suggesting for whether a method is "only declared in an 
`@implementation`"?  Where exactly are you proposing to look for other 
declarations?

Objective-C does not have a formalized notion of an `@implementation`-private 
method, or really even an unambiguous convention for them (underscoring is 
*library*-private by convention, not class-private).  The Objective-C ecosystem 
includes a lot of informal protocols and conventions based around discovery via 
naming, and Objective-C programmers do (admittedly rarely) just override 
methods that they know are there but which they can't see.  These things make 
me very worried about trying to port assumptions from other languages.

Anyway, put that aside for a minute.  As I understand it, your current proposal 
is to make `-Wunused-parameter` not fire on the majority of Objective-C methods 
and then add a new `-Wunused-objc-parameter` warning that's not really formally 
related to `-Wunused-parameter`.  I think the right way to think about that is 
that the new warning is an independent feature which can happen at its own 
pace.  You will need to do some investigation in order to implement that 
feature, so in the short term, you should just not warn about unused parameters 
in Objective-C methods, and then you can do the investigation for generalizing 
the warning as time permits.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D61173: [BPF] do not generate predefined macro bpf

2019-04-25 Thread Yonghong Song via Phabricator via cfe-commits
yonghong-song created this revision.
yonghong-song added a reviewer: ast.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

"DefineStd(Builder, "bpf", Opts)" generates the following three macros:

  bpf
  __bpf
  __bpf__

and the macro "bpf" is due to the fact that the target language
is C which allows GNU extensions.

The name "bpf" could be easily used as variable name or type
field name. For example, in current linux kernel, there are
four places where bpf is used as a field name. If the corresponding
types are included in bpf program, the compilation error will
occur.

This patch removed predefined macro "bpf".


Repository:
  rC Clang

https://reviews.llvm.org/D61173

Files:
  lib/Basic/Targets/BPF.cpp
  test/Preprocessor/bpf-predefined-macros.c


Index: test/Preprocessor/bpf-predefined-macros.c
===
--- /dev/null
+++ test/Preprocessor/bpf-predefined-macros.c
@@ -0,0 +1,20 @@
+// RUN: %clang -E -target bpfel -x c -o - %s | FileCheck %s
+// RUN: %clang -E -target bpfeb -x c -o - %s | FileCheck %s
+
+#ifdef __bpf
+int a;
+#endif
+#ifdef __bpf__
+int b;
+#endif
+#ifdef __BPF__
+int c;
+#endif
+#ifdef bpf
+int d;
+#endif
+
+// CHECK: int a;
+// CHECK: int b;
+// CHECK: int c;
+// CHECK-NOT: int d;
Index: lib/Basic/Targets/BPF.cpp
===
--- lib/Basic/Targets/BPF.cpp
+++ lib/Basic/Targets/BPF.cpp
@@ -20,7 +20,8 @@
 
 void BPFTargetInfo::getTargetDefines(const LangOptions ,
  MacroBuilder ) const {
-  DefineStd(Builder, "bpf", Opts);
+  Builder.defineMacro("__bpf");
+  Builder.defineMacro("__bpf__");
   Builder.defineMacro("__BPF__");
 }
 


Index: test/Preprocessor/bpf-predefined-macros.c
===
--- /dev/null
+++ test/Preprocessor/bpf-predefined-macros.c
@@ -0,0 +1,20 @@
+// RUN: %clang -E -target bpfel -x c -o - %s | FileCheck %s
+// RUN: %clang -E -target bpfeb -x c -o - %s | FileCheck %s
+
+#ifdef __bpf
+int a;
+#endif
+#ifdef __bpf__
+int b;
+#endif
+#ifdef __BPF__
+int c;
+#endif
+#ifdef bpf
+int d;
+#endif
+
+// CHECK: int a;
+// CHECK: int b;
+// CHECK: int c;
+// CHECK-NOT: int d;
Index: lib/Basic/Targets/BPF.cpp
===
--- lib/Basic/Targets/BPF.cpp
+++ lib/Basic/Targets/BPF.cpp
@@ -20,7 +20,8 @@
 
 void BPFTargetInfo::getTargetDefines(const LangOptions ,
  MacroBuilder ) const {
-  DefineStd(Builder, "bpf", Opts);
+  Builder.defineMacro("__bpf");
+  Builder.defineMacro("__bpf__");
   Builder.defineMacro("__BPF__");
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

In D61147#1479932 , @rjmccall wrote:

> I do not think the ObjC version of this diagnostic is useful as it's been 
> explained here, and I would strongly recommend just not including such a 
> warning for the time being.


Why? It seems to me like the vast majority of methods only declared in an 
`@implementation` are used as local helpers (even if people probably should be 
using functions for this), where this warning would be useful. Maybe I'm 
missing something? Still trying to learn more about Objective-C.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D59168: [runtimes] Move libunwind, libc++abi and libc++ to lib/clang/ and include/

2019-04-25 Thread Petr Hosek via Phabricator via cfe-commits
phosek added a comment.

In D59168#1474715 , @jdenny wrote:

> Does the following match what you have in mind?
>
>   $prefix/
> include/
>   c++/
> v1/
>   limits.h
>   ...
>   openmp/
> v1/ <-- I don't think openmp has anything like this now


Iibc++ uses this scheme to allow the possibility of evolving the API but I'm 
not sure if it's necessary for openmp.

> omp.h
> ompt.h
> ...
>   lib/
> c++/
>   $target/
> libc++abi.so
> ...
> openmp/
>   $target/
> libomp.so
> ...
> clang/
>   9.0.0/ <- resource directory
> include/
> lib/
>   $target/
> libclang_rt.asan_cxx.a
> ...
> 
>   

Almost with a small variation:

  $prefix/
include/
  c++/
v1/
  limits.h
  ...
  openmp/
omp.h
ompt.h
...
lib/
  $target/ <--- This way we don't have to duplicate $target in each 
subdirectory and it matches the layout of the resource directory
c++/
  libc++abi.so
  ...
openmp/
  libomp.so
  ...
  clang/
9.0.0/
  include/
  lib/
$target/
  libclang_rt.asan_cxx.a
  ...

WDYT?


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

https://reviews.llvm.org/D59168



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


[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Are you sure these are the right semantics for `nodestroy`?  I think there's a 
reasonable argument that we should not destroy previous elements of a 
`nodestroy` array just because an element constructor throws.  It's basically 
academic for true globals because the exception will terminate the process 
anyway, and even for `thread_local`s and `static` locals (where I believe the 
exception is theoretically recoverable) it's at least arguable that we should 
either decline to destroy (possibly causing leaks) or simply call 
`std::terminate`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61165



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


[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

2019-04-25 Thread Patrick Nappa via Phabricator via cfe-commits
pnappa updated this revision to Diff 196795.
pnappa added a comment.

Apologies for taking a while, I'm afraid I hadn't had a moment of spare time 
over the past few weeks.

I believe I've applied the requested changes, bar one that I'm unsure about:

@alexfh to be sure, would adding an Option to 
"readability-implicit-bool-conversion" called say, "EnforceUppercase", suffice? 
With that option enabled the synthesis of the explicit comparison would be 
uppercase.

I'll make the appropriate changes to documentation when I get everything 
finalised.


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

https://reviews.llvm.org/D59859

Files:
  clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp
  clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.h
  clang-tools-extra/test/clang-tidy/readability-implicit-bool-conversion.cpp
  
clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-uppercase-literal-suffix-implicit-bool-conversion.cpp
@@ -0,0 +1,124 @@
+// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix,readability-implicit-bool-conversion %t
+
+template
+void functionTaking(T);
+
+
+// Implicit conversion to bool with enforced uppercase suffix.
+
+void implicitConversionToBoolSimpleCases() {
+  unsigned int unsignedInteger = 10u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:34: warning: integer literal has suffix 'u', which is not uppercase
+  // CHECK-MESSAGES-NEXT: unsigned int unsignedInteger = 10u;
+  // CHECK-MESSAGES-NEXT: ^ ~
+  // CHECK-MESSAGES-NEXT: {{^ *}}U{{$}}
+  // CHECK-FIXES: unsigned int unsignedInteger = 10U;
+
+  functionTaking(unsignedInteger);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned int' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(unsignedInteger != 0U);
+
+  unsigned long unsignedLong = 10ul;
+  // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: integer literal has suffix 'ul', which is not uppercase [readability-uppercase-literal-suffix]
+  // CHECK-MESSAGES-NEXT: unsigned long unsignedLong = 10ul;
+  // CHECK-MESSAGES-NEXT: ^ ~~
+  // CHECK-MESSAGES-NEXT: {{^ *}}UL{{$}}
+  // CHECK-FIXES: unsigned long unsignedLong = 10UL;
+
+  functionTaking(unsignedLong);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'unsigned long' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(unsignedLong != 0U);
+
+  float floating = 10.0f;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
+  // CHECK-MESSAGES-NEXT: float floating = 10.0f;
+  // CHECK-MESSAGES-NEXT: ^   ~
+  // CHECK-MESSAGES-NEXT: {{^ *}}F{{$}}
+  // CHECK-FIXES: float floating = 10.0F;
+
+  functionTaking(floating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'float' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(floating != 0.0F);
+
+  double doubleFloating = 10.0;
+  functionTaking(doubleFloating);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: implicit conversion 'double' -> bool [readability-implicit-bool-conversion]
+  // CHECK-FIXES: functionTaking(doubleFloating != 0.0);
+}
+
+void implicitConversionToBoolInComplexExpressions() {
+  bool boolean = true;
+
+  unsigned int integer = 10U;
+  unsigned int anotherInteger = 20U;
+  bool boolComingFromUnsignedInteger = integer + anotherInteger;
+  // CHECK-MESSAGES: :[[@LINE-1]]:40: warning: implicit conversion 'unsigned int' -> bool
+  // CHECK-FIXES: bool boolComingFromUnsignedInteger = (integer + anotherInteger) != 0U;
+
+  float floating = 0.2F;
+  // combination of two errors on the same line
+  bool boolComingFromFloating = floating - 0.3f || boolean;
+  // CHECK-MESSAGES: :[[@LINE-1]]:33: warning: implicit conversion 'float' -> bool
+  // CHECK-MESSAGES: :[[@LINE-2]]:44: warning: floating point literal has suffix 'f', which is not uppercase [readability-uppercase-literal-suffix]
+  // CHECK-FIXES: bool boolComingFromFloating = ((floating - 0.3F) != 0.0F) || boolean;
+}
+
+void implicitConversionInNegationExpressions() {
+  // ensure that in negation the replaced suffix is also capitalized
+  float floating = 10.0F;
+  bool boolComingFromNegatedFloat = ! floating;
+  // CHECK-MESSAGES: :[[@LINE-1]]:39: warning: implicit conversion 'float' -> bool
+  // CHECK-FIXES: bool boolComingFromNegatedFloat = floating == 0.0F;
+}
+
+void implicitConversionToBoolInControlStatements() {
+  unsigned long longInteger = 2U;
+  for (;longInteger;) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: implicit conversion 'unsigned long' -> bool
+  // CHECK-FIXES: for (;longInteger != 

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

I do not think the ObjC version of this diagnostic is useful as it's been 
explained here, and I would strongly recommend just not including such a 
warning for the time being.

I would also recommend that you go fix the warning to never fire on virtual C++ 
methods.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D60953: [clangd] Respect clang-tidy suppression comments

2019-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge edited reviewers, added: ilya-biryukov; removed: hokein.
nridge added a comment.

Changing reviewers as I understand @hokein is on vacation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60953



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


[PATCH] D59407: [clangd] Add RelationSlab

2019-04-25 Thread Nathan Ridge via Phabricator via cfe-commits
nridge planned changes to this revision.
nridge added a comment.

In D59407#1478794 , @sammccall wrote:

> So if you can stomach it, I think
>
> > **Approach 2: Add a RelationSlab storing (subject, predicate, object) 
> > triples, intended for sparse relations***
>
> is certainly fine with us (having spoken with @kadircet @ilya-biryukov 
> @sammccall @gribozavr - @hokein is on vacation but not nearly as stubborn as 
> I am!)


Yep, I'm happy to move forward with that approach. Thanks for the guidance!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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


[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-04-25 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment.

I don't have any particular comment, and I'll give an LGTM soon as people seem 
to generally happy about this. If you are not happy about this patch or the 
feature itself, please shout. This feature is about to land.


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

https://reviews.llvm.org/D60274



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


[PATCH] D52521: [Sema] DR727: Ensure we pick up enclosing template instantiation arguments for a class-scope explicit specialization.

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington abandoned this revision.
erik.pilkington added a comment.
Herald added a subscriber: jkorous.

Looks like @rsmith fixed this in r359266.


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

https://reviews.llvm.org/D52521



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


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

Yes, I guess we can make `-Wunused-parameter ` cover unused parameters of pure 
@implementation methods too since users can probably just remove them to 
silence the warning.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D61040: [Fuchsia] Support multilib for -fsanitize=address and -fno-exceptions

2019-04-25 Thread Petr Hosek via Phabricator via cfe-commits
phosek updated this revision to Diff 196791.
phosek marked 2 inline comments as done.

Repository:
  rC Clang

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

https://reviews.llvm.org/D61040

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/aarch64-fuchsia/lib/noexcept/.keep
  
clang/test/Driver/Inputs/resource_dir_with_per_target_subdir/x86_64-fuchsia/lib/noexcept/.keep
  clang/test/Driver/fuchsia.cpp

Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -49,3 +49,26 @@
 // CHECK-NOSTDLIBXX-NOT: "-lc++"
 // CHECK-NOSTDLIBXX-NOT: "-lm"
 // CHECK-NOSTDLIBXX: "-lc"
+
+// RUN: %clang %s -### --target=x86_64-fuchsia \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=MULTILIB-X86,NO-ASAN-X86,NO-NOEXCEPT-X86
+// RUN: %clang %s -### --target=x86_64-fuchsia -fsanitize=address \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=MULTILIB-X86,ASAN-X86
+// RUN: %clang %s -### --target=x86_64-fuchsia -fno-exceptions \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=MULTILIB-X86,NOEXCEPT-X86
+// RUN: %clang %s -### --target=x86_64-fuchsia -fsanitize=address -fno-exceptions \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: -fuse-ld=lld 2>&1\
+// RUN: | FileCheck %s -check-prefixes=MULTILIB-X86,ASAN-X86,NO-NOEXCEPT-X86
+// MULTILIB-X86: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// NOEXCEPT-X86: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}noexcept"
+// NO-NOEXCEPT-X86-NOT: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}noexcept"
+// ASAN-X86: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}asan"
+// NO-ASAN-X86-NOT: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib{{/|}}asan"
+// MULTILIB-X86: "-L[[RESOURCE_DIR]]{{/|}}x86_64-fuchsia{{/|}}lib"
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -33,6 +33,8 @@
 using namespace clang;
 using namespace llvm::opt;
 
+using tools::addMultilibFlag;
+
 void tools::GnuTool::anchor() {}
 
 static bool forwardToGCC(const Option ) {
@@ -871,16 +873,6 @@
   A->getValue() == StringRef("soft"));
 }
 
-/// \p Flag must be a flag accepted by the driver with its leading '-' removed,
-// otherwise '-print-multi-lib' will not emit them correctly.
-static void addMultilibFlag(bool Enabled, const char *const Flag,
-std::vector ) {
-  if (Enabled)
-Flags.push_back(std::string("+") + Flag);
-  else
-Flags.push_back(std::string("-") + Flag);
-}
-
 static bool isArmOrThumbArch(llvm::Triple::ArchType Arch) {
   return Arch == llvm::Triple::arm || Arch == llvm::Triple::thumb;
 }
Index: clang/lib/Driver/ToolChains/Fuchsia.cpp
===
--- clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -15,7 +15,9 @@
 #include "clang/Driver/Options.h"
 #include "clang/Driver/SanitizerArgs.h"
 #include "llvm/Option/ArgList.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 using namespace clang::driver;
 using namespace clang::driver::toolchains;
@@ -23,6 +25,8 @@
 using namespace clang;
 using namespace llvm::opt;
 
+using tools::addMultilibFlag;
+
 void fuchsia::Linker::ConstructJob(Compilation , const JobAction ,
const InputInfo ,
const InputInfoList ,
@@ -98,8 +102,6 @@
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   Args.AddAllArgs(CmdArgs, options::OPT_u);
 
-  addSanitizerPathLibArgs(ToolChain, Args, CmdArgs);
-
   ToolChain.AddFilePathLibArgs(Args, CmdArgs);
 
   if (D.isUsingLTO()) {
@@ -169,6 +171,42 @@
 llvm::sys::path::append(P, "lib");
 getFilePaths().push_back(P.str());
   }
+
+  auto RuntimeDir = [](const Multilib ) -> std::string {
+SmallString<128> P(D.ResourceDir);
+llvm::sys::path::append(P, D.getTargetTriple(), "lib", M.gccSuffix());
+return P.str();
+  };
+
+  Multilib Default;
+  // Use the noexcept variant with -fno-exceptions to avoid the extra overhead.
+  Multilib NoExcept = Multilib("/noexcept", {}, {}, 1)
+  .flag("-fexceptions")
+  .flag("+fno-exceptions");

r359266 - PR41607: Don't forget to substitute outer template arguments into a

2019-04-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Apr 25 19:11:23 2019
New Revision: 359266

URL: http://llvm.org/viewvc/llvm-project?rev=359266=rev
Log:
PR41607: Don't forget to substitute outer template arguments into a
class-scope explicit specialization of a class template.

Modified:
cfe/trunk/include/clang/AST/DeclTemplate.h
cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp

Modified: cfe/trunk/include/clang/AST/DeclTemplate.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclTemplate.h?rev=359266=359265=359266=diff
==
--- cfe/trunk/include/clang/AST/DeclTemplate.h (original)
+++ cfe/trunk/include/clang/AST/DeclTemplate.h Thu Apr 25 19:11:23 2019
@@ -1746,6 +1746,20 @@ public:
 return getSpecializationKind() == TSK_ExplicitSpecialization;
   }
 
+  /// Is this an explicit specialization at class scope (within the class that
+  /// owns the primary template)? For example:
+  ///
+  /// \code
+  /// template struct Outer {
+  ///   template struct Inner;
+  ///   template<> struct Inner; // class-scope explicit specialization
+  /// };
+  /// \endcode
+  bool isClassScopeExplicitSpecialization() const {
+return isExplicitSpecialization() &&
+   isa(getLexicalDeclContext());
+  }
+
   /// True if this declaration is an explicit specialization,
   /// explicit instantiation declaration, or explicit instantiation
   /// definition.
@@ -2581,6 +2595,11 @@ public:
 return getSpecializationKind() == TSK_ExplicitSpecialization;
   }
 
+  bool isClassScopeExplicitSpecialization() const {
+return isExplicitSpecialization() &&
+   isa(getLexicalDeclContext());
+  }
+
   /// True if this declaration is an explicit specialization,
   /// explicit instantiation declaration, or explicit instantiation
   /// definition.

Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp?rev=359266=359265=359266=diff
==
--- cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp Thu Apr 25 19:11:23 2019
@@ -66,9 +66,12 @@ Sema::getTemplateInstantiationArgs(Named
   if (!Ctx) {
 Ctx = D->getDeclContext();
 
-// Add template arguments from a variable template instantiation.
-if (VarTemplateSpecializationDecl *Spec =
-dyn_cast(D)) {
+// Add template arguments from a variable template instantiation. For a
+// class-scope explicit specialization, there are no template arguments
+// at this level, but there may be enclosing template arguments.
+VarTemplateSpecializationDecl *Spec =
+dyn_cast(D);
+if (Spec && !Spec->isClassScopeExplicitSpecialization()) {
   // We're done when we hit an explicit specialization.
   if (Spec->getSpecializationKind() == TSK_ExplicitSpecialization &&
   !isa(Spec))
@@ -111,8 +114,9 @@ Sema::getTemplateInstantiationArgs(Named
 
   while (!Ctx->isFileContext()) {
 // Add template arguments from a class template instantiation.
-if (ClassTemplateSpecializationDecl *Spec
-  = dyn_cast(Ctx)) {
+ClassTemplateSpecializationDecl *Spec
+  = dyn_cast(Ctx);
+if (Spec && !Spec->isClassScopeExplicitSpecialization()) {
   // We're done when we hit an explicit specialization.
   if (Spec->getSpecializationKind() == TSK_ExplicitSpecialization &&
   !isa(Spec))

Modified: cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp?rev=359266=359265=359266=diff
==
--- cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp (original)
+++ cfe/trunk/test/SemaTemplate/explicit-specialization-member.cpp Thu Apr 25 
19:11:23 2019
@@ -62,3 +62,20 @@ namespace SpecLoc {
   template<> float A::n; // expected-error {{different type}}
   template<> void A::f() throw(); // expected-error {{does not match}}
 }
+
+namespace PR41607 {
+  template struct Outer {
+template struct Inner;
+template<> struct Inner<> {
+  static constexpr int f() { return N; }
+};
+
+template static int a; // expected-note 2{{}}
+template<> static constexpr int a<> = 42;
+  };
+  static_assert(Outer<123>::Inner<>::f() == 123, "");
+  static_assert(Outer<123>::Inner<>::f() != 125, "");
+  // FIXME: The class-scope explicit specialization of the variable template 
doesn't work!
+  static_assert(Outer<123>::a<> == 42, ""); // expected-error {{}} 
expected-note {{}}
+  static_assert(Outer<123>::a<> != 43, ""); // expected-error {{}} 
expected-note {{}}
+}


___
cfe-commits mailing list

[PATCH] D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule".

2019-04-25 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359264: [analyzer] RetainCount: Add a suppression for 
the Matching rule. (authored by dergachev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61161?vs=196764=196782#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61161

Files:
  lib/Analysis/RetainSummaryManager.cpp
  test/Analysis/osobject-retain-release.cpp


Index: test/Analysis/osobject-retain-release.cpp
===
--- test/Analysis/osobject-retain-release.cpp
+++ test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: lib/Analysis/RetainSummaryManager.cpp
===
--- lib/Analysis/RetainSummaryManager.cpp
+++ lib/Analysis/RetainSummaryManager.cpp
@@ -228,29 +228,36 @@
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
 StringRef FName, QualType RetTy) {
+  assert(TrackOSObjects &&
+ "Requesting a summary for an OSObject but OSObjects are not tracked");
+
   if (RetTy->isPointerType()) {
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
-  if (const IdentifierInfo *II = FD->getIdentifier()) {
-StringRef FuncName = II->getName();
-if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
-  return getDefaultSummary();
-
-// All objects returned with functions *not* starting with
-// get, or iterators, are returned at +1.
-if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-isOSIteratorSubclass(PD)) {
-  return getOSSummaryCreateRule(FD);
-} else {
-  return getOSSummaryGetRule(FD);
-}
+  if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+return getDefaultSummary();
+
+  // TODO: Add support for the slightly common *Matching(table) idiom.
+  // Cf. IOService::nameMatching() etc. - these function have an unusual
+  // contract of returning at +0 or +1 depending on their last argument.
+  if (FName.endswith("Matching")) {
+return getPersistentStopSummary();
+  }
+
+  // All objects returned with functions *not* starting with 'get',
+  // or iterators, are returned at +1.
+  if ((!FName.startswith("get") && !FName.startswith("Get")) ||
+  isOSIteratorSubclass(PD)) {
+return getOSSummaryCreateRule(FD);
+  } else {
+return getOSSummaryGetRule(FD);
   }
 }
   }
 
   if (const auto *MD = dyn_cast(FD)) {
 const CXXRecordDecl *Parent = MD->getParent();
-if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+if (Parent && isOSObjectSubclass(Parent)) {
   if (FName == "release" || FName == "taggedRelease")
 return getOSSummaryReleaseRule(FD);
 


Index: test/Analysis/osobject-retain-release.cpp
===
--- test/Analysis/osobject-retain-release.cpp
+++ test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return 

[PATCH] D60991: [analyzer] RetainCount: Allow offsets in return values.

2019-04-25 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359263: [analyzer] RetainCount: Allow offsets in return 
values. (authored by dergachev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60991?vs=196170=196781#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60991

Files:
  lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  test/Analysis/retain-release.mm


Index: test/Analysis/retain-release.mm
===
--- test/Analysis/retain-release.mm
+++ test/Analysis/retain-release.mm
@@ -515,3 +515,35 @@
 }
 
 }
+
+namespace reinterpret_casts {
+
+void *foo() {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // FIXME: Should warn about a leak here. The function should return at +0,
+  // but it returns at +1 instead.
+  return q;
+}
+
+void *fooCreate() {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // The function follows the Create Rule.
+  return q; // no-warning
+}
+
+void *fooBar() CF_RETURNS_RETAINED {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // The function follows the Create Rule.
+  return q; // no-warning
+}
+
+}
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -970,8 +970,10 @@
 return Pred;
 
   ProgramStateRef state = C.getState();
-  SymbolRef Sym =
-state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
+  // We need to dig down to the symbolic base here because various
+  // custom allocators do sometimes return the symbol with an offset.
+  SymbolRef Sym = state->getSValAsScalarOrLoc(RetE, C.getLocationContext())
+  .getAsLocSymbol(/*IncludeBaseRegions=*/true);
   if (!Sym)
 return Pred;
 


Index: test/Analysis/retain-release.mm
===
--- test/Analysis/retain-release.mm
+++ test/Analysis/retain-release.mm
@@ -515,3 +515,35 @@
 }
 
 }
+
+namespace reinterpret_casts {
+
+void *foo() {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // FIXME: Should warn about a leak here. The function should return at +0,
+  // but it returns at +1 instead.
+  return q;
+}
+
+void *fooCreate() {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // The function follows the Create Rule.
+  return q; // no-warning
+}
+
+void *fooBar() CF_RETURNS_RETAINED {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // The function follows the Create Rule.
+  return q; // no-warning
+}
+
+}
Index: lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -970,8 +970,10 @@
 return Pred;
 
   ProgramStateRef state = C.getState();
-  SymbolRef Sym =
-state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
+  // We need to dig down to the symbolic base here because various
+  // custom allocators do sometimes return the symbol with an offset.
+  SymbolRef Sym = state->getSValAsScalarOrLoc(RetE, C.getLocationContext())
+  .getAsLocSymbol(/*IncludeBaseRegions=*/true);
   if (!Sym)
 return Pred;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60988: [analyzer] Fix crash when returning C++ objects from ObjC messages-to-nil.

2019-04-25 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359262: [analyzer] Fix crash when returning C++ objects from 
ObjC messages-to-nil. (authored by dergachev, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60988?vs=196161=196780#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60988

Files:
  lib/StaticAnalyzer/Core/RegionStore.cpp
  test/Analysis/nil-receiver.mm


Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2361,7 +2361,14 @@
   // In C++17 aggregates may have base classes, handle those as well.
   // They appear before fields in the initializer list / compound value.
   if (const auto *CRD = dyn_cast(RD)) {
-assert(CRD->isAggregate() &&
+// If the object was constructed with a constructor, its value is a
+// LazyCompoundVal. If it's a raw CompoundVal, it means that we're
+// performing aggregate initialization. The only exception from this
+// rule is sending an Objective-C++ message that returns a C++ object
+// to a nil receiver; in this case the semantics is to return a
+// zero-initialized object even if it's a C++ object that doesn't have
+// this sort of constructor; the CompoundVal is empty in this case.
+assert((CRD->isAggregate() || (Ctx.getLangOpts().ObjC && VI == VE)) &&
"Non-aggregates are constructed with a constructor!");
 
 for (const auto  : CRD->bases()) {
Index: test/Analysis/nil-receiver.mm
===
--- test/Analysis/nil-receiver.mm
+++ test/Analysis/nil-receiver.mm
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:-verify %s
+
+#define nil ((id)0)
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int x;
+  S();
+};
+
+@interface I
+@property S s;
+@end
+
+void foo() {
+  // This produces a zero-initialized structure.
+  // FIXME: This very fact does deserve the warning, because zero-initialized
+  // structures aren't always valid in C++. It's particularly bad when the
+  // object has a vtable.
+  S s = ((I *)nil).s;
+  clang_analyzer_eval(s.x == 0); // expected-warning{{TRUE}}
+}


Index: lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- lib/StaticAnalyzer/Core/RegionStore.cpp
+++ lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2361,7 +2361,14 @@
   // In C++17 aggregates may have base classes, handle those as well.
   // They appear before fields in the initializer list / compound value.
   if (const auto *CRD = dyn_cast(RD)) {
-assert(CRD->isAggregate() &&
+// If the object was constructed with a constructor, its value is a
+// LazyCompoundVal. If it's a raw CompoundVal, it means that we're
+// performing aggregate initialization. The only exception from this
+// rule is sending an Objective-C++ message that returns a C++ object
+// to a nil receiver; in this case the semantics is to return a
+// zero-initialized object even if it's a C++ object that doesn't have
+// this sort of constructor; the CompoundVal is empty in this case.
+assert((CRD->isAggregate() || (Ctx.getLangOpts().ObjC && VI == VE)) &&
"Non-aggregates are constructed with a constructor!");
 
 for (const auto  : CRD->bases()) {
Index: test/Analysis/nil-receiver.mm
===
--- test/Analysis/nil-receiver.mm
+++ test/Analysis/nil-receiver.mm
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:-verify %s
+
+#define nil ((id)0)
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int x;
+  S();
+};
+
+@interface I
+@property S s;
+@end
+
+void foo() {
+  // This produces a zero-initialized structure.
+  // FIXME: This very fact does deserve the warning, because zero-initialized
+  // structures aren't always valid in C++. It's particularly bad when the
+  // object has a vtable.
+  S s = ((I *)nil).s;
+  clang_analyzer_eval(s.x == 0); // expected-warning{{TRUE}}
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359263 - [analyzer] RetainCount: Allow offsets in return values.

2019-04-25 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Apr 25 19:05:15 2019
New Revision: 359263

URL: http://llvm.org/viewvc/llvm-project?rev=359263=rev
Log:
[analyzer] RetainCount: Allow offsets in return values.

Because RetainCountChecker has custom "local" reasoning about escapes,
it has a separate facility to deal with tracked symbols at end of analysis
and check them for leaks regardless of whether they're dead or not.
This facility iterates over the list of tracked symbols and reports
them as leaks, but it needs to treat the return value specially.

Some custom allocators tend to return the value with an offset, storing
extra metadata at the beginning of the buffer. In this case the return value
would be a non-base region. In order to avoid false positives, we still need to
find the original symbol within the return value, otherwise it'll be unable
to match it to the item in the list of tracked symbols.

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
cfe/trunk/test/Analysis/retain-release.mm

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp?rev=359263=359262=359263=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp 
Thu Apr 25 19:05:15 2019
@@ -970,8 +970,10 @@ ExplodedNode * RetainCountChecker::proce
 return Pred;
 
   ProgramStateRef state = C.getState();
-  SymbolRef Sym =
-state->getSValAsScalarOrLoc(RetE, C.getLocationContext()).getAsLocSymbol();
+  // We need to dig down to the symbolic base here because various
+  // custom allocators do sometimes return the symbol with an offset.
+  SymbolRef Sym = state->getSValAsScalarOrLoc(RetE, C.getLocationContext())
+  .getAsLocSymbol(/*IncludeBaseRegions=*/true);
   if (!Sym)
 return Pred;
 

Modified: cfe/trunk/test/Analysis/retain-release.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/retain-release.mm?rev=359263=359262=359263=diff
==
--- cfe/trunk/test/Analysis/retain-release.mm (original)
+++ cfe/trunk/test/Analysis/retain-release.mm Thu Apr 25 19:05:15 2019
@@ -515,3 +515,35 @@ void foo() {
 }
 
 }
+
+namespace reinterpret_casts {
+
+void *foo() {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // FIXME: Should warn about a leak here. The function should return at +0,
+  // but it returns at +1 instead.
+  return q;
+}
+
+void *fooCreate() {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // The function follows the Create Rule.
+  return q; // no-warning
+}
+
+void *fooBar() CF_RETURNS_RETAINED {
+  void *p = const_cast(
+  reinterpret_cast(CFArrayCreate(0, 0, 0, 0)));
+  void *q = reinterpret_cast(
+  reinterpret_cast(p) + 1);
+  // The function follows the Create Rule.
+  return q; // no-warning
+}
+
+}


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


r359262 - [analyzer] Fix crash when returning C++ objects from ObjC messages-to-nil.

2019-04-25 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Apr 25 19:05:12 2019
New Revision: 359262

URL: http://llvm.org/viewvc/llvm-project?rev=359262=rev
Log:
[analyzer] Fix crash when returning C++ objects from ObjC messages-to-nil.

the assertion is in fact incorrect: there is a cornercase in Objective-C++
in which a C++ object is not constructed with a constructor, but merely
zero-initialized. Namely, this happens when an Objective-C message is sent
to a nil and it is supposed to return a C++ object.

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

Added:
cfe/trunk/test/Analysis/nil-receiver.mm
Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=359262=359261=359262=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Apr 25 19:05:12 2019
@@ -2361,7 +2361,14 @@ RegionBindingsRef RegionStoreManager::bi
   // In C++17 aggregates may have base classes, handle those as well.
   // They appear before fields in the initializer list / compound value.
   if (const auto *CRD = dyn_cast(RD)) {
-assert(CRD->isAggregate() &&
+// If the object was constructed with a constructor, its value is a
+// LazyCompoundVal. If it's a raw CompoundVal, it means that we're
+// performing aggregate initialization. The only exception from this
+// rule is sending an Objective-C++ message that returns a C++ object
+// to a nil receiver; in this case the semantics is to return a
+// zero-initialized object even if it's a C++ object that doesn't have
+// this sort of constructor; the CompoundVal is empty in this case.
+assert((CRD->isAggregate() || (Ctx.getLangOpts().ObjC && VI == VE)) &&
"Non-aggregates are constructed with a constructor!");
 
 for (const auto  : CRD->bases()) {

Added: cfe/trunk/test/Analysis/nil-receiver.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/nil-receiver.mm?rev=359262=auto
==
--- cfe/trunk/test/Analysis/nil-receiver.mm (added)
+++ cfe/trunk/test/Analysis/nil-receiver.mm Thu Apr 25 19:05:12 2019
@@ -0,0 +1,24 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:-verify %s
+
+#define nil ((id)0)
+
+void clang_analyzer_eval(int);
+
+struct S {
+  int x;
+  S();
+};
+
+@interface I
+@property S s;
+@end
+
+void foo() {
+  // This produces a zero-initialized structure.
+  // FIXME: This very fact does deserve the warning, because zero-initialized
+  // structures aren't always valid in C++. It's particularly bad when the
+  // object has a vtable.
+  S s = ((I *)nil).s;
+  clang_analyzer_eval(s.x == 0); // expected-warning{{TRUE}}
+}


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


r359264 - [analyzer] RetainCount: Add a suppression for "the Matching rule".

2019-04-25 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Apr 25 19:05:18 2019
New Revision: 359264

URL: http://llvm.org/viewvc/llvm-project?rev=359264=rev
Log:
[analyzer] RetainCount: Add a suppression for "the Matching rule".

In the OSObject universe there appears to be another slightly popular contract,
apart from "create" and "get", which is "matching". It optionally consumes
a "table" parameter and if a table is passed, it fills in the table and
returns it at +0; otherwise, it creates a new table, fills it in and
returns it at +1.

For now suppress false positives by doing a conservative escape on all functions
that end with "Matching", which is the naming convention that seems to be
followed by all such methods.

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

Modified:
cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
cfe/trunk/test/Analysis/osobject-retain-release.cpp

Modified: cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/RetainSummaryManager.cpp?rev=359264=359263=359264=diff
==
--- cfe/trunk/lib/Analysis/RetainSummaryManager.cpp (original)
+++ cfe/trunk/lib/Analysis/RetainSummaryManager.cpp Thu Apr 25 19:05:18 2019
@@ -228,29 +228,36 @@ RetainSummaryManager::isKnownSmartPointe
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
 StringRef FName, QualType RetTy) {
+  assert(TrackOSObjects &&
+ "Requesting a summary for an OSObject but OSObjects are not tracked");
+
   if (RetTy->isPointerType()) {
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
-  if (const IdentifierInfo *II = FD->getIdentifier()) {
-StringRef FuncName = II->getName();
-if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
-  return getDefaultSummary();
+  if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+return getDefaultSummary();
+
+  // TODO: Add support for the slightly common *Matching(table) idiom.
+  // Cf. IOService::nameMatching() etc. - these function have an unusual
+  // contract of returning at +0 or +1 depending on their last argument.
+  if (FName.endswith("Matching")) {
+return getPersistentStopSummary();
+  }
 
-// All objects returned with functions *not* starting with
-// get, or iterators, are returned at +1.
-if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-isOSIteratorSubclass(PD)) {
-  return getOSSummaryCreateRule(FD);
-} else {
-  return getOSSummaryGetRule(FD);
-}
+  // All objects returned with functions *not* starting with 'get',
+  // or iterators, are returned at +1.
+  if ((!FName.startswith("get") && !FName.startswith("Get")) ||
+  isOSIteratorSubclass(PD)) {
+return getOSSummaryCreateRule(FD);
+  } else {
+return getOSSummaryGetRule(FD);
   }
 }
   }
 
   if (const auto *MD = dyn_cast(FD)) {
 const CXXRecordDecl *Parent = MD->getParent();
-if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+if (Parent && isOSObjectSubclass(Parent)) {
   if (FName == "release" || FName == "taggedRelease")
 return getOSSummaryReleaseRule(FD);
 

Modified: cfe/trunk/test/Analysis/osobject-retain-release.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/osobject-retain-release.cpp?rev=359264=359263=359264=diff
==
--- cfe/trunk/test/Analysis/osobject-retain-release.cpp (original)
+++ cfe/trunk/test/Analysis/osobject-retain-release.cpp Thu Apr 25 19:05:18 2019
@@ -679,3 +679,26 @@ void test_tagged_retain_no_uaf() {
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}


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


[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1078
+return true;
+  if (RD->hasUserDeclaredConstructor())
+return true;

Like I mentioned before, the rule says "user-provided", not "user-declared".



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1080
+return true;
+  if (RD->hasUserDeclaredDestructor())
+return true;

The rule says "trivial" not "user-declared".


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

https://reviews.llvm.org/D60349



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


r359260 - C++ DR2387: a variable template declared wtih (or instantiated with) a

2019-04-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Apr 25 18:51:08 2019
New Revision: 359260

URL: http://llvm.org/viewvc/llvm-project?rev=359260=rev
Log:
C++ DR2387: a variable template declared wtih (or instantiated with) a
const-qualified type is not implicitly given internal linkage. But a
variable template declared 'static' is.

This reinstates part of r359048, reverted in r359076.

Added:
cfe/trunk/test/CXX/drs/dr23xx.cpp
Modified:
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/test/CXX/module/module.interface/p3.cpp
cfe/trunk/test/CXX/module/module.interface/p5.cpp
cfe/trunk/test/CodeGenCXX/cxx1y-variable-template-linkage.cpp
cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp
cfe/trunk/test/SemaCXX/warn-unused-variables.cpp

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=359260=359259=359260=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Apr 25 18:51:08 2019
@@ -610,6 +610,18 @@ static LinkageInfo getExternalLinkageFor
   return LinkageInfo::external();
 }
 
+static StorageClass getStorageClass(const Decl *D) {
+  if (auto *TD = dyn_cast(D))
+D = TD->getTemplatedDecl();
+  if (D) {
+if (auto *VD = dyn_cast(D))
+  return VD->getStorageClass();
+if (auto *FD = dyn_cast(D))
+  return FD->getStorageClass();
+  }
+  return SC_None;
+}
+
 LinkageInfo
 LinkageComputer::getLVForNamespaceScopeDecl(const NamedDecl *D,
 LVComputationKind computation,
@@ -621,24 +633,28 @@ LinkageComputer::getLVForNamespaceScopeD
   // C++ [basic.link]p3:
   //   A name having namespace scope (3.3.6) has internal linkage if it
   //   is the name of
-  // - an object, reference, function or function template that is
-  //   explicitly declared static; or,
-  // (This bullet corresponds to C99 6.2.2p3.)
+
+  if (getStorageClass(D->getCanonicalDecl()) == SC_Static) {
+// - a variable, variable template, function, or function template
+//   that is explicitly declared static; or
+// (This bullet corresponds to C99 6.2.2p3.)
+return getInternalLinkageFor(D);
+  }
+
   if (const auto *Var = dyn_cast(D)) {
-// Explicitly declared static.
-if (Var->getStorageClass() == SC_Static)
-  return getInternalLinkageFor(Var);
-
-// - a non-inline, non-volatile object or reference that is explicitly
-//   declared const or constexpr and neither explicitly declared extern
-//   nor previously declared to have external linkage; or (there is no
-//   equivalent in C99)
-// The C++ modules TS adds "non-exported" to this list.
+// - a non-template variable of non-volatile const-qualified type, unless
+//   - it is explicitly declared extern, or
+//   - it is inline or exported, or
+//   - it was previously declared and the prior declaration did not have
+// internal linkage
+// (There is no equivalent in C99.)
 if (Context.getLangOpts().CPlusPlus &&
 Var->getType().isConstQualified() &&
 !Var->getType().isVolatileQualified() &&
 !Var->isInline() &&
-!isExportedFromModuleInterfaceUnit(Var)) {
+!isExportedFromModuleInterfaceUnit(Var) &&
+!isa(Var) &&
+!Var->getDescribedVarTemplate()) {
   const VarDecl *PrevVar = Var->getPreviousDecl();
   if (PrevVar)
 return getLVForDecl(PrevVar, computation);
@@ -658,14 +674,6 @@ LinkageComputer::getLVForNamespaceScopeD
   if (PrevVar->getStorageClass() == SC_Static)
 return getInternalLinkageFor(Var);
 }
-  } else if (const FunctionDecl *Function = D->getAsFunction()) {
-// C++ [temp]p4:
-//   A non-member function template can have internal linkage; any
-//   other template name shall have external linkage.
-
-// Explicitly declared static.
-if (Function->getCanonicalDecl()->getStorageClass() == SC_Static)
-  return getInternalLinkageFor(Function);
   } else if (const auto *IFD = dyn_cast(D)) {
 //   - a data member of an anonymous union.
 const VarDecl *VD = IFD->getVarDecl();
@@ -674,6 +682,8 @@ LinkageComputer::getLVForNamespaceScopeD
   }
   assert(!isa(D) && "Didn't expect a FieldDecl!");
 
+  // FIXME: This gives internal linkage to names that should have no linkage
+  // (those not covered by [basic.link]p6).
   if (D->isInAnonymousNamespace()) {
 const auto *Var = dyn_cast(D);
 const auto *Func = dyn_cast(D);
@@ -733,10 +743,20 @@ LinkageComputer::getLVForNamespaceScopeD
 
   // C++ [basic.link]p4:
 
-  //   A name having namespace scope has external linkage if it is the
-  //   name of
+  //   A name having namespace scope that has not been given internal linkage
+  //   above and that is the name of
+  //   [...bullets...]
+  //   has its linkage determined as follows:
+  // - if the enclosing namespace has internal linkage, the name has
+  //   internal 

r359261 - [www] Rebuild cxx_dr_status.

2019-04-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Apr 25 18:51:08 2019
New Revision: 359261

URL: http://llvm.org/viewvc/llvm-project?rev=359261=rev
Log:
[www] Rebuild cxx_dr_status.

Modified:
cfe/trunk/test/CXX/drs/dr17xx.cpp
cfe/trunk/www/cxx_dr_status.html

Modified: cfe/trunk/test/CXX/drs/dr17xx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr17xx.cpp?rev=359261=359260=359261=diff
==
--- cfe/trunk/test/CXX/drs/dr17xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr17xx.cpp Thu Apr 25 18:51:08 2019
@@ -77,7 +77,7 @@ namespace dr1758 { // dr1758: 3.7
 #endif
 }
 
-namespace dr1722 { // dr1722: 9.0
+namespace dr1722 { // dr1722: 9
 #if __cplusplus >= 201103L
 void f() {
   const auto lambda = [](int x) { return x + 1; };


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


r359259 - Add missing diagnostic for explicit instantiation declarations naming

2019-04-25 Thread Richard Smith via cfe-commits
Author: rsmith
Date: Thu Apr 25 18:51:07 2019
New Revision: 359259

URL: http://llvm.org/viewvc/llvm-project?rev=359259=rev
Log:
Add missing diagnostic for explicit instantiation declarations naming
internal linkage entities.

Such constructs are ill-formed by [temp.explicit]p13. We make a special
exception to permit an invalid construct used by libc++ in some build
modes: its  header declares some functions with the
internal_linkage attribute and then (meaninglessly) provides explicit
instantiation declarations for them. Luckily, Clang happens to
effectively ignore the explicit instantiation declaration when
generating code in this case, and this change codifies that behavior.

This reinstates part of r359048, reverted in r359076. (The libc++ issue
triggering the rollback has been addressed.)

Added:
cfe/trunk/test/SemaCXX/libcxx_valarray_hack.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/CXX/drs/dr0xx.cpp
cfe/trunk/test/SemaCXX/PR10177.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=359259=359258=359259=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Apr 25 18:51:07 
2019
@@ -4384,6 +4384,8 @@ def err_explicit_instantiation_of_typede
   "explicit instantiation of typedef %0">;
 def err_explicit_instantiation_storage_class : Error<
   "explicit instantiation cannot have a storage class">;
+def err_explicit_instantiation_internal_linkage : Error<
+  "explicit instantiation declaration of %0 with internal linkage">;
 def err_explicit_instantiation_not_known : Error<
   "explicit instantiation of %0 does not refer to a function template, "
   "variable template, member function, member class, or static data member">;

Modified: cfe/trunk/lib/Sema/SemaTemplate.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplate.cpp?rev=359259=359258=359259=diff
==
--- cfe/trunk/lib/Sema/SemaTemplate.cpp (original)
+++ cfe/trunk/lib/Sema/SemaTemplate.cpp Thu Apr 25 18:51:07 2019
@@ -8619,6 +8619,29 @@ static bool CheckExplicitInstantiationSc
   return false;
 }
 
+/// Common checks for whether an explicit instantiation of \p D is valid.
+static bool CheckExplicitInstantiation(Sema , NamedDecl *D,
+   SourceLocation InstLoc,
+   bool WasQualifiedName,
+   TemplateSpecializationKind TSK) {
+  // C++ [temp.explicit]p13:
+  //   An explicit instantiation declaration shall not name a specialization of
+  //   a template with internal linkage.
+  if (TSK == TSK_ExplicitInstantiationDeclaration &&
+  D->getFormalLinkage() == InternalLinkage) {
+S.Diag(InstLoc, diag::err_explicit_instantiation_internal_linkage) << D;
+return true;
+  }
+
+  // C++11 [temp.explicit]p3: [DR 275]
+  //   An explicit instantiation shall appear in an enclosing namespace of its
+  //   template.
+  if (CheckExplicitInstantiationScope(S, D, InstLoc, WasQualifiedName))
+return true;
+
+  return false;
+}
+
 /// Determine whether the given scope specifier has a template-id in it.
 static bool ScopeSpecifierHasTemplateId(const CXXScopeSpec ) {
   if (!SS.isSet())
@@ -8770,13 +8793,8 @@ DeclResult Sema::ActOnExplicitInstantiat
   TemplateSpecializationKind PrevDecl_TSK
 = PrevDecl ? PrevDecl->getTemplateSpecializationKind() : TSK_Undeclared;
 
-  // C++0x [temp.explicit]p2:
-  //   [...] An explicit instantiation shall appear in an enclosing
-  //   namespace of its template. [...]
-  //
-  // This is C++ DR 275.
-  if (CheckExplicitInstantiationScope(*this, ClassTemplate, TemplateNameLoc,
-  SS.isSet()))
+  if (CheckExplicitInstantiation(*this, ClassTemplate, TemplateNameLoc,
+ SS.isSet(), TSK))
 return true;
 
   ClassTemplateSpecializationDecl *Specialization = nullptr;
@@ -8999,12 +9017,7 @@ Sema::ActOnExplicitInstantiation(Scope *
 = ExternLoc.isInvalid()? TSK_ExplicitInstantiationDefinition
: TSK_ExplicitInstantiationDeclaration;
 
-  // C++0x [temp.explicit]p2:
-  //   [...] An explicit instantiation shall appear in an enclosing
-  //   namespace of its template. [...]
-  //
-  // This is C++ DR 275.
-  CheckExplicitInstantiationScope(*this, Record, NameLoc, true);
+  CheckExplicitInstantiation(*this, Record, NameLoc, true, TSK);
 
   // Verify that it is okay to explicitly instantiate here.
   CXXRecordDecl *PrevDecl
@@ -9235,8 +9248,7 @@ DeclResult Sema::ActOnExplicitInstantiat
diag::ext_explicit_instantiation_without_qualified_id)
 << 

r359258 - Revert lib/Header: Fix Visual Studio builds

2019-04-25 Thread Tom Stellard via cfe-commits
Author: tstellar
Date: Thu Apr 25 18:43:59 2019
New Revision: 359258

URL: http://llvm.org/viewvc/llvm-project?rev=359258=rev
Log:
Revert lib/Header: Fix Visual Studio builds

This reverts r359257 (git commit 00d9789509a4c573a48f60893b95314a119edd42)

This broke check-clang.

Modified:
cfe/trunk/lib/Headers/CMakeLists.txt

Modified: cfe/trunk/lib/Headers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/CMakeLists.txt?rev=359258=359257=359258=diff
==
--- cfe/trunk/lib/Headers/CMakeLists.txt (original)
+++ cfe/trunk/lib/Headers/CMakeLists.txt Thu Apr 25 18:43:59 2019
@@ -126,7 +126,7 @@ set(ppc_wrapper_files
   ppc_wrappers/mmintrin.h
 )
 
-set(output_dir 
${CMAKE_CURRENT_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
+set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
 set(out_files)
 
 function(copy_header_to_output_dir src_dir file)


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


[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Mandeep Singh Grang via Phabricator via cfe-commits
mgrang updated this revision to Diff 196774.

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

https://reviews.llvm.org/D60349

Files:
  include/clang/AST/DeclCXX.h
  include/clang/CodeGen/CGFunctionInfo.h
  lib/CodeGen/CGCall.cpp
  lib/CodeGen/MicrosoftCXXABI.cpp
  lib/Sema/SemaDeclCXX.cpp
  test/CodeGen/arm64-microsoft-arguments.cpp
  test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp

Index: test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
===
--- test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
+++ test/CodeGenCXX/microsoft-abi-sret-and-byval.cpp
@@ -69,6 +69,11 @@
   int bb;
 };
 
+struct SmallWithPrivate {
+private:
+ int i;
+};
+
 // WIN32: declare dso_local void @"{{.*take_bools_and_chars.*}}"
 // WIN32:   (<{ i8, [3 x i8], i8, [3 x i8], %struct.SmallWithDtor,
 // WIN32:   i8, [3 x i8], i8, [3 x i8], i32, i8, [3 x i8] }>* inalloca)
@@ -164,8 +169,8 @@
 // WIN64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(i32 %s.coerce) {{.*}} {
 // WIN64:   call void @"??1SmallWithDtor@@QEAA@XZ"
 // WIN64: }
-// WOA64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(i64 %s.coerce) {{.*}} {
-// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"
+// WOA64: define dso_local void @"?small_arg_with_dtor@@YAXUSmallWithDtor@@@Z"(%struct.SmallWithDtor* %s) {{.*}} {
+// WOA64:   call void @"??1SmallWithDtor@@QEAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA64: }
 
 // FIXME: MSVC incompatible!
@@ -173,6 +178,12 @@
 // WOA:   call arm_aapcs_vfpcc void @"??1SmallWithDtor@@QAA@XZ"(%struct.SmallWithDtor* %s)
 // WOA: }
 
+
+// Test that the eligible non-aggregate is passed directly, but returned
+// indirectly on ARM64 Windows.
+// WOA64: define dso_local void @"?small_arg_with_private_member@@YA?AUSmallWithPrivate@@U1@@Z"(%struct.SmallWithPrivate* inreg noalias sret %agg.result, i64 %s.coerce) {{.*}} {
+SmallWithPrivate small_arg_with_private_member(SmallWithPrivate s) { return s; }
+
 void call_small_arg_with_dtor() {
   small_arg_with_dtor(SmallWithDtor());
 }
Index: test/CodeGen/arm64-microsoft-arguments.cpp
===
--- test/CodeGen/arm64-microsoft-arguments.cpp
+++ test/CodeGen/arm64-microsoft-arguments.cpp
@@ -1,25 +1,88 @@
 // RUN: %clang_cc1 -triple aarch64-windows -ffreestanding -emit-llvm -O0 \
 // RUN: -x c++ -o - %s | FileCheck %s
 
-struct pod { int a, b, c, d, e; };
+// Pass and return for type size <= 8 bytes.
+// CHECK: define {{.*}} i64 @{{.*}}f1{{.*}}()
+// CHECK: call i64 {{.*}}func1{{.*}}(i64 %3)
+struct S1 {
+  int a[2];
+};
+
+S1 func1(S1 x);
+S1 f1() {
+  S1 x;
+  return func1(x);
+}
 
-struct non_pod {
-  int a;
-  non_pod() {}
+// Pass and return type size <= 16 bytes.
+// CHECK: define {{.*}} [2 x i64] @{{.*}}f2{{.*}}()
+// CHECK: call [2 x i64] {{.*}}func2{{.*}}([2 x i64] %3)
+struct S2 {
+  int a[4];
 };
 
-struct pod s;
-struct non_pod t;
+S2 func2(S2 x);
+S2 f2() {
+  S2 x;
+  return func2(x);
+}
+
+// Pass and return for type size > 16 bytes.
+// CHECK: define {{.*}} void @{{.*}}f3{{.*}}(%struct.S3* noalias sret %agg.result)
+// CHECK: call void {{.*}}func3{{.*}}(%struct.S3* sret %agg.result, %struct.S3* %agg.tmp)
+struct S3 {
+  int a[5];
+};
+
+S3 func3(S3 x);
+S3 f3() {
+  S3 x;
+  return func3(x);
+}
+
+// Pass and return aggregate (of size < 16 bytes) with non-trivial destructor.
+// Passed directly but returned indirectly.
+// CHECK: define {{.*}} void {{.*}}f4{{.*}}(%struct.S4* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}func4{{.*}}(%struct.S4* inreg sret %agg.result, [2 x i64] %4)
+struct S4 {
+  int a[3];
+  ~S4();
+};
 
-struct pod bar() { return s; }
-struct non_pod foo() { return t; }
-// CHECK: define {{.*}} void @{{.*}}bar{{.*}}(%struct.pod* noalias sret %agg.result)
-// CHECK: define {{.*}} void @{{.*}}foo{{.*}}(%struct.non_pod* noalias %agg.result)
+S4 func4(S4 x);
+S4 f4() {
+  S4 x;
+  return func4(x);
+}
 
+// Pass and return from instance method called from instance method.
+// CHECK: define {{.*}} void @{{.*}}bar@Q1{{.*}}(%class.Q1* %this, %class.P1* inreg noalias sret %agg.result)
+// CHECK: call void {{.*}}foo@P1{{.*}}(%class.P1* %ref.tmp, %class.P1* inreg sret %agg.result, i8 %0)
 
-// Check instance methods.
-struct pod2 { int x; };
-struct Baz { pod2 baz(); };
+class P1 {
+public:
+  P1 foo(P1 x);
+};
+
+class Q1 {
+public:
+  P1 bar();
+};
+
+P1 Q1::bar() {
+  P1 p1;
+  return P1().foo(p1);
+}
+
+// Pass and return from instance method called from free function.
+// CHECK: define {{.*}} void {{.*}}bar{{.*}}()
+// CHECK: call void {{.*}}foo@P2{{.*}}(%class.P2* %ref.tmp, %class.P2* inreg sret %retval, i8 %0)
+class P2 {
+public:
+  P2 foo(P2 x);
+};
 
-int qux() { return Baz().baz().x; }
-// CHECK: declare {{.*}} void @{{.*}}baz@Baz{{.*}}(%struct.Baz*, %struct.pod2*)
+P2 bar() {
+  P2 p2;
+  return P2().foo(p2);
+}
Index: lib/Sema/SemaDeclCXX.cpp

r359257 - lib/Header: Fix Visual Studio builds

2019-04-25 Thread Tom Stellard via cfe-commits
Author: tstellar
Date: Thu Apr 25 18:18:59 2019
New Revision: 359257

URL: http://llvm.org/viewvc/llvm-project?rev=359257=rev
Log:
lib/Header: Fix Visual Studio builds

Summary:
This is a follow up to r355253, which inadvertently broke Visual
Studio builds by trying to copy files from CMAKE_CFG_INTDIR.

See https://reviews.llvm.org/D58537#inline-532492

Reviewers: smeenai, vzakhari, phosek

Reviewed By: smeenai

Subscribers: mgorny, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Headers/CMakeLists.txt

Modified: cfe/trunk/lib/Headers/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/CMakeLists.txt?rev=359257=359256=359257=diff
==
--- cfe/trunk/lib/Headers/CMakeLists.txt (original)
+++ cfe/trunk/lib/Headers/CMakeLists.txt Thu Apr 25 18:18:59 2019
@@ -126,7 +126,7 @@ set(ppc_wrapper_files
   ppc_wrappers/mmintrin.h
 )
 
-set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
+set(output_dir 
${CMAKE_CURRENT_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
 set(out_files)
 
 function(copy_header_to_output_dir src_dir file)


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


[PATCH] D61054: lib/Header: Fix Visual Studio builds

2019-04-25 Thread Tom Stellard via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359257: lib/Header: Fix Visual Studio builds (authored by 
tstellar, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61054?vs=196373=196771#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61054

Files:
  lib/Headers/CMakeLists.txt


Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -126,7 +126,7 @@
   ppc_wrappers/mmintrin.h
 )
 
-set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
+set(output_dir 
${CMAKE_CURRENT_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
 set(out_files)
 
 function(copy_header_to_output_dir src_dir file)


Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -126,7 +126,7 @@
   ppc_wrappers/mmintrin.h
 )
 
-set(output_dir ${LLVM_LIBRARY_OUTPUT_INTDIR}/clang/${CLANG_VERSION}/include)
+set(output_dir ${CMAKE_CURRENT_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/clang/${CLANG_VERSION}/include)
 set(out_files)
 
 function(copy_header_to_output_dir src_dir file)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61165: Fix a crash where a [[no_destroy]] destructor was not emitted in an array

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, rsmith, jfb.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.

Previously, we didn't mark an array's element type's destructor referenced when 
it was annotated with no_destroy. This is not correct: we still need the 
destructor if we need to do any cleanup if an element's constructor throws. 
This was leading to crashes and linker errors. The fix is just to mark the 
destructor referenced in the array case.

This leads to an inconsistency with access control: If the array element type's 
destructor is used, then we really ought check its access. However, that would 
be a source breaking change. This patch ignores access checking, which is a bit 
unfortunate, but I think its the best we can do if we're not willing to break 
source compatibility.

Fixes rdar://48462498

Thanks!
Erik


Repository:
  rC Clang

https://reviews.llvm.org/D61165

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CodeGenCXX/no_destroy.cpp
  clang/test/SemaCXX/no_destroy.cpp

Index: clang/test/SemaCXX/no_destroy.cpp
===
--- clang/test/SemaCXX/no_destroy.cpp
+++ clang/test/SemaCXX/no_destroy.cpp
@@ -6,6 +6,9 @@
   // expected-note@+2 4 {{private}}
 #endif
 private: ~SecretDestructor(); // expected-note 2 {{private}}
+#ifndef NO_DTORS
+  // expected-note@-2 3 {{private}}
+#endif
 };
 
 SecretDestructor sd1;
@@ -44,3 +47,32 @@
 
 [[clang::no_destroy(0)]] int no_args; // expected-error{{'no_destroy' attribute takes no arguments}}
 [[clang::always_destroy(0)]] int no_args2; // expected-error{{'always_destroy' attribute takes no arguments}}
+
+SecretDestructor arr[10];
+#ifndef NO_DTORS
+// expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+
+void local_arrays() {
+  static SecretDestructor arr2[10];
+#ifndef NO_DTORS
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+  thread_local SecretDestructor arr3[10];
+#ifndef NO_DTORS
+  // expected-error@-2 {{variable of type 'SecretDestructor [10]' has private destructor}}
+#endif
+}
+
+struct Base {
+  ~Base();
+};
+struct Derived1 {
+  Derived1(int);
+  Base b;
+};
+struct Derived2 {
+  Derived1 b;
+};
+
+[[clang::no_destroy]] Derived2 d2[] = {0, 0};
Index: clang/test/CodeGenCXX/no_destroy.cpp
===
--- clang/test/CodeGenCXX/no_destroy.cpp
+++ clang/test/CodeGenCXX/no_destroy.cpp
@@ -1,11 +1,14 @@
 // RUN: %clang_cc1 %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s
+// RUN: %clang_cc1 -fexceptions %s -emit-llvm -triple x86_64-apple-macosx10.13.0 -o - | FileCheck %s --check-prefixes=CHECK,EXCEPTIONS
 
 struct NonTrivial {
   ~NonTrivial();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] NonTrivial nt1;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK-NOT: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::no_destroy]] thread_local NonTrivial nt2;
 
@@ -13,11 +16,14 @@
   ~NonTrivial2();
 };
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
 NonTrivial2 nt21;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN11NonTrivial2D1Ev
 thread_local NonTrivial2 nt22;
 
+// CHECK-LABEL: define void @_Z1fv
 void f() {
   // CHECK: __cxa_atexit{{.*}}_ZN11NonTrivial2D1Ev
   static NonTrivial2 nt21;
@@ -25,7 +31,21 @@
   thread_local NonTrivial2 nt22;
 }
 
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: __cxa_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] NonTrivial nt3;
+// CHECK-LABEL: define internal void @__cxx_global_var_init
 // CHECK: _tlv_atexit{{.*}}_ZN10NonTrivialD1Ev
 [[clang::always_destroy]] thread_local NonTrivial nt4;
+
+
+struct NonTrivial3 {
+  NonTrivial3();
+  ~NonTrivial3();
+};
+[[clang::no_destroy]] NonTrivial3 arr[10];
+
+// CHECK-LABEL: define internal void @__cxx_global_var_init
+// CHECK: {{invoke|call}} void @_ZN11NonTrivial3C1Ev
+// EXCEPTIONS: call void @_ZN11NonTrivial3D1Ev
+// CHECK-NOT: call i32 @__cxa_atexit
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13108,11 +13108,16 @@
   if (ClassDecl->hasIrrelevantDestructor()) return;
   if (ClassDecl->isDependentContext()) return;
 
-  if (VD->isNoDestroy(getASTContext()))
+  bool IsNoDestroy = VD->isNoDestroy(getASTContext());
+  if (IsNoDestroy && !VD->getType()->isArrayType())
 return;
 
   CXXDestructorDecl *Destructor = LookupDestructor(ClassDecl);
   MarkFunctionReferenced(VD->getLocation(), Destructor);
+
+  if (IsNoDestroy)
+return;
+
   CheckDestructorAccess(VD->getLocation(), 

[PATCH] D60974: Clang IFSO driver action.

2019-04-25 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment.

Why would you convert the actual format, which is already using YAML, to the 
yaml2obj format? That seems to have zero gain to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-25 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi added a comment.

In D60974#1479467 , @jakehehrlich 
wrote:

> As an example, .tbe and .tbd files use YAML but in a very specific well 
> defined structure. yaml2obj also uses a specific format which is actually 
> well defined, it just doesn't map on very well here.


I specifically meant the yaml2obj format. Mainly because yaml2obj is a handy 
tool for developing things. I suppose one way to do it could be to put together 
a more stable format and output the yaml2obj format for that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D60848: [Parser] Avoid correcting delayed typos in array subscript multiple times.

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington accepted this revision.
erik.pilkington added a comment.
This revision is now accepted and ready to land.

Huh, okay. I guess there are two separate bugs that are conspiring to crash the 
compiler here: we shouldn't correct a TypoExpr more than once, and we shouldn't 
correct a TypoExpr less than once. I think fixing one of the two is fine. FYI I 
think you should be able to write a testcase if you RUN it with `-disable-free`.


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

https://reviews.llvm.org/D60848



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


[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> Could you provide some more details on why NotPod is HFA, according to AAPCS?

In general, computing whether a composite type is a "homogeneous aggregate" 
according to section 4.3.5 depends only on the "fundamental data types".  It 
looks through "aggregates" (C structs/C++ classes), unions, and arrays.  The 
test applies "without regard to access control or other source language 
restrictions".  Section 7.1.6 mentions there are additional rules that apply to 
non-POD structs, but that's just referring to the Itanium C++ ABI rule that 
types which are "non-trivial for the purposes of calls" are passed and returned 
indirectly.  Both clang and gcc agree that "NotPod" is an HFA on AArch64 Linux.

So the question is, what rule is MSVC using to exclude "NotPod" from being an 
HFA?


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

https://reviews.llvm.org/D60349



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


[PATCH] D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule".

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 196764.
NoQ added a comment.

Fix whitespace.


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

https://reviews.llvm.org/D61161

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -228,29 +228,36 @@
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
 StringRef FName, QualType RetTy) {
+  assert(TrackOSObjects &&
+ "Requesting a summary for an OSObject but OSObjects are not tracked");
+
   if (RetTy->isPointerType()) {
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
-  if (const IdentifierInfo *II = FD->getIdentifier()) {
-StringRef FuncName = II->getName();
-if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
-  return getDefaultSummary();
-
-// All objects returned with functions *not* starting with
-// get, or iterators, are returned at +1.
-if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-isOSIteratorSubclass(PD)) {
-  return getOSSummaryCreateRule(FD);
-} else {
-  return getOSSummaryGetRule(FD);
-}
+  if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+return getDefaultSummary();
+
+  // TODO: Add support for the slightly common *Matching(table) idiom.
+  // Cf. IOService::nameMatching() etc. - these function have an unusual
+  // contract of returning at +0 or +1 depending on their last argument.
+  if (FName.endswith("Matching")) {
+return getPersistentStopSummary();
+  }
+
+  // All objects returned with functions *not* starting with 'get',
+  // or iterators, are returned at +1.
+  if ((!FName.startswith("get") && !FName.startswith("Get")) ||
+  isOSIteratorSubclass(PD)) {
+return getOSSummaryCreateRule(FD);
+  } else {
+return getOSSummaryGetRule(FD);
   }
 }
   }
 
   if (const auto *MD = dyn_cast(FD)) {
 const CXXRecordDecl *Parent = MD->getParent();
-if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+if (Parent && isOSObjectSubclass(Parent)) {
   if (FName == "release" || FName == "taggedRelease")
 return getOSSummaryReleaseRule(FD);
 


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- 

[PATCH] D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule".

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 196763.
NoQ added a comment.

Make tests more C++-y. They still test the C function case.


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

https://reviews.llvm.org/D61161

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -228,29 +228,35 @@
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
 StringRef FName, QualType RetTy) {
+  assert(TrackOSObjects &&
+ "Requesting a summary for an OSObject but OSObjects are not tracked");
+
   if (RetTy->isPointerType()) {
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
-  if (const IdentifierInfo *II = FD->getIdentifier()) {
-StringRef FuncName = II->getName();
-if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
-  return getDefaultSummary();
-
-// All objects returned with functions *not* starting with
-// get, or iterators, are returned at +1.
-if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-isOSIteratorSubclass(PD)) {
-  return getOSSummaryCreateRule(FD);
-} else {
-  return getOSSummaryGetRule(FD);
-}
+  if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+return getDefaultSummary();
+  // TODO: Add support for the slightly common *Matching(table) idiom.
+  // Cf. IOService::nameMatching() etc. - these function have an unusual
+  // contract of returning at +0 or +1 depending on their last argument.
+  if (FName.endswith("Matching")) {
+return getPersistentStopSummary();
+  }
+
+  // All objects returned with functions *not* starting with 'get',
+  // or iterators, are returned at +1.
+  if ((!FName.startswith("get") && !FName.startswith("Get")) ||
+  isOSIteratorSubclass(PD)) {
+return getOSSummaryCreateRule(FD);
+  } else {
+return getOSSummaryGetRule(FD);
   }
 }
   }
 
   if (const auto *MD = dyn_cast(FD)) {
 const CXXRecordDecl *Parent = MD->getParent();
-if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+if (Parent && isOSObjectSubclass(Parent)) {
   if (FName == "release" || FName == "taggedRelease")
 return getOSSummaryReleaseRule(FD);
 


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,26 @@
   obj->release();
   obj->release();
 }
+
+class IOService {
+public:
+  OSObject *somethingMatching(OSObject *table = 0);
+};
+
+OSObject *testSuppressionForMethodsEndingWithMatching(IOService *svc,
+  OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = svc->somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = svc->somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+
+  // This function itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp

[PATCH] D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule".

2019-04-25 Thread Devin Coughlin via Phabricator via cfe-commits
dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

Looks good to me!


Repository:
  rC Clang

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

https://reviews.llvm.org/D61161



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


[PATCH] D55229: [COFF] Statically link certain runtime library functions

2019-04-25 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

Looks like this might have broken bots:

  
/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/CodeGenCXX/runtime-dllstorage.cpp:121:26:
 error: CHECK-IA-DAG: expected string not found in input
  // CHECK-DYNAMIC-IA-DAG: declare dllimport i32 @__cxa_thread_atexit(void 
(i8*)*, i8*, i8*)
   ^
  :1:1: note: scanning from here
  ; ModuleID = 
'/Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm/tools/clang/test/CodeGenCXX/runtime-dllstorage.cpp'
  ^
  :42:1: note: possible intended match here
  declare dso_local i32 @__cxa_thread_atexit(void (i8*)*, i8*, i8*) #2
  ^

http://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/60461/


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55229



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


[PATCH] D60274: [ELF] Implement Dependent Libraries Feature

2019-04-25 Thread ben via Phabricator via cfe-commits
bd1976llvm added a comment.

I am keen to keep this moving.

I think there are a few things outstanding:

1. Need to resolve concerns w.r.t the design.
2. I need to find out whether I should be doing validation when reading the 
metadata in LLVM.
3. The LLD side needs a more detailed review.

@jyknight @compnerd can we  progress the design conversation?


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

https://reviews.llvm.org/D60274



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


[PATCH] D61161: [analyzer] RetainCount: Add a suppression for functions that follow "the Matching rule".

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added a reviewer: dcoughlin.
Herald added subscribers: cfe-commits, Charusso, dkrupp, donat.nagy, Szelethus, 
mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

In the OSObject universe there appears to be another slightly popular contract, 
apart from "create" and "get", which is "matching". It optionally consumes a 
"table" parameter and if a table is passed, it fills in the table and returns 
it at +0; otherwise, it creates a new table, fills it in and returns it at +1.

For now suppress false positives by doing a conservative escape on all 
functions that end with "Matching", which is the naming convention that seems 
to be followed by all such methods. Let's see if we'll ever get to actually 
supporting these semantics.


Repository:
  rC Clang

https://reviews.llvm.org/D61161

Files:
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/test/Analysis/osobject-retain-release.cpp


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,20 @@
   obj->release();
   obj->release();
 }
+
+OSObject *somethingMatching(OSObject *table = 0);
+OSObject *testSuppressionForMethodsEndingWithMatching(OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create Rule regardless.
+  // We should warn about the leak here.
+  OSObject *ptr2 = somethingMatching(); // no-warning
+
+  if (!table)
+table = OSTypeAlloc(OSArray);
+  // This method itself ends with "Matching"! Do not warn when we're
+  // returning from it at +0.
+  return table; // no-warning
+}
Index: clang/lib/Analysis/RetainSummaryManager.cpp
===
--- clang/lib/Analysis/RetainSummaryManager.cpp
+++ clang/lib/Analysis/RetainSummaryManager.cpp
@@ -228,29 +228,35 @@
 const RetainSummary *
 RetainSummaryManager::getSummaryForOSObject(const FunctionDecl *FD,
 StringRef FName, QualType RetTy) {
+  assert(TrackOSObjects &&
+ "Requesting a summary for an OSObject but OSObjects are not tracked");
+
   if (RetTy->isPointerType()) {
 const CXXRecordDecl *PD = RetTy->getPointeeType()->getAsCXXRecordDecl();
 if (PD && isOSObjectSubclass(PD)) {
-  if (const IdentifierInfo *II = FD->getIdentifier()) {
-StringRef FuncName = II->getName();
-if (isOSObjectDynamicCast(FuncName) || isOSObjectThisCast(FuncName))
-  return getDefaultSummary();
-
-// All objects returned with functions *not* starting with
-// get, or iterators, are returned at +1.
-if ((!FuncName.startswith("get") && !FuncName.startswith("Get")) ||
-isOSIteratorSubclass(PD)) {
-  return getOSSummaryCreateRule(FD);
-} else {
-  return getOSSummaryGetRule(FD);
-}
+  if (isOSObjectDynamicCast(FName) || isOSObjectThisCast(FName))
+return getDefaultSummary();
+  // TODO: Add support for the slightly common *Matching(table) idiom.
+  // Cf. IOService::nameMatching() etc. - these function have an unusual
+  // contract of returning at +0 or +1 depending on their last argument.
+  if (FName.endswith("Matching")) {
+return getPersistentStopSummary();
+  }
+
+  // All objects returned with functions *not* starting with 'get',
+  // or iterators, are returned at +1.
+  if ((!FName.startswith("get") && !FName.startswith("Get")) ||
+  isOSIteratorSubclass(PD)) {
+return getOSSummaryCreateRule(FD);
+  } else {
+return getOSSummaryGetRule(FD);
   }
 }
   }
 
   if (const auto *MD = dyn_cast(FD)) {
 const CXXRecordDecl *Parent = MD->getParent();
-if (TrackOSObjects && Parent && isOSObjectSubclass(Parent)) {
+if (Parent && isOSObjectSubclass(Parent)) {
   if (FName == "release" || FName == "taggedRelease")
 return getOSSummaryReleaseRule(FD);
 


Index: clang/test/Analysis/osobject-retain-release.cpp
===
--- clang/test/Analysis/osobject-retain-release.cpp
+++ clang/test/Analysis/osobject-retain-release.cpp
@@ -679,3 +679,20 @@
   obj->release();
   obj->release();
 }
+
+OSObject *somethingMatching(OSObject *table = 0);
+OSObject *testSuppressionForMethodsEndingWithMatching(OSObject *table = 0) {
+  // This probably just passes table through. We should probably not make
+  // ptr1 definitely equal to table, but we should not warn about leaks.
+  OSObject *ptr1 = somethingMatching(table); // no-warning
+
+  // FIXME: This, however, should follow the Create 

[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

2019-04-25 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik updated this revision to Diff 196757.
shafik added a comment.

Added test


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

https://reviews.llvm.org/D61140

Files:
  lib/AST/ASTImporter.cpp
  test/Import/cxx-record-flags/Inputs/F.cpp
  test/Import/cxx-record-flags/test.cpp


Index: test/Import/cxx-record-flags/test.cpp
===
--- /dev/null
+++ test/Import/cxx-record-flags/test.cpp
@@ -0,0 +1,14 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
+
+// CHECK: FTrivial
+// CHECK: DefinitionData
+// CHECK-SAME: pass_in_registers
+
+// CHECK: FNonTrivial
+// CHECK-NOT: pass_in_registers
+// CHECK: DefaultConstructor
+
+void expr() {
+  FTrivial f1;
+  FNonTrivial f2;
+}
Index: test/Import/cxx-record-flags/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-record-flags/Inputs/F.cpp
@@ -0,0 +1,9 @@
+class FTrivial {
+  int i;
+};
+
+struct FNonTrivial {
+  virtual ~FNonTrivial() = default;
+  int i;
+};
+
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1767,6 +1767,9 @@
 ToData.HasDeclaredCopyAssignmentWithConstParam
   = FromData.HasDeclaredCopyAssignmentWithConstParam;
 
+// Copy over the data stored in RecordDeclBits
+ToCXX->setArgPassingRestrictions(FromCXX->getArgPassingRestrictions());
+
 SmallVector Bases;
 for (const auto  : FromCXX->bases()) {
   ExpectedType TyOrErr = import(Base1.getType());


Index: test/Import/cxx-record-flags/test.cpp
===
--- /dev/null
+++ test/Import/cxx-record-flags/test.cpp
@@ -0,0 +1,14 @@
+// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | FileCheck %s
+
+// CHECK: FTrivial
+// CHECK: DefinitionData
+// CHECK-SAME: pass_in_registers
+
+// CHECK: FNonTrivial
+// CHECK-NOT: pass_in_registers
+// CHECK: DefaultConstructor
+
+void expr() {
+  FTrivial f1;
+  FNonTrivial f2;
+}
Index: test/Import/cxx-record-flags/Inputs/F.cpp
===
--- /dev/null
+++ test/Import/cxx-record-flags/Inputs/F.cpp
@@ -0,0 +1,9 @@
+class FTrivial {
+  int i;
+};
+
+struct FNonTrivial {
+  virtual ~FNonTrivial() = default;
+  int i;
+};
+
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -1767,6 +1767,9 @@
 ToData.HasDeclaredCopyAssignmentWithConstParam
   = FromData.HasDeclaredCopyAssignmentWithConstParam;
 
+// Copy over the data stored in RecordDeclBits
+ToCXX->setArgPassingRestrictions(FromCXX->getArgPassingRestrictions());
+
 SmallVector Bases;
 for (const auto  : FromCXX->bases()) {
   ExpectedType TyOrErr = import(Base1.getType());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359251 - Revert [COFF] Statically link certain runtime library functions

2019-04-25 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Apr 25 16:30:41 2019
New Revision: 359251

URL: http://llvm.org/viewvc/llvm-project?rev=359251=rev
Log:
Revert [COFF] Statically link certain runtime library functions

This reverts r359250 (git commit 4730604bd3a361c68b92b18bf73a5daa15afe9f4)

The newly added test should use -cc1 and -emit-llvm and there are other
test failures that need fixing.

Removed:
cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
cfe/trunk/test/CodeGenObjC/gnu-init.m
cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=359251=359250=359251=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Apr 25 16:30:41 2019
@@ -3000,13 +3000,9 @@ CodeGenModule::CreateRuntimeFunction(llv
 if (F->empty()) {
   F->setCallingConv(getRuntimeCC());
 
-  // In Windows Itanium environments, try to mark runtime functions
-  // dllimport. For Mingw and MSVC, don't. We don't really know if the user
-  // will link their standard library statically or dynamically. Marking
-  // functions imported when they are not imported can cause linker errors
-  // and warnings.
-  if (!Local && getTriple().isWindowsItaniumEnvironment() &&
-  !getCodeGenOpts().LTOVisibilityPublicStd) {
+  if (!Local && getTriple().isOSBinFormatCOFF() &&
+  !getCodeGenOpts().LTOVisibilityPublicStd &&
+  !getTriple().isWindowsGNUEnvironment()) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
 if (!FD || FD->hasAttr()) {
   F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);

Removed: cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp?rev=359250=auto
==
--- cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp (original)
+++ cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp (removed)
@@ -1,20 +0,0 @@
-// RUN: %clangxx -target aarch64-windows \
-// RUN: -fcxx-exceptions -c -o - %s \
-// RUN: | llvm-objdump -syms - 2>&1 | FileCheck %s
-
-void foo1() { throw 1; }
-// CHECK-LABEL: foo1
-// CHECK-NOT: __imp__CxxThrowException
-
-void bar();
-void foo2() noexcept(true) { bar(); }
-// CHECK-LABEL: foo2
-// CHECK-NOT: __imp___std_terminate
-
-struct A {};
-struct B { virtual void f(); };
-struct C : A, virtual B {};
-struct T {};
-T *foo3() { return dynamic_cast((C *)0); }
-// CHECK-LABEL: foo3
-// CHECK-NOT: __imp___RTDynamicCast

Modified: cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp?rev=359251=359250=359251=diff
==
--- cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp Thu Apr 25 16:30:41 2019
@@ -108,7 +108,7 @@ void l() {
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(void ()*)
 // CHECK-MS-DAG: declare dso_local i32 @atexit(void ()*)
-// CHECK-MS-DYNAMIC-DAG: declare {{.*}} void @_CxxThrowException
+// CHECK-MS-DYNAMIC-DAG: declare dllimport {{.*}} void @_CxxThrowException
 // CHECK-MS-STATIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-DAG: declare dso_local noalias i8* @"??2@YAPAXI@Z"
 // CHECK-MS-DAG: declare dso_local void @_Init_thread_header(i32*)

Modified: cfe/trunk/test/CodeGenObjC/gnu-init.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/gnu-init.m?rev=359251=359250=359251=diff
==
--- cfe/trunk/test/CodeGenObjC/gnu-init.m (original)
+++ cfe/trunk/test/CodeGenObjC/gnu-init.m Thu Apr 25 16:30:41 2019
@@ -100,6 +100,6 @@
 // Check our load function is in a comdat.
 // CHECK-WIN: define linkonce_odr hidden void @.objcv2_load_function() comdat {
 
-// Make sure we do not have dllimport on the load function
-// CHECK-WIN: declare dso_local void @__objc_load
+// Make sure we have dllimport on the load function
+// CHECK-WIN: declare dllimport void @__objc_load
 

Modified: cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm?rev=359251=359250=359251=diff
==
--- cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm (original)
+++ cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm Thu Apr 25 16:30:41 2019
@@ -13,5 +13,6 @@ S f() {
   return [I m:S()];
 }
 
-// CHECK: declare dso_local void 

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D60349#1479604 , @efriedma wrote:

> > Return info for HFA and HVA is updated
>
> That's helpful, but not really sufficient; according to the AAPCS rules, both 
> "Pod" and "NotPod" from my testcase are HFAs.


Could you provide some more details on why `NotPod` is HFA, according to AAPCS?


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

https://reviews.llvm.org/D60349



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


[PATCH] D60283: [DebugInfo] Don't emit checksums when compiling a preprocessed CPP

2019-04-25 Thread Reid Kleckner via Phabricator via cfe-commits
rnk added a comment.

In D60283#1456497 , @thakis wrote:

> See 
> http://llvm-cs.pcc.me.uk/tools/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp#756
>  for a "is same file" example. I'm not sure adding a bool to PresumedLoc is 
> worth it for this.


@thakis, do you still object to adding info to PresumedLoc for this? I think in 
the absence of review from @rsmith we should go forward with this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60283



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


[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment.

> Return info for HFA and HVA is updated

That's helpful, but not really sufficient; according to the AAPCS rules, both 
"Pod" and "NotPod" from my testcase are HFAs.


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

https://reviews.llvm.org/D60349



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


[PATCH] D55229: [COFF] Statically link certain runtime library functions

2019-04-25 Thread Reid Kleckner via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359250: [COFF] Statically link certain runtime library 
functions (authored by rnk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D55229?vs=196553=196747#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D55229

Files:
  cfe/trunk/lib/CodeGen/CodeGenModule.cpp
  cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
  cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
  cfe/trunk/test/CodeGenObjC/gnu-init.m
  cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm


Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -3000,9 +3000,13 @@
 if (F->empty()) {
   F->setCallingConv(getRuntimeCC());
 
-  if (!Local && getTriple().isOSBinFormatCOFF() &&
-  !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  // In Windows Itanium environments, try to mark runtime functions
+  // dllimport. For Mingw and MSVC, don't. We don't really know if the user
+  // will link their standard library statically or dynamically. Marking
+  // functions imported when they are not imported can cause linker errors
+  // and warnings.
+  if (!Local && getTriple().isWindowsItaniumEnvironment() &&
+  !getCodeGenOpts().LTOVisibilityPublicStd) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
 if (!FD || FD->hasAttr()) {
   F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);
Index: cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
===
--- cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
+++ cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
@@ -0,0 +1,20 @@
+// RUN: %clangxx -target aarch64-windows \
+// RUN: -fcxx-exceptions -c -o - %s \
+// RUN: | llvm-objdump -syms - 2>&1 | FileCheck %s
+
+void foo1() { throw 1; }
+// CHECK-LABEL: foo1
+// CHECK-NOT: __imp__CxxThrowException
+
+void bar();
+void foo2() noexcept(true) { bar(); }
+// CHECK-LABEL: foo2
+// CHECK-NOT: __imp___std_terminate
+
+struct A {};
+struct B { virtual void f(); };
+struct C : A, virtual B {};
+struct T {};
+T *foo3() { return dynamic_cast((C *)0); }
+// CHECK-LABEL: foo3
+// CHECK-NOT: __imp___RTDynamicCast
Index: cfe/trunk/test/CodeGenObjC/gnu-init.m
===
--- cfe/trunk/test/CodeGenObjC/gnu-init.m
+++ cfe/trunk/test/CodeGenObjC/gnu-init.m
@@ -100,6 +100,6 @@
 // Check our load function is in a comdat.
 // CHECK-WIN: define linkonce_odr hidden void @.objcv2_load_function() comdat {
 
-// Make sure we have dllimport on the load function
-// CHECK-WIN: declare dllimport void @__objc_load
+// Make sure we do not have dllimport on the load function
+// CHECK-WIN: declare dso_local void @__objc_load
 
Index: cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm
===
--- cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm
+++ cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm
@@ -13,6 +13,5 @@
   return [I m:S()];
 }
 
-// CHECK: declare dllimport void @objc_msgSend_stret(i8*, i8*, ...)
+// CHECK: declare dso_local void @objc_msgSend_stret(i8*, i8*, ...)
 // CHECK-NOT: declare dllimport void @objc_msgSend(i8*, i8*, ...)
-
Index: cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
===
--- cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
+++ cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
@@ -108,7 +108,7 @@
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(void ()*)
 // CHECK-MS-DAG: declare dso_local i32 @atexit(void ()*)
-// CHECK-MS-DYNAMIC-DAG: declare dllimport {{.*}} void @_CxxThrowException
+// CHECK-MS-DYNAMIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-STATIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-DAG: declare dso_local noalias i8* @"??2@YAPAXI@Z"
 // CHECK-MS-DAG: declare dso_local void @_Init_thread_header(i32*)


Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -3000,9 +3000,13 @@
 if (F->empty()) {
   F->setCallingConv(getRuntimeCC());
 
-  if (!Local && getTriple().isOSBinFormatCOFF() &&
-  !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  // In Windows Itanium environments, try to mark runtime functions
+  

r359250 - [COFF] Statically link certain runtime library functions

2019-04-25 Thread Reid Kleckner via cfe-commits
Author: rnk
Date: Thu Apr 25 16:04:20 2019
New Revision: 359250

URL: http://llvm.org/viewvc/llvm-project?rev=359250=rev
Log:
[COFF] Statically link certain runtime library functions

Statically link certain runtime library functions for MSVC/GNU Windows
environments. This is consistent with MSVC behavior.

Fixes LNK4286 and LNK4217 warnings from link.exe when linking the static
CRT:
  LINK : warning LNK4286: symbol '__std_terminate' defined in 
'libvcruntime.lib(ehhelpers.obj)' is imported by 
'ASAN_NOINST_TEST_OBJECTS.asan_noinst_test.cc.x86_64-calls.o'
  LINK : warning LNK4286: symbol '__std_terminate' defined in 
'libvcruntime.lib(ehhelpers.obj)' is imported by 
'ASAN_NOINST_TEST_OBJECTS.asan_test_main.cc.x86_64-calls.o'
  LINK : warning LNK4217: symbol '_CxxThrowException' defined in 
'libvcruntime.lib(throw.obj)' is imported by 
'ASAN_NOINST_TEST_OBJECTS.gtest-all.cc.x86_64-calls.o' in function '"int 
`public: static class UnitTest::GetInstance * __cdecl 
testing::UnitTest::GetInstance(void)'::`1'::dtor$5" 
(?dtor$5@?0??GetInstance@UnitTest@testing@@SAPEAV12@XZ@4HA)'

Reviewers: mstorsjo, efriedma, TomTan, compnerd, smeenai, mgrang

Subscribers: abdulras, theraven, smeenai, pcc, mehdi_amini, javed.absar, 
inglorion, kristof.beyls, dexonsmith, cfe-commits

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

Added:
cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
Modified:
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
cfe/trunk/test/CodeGenObjC/gnu-init.m
cfe/trunk/test/CodeGenObjCXX/msabi-stret.mm

Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=359250=359249=359250=diff
==
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Thu Apr 25 16:04:20 2019
@@ -3000,9 +3000,13 @@ CodeGenModule::CreateRuntimeFunction(llv
 if (F->empty()) {
   F->setCallingConv(getRuntimeCC());
 
-  if (!Local && getTriple().isOSBinFormatCOFF() &&
-  !getCodeGenOpts().LTOVisibilityPublicStd &&
-  !getTriple().isWindowsGNUEnvironment()) {
+  // In Windows Itanium environments, try to mark runtime functions
+  // dllimport. For Mingw and MSVC, don't. We don't really know if the user
+  // will link their standard library statically or dynamically. Marking
+  // functions imported when they are not imported can cause linker errors
+  // and warnings.
+  if (!Local && getTriple().isWindowsItaniumEnvironment() &&
+  !getCodeGenOpts().LTOVisibilityPublicStd) {
 const FunctionDecl *FD = GetRuntimeFunctionDecl(Context, Name);
 if (!FD || FD->hasAttr()) {
   F->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass);

Added: cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp?rev=359250=auto
==
--- cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp (added)
+++ cfe/trunk/test/CodeGen/ms-symbol-linkage.cpp Thu Apr 25 16:04:20 2019
@@ -0,0 +1,20 @@
+// RUN: %clangxx -target aarch64-windows \
+// RUN: -fcxx-exceptions -c -o - %s \
+// RUN: | llvm-objdump -syms - 2>&1 | FileCheck %s
+
+void foo1() { throw 1; }
+// CHECK-LABEL: foo1
+// CHECK-NOT: __imp__CxxThrowException
+
+void bar();
+void foo2() noexcept(true) { bar(); }
+// CHECK-LABEL: foo2
+// CHECK-NOT: __imp___std_terminate
+
+struct A {};
+struct B { virtual void f(); };
+struct C : A, virtual B {};
+struct T {};
+T *foo3() { return dynamic_cast((C *)0); }
+// CHECK-LABEL: foo3
+// CHECK-NOT: __imp___RTDynamicCast

Modified: cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp?rev=359250=359249=359250=diff
==
--- cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/runtime-dllstorage.cpp Thu Apr 25 16:04:20 2019
@@ -108,7 +108,7 @@ void l() {
 // CHECK-MS-DAG: @_Init_thread_epoch = external thread_local global i32
 // CHECK-MS-DAG: declare dso_local i32 @__tlregdtor(void ()*)
 // CHECK-MS-DAG: declare dso_local i32 @atexit(void ()*)
-// CHECK-MS-DYNAMIC-DAG: declare dllimport {{.*}} void @_CxxThrowException
+// CHECK-MS-DYNAMIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-STATIC-DAG: declare {{.*}} void @_CxxThrowException
 // CHECK-MS-DAG: declare dso_local noalias i8* @"??2@YAPAXI@Z"
 // CHECK-MS-DAG: declare dso_local void @_Init_thread_header(i32*)

Modified: cfe/trunk/test/CodeGenObjC/gnu-init.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/gnu-init.m?rev=359250=359249=359250=diff

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

2019-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: rnkovacs.

Some cleanups and simplifications, but LGTM with those applied. Thank you!




Comment at: docs/LanguageExtensions.rst:2335
+
+When the builtins appears as part of a default function argument the invocation
+point is the location of the caller. When the builtins appear as part of a

appears -> appear



Comment at: include/clang/AST/CurrentSourceLocExprScope.h:31
+public:
+  /// A RAII style scope gaurd used for tracking the current source
+  /// location and context as used by the source location builtins

gaurd -> guard



Comment at: include/clang/AST/Expr.h:4168
+/// Represents a function call to one of __builtin_LINE(), __builtin_COLUMN(),
+/// __builtin_FUNCTION(), or __builtin_FILE()
+class SourceLocExpr final : public Expr {

Missing period at end of comment.



Comment at: lib/AST/ASTContext.cpp:10151-10153
+  // Get an array type for the string, according to C99 6.4.5.  This includes
+  // the nul terminator character as well as the string length for pascal
+  // strings.

"the string length for pascal strings" part here seems out of place, since you 
don't handle that here.



Comment at: lib/AST/Expr.cpp:2073
+StringLiteral *Res = Ctx.getPredefinedStringLiteralFromCache(Tmp);
+return APValue(Res, CharUnits::Zero(), APValue::NoLValuePath(), 0);
+  };

Given that the type of the `SourceLocExpr` is `const char*`, I'd expect the 
`APValue` result to be a decayed pointer to the first character in the string, 
not a pointer to the string itself. (So I think this should have a path of 
length 1 containing `LValuePathEntry{.ArrayIndex = 0}`.)



Comment at: lib/AST/ExprConstant.cpp:5861-5863
+Result.addArray(
+Info, E,
+cast(Str->getType()->getAsArrayTypeUnsafe()));

(This won't be necessary if/when `EvaluateInContext` returns a pointer to the 
first character in the string.)



Comment at: lib/AST/ExprConstant.cpp:7616
+  Info.Ctx, Info.CurrentCall->CurSourceLocExprScope.getDefaultExpr());
+  return Success(Evaluated.getInt(), E);
+}

Consider dropping the `.getInt()` here? (Then this version and the pointer 
version will be identical, and we could consider moving this to 
`ExprEvaluatorBase`.)



Comment at: lib/AST/ExprConstant.cpp:11286
+  case Expr::SourceLocExprClass:
 // GCC considers the GNU __null value to be an integral constant 
expression.
 return NoDiag();

This comment is getting detached from its case label. Maybe just remove it? It 
seems reasonably obvious that `__null` should be an ICE.



Comment at: lib/AST/ExprConstant.cpp:3370-3371
+
+  assert((!Base || !isa(Base)) &&
+ "Base should have already been transformed into a StringLiteral");
+

EricWF wrote:
> rsmith wrote:
> > There are lots of forms of expression that cannot be the base of an 
> > `LValue` (see the list above `LValueExprEvaluator` for the expression forms 
> > that *can* be the base of an `LValue`); is it important to give this one 
> > special treatment?
> Because a `SourceLocExpr` *can* be the base of an `LValue`. But the way 
> that's supported is by transforming the `SourceLocExpr` into a 
> `StringLiteral`. 
I don't agree: a `SourceLocExpr` cannot be the base of an `LValue`. It is 
evaluated into something else that can be (a `StringLiteral`), but it itself 
cannot be (and this is in no way unusual; that's probably true of most `Expr` 
nodes). I think this is a remnant of an earlier design?



Comment at: lib/CodeGen/CGExpr.cpp:3241-3281
 Address CodeGenFunction::EmitArrayToPointerDecay(const Expr *E,
  LValueBaseInfo *BaseInfo,
  TBAAAccessInfo *TBAAInfo) {
-  assert(E->getType()->isArrayType() &&
+  LValue LV = EmitLValue(E);
+  return EmitArrayToPointerDecay(E, E->getType(), LV, BaseInfo, TBAAInfo);
+}
+

This change should be unnecessary after changing `EvaluateInContext` to decay 
the pointer.



Comment at: lib/CodeGen/CGExprScalar.cpp:623-634
+
+if (SLE->isIntType())
+  return Builder.getInt(Evaluated.getInt());
+
+// If we're not building an int, then we're building a string literal.
+const APValue::LValueBase  = Evaluated.getLValueBase();
+const StringLiteral *Str = cast(Base.get());

This can be simplified to:
```
return ConstantEmitter(CGF.CGM, CGF).emitAbstract(SLE->getLocation(), 
Evaluated, SLE->getType());
```
(once `EvaluateInContext` decays the pointer).



Comment at: lib/Serialization/ASTReaderStmt.cpp:971-977
+void 

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

2019-04-25 Thread Jan Korous via Phabricator via cfe-commits
jkorous added a comment.

Also, IIUC the test case that I deleted wasn't actually supposed to produce any 
diagnostics and the fact that it did was a bug. We could keep it as a 
regression test but I think it has a rather low value. WDYT?


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

https://reviews.llvm.org/D61103



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


[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

2019-04-25 Thread Jan Korous via Phabricator via cfe-commits
jkorous updated this revision to Diff 196744.
jkorous added a comment.

- clang-format
- comments


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

https://reviews.llvm.org/D61103

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-documentation.cpp

Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -760,16 +760,6 @@
 /// \endcode
 int test_verbatim_2();
 
-// FIXME: we give a bad diagnostic here because we throw away non-documentation
-// comments early.
-//
-// expected-warning@+3 {{'\endcode' command does not terminate a verbatim text block}}
-/// \code
-//  foo
-/// \endcode
-int test_verbatim_3();
-
-
 // expected-warning@+1 {{empty paragraph passed to '\brief' command}}
 int test1; ///< \brief\author Aaa
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12380,19 +12380,7 @@
   }
 
   // See if there are any new comments that are not attached to a decl.
-  ArrayRef Comments = Context.getRawCommentList().getComments();
-  if (!Comments.empty() &&
-  !Comments.back()->isAttached()) {
-// There is at least one comment that not attached to a decl.
-// Maybe it should be attached to one of these decls?
-//
-// Note that this way we pick up not only comments that precede the
-// declaration, but also comments that *follow* the declaration -- thanks to
-// the lookahead in the lexer: we've consumed the semicolon and looked
-// ahead through comments.
-for (unsigned i = 0, e = Group.size(); i != e; ++i)
-  Context.getCommentForDecl(Group[i], );
-  }
+  Context.tryToAttachCommentsToDecls(Group, );
 }
 
 /// ActOnParamDeclarator - Called from Parser::ParseFunctionDeclarator()
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -489,6 +489,140 @@
   return RC;
 }
 
+void ASTContext::tryToAttachCommentsToDecls(ArrayRef Decls,
+const Preprocessor *PP) {
+  // Explicitly not calling ExternalSource->ReadComments() as we're interested
+  // only in comments and decls that were parsed just now.
+  ArrayRef RawComments = Comments.getComments();
+  if (RawComments.empty())
+return;
+
+  auto CacheCommentForDecl = [this, PP](const Decl *D, const RawComment *C) {
+RawCommentAndCacheFlags CacheEntry;
+CacheEntry.setKind(RawCommentAndCacheFlags::FromDecl);
+CacheEntry.setRaw(C);
+CacheEntry.setOriginalDecl(D);
+RedeclComments[D] = CacheEntry;
+
+// Always try to parse in order to eventually produce diagnostics.
+comments::FullComment *FC = C->parse(*this, PP, D);
+// But cache only if we don't have a comment yet
+const Decl *Canonical = D->getCanonicalDecl();
+auto ParsedComment = ParsedComments.find(Canonical);
+if (ParsedComment != ParsedComments.end())
+  ParsedComment->second = FC;
+  };
+
+  // explicit comment location caching
+  std::unordered_map>
+  DecomposedCommentBegin;
+  std::unordered_map>
+  DecomposedCommentEnd;
+  std::unordered_map>
+  CommentBeginLine;
+
+  // Don't store the result for long - might go dangling.
+  auto GetCachedCommentBegin =
+  [,
+   this](RawComment *RC) -> const std::pair & {
+assert(RC);
+auto BeginIt = DecomposedCommentBegin.find(RC);
+if (BeginIt != DecomposedCommentBegin.end()) {
+  return BeginIt->second;
+}
+DecomposedCommentBegin[RC] =
+SourceMgr.getDecomposedLoc(RC->getSourceRange().getBegin());
+return DecomposedCommentBegin[RC];
+  };
+  // Don't store the result for long - might go dangling.
+  auto GetCachedCommentEnd =
+  [,
+   this](RawComment *RC) -> const std::pair & {
+assert(RC);
+auto EndIt = DecomposedCommentEnd.find(RC);
+if (EndIt != DecomposedCommentEnd.end()) {
+  return EndIt->second;
+}
+DecomposedCommentEnd[RC] =
+SourceMgr.getDecomposedLoc(RC->getSourceRange().getEnd());
+return DecomposedCommentEnd[RC];
+  };
+  auto GetCachedCommentBeginLine =
+  [,
+   this](const std::pair ) -> unsigned {
+auto BeginFileIt =
+CommentBeginLine.find(CommentBeginLoc.first.getHashValue());
+if (BeginFileIt != CommentBeginLine.end()) {
+  auto BeginLineIt = BeginFileIt->second.find(CommentBeginLoc.second);
+  if (BeginLineIt != BeginFileIt->second.end()) {
+return BeginLineIt->second;
+  }
+}
+CommentBeginLine[CommentBeginLoc.first.getHashValue()]
+[CommentBeginLoc.second] = SourceMgr.getLineNumber(
+CommentBeginLoc.first, CommentBeginLoc.second);
+return 

[PATCH] D61103: [clang] Add tryToAttachCommentsToDecls method to ASTContext

2019-04-25 Thread Jan Korous via Phabricator via cfe-commits
jkorous marked 5 inline comments as done.
jkorous added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:494
+  // Explicitly not calling ExternalSource->ReadComments() as we're not
+  // interested in those.
+  ArrayRef RawComments = Comments.getComments();

gribozavr wrote:
> Would be great to explain why (because we assume that the decls and their 
> comments were parsed just now).  Otherwise the comment could enumerate a lot 
> of other things that we are not calling here either...
Haha, you're absolutely right! Thanks.



Comment at: clang/lib/AST/ASTContext.cpp:584
+// terminating early.
+for (auto CIt = RawComments.begin(); CIt != RawComments.end(); ++CIt) {
+  RawComment *C = *CIt;

gribozavr wrote:
> Scanning all comments for every decl?  Isn't that O(n^2)?
> 
> Also logic duplication below.  I was expecting a call to 
> `getRawCommentForDeclNoCache`, with an appropriate flag to disable loading 
> external comments (it is a low-level API, users generally don't call it).
The important thing is that the expensive operation is source location 
decomposition. That's why I cache everything - `GetCachedCommentBegin` etc.

So while you're right that iteration-wise it's O(iteration*c*d) it`s actually 
O(decompose*c + decompose*d) because of the caching. The current code (which is 
sorting all the comments) is at least O(decompose*c*ln(c)) once you have more 
comments than `sqrt(300)` (==`MagicCacheSize` in 
`SourceManager::getInBeforeInTUCache()`).

That being said - you're right that just not-loading external comments in 
`getRawCommentForDeclNoCache` definitely has it's appeal. I'm running a test 
now get some idea about performance of both approaches.

BTW in theory we could also do one of these:
1. Allow clients to transparently set `MagicCacheSize` in 
`SourceManager::getInBeforeInTUCache()` which is used for SourceLocation 
sorting (`BeforeThanCompare`) is currently hard-coded to 300 while 
we are comparing ~100k x ~100k locations.
2. Change caching strategies in `SourceManager::getFileID` and 
`SourceManager::getLineNumber`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61103



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


[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

2019-04-25 Thread Aleksei Sidorin via Phabricator via cfe-commits
a_sidorin added a comment.

Hello Shafik!
The patch itself is fine, but, as other reviewers pointed, tests are 
appreciated. I suggest to add a test into ASTImporterTests.cpp - you will find 
several ways to write tests of different complexity here. I think this change 
can be tested even with the simplest testImport() facility.


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

https://reviews.llvm.org/D61140



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


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

In D61147#1479530 , @ahatanak wrote:

> In D61147#1479468 , @erik.pilkington 
> wrote:
>
> > > Yeah, I tend to think we should just suppress this unconditionally in 
> > > Objective-C.
> >
> > IMO this warning still makes sense for normal functions, or methods that 
> > are only declared in an @implementation. Adding a fix-it to cast to void in 
> > the function/method body would probably go a long way too.
>
>
> Should we use the new warning to warn just about unused ObjC method 
> parameters and have `-Wunused-parameter` warn about everything else? If we 
> make `-Wunused-parameter` unconditionally ignore ObjC methods, a user might 
> complain about it later if `-Wunused-parameter` didn't diagnose an unused 
> parameter when it should have.


Yeah, that seems fine to me, leave the method diagnostic off-by-default, since 
I doubt many would actually want it. Do you think its worth emitting 
-Wunused-parameter in pure-`@implementation` methods?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment.

In D61147#1479468 , @erik.pilkington 
wrote:

> > Yeah, I tend to think we should just suppress this unconditionally in 
> > Objective-C.
>
> IMO this warning still makes sense for normal functions, or methods that are 
> only declared in an @implementation. Adding a fix-it to cast to void in the 
> function/method body would probably go a long way too.


Should we use the new warning to warn just about unused ObjC method parameters 
and have `-Wunused-parameter` warn about everything else? If we make 
`-Wunused-parameter` unconditionally ignore ObjC methods, a user might complain 
about it later if `-Wunused-parameter` didn't diagnose an unused parameter when 
it should have.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D60279: [CUDA] Implemented _[bi]mma* builtins.

2019-04-25 Thread Artem Belevich via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359248: [CUDA] Implemented _[bi]mma* builtins. (authored by 
tra, committed by ).
Herald added a subscriber: kristina.
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D60279?vs=194226=196738#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60279

Files:
  include/clang/Basic/BuiltinsNVPTX.def
  lib/Basic/Targets/NVPTX.cpp
  lib/CodeGen/CGBuiltin.cpp
  lib/Driver/ToolChains/Cuda.cpp
  test/CodeGen/builtins-nvptx-mma.cu
  test/CodeGen/builtins-nvptx-mma.py

Index: include/clang/Basic/BuiltinsNVPTX.def
===
--- include/clang/Basic/BuiltinsNVPTX.def
+++ include/clang/Basic/BuiltinsNVPTX.def
@@ -18,13 +18,22 @@
 #endif
 
 #pragma push_macro("SM_70")
-#define SM_70 "sm_70|sm_71"
+#pragma push_macro("SM_72")
+#pragma push_macro("SM_75")
+#define SM_75 "sm_75"
+#define SM_72 "sm_72|" SM_75
+#define SM_70 "sm_70|" SM_72
+
 #pragma push_macro("SM_60")
 #define SM_60 "sm_60|sm_61|sm_62|" SM_70
 
-#pragma push_macro("PTX61")
-#define PTX61 "ptx61"
 #pragma push_macro("PTX60")
+#pragma push_macro("PTX61")
+#pragma push_macro("PTX63")
+#pragma push_macro("PTX64")
+#define PTX64 "ptx64"
+#define PTX63 "ptx63|" PTX64
+#define PTX61 "ptx61|" PTX63
 #define PTX60 "ptx60|" PTX61
 
 #pragma push_macro("AND")
@@ -666,10 +675,53 @@
 TARGET_BUILTIN(__hmma_m8n32k16_mma_f32f32, "vf*iC*iC*fC*IiIi", "", AND(SM_70,PTX61))
 TARGET_BUILTIN(__hmma_m8n32k16_mma_f16f32, "vi*iC*iC*fC*IiIi", "", AND(SM_70,PTX61))
 
+// Builtins to support integer and sub-integer WMMA instructions on sm_72/sm_75
+TARGET_BUILTIN(__bmma_m8n8k128_ld_a_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_ld_b_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_ld_c, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_mma_xor_popc_b1, "vi*iC*iC*iC*Ii", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_st_c_i32, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_a_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_b_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_b_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_c, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_mma_s8, "vi*iC*iC*iC*IiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_mma_u8, "vi*iC*iC*iC*IiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_st_c_i32, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_a_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_b_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_b_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_c, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_mma_s8, "vi*iC*iC*iC*IiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_mma_u8, "vi*iC*iC*iC*IiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_st_c_i32, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_a_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_b_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_b_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_c, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_mma_s8, "vi*iC*iC*iC*IiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_mma_u8, "vi*iC*iC*iC*IiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_st_c_i32, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_a_s4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_a_u4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_b_s4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_b_u4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_c, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_mma_s4, "vi*iC*iC*iC*IiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_mma_u4, "vi*iC*iC*iC*IiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_st_c_i32, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+
 #undef BUILTIN
 #undef TARGET_BUILTIN
 #pragma pop_macro("AND")
 #pragma pop_macro("SM_60")
 #pragma pop_macro("SM_70")
+#pragma pop_macro("SM_72")
+#pragma pop_macro("SM_75")
 #pragma pop_macro("PTX60")
 #pragma pop_macro("PTX61")
+#pragma pop_macro("PTX63")
+#pragma pop_macro("PTX64")
Index: test/CodeGen/builtins-nvptx-mma.cu

r359248 - [CUDA] Implemented _[bi]mma* builtins.

2019-04-25 Thread Artem Belevich via cfe-commits
Author: tra
Date: Thu Apr 25 15:28:09 2019
New Revision: 359248

URL: http://llvm.org/viewvc/llvm-project?rev=359248=rev
Log:
[CUDA] Implemented _[bi]mma* builtins.

These builtins provide access to the new integer and
sub-integer variants of MMA (matrix multiply-accumulate) instructions
provided by CUDA-10.x on sm_75 (AKA Turing) GPUs.

Also added a feature for PTX 6.4. While Clang/LLVM does not generate
any PTX instructions that need it, we still need to pass it through to
ptxas in order to be able to compile code that uses the new 'mma'
instruction as inline assembly (e.g used by NVIDIA's CUTLASS library
https://github.com/NVIDIA/cutlass/blob/master/cutlass/arch/mma.h#L101)

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

Added:
cfe/trunk/test/CodeGen/builtins-nvptx-mma.cu
cfe/trunk/test/CodeGen/builtins-nvptx-mma.py
Modified:
cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def
cfe/trunk/lib/Basic/Targets/NVPTX.cpp
cfe/trunk/lib/CodeGen/CGBuiltin.cpp
cfe/trunk/lib/Driver/ToolChains/Cuda.cpp

Modified: cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def?rev=359248=359247=359248=diff
==
--- cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def (original)
+++ cfe/trunk/include/clang/Basic/BuiltinsNVPTX.def Thu Apr 25 15:28:09 2019
@@ -18,13 +18,22 @@
 #endif
 
 #pragma push_macro("SM_70")
-#define SM_70 "sm_70|sm_71"
+#pragma push_macro("SM_72")
+#pragma push_macro("SM_75")
+#define SM_75 "sm_75"
+#define SM_72 "sm_72|" SM_75
+#define SM_70 "sm_70|" SM_72
+
 #pragma push_macro("SM_60")
 #define SM_60 "sm_60|sm_61|sm_62|" SM_70
 
-#pragma push_macro("PTX61")
-#define PTX61 "ptx61"
 #pragma push_macro("PTX60")
+#pragma push_macro("PTX61")
+#pragma push_macro("PTX63")
+#pragma push_macro("PTX64")
+#define PTX64 "ptx64"
+#define PTX63 "ptx63|" PTX64
+#define PTX61 "ptx61|" PTX63
 #define PTX60 "ptx60|" PTX61
 
 #pragma push_macro("AND")
@@ -666,10 +675,53 @@ TARGET_BUILTIN(__hmma_m8n32k16_mma_f32f1
 TARGET_BUILTIN(__hmma_m8n32k16_mma_f32f32, "vf*iC*iC*fC*IiIi", "", 
AND(SM_70,PTX61))
 TARGET_BUILTIN(__hmma_m8n32k16_mma_f16f32, "vi*iC*iC*fC*IiIi", "", 
AND(SM_70,PTX61))
 
+// Builtins to support integer and sub-integer WMMA instructions on sm_72/sm_75
+TARGET_BUILTIN(__bmma_m8n8k128_ld_a_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_ld_b_b1, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_ld_c, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_mma_xor_popc_b1, "vi*iC*iC*iC*Ii", "", 
AND(SM_75,PTX63))
+TARGET_BUILTIN(__bmma_m8n8k128_st_c_i32, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_a_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_b_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_b_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_ld_c, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_mma_s8, "vi*iC*iC*iC*IiIi", "", 
AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_mma_u8, "vi*iC*iC*iC*IiIi", "", 
AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m16n16k16_st_c_i32, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_a_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_b_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_b_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_ld_c, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_mma_s8, "vi*iC*iC*iC*IiIi", "", 
AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_mma_u8, "vi*iC*iC*iC*IiIi", "", 
AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m32n8k16_st_c_i32, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_a_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_a_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_b_s8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_b_u8, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_ld_c, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_mma_s8, "vi*iC*iC*iC*IiIi", "", 
AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_mma_u8, "vi*iC*iC*iC*IiIi", "", 
AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n32k16_st_c_i32, "vi*iC*UiIi", "", AND(SM_72,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_a_s4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_a_u4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_b_s4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_b_u4, "vi*iC*UiIi", "", AND(SM_75,PTX63))
+TARGET_BUILTIN(__imma_m8n8k32_ld_c, 

[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Tom Tan via Phabricator via cfe-commits
TomTan added a comment.

In D60349#140 , @TomTan wrote:

> In D60349#1477183 , @efriedma wrote:
>
> > > For NotPod, it is aggregate which is specific in the document
> >
> > Yes, it's an aggregate which is returned in registers... but it's returned 
> > in integer registers, unlike Pod which is returned in floating-point 
> > registers.
>
>
> `struct Pod` is HFA (Homogenous Floating-Point Aggregates) which is returned 
> in floating-point register, `struct NotPod` is not HFA so still returned in 
> integer registers. The ARM64 ABI document will be updated to reflect this, 
> thanks.


@efriedma Return info for `HFA` and `HVA` is updated in 
https://github.com/MicrosoftDocs/cpp-docs/pull/970.


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

https://reviews.llvm.org/D60349



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


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Sorry, yes, I meant in Objective-C methods, of course, not unconditionally even 
in C-like code whenever Objective-C is enabled.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D61051: [analyzer] Treat functions without runtime branches as "small".

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 196733.
NoQ marked 4 inline comments as done.
NoQ added a comment.

Fxd.


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

https://reviews.llvm.org/D61051

Files:
  clang/include/clang/Analysis/CFG.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/lib/Analysis/CFG.cpp
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/inline-if-constexpr.cpp
  clang/unittests/Analysis/CFGTest.cpp

Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -17,27 +17,41 @@
 namespace analysis {
 namespace {
 
-enum BuildResult {
-  ToolFailed,
-  ToolRan,
-  SawFunctionBody,
-  BuiltCFG,
+class BuildResult {
+public:
+  enum Status {
+ToolFailed,
+ToolRan,
+SawFunctionBody,
+BuiltCFG,
+  };
+
+  BuildResult(Status S, std::unique_ptr Cfg = nullptr)
+  : S(S), Cfg(std::move(Cfg)) {}
+
+  Status getStatus() const { return S; }
+  CFG *getCFG() const { return Cfg.get(); }
+
+private:
+  Status S;
+  std::unique_ptr Cfg;
 };
 
 class CFGCallback : public ast_matchers::MatchFinder::MatchCallback {
 public:
-  BuildResult TheBuildResult = ToolRan;
+  BuildResult TheBuildResult = BuildResult::ToolRan;
 
   void run(const ast_matchers::MatchFinder::MatchResult ) override {
 const auto *Func = Result.Nodes.getNodeAs("func");
 Stmt *Body = Func->getBody();
 if (!Body)
   return;
-TheBuildResult = SawFunctionBody;
+TheBuildResult = BuildResult::SawFunctionBody;
 CFG::BuildOptions Options;
 Options.AddImplicitDtors = true;
-if (CFG::buildCFG(nullptr, Body, Result.Context, Options))
-TheBuildResult = BuiltCFG;
+if (std::unique_ptr Cfg =
+CFG::buildCFG(nullptr, Body, Result.Context, Options))
+  TheBuildResult = {BuildResult::BuiltCFG, std::move(Cfg)};
   }
 };
 
@@ -50,8 +64,8 @@
   tooling::newFrontendActionFactory());
   std::vector Args = {"-std=c++11", "-fno-delayed-template-parsing"};
   if (!tooling::runToolOnCodeWithArgs(Factory->create(), Code, Args))
-return ToolFailed;
-  return Callback.TheBuildResult;
+return BuildResult::ToolFailed;
+  return std::move(Callback.TheBuildResult);
 }
 
 // Constructing a CFG for a range-based for over a dependent type fails (but
@@ -63,7 +77,7 @@
  "  for (const Foo *TheFoo : Range) {\n"
  "  }\n"
  "}\n";
-  EXPECT_EQ(SawFunctionBody, BuildCFG(Code));
+  EXPECT_EQ(BuildResult::SawFunctionBody, BuildCFG(Code).getStatus());
 }
 
 // Constructing a CFG containing a delete expression on a dependent type should
@@ -73,7 +87,7 @@
  "void f(T t) {\n"
  "  delete t;\n"
  "}\n";
-  EXPECT_EQ(BuiltCFG, BuildCFG(Code));
+  EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
 // Constructing a CFG on a function template with a variable of incomplete type
@@ -83,7 +97,24 @@
  "  class Undefined;\n"
  "  Undefined u;\n"
  "}\n";
-  EXPECT_EQ(BuiltCFG, BuildCFG(Code));
+  EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
+}
+
+TEST(CFG, IsLinear) {
+  auto expectLinear = [](bool IsLinear, const char *Code) {
+BuildResult B = BuildCFG(Code);
+EXPECT_EQ(BuildResult::BuiltCFG, B.getStatus());
+EXPECT_EQ(IsLinear, B.getCFG()->isLinear());
+  };
+
+  expectLinear(true,  "void foo() {}");
+  expectLinear(true,  "void foo() { if (true) return; }");
+  expectLinear(true,  "void foo() { if constexpr (false); }");
+  expectLinear(false, "void foo(bool coin) { if (coin) return; }");
+  expectLinear(false, "void foo() { for(;;); }");
+  expectLinear(false, "void foo() { do {} while (true); }");
+  expectLinear(true,  "void foo() { do {} while (false); }");
+  expectLinear(true,  "void foo() { foo(); }"); // Recursion is not our problem.
 }
 
 } // namespace
Index: clang/test/Analysis/inline-if-constexpr.cpp
===
--- /dev/null
+++ clang/test/Analysis/inline-if-constexpr.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection \
+// RUN:   -analyzer-inline-max-stack-depth=5 -w -std=c++17 -verify %s
+
+void clang_analyzer_eval(bool);
+
+namespace inline_large_functions_with_if_constexpr {
+bool f0() { if constexpr (true); return true; }
+bool f1() { if constexpr (true); return f0(); }
+bool f2() { if constexpr (true); return f1(); }
+bool f3() { if constexpr (true); return f2(); }
+bool f4() { if constexpr (true); return f3(); }
+bool f5() { if constexpr (true); return f4(); }
+bool f6() { if constexpr (true); return f5(); }
+bool f7() { if constexpr (true); return f6(); }
+void bar() {
+  clang_analyzer_eval(f7()); 

[PATCH] D61051: [analyzer] Treat functions without runtime branches as "small".

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: clang/lib/Analysis/CFG.cpp:4684
+  if (size() <= 3)
+return true;
+

Szelethus wrote:
> What are the cases for the size being 2 or 1? Empty function? Is a size of 1 
> even possible? Can we enforce something here with asserts?
An empty function, i.e. `void foo() {}`, has two CFG blocks. Every CFG has (by 
construction) at least an ENTRY and an EXIT, but i don't think this is the 
right place to introduce such sanity check.



Comment at: clang/test/Analysis/inline-if-constexpr.cpp:16
+void bar() {
+  clang_analyzer_eval(f7()); // expected-warning{{TRUE}}
+}

NoQ wrote:
> Szelethus wrote:
> > Aha, we wouldve seen UNKNOWN because the analyzer wouldn't inline these 
> > many functions, right? Neat!
> Aha, yup.
Hardcoded the max stack depth count to make this more apparent and prevent the 
test from not testing anything if it's bumped.


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

https://reviews.llvm.org/D61051



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


[PATCH] D57858: [analyzer] Add a new frontend flag to display all checker options

2019-04-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Ping^3


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

https://reviews.llvm.org/D57858



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


[PATCH] D57860: [analyzer] Validate checker option names and values

2019-04-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Ping^3


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

https://reviews.llvm.org/D57860



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


[PATCH] D59725: Additions to creduce script

2019-04-25 Thread Amy Huang via Phabricator via cfe-commits
akhuang marked an inline comment as done.
akhuang added inline comments.



Comment at: cfe/trunk/utils/creduce-clang-crash.py:185
+for msg in self.expected_output:
+  output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+

lebedev.ri wrote:
> akhuang wrote:
> > lebedev.ri wrote:
> > > >>! In D59725#1477362, @arichardson wrote:
> > > >>>! In D59725#1477042, @lebedev.ri wrote:
> > > >> I've stumbled into an issue with the script:
> > > >> It got a line: `clang: 
> > > >> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
> > > >> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
> > > >> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run 
> > > >> indvars!"' failed.`
> > > >> And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, 
> > > >> *LI) && "LCSSA required to run indvars!"' t.log || exit 1`
> > > >> But that doesn't work, escapes should be applied. it should be
> > > >> `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to 
> > > >> run indvars!\"' t.log || exit 1`
> > > > 
> > > > In my script I use `grep -F " + shlex.quote(self.args.crash_message)` 
> > > > which should escape both the message and ensure that the grep argument 
> > > > is not interpreted as a regex.
> > > 
> > > Great to hear!
> > > It appears that the script is currently more primitive than that 
> > > currently though.
> > Thanks; I added a `-F` flag (r359216), and it seems like that fixes the 
> > issue. 
> Thanks!
> What about `shlex`? How do you know bash won't expand that as regex already?
Sorry, not sure what you mean by bash expanding `shlex` as regex? It should be 
printing a quoted string to the bash script. 


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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


Re: r359067 - [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

2019-04-25 Thread Richard Smith via cfe-commits
On Wed, 24 Apr 2019 at 19:28, Eric Fiselier via cfe-commits
 wrote:
> Do I just edit the HTML file directly?
> Or is it generated by something?

Just edit the HTML file directly.

> On Wed, Apr 24, 2019 at 3:35 PM Richard Smith  wrote:
>>
>> Thanks! Can you update cxx_status.html to mark P0595R2 as done?
>>
>> On Tue, 23 Apr 2019 at 19:21, Eric Fiselier via cfe-commits
>>  wrote:
>> >
>> > Author: ericwf
>> > Date: Tue Apr 23 19:23:30 2019
>> > New Revision: 359067
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=359067=rev
>> > Log:
>> > [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a
>> >
>> > Summary:
>> > This patch implements `__builtin_is_constant_evaluated` as specifier by 
>> > [P0595R2](https://wg21.link/p0595r2). It is built on the back of Bill 
>> > Wendling's work for `__builtin_constant_p()`.
>> >
>> > More tests to come, but early feedback is appreciated.
>> >
>> > I plan to implement warnings for common mis-usages like those belowe in a 
>> > following patch:
>> > ```
>> > void foo(int x) {
>> >   if constexpr (std::is_constant_evaluated())) { // condition is always 
>> > `true`. Should use plain `if` instead.
>> >foo_constexpr(x);
>> >   } else {
>> > foo_runtime(x);
>> >   }
>> > }
>> > ```
>> >
>> >
>> >
>> > Reviewers: rsmith, MaskRay, bruno, void
>> >
>> > Reviewed By: rsmith
>> >
>> > Subscribers: dexonsmith, zoecarver, fdeazeve, kristina, cfe-commits
>> >
>> > Differential Revision: https://reviews.llvm.org/D55500
>> >
>> > Added:
>> > cfe/trunk/test/CodeGenCXX/builtin-is-constant-evaluated.cpp
>> > cfe/trunk/test/SemaCXX/builtin-is-constant-evaluated.cpp
>> > Modified:
>> > cfe/trunk/include/clang/Basic/Builtins.def
>> > cfe/trunk/lib/AST/ExprConstant.cpp
>> > cfe/trunk/lib/Basic/Builtins.cpp
>> > cfe/trunk/lib/CodeGen/CGDecl.cpp
>> > cfe/trunk/test/Sema/builtins.c
>> >
>> > Modified: cfe/trunk/include/clang/Basic/Builtins.def
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=359067=359066=359067=diff
>> > ==
>> > --- cfe/trunk/include/clang/Basic/Builtins.def (original)
>> > +++ cfe/trunk/include/clang/Basic/Builtins.def Tue Apr 23 19:23:30 2019
>> > @@ -500,6 +500,7 @@ BUILTIN(__builtin_vsprintf, "ic*cC*a", "
>> >  BUILTIN(__builtin_vsnprintf, "ic*zcC*a", "nFP:2:")
>> >  BUILTIN(__builtin_thread_pointer, "v*", "nc")
>> >  BUILTIN(__builtin_launder, "v*v*", "nt")
>> > +LANGBUILTIN(__builtin_is_constant_evaluated, "b", "n", CXX_LANG)
>> >
>> >  // GCC exception builtins
>> >  BUILTIN(__builtin_eh_return, "vzv*", "r") // FIXME: Takes intptr_t, not 
>> > size_t!
>> >
>> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=359067=359066=359067=diff
>> > ==
>> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
>> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Tue Apr 23 19:23:30 2019
>> > @@ -8279,6 +8279,9 @@ bool IntExprEvaluator::VisitBuiltinCallE
>> >  return Success(false, E);
>> >}
>> >
>> > +  case Builtin::BI__builtin_is_constant_evaluated:
>> > +return Success(Info.InConstantContext, E);
>> > +
>> >case Builtin::BI__builtin_ctz:
>> >case Builtin::BI__builtin_ctzl:
>> >case Builtin::BI__builtin_ctzll:
>> > @@ -11139,6 +11142,7 @@ bool Expr::EvaluateAsConstantExpr(EvalRe
>> >EvalInfo::EvaluationMode EM = EvalInfo::EM_ConstantExpression;
>> >EvalInfo Info(Ctx, Result, EM);
>> >Info.InConstantContext = true;
>> > +
>> >if (!::Evaluate(Result.Val, Info, this))
>> >  return false;
>> >
>> >
>> > Modified: cfe/trunk/lib/Basic/Builtins.cpp
>> > URL: 
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Builtins.cpp?rev=359067=359066=359067=diff
>> > ==
>> > --- cfe/trunk/lib/Basic/Builtins.cpp (original)
>> > +++ cfe/trunk/lib/Basic/Builtins.cpp Tue Apr 23 19:23:30 2019
>> > @@ -75,9 +75,12 @@ bool Builtin::Context::builtinIsSupporte
>> >bool OclCUnsupported = !LangOpts.OpenCL &&
>> >   (BuiltinInfo.Langs & ALL_OCLC_LANGUAGES);
>> >bool OpenMPUnsupported = !LangOpts.OpenMP && BuiltinInfo.Langs == 
>> > OMP_LANG;
>> > +  bool CPlusPlusUnsupported =
>> > +  !LangOpts.CPlusPlus && BuiltinInfo.Langs == CXX_LANG;
>> >return !BuiltinsUnsupported && !MathBuiltinsUnsupported && 
>> > !OclCUnsupported &&
>> >   !OclC1Unsupported && !OclC2Unsupported && !OpenMPUnsupported &&
>> > - !GnuModeUnsupported && !MSModeUnsupported && !ObjCUnsupported;
>> > + !GnuModeUnsupported && !MSModeUnsupported && !ObjCUnsupported &&
>> > + !CPlusPlusUnsupported;
>> >  }
>> >
>> >  /// initializeBuiltins - Mark the identifiers for all the builtins with 
>> > their
>> >
>> > 

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

> Yeah, I tend to think we should just suppress this unconditionally in 
> Objective-C.

IMO this warning still makes sense for normal functions, or methods that are 
only declared in an @implementation. Adding a fix-it to cast to void in the 
function/method body would probably go a long way too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-25 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment.

As an example, .tbe and .tbd files use YAML but in a very specific well defined 
structure. yaml2obj also uses a specific format which is actually well defined, 
it just doesn't map on very well here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D60974: Clang IFSO driver action.

2019-04-25 Thread Jake Ehrlich via Phabricator via cfe-commits
jakehehrlich added a comment.

There seems to be some confusion about the term "format" here. There's the YAML 
format but within that there are specific YAML formats. My proposal will be to 
have a specific YAML format that just isn't the one used by yaml2obj. YAML 
isn't the issue, it's the structure that that tool specifically uses.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60974



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


[PATCH] D58033: Add option for emitting dbg info for call site parameters

2019-04-25 Thread Vedant Kumar via Phabricator via cfe-commits
vsk added a comment.

I think a small extension to `test/CodeGenCXX/dbg-info-all-calls-described.cpp` 
for the 'Supported: DWARF4 + -femit_param_entry_values' case would be 
appropriate here.


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

https://reviews.llvm.org/D58033



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


[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

2019-04-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

Upon further investigation, this still seems to be okay. This is not a 
bug-finding checker, but rather a tool to enforce the idiom of every 
non-trivial object should be fully initialized after construction. From what 
I've seen, `ATOMIC_VAR_INIT` should be used here, but it isn't.

Do you think that I still should be more forgiving with atomics?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61106



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


[PATCH] D61136: [Analyzer] IteratorChecker - Ensure end()>=begin() and refactor begin and end symbol creation

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

Aha, yup, thx!

Do you plan to centralize other code paths on which you conjure symbols?

'Cause i still believe that given that we now support C++ (our prvalues are now 
properly materialized) (most of the time), you should be able to remove support 
for symbol-like iterators, and then replace your position symbols with 
`SymbolMetadata`.




Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:1929-1930
+
+  auto  = State->getSymbolManager();
+  auto Sym = SymMgr.conjureSymbol(E, LCtx, T, BlockCount, "end");
+  State = assumeNoOverflow(State, Sym, 4);

This is a bit more `auto` than allowed by [[ 
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
 | our coding standards ]] (it pretty much disallows `auto` unless it's some 
sort of `dyn_cast` (`getAs`) or an iterator.



Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:2247
 
+ProgramStateRef ensureNonNegativeDiff(ProgramStateRef State, SymbolRef Sym1,
+  SymbolRef Sym2) {

This looks like a new feature. Is it testable?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61136



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


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

Yeah, I tend to think we should just suppress this unconditionally in 
Objective-C.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


r359241 - [PGO] Fix buildbot failure in 359215

2019-04-25 Thread Rong Xu via cfe-commits
Author: xur
Date: Thu Apr 25 14:16:41 2019
New Revision: 359241

URL: http://llvm.org/viewvc/llvm-project?rev=359241=rev
Log:
[PGO] Fix buildbot failure in 359215

Revert the part of changes in r359215 that failing in some platforms.
I will re-enable them later.

Modified:
cfe/trunk/test/Profile/c-general.c

Modified: cfe/trunk/test/Profile/c-general.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Profile/c-general.c?rev=359241=359240=359241=diff
==
--- cfe/trunk/test/Profile/c-general.c (original)
+++ cfe/trunk/test/Profile/c-general.c Thu Apr 25 14:16:41 2019
@@ -1,12 +1,10 @@
 // Test instrumentation of general constructs in C.
 
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c 
%s -o - -emit-llvm -fprofile-instrument=clang | FileCheck 
-allow-deprecated-dag-overlap  -check-prefix=PGOGEN %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c 
%s -o - -emit-llvm -fprofile-instrument=clang -fexperimental-new-pass-manager | 
FileCheck -allow-deprecated-dag-overlap  -check-prefix=PGOGEN %s
 
 // RUN: llvm-profdata merge %S/Inputs/c-general.proftext -o %t.profdata
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c 
%s -o - -emit-llvm -fprofile-instrument-use-path=%t.profdata | FileCheck 
-allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c 
%s -o - -emit-llvm 
-fprofile-instrument-use-path=%S/Inputs/c-general.profdata.v3 | FileCheck 
-allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
-// RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c 
%s -o - -emit-llvm 
-fprofile-instrument-use-path=%S/Inputs/c-general.profdata.v3 
-fexperimental-new-pass-manager | FileCheck -allow-deprecated-dag-overlap  
-check-prefix=PGOUSE %s
 // Also check compatibility with older profiles.
 // RUN: %clang_cc1 -triple x86_64-apple-macosx10.9 -main-file-name c-general.c 
%s -o - -emit-llvm 
-fprofile-instrument-use-path=%S/Inputs/c-general.profdata.v1 | FileCheck 
-allow-deprecated-dag-overlap  -check-prefix=PGOUSE %s
 


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


[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

2019-04-25 Thread Kristóf Umann via Phabricator via cfe-commits
Szelethus added a comment.

In D61106#1479336 , @jfb wrote:

> Have you checked what the C and C++ standards say about `_Atomic` / 
> `std::atomic` initialization, and how they should actually be initialized by 
> developers?


No, I have not, I'll admit. Did I make a mistake here? Can you tell me where I 
can read up on it maybe?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61106



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


[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

2019-04-25 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea updated this revision to Diff 196725.
gtbercea added a comment.

- Update patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907

Files:
  include/clang/Driver/ToolChain.h
  lib/Driver/ToolChains/Clang.cpp
  lib/Driver/ToolChains/Cuda.cpp
  lib/Driver/ToolChains/Cuda.h
  lib/Headers/CMakeLists.txt
  lib/Headers/__clang_openmp_math.h

Index: lib/Headers/__clang_openmp_math.h
===
--- /dev/null
+++ lib/Headers/__clang_openmp_math.h
@@ -0,0 +1,96 @@
+/*=== __clang_openmp_math.h - Target OpenMP math support ---===
+ *
+ * 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 __CLANG_OPENMP_MATH_H__
+#define __CLANG_OPENMP_MATH_H__
+
+#ifdef __NVPTX__
+#pragma omp declare target
+
+// Declarations of function in libomptarget
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+// POW
+float __kmpc_powf(float, float);
+double __kmpc_pow(double, double);
+long double __kmpc_powl(long double, long double);
+
+// LOG
+double __kmpc_log(double);
+float __kmpc_logf(float);
+double __kmpc_log10(double);
+float __kmpc_log10f(float);
+double __kmpc_log1p(double);
+float __kmpc_log1pf(float);
+double __kmpc_log2(double);
+float __kmpc_log2f(float);
+double __kmpc_logb(double);
+float __kmpc_logbf(float);
+
+// SIN
+float __kmpc_sinf(float);
+double __kmpc_sin(double);
+long double __kmpc_sinl(long double);
+
+// COS
+float __kmpc_cosf(float);
+double __kmpc_cos(double);
+long double __kmpc_cosl(long double);
+
+#if defined(__cplusplus)
+}
+#endif
+
+// Single argument functions
+#define __OPENMP_MATH_FUNC_1(__ty, __fn, __kmpc_fn)\
+  __attribute__((always_inline, used)) static __ty \
+  __fn(__ty __x) { \
+return __kmpc_fn(__x); \
+  }
+
+// Double argument functions
+#define __OPENMP_MATH_FUNC_2(__ty, __fn, __kmpc_fn)\
+  __attribute__((always_inline, used)) static __ty \
+  __fn(__ty __x, __ty __y) {   \
+return __kmpc_fn(__x, __y);\
+  }
+
+// POW
+__OPENMP_MATH_FUNC_2(float, powf, __kmpc_powf);
+__OPENMP_MATH_FUNC_2(double, pow, __kmpc_pow);
+__OPENMP_MATH_FUNC_2(long double, powl, __kmpc_powl);
+
+// LOG
+__OPENMP_MATH_FUNC_1(double, log, __kmpc_log);
+__OPENMP_MATH_FUNC_1(float, logf, __kmpc_logf);
+__OPENMP_MATH_FUNC_1(double, log10, __kmpc_log10);
+__OPENMP_MATH_FUNC_1(float, log10f, __kmpc_log10f);
+__OPENMP_MATH_FUNC_1(double, log1p, __kmpc_log1p);
+__OPENMP_MATH_FUNC_1(float, log1pf, __kmpc_log1pf);
+__OPENMP_MATH_FUNC_1(double, log2, __kmpc_log2);
+__OPENMP_MATH_FUNC_1(float, log2f, __kmpc_log2f);
+__OPENMP_MATH_FUNC_1(double, logb, __kmpc_logb);
+__OPENMP_MATH_FUNC_1(float, logbf, __kmpc_logbf);
+
+// SIN
+__OPENMP_MATH_FUNC_1(float, sinf, __kmpc_sinf);
+__OPENMP_MATH_FUNC_1(double, sin, __kmpc_sin);
+__OPENMP_MATH_FUNC_1(long double, sinl, __kmpc_sinl);
+
+// COS
+__OPENMP_MATH_FUNC_1(float, cosf, __kmpc_cosf);
+__OPENMP_MATH_FUNC_1(double, cos, __kmpc_cos);
+__OPENMP_MATH_FUNC_1(long double, cosl, __kmpc_cosl);
+
+#pragma omp end declare target
+#endif
+#endif
+
Index: lib/Headers/CMakeLists.txt
===
--- lib/Headers/CMakeLists.txt
+++ lib/Headers/CMakeLists.txt
@@ -31,6 +31,7 @@
   avxintrin.h
   bmi2intrin.h
   bmiintrin.h
+  __clang_openmp_math.h
   __clang_cuda_builtin_vars.h
   __clang_cuda_cmath.h
   __clang_cuda_complex_builtins.h
Index: lib/Driver/ToolChains/Cuda.h
===
--- lib/Driver/ToolChains/Cuda.h
+++ lib/Driver/ToolChains/Cuda.h
@@ -48,6 +48,9 @@
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const;
 
+  void AddMathDeviceFunctions(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const;
+
   /// Emit an error if Version does not support the given Arch.
   ///
   /// If either Version or Arch is unknown, does not emit an error.  Emits at
@@ -165,6 +168,9 @@
   void AddCudaIncludeArgs(const llvm::opt::ArgList ,
   llvm::opt::ArgStringList ) const override;
 
+  void AddMathDeviceFunctions(const llvm::opt::ArgList ,
+  llvm::opt::ArgStringList ) const override;
+
   void addClangWarningOptions(llvm::opt::ArgStringList ) const override;
   CXXStdlibType GetCXXStdlibType(const llvm::opt::ArgList ) const 

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment.

> but don't want to annotate each unused ObjC method parameters with 
> `__attribute__((unused))` or insert `(void)unused_param` into the body of the 
> method to silence the warning since, unlike C/C++ functions, it's not 
> possible to comment out the unused parameters of ObjC methods.

Thats also true of function parameters in C (and Objective-C), so I don't think 
that argument is very convincing. I think it is true that this is a much less 
useful diagnostic for methods though, because they're more likely to be 
overriding something that requires a certain selector. Maybe we should just 
suppress this warning in general for methods (and even virtual functions in 
C++)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61147



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


[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

2019-04-25 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

In D60907#1479142 , @hfinkel wrote:

> In D60907#1479118 , @gtbercea wrote:
>
> > Ping @hfinkel @tra
>
>
> The last two comments in D47849  indicated 
> exploration of a different approach, and one which still seems superior to 
> this one. Can you please comment on why you're now pursuing this approach 
> instead?


Hal, as far as I can tell, this solution is similar to yours but with a 
slightly different implementation. If there are particular aspects about this 
patch you would like to discuss/give feedback on please let me know.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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


[PATCH] D61134: [Analyzer] Iterator Checkers - Do an early return after handling calls

2019-04-25 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.
This revision is now accepted and ready to land.

Thx, this is safer!




Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:566
   const auto *InstCall = dyn_cast();
   if (Func->getParamDecl(0)->getType()->isRValueReferenceType()) {
 handleAssign(C, InstCall->getCXXThisVal(), Call.getOriginExpr(),

`CXXMethodDecl` has a lot of fancy getters on it, such as 
`isMoveAssignmentOperator()`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61134



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


[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision.
ahatanak added reviewers: erik.pilkington, rjmccall.
ahatanak added a project: clang.
Herald added subscribers: dexonsmith, jkorous.

The new warning `-Wunused-parameter-non-objc-method` works exactly the same as 
`-Wunused-parameter ` except for unused parameters of ObjC methods. This can be 
handy when users want to use `-Wunused-parameter ` but don't want to annotate 
each unused ObjC method parameters with `__attribute__((unused))` or insert 
`(void)unused_param` into the body of the method to silence the warning since, 
unlike C/C++ functions, it's not possible to comment out the unused parameters 
of ObjC methods.

  // Can't do this for ObjC methods.
  int foo(int /*param*) {}

rdar://problem/41561853


Repository:
  rC Clang

https://reviews.llvm.org/D61147

Files:
  include/clang/Basic/DiagnosticGroups.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Sema/Sema.h
  lib/Sema/SemaDecl.cpp
  test/Sema/warn-unused-parameters.c
  test/SemaObjC/method-unused-attribute.m

Index: test/SemaObjC/method-unused-attribute.m
===
--- test/SemaObjC/method-unused-attribute.m
+++ test/SemaObjC/method-unused-attribute.m
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1  -fsyntax-only -Wunused-parameter -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1  -fsyntax-only -Wunused-parameter-non-objc-method -verify -Wno-objc-root-class -DOBJMETHOD %s
 
 @interface INTF
 - (void) correct_use_of_unused: (void *) notice : (id)another_arg;
@@ -9,7 +10,13 @@
 @implementation INTF
 - (void) correct_use_of_unused: (void *)  __attribute__((unused)) notice : (id) __attribute__((unused)) newarg{
 }
-- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : (id)warn_unused {}  // expected-warning {{unused parameter 'warn_unused'}}
-- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}  // expected-warning {{unused parameter 'will_warn'}}
+- (void) will_warn_unused_arg: (void *) __attribute__((unused))  notice : (id)warn_unused {}
+- (void) unused_attr_on_decl_ignored: (void *)  will_warn{}
 @end
 
+void foo(int unused1) {} // expected-warning {{unused parameter 'unused1'}}
+
+#ifndef OBJMETHOD
+// expected-warning@-7 {{unused parameter 'warn_unused'}}
+// expected-warning@-7 {{unused parameter 'will_warn'}}
+#endif
Index: test/Sema/warn-unused-parameters.c
===
--- test/Sema/warn-unused-parameters.c
+++ test/Sema/warn-unused-parameters.c
@@ -24,7 +24,7 @@
 // RUN: %clang_cc1 -fblocks -fsyntax-only -Weverything %s 2>&1 | FileCheck -check-prefix=CHECK-everything %s
 // RUN: not %clang_cc1 -fblocks -fsyntax-only -Weverything -Werror %s 2>&1 | FileCheck -check-prefix=CHECK-everything-error %s
 // RUN: %clang_cc1 -fblocks -fsyntax-only -Weverything -Wno-unused %s 2>&1 | FileCheck -check-prefix=CHECK-everything-no-unused %s
-// CHECK-everything: 6 warnings generated
-// CHECK-everything-error: 5 errors generated
-// CHECK-everything-no-unused: 5 warnings generated
+// CHECK-everything: 8 warnings generated
+// CHECK-everything-error: 7 errors generated
+// CHECK-everything-no-unused: 7 warnings generated
 
Index: lib/Sema/SemaDecl.cpp
===
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -12545,7 +12545,8 @@
   return Param;
 }
 
-void Sema::DiagnoseUnusedParameters(ArrayRef Parameters) {
+void Sema::DiagnoseUnusedParameters(ArrayRef Parameters,
+bool IsObjCMethodParam) {
   // Don't diagnose unused-parameter errors in template instantiations; we
   // will already have done so in the template itself.
   if (inTemplateInstantiation())
@@ -12556,6 +12557,10 @@
 !Parameter->hasAttr()) {
   Diag(Parameter->getLocation(), diag::warn_unused_parameter)
 << Parameter->getDeclName();
+  if (!IsObjCMethodParam)
+Diag(Parameter->getLocation(),
+ diag::warn_unused_parameter_non_objc_method)
+<< Parameter->getDeclName();
 }
   }
 }
@@ -13355,7 +13360,7 @@
 MD->setBody(Body);
 if (!MD->isInvalidDecl()) {
   if (!MD->hasSkippedBody())
-DiagnoseUnusedParameters(MD->parameters());
+DiagnoseUnusedParameters(MD->parameters(), true);
   DiagnoseSizeOfParametersAndReturnValue(MD->parameters(),
  MD->getReturnType(), MD);
 
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -2157,7 +2157,8 @@
 
   /// Diagnose any unused parameters in the given sequence of
   /// ParmVarDecl pointers.
-  void DiagnoseUnusedParameters(ArrayRef Parameters);
+  void DiagnoseUnusedParameters(ArrayRef Parameters,
+bool IsObjCMethodParam = false);
 
   /// Diagnose whether the size of parameters or return value of a
   /// 

[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

2019-04-25 Thread JF Bastien via Phabricator via cfe-commits
jfb added a comment.

Have you checked what the C and C++ standards say about `_Atomic` / 
`std::atomic` initialization, and how they should actually be initialized by 
developers?


Repository:
  rC Clang

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

https://reviews.llvm.org/D61106



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


r359237 - [analyzer] Add FIXMEs for alpha.unix.cstring.OutOfBounds false positives.

2019-04-25 Thread Artem Dergachev via cfe-commits
Author: dergachev
Date: Thu Apr 25 13:30:14 2019
New Revision: 359237

URL: http://llvm.org/viewvc/llvm-project?rev=359237=rev
Log:
[analyzer] Add FIXMEs for alpha.unix.cstring.OutOfBounds false positives.

Caused by incorrect strlcat() modeling in r332303,
cf. https://bugs.llvm.org/show_bug.cgi?id=37687#c8

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
cfe/trunk/test/Analysis/bsd-string.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp?rev=359237=359236=359237=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp Thu Apr 25 
13:30:14 2019
@@ -1528,6 +1528,10 @@ void CStringChecker::evalStrlcat(Checker
   if (CE->getNumArgs() < 3)
 return;
 
+  // FIXME: strlcat() uses a different rule for bound checking, i.e. 'n' means
+  // a different thing as compared to strncat(). This currently causes
+  // false positives in the alpha string bound checker.
+
   //char *strlcat(char *s1, const char *s2, size_t n);
   evalStrcpyCommon(C, CE,
/* returnEnd = */ false,

Modified: cfe/trunk/test/Analysis/bsd-string.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/bsd-string.c?rev=359237=359236=359237=diff
==
--- cfe/trunk/test/Analysis/bsd-string.c (original)
+++ cfe/trunk/test/Analysis/bsd-string.c Thu Apr 25 13:30:14 2019
@@ -15,6 +15,7 @@ void f1() {
 void f2() {
   char buf[5];
   strlcpy(buf, "abcd", sizeof(buf)); // expected-no-warning
+  // FIXME: This should not warn. The string is safely truncated.
   strlcat(buf, "efgh", sizeof(buf)); // expected-warning{{Size argument is 
greater than the free space in the destination buffer}}
 }
 


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


[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

2019-04-25 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

In D60907#1479298 , @gtbercea wrote:

> In D60907#1479142 , @hfinkel wrote:
>
> > In D60907#1479118 , @gtbercea 
> > wrote:
> >
> > > Ping @hfinkel @tra
> >
> >
> > The last two comments in D47849  indicated 
> > exploration of a different approach, and one which still seems superior to 
> > this one. Can you please comment on why you're now pursuing this approach 
> > instead?
>
>
> This solution is following Alexey's suggestions. This solution allows the 
> optimization of math calls if they apply (example: pow(x,2) => x*x ) which 
> was one of the issues in the previous solution I implemented.





Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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


[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

2019-04-25 Thread Gheorghe-Teodor Bercea via Phabricator via cfe-commits
gtbercea added a comment.

In D60907#1479142 , @hfinkel wrote:

> In D60907#1479118 , @gtbercea wrote:
>
> > Ping @hfinkel @tra
>
>
> The last two comments in D47849  indicated 
> exploration of a different approach, and one which still seems superior to 
> this one. Can you please comment on why you're now pursuing this approach 
> instead?


This solution is following Alexey's suggestions. This solution allows the 
optimization of math calls if they apply (example: pow(x,2) => x*x ).


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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


r359235 - Skip type units/type uniquing when we know we're only emitting the type once (vtable-based emission when triggered by a strong vtable, with -fno-standalone-debug)

2019-04-25 Thread David Blaikie via cfe-commits
Author: dblaikie
Date: Thu Apr 25 13:05:47 2019
New Revision: 359235

URL: http://llvm.org/viewvc/llvm-project?rev=359235=rev
Log:
Skip type units/type uniquing when we know we're only emitting the type once 
(vtable-based emission when triggered by a strong vtable, with 
-fno-standalone-debug)

(this would regress size without a corresponding LLVM change that avoids
putting other user defined types inside type units when they aren't in
their own type units - instead emitting declarations inside the type
unit and a definition in the primary CU)

Reviewers: aprantl

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

Modified:
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/test/CodeGenCXX/debug-info-class.cpp
cfe/trunk/test/Modules/ExtDebugInfo.cpp
cfe/trunk/test/Modules/ModuleDebugInfo.cpp

Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=359235=359234=359235=diff
==
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Apr 25 13:05:47 2019
@@ -915,6 +915,11 @@ static SmallString<256> getTypeIdentifie
 
   if (!needsTypeIdentifier(TD, CGM, TheCU))
 return Identifier;
+  if (const auto *RD = dyn_cast(TD))
+if (RD->getDefinition())
+  if (RD->isDynamicClass() &&
+  CGM.getVTableLinkage(RD) == llvm::GlobalValue::ExternalLinkage)
+return Identifier;
 
   // TODO: This is using the RTTI name. Is there a better way to get
   // a unique string for a type?

Modified: cfe/trunk/test/CodeGenCXX/debug-info-class.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-class.cpp?rev=359235=359234=359235=diff
==
--- cfe/trunk/test/CodeGenCXX/debug-info-class.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/debug-info-class.cpp Thu Apr 25 13:05:47 2019
@@ -57,6 +57,11 @@ struct I : virtual H {};
 struct J : I {};
 J j;
 
+struct K {
+  virtual void func() {
+  }
+};
+
 struct A {
   int one;
   static const int HdrSize = 52;
@@ -72,6 +77,8 @@ void f1() {
   E y;
   int i = F::i;
   F::inner z;
+  K k;
+  k.func();
 }
 
 int main(int argc, char **argv) {
@@ -98,7 +105,8 @@ int main(int argc, char **argv) {
 
 // CHECK: [[F:![0-9]*]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"F"
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1F"
+// CHECK-NOT: identifier:
+// CHECK-SAME:){{$}}
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "I"
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: ){{$}}
@@ -117,7 +125,8 @@ int main(int argc, char **argv) {
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: elements: [[C_MEM:![0-9]*]]
 // CHECK-SAME: vtableHolder: [[C]]
-// CHECK-SAME: identifier: "_ZTS1C"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
 // CHECK: [[C_MEM]] = !{[[C_VPTR:![0-9]*]], [[C_S:![0-9]*]], 
[[C_DTOR:![0-9]*]]}
 // CHECK: [[C_VPTR]] = !DIDerivedType(tag: DW_TAG_member, name: "_vptr$C"
 // CHECK-SAME:DIFlagArtificial
@@ -129,10 +138,16 @@ int main(int argc, char **argv) {
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: 
"D"
 // CHECK-NOT:  size:
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1D"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "E"
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1E"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "K"
+// CHECK-SAME: identifier: "_ZTS1K"
+// CHECK-SAME: ){{$}}
 
 // CHECK: !DISubprogram(name: "func",{{.*}} scope: [[D]]
 // CHECK-SAME:  DISPFlagDefinition
@@ -146,7 +161,8 @@ int main(int argc, char **argv) {
 
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "G"
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1G"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
 // CHECK: [[G_INNER_MEM]] = !{[[G_INNER_I:![0-9]*]]}
 // CHECK: [[G_INNER_I]] = !DIDerivedType(tag: DW_TAG_member, name: "j"
 // CHECK-SAME:   baseType: ![[INT]]
@@ -154,5 +170,5 @@ int main(int argc, char **argv) {
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "HdrSize"
 //
-// CHECK: ![[EXCEPTLOC]] = 

[PATCH] D61079: Skip type units/type uniquing when we know we're only emitting the type once (vtable-based emission when triggered by a strong vtable, with -fno-standalone-debug)

2019-04-25 Thread David Blaikie via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359235: Skip type units/type uniquing when we know 
were only emitting the type once… (authored by dblaikie, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D61079?vs=196484=196709#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D61079

Files:
  lib/CodeGen/CGDebugInfo.cpp
  test/CodeGenCXX/debug-info-class.cpp
  test/Modules/ExtDebugInfo.cpp
  test/Modules/ModuleDebugInfo.cpp

Index: test/Modules/ExtDebugInfo.cpp
===
--- test/Modules/ExtDebugInfo.cpp
+++ test/Modules/ExtDebugInfo.cpp
@@ -214,7 +214,7 @@
 // CHECK-PCH:dwoId: 18446744073709551614
 
 // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A",
-// CHECK-SAME: DIFlagFwdDecl, identifier: "_ZTS1A")
+// CHECK-SAME: DIFlagFwdDecl)
 
 // There is a full definition of the type available in the module.
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Virtual",
Index: test/Modules/ModuleDebugInfo.cpp
===
--- test/Modules/ModuleDebugInfo.cpp
+++ test/Modules/ModuleDebugInfo.cpp
@@ -119,8 +119,7 @@
 
 // CHECK: ![[A:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type, name: "A",
 // CHECK-SAME:   elements:
-// CHECK-SAME:   vtableHolder: ![[A]],
-// CHECK-SAME:   identifier: "_ZTS1A")
+// CHECK-SAME:   vtableHolder: ![[A]])
 
 // CHECK: ![[DERIVED:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type, name: "Derived",
 // CHECK-SAME: identifier: "_ZTS7Derived")
Index: test/CodeGenCXX/debug-info-class.cpp
===
--- test/CodeGenCXX/debug-info-class.cpp
+++ test/CodeGenCXX/debug-info-class.cpp
@@ -57,6 +57,11 @@
 struct J : I {};
 J j;
 
+struct K {
+  virtual void func() {
+  }
+};
+
 struct A {
   int one;
   static const int HdrSize = 52;
@@ -72,6 +77,8 @@
   E y;
   int i = F::i;
   F::inner z;
+  K k;
+  k.func();
 }
 
 int main(int argc, char **argv) {
@@ -98,7 +105,8 @@
 
 // CHECK: [[F:![0-9]*]] = !DICompositeType(tag: DW_TAG_structure_type, name: "F"
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1F"
+// CHECK-NOT: identifier:
+// CHECK-SAME:){{$}}
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "I"
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: ){{$}}
@@ -117,7 +125,8 @@
 // CHECK-NOT:  DIFlagFwdDecl
 // CHECK-SAME: elements: [[C_MEM:![0-9]*]]
 // CHECK-SAME: vtableHolder: [[C]]
-// CHECK-SAME: identifier: "_ZTS1C"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
 // CHECK: [[C_MEM]] = !{[[C_VPTR:![0-9]*]], [[C_S:![0-9]*]], [[C_DTOR:![0-9]*]]}
 // CHECK: [[C_VPTR]] = !DIDerivedType(tag: DW_TAG_member, name: "_vptr$C"
 // CHECK-SAME:DIFlagArtificial
@@ -129,10 +138,16 @@
 // CHECK: [[D:![0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type, name: "D"
 // CHECK-NOT:  size:
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1D"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "E"
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1E"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
+
+// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "K"
+// CHECK-SAME: identifier: "_ZTS1K"
+// CHECK-SAME: ){{$}}
 
 // CHECK: !DISubprogram(name: "func",{{.*}} scope: [[D]]
 // CHECK-SAME:  DISPFlagDefinition
@@ -146,7 +161,8 @@
 
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "G"
 // CHECK-SAME: DIFlagFwdDecl
-// CHECK-SAME: identifier: "_ZTS1G"
+// CHECK-NOT:  identifier:
+// CHECK-SAME: ){{$}}
 // CHECK: [[G_INNER_MEM]] = !{[[G_INNER_I:![0-9]*]]}
 // CHECK: [[G_INNER_I]] = !DIDerivedType(tag: DW_TAG_member, name: "j"
 // CHECK-SAME:   baseType: ![[INT]]
@@ -154,5 +170,5 @@
 // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "A"
 // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "HdrSize"
 //
-// CHECK: ![[EXCEPTLOC]] = !DILocation(line: 84,
-// CHECK: ![[RETLOC]] = !DILocation(line: 83,
+// CHECK: ![[EXCEPTLOC]] = 

[PATCH] D61121: [Windows] Separate elements in -print-search-dirs with semicolons

2019-04-25 Thread Martin Storsjö via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL359233: [Windows] Separate elements in -print-search-dirs 
with semicolons (authored by mstorsjo, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D61121?vs=196593=196708#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D61121

Files:
  cfe/trunk/lib/Driver/Driver.cpp


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1695,7 +1695,7 @@
 bool separator = false;
 for (const std::string  : TC.getProgramPaths()) {
   if (separator)
-llvm::outs() << ':';
+llvm::outs() << llvm::sys::EnvPathSeparator;
   llvm::outs() << Path;
   separator = true;
 }
@@ -1706,7 +1706,7 @@
 
 for (const std::string  : TC.getFilePaths()) {
   // Always print a separator. ResourceDir was the first item shown.
-  llvm::outs() << ':';
+  llvm::outs() << llvm::sys::EnvPathSeparator;
   // Interpretation of leading '=' is needed only for NetBSD.
   if (Path[0] == '=')
 llvm::outs() << sysroot << Path.substr(1);


Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -1695,7 +1695,7 @@
 bool separator = false;
 for (const std::string  : TC.getProgramPaths()) {
   if (separator)
-llvm::outs() << ':';
+llvm::outs() << llvm::sys::EnvPathSeparator;
   llvm::outs() << Path;
   separator = true;
 }
@@ -1706,7 +1706,7 @@
 
 for (const std::string  : TC.getFilePaths()) {
   // Always print a separator. ResourceDir was the first item shown.
-  llvm::outs() << ':';
+  llvm::outs() << llvm::sys::EnvPathSeparator;
   // Interpretation of leading '=' is needed only for NetBSD.
   if (Path[0] == '=')
 llvm::outs() << sysroot << Path.substr(1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359233 - [Windows] Separate elements in -print-search-dirs with semicolons

2019-04-25 Thread Martin Storsjo via cfe-commits
Author: mstorsjo
Date: Thu Apr 25 13:03:20 2019
New Revision: 359233

URL: http://llvm.org/viewvc/llvm-project?rev=359233=rev
Log:
[Windows] Separate elements in -print-search-dirs with semicolons

Path lists on windows should always be separated by semicolons, not
colons. Reuse llvm::sys::EnvPathSeparator for this purpose (as that's
also a path list that is separated in the same way).

Alternatively, this could just be a local ifdef _WIN32 in this function,
or generalizing the existing EnvPathSeparator to e.g. a
llvm::sys::path::PathListSeparator?

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=359233=359232=359233=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu Apr 25 13:03:20 2019
@@ -1695,7 +1695,7 @@ bool Driver::HandleImmediateArgs(const C
 bool separator = false;
 for (const std::string  : TC.getProgramPaths()) {
   if (separator)
-llvm::outs() << ':';
+llvm::outs() << llvm::sys::EnvPathSeparator;
   llvm::outs() << Path;
   separator = true;
 }
@@ -1706,7 +1706,7 @@ bool Driver::HandleImmediateArgs(const C
 
 for (const std::string  : TC.getFilePaths()) {
   // Always print a separator. ResourceDir was the first item shown.
-  llvm::outs() << ':';
+  llvm::outs() << llvm::sys::EnvPathSeparator;
   // Interpretation of leading '=' is needed only for NetBSD.
   if (Path[0] == '=')
 llvm::outs() << sysroot << Path.substr(1);


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


[PATCH] D48292: use modern type trait implementations when available

2019-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

In D48292#1478617 , @ldionne wrote:

> I think this broke the C++03 bots: 
> http://green.lab.llvm.org/green/view/Libcxx/job/libc++%20and%20libc++abi%20trunk/CI_ARCH=64,CI_EXCEPTIONS=ON,CI_STD=c++03/103/consoleFull


Thanks, should be fixed in r359229.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D48292



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


[PATCH] D61106: [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

2019-04-25 Thread Kristóf Umann via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rC359230: [analyzer][UninitializedObjectChecker] PR41590: 
Regard _Atomic types as… (authored by Szelethus, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D61106

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  test/Analysis/cxx-uninitialized-object.cpp


Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -324,7 +324,7 @@
 inline bool isPrimitiveType(const QualType ) {
   return T->isBuiltinType() || T->isEnumeralType() ||
  T->isMemberPointerType() || T->isBlockPointerType() ||
- T->isFunctionType();
+ T->isFunctionType() || T->isAtomicType();
 }
 
 inline bool isDereferencableType(const QualType ) {
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -1130,3 +1130,18 @@
   // TODO: we'd expect the warning: {{2 uninitializeds field}}
   CXX11MemberInitTest2(); // no-warning
 }
+
+//===--===//
+// _Atomic tests.
+//===--===//
+
+struct MyAtomicInt {
+  _Atomic(int) x; // expected-note{{uninitialized field 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
+};
+
+void entry() {
+  MyAtomicInt b;
+}


Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
@@ -324,7 +324,7 @@
 inline bool isPrimitiveType(const QualType ) {
   return T->isBuiltinType() || T->isEnumeralType() ||
  T->isMemberPointerType() || T->isBlockPointerType() ||
- T->isFunctionType();
+ T->isFunctionType() || T->isAtomicType();
 }
 
 inline bool isDereferencableType(const QualType ) {
Index: test/Analysis/cxx-uninitialized-object.cpp
===
--- test/Analysis/cxx-uninitialized-object.cpp
+++ test/Analysis/cxx-uninitialized-object.cpp
@@ -1130,3 +1130,18 @@
   // TODO: we'd expect the warning: {{2 uninitializeds field}}
   CXX11MemberInitTest2(); // no-warning
 }
+
+//===--===//
+// _Atomic tests.
+//===--===//
+
+struct MyAtomicInt {
+  _Atomic(int) x; // expected-note{{uninitialized field 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
+};
+
+void entry() {
+  MyAtomicInt b;
+}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


r359230 - [analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as primitive

2019-04-25 Thread Kristof Umann via cfe-commits
Author: szelethus
Date: Thu Apr 25 13:00:51 2019
New Revision: 359230

URL: http://llvm.org/viewvc/llvm-project?rev=359230=rev
Log:
[analyzer][UninitializedObjectChecker] PR41590: Regard _Atomic types as 
primitive

https://bugs.llvm.org/show_bug.cgi?id=41590

For the following code snippet, UninitializedObjectChecker crashed:

struct MyAtomicInt {
  _Atomic(int) x;
  MyAtomicInt() {}
};

void entry() {
  MyAtomicInt b;
}

The problem was that _Atomic types were not regular records, unions,
dereferencable or primitive, making the checker hit the llvm_unreachable at
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:347.
The solution is to regard these types as primitive as well. The test case shows
that with this addition, not only are we able to get rid of the crash, but we
can identify x as uninitialized.

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

Modified:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp

Modified: 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h?rev=359230=359229=359230=diff
==
--- 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
(original)
+++ 
cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h 
Thu Apr 25 13:00:51 2019
@@ -324,7 +324,7 @@ private:
 inline bool isPrimitiveType(const QualType ) {
   return T->isBuiltinType() || T->isEnumeralType() ||
  T->isMemberPointerType() || T->isBlockPointerType() ||
- T->isFunctionType();
+ T->isFunctionType() || T->isAtomicType();
 }
 
 inline bool isDereferencableType(const QualType ) {

Modified: cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp?rev=359230=359229=359230=diff
==
--- cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp (original)
+++ cfe/trunk/test/Analysis/cxx-uninitialized-object.cpp Thu Apr 25 13:00:51 
2019
@@ -1130,3 +1130,18 @@ void fCXX11MemberInitTest2() {
   // TODO: we'd expect the warning: {{2 uninitializeds field}}
   CXX11MemberInitTest2(); // no-warning
 }
+
+//===--===//
+// _Atomic tests.
+//===--===//
+
+struct MyAtomicInt {
+  _Atomic(int) x; // expected-note{{uninitialized field 'this->x'}}
+  int dontGetFilteredByNonPedanticMode = 0;
+
+  MyAtomicInt() {} // expected-warning{{1 uninitialized field}}
+};
+
+void entry() {
+  MyAtomicInt b;
+}


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


[PATCH] D61118: [MinGW] Fix dllexport of explicit template instantiation

2019-04-25 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo updated this revision to Diff 196704.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Adjusted the patch to not warn if there is no previous declaration of the 
instantiation, allowing the dllexport attribute on the definition in that case.


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

https://reviews.llvm.org/D61118

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaDeclCXX.cpp
  lib/Sema/SemaTemplate.cpp
  test/CodeGenCXX/mingw-template-dllexport.cpp
  test/SemaCXX/dllexport.cpp

Index: test/SemaCXX/dllexport.cpp
===
--- test/SemaCXX/dllexport.cpp
+++ test/SemaCXX/dllexport.cpp
@@ -367,10 +367,16 @@
 
 // Don't instantiate class members of templates with explicit instantiation declarations, even if they are exported.
 struct IncompleteType2;
-template  struct __declspec(dllexport) ExportedTemplateWithExplicitInstantiationDecl { // expected-note{{attribute is here}}
+#ifdef MS
+// expected-note@+2{{attribute is here}}
+#endif
+template  struct __declspec(dllexport) ExportedTemplateWithExplicitInstantiationDecl {
   int f() { return sizeof(T); } // no-error
 };
-extern template struct ExportedTemplateWithExplicitInstantiationDecl; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}}
+#ifdef MS
+// expected-warning@+2{{explicit instantiation declaration should not be 'dllexport'}}
+#endif
+extern template struct ExportedTemplateWithExplicitInstantiationDecl;
 
 // Instantiate class members for explicitly instantiated exported templates.
 struct IncompleteType3; // expected-note{{forward declaration of 'IncompleteType3'}}
@@ -402,10 +408,17 @@
 
 // Warn about explicit instantiation declarations of dllexport classes.
 template  struct ExplicitInstantiationDeclTemplate {};
-extern template struct __declspec(dllexport) ExplicitInstantiationDeclTemplate; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}} expected-note{{attribute is here}}
+#ifdef MS
+// expected-warning@+2{{explicit instantiation declaration should not be 'dllexport'}} expected-note@+2{{attribute is here}}
+#endif
+extern template struct __declspec(dllexport) ExplicitInstantiationDeclTemplate;
 
-template  struct __declspec(dllexport) ExplicitInstantiationDeclExportedTemplate {}; // expected-note{{attribute is here}}
-extern template struct ExplicitInstantiationDeclExportedTemplate; // expected-warning{{explicit instantiation declaration should not be 'dllexport'}}
+template  struct __declspec(dllexport) ExplicitInstantiationDeclExportedTemplate {};
+#ifdef MS
+// expected-note@-2{{attribute is here}}
+// expected-warning@+2{{explicit instantiation declaration should not be 'dllexport'}}
+#endif
+extern template struct ExplicitInstantiationDeclExportedTemplate;
 
 namespace { struct InternalLinkageType {}; }
 struct __declspec(dllexport) PR23308 {
@@ -438,6 +451,12 @@
 template struct ExplicitlyInstantiatedTemplate;
 template  struct ExplicitlyExportInstantiatedTemplate { void func() {} };
 template struct __declspec(dllexport) ExplicitlyExportInstantiatedTemplate;
+template  struct ExplicitlyExportDeclaredInstantiatedTemplate { void func() {} };
+extern template struct ExplicitlyExportDeclaredInstantiatedTemplate;
+#ifndef MS
+// expected-warning@+2{{'dllexport' attribute ignored on explicit instantiation definition}}
+#endif
+template struct __declspec(dllexport) ExplicitlyExportDeclaredInstantiatedTemplate;
 template  struct ExplicitlyImportInstantiatedTemplate { void func() {} };
 template struct __declspec(dllimport) ExplicitlyImportInstantiatedTemplate;
 
Index: test/CodeGenCXX/mingw-template-dllexport.cpp
===
--- /dev/null
+++ test/CodeGenCXX/mingw-template-dllexport.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -triple i686-mingw32 %s -o - | FileCheck %s
+
+template 
+class c {
+  void f();
+};
+
+template  void c::f() {}
+
+template class __declspec(dllexport) c;
+
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIiE1fEv
+
+extern template class __declspec(dllexport) c;
+template class c;
+
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN1cIcE1fEv
+
+extern template class c;
+template class __declspec(dllexport) c;
+
+// CHECK-NOT: define {{.*}} dllexport {{.*}} @_ZN1cIdE1fEv
+
+template 
+struct outer {
+  void f();
+  struct inner {
+void f();
+  };
+};
+
+template  void outer::f() {}
+template  void outer::inner::f() {}
+
+template class __declspec(dllexport) outer;
+
+// CHECK: define {{.*}} dllexport {{.*}} @_ZN5outerIiE1fEv
+// CHECK-NOT: define {{.*}} dllexport {{.*}} @_ZN5outerIiE5inner1fEv
Index: lib/Sema/SemaTemplate.cpp
===
--- lib/Sema/SemaTemplate.cpp
+++ lib/Sema/SemaTemplate.cpp
@@ -8709,8 +8709,10 @@
? TSK_ExplicitInstantiationDefinition
 

[PATCH] D58033: Add option for emitting dbg info for call site parameters

2019-04-25 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment.

I'm okay with this, but give @aprantl a chance to confirm.


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

https://reviews.llvm.org/D58033



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


[PATCH] D60349: [COFF, ARM64] Fix ABI implementation of struct returns

2019-04-25 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added inline comments.



Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1074
+  if (!RD->hasTrivialCopyAssignment())
+return true;
+  return false;

richard.townsend.arm wrote:
> richard.townsend.arm wrote:
> > Should this function also check for user-provided constructors?
> I think it should: I speculatively added these two lines
> 
>   if (RD->hasUserDeclaredConstructor())
> return true;
> 
> and it resolved the problem with `std::setw` I mentioned in the bug tracker 
> (which means Electron could start). 
The comment says it's supposed to check for user-provided constructors, but 
there isn't any code to perform that check, yes.

hasUserDeclaredConstructor() isn't precisely correct; from the C++ standard, "A 
function is user-provided if it is user-declared and not explicitly defaulted 
or deleted on its first declaration."


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

https://reviews.llvm.org/D60349



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


[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

2019-04-25 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment.

Sorry, that test case actually was a bit too complicated. This seems to work:

  diff --git a/clang/test/Import/cxx-record-flags/Inputs/F.cpp 
b/clang/test/Import/cxx-record-flags/Inputs/F.cpp
  new file mode 100644
  index 000..1294c67f68d
  --- /dev/null
  +++ b/clang/test/Import/cxx-record-flags/Inputs/F.cpp
  @@ -0,0 +1,9 @@
  +class FTrivial {
  +  int i;
  +};
  +
  +struct FNonTrivial {
  +  virtual ~FNonTrivial() = default;
  +  int i;
  +};
  +
  diff --git a/clang/test/Import/cxx-record-flags/test.cpp 
b/clang/test/Import/cxx-record-flags/test.cpp
  new file mode 100644
  index 000..bff76274fba
  --- /dev/null
  +++ b/clang/test/Import/cxx-record-flags/test.cpp
  @@ -0,0 +1,14 @@
  +// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
  +
  +// CHECK: FTrivial
  +// CHECK: DefinitionData
  +// CHECK-SAME: pass_in_registers
  +
  +// CHECK: FNonTrivial
  +// CHECK-NOT: pass_in_registers
  +// CHECK: DefaultConstructor
  +
  +void expr() {
  +  FTrivial f1;
  +  FNonTrivial f2;
  +}


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

https://reviews.llvm.org/D61140



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


[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

2019-04-25 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone updated this revision to Diff 196699.
Quuxplusone added a comment.

Add tests that `LIBCPP_ASSERT_NOEXCEPT` the `options` and `upstream_resource` 
accessors.


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D47358

Files:
  include/experimental/memory_resource
  src/experimental/memory_resource.cpp
  
test/libcxx/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsynchronized_buffer.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/ctor_does_not_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/sync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.ctor/unsync_with_default_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/equality.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_deallocate_matches_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_options.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/sync_upstream_resource.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_overaligned_request.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_allocate_reuse_blocks.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_deallocate_matches_allocate.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_options.pass.cpp
  
test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_upstream_resource.pass.cpp
  test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
  test/support/count_new.hpp

Index: test/support/count_new.hpp
===
--- test/support/count_new.hpp
+++ test/support/count_new.hpp
@@ -210,6 +210,11 @@
 return disable_checking || n != delete_called;
 }
 
+bool checkDeleteCalledGreaterThan(int n) const
+{
+return disable_checking || delete_called > n;
+}
+
 bool checkAlignedNewCalledEq(int n) const
 {
 return disable_checking || n == aligned_new_called;
Index: test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/pool_options.pass.cpp
@@ -0,0 +1,28 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03, c++11, c++14
+// UNSUPPORTED: apple-clang-7
+
+// 
+
+// struct pool_options
+
+#include 
+#include 
+
+int main(int, char**)
+{
+const std::experimental::pmr::pool_options p;
+assert(p.max_blocks_per_chunk == 0);
+assert(p.largest_required_pool_block == 0);
+
+return 0;
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_upstream_resource.pass.cpp
===
--- /dev/null
+++ test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_upstream_resource.pass.cpp
@@ -0,0 +1,28 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// REQUIRES: c++experimental
+// UNSUPPORTED: c++98, c++03
+
+// 
+
+// class unsynchronized_pool_resource
+
+#include 
+#include 
+
+#include "count_new.hpp"
+
+int main(int, char**)
+{
+std::experimental::pmr::unsynchronized_pool_resource unsync;
+LIBCPP_ASSERT_NOEXCEPT(unsync.upstream_resource());
+
+return 0;
+}
Index: test/std/experimental/memory/memory.resource.pool/memory.resource.pool.mem/unsync_options.pass.cpp

[PATCH] D60934: [clang] adding explicit(bool) from c++2a

2019-04-25 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith marked an inline comment as done.
rsmith added inline comments.



Comment at: clang/include/clang/AST/DeclCXX.h:2022
+
+  bool isEquivalentExplicit(const CXXDeductionGuideDecl *Other) const;
+  bool isExplicit() const {

I think this is too special-case to live on `CXXDeductionGuideDecl`. Instead, I 
suggest:

 * Add a class wrapping `ExplicitSpecInfo`, maybe called `ExplicitSpecifier`
 * Change all the AST interfaces that deal in `ExplicitSpecInfo` to instead 
deal in `ExplicitSpecifier` objects
 * Move this comparison logic there, along with the more uncommon members you 
add below.

I think we should be able to reduce the set of `explicit`-related members on 
the various `Decl` subclasses to just `getExplicitSpecifier()` and 
`isExplicit()`.



Comment at: clang/include/clang/AST/DeclCXX.h:2033
+
+  void setExplicitSpecifier(ExplicitSpecInfo ESI);
+

Generally we don't want to have setters in the AST; the AST is intended to be 
immutable after creation. Is this necessary?



Comment at: clang/include/clang/AST/DeclCXX.h:2519
 
+  ExplicitSpecInfo ExplicitSpecifier;
+

Consider storing this in a `TrailingObject` rather than making all constructors 
a pointer wider for this (presumably relatively uncommon) special case.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:37
+def warn_explicit_bool_breaking_change_cxx17 : Warning<
+  "this expression would be parsed as explicit(bool) in C++2a">
+  , InGroup, DefaultIgnore;

would be -> will be



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:38
+  "this expression would be parsed as explicit(bool) in C++2a">
+  , InGroup, DefaultIgnore;
+def note_explicit_bool_breaking_change_cxx2a : Note<

Nit: comma on previous line, please.



Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:40
+def note_explicit_bool_breaking_change_cxx2a : Note<
+  "this expression is parsed as explicit(bool) since C++2a">;
+

This should be a warning in the `CXXPre2aCompat` group, phrased as 
"explicit(bool) is incompatible with C++ standards before C++2a".



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2124-2129
+def err_deduction_guide_explicit_bool_mismatch : Error<
+  "the deduction guide is %select{not ||potentially }0explicit but "
+  "previous declaration was %select{not ||potentially }1explicit">;
+def err_unresolved_explicit_mismatch : Error<
+  "explicit specifier is not equivalent to the explicit specifier"
+  " from previous declaration">;

I don't believe there is any such rule; instead, the rule is simply that 
deduction guides cannot be redeclared at all. See [temp.deduct.guide]/3: "Two 
deduction guide declarations in the same translation unit for the same class 
template shall not have equivalent parameter-declaration-clauses." (That's 
obviously wrong; we should compare the template-head as well, but the intention 
is that deduction guides cannot be redeclared.)



Comment at: clang/include/clang/Basic/Specifiers.h:27-29
+ESF_resolved_false,
+ESF_resolved_true,
+ESF_unresolved

Please capitalize these enumerator names, and consider using an 'enum class' 
instead of an `ESF_` prefix. (We use lowercase for the next few `enum`s because 
their enumerators are (prefixed) keywords.)



Comment at: clang/include/clang/Sema/DeclSpec.h:376
+  using ExplicitSpecifierInfo =
+  llvm::PointerIntPair;
+

If this is a fully-semantically-analyzed explicit-specifier, this should use 
the same type we use in the AST representation. If it's a 
parsed-but-not-yet-semantically-analyzed explicit-specifier, using 
`ExplicitSpecFlag` doesn't seem right: you haven't tried to resolve it yet in 
that case.



Comment at: clang/lib/AST/DeclCXX.cpp:1867-1878
+static bool MaybeResolveExplicit(FunctionDecl *Decl, ExplicitSpecInfo ) {
+  if (ESI.getInt() != ESF_unresolved || !ESI.getPointer())
+return true;
+  APValue Result;
+  if (ESI.getPointer()->EvaluateWithSubstitution(
+  Result, Decl->getASTContext(), Decl, llvm::ArrayRef())) {
+ESI.setInt(Result.getInt().getBoolValue() ? ESF_resolved_true

Please don't evaluate the explicit specifier here. Instead, it should be 
evaluated by `Sema` when it's initially formed (if it's not value-dependent) 
and during template instantiation (if a value-dependent explicit-specifier 
becomes non-value-dependent). Note that `Sema` needs to evaluate it at those 
times anyway, in order to properly diagnose non-constant explicit-specifiers.



Comment at: clang/lib/AST/DeclCXX.cpp:2514-2522
 bool CXXConstructorDecl::isConvertingConstructor(bool AllowExplicit) const {
   // C++ [class.conv.ctor]p1:
   //   A constructor declared without 

[PATCH] D61142: Set LoopInterleaved in the PassManagerBuilder.

2019-04-25 Thread Alina Sbirlea via Phabricator via cfe-commits
asbirlea created this revision.
Herald added subscribers: cfe-commits, jlebar.
Herald added a project: clang.

Corresponds to D61030 .


Repository:
  rC Clang

https://reviews.llvm.org/D61142

Files:
  lib/CodeGen/BackendUtil.cpp


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -544,6 +544,7 @@
   PMBuilder.LoopVectorize = CodeGenOpts.VectorizeLoop;
 
   PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops;
+  PMBuilder.LoopsInterleaved = CodeGenOpts.UnrollLoops;
   PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
   PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;


Index: lib/CodeGen/BackendUtil.cpp
===
--- lib/CodeGen/BackendUtil.cpp
+++ lib/CodeGen/BackendUtil.cpp
@@ -544,6 +544,7 @@
   PMBuilder.LoopVectorize = CodeGenOpts.VectorizeLoop;
 
   PMBuilder.DisableUnrollLoops = !CodeGenOpts.UnrollLoops;
+  PMBuilder.LoopsInterleaved = CodeGenOpts.UnrollLoops;
   PMBuilder.MergeFunctions = CodeGenOpts.MergeFunctions;
   PMBuilder.PrepareForThinLTO = CodeGenOpts.PrepareForThinLTO;
   PMBuilder.PrepareForLTO = CodeGenOpts.PrepareForLTO;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D60485: [AArch64] Add support for MTE intrinsics

2019-04-25 Thread Tim Northover via Phabricator via cfe-commits
t.p.northover accepted this revision.
t.p.northover added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/CodeGen/CGBuiltin.cpp:7129-7131
+// Although it is possible to supply a different return
+// address (first arg) to this intrinsic, for now we set
+// return address same as input address.

javed.absar wrote:
> t.p.northover wrote:
> > I think this should be fixed now.  It looks like technical debt from the 
> > fact that the instructions only fairly recently gained that feature after 
> > the intrinsics were implemented internally. There's no good way to justify 
> > the current semantics to someone unaware of that history.
> Not quite that really.  So the instruction did gain the feature recently like 
> you mentioned. But the ACLE/intrinsics were designed and agreed upon after it 
> and it was decided in ACLE discussions that the exta feature added complexity 
> that need not be exposed at ACLE level yet. No big use case to justify 
> complicating the ACLE MTE spec yet. Directly assembly can use that 
> instruction though.
I think the ACLE decision was a mistake, but since it happened I withdraw this 
request. I expect (and hope) far more people will use these through ACLE than 
as compiler-specific builtins.


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

https://reviews.llvm.org/D60485



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


[PATCH] D60907: [OpenMP] Add math functions support in OpenMP offloading

2019-04-25 Thread Hal Finkel via Phabricator via cfe-commits
hfinkel added a comment.

In D60907#1479118 , @gtbercea wrote:

> Ping @hfinkel @tra


The last two comments in D47849  indicated 
exploration of a different approach, and one which still seems superior to this 
one. Can you please comment on why you're now pursuing this approach instead?


Repository:
  rC Clang

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

https://reviews.llvm.org/D60907



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


[PATCH] D47358: : Implement {un,}synchronized_pool_resource.

2019-04-25 Thread Marshall Clow via Phabricator via cfe-commits
mclow.lists added a comment.

In D47358#1438929 , @Quuxplusone wrote:

> Rebased. Added `_NOEXCEPT` to `upstream_resource()` and `options()` (this is 
> OK per [res.on.exception.handling]/5).


That's fine, but then we should have a test for that.
We have the macro `LIBCPP_ASSERT_NOEXCEPT` specifically for this purpose 
(libc++ - specific noexcept markings).


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

https://reviews.llvm.org/D47358



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


[PATCH] D61140: Copy Argument Passing Restrictions setting when importing a CXXRecordDecl definition

2019-04-25 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment.

In D61140#1479111 , @aprantl wrote:

> Could we test this by doing -dump-ast of From and To and FileCheck-ing the 
> output?


+1 for this. Importing a simple record should test this? E.g. something like 
this:

  diff --git a/clang/test/Import/cxx-record-flags/Inputs/F.cpp 
b/clang/test/Import/cxx-record-flags/Inputs/F.cpp
  new file mode 100644
  index 000..8d1d88c632d
  --- /dev/null
  +++ b/clang/test/Import/cxx-record-flags/Inputs/F.cpp
  @@ -0,0 +1,11 @@
  +class FTrivial {
  +  int i;
  +};
  +
  +void foo();
  +
  +struct FNonTrivial {
  +  FNonTrivial(const FNonTrivial ) { foo(); }
  +  int i;
  +};
  +
  diff --git a/clang/test/Import/cxx-record-flags/test.cpp 
b/clang/test/Import/cxx-record-flags/test.cpp
  new file mode 100644
  index 000..c51558b4793
  --- /dev/null
  +++ b/clang/test/Import/cxx-record-flags/test.cpp
  @@ -0,0 +1,15 @@
  +// RUN: clang-import-test -dump-ast -import %S/Inputs/F.cpp -expression %s | 
FileCheck %s
  +
  +// CHECK: FTrivial
  +// CHECK-NEXT: DefinitionData
  +// CHECK-SAME: pass_in_registers
  +
  +// CHECK: FNonTrivial
  +// CHECK-NEXT: DefinitionData
  +// CHECK-NOT: pass_in_registers
  +// CHECK: DefaultConstructor
  +
  +void expr() {
  +  FTrivial f1;
  +  FNonTrivial f2;
  +}


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

https://reviews.llvm.org/D61140



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


[PATCH] D59725: Additions to creduce script

2019-04-25 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added inline comments.



Comment at: cfe/trunk/utils/creduce-clang-crash.py:185
+for msg in self.expected_output:
+  output += 'grep %s t.log || exit 1\n' % pipes.quote(msg)
+

akhuang wrote:
> lebedev.ri wrote:
> > >>! In D59725#1477362, @arichardson wrote:
> > >>>! In D59725#1477042, @lebedev.ri wrote:
> > >> I've stumbled into an issue with the script:
> > >> It got a line: `clang: 
> > >> /build/llvm/lib/Transforms/Scalar/IndVarSimplify.cpp:2582: bool 
> > >> {anonymous}::IndVarSimplify::run(llvm::Loop*): Assertion 
> > >> `L->isRecursivelyLCSSAForm(*DT, *LI) && "LCSSA required to run 
> > >> indvars!"' failed.`
> > >> And produced the following grep: `grep 'L->isRecursivelyLCSSAForm(*DT, 
> > >> *LI) && "LCSSA required to run indvars!"' t.log || exit 1`
> > >> But that doesn't work, escapes should be applied. it should be
> > >> `grep 'L->isRecursivelyLCSSAForm(\*DT, \*LI) && \"LCSSA required to run 
> > >> indvars!\"' t.log || exit 1`
> > > 
> > > In my script I use `grep -F " + shlex.quote(self.args.crash_message)` 
> > > which should escape both the message and ensure that the grep argument is 
> > > not interpreted as a regex.
> > 
> > Great to hear!
> > It appears that the script is currently more primitive than that currently 
> > though.
> Thanks; I added a `-F` flag (r359216), and it seems like that fixes the 
> issue. 
Thanks!
What about `shlex`? How do you know bash won't expand that as regex already?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59725



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


  1   2   >