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

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

In D71241#1778736 , @ABataev wrote:

> In D71241#1778717 , @jdoerfert wrote:
>
> > >> There is no evidence that this is more complicated. By all measures, 
> > >> this is less complicated (see also below). It is also actually doing the 
> > >> right thing when it comes to code emission. Take 
> > >> https://godbolt.org/z/sJiP3B for example. The calls are wrong and the 
> > >> definition of base is missing.
> > > 
> > > How did you measure it? I have a completely different opinion. Also, 
> > > tried to reproduce the problem locally, could not reproduce. It seems to 
> > > me, the output of the test misses several important things. You can check 
> > > it yourself. `tgt_target_teams()` call uses `@.offload_maptypes` global 
> > > var but it is not defined.
> >
> > Here is the link with the globals not hidden: https://godbolt.org/z/5etB5S
> >  The base function is called both times but should not be called at all. 
> > What is your local output and why does it differ?
>
>
> On the host `base` is an alias for the `hst` function. On the device `base` 
> has the body of `dev` function because NVPTX does nit support function 
> aliases (10+ suppprts it, but LLVM does not support it yet). Try to change 
> the bodies of dev and hst and you will see.
>
> I tried to keep original function names to improve debugging and make users 
> less wonder why instead of `base` something else is called.


How does that work with linking? Another translation unit can call both the 
base and hst/dev function, right? I mean they both need to be present.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71082: Allow system header to provide their own implementation of some builtin

2019-12-10 Thread serge via Phabricator via cfe-commits
serge-sans-paille added a comment.

@george.burgess.iv : up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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


[PATCH] D71300: [clangd] Deduplicate refs from index for cross-file rename.

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

The fix itself LGTM. Just a few NITs and this is good to go.




Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:34
+// Covnert a Range to a Ref.
+std::pair toRef(const clangd::Range ,
+  llvm::StringRef Path) {

- Returning both storage and `Ref` is quite a complicated interface. Could we 
rely on the clients to pass strings with URIs instead? It's not a lot of code 
to create URIs.
- I think the function name might end up a little confusing when reading the 
code using it (it certainly did for me). Could we rename it to `refWithRange`? 
That seems to describe what this function is doing more accurately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71300



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


[PATCH] D71208: CodeGen: Allow annotations on globals in non-zero address space

2019-12-10 Thread John McCall via Phabricator via cfe-commits
rjmccall accepted this revision.
rjmccall added a comment.
This revision is now accepted and ready to land.

In D71208#1776667 , @nhaehnle wrote:

> > My concern is that there's something that's going to blow up or miscompile 
> > if we start passing in constants that aren't in a regular address space.  
> > Aren't there kinds of annotations which get persisted into the emitted code?
>
> Annotations don't seem to be used for much at the moment in the first place. 
> They're definitely not emitted in the resulting binary by default. Also, the 
> only testcase in LLVM proper that has @llvm.global.annotations also happens 
> to have addrspacecasts in there, so at least at some point in the past 
> somebody thought it's okay to allow those casts there.


Alright, I relent.  This looks okay to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71208



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


[PATCH] D71325: [Remarks][Driver] Ask for line tables when remarks are enabled

2019-12-10 Thread Francis Visoiu Mistrih via Phabricator via cfe-commits
thegameg created this revision.
thegameg added reviewers: JDevlieghere, friss, aprantl.
Herald added a project: clang.

Serialized remarks contain debug locations for each remark, by storing a file 
path, a line, and a column.

Also, remarks support being embedded in a .dSYM bundle using a separate section 
in object files, that is found by `dsymutil` through the debug map.

In order for tools to map addresses to source and display remarks in the 
source, we need line tables, and in order for `dsymutil` to find the object 
files containing the remark section, we need to keep the debug map around.


Repository:
  rC Clang

https://reviews.llvm.org/D71325

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/debug-options.c


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,13 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  (DebugInfoKind == codegenoptions::NoDebugInfo ||
+   DebugInfoKind == codegenoptions::LocTrackingOnly ||
+   DebugInfoKind == codegenoptions::DebugDirectivesOnly))
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 


Index: clang/test/Driver/debug-options.c
===
--- clang/test/Driver/debug-options.c
+++ clang/test/Driver/debug-options.c
@@ -64,6 +64,12 @@
 // RUN: %clang -### -c -g %s -target arm64-apple-tvos9.0 2>&1 \
 // RUN: | FileCheck -check-prefix=G_STANDALONE \
 // RUN: -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLTO_ONLY %s
+// RUN: %clang -### -c -g -fsave-optimization-record %s \
+// RUN:-target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE %s
 
 // FreeBSD.
 // RUN: %clang -### -c -g %s -target x86_64-pc-freebsd11.0 2>&1 \
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3717,6 +3717,13 @@
   // Adjust the debug info kind for the given toolchain.
   TC.adjustDebugInfoKind(DebugInfoKind, Args);
 
+  // When emitting remarks, we need at least debug lines in the output.
+  if (shouldEmitRemarks(Args) &&
+  (DebugInfoKind == codegenoptions::NoDebugInfo ||
+   DebugInfoKind == codegenoptions::LocTrackingOnly ||
+   DebugInfoKind == codegenoptions::DebugDirectivesOnly))
+DebugInfoKind = codegenoptions::DebugLineTablesOnly;
+
   RenderDebugEnablingArgs(Args, CmdArgs, DebugInfoKind, DWARFVersion,
   DebuggerTuning);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] f7235ac - [cxx_status] Re-add missing cell.

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T19:56:07-08:00
New Revision: f7235ac1d3154d5cd82a9a6bfdfffd050e6f5d05

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

LOG: [cxx_status] Re-add missing cell.

Added: 


Modified: 
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index 2d5a7a32f4a3..ffac33387960 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -951,6 +951,7 @@ C++2a implementation status
   
   
 https://wg21.link/p1959r0;>P1959R0
+No
   
 
   Access checking on specializations



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


[clang] ffe6129 - [c++20] Implement P1946R0: allow defaulted comparisons to take their

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T19:54:35-08:00
New Revision: ffe612922cb5aa2767c79d47a1b162811a08583f

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

LOG: [c++20] Implement P1946R0: allow defaulted comparisons to take their
arguments by value.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
clang/test/CXX/class/class.compare/class.eq/p3.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 4eec4b066ae5..423ccbd293d2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8194,7 +8194,10 @@ def err_defaulted_comparison_out_of_class : Error<
   "definition">;
 def err_defaulted_comparison_param : Error<
   "invalid parameter type for defaulted 
%sub{select_defaulted_comparison_kind}0"
-  "%
diff {; found $, expected $|}1,2">;
+  "; found %1, expected %2%select{| or %4}3">;
+def err_defaulted_comparison_param_mismatch : Error<
+  "parameters for defaulted %sub{select_defaulted_comparison_kind}0 "
+  "must have the same type%
diff { (found $ vs $)|}1,2">;
 def err_defaulted_comparison_non_const : Error<
   "defaulted member %sub{select_defaulted_comparison_kind}0 must be "
   "const-qualified">;

diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 365286097699..5373bb422ad6 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -7921,22 +7921,41 @@ bool Sema::CheckExplicitlyDefaultedComparison(Scope *S, 
FunctionDecl *FD,
   //   non-template function declared in the member-specification of C that is
   //-- a non-static const member of C having one parameter of type
   //   const C&, or
-  //-- a friend of C having two parameters of type const C&.
-  QualType ExpectedParmType =
-  Context.getLValueReferenceType(Context.getRecordType(RD).withConst());
+  //-- a friend of C having two parameters of type const C& or two
+  //   parameters of type C.
+  QualType ExpectedParmType1 = Context.getRecordType(RD);
+  QualType ExpectedParmType2 =
+  Context.getLValueReferenceType(ExpectedParmType1.withConst());
+  if (isa(FD))
+ExpectedParmType1 = ExpectedParmType2;
   for (const ParmVarDecl *Param : FD->parameters()) {
 if (!Param->getType()->isDependentType() &&
-!Context.hasSameType(Param->getType(), ExpectedParmType)) {
+!Context.hasSameType(Param->getType(), ExpectedParmType1) &&
+!Context.hasSameType(Param->getType(), ExpectedParmType2)) {
   // Don't diagnose an implicit 'operator=='; we will have diagnosed the
   // corresponding defaulted 'operator<=>' already.
   if (!FD->isImplicit()) {
 Diag(FD->getLocation(), diag::err_defaulted_comparison_param)
-<< (int)DCK << Param->getType() << ExpectedParmType
-<< Param->getSourceRange();
+<< (int)DCK << Param->getType() << ExpectedParmType1
+<< !isa(FD)
+<< ExpectedParmType2 << Param->getSourceRange();
   }
   return true;
 }
   }
+  if (FD->getNumParams() == 2 &&
+  !Context.hasSameType(FD->getParamDecl(0)->getType(),
+   FD->getParamDecl(1)->getType())) {
+if (!FD->isImplicit()) {
+  Diag(FD->getLocation(), diag::err_defaulted_comparison_param_mismatch)
+  << (int)DCK
+  << FD->getParamDecl(0)->getType()
+  << FD->getParamDecl(0)->getSourceRange()
+  << FD->getParamDecl(1)->getType()
+  << FD->getParamDecl(1)->getSourceRange();
+}
+return true;
+  }
 
   // ... non-static const member ...
   if (auto *MD = dyn_cast(FD)) {

diff  --git a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp 
b/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
index a73b34870769..1a0ccc91741b 100644
--- a/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
+++ b/clang/test/CXX/class/class.compare/class.compare.default/p1.cpp
@@ -1,7 +1,7 @@
 // RUN: %clang_cc1 -std=c++2a -verify %s
 
 struct B {};
-bool operator==(const B&, const B&) = default; // expected-error {{equality 
comparison operator can only be defaulted in a class definition}}
+bool operator==(const B&, const B&) = default; // expected-error {{equality 
comparison operator can only be defaulted in a class definition}} expected-note 
{{candidate}}
 bool operator<=>(const B&, const B&) = default; // expected-error {{three-way 
comparison operator can only be defaulted in a class definition}}
 
 template
@@ -9,18 +9,19 @@ template
 
 struct A {
   friend 

[PATCH] D70411: [analyzer] CERT: StrChecker: Implementing most of the STR31-C

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso added a comment.

In D70411#1776460 , @NoQ wrote:

> Ok, so, what should the checker warn on? What should be the intended state 
> machine for this checker?
>
> We basically have two possible categories of answers to this question:
>
> - "The checker warns every time it finds an execution path on which 
> unintended behavior occurs" - describe unintended behavior (say, 
> "out-of-bounds buffer access") and define the checker in such a way that all 
> warnings describe actual bugs of this kind (in our example, out-of-bound 
> buffer accesses) as long as the rest of the static analyzer works correctly.
> - Or, you could say "The checker enforces a certain coding guideline on the 
> user" - and then you should describe the guideline in a very short and 
> easy-to-understand manner (say, "don't pass the string by pointer anywhere 
> before you null-terminate it"), and then make checker check exactly this 
> guideline. You may introduce some suppressions for intentional violations of 
> the guideline, but the guideline itself should be simple, it should be always 
> possible to follow it, and it should sound nice enough for developers to feel 
> good about themselves when they follow it.


If I am right you are thinking about warn on the unsafe function calls which is 
the first case. I did that on by default. The idea is that, in a security 
manner we cannot allow hand-waving fixes. To measure stuff we could introduce 
options which serves the second case. I have added an option to suppress that 
common null-termination-by-hand idiom, so that I do not recommend anything. The 
CERT rules are not recommendations so I think now the warning is fine.

> Yup, i mean, you can go as far as you want with generalizing over "fixed size 
> buffer" in this approach, but what i'm saying is that you're still not 
> addressing the whole train of thought about the length of the source string.

I see, and you are right, but we are getting closer and closer to catch the 
most of.

> In D70411#1773703 , @Charusso wrote:
> 
>> > F10967277: ftp.c_8cb07866de61ef56be82135a4d3a5b7e.plist.html 
>> > 
>> >  The source string is an IP address and port, which has a known limit on 
>> > the number of digits it can have.
>>
>> The known size does not mean that the string is going to be null-terminated.
> 
> 
> It does, because `strcpy` is guaranteed to null-terminate as long as it has 
> enough storage capacity.

It does not, because the storage capacity is arbitrary. If we would be sure the 
**copied stuff's length** cannot be larger than the destination's arbitrary 
capacity, we would not warn. There are tests for that case.

> Checks that are part of the generic taint checker are currently in a pretty 
> bad shape, but the taint analysis itself is pretty good, and checks that rely 
> on taint but aren't part of the generic taint checker are also pretty good. I 
> actually believe taint analysis could be enabled by default as soon as 
> somebody goes through the broken checks and disables/removes them. If you 
> rely on the existing taint analysis infrastructure and make a good check, 
> that'll be wonderful and would further encourage us to move taint analysis 
> out of alpha.

I think it is too far away, sadly. I like the idea because it would be the root 
of security checkers, but I am mostly interested in data-flow analysis.




Comment at: clang/test/Analysis/cert/str31-c-fp-suppression.cpp:51-53
+  strcpy(dest, src);
+  do_something(dest);
+  // expected-warning@-1 {{'dest' is not null-terminated}}

NoQ wrote:
> Charusso wrote:
> > NoQ wrote:
> > > Looks like a false positive. Consider the following perfectly correct 
> > > (even if a bit inefficient) code that only differs from the current code 
> > > only in names and context:
> > > ```lang=c++
> > > void copy_and_test_if_short(const char *src) {
> > >   char dest[128];
> > >   strcpy(dest, src);
> > >   warn_if_starts_with_foo(dest);
> > > }
> > > 
> > > void copy_and_test(const char *src) {
> > >   if (strlen(src) < 64)
> > > copy_and_test_if_short(src);
> > >   else
> > > copy_and_test_if_long(src);
> > > }
> > > ```
> > That is why I have asked whether we have a way to say "when the string is 
> > read". I like that heuristic for now, because any kind of not 
> > null-terminated string is the root of the evil for [[ 
> > https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
> >  | STR31-C ]].
> > 
> > In this case the attacker is the programmer who sends a possibly not 
> > null-terminated string around and the function which receives an arbitrary 
> > input would carry the issue.
> > 
> > Thanks for the example, I wanted to cover ranges, but I have forgotten it, 
> > because in the wild peoples do not check for 

[PATCH] D70411: [analyzer] CERT: StrChecker: Implementing most of the STR31-C

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso updated this revision to Diff 233252.
Charusso marked 7 inline comments as done.
Charusso retitled this revision from "[analyzer] CERT: StrChecker: 31.c" to 
"[analyzer] CERT: StrChecker: Implementing most of the STR31-C".
Charusso edited the summary of this revision.
Charusso added a comment.

- Fix.


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

https://reviews.llvm.org/D70411

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/BugReporter/CommonBugCategories.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/DynamicSizeInfo.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/lib/StaticAnalyzer/Checkers/AllocationState.h
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/cert/StrChecker.cpp
  clang/lib/StaticAnalyzer/Core/CommonBugCategories.cpp
  clang/test/Analysis/Inputs/system-header-simulator.h
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/cert/str31-c-notes-warn-on-call-off.cpp
  clang/test/Analysis/cert/str31-c-notes-warn-on-call-on.cpp
  clang/test/Analysis/cert/str31-c-warn-on-call-off.cpp
  clang/test/Analysis/cert/str31-c-warn-on-call-on.cpp
  clang/test/Analysis/cert/str31-c.cpp

Index: clang/test/Analysis/cert/str31-c.cpp
===
--- /dev/null
+++ clang/test/Analysis/cert/str31-c.cpp
@@ -0,0 +1,196 @@
+// RUN: %clang_analyze_cc1 \
+// RUN:  -analyzer-checker=core,unix,alpha.security.cert.str.31c \
+// RUN:  -verify %s
+
+// See the examples on the page of STR31-C:
+// https://wiki.sei.cmu.edu/confluence/display/c/STR31-C.+Guarantee+that+storage+for+strings+has+sufficient+space+for+character+data+and+the+null+terminator
+
+#include "../Inputs/system-header-simulator.h"
+
+#define EOF -1
+typedef __SIZE_TYPE__ size_t;
+
+void free(void *memblock);
+void *malloc(size_t size);
+
+void do_something(char *buffer);
+
+namespace test_gets_bad {
+#define BUFFER_SIZE 1024
+
+void func(void) {
+  char buf[BUFFER_SIZE];
+  if (gets(buf)) {
+// expected-warning@-1 {{'gets' could write outside of 'buf'}}
+  }
+}
+} // namespace test_gets_bad
+
+namespace test_gets_good {
+enum { BUFFERSIZE = 32 };
+
+void func(void) {
+  char buff[BUFFERSIZE];
+
+  if (fgets(buff, sizeof(buff), stdin)) {
+  }
+}
+} // namespace test_gets_good
+
+namespace test_sprintf_bad {
+void func(const char *name) {
+  char buf[128];
+  sprintf(buf, "%s.txt", name);
+  // expected-warning@-1 {{'sprintf' could write outside of 'buf'}}
+}
+} // namespace test_sprintf_bad
+
+namespace test_sprintf_good {
+void func(const char *name) {
+  char buff[128];
+  snprintf(buff, sizeof(buff), "%s.txt", name);
+
+  do_something(buff);
+}
+} // namespace test_sprintf_good
+
+namespace test_fscanf_bad {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buf[BUF_LENGTH];
+  if (fscanf(stdin, "%s", buf)) {
+// expected-warning@-1 {{'fscanf' could write outside of 'buf'}}
+  }
+}
+} // namespace test_fscanf_bad
+
+namespace test_fscanf_good {
+enum { BUF_LENGTH = 1024 };
+
+void get_data(void) {
+  char buff[BUF_LENGTH];
+  if (fscanf(stdin, "%1023s", buff)) {
+do_something(buff);
+  }
+}
+} // namespace test_fscanf_good
+
+namespace test_strcpy_bad {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char prog_name[128];
+  strcpy(prog_name, name);
+  // expected-warning@-1 {{'strcpy' could write outside of 'prog_name'}}
+
+  return 0;
+}
+
+void func(void) {
+  char buff[256];
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+strcpy(buff, editor);
+// expected-warning@-1 {{'strcpy' could write outside of 'buff'}}
+  }
+}
+} // namespace test_strcpy_bad
+
+namespace test_strcpy_good {
+int main(int argc, char *argv[]) {
+  const char *const name = (argc && argv[0]) ? argv[0] : "";
+  char *prog_name2 = (char *)malloc(strlen(name) + 1);
+  if (prog_name2 != NULL) {
+strcpy(prog_name2, name);
+  }
+
+  do_something(prog_name2);
+
+  free(prog_name2);
+  return 0;
+}
+
+void func(void) {
+  char *buff2;
+  char *editor = getenv("EDITOR");
+  if (editor != NULL) {
+size_t len = strlen(editor) + 1;
+buff2 = (char *)malloc(len);
+if (buff2 != NULL) {
+  strcpy(buff2, editor);
+}
+
+do_something(buff2);
+free(buff2);
+  }
+}
+} // namespace test_strcpy_good
+
+//===--===//
+// The following are from the rule's page which we do not handle yet.
+//===--===//
+
+namespace test_loop_index_bad {
+void copy(size_t n, char src[n], char dest[n]) {
+  size_t i;
+
+  for (i = 0; src[i] && (i < n); ++i) {
+dest[i] = src[i];
+  }
+  dest[i] = '\0';
+}
+} // namespace test_loop_index_bad
+

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

2019-12-10 Thread Michael Liao via Phabricator via cfe-commits
hliao added a comment.

In D71227#1778136 , @tra wrote:

> I wonder if this patch will help with this case:
>
> https://godbolt.org/z/X4KdsV
>
>   __device__ float fn(int) { return threadIdx.x; };
>   __host__ float fn(float);
>  
>   float gvar1 = []()__device__ { return fn(1);} (); // This ends up calling 
> fn(int) on *host*
>  
>
>
> We seem to happily let host code call __device__ function from a lambda 
> function used as an initializer.


It's turned out that `Sema::CheckCUDACall` needs to consider global initializer 
as well. I will revise that part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227



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


[clang] 8e0c9e2 - [c++20] Delete defaulted comparison functions if they would invoke an

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T19:28:30-08:00
New Revision: 8e0c9e21bf5f3e7a427b07e3eaf3bc80d2c74cb6

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

LOG: [c++20] Delete defaulted comparison functions if they would invoke an
inaccessible comparison function.

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaAccess.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/CXX/class/class.compare/class.eq/p2.cpp
clang/test/CXX/class/class.compare/class.rel/p2.cpp
clang/test/CXX/class/class.compare/class.spaceship/p1.cpp
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a5f35996cfdc..4eec4b066ae5 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8220,6 +8220,10 @@ def note_defaulted_comparison_reference_member : Note<
 def note_defaulted_comparison_ambiguous : Note<
   "defaulted %0 is implicitly deleted because implied %select{|'==' |'<' }1"
   "comparison %select{|for member %3 |for base class %3 }2is ambiguous">;
+def note_defaulted_comparison_inaccessible : Note<
+  "defaulted %0 is implicitly deleted because it would invoke a "
+  "%select{private|protected}3 %4%select{ member of %6|"
+  " member of %6 to compare member %2| to compare base class %2}1">;
 def note_defaulted_comparison_calls_deleted : Note<
   "defaulted %0 is implicitly deleted because it would invoke a deleted "
   "comparison function%select{| for member %2| for base class %2}1">;

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 5a1ee507218c..53db210a0177 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6664,9 +6664,16 @@ class Sema final {
   void CheckLookupAccess(const LookupResult );
   bool IsSimplyAccessible(NamedDecl *Decl, CXXRecordDecl *NamingClass,
   QualType BaseType);
-  bool isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
-AccessSpecifier access,
-QualType objectType);
+  bool isMemberAccessibleForDeletion(CXXRecordDecl *NamingClass,
+ DeclAccessPair Found, QualType ObjectType,
+ SourceLocation Loc,
+ const PartialDiagnostic );
+  bool isMemberAccessibleForDeletion(CXXRecordDecl *NamingClass,
+ DeclAccessPair Found,
+ QualType ObjectType) {
+return isMemberAccessibleForDeletion(NamingClass, Found, ObjectType,
+ SourceLocation(), PDiag());
+  }
 
   void HandleDependentAccessCheck(const DependentDiagnostic ,
  const MultiLevelTemplateArgumentList );

diff  --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp
index 9dbb93322b7d..bd15b81cbed0 100644
--- a/clang/lib/Sema/SemaAccess.cpp
+++ b/clang/lib/Sema/SemaAccess.cpp
@@ -1560,21 +1560,24 @@ Sema::AccessResult 
Sema::CheckUnresolvedMemberAccess(UnresolvedMemberExpr *E,
   return CheckAccess(*this, E->getMemberLoc(), Entity);
 }
 
-/// Is the given special member function accessible for the purposes of
-/// deciding whether to define a special member function as deleted?
-bool Sema::isSpecialMemberAccessibleForDeletion(CXXMethodDecl *decl,
-AccessSpecifier access,
-QualType objectType) {
+/// Is the given member accessible for the purposes of deciding whether to
+/// define a special member function as deleted?
+bool Sema::isMemberAccessibleForDeletion(CXXRecordDecl *NamingClass,
+ DeclAccessPair Found,
+ QualType ObjectType,
+ SourceLocation Loc,
+ const PartialDiagnostic ) {
   // Fast path.
-  if (access == AS_public || !getLangOpts().AccessControl) return true;
+  if (Found.getAccess() == AS_public || !getLangOpts().AccessControl)
+return true;
 
-  AccessTarget entity(Context, AccessTarget::Member, decl->getParent(),
-  DeclAccessPair::make(decl, access), objectType);
+  AccessTarget Entity(Context, AccessTarget::Member, NamingClass, Found,
+  ObjectType);
 
   // Suppress diagnostics.
-  entity.setDiag(PDiag());
+  Entity.setDiag(Diag);
 
-  switch (CheckAccess(*this, SourceLocation(), entity)) {
+  switch 

[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

2019-12-10 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added a comment.

Should probably add a check for `__block` variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71301



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


[PATCH] D71322: [analyzer] CStringChecker: Fix overly eager assumption that memcmp arguments overlap.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

> Instead, i conservatively assume that they don't overlap, unless they're 
> already known to certainly overlap on the current execution path. This loses 
> a bit of coverage but the lost path is where they do actually overlap. This 
> path is in my opinion not only rare but also fairly useless, as it 
> immediately introduces an aliasing problem that we aren't quite ready to deal 
> with.

Wait, no, that's not what i'm doing. I'm simply forgetting about the assumption 
that the buffers don't overlap. So never mind, we're not losing any coverage 
and we're not stepping into an aliasing problem.


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

https://reviews.llvm.org/D71322



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


[PATCH] D71322: [analyzer] CStringChecker: Fix overly eager assumption that memcmp arguments overlap.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ updated this revision to Diff 233249.
NoQ added a comment.

Fair point. Yeah, i wonder what the real difference is :/ Added tests.


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

https://reviews.llvm.org/D71322

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/bstring.c
  clang/test/Analysis/string.c

Index: clang/test/Analysis/string.c
===
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -867,6 +867,12 @@
   fPtr();
 }
 
+int strcmp_null_argument(char *a) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return strcmp(a, b); // expected-warning{{Null pointer passed as 2nd argument to string comparison function}}
+}
+
 //===--===
 // strncmp()
 //===--===
@@ -976,6 +982,12 @@
 	clang_analyzer_eval(strncmp("ab\0zz", "ab\0yy", 4) == 0); // expected-warning{{TRUE}}
 }
 
+int strncmp_null_argument(char *a, size_t n) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return strncmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to string comparison function}}
+}
+
 //===--===
 // strcasecmp()
 //===--===
@@ -1067,6 +1079,12 @@
 	clang_analyzer_eval(strcasecmp("ab\0zz", "ab\0yy") == 0); // expected-warning{{TRUE}}
 }
 
+int strcasecmp_null_argument(char *a) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return strcasecmp(a, b); // expected-warning{{Null pointer passed as 2nd argument to string comparison function}}
+}
+
 //===--===
 // strncasecmp()
 //===--===
@@ -1176,6 +1194,12 @@
 	clang_analyzer_eval(strncasecmp("ab\0zz", "ab\0yy", 4) == 0); // expected-warning{{TRUE}}
 }
 
+int strncasecmp_null_argument(char *a, size_t n) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return strncasecmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to string comparison function}}
+}
+
 //===--===
 // strsep()
 //===--===
Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -462,6 +462,12 @@
  memcmp([x*y], a, n);
 }
 
+int memcmp8(char *a, size_t n) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return memcmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to memory comparison function}}
+}
+
 //===--===
 // bcopy()
 //===--===
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1313,9 +1313,9 @@
 ProgramStateRef StSameBuf, StNotSameBuf;
 std::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
 
-// If the two arguments might be the same buffer, we know the result is 0,
+// If the two arguments are the same buffer, we know the result is 0,
 // and we only need to check one size.
-if (StSameBuf) {
+if (StSameBuf && !StNotSameBuf) {
   state = StSameBuf;
   state = CheckBufferAccess(C, state, Size, Left);
   if (state) {
@@ -1323,20 +1323,19 @@
 svalBuilder.makeZeroVal(CE->getType()));
 C.addTransition(state);
   }
+  return;
 }
 
-// If the two arguments might be different buffers, we have to check the
-// size of both of them.
-if (StNotSameBuf) {
-  state = StNotSameBuf;
-  state = CheckBufferAccess(C, state, Size, Left, Right);
-  if (state) {
-// The return value is the comparison result, which we don't know.
-SVal CmpV = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx,
- C.blockCount());
-state = state->BindExpr(CE, LCtx, CmpV);
-C.addTransition(state);
-  }
+// If the two arguments might be different buffers, we have to check
+// the size of both of them.
+assert(StNotSameBuf);
+state = CheckBufferAccess(C, state, Size, Left, Right);
+if (state) {
+  // The return value is the comparison result, which we don't know.
+  SVal CmpV =
+  svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
+  state = 

[PATCH] D71322: [analyzer] CStringChecker: Fix overly eager assumption that memcmp arguments overlap.

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision.
Charusso added a comment.
This revision is now accepted and ready to land.

Could you write a test for `strcmp` please? I have found out the `evalMemcmp` 
behaves differently than `evalStrcmp`. However when the `memcmp` is going to be 
used like `strcmp`/`strncmp` the ideal would be to delegate the work to 
`evalStrcmp`. At the moment there is no connection between the two evaluation 
at all, which surprised me at first.

Other than that I really like the idea to remove overhead. Thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D71322



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


[PATCH] D71322: [analyzer] CStringChecker: Fix overly eager assumption that memcmp arguments overlap.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ created this revision.
NoQ added reviewers: dcoughlin, xazax.hun, a_sidorin, rnkovacs, Szelethus, 
baloghadamsoftware, Charusso.
Herald added subscribers: cfe-commits, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet.
Herald added a project: clang.
NoQ added a parent revision: D71321: [analyzer] CStringChecker: Warning text 
fixes..

While analyzing code

  memcmp(a, NULL, n);

where `a` has an unconstrained symbolic value, the analyzer is currently 
emitting a warning about the //first// argument being a null pointer, even 
though we'd rather have it warn about the //second// argument.

This happens because `CStringChecker` first checks that the two argument 
buffers are in fact the same buffer, in order to take the fast path. This boils 
down to assuming `a == NULL` to true. Then the subsequent check for null 
pointer argument "discovers" that `a` is null.

This could have been fixed by reordering the checks (first check null 
arguments, then check for overlap) but i went a bit further and entirely 
removed the state split for "same buffer vs. different buffers". This state 
split is not well-justified: we cannot deduce from the presence of `memcpy` in 
the code that the buffers may in fact overlap. Instead, i conservatively assume 
that they don't overlap, unless they're already known to certainly overlap on 
the current execution path. This loses a bit of coverage but the lost path is 
where they do actually overlap. This path is in my opinion not only rare but 
also fairly useless, as it immediately introduces an aliasing problem that we 
aren't quite ready to deal with.


Repository:
  rC Clang

https://reviews.llvm.org/D71322

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/bstring.c


Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -462,6 +462,12 @@
  memcmp([x*y], a, n);
 }
 
+int memcmp8(char *a, size_t n) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return memcmp(a, b, n); // expected-warning{{Null pointer passed as 2nd 
argument to memory comparison function}}
+}
+
 //===--===
 // bcopy()
 //===--===
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -1313,9 +1313,9 @@
 ProgramStateRef StSameBuf, StNotSameBuf;
 std::tie(StSameBuf, StNotSameBuf) = state->assume(SameBuf);
 
-// If the two arguments might be the same buffer, we know the result is 0,
+// If the two arguments are the same buffer, we know the result is 0,
 // and we only need to check one size.
-if (StSameBuf) {
+if (StSameBuf && !StNotSameBuf) {
   state = StSameBuf;
   state = CheckBufferAccess(C, state, Size, Left);
   if (state) {
@@ -1323,20 +1323,19 @@
 svalBuilder.makeZeroVal(CE->getType()));
 C.addTransition(state);
   }
+  return;
 }
 
-// If the two arguments might be different buffers, we have to check the
-// size of both of them.
-if (StNotSameBuf) {
-  state = StNotSameBuf;
-  state = CheckBufferAccess(C, state, Size, Left, Right);
-  if (state) {
-// The return value is the comparison result, which we don't know.
-SVal CmpV = svalBuilder.conjureSymbolVal(nullptr, CE, LCtx,
- C.blockCount());
-state = state->BindExpr(CE, LCtx, CmpV);
-C.addTransition(state);
-  }
+// If the two arguments might be different buffers, we have to check
+// the size of both of them.
+assert(StNotSameBuf);
+state = CheckBufferAccess(C, state, Size, Left, Right);
+if (state) {
+  // The return value is the comparison result, which we don't know.
+  SVal CmpV =
+  svalBuilder.conjureSymbolVal(nullptr, CE, LCtx, C.blockCount());
+  state = state->BindExpr(CE, LCtx, CmpV);
+  C.addTransition(state);
 }
   }
 }


Index: clang/test/Analysis/bstring.c
===
--- clang/test/Analysis/bstring.c
+++ clang/test/Analysis/bstring.c
@@ -462,6 +462,12 @@
  memcmp([x*y], a, n);
 }
 
+int memcmp8(char *a, size_t n) {
+  char *b = 0;
+  // Do not warn about the first argument!
+  return memcmp(a, b, n); // expected-warning{{Null pointer passed as 2nd argument to memory comparison function}}
+}
+
 //===--===
 // bcopy()
 //===--===
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

[PATCH] D71049: Remove implicit conversion that promotes half to other larger precision types for fp classification builtins

2019-12-10 Thread Jim Lin via Phabricator via cfe-commits
Jim added a comment.

@uweigand
This is fixed by 
https://reviews.llvm.org/rG9c3966379813c198129c57aa3ebecd68d6af1ebd
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71049



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


[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

2019-12-10 Thread Ronan Keryell via Phabricator via cfe-commits
keryell added inline comments.



Comment at: clang/lib/Sema/SemaSYCL.cpp:44
+
+class KernelBodyTransform : public TreeTransform {
+public:

Feel free to add more comments in this area up to line 103.



Comment at: clang/lib/Sema/SemaSYCL.cpp:417
+  Util::DeclContextDesc{clang::Decl::Kind::ClassTemplateSpecialization,
+"accessor"}};
+  return matchQualifiedTypeName(Ty, Scopes);

After more thought, perhaps the solution proposed by Victor Lomüller during the 
last SYCL upstreaming meeting about marking accessor classes with some 
attribute instead of detecting concrete type names is a better generic approach.
I am more convinced now by his argument allowing more experimenting with 
alternative runtimes, since it looks like it is the only place we detect type 
names. For example the kernels are marked with an attribute in the runtime 
instead of concretely detecting the `parallel_for()` functions and so on.



Comment at: clang/test/CodeGenSYCL/device-functions.cpp:2
+// RUN: %clang_cc1 -triple spir64 -fsycl-is-device -S -emit-llvm %s -o - | 
FileCheck %s
+
+template 

Missing description about the purpose of this test



Comment at: clang/test/SemaSYCL/fake-accessors.cpp:2
+// RUN: %clang_cc1 -I %S/Inputs -fsycl-is-device -ast-dump %s | FileCheck %s
+
+#include 

Missing description about the purpose of this test.
I am struggling about understanding what this test is for...
OK, after coming back later, I think I got it. I was confused by the fact that 
in the kernels you are using both true accessors (A, B & C) *and* some objects 
with names similar to SYCL accessor.
Is it possible to have some tests without true accessors?



Comment at: clang/test/SemaSYCL/mangle-kernel.cpp:3
+// RUN: %clang_cc1 -triple spir-unknown-unknown-unknown -I %S/Inputs -I 
%S/../Headers/Inputs/include/ -fsycl-is-device -ast-dump %s | FileCheck %s 
--check-prefix=CHECK-32
+#include 
+#include 

Missing description about the purpose of this test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71016



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


[PATCH] D71321: [analyzer] CStringChecker: Warning text fixes.

2019-12-10 Thread Csaba Dabis via Phabricator via cfe-commits
Charusso accepted this revision.
Charusso added a comment.
This revision is now accepted and ready to land.

Cool, thanks!


Repository:
  rC Clang

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

https://reviews.llvm.org/D71321



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


Re: [PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

2019-12-10 Thread Richard Smith via cfe-commits
On Tue, 10 Dec 2019 at 13:24, Kian Moniri via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> kianm added a comment.
>
> Hi, I am still seeing problems with this assertion. Could we please get a
> fix? I've posted the reduced test case and reproducible command on this
> Phabricator patch.
>

Thanks for the reminder, fixed in rG2e48be09b.


> Thanks.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D68896/new/
>
> https://reviews.llvm.org/D68896
>
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 2e48be0 - Fix mishandling of invalid-but-non-empty nested name specifiers in name

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T17:55:30-08:00
New Revision: 2e48be09b02e6d01b85d31704d768b6d0c751751

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

LOG: Fix mishandling of invalid-but-non-empty nested name specifiers in name
classification.

We were accidentally treating invalid scope specs as being empty,
resulting in our trying to form an ADL-only call with a qualified
callee, which tripped up an assert later on.

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/Parser/cxx-template-decl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 76bf7b034518..1cf87e45a299 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -867,6 +867,9 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
   LookupResult Result(*this, Name, NameLoc, LookupOrdinaryName);
   LookupParsedName(Result, S, , !CurMethod);
 
+  if (SS.isInvalid())
+return NameClassification::Error();
+
   // For unqualified lookup in a class template in MSVC mode, look into
   // dependent base classes where the primary class template is known.
   if (Result.empty() && SS.isEmpty() && getLangOpts().MSVCCompat) {
@@ -879,7 +882,7 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
   // synthesized instance variables), if we're in an Objective-C method.
   // FIXME: This lookup really, really needs to be folded in to the normal
   // unqualified lookup mechanism.
-  if (!SS.isSet() && CurMethod && !isResultTypeOrTemplate(Result, NextToken)) {
+  if (SS.isEmpty() && CurMethod && !isResultTypeOrTemplate(Result, NextToken)) 
{
 DeclResult Ivar = LookupIvarInObjCMethod(Result, S, Name);
 if (Ivar.isInvalid())
   return NameClassification::Error();
@@ -899,7 +902,7 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
   case LookupResult::NotFound:
 // If an unqualified-id is followed by a '(', then we have a function
 // call.
-if (!SS.isSet() && NextToken.is(tok::l_paren)) {
+if (SS.isEmpty() && NextToken.is(tok::l_paren)) {
   // In C++, this is an ADL-only call.
   // FIXME: Reference?
   if (getLangOpts().CPlusPlus)
@@ -921,7 +924,7 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
 return NameClassification::NonType(D);
 }
 
-if (getLangOpts().CPlusPlus2a && !SS.isSet() && NextToken.is(tok::less)) {
+if (getLangOpts().CPlusPlus2a && SS.isEmpty() && NextToken.is(tok::less)) {
   // In C++20 onwards, this could be an ADL-only call to a function
   // template, and we're required to assume that this is a template name.
   //
@@ -1063,7 +1066,7 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
hasAnyAcceptableTemplateNames(
Result, /*AllowFunctionTemplates=*/true,
/*AllowDependent=*/false,
-   /*AllowNonTemplateFunctions*/ !SS.isSet() &&
+   /*AllowNonTemplateFunctions*/ SS.isEmpty() &&
getLangOpts().CPlusPlus2a))) {
 // C++ [temp.names]p3:
 //   After name lookup (3.4) finds that a name is a template-name or that
@@ -1092,7 +1095,7 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
   IsFunctionTemplate = isa(TD);
   IsVarTemplate = isa(TD);
 
-  if (SS.isSet() && !SS.isInvalid())
+  if (SS.isNotEmpty())
 Template =
 Context.getQualifiedTemplateName(SS.getScopeRep(),
  /*TemplateKeyword=*/false, TD);

diff  --git a/clang/test/Parser/cxx-template-decl.cpp 
b/clang/test/Parser/cxx-template-decl.cpp
index 07316782a830..cb8a93fdecb1 100644
--- a/clang/test/Parser/cxx-template-decl.cpp
+++ b/clang/test/Parser/cxx-template-decl.cpp
@@ -261,3 +261,11 @@ namespace PR42071 {
   template struct C; // expected-error {{parameter declarator cannot 
be qualified}}
   template struct D; // expected-error {{default arguments 
can only be specified for parameters in a function declaration}}
 }
+
+namespace AnnotateAfterInvalidTemplateId {
+  template struct A { };
+  template struct A<0, J> { }; // expected-note {{J = 0}}
+  template struct A { }; // expected-note {{I = 0}}
+
+  void f() { A<0, 0>::f(); } // expected-error {{ambiguous partial 
specializations}}
+}



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


[PATCH] D66333: [analyzer] NonNullParamChecker and CStringChecker parameter number in checker message

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/CStringChecker.cpp:295-297
+  OS << "Null pointer argument in call to " << CurrentFunctionDescription
+ << ' ' << IdxOfArg << llvm::getOrdinalSuffix(IdxOfArg)
+ << " parameter";

NoQ wrote:
> It sounds like this code remained untested because tests don't match 
> end-of-line.
> 
> I noticed it because it produces warnings that don't look like valid English, 
> such as:
> 
> > "Null pointer argument in call to memory set function 1st parameter"
> 
> Patches are welcome :)
D71321.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66333



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


[PATCH] D71321: [analyzer] CStringChecker: Warning text fixes.

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

Fixes the problem mentioned in D66333#1726488 
. Additionally refers to `strcat` as 
"memory concatenation function" as opposed to "memory copy functions".

TODO: Add a flag to `VerifyDiagnosticConsumer` to match full messages by 
default.


Repository:
  rC Clang

https://reviews.llvm.org/D71321

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/bsd-string.c
  clang/test/Analysis/bstring.c
  clang/test/Analysis/cstring-ranges.c
  clang/test/Analysis/null-deref-path-notes.c
  clang/test/Analysis/null-deref-ps-region.c
  clang/test/Analysis/string.c

Index: clang/test/Analysis/string.c
===
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -97,7 +97,7 @@
 }
 
 size_t strlen_null() {
-  return strlen(0); // expected-warning{{Null pointer argument in call to string length function}}
+  return strlen(0); // expected-warning{{Null pointer passed as 1st argument to string length function}}
 }
 
 size_t strlen_fn() {
@@ -251,7 +251,7 @@
 }
 
 size_t strnlen_null() {
-  return strnlen(0, 3); // expected-warning{{Null pointer argument in call to string length function}}
+  return strnlen(0, 3); // expected-warning{{Null pointer passed as 1st argument to string length function}}
 }
 
 size_t strnlen_fn() {
@@ -322,11 +322,11 @@
 
 
 void strcpy_null_dst(char *x) {
-  strcpy(NULL, x); // expected-warning{{Null pointer argument in call to string copy function}}
+  strcpy(NULL, x); // expected-warning{{Null pointer passed as 1st argument to string copy function}}
 }
 
 void strcpy_null_src(char *x) {
-  strcpy(x, NULL); // expected-warning{{Null pointer argument in call to string copy function}}
+  strcpy(x, NULL); // expected-warning{{Null pointer passed as 2nd argument to string copy function}}
 }
 
 void strcpy_fn(char *x) {
@@ -424,15 +424,15 @@
 
 
 void strcat_null_dst(char *x) {
-  strcat(NULL, x); // expected-warning{{Null pointer argument in call to string copy function}}
+  strcat(NULL, x); // expected-warning{{Null pointer passed as 1st argument to string concatenation function}}
 }
 
 void strcat_null_src(char *x) {
-  strcat(x, NULL); // expected-warning{{Null pointer argument in call to string copy function}}
+  strcat(x, NULL); // expected-warning{{Null pointer passed as 2nd argument to string concatenation function}}
 }
 
 void strcat_fn(char *x) {
-  strcat(x, (char*)_fn); // expected-warning{{Argument to string copy function is the address of the function 'strcat_fn', which is not a null-terminated string}}
+  strcat(x, (char*)_fn); // expected-warning{{Argument to string concatenation function is the address of the function 'strcat_fn', which is not a null-terminated string}}
 }
 
 void strcat_effects(char *y) {
@@ -523,11 +523,11 @@
 
 
 void strncpy_null_dst(char *x) {
-  strncpy(NULL, x, 5); // expected-warning{{Null pointer argument in call to string copy function}}
+  strncpy(NULL, x, 5); // expected-warning{{Null pointer passed as 1st argument to string copy function}}
 }
 
 void strncpy_null_src(char *x) {
-  strncpy(x, NULL, 5); // expected-warning{{Null pointer argument in call to string copy function}}
+  strncpy(x, NULL, 5); // expected-warning{{Null pointer passed as 2nd argument to string copy function}}
 }
 
 void strncpy_fn(char *x) {
@@ -631,15 +631,15 @@
 
 
 void strncat_null_dst(char *x) {
-  strncat(NULL, x, 4); // expected-warning{{Null pointer argument in call to string copy function}}
+  strncat(NULL, x, 4); // expected-warning{{Null pointer passed as 1st argument to string concatenation function}}
 }
 
 void strncat_null_src(char *x) {
-  strncat(x, NULL, 4); // expected-warning{{Null pointer argument in call to string copy function}}
+  strncat(x, NULL, 4); // expected-warning{{Null pointer passed as 2nd argument to string concatenation function}}
 }
 
 void strncat_fn(char *x) {
-  strncat(x, (char*)_fn, 4); // expected-warning{{Argument to string copy function is the address of the function 'strncat_fn', which is not a null-terminated string}}
+  strncat(x, (char*)_fn, 4); // expected-warning{{Argument to string concatenation function is the address of the function 'strncat_fn', which is not a null-terminated string}}
 }
 
 void strncat_effects(char *y) {
@@ -812,13 +812,13 @@
 void strcmp_null_0() {
   char *x = NULL;
   char *y = "123";
-  strcmp(x, y); // expected-warning{{Null pointer argument in call to string comparison function}}
+  strcmp(x, y); // expected-warning{{Null pointer passed as 1st argument to string comparison function}}
 }
 
 void strcmp_null_1() {
   char *x = "123";
   char *y = NULL;
-  strcmp(x, y); // expected-warning{{Null pointer 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

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

Build result: FAILURE - Could not check out parent git hash 
"9a3d576b08c13533597182498ba5e739924f892f". It was not found in the repository. 
Did you configure the "Parent Revision" in Phabricator properly? Trying to 
apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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


[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

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

Build result: pass - 60692 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268



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


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

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

In D71241#1778717 , @jdoerfert wrote:

> >> There is no evidence that this is more complicated. By all measures, this 
> >> is less complicated (see also below). It is also actually doing the right 
> >> thing when it comes to code emission. Take https://godbolt.org/z/sJiP3B 
> >> for example. The calls are wrong and the definition of base is missing.
> > 
> > How did you measure it? I have a completely different opinion. Also, tried 
> > to reproduce the problem locally, could not reproduce. It seems to me, the 
> > output of the test misses several important things. You can check it 
> > yourself. `tgt_target_teams()` call uses `@.offload_maptypes` global var 
> > but it is not defined.
>
> Here is the link with the globals not hidden: https://godbolt.org/z/5etB5S
>  The base function is called both times but should not be called at all. What 
> is your local output and why does it differ?


On the host `base` is an alias for the `hst` function. On the device `base` has 
the body of `dev` function because NVPTX does nit support function aliases (10+ 
suppprts it, but LLVM does not support it yet). Try to change the bodies of dev 
and hst and you will see.

I tried to keep original function names to improve debugging and make users 
less wonder why instead of `base` something else is called.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[libunwind] 57fc8ec - Reland "Enable `-funwind-tables` flag when building libunwind"

2019-12-10 Thread Sergej Jaskiewicz via cfe-commits

Author: Sergej Jaskiewicz
Date: 2019-12-11T04:27:04+03:00
New Revision: 57fc8ecdbaa7fbb1a13907ca3f7a3cb9f9459d27

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

LOG: Reland "Enable `-funwind-tables` flag when building libunwind"

Summary:
Relands https://reviews.llvm.org/D70815.

The original commit set `CMAKE_TRY_COMPILE_TARGET_TYPE` to
`STATIC_LIBRARY` globally in libunwind/CMakeLists.txt, which effectively
disabled the linking step in CMake checks.

This broke some builds (see 938c70b86c7d2165f8c28d5700e9c1ac1263307e).

Here we set CMAKE_TRY_COMPILE_TARGET_TYPE to
STATIC_LIBRARY only when checking for presence of the `-funwind-tables`
flag, and then set it back to the original value so it doesn't affect
other checks.

Reviewers: mstorsjo, jfb

Subscribers: mgorny, christof, libcxx-commits

Tags: #libc

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

Added: 


Modified: 
libunwind/CMakeLists.txt
libunwind/cmake/config-ix.cmake
libunwind/test/CMakeLists.txt
libunwind/test/libunwind/test/config.py
libunwind/test/lit.site.cfg.in
libunwind/test/signal_frame.pass.cpp

Removed: 




diff  --git a/libunwind/CMakeLists.txt b/libunwind/CMakeLists.txt
index 08095d1333a5..6dd0a15405b5 100644
--- a/libunwind/CMakeLists.txt
+++ b/libunwind/CMakeLists.txt
@@ -291,7 +291,29 @@ endif()
 add_cxx_compile_flags_if_supported(-fstrict-aliasing)
 add_cxx_compile_flags_if_supported(-EHsc)
 
+# Don't run the linker in this CMake check.
+#
+# The reason why this was added is that when building libunwind for
+# ARM Linux, we need to pass the -funwind-tables flag in order for it to
+# work properly with ARM EHABI.
+#
+# However, when performing CMake checks, adding this flag causes the check
+# to produce a false negative, because the compiler generates calls
+# to __aeabi_unwind_cpp_pr0, which is defined in libunwind itself,
+# which isn't built yet, so the linker complains about undefined symbols.
+#
+# This leads to libunwind not being built with this flag, which makes
+# libunwind quite useless in this setup.
+set(_previous_CMAKE_TRY_COMPILE_TARGET_TYPE ${CMAKE_TRY_COMPILE_TARGET_TYPE})
+set(CMAKE_TRY_COMPILE_TARGET_TYPE STATIC_LIBRARY)
 add_compile_flags_if_supported(-funwind-tables)
+set(CMAKE_TRY_COMPILE_TARGET_TYPE ${_previous_CMAKE_TRY_COMPILE_TARGET_TYPE})
+
+if (LIBUNWIND_USES_ARM_EHABI AND NOT LIBUNWIND_SUPPORTS_FUNWIND_TABLES_FLAG)
+  message(SEND_ERROR "The -funwind-tables flag must be supported "
+ "because this target uses ARM Exception Handling ABI")
+endif()
+
 add_cxx_compile_flags_if_supported(-fno-exceptions)
 add_cxx_compile_flags_if_supported(-fno-rtti)
 

diff  --git a/libunwind/cmake/config-ix.cmake b/libunwind/cmake/config-ix.cmake
index 02d2f13f2e28..0d833e996ca1 100644
--- a/libunwind/cmake/config-ix.cmake
+++ b/libunwind/cmake/config-ix.cmake
@@ -2,6 +2,7 @@ include(CMakePushCheckState)
 include(CheckCCompilerFlag)
 include(CheckCXXCompilerFlag)
 include(CheckLibraryExists)
+include(CheckSymbolExists)
 include(CheckCSourceCompiles)
 
 check_library_exists(c fopen "" LIBUNWIND_HAS_C_LIB)
@@ -73,3 +74,13 @@ check_cxx_compiler_flag(-nostdinc++ 
LIBUNWIND_HAS_NOSTDINCXX_FLAG)
 # Check libraries
 check_library_exists(dl dladdr "" LIBUNWIND_HAS_DL_LIB)
 check_library_exists(pthread pthread_once "" LIBUNWIND_HAS_PTHREAD_LIB)
+
+# Check symbols
+check_symbol_exists(__arm__ "" LIBUNWIND_TARGET_ARM)
+check_symbol_exists(__USING_SJLJ_EXCEPTIONS__ "" 
LIBUNWIND_USES_SJLJ_EXCEPTIONS)
+check_symbol_exists(__ARM_DWARF_EH__ "" LIBUNWIND_USES_DWARF_EH)
+
+if(LIBUNWIND_TARGET_ARM AND NOT LIBUNWIND_USES_SJLJ_EXCEPTIONS AND NOT 
LIBUNWIND_USES_DWARF_EH)
+  # This condition is copied from __libunwind_config.h
+  set(LIBUNWIND_USES_ARM_EHABI ON)
+endif()

diff  --git a/libunwind/test/CMakeLists.txt b/libunwind/test/CMakeLists.txt
index d902e3e82941..40d4acd4e8c2 100644
--- a/libunwind/test/CMakeLists.txt
+++ b/libunwind/test/CMakeLists.txt
@@ -16,6 +16,7 @@ pythonize_bool(LIBCXX_ENABLE_SHARED)
 pythonize_bool(LIBUNWIND_ENABLE_SHARED)
 pythonize_bool(LIBUNWIND_ENABLE_THREADS)
 pythonize_bool(LIBUNWIND_ENABLE_EXCEPTIONS)
+pythonize_bool(LIBUNWIND_USES_ARM_EHABI)
 pythonize_bool(LIBUNWIND_USE_COMPILER_RT)
 pythonize_bool(LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY)
 set(LIBUNWIND_TARGET_INFO "libcxx.test.target_info.LocalTI" CACHE STRING

diff  --git a/libunwind/test/libunwind/test/config.py 
b/libunwind/test/libunwind/test/config.py
index 05e3f3cc21f3..41ca3f9b4a44 100644
--- a/libunwind/test/libunwind/test/config.py
+++ b/libunwind/test/libunwind/test/config.py
@@ -37,6 +37,8 @@ def configure_features(self):
 super(Configuration, self).configure_features()
 if not self.get_lit_bool('enable_exceptions', True):
 

[clang] bc24014 - [c++20] Implement P1185R2 (as modified by P2002R0).

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T17:24:27-08:00
New Revision: bc24014b9765a454cb5214f4871451a41ffb7d29

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

LOG: [c++20] Implement P1185R2 (as modified by P2002R0).

For each defaulted operator<=> in a class that doesn't explicitly
declare any operator==, also inject a matching implicit defaulted
operator==.

Added: 
clang/test/CXX/class/class.compare/class.compare.default/p4.cpp

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/Template.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/CodeGenCXX/cxx2a-three-way-comparison.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index aeeff2b9a76e..a5f35996cfdc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3840,6 +3840,7 @@ def select_ovl_candidate_kind : TextSubstitution<
 "constructor (the implicit move constructor)|"
 "function (the implicit copy assignment operator)|"
 "function (the implicit move assignment operator)|"
+"function (the implicit 'operator==' for this 'operator<=>)'|"
 "inherited constructor}0%select{| template| %2}1">;
 
 def note_ovl_candidate : Note<
@@ -8207,7 +8208,8 @@ def warn_defaulted_comparison_deleted : Warning<
   "explicitly defaulted %sub{select_defaulted_comparison_kind}0 is implicitly "
   "deleted">, InGroup;
 def err_non_first_default_compare_deletes : Error<
-  "defaulting this %sub{select_defaulted_comparison_kind}0 "
+  "defaulting %select{this %sub{select_defaulted_comparison_kind}1|"
+  "the corresponding implicit 'operator==' for this defaulted 'operator<=>'}0 "
   "would delete it after its first declaration">;
 def note_defaulted_comparison_union : Note<
   "defaulted %0 is implicitly deleted because "
@@ -8237,14 +8239,19 @@ def note_defaulted_comparison_cannot_deduce : Note<
 def note_defaulted_comparison_cannot_deduce_callee : Note<
   "selected 'operator<=>' for %select{|member|base class}0 %1 declared here">;
 def err_incorrect_defaulted_comparison_constexpr : Error<
-  "defaulted definition of %sub{select_defaulted_comparison_kind}0 "
-  "cannot be declared %select{constexpr|consteval}1 because it invokes "
-  "a non-constexpr comparison function">;
+  "defaulted definition of %select{%sub{select_defaulted_comparison_kind}1|"
+  "three-way comparison operator}0 "
+  "cannot be declared %select{constexpr|consteval}2 because "
+  "%select{it|the corresponding implicit 'operator=='}0 "
+  "invokes a non-constexpr comparison function">;
 def note_defaulted_comparison_not_constexpr : Note<
   "non-constexpr comparison function would be used to compare "
   "%select{|member %1|base class %1}0">;
 def note_defaulted_comparison_not_constexpr_here : Note<
   "non-constexpr comparison function declared here">;
+def note_in_declaration_of_implicit_equality_comparison : Note<
+  "while declaring the corresponding implicit 'operator==' "
+  "for this defaulted 'operator<=>'">;
 
 def ext_implicit_exception_spec_mismatch : ExtWarn<
   "function previously declared with an %select{explicit|implicit}0 exception "

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1cdaab3dc28c..5a1ee507218c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -6524,6 +6524,8 @@ class Sema final {
 
   bool CheckExplicitlyDefaultedComparison(Scope *S, FunctionDecl *MD,
   DefaultedComparisonKind DCK);
+  void DeclareImplicitEqualityComparison(CXXRecordDecl *RD,
+ FunctionDecl *Spaceship);
   void DefineDefaultedComparison(SourceLocation Loc, FunctionDecl *FD,
  DefaultedComparisonKind DCK);
 
@@ -7838,6 +7840,10 @@ class Sema final {
   /// We are declaring an implicit special member function.
   DeclaringSpecialMember,
 
+  /// We are declaring an implicit 'operator==' for a defaulted
+  /// 'operator<=>'.
+  DeclaringImplicitEqualityComparison,
+
   /// We are defining a synthesized function (such as a defaulted special
   /// member).
   DefiningSynthesizedFunction,
@@ -8468,6 +8474,11 @@ class Sema final {
   Decl *SubstDecl(Decl *D, DeclContext *Owner,
   const MultiLevelTemplateArgumentList );
 
+  /// Substitute the name and return type of a defaulted 'operator<=>' to form
+  /// an implicit 'operator=='.
+  

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

2019-12-10 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert updated this revision to Diff 233238.
jdoerfert added a comment.

Fix math problem


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant.c
  clang/test/OpenMP/begin_declare_variant_codegen.cpp
  clang/test/OpenMP/declare_variant_ast_print.cpp
  clang/test/OpenMP/math_codegen.cpp
  clang/test/OpenMP/math_fp_macro.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -91,6 +91,8 @@
 __OMP_DIRECTIVE_EXT(master_taskloop_simd, "master taskloop simd")
 __OMP_DIRECTIVE_EXT(parallel_master_taskloop_simd,
 "parallel master taskloop simd")
+__OMP_DIRECTIVE_EXT(begin_declare_variant, "begin declare variant")
+__OMP_DIRECTIVE_EXT(end_declare_variant, "end declare variant")
 
 // Has to be the last because Clang implicitly expects it to be.
 __OMP_DIRECTIVE(unknown)
Index: clang/test/OpenMP/math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_fp_macro.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/math_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_codegen.cpp
@@ -0,0 +1,15 @@
+#include 
+
+void math(short s, int i, float f, double d) {
+  sin(s);
+  sin(i);
+  sin(f);
+  sin(d);
+}
+
+void foo(short s, int i, float f, double d, long double ld) {
+  //sin(ld);
+  math(s, i, f, d);
+#pragma omp target
+  { math(s, i, f, d); }
+}
Index: clang/test/OpenMP/declare_variant_ast_print.cpp
===
--- clang/test/OpenMP/declare_variant_ast_print.cpp
+++ clang/test/OpenMP/declare_variant_ast_print.cpp
@@ -40,7 +40,6 @@
 #pragma omp declare variant(foofoo ) match(user = {condition()})
 #pragma omp declare variant(foofoo ) match(implementation={vendor(llvm)},device={kind(cpu)})
 #pragma omp declare variant(foofoo ) match(implementation={vendor(unknown)})
-// TODO: Handle template instantiation
 #pragma omp declare variant(foofoo ) match(implementation={vendor(score(C+5): ibm, xxx, ibm)},device={kind(cpu,host)})
 template 
 T barbar();
Index: clang/test/OpenMP/begin_declare_variant_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_codegen.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+int bar(void) {
+  return 0;
+}
+
+template 
+T baz(void) { return 0; }
+
+#pragma omp begin declare variant match(device={kind(cpu)})
+int foo(void) {
+  return 1;
+}
+int bar(void) {
+  return 1;
+}
+template 
+T baz(void) { return 1; }
+
+template 
+T biz(void) { return 1; }
+
+template 
+T buz(void) { return 3; }
+
+template <>
+char buz(void) { return 1; }
+
+template 
+T bez(void) { return 3; }
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(device={kind(gpu)})
+int foo(void) {
+  return 2;
+}
+int bar(void) {
+  return 2;
+}
+#pragma omp end declare variant
+
+
+#pragma omp begin declare variant match(device={kind(fpga)})
+
+This text is never parsed!
+
+#pragma omp end declare variant
+
+int foo(void) {
+  return 0;
+}
+
+template 
+T biz(void) { return 0; }
+
+template <>
+char buz(void) { return 0; }
+
+template <>
+long bez(void) { return 0; }
+
+#pragma omp begin 

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

2019-12-10 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268



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


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

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

>> There is no evidence that this is more complicated. By all measures, this is 
>> less complicated (see also below). It is also actually doing the right thing 
>> when it comes to code emission. Take https://godbolt.org/z/sJiP3B for 
>> example. The calls are wrong and the definition of base is missing.
> 
> How did you measure it? I have a completely different opinion. Also, tried to 
> reproduce the problem locally, could not reproduce. It seems to me, the 
> output of the test misses several important things. You can check it 
> yourself. `tgt_target_teams()` call uses `@.offload_maptypes` global var but 
> it is not defined.

Here is the link with the globals not hidden: https://godbolt.org/z/5etB5S
The base function is called both times but should not be called at all. What is 
your local output and why does it differ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

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

Build result: FAILURE - Could not check out parent git hash 
"9a3d576b08c13533597182498ba5e739924f892f". It was not found in the repository. 
Did you configure the "Parent Revision" in Phabricator properly? Trying to 
apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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


[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

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

Build result: FAILURE - Could not check out parent git hash 
"9a3d576b08c13533597182498ba5e739924f892f". It was not found in the repository. 
Did you configure the "Parent Revision" in Phabricator properly? Trying to 
apply the patch to the master branch instead...

ERROR: arc patch failed with error code 1. Check build log for details.
Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179



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


[clang] d5e66f0 - NFC: Get rid of an unused parameter to CGObjCMac::EmitSelectorAddr.

2019-12-10 Thread Erik Pilkington via cfe-commits

Author: Erik Pilkington
Date: 2019-12-10T16:54:48-08:00
New Revision: d5e66f0e060da7f175cedf4f545fb4e13df7707e

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

LOG: NFC: Get rid of an unused parameter to CGObjCMac::EmitSelectorAddr.

Added: 


Modified: 
clang/lib/CodeGen/CGObjCMac.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjCMac.cpp b/clang/lib/CodeGen/CGObjCMac.cpp
index e6a3e26499b2..a38e5a49f0f4 100644
--- a/clang/lib/CodeGen/CGObjCMac.cpp
+++ b/clang/lib/CodeGen/CGObjCMac.cpp
@@ -1315,7 +1315,7 @@ class CGObjCMac : public CGObjCCommonMac {
   /// EmitSelector - Return a Value*, of type ObjCTypes.SelectorPtrTy,
   /// for the given selector.
   llvm::Value *EmitSelector(CodeGenFunction , Selector Sel);
-  Address EmitSelectorAddr(CodeGenFunction , Selector Sel);
+  Address EmitSelectorAddr(Selector Sel);
 
 public:
   CGObjCMac(CodeGen::CodeGenModule );
@@ -1543,7 +1543,7 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
   /// EmitSelector - Return a Value*, of type ObjCTypes.SelectorPtrTy,
   /// for the given selector.
   llvm::Value *EmitSelector(CodeGenFunction , Selector Sel);
-  Address EmitSelectorAddr(CodeGenFunction , Selector Sel);
+  Address EmitSelectorAddr(Selector Sel);
 
   /// GetInterfaceEHType - Get the cached ehtype for the given Objective-C
   /// interface. The return value has type EHTypePtrTy.
@@ -1635,7 +1635,7 @@ class CGObjCNonFragileABIMac : public CGObjCCommonMac {
   llvm::Value *GetSelector(CodeGenFunction , Selector Sel) override
 { return EmitSelector(CGF, Sel); }
   Address GetAddrOfSelector(CodeGenFunction , Selector Sel) override
-{ return EmitSelectorAddr(CGF, Sel); }
+{ return EmitSelectorAddr(Sel); }
 
   /// The NeXT/Apple runtimes do not support typed selectors; just emit an
   /// untyped one.
@@ -1903,7 +1903,7 @@ llvm::Value *CGObjCMac::GetSelector(CodeGenFunction , 
Selector Sel) {
   return EmitSelector(CGF, Sel);
 }
 Address CGObjCMac::GetAddrOfSelector(CodeGenFunction , Selector Sel) {
-  return EmitSelectorAddr(CGF, Sel);
+  return EmitSelectorAddr(Sel);
 }
 llvm::Value *CGObjCMac::GetSelector(CodeGenFunction , const ObjCMethodDecl
 *Method) {
@@ -5263,11 +5263,11 @@ llvm::Value 
*CGObjCMac::EmitNSAutoreleasePoolClassRef(CodeGenFunction ) {
 }
 
 llvm::Value *CGObjCMac::EmitSelector(CodeGenFunction , Selector Sel) {
-  return CGF.Builder.CreateLoad(EmitSelectorAddr(CGF, Sel));
+  return CGF.Builder.CreateLoad(EmitSelectorAddr(Sel));
 }
 
-Address CGObjCMac::EmitSelectorAddr(CodeGenFunction , Selector Sel) {
-  CharUnits Align = CGF.getPointerAlign();
+Address CGObjCMac::EmitSelectorAddr(Selector Sel) {
+  CharUnits Align = CGM.getPointerAlign();
 
   llvm::GlobalVariable * = SelectorReferences[Sel];
   if (!Entry) {
@@ -7607,7 +7607,7 @@ 
CGObjCNonFragileABIMac::GenerateMessageSendSuper(CodeGen::CodeGenFunction ,
 
 llvm::Value *CGObjCNonFragileABIMac::EmitSelector(CodeGenFunction ,
   Selector Sel) {
-  Address Addr = EmitSelectorAddr(CGF, Sel);
+  Address Addr = EmitSelectorAddr(Sel);
 
   llvm::LoadInst* LI = CGF.Builder.CreateLoad(Addr);
   LI->setMetadata(CGM.getModule().getMDKindID("invariant.load"),
@@ -7615,11 +7615,9 @@ llvm::Value 
*CGObjCNonFragileABIMac::EmitSelector(CodeGenFunction ,
   return LI;
 }
 
-Address CGObjCNonFragileABIMac::EmitSelectorAddr(CodeGenFunction ,
- Selector Sel) {
+Address CGObjCNonFragileABIMac::EmitSelectorAddr(Selector Sel) {
   llvm::GlobalVariable * = SelectorReferences[Sel];
-
-  CharUnits Align = CGF.getPointerAlign();
+  CharUnits Align = CGM.getPointerAlign();
   if (!Entry) {
 llvm::Constant *Casted =
   llvm::ConstantExpr::getBitCast(GetMethodVarName(Sel),



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 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.

Thanks!! Nothing controversial remains here, right? :)




Comment at: clang/include/clang/StaticAnalyzer/Core/CheckerManager.h:91
+  /// that the analyzer cannot follow during a conservative call.
+  PSK_EscapeOnConservativeCall,
+

All three escape-on-calls are on conservatively evaluated calls only, so this 
name isn't very descriptive.

Maybe `PSK_EscapeOutParameters`?



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp:2736
 SVal Val, const LocationContext *LCtx) 
{
-
-  // Cases (1) and (2).
-  const MemRegion *MR = Loc.getAsRegion();
-  if (!MR || !MR->hasStackStorage())
-return escapeValue(State, Val, PSK_EscapeOnBind);
-
-  // Case (3).
-  if (const auto *VR = dyn_cast(MR->getBaseRegion()))
-if (VR->hasStackParametersStorage() && VR->getStackFrame()->inTopFrame())
-  if (const auto *RD = VR->getValueType()->getAsCXXRecordDecl())
-if (!RD->hasTrivialDestructor())
-  return escapeValue(State, Val, PSK_EscapeOnBind);
-
-  // Case (4): in order to test that, generate a new state with the binding
-  // added. If it is the same state, then it escapes (since the store cannot
-  // represent the binding).
-  // Do this only if we know that the store is not supposed to generate the
-  // same state.
-  SVal StoredVal = State->getSVal(MR);
-  if (StoredVal != Val)
-if (State == (State->bindLoc(loc::MemRegionVal(MR), Val, LCtx)))
-  return escapeValue(State, Val, PSK_EscapeOnBind);
-
-  return State;
+  std::pair LocAndVal(Loc, Val);
+  return processPointerEscapedOnBind(State, LocAndVal, LCtx, PSK_EscapeOnBind,

Dunno, `make_pair`?


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

https://reviews.llvm.org/D71224



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


Buildbot numbers for the week of 12/1/2019 - 12/7/2019

2019-12-10 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the last week of 12/1/2019 - 12/7/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 lldb-x64-windows-ninja   | 99:22:13
 clang-cmake-x86_64-avx2-linux| 19:32:10
 clang-cmake-x86_64-sde-avx512-linux  | 18:36:26
 clang-cmake-aarch64-global-isel  | 18:07:23
 clang-cmake-armv7-full   | 17:17:58
 clang-cmake-armv7-selfhost   | 14:44:40
 clang-cmake-armv7-global-isel| 14:00:09
 clang-cmake-aarch64-quick| 13:15:51
 clang-ppc64le-linux-multistage   | 13:10:38
 llvm-clang-win-x-aarch64 | 11:30:39
 clang-cmake-thumbv7-full-sh  | 11:26:30
 llvm-clang-win-x-armv7l  | 11:20:33
 clang-cmake-armv7-selfhost-neon  | 10:41:57
 clang-ppc64le-linux  | 10:00:00
 clang-cmake-aarch64-full | 09:47:12
 clang-s390x-linux-multistage | 09:37:44
 clang-cmake-armv7-quick  | 09:35:51
 clang-cmake-aarch64-lld  | 09:31:18
 clang-cmake-armv8-lld| 09:22:21
 clang-ppc64be-linux  | 08:37:06
 clang-ppc64be-linux-multistage   | 08:28:03
 clang-cmake-armv7-lnt| 08:23:48
 clang-ppc64be-linux-lnt  | 08:16:39
 clang-s390x-linux-lnt| 07:38:21
 clang-s390x-linux| 07:37:19
 clang-x86_64-linux-abi-test  | 07:18:26
 reverse-iteration| 07:15:54
 clang-with-thin-lto-ubuntu   | 07:08:23
 clang-with-lto-ubuntu| 06:52:53
 clang-native-arm-lnt-perf| 06:42:22
 clang-ppc64le-linux-lnt  | 06:39:20
 llvm-clang-x86_64-expensive-checks-win   | 06:38:31
 llvm-clang-x86_64-win-fast   | 06:38:27
 sanitizer-x86_64-linux-fast  | 06:04:40
 sanitizer-x86_64-linux-bootstrap | 05:33:19
 ppc64le-lld-multistage-test  | 05:06:20
 clang-hexagon-elf| 05:04:13
 clang-x64-windows-msvc   | 05:01:45
 aosp-O3-polly-before-vectorizer-unprofitable | 05:01:31
 sanitizer-x86_64-linux-bootstrap-ubsan   | 04:56:16
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 04:51:57
 openmp-gcc-x86_64-linux-debian   | 04:46:36
 openmp-clang-x86_64-linux-debian | 04:43:15
 lld-x86_64-win7  | 03:53:22
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 03:47:33
 sanitizer-x86_64-linux-bootstrap-msan| 03:41:26
 sanitizer-ppc64be-linux  | 03:39:07
 polly-arm-linux  | 03:31:12
 sanitizer-x86_64-linux-autoconf  | 02:04:23
 sanitizer-x86_64-linux   | 02:02:08
 clang-atom-d525-fedora-rel   | 01:26:34
 lld-x86_64-darwin13  | 01:26:18
 lld-x86_64-ubuntu-fast   | 01:17:55
 llvm-clang-x86_64-expensive-checks-ubuntu| 01:17:53
 clang-x86_64-debian-new-pass-manager-fast| 01:05:55
 clang-x86_64-debian-fast | 01:05:05
 llvm-clang-x86_64-expensive-checks-debian| 01:04:36
 sanitizer-x86_64-linux-android   | 00:59:11
 sanitizer-x86_64-linux-fuzzer| 00:43:50
 lld-perf-testsuite   | 00:40:12
 sanitizer-windows| 00:33:31
 clang-aarch64-linux-build-cache  | 00:32:13
 clang-armv7-linux-build-cache| 00:25:18
 lldb-x86_64-debian   | 00:20:47
(64 rows)


"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green):
   buildername   | builds | changes
| status_change_ratio
-++-+
 

Buildbot numbers for the week of 11/24/2019 - 11/30/2019

2019-12-10 Thread Galina Kistanova via cfe-commits
Hello everyone,

Below are some buildbot numbers for the week of 11/24/2019 - 11/30/2019.

Please see the same data in attached csv files:

The longest time each builder was red during the week;
"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green);
Count of commits by project;
Number of completed builds, failed builds and average build time for
successful builds per active builder;
Average waiting time for a revision to get build result per active builder
(response time).

Thanks

Galina


The longest time each builder was red during the week:
   buildername| was_red
--+-
 sanitizer-x86_64-linux-bootstrap-ubsan   | 64:45:56
 sanitizer-x86_64-linux-fast  | 63:57:59
 ppc64le-lld-multistage-test  | 36:57:35
 aosp-O3-polly-before-vectorizer-unprofitable | 31:44:46
 clang-ppc64be-linux-multistage   | 19:29:57
 sanitizer-x86_64-linux   | 17:23:51
 clang-cmake-armv7-full   | 13:18:30
 llvm-clang-x86_64-expensive-checks-debian| 13:12:12
 clang-cmake-armv7-global-isel| 09:38:38
 clang-cmake-armv8-lld| 07:44:44
 sanitizer-ppc64be-linux  | 05:32:20
 sanitizer-x86_64-linux-bootstrap | 05:29:09
 lld-x86_64-ubuntu-fast   | 05:29:07
 clang-x64-windows-msvc   | 05:04:29
 llvm-clang-win-x-aarch64 | 04:43:46
 llvm-clang-win-x-armv7l  | 04:43:23
 clang-cmake-aarch64-lld  | 04:41:48
 clang-cmake-aarch64-global-isel  | 04:25:33
 clang-x86_64-debian-fast | 04:16:02
 llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast | 04:09:03
 llvm-clang-x86_64-expensive-checks-win   | 04:00:09
 clang-cmake-aarch64-quick| 03:58:51
 clang-with-lto-ubuntu| 03:54:16
 clang-cmake-aarch64-full | 03:37:45
 lld-x86_64-win7  | 03:06:32
 clang-with-thin-lto-ubuntu   | 02:34:34
 clang-ppc64le-linux  | 02:11:32
 clang-cmake-x86_64-avx2-linux| 01:52:19
 openmp-gcc-x86_64-linux-debian   | 01:45:48
 clang-ppc64le-linux-lnt  | 01:42:21
 clang-cmake-armv7-quick  | 01:37:17
 sanitizer-x86_64-linux-autoconf  | 01:37:11
 clang-cmake-x86_64-sde-avx512-linux  | 01:22:25
 lldb-x64-windows-ninja   | 01:14:37
 clang-ppc64be-linux-lnt  | 01:14:06
 sanitizer-x86_64-linux-bootstrap-msan| 01:07:48
 sanitizer-x86_64-linux-fuzzer| 01:01:46
 clang-cuda-build | 01:01:10
 lld-x86_64-darwin13  | 00:59:46
 clang-ppc64be-linux  | 00:43:15
 llvm-clang-x86_64-expensive-checks-ubuntu| 00:42:29
 clang-s390x-linux-lnt| 00:38:15
 llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast   | 00:34:02
 clang-s390x-linux| 00:30:31
 lld-perf-testsuite   | 00:30:27
 clang-armv7-linux-build-cache| 00:24:59
 sanitizer-windows| 00:22:19
 llvm-clang-x86_64-win-fast   | 00:19:59
 openmp-clang-x86_64-linux-debian | 00:14:49
 clang-x86_64-linux-abi-test  | 00:14:06
(50 rows)


"Status change ratio" by active builder (percent of builds that changed the
builder status from greed to red or from red to green):
   buildername   | builds | changes
| status_change_ratio
-++-+
 sanitizer-x86_64-linux  |100 |  22
|22.0
 clang-cmake-aarch64-global-isel | 73 |  14
|19.2
 clang-cmake-aarch64-lld | 33 |   6
|18.2
 clang-cmake-aarch64-quick   | 73 |  11
|15.1
 clang-cuda-build|147 |  20
|13.6
 openmp-gcc-x86_64-linux-debian  | 34 |   4
|11.8
 ppc64le-lld-multistage-test |129 |  15
|11.6
 lld-x86_64-darwin13 |125 |  14
|11.2
 clang-cmake-armv8-lld   | 36 |   4

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233235.
xazax.hun added a comment.

- Rebasing after reverting the pre-escaping.


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

https://reviews.llvm.org/D71224

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/expr-inspection.c
  clang/test/Analysis/pointer-escape-on-conservative-calls.c

Index: clang/test/Analysis/pointer-escape-on-conservative-calls.c
===
--- /dev/null
+++ clang/test/Analysis/pointer-escape-on-conservative-calls.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+int *getMem();
+
+int main() {
+f(getMem());
+return 0;
+}
+
+// CHECK: PostCall (f)
+// CHECK-NEXT: PointerEscape
Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -50,5 +50,5 @@
 
 void test_field_dumps(struct S s, struct S *p) {
   clang_analyzer_dump_pointer(); // expected-warning{{}}
-  clang_analyzer_dump_pointer(>x); // expected-warning{{{reg_$0}.x}}
+  clang_analyzer_dump_pointer(>x); // expected-warning{{{reg_$1}.x}}
 }
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -37,6 +37,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/AST/Decl.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
@@ -592,9 +593,45 @@
   for (auto I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Escaping symbols conjured during invalidating the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  SmallVector, 8> Escaped;
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+ProgramStateRef State = I->getState();
+Escaped.clear();
+{
+  unsigned Arg = -1;
+  for (const ParmVarDecl *PVD : Call.parameters()) {
+++Arg;
+QualType ParamTy = PVD->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+QualType Pointee = ParamTy->getPointeeType();
+if (Pointee.isConstQualified() || Pointee->isVoidType())
+  continue;
+if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+  Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee));
+  }
+}
+
+State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(),
+PSK_EscapeOnConservativeCall, );
+
+if (State == I->getState())
+  Dst.insert(I);
+else
+  B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

2019-12-10 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert updated this revision to Diff 233232.
jdoerfert added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Consistent overload based solution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant.c
  clang/test/OpenMP/begin_declare_variant_codegen.cpp
  clang/test/OpenMP/declare_variant_ast_print.cpp
  clang/test/OpenMP/math_codegen.cpp
  clang/test/OpenMP/math_fp_macro.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -91,6 +91,8 @@
 __OMP_DIRECTIVE_EXT(master_taskloop_simd, "master taskloop simd")
 __OMP_DIRECTIVE_EXT(parallel_master_taskloop_simd,
 "parallel master taskloop simd")
+__OMP_DIRECTIVE_EXT(begin_declare_variant, "begin declare variant")
+__OMP_DIRECTIVE_EXT(end_declare_variant, "end declare variant")
 
 // Has to be the last because Clang implicitly expects it to be.
 __OMP_DIRECTIVE(unknown)
Index: clang/test/OpenMP/math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_fp_macro.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/math_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_codegen.cpp
@@ -0,0 +1,15 @@
+#include 
+
+void math(short s, int i, float f, double d) {
+  sin(s);
+  sin(i);
+  sin(f);
+  sin(d);
+}
+
+void foo(short s, int i, float f, double d, long double ld) {
+  //sin(ld);
+  math(s, i, f, d);
+#pragma omp target
+  { math(s, i, f, d); }
+}
Index: clang/test/OpenMP/declare_variant_ast_print.cpp
===
--- clang/test/OpenMP/declare_variant_ast_print.cpp
+++ clang/test/OpenMP/declare_variant_ast_print.cpp
@@ -40,7 +40,6 @@
 #pragma omp declare variant(foofoo ) match(user = {condition()})
 #pragma omp declare variant(foofoo ) match(implementation={vendor(llvm)},device={kind(cpu)})
 #pragma omp declare variant(foofoo ) match(implementation={vendor(unknown)})
-// TODO: Handle template instantiation
 #pragma omp declare variant(foofoo ) match(implementation={vendor(score(C+5): ibm, xxx, ibm)},device={kind(cpu,host)})
 template 
 T barbar();
Index: clang/test/OpenMP/begin_declare_variant_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_codegen.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+int bar(void) {
+  return 0;
+}
+
+template 
+T baz(void) { return 0; }
+
+#pragma omp begin declare variant match(device={kind(cpu)})
+int foo(void) {
+  return 1;
+}
+int bar(void) {
+  return 1;
+}
+template 
+T baz(void) { return 1; }
+
+template 
+T biz(void) { return 1; }
+
+template 
+T buz(void) { return 3; }
+
+template <>
+char buz(void) { return 1; }
+
+template 
+T bez(void) { return 3; }
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(device={kind(gpu)})
+int foo(void) {
+  return 2;
+}
+int bar(void) {
+  return 2;
+}
+#pragma omp end declare variant
+
+
+#pragma omp begin declare variant match(device={kind(fpga)})
+
+This text is never parsed!
+
+#pragma omp end declare variant
+
+int foo(void) {
+  return 0;
+}
+
+template 
+T biz(void) { return 0; }
+
+template <>
+char 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

2019-12-10 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert updated this revision to Diff 233234.
jdoerfert added a comment.

Diff against TOT


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71179

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/StmtOpenMP.h
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/StmtOpenMP.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Headers/__clang_cuda_cmath.h
  clang/lib/Headers/__clang_cuda_device_functions.h
  clang/lib/Headers/__clang_cuda_math_forward_declares.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math.h
  clang/lib/Headers/openmp_wrappers/__clang_openmp_math_declares.h
  clang/lib/Headers/openmp_wrappers/cmath
  clang/lib/Headers/openmp_wrappers/math.h
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant.c
  clang/test/OpenMP/begin_declare_variant_codegen.cpp
  clang/test/OpenMP/declare_variant_ast_print.cpp
  clang/test/OpenMP/math_codegen.cpp
  clang/test/OpenMP/math_fp_macro.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPKinds.def

Index: llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
===
--- llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
+++ llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
@@ -91,6 +91,8 @@
 __OMP_DIRECTIVE_EXT(master_taskloop_simd, "master taskloop simd")
 __OMP_DIRECTIVE_EXT(parallel_master_taskloop_simd,
 "parallel master taskloop simd")
+__OMP_DIRECTIVE_EXT(begin_declare_variant, "begin declare variant")
+__OMP_DIRECTIVE_EXT(end_declare_variant, "end declare variant")
 
 // Has to be the last because Clang implicitly expects it to be.
 __OMP_DIRECTIVE(unknown)
Index: clang/test/OpenMP/math_fp_macro.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_fp_macro.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+#include 
+
+int main() {
+  double a(0);
+  return (std::fpclassify(a) != FP_ZERO);
+}
Index: clang/test/OpenMP/math_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/math_codegen.cpp
@@ -0,0 +1,15 @@
+#include 
+
+void math(short s, int i, float f, double d) {
+  sin(s);
+  sin(i);
+  sin(f);
+  sin(d);
+}
+
+void foo(short s, int i, float f, double d, long double ld) {
+  //sin(ld);
+  math(s, i, f, d);
+#pragma omp target
+  { math(s, i, f, d); }
+}
Index: clang/test/OpenMP/declare_variant_ast_print.cpp
===
--- clang/test/OpenMP/declare_variant_ast_print.cpp
+++ clang/test/OpenMP/declare_variant_ast_print.cpp
@@ -40,7 +40,6 @@
 #pragma omp declare variant(foofoo ) match(user = {condition()})
 #pragma omp declare variant(foofoo ) match(implementation={vendor(llvm)},device={kind(cpu)})
 #pragma omp declare variant(foofoo ) match(implementation={vendor(unknown)})
-// TODO: Handle template instantiation
 #pragma omp declare variant(foofoo ) match(implementation={vendor(score(C+5): ibm, xxx, ibm)},device={kind(cpu,host)})
 template 
 T barbar();
Index: clang/test/OpenMP/begin_declare_variant_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/begin_declare_variant_codegen.cpp
@@ -0,0 +1,134 @@
+// RUN: %clang_cc1 -verify -fopenmp -x c++ -emit-llvm %s -triple %itanium_abi_triple -fexceptions -fcxx-exceptions -o - | FileCheck %s
+// expected-no-diagnostics
+
+int bar(void) {
+  return 0;
+}
+
+template 
+T baz(void) { return 0; }
+
+#pragma omp begin declare variant match(device={kind(cpu)})
+int foo(void) {
+  return 1;
+}
+int bar(void) {
+  return 1;
+}
+template 
+T baz(void) { return 1; }
+
+template 
+T biz(void) { return 1; }
+
+template 
+T buz(void) { return 3; }
+
+template <>
+char buz(void) { return 1; }
+
+template 
+T bez(void) { return 3; }
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(device={kind(gpu)})
+int foo(void) {
+  return 2;
+}
+int bar(void) {
+  return 2;
+}
+#pragma omp end declare variant
+
+
+#pragma omp begin declare variant match(device={kind(fpga)})
+
+This text is never parsed!
+
+#pragma omp end declare variant
+
+int foo(void) {
+  return 0;
+}
+
+template 
+T biz(void) { return 0; }
+
+template <>
+char buz(void) { return 0; }
+
+template <>
+long bez(void) { return 0; }
+
+#pragma omp begin 

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

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

There can be another one issue with this solution with inline assembly. I’m not 
completely sure about it, will try to investigate it tomorrow. I suggest to 
discuss this solution with Richard Smith (or John McCall). If he/they are ok 
with this transformation of the AST, we can switch to this scheme.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

2019-12-10 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese updated this revision to Diff 233233.
Bigcheese added a comment.

Refactored `FullDependencies::getAdditionalCommandLine` and 
`ModuleDeps::getFullCommandLine` to share code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
@@ -129,6 +130,11 @@
 llvm::cl::init(ScanningOutputFormat::Make),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt FullCommandLine(
+"full-command-line",
+llvm::cl::desc("Include the full command lines to use to build modules"),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -189,9 +195,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string ,
-   llvm::Expected ,
-   SharedStream , SharedStream ) {
+static bool
+handleMakeDependencyToolResult(const std::string ,
+   llvm::Expected ,
+   SharedStream , SharedStream ) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [, ](llvm::StringError ) {
@@ -206,6 +213,184 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> ) {
+  std::vector Strings;
+  for (auto & : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+static llvm::json::Array toJSONSorted(std::vector V) {
+  std::sort(V.begin(), V.end(),
+[](const ClangModuleDep , const ClangModuleDep ) {
+  return std::tie(A.ModuleName, A.ContextHash) <
+ std::tie(B.ModuleName, B.ContextHash);
+});
+
+  llvm::json::Array Ret;
+  for (const ClangModuleDep  : V)
+Ret.push_back(llvm::json::Object(
+{{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+  return Ret;
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+ size_t InputIndex) {
+const FullDependencies  = FDR.FullDeps;
+
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.FileDeps);
+ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+
+std::unique_lock ul(Lock);
+for (const ModuleDeps  : FDR.DiscoveredModules) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+if (FullCommandLine)
+  ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
+  [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
+  [&](ClangModuleDep CMD) -> const ModuleDeps & {
+return lookupModuleDeps(CMD);
+  });
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream ) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto & : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair , const ContextModulePair ) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps , const InputDeps ) {
+return A.FileName < B.FileName;
+  });
+
+using namespace llvm::json;
+
+Array OutModules;
+for (auto & : ModuleNames) {
+  auto  = Modules[ModName];
+  

[PATCH] D71317: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

2019-12-10 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi created this revision.
plotfi added reviewers: compnerd, cishida.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Built libdispatch with clang interface stubs. Ran into some block related 
issues. Basically VarDecl symbols can leak out because I wasn't checking the 
case where a VarDecl is contained inside a BlockDecl (versus a method or 
function).

This patch checks that a VarDecl is not a child decl of a BlockDecl.
This patch also does something very similar for c++ lambdas as well.

  


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71317

Files:
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/test/InterfaceStubs/blocks.c
  clang/test/InterfaceStubs/lambda.cpp


Index: clang/test/InterfaceStubs/lambda.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -emit-interface-stubs -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT:   "f" : { Type: Object, Size: 1 }
+// CHECK-NEXT: ...
+auto f = [](void* data) { int i; };
Index: clang/test/InterfaceStubs/blocks.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/blocks.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-interface-stubs -fblocks -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT: ...
+static void (^f)(void*) = ^(void* data) { int i; };
Index: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -52,6 +52,14 @@
   if (!isVisible(ND))
 return true;
 
+  if (const VarDecl *VD = dyn_cast(ND))
+if (const auto *Parent = VD->getParentFunctionOrMethod()) {
+  if (const auto *BD = dyn_cast_or_null(Parent))
+return true;
+  if (const auto *MD = dyn_cast_or_null(Parent))
+return true;
+}
+
   if (const VarDecl *VD = dyn_cast(ND))
 if ((VD->getStorageClass() == StorageClass::SC_Extern) ||
 (VD->getStorageClass() == StorageClass::SC_Static &&


Index: clang/test/InterfaceStubs/lambda.cpp
===
--- /dev/null
+++ clang/test/InterfaceStubs/lambda.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -emit-interface-stubs -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT:   "f" : { Type: Object, Size: 1 }
+// CHECK-NEXT: ...
+auto f = [](void* data) { int i; };
Index: clang/test/InterfaceStubs/blocks.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/blocks.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-interface-stubs -fblocks -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT: ...
+static void (^f)(void*) = ^(void* data) { int i; };
Index: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -52,6 +52,14 @@
   if (!isVisible(ND))
 return true;
 
+  if (const VarDecl *VD = dyn_cast(ND))
+if (const auto *Parent = VD->getParentFunctionOrMethod()) {
+  if (const auto *BD = dyn_cast_or_null(Parent))
+return true;
+  if (const auto *MD = dyn_cast_or_null(Parent))
+return true;
+}
+
   if (const VarDecl *VD = dyn_cast(ND))
 if ((VD->getStorageClass() == StorageClass::SC_Extern) ||
 (VD->getStorageClass() == StorageClass::SC_Static &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 8434fbb - Revert "[analyzer] Keep track of escaped locals"

2019-12-10 Thread Gabor Horvath via cfe-commits

Author: Gabor Horvath
Date: 2019-12-10T16:42:03-08:00
New Revision: 8434fbbee62e382376a39787785909bd55ae1696

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

LOG: Revert "[analyzer] Keep track of escaped locals"

It was a step in the right direction but it is not clear how can this
fit into the checker API at this point. The pre-escape happens in the
analyzer core and the checker has no control over it. If the checker
is not interestd in a pre-escape it would need to do additional work
on each escape to check if the escaped symbol is originated from an
"uninteresting" pre-escaped memory region. In order to keep the
checker API simple we abandoned this solution for now.

We will reland this once we have a better answer for what to do on the
checker side.

This reverts commit f3a28202ef58551db15818f8f51afd21e0f3e231.

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
clang/lib/StaticAnalyzer/Core/ProgramState.cpp
clang/test/Analysis/symbol-escape.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index c85a66db3457..2d0967616ff2 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -627,9 +627,6 @@ class ExprEngine : public SubEngine {
const CallEvent *Call,
RegionAndSymbolInvalidationTraits ) 
override;
 
-  ProgramStateRef processLocalRegionEscape(ProgramStateRef State,
-   const MemRegion *R) const override;
-
   /// A simple wrapper when you only need to notify checkers of pointer-escape
   /// of a single value.
   ProgramStateRef escapeValue(ProgramStateRef State, SVal V,

diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
index b6b4a86acbb2..bdd12a3ffe33 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
@@ -27,7 +27,7 @@
 
 namespace llvm {
 class APSInt;
-} // namespace llvm
+}
 
 namespace clang {
 class ASTContext;
@@ -872,8 +872,8 @@ class ScanReachableSymbols {
   bool scan(const SymExpr *sym);
 };
 
-} // namespace ento
+} // end ento namespace
 
-} // namespace clang
+} // end clang namespace
 
 #endif

diff  --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
index 5866be2b2e7c..7789b431c0a6 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
@@ -149,16 +149,14 @@ class SubEngine {
   }
 
   virtual ProgramStateRef
-  processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val,
-  const LocationContext *LCtx) = 0;
-
-  virtual ProgramStateRef notifyCheckersOfPointerEscape(
-  ProgramStateRef State, const InvalidatedSymbols *Invalidated,
-  ArrayRef ExplicitRegions, const CallEvent *Call,
-  RegionAndSymbolInvalidationTraits ) = 0;
+  processPointerEscapedOnBind(ProgramStateRef State, SVal Loc, SVal Val, const 
LocationContext *LCtx) = 0;
 
   virtual ProgramStateRef
-  processLocalRegionEscape(ProgramStateRef State, const MemRegion *R) const = 
0;
+  notifyCheckersOfPointerEscape(ProgramStateRef State,
+   const InvalidatedSymbols *Invalidated,
+   ArrayRef ExplicitRegions,
+   const CallEvent *Call,
+   RegionAndSymbolInvalidationTraits ) = 0;
 
   /// printJson - Called by ProgramStateManager to print checker-specific data.
   virtual void printJson(raw_ostream , ProgramStateRef State,

diff  --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp 
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index b6f6481c369d..efbc20f09250 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -193,8 +193,6 @@ typedef llvm::ImmutableMap
 REGISTER_TRAIT_WITH_PROGRAMSTATE(ObjectsUnderConstruction,
  ObjectsUnderConstructionMap)
 
-REGISTER_SET_WITH_PROGRAMSTATE(EscapedLocals, const MemRegion *)
-
 
//===--===//
 // Engine construction and 

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-12-10 Thread Michele Scandale via Phabricator via cfe-commits
michele.scandale added a comment.

In D62731#1775046 , @mibintc wrote:

> In D62731#1773854 , 
> @michele.scandale wrote:
>
> > I've noticed you removed the change for `CompilerInvocation.cpp` about the 
> > initialization of the codegen option `NoTrappingMath`. Was that an accident?
>
>
> I checked the old and new version of the patch and it seems like 
> initialization of NoTrappingMath is unchanged, the definition of the option 
> has it default to 0, and CompilerInvocation.cpp sets it like this in 
> ParseLangArgs, was it something else you were looking at?
>
>   Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);


In the driver code you don't always pass `-fno-trapping-math`, therefore when 
when the compiler setup the CodeGen options in `ParseCodeGenArgs` you will end 
up executing `Opts.NoTrappingMath = Args.hasArg(OPT_fno_trapping_math);` hence 
you will have that `Opt.NoTrappingMath = false`. This is inconsistent with the 
state of the compiler driver where no-trapping-math is enabled default.

If you want to keep the default of the CC1 different than the default of the 
compiler driver, that's fine to me, but in that case the compiler driver needs 
to pass `-fno-trapping-math` to the CC1.
If we want the same new default, then the logic in `ParseCodeGenArgs` must be 
updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62731



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


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

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

In D71241#1778564 , @hfinkel wrote:

> In D71241#1778134 , @ABataev wrote:
>
> >
>
>
> ...
>
> >> 
> >> 
> >>> Also, check how -ast-print works with your solution. It returns different 
> >>> result than expected because you're transform the code too early. It is 
> >>> incorrect behavior.
> >> 
> >> This is debatable. AST print does not print the input but the AST, thus 
> >> what is correct wrt. OpenMP declare variant is nowhere defined but by us.
> >>  Arguably, having it print the actually called function and not the base 
> >> function is preferable. Thus, the new way is actually more informative.
> > 
> > You're completely wrong here! We shall keep the original AST. This is used 
> > in several tools and you significantly modify the user code. I consider it 
> > a real issue here.
>
> Alexey, again, this kind of comment is not appropriate. We're all experienced 
> developers here, and we all understand the importance of tooling support in 
> Clang. We also serve developers who write tools using AST matchers and other 
> Clang analysis facilities. Having the resolved callee represented in the AST 
> for what looks like a static call from the base-language perspective makes a 
> lot of sense from a tooling perspective. When performing static analysis on 
> the code, forcing a tool to understand how OpenMP variant selectors work in 
> order to perform inter-procedural static analysis is suboptimal in nearly all 
> cases. It is also true that we might want the base callee represented in some 
> way, but as that callee is never actually called, and one of the variants is 
> always called at that call site, it is important that IPA propagate 
> information into and out of the correct callee in order to produce the 
> correct results. If we currently represent the base callee as the callee that 
> will appear in the call graph, that's a bug: Clang's static analyzer will 
> produce incorrect results.
>
> If you know of specific tools that indeed depend on the current behavior to 
> produce correct results, please provide us with details on what they're doing 
> so that we understand the use cases. Regardless, we should prioritize 
> correct-by-default functioning of AST-based call graphs and their associated 
> static analysis.




1. This is significant issue that you're modifiy the original user code.
2. It may have some troubles with serialization/deserialization probably. Plus, 
I just don't understand why it is good to rewrite the code for the user but to 
keep the original code is bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


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

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

In D71241#1778564 , @hfinkel wrote:

> In D71241#1778134 , @ABataev wrote:
>
> >
>
>
> ...
>
> >> 
> >> 
> >>> Also, check how -ast-print works with your solution. It returns different 
> >>> result than expected because you're transform the code too early. It is 
> >>> incorrect behavior.
> >> 
> >> This is debatable. AST print does not print the input but the AST, thus 
> >> what is correct wrt. OpenMP declare variant is nowhere defined but by us.
> >>  Arguably, having it print the actually called function and not the base 
> >> function is preferable. Thus, the new way is actually more informative.
> > 
> > You're completely wrong here! We shall keep the original AST. This is used 
> > in several tools and you significantly modify the user code. I consider it 
> > a real issue here.
>
> Alexey, again, this kind of comment is not appropriate. We're all experienced 
> developers here, and we all understand the importance of tooling support in 
> Clang. We also serve developers who write tools using AST matchers and other 
> Clang analysis facilities. Having the resolved callee represented in the AST 
> for what looks like a static call from the base-language perspective makes a 
> lot of sense from a tooling perspective. When performing static analysis on 
> the code, forcing a tool to understand how OpenMP variant selectors work in 
> order to perform inter-procedural static analysis is suboptimal in nearly all 
> cases. It is also true that we might want the base callee represented in some 
> way, but as that callee is never actually called, and one of the variants is 
> always called at that call site, it is important that IPA propagate 
> information into and out of the correct callee in order to produce the 
> correct results. If we currently represent the base callee as the callee that 
> will appear in the call graph, that's a bug: Clang's static analyzer will 
> produce incorrect results.
>
> If you know of specific tools that indeed depend on the current behavior to 
> produce correct results, please provide us with details on what they're doing 
> so that we understand the use cases. Regardless, we should prioritize 
> correct-by-default functioning of AST-based call graphs and their associated 
> static analysis.


What's not appropriate here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D70805: [analyzer] SValHasDescendant: Determine whether the SVal has an SVal descendant

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValHasDescendant.h:55
+  bool VisitSymbolRegionValue(const SymbolRegionValue *S) {
+return Visit(S->getRegion());
+  }

Arithmetic is indeed easy, but for example this part requires a much deeper 
justification.


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

https://reviews.llvm.org/D70805



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


[PATCH] D70469: [attributes] [analyzer] Add handle related attributes

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

What about `__attribute__((acquire_handle("fuchsia")))`?


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

https://reviews.llvm.org/D70469



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


[PATCH] D70320: [Analyzer] [NFC] Iterator Checkers - Separate iterator modeling and the actual checkers

2019-12-10 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.

> I plan some refactoring but this first patch is meant to be a single 
> separation of code.

Sounds great!




Comment at: clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp:1308-1318
+ProgramStateRef removeIteratorPosition(ProgramStateRef State, const SVal ) 
{
   if (auto Reg = Val.getAsRegion()) {
 Reg = Reg->getMostDerivedObjectRegion();
-return State->get(Reg);
+return State->remove(Reg);
   } else if (const auto Sym = Val.getAsSymbol()) {
-return State->get(Sym);
+return State->remove(Sym);
   } else if (const auto LCVal = Val.getAs()) {

baloghadamsoftware wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > Charusso wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > Maybe move this function to `Iterator.cpp` as well, and then move 
> > > > > > the definitions for iterator maps from `Iterator.h` to 
> > > > > > `Iterator.cpp`, which will allow you to use the usual 
> > > > > > `REGISTER_MAP_WITH_PROGRAMSTATE` macros, and additionally guarantee 
> > > > > > that all access to the maps goes through the accessor methods that 
> > > > > > you provide?
> > > > > Hmm, I was trying hard to use these macros but failed so I reverted 
> > > > > to the manual solution. I will retry now.
> > > > Here is a how-to: https://reviews.llvm.org/D69726
> > > > 
> > > > You need to add the fully qualified names to the register macro because 
> > > > of the global scope. I hope it helps.
> > > OK, I checked it now. If we want to put the maps into `Iterator.cpp` then 
> > > we also have to move a couple of functions there which are only used by 
> > > the modeling part: the internals of `checkDeadSymbols()` and 
> > > `checkLiveSymbols()` must go there, although no other checker should use 
> > > them. Also `processIteratorPositions()` iterates over these maps, thus it 
> > > must also go there. Should I do it? Generally I like the current solution 
> > > better, only functions used by multiple checker classes are in the 
> > > library.
> > > 
> > > On the other hand I do not see why I should move `assumeNoOverflow()` to 
> > > `Iterator.cpp`? This function is only used by the modeling part, and does 
> > > not refer to the maps. You mean that we should ensure this constraint 
> > > whenever setting the position for any iterator? This would mean that we 
> > > should decompose every symblic expression and (re)assume this range on 
> > > the symbolic part. Or we should replace `setPosition()` by at least two 
> > > different functions (e.g. `setAdvancedPosition()` and 
> > > `setConjuredPosition()`) but this means a total reqriting of the modeling.
> > > 
> > > I plan some refactoring but this first patch is meant to be a single 
> > > separation of code.
> > @Charusso It does not help because the macros put the maps into a local 
> > namespace. If it is in a header, it means separate maps for every checker 
> > and the modeling which of course does not work.
> @NoQ What is the decision? Should I move everything to the lib such as the 
> body of `checkDeadSymbols()` and `checkLiveSymbols()` which I am reluctant 
> because they belong only to the modeling, or leave it as it is now?
I think all the functions for manipulating the state trait (including dead 
symbol cleanup and escapes) should live in the modeling file. Like, it doesn't 
mean that `checkDeadSymbols` itself should necessarily live there, but in any 
case the file should provide a nice cleanup method that can be invoked from the 
real `checkDeadSymbols`.

This way you can encapsulate all the implementation details in the cpp file and 
only interact with it with fancy accessors.

I'm perfectly ok with delaying this work ^.^


Repository:
  rC Clang

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

https://reviews.llvm.org/D70320



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


[PATCH] D71314: Emit a warning if a variable is uninitialized in indirect ASM goto destination.

2019-12-10 Thread Bill Wendling via Phabricator via cfe-commits
void created this revision.
void added reviewers: jyknight, nickdesaulniers, hfinkel.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71314

Files:
  clang/lib/Analysis/UninitializedValues.cpp
  clang/test/Analysis/uninit-asm-goto.cpp


Index: clang/test/Analysis/uninit-asm-goto.cpp
===
--- clang/test/Analysis/uninit-asm-goto.cpp
+++ clang/test/Analysis/uninit-asm-goto.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -Wuninitialized -verify %s
-// expected-no-diagnostics
 
+// test1: Expect no diagnostics
 int test1(int x) {
 int y;
 asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : err);
@@ -8,3 +8,31 @@
   err:
 return -1;
 }
+
+int test2(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its 
declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  if (x < 42)
+asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) 
: "r"(x) :: indirect_1, indirect_2);
+  else
+asm volatile goto("testl %0, %1; testl %2, %3; jne %l5" : "+S"(x), "+D"(y) 
: "r"(x), "r"(y) :: indirect_1, indirect_2);
+  return x + y;
+indirect_1:
+  return -42;
+indirect_2:
+  return y; // expected-note {{uninitialized use occurs here}}
+}
+
+int foo(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its 
declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  asm goto("xorl %1, %0; jmp %l2" : "="(y) : "r"(x) : : fail);
+normal:
+  y += x;
+  return y;
+  if (x) {
+fail:
+return y; // expected-note {{uninitialized use occurs here}}
+  }
+  return 0;
+}
Index: clang/lib/Analysis/UninitializedValues.cpp
===
--- clang/lib/Analysis/UninitializedValues.cpp
+++ clang/lib/Analysis/UninitializedValues.cpp
@@ -637,6 +637,34 @@
   continue;
 }
 
+if (AtPredExit == MayUninitialized) {
+  // If the predecessor's terminator is an "asm goto" that initializes
+  // the variable, then it won't be counted as "initialized" on the
+  // non-fallthrough paths.
+  CFGTerminator terminator = Pred->getTerminator();
+  if (GCCAsmStmt *as = 
dyn_cast_or_null(terminator.getStmt()))
+if (as->isAsmGoto()) {
+  bool uninitedAfterDecl = false;
+  for (const auto  : as->outputs()) {
+if (vd != findVar(o).getDecl())
+  continue;
+for (const auto  : as->labels()) {
+  const LabelStmt *ls = label->getLabel()->getStmt();
+  if (ls == B->Label) {
+uninitedAfterDecl = true;
+break;
+  }
+}
+if (uninitedAfterDecl)
+  break;
+  }
+  if (uninitedAfterDecl) {
+Use.setUninitAfterDecl();
+continue;
+  }
+}
+}
+
 unsigned  = SuccsVisited[Pred->getBlockID()];
 if (!SV) {
   // When visiting the first successor of a block, mark all NULL
@@ -829,7 +857,8 @@
 
   for (const auto  : as->outputs())
 if (const VarDecl *VD = findVar(o).getDecl())
-  vals[VD] = Initialized;
+  if (vals[VD] != Initialized)
+vals[VD] = MayUninitialized;
 }
 
 void TransferFunctions::VisitObjCMessageExpr(ObjCMessageExpr *ME) {


Index: clang/test/Analysis/uninit-asm-goto.cpp
===
--- clang/test/Analysis/uninit-asm-goto.cpp
+++ clang/test/Analysis/uninit-asm-goto.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -std=c++11 -Wuninitialized -verify %s
-// expected-no-diagnostics
 
+// test1: Expect no diagnostics
 int test1(int x) {
 int y;
 asm goto("# %0 %1 %2" : "=r"(y) : "r"(x) : : err);
@@ -8,3 +8,31 @@
   err:
 return -1;
 }
+
+int test2(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  if (x < 42)
+asm volatile goto("testl %0, %0; testl %1, %2; jne %l3" : "+S"(x), "+D"(y) : "r"(x) :: indirect_1, indirect_2);
+  else
+asm volatile goto("testl %0, %1; testl %2, %3; jne %l5" : "+S"(x), "+D"(y) : "r"(x), "r"(y) :: indirect_1, indirect_2);
+  return x + y;
+indirect_1:
+  return -42;
+indirect_2:
+  return y; // expected-note {{uninitialized use occurs here}}
+}
+
+int foo(int x) {
+  int y; // expected-warning {{variable 'y' is used uninitialized whenever its declaration is reached}} \
+ // expected-note {{initialize the variable}}
+  asm goto("xorl %1, %0; jmp %l2" : "="(y) : "r"(x) : : fail);
+normal:
+  y += x;
+  return y;
+  if (x) {
+fail:
+return y; // expected-note {{uninitialized use occurs here}}
+  }
+  return 

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

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

In D71241#1778134 , @ABataev wrote:

>


...

>> 
>> 
>>> Also, check how -ast-print works with your solution. It returns different 
>>> result than expected because you're transform the code too early. It is 
>>> incorrect behavior.
>> 
>> This is debatable. AST print does not print the input but the AST, thus what 
>> is correct wrt. OpenMP declare variant is nowhere defined but by us.
>>  Arguably, having it print the actually called function and not the base 
>> function is preferable. Thus, the new way is actually more informative.
> 
> You're completely wrong here! We shall keep the original AST. This is used in 
> several tools and you significantly modify the user code. I consider it a 
> real issue here.

Alexey, again, this kind of comment is not appropriate. We're all experienced 
developers here, and we all understand the importance of tooling support in 
Clang. We also serve developers who write tools using AST matchers and other 
Clang analysis facilities. Having the resolved callee represented in the AST 
for what looks like a static call from the base-language perspective makes a 
lot of sense from a tooling perspective. When performing static analysis on the 
code, forcing a tool to understand how OpenMP variant selectors work in order 
to perform inter-procedural static analysis is suboptimal in nearly all cases. 
It is also true that we might want the base callee represented in some way, but 
as that callee is never actually called, and one of the variants is always 
called at that call site, it is important that IPA propagate information into 
and out of the correct callee in order to produce the correct results. If we 
currently represent the base callee as the callee that will appear in the call 
graph, that's a bug: Clang's static analyzer will produce incorrect results.

If you know of specific tools that indeed depend on the current behavior to 
produce correct results, please provide us with details on what they're doing 
so that we understand the use cases. Regardless, we should prioritize 
correct-by-default functioning of AST-based call graphs and their associated 
static analysis.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


[PATCH] D70912: [Analyzer] Iterator Modeling: Print Container Data and Iterator Positions when printing the Program State

2019-12-10 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.

Fantastic, thanks!!


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

https://reviews.llvm.org/D70912



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


[PATCH] D68213: [LTO] Support for embedding bitcode section during LTO

2019-12-10 Thread Steven Wu via Phabricator via cfe-commits
steven_wu added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:335
+   .Case("bitcode", EmbedBitcodeKind::Embed_Bitcode)
+   .Case("marker", EmbedBitcodeKind::Embed_Marker)
+   .Default(~0U);

tejohnson wrote:
> zapster wrote:
> > tejohnson wrote:
> > > Does this value make any sense since CmdArgs is always empty below?
> > You are right. Currently, this option value is not utterly useful, but I 
> > kept it for compatibility with clangs `-fembed-bitcode`. It seems the 
> > marker section is needed (even if empty), otherwise certain tools will 
> > complain (notably the MacOS linker). Ideally, we would have something 
> > sensible to write into the section but I am not sure about reasonable 
> > values. I was not able to find any information about consumers of these 
> > commands. Pointers in that regard would be highly appreciated.
> > 
> > Anyhow, I lean toward keeping the options for compatibility with clang and 
> > for making it simple to properly implement the `marker` case. That said, I 
> > am perfectly fine with removing the option. In that case, I guess I should 
> > get rid of `EmbedBitcodeKind` altogether and always insert the bitcode and 
> > the marker, right?
> > 
> > On the other hand, we should maybe just consolidate the option (parsing) 
> > with the clang option. Some thoughts about that in my inline comment above.
> It's unclear to me whether it is better to not provide the marker, and get a 
> clear error, or put in an empty marker which may cause these tools to do the 
> wrong thing. @steven_wu who might be able to provide some guidance about how 
> this is used by the MacOS linker. Note that if someone is using MacOS to do 
> their LTO linking, they won't even trigger this code, as that toolchain uses 
> the old legacy LTO, which doesn't come here.
> 
> Presumably your tool doesn't use the marker section?
I don't think marker is useful for this use case. I will suggest not to have 
the `marker` LTO option unless we have the need. It is being tested by clang 
and that is probably enough coverage for testing.

I don't think any users of the legacy interface express interests in using a 
unified bitcode section so it is fine just to leave this out of the legacy C 
interface.




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

https://reviews.llvm.org/D68213



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


[PATCH] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ accepted this revision.
NoQ added a comment.

Thanks!!~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70439



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


[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

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

In D71179#1778512 , @hfinkel wrote:

> In D71179#1776761 , @ABataev wrote:
>
> > In D71179#1776528 , @hfinkel wrote:
> >
> > > In D71179#1776491 , @ABataev 
> > > wrote:
> > >
> > > > In D71179#1776487 , @hfinkel 
> > > > wrote:
> > > >
> > > > > In D71179#1776467 , @ABataev 
> > > > > wrote:
> > > > >
> > > > > > In D71179#1776457 , 
> > > > > > @jdoerfert wrote:
> > > > > >
> > > > > > > > You're doing absolutely the same thing as the original declare 
> > > > > > > > variant implementation.
> > > > > > >
> > > > > > > I don't think so but if you do why do you oppose this approach?
> > > > > > >
> > > > > > > > And I don't think it would be correct to add them as 
> > > > > > > > multiversiin variants to the original function.
> > > > > > >
> > > > > > > Why wouldn't it be correct to pick the version through the 
> > > > > > > overload resolution instead of the code generation?
> > > > > > >  How this could work is already described in the TODO 
> > > > > > > (CodeGenModule.cpp):
> > > > > > >
> > > > > > >   // TODO: We should introduce function aliases for `omp declare 
> > > > > > > variant`
> > > > > > >   //   directives such that we can treat them through the 
> > > > > > > same overload
> > > > > > >   //   resolution scheme (via multi versioning) as `omp begin 
> > > > > > > declare
> > > > > > >   //   variant` functions. For an `omp declare 
> > > > > > > variant(VARIANT) ...`
> > > > > > >   //   that is attached to a BASE function we would create a 
> > > > > > > global alias
> > > > > > >   //   VARIANT = BASE which will participate in the multi 
> > > > > > > version overload
> > > > > > >   //   resolution. If picked, here is no need to emit them 
> > > > > > > explicitly.
> > > > > > >   
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >  ---
> > > > > > >
> > > > > > > I still haven't understood why we cannot/should not reuse the 
> > > > > > > existing multi-version support and instead duplicate the logic in 
> > > > > > > some custom scheme.
> > > > > > >  We have this patch that shows how we can reuse the logic in 
> > > > > > > Clang. It works on a per-call basis, so it will work for all 
> > > > > > > context selector (incl. construct).
> > > > > > >  If you think there is something conceptually not working, I'd 
> > > > > > > like to hear about it. However, just saying "it wouldn't be 
> > > > > > > correct" is not sufficient. You need to provide details about the 
> > > > > > > situation, what you think would not work, and why.
> > > > > >
> > > > > >
> > > > > > I explayned already: declare variant cannot be represented as 
> > > > > > mutiversion functiin, for example.
> > > > >
> > > > >
> > > > > @ABataev, can you please elaborate? It's not obvious to me that we 
> > > > > cannot handle the existing declare variant with the same scheme (as 
> > > > > @jdoerfert highlighted above). In general, I believe it's preferable 
> > > > > to have one generic scheme and use it to handle all cases as opposed 
> > > > > to continuing to use a more-limited scheme in addition to the generic 
> > > > > scheme.
> > > >
> > > >
> > > > Eaine already. Current version of declare variant cannot be represented 
> > > > as multiversiin functions, because it is not. We have a function that 
> > > > is the alias to other functions with different names. They just are not 
> > > > multiversion functions by definition.
> > >
> > >
> > > I understand that they have different names. I don't see why we that 
> > > means that they can't be added to the overload set as multi-version 
> > > candidates if we add logic which does exactly that.
> >
> >
> >
>
>
> @jdoerfert posted a prototype implementation in D71241 
> , so we don't need to just have a 
> theoretical discussion, but I'd like to address a high-level issue here:
>
> > Because this is exactly what I said- you want to reuse the exiwting 
> > solution for completely different purpose just because you want to you 
> > reuse though even semantically it has nothing to do with multiversioning.
>
> This kind of comment really isn't appropriate. We're all experienced 
> developers here, and no one is proposing to reuse code in an inappropriate 
> manner "just because" or for any other reason. I ask you to reconsider your 
> reasoning here for two reasons:
>
> 1. "Reus[ing] the existing solution for a completely different purpose", 
> which I'll classify as structural code reuse, is not necessarily bad. 
> Structural code reuse, where you reuse code with a similar structure, but 
> different purpose, from what you need, is often a useful impetus for the 
> creation of new abstractions. 

[PATCH] D70244: [Analyzer] Iterator Checkers: Replace `UnknownVal` in comparison result by a conjured value

2019-12-10 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.

That's a fair point, these days we always have to account for unknown values.

I'm still curious (for science!) about how exactly it came to be in the tests.


Repository:
  rC Clang

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

https://reviews.llvm.org/D70244



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


[PATCH] D71313: [AST] Split parent map traversal logic into ParentMapContext.h

2019-12-10 Thread Reid Kleckner via Phabricator via cfe-commits
rnk created this revision.
rnk added reviewers: bkramer, klimek, rsmith.
Herald added subscribers: lldb-commits, kbarton, mgorny, nemanjai.
Herald added projects: clang, LLDB.

The only part of ASTContext.h that requires most AST types to be
complete is the parent map. Nothing in Clang proper uses the ParentMap,
so split it out into its own class. Make ASTContext own the
ParentMapContext so there is still a one-to-one relationship.

After this change, 562 fewer files depend on ASTTypeTraits.h, and 66
fewer depend on TypeLoc.h:

  $ diff -u deps-before.txt deps-after.txt | \
grep '^[-+] ' | sort | uniq -c | sort -nr | less
  562 -../clang/include/clang/AST/ASTTypeTraits.h
  340 +../clang/include/clang/AST/ParentMapContext.h
   66 -../clang/include/clang/AST/TypeLocNodes.def
   66 -../clang/include/clang/AST/TypeLoc.h
   15 -../clang/include/clang/AST/TemplateBase.h
  ...

I computed deps-before.txt and deps-after.txt with `ninja -t deps`.

This removes a common and key dependency on TemplateBase.h and
TypeLoc.h.

This also has the effect of breaking the ParentMap RecursiveASTVisitor
instantiation into its own file, which roughly halves the compilation
time of ASTContext.cpp (29.75s -> 17.66s). The new file takes 13.8s to
compile.

I left behind forwarding methods for getParents(), but clients will need
to include a new header to make them work:

  #include "clang/AST/ParentMapContext.h"

I noticed that this parent map functionality is unfortunately duplicated
in ParentMap.h, which only works for Stmt nodes.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71313

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp
  clang-tools-extra/clang-tidy/readability/MakeMemberFunctionConstCheck.cpp
  clang-tools-extra/clang-tidy/utils/ExprSequence.cpp
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ParentMapContext.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Linkage.h
  clang/lib/AST/ParentMapContext.cpp
  clang/lib/ASTMatchers/ASTMatchFinder.cpp
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/CodeGen/CGCall.h
  clang/lib/Tooling/ASTDiff/ASTDiff.cpp
  clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
  lldb/include/lldb/Symbol/ClangASTContext.h

Index: lldb/include/lldb/Symbol/ClangASTContext.h
===
--- lldb/include/lldb/Symbol/ClangASTContext.h
+++ lldb/include/lldb/Symbol/ClangASTContext.h
@@ -21,6 +21,7 @@
 #include 
 
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTFwd.h"
 #include "clang/AST/ExternalASTMerger.h"
 #include "clang/AST/TemplateBase.h"
 #include "llvm/ADT/APSInt.h"
Index: clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
===
--- clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
+++ clang/lib/Tooling/Refactoring/Rename/USRLocFinder.cpp
@@ -15,6 +15,7 @@
 
 #include "clang/Tooling/Refactoring/Rename/USRLocFinder.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
Index: clang/lib/Tooling/ASTDiff/ASTDiff.cpp
===
--- clang/lib/Tooling/ASTDiff/ASTDiff.cpp
+++ clang/lib/Tooling/ASTDiff/ASTDiff.cpp
@@ -12,6 +12,7 @@
 
 #include "clang/Tooling/ASTDiff/ASTDiff.h"
 
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/PriorityQueue.h"
Index: clang/lib/CodeGen/CGCall.h
===
--- clang/lib/CodeGen/CGCall.h
+++ clang/lib/CodeGen/CGCall.h
@@ -16,6 +16,7 @@
 
 #include "CGValue.h"
 #include "EHScopeStack.h"
+#include "clang/AST/ASTFwd.h"
 #include "clang/AST/CanonicalType.h"
 #include "clang/AST/GlobalDecl.h"
 #include "clang/AST/Type.h"
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/LLVM.h"
@@ -222,7 +223,8 @@
   TraversalKindScope RAII(Finder->getASTContext(),
   Implementation->TraversalKind());
 
-  auto N = Finder->getASTContext().traverseIgnored(DynNode);
+  auto N =
+  Finder->getASTContext().getParentMapContext().traverseIgnored(DynNode);
 
   if 

[PATCH] D71179: [OpenMP][WIP] Initial support for `begin/end declare variant`

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

In D71179#1776761 , @ABataev wrote:

> In D71179#1776528 , @hfinkel wrote:
>
> > In D71179#1776491 , @ABataev wrote:
> >
> > > In D71179#1776487 , @hfinkel 
> > > wrote:
> > >
> > > > In D71179#1776467 , @ABataev 
> > > > wrote:
> > > >
> > > > > In D71179#1776457 , 
> > > > > @jdoerfert wrote:
> > > > >
> > > > > > > You're doing absolutely the same thing as the original declare 
> > > > > > > variant implementation.
> > > > > >
> > > > > > I don't think so but if you do why do you oppose this approach?
> > > > > >
> > > > > > > And I don't think it would be correct to add them as multiversiin 
> > > > > > > variants to the original function.
> > > > > >
> > > > > > Why wouldn't it be correct to pick the version through the overload 
> > > > > > resolution instead of the code generation?
> > > > > >  How this could work is already described in the TODO 
> > > > > > (CodeGenModule.cpp):
> > > > > >
> > > > > >   // TODO: We should introduce function aliases for `omp declare 
> > > > > > variant`
> > > > > >   //   directives such that we can treat them through the same 
> > > > > > overload
> > > > > >   //   resolution scheme (via multi versioning) as `omp begin 
> > > > > > declare
> > > > > >   //   variant` functions. For an `omp declare variant(VARIANT) 
> > > > > > ...`
> > > > > >   //   that is attached to a BASE function we would create a 
> > > > > > global alias
> > > > > >   //   VARIANT = BASE which will participate in the multi 
> > > > > > version overload
> > > > > >   //   resolution. If picked, here is no need to emit them 
> > > > > > explicitly.
> > > > > >   
> > > > > >
> > > > > >
> > > > > >
> > > > > >  ---
> > > > > >
> > > > > > I still haven't understood why we cannot/should not reuse the 
> > > > > > existing multi-version support and instead duplicate the logic in 
> > > > > > some custom scheme.
> > > > > >  We have this patch that shows how we can reuse the logic in Clang. 
> > > > > > It works on a per-call basis, so it will work for all context 
> > > > > > selector (incl. construct).
> > > > > >  If you think there is something conceptually not working, I'd like 
> > > > > > to hear about it. However, just saying "it wouldn't be correct" is 
> > > > > > not sufficient. You need to provide details about the situation, 
> > > > > > what you think would not work, and why.
> > > > >
> > > > >
> > > > > I explayned already: declare variant cannot be represented as 
> > > > > mutiversion functiin, for example.
> > > >
> > > >
> > > > @ABataev, can you please elaborate? It's not obvious to me that we 
> > > > cannot handle the existing declare variant with the same scheme (as 
> > > > @jdoerfert highlighted above). In general, I believe it's preferable to 
> > > > have one generic scheme and use it to handle all cases as opposed to 
> > > > continuing to use a more-limited scheme in addition to the generic 
> > > > scheme.
> > >
> > >
> > > Eaine already. Current version of declare variant cannot be represented 
> > > as multiversiin functions, because it is not. We have a function that is 
> > > the alias to other functions with different names. They just are not 
> > > multiversion functions by definition.
> >
> >
> > I understand that they have different names. I don't see why we that means 
> > that they can't be added to the overload set as multi-version candidates if 
> > we add logic which does exactly that.
>
>
>


@jdoerfert posted a prototype implementation in D71241 
, so we don't need to just have a theoretical 
discussion, but I'd like to address a high-level issue here:

> Because this is exactly what I said- you want to reuse the exiwting solution 
> for completely different purpose just because you want to you reuse though 
> even semantically it has nothing to do with multiversioning.

This kind of comment really isn't appropriate. We're all experienced developers 
here, and no one is proposing to reuse code in an inappropriate manner "just 
because" or for any other reason. I ask you to reconsider your reasoning here 
for two reasons:

1. "Reus[ing] the existing solution for a completely different purpose", which 
I'll classify as structural code reuse, is not necessarily bad. Structural code 
reuse, where you reuse code with a similar structure, but different purpose, 
from what you need, is often a useful impetus for the creation of new 
abstractions. The trade off relevant here, in my experience, is against future 
structural divergence. In the future, is it likely that the abstraction will 
break down because the different purposes will tend to require the code 
structure to change in the future in divergent ways? If so, that can be 

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D71224#1778332 , @xazax.hun wrote:

> So basically what I am wonder/worrying about is the following:
>  The analyzer core will decide that the stack region is escaped and the 
> checkers has no word about this.


Yup, you got me. Pre-escaped locals are indeed material and beyond the 
checker's control. I don't seem to have any immediate solutions. I think we 
could postpone the work on pre-escaped locals (until we figure out how to do 
them correctly) if they're not immediately necessary to you (after all i was 
the one who suggested it). Or ignore the problem (depending on how we do our FP 
vs. FN trade-off).

In D71224#1778357 , @xazax.hun wrote:

> Consider the following two snippets:


Mm, these snippets don't have pre-escaped locals. Like, they accidentally do, 
but above i proposed to work around this by removing the first escape 
invocation (that happens during the call) and only doing it after the call. 
This way these locals don't have time to become pre-escaped. I think these are 
not a problem.


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

https://reviews.llvm.org/D71224



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


Re: [clang] 7f9b513 - Reapply af57dbf12e54 "Add support for options -frounding-math, ftrapping-math, -ffp-model=, and -ffp-exception-behavior="

2019-12-10 Thread Eric Christopher via cfe-commits
So you left some debugging code in that I've fixed thusly:

"Enclosing function uses fp intrinsics"

commit f4a7d5659df7cb56c1baa34a39e9fe2639472741
Author: Eric Christopher 
Date:   Tue Dec 10 15:02:29 2019 -0800

Remove debugging printf and reformat code.

On Thu, Dec 5, 2019 at 3:48 AM Melanie Blower via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

>
> Author: Melanie Blower
> Date: 2019-12-05T03:48:04-08:00
> New Revision: 7f9b5138470db1dc58f3bc05631284c653c9ed7a
>
> URL:
> https://github.com/llvm/llvm-project/commit/7f9b5138470db1dc58f3bc05631284c653c9ed7a
> DIFF:
> https://github.com/llvm/llvm-project/commit/7f9b5138470db1dc58f3bc05631284c653c9ed7a.diff
>
> LOG: Reapply af57dbf12e54 "Add support for options
> -frounding-math, ftrapping-math, -ffp-model=, and -ffp-exception-behavior="
>
> Patch was reverted because
> https://bugs.llvm.org/show_bug.cgi?id=44048
> The original patch is modified to set the strictfp IR attribute
> explicitly in CodeGen instead of as a side effect of IRBuilder.
> In the 2nd attempt to reapply there was a windows lit test fail,
> the
> tests were fixed to use wildcard matching.
>
> Differential Revision: https://reviews.llvm.org/D62731
>
> Added:
> clang/test/CodeGen/fpconstrained.c
> clang/test/CodeGen/fpconstrained.cpp
> clang/test/Driver/fp-model.c
>
> Modified:
> clang/docs/UsersManual.rst
> clang/include/clang/AST/Decl.h
> clang/include/clang/AST/DeclBase.h
> clang/include/clang/Basic/DiagnosticDriverKinds.td
> clang/include/clang/Basic/DiagnosticGroups.td
> clang/include/clang/Basic/LangOptions.def
> clang/include/clang/Basic/LangOptions.h
> clang/include/clang/Driver/Options.td
> clang/lib/AST/Decl.cpp
> clang/lib/CodeGen/CGCall.cpp
> clang/lib/CodeGen/CodeGenFunction.cpp
> clang/lib/CodeGen/CodeGenFunction.h
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/lib/Frontend/CompilerInvocation.cpp
> clang/lib/Sema/SemaExpr.cpp
> clang/lib/Serialization/ASTReaderDecl.cpp
> clang/lib/Serialization/ASTWriterDecl.cpp
> clang/test/Driver/clang_f_opts.c
> clang/test/Driver/fast-math.c
> llvm/include/llvm/IR/IRBuilder.h
> llvm/include/llvm/Target/TargetOptions.h
> llvm/unittests/IR/IRBuilderTest.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
> index 714681d7f4ce..62e2575c6b26 100644
> --- a/clang/docs/UsersManual.rst
> +++ b/clang/docs/UsersManual.rst
> @@ -1231,10 +1231,10 @@ are listed below.
>
>  **-f[no-]trapping-math**
>
> -   ``-fno-trapping-math`` allows optimizations that assume that
> -   floating point operations cannot generate traps such as divide-by-zero,
> -   overflow and underflow. Defaults to ``-ftrapping-math``.
> -   Currently this option has no effect.
> +   Control floating point exception behavior. ``-fno-trapping-math``
> allows optimizations that assume that floating point operations cannot
> generate traps such as divide-by-zero, overflow and underflow.
> +
> +- The option ``-ftrapping-math`` behaves identically to
> ``-ffp-exception-behavior=strict``.
> +- The option ``-fno-trapping-math`` behaves identically to
> ``-ffp-exception-behavior=ignore``.   This is the default.
>
>  .. option:: -ffp-contract=
>
> @@ -1319,6 +1319,52 @@ are listed below.
>
> Defaults to ``-fno-finite-math``.
>
> +.. _opt_frounding-math:
> +
> +**-f[no-]rounding-math**
> +
> +Force floating-point operations to honor the dynamically-set rounding
> mode by default.
> +
> +The result of a floating-point operation often cannot be exactly
> represented in the result type and therefore must be rounded.  IEEE 754
> describes
> diff erent rounding modes that control how to perform this rounding, not
> all of which are supported by all implementations.  C provides interfaces
> (``fesetround`` and ``fesetenv``) for dynamically controlling the rounding
> mode, and while it also recommends certain conventions for changing the
> rounding mode, these conventions are not typically enforced in the ABI.
> Since the rounding mode changes the numerical result of operations, the
> compiler must understand something about it in order to optimize floating
> point operations.
> +
> +Note that floating-point operations performed as part of constant
> initialization are formally performed prior to the start of the program and
> are therefore not subject to the current rounding mode.  This includes the
> initialization of global variables and local ``static`` variables.
> Floating-point operations in these contexts will be rounded using
> ``FE_TONEAREST``.
> +
> +- The option ``-fno-rounding-math`` allows the compiler to assume that
> the rounding mode is set to ``FE_TONEAREST``.  This is the default.
> +- The option ``-frounding-math`` forces the compiler to honor the
> dynamically-set rounding mode.  This 

[clang] f4a7d56 - Remove debugging printf and reformat code.

2019-12-10 Thread Eric Christopher via cfe-commits

Author: Eric Christopher
Date: 2019-12-10T15:04:45-08:00
New Revision: f4a7d5659df7cb56c1baa34a39e9fe2639472741

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

LOG: Remove debugging printf and reformat code.

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index f6bcf899a7fb..a941d524b7f2 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13010,11 +13010,9 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation 
OpLoc,
   (getLangOpts().getFPRoundingMode() != LangOptions::FPR_ToNearest ||
getLangOpts().getFPExceptionMode() != LangOptions::FPE_Ignore))
 // Mark the current function as usng floating point constrained intrinsics
-if (FunctionDecl *F = dyn_cast(CurContext))
-{
+if (FunctionDecl *F = dyn_cast(CurContext)) {
   F->setUsesFPIntrin(true);
-  printf("Enclosing function uses fp intrinsics\n");
-}
+}
 
   // Some of the binary operations require promoting operands of half vector to
   // float vectors and truncating the result back to half vector. For now, we 
do



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


[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

Whoops, this one was updated accidentally. Wrong tab :(


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

https://reviews.llvm.org/D71152



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233200.
xazax.hun added a comment.

- First take on trying to reuse `processPointerEscapedOnBind`.


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

https://reviews.llvm.org/D71224

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ponter-escape-on-conservative-calls.c

Index: clang/test/Analysis/ponter-escape-on-conservative-calls.c
===
--- /dev/null
+++ clang/test/Analysis/ponter-escape-on-conservative-calls.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+
+int main() {
+int a;
+f();
+return 0;
+}
+
+// CHECK: PostCall
+// CHECK-NEXT: PointerEscape
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -37,6 +37,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/AST/Decl.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
@@ -592,9 +593,45 @@
   for (auto I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Escaping symbols conjured during invalidating the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  SmallVector, 8> Escaped;
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+ProgramStateRef State = I->getState();
+Escaped.clear();
+{
+  unsigned Arg = -1;
+  for (const ParmVarDecl *PVD : Call.parameters()) {
+++Arg;
+QualType ParamTy = PVD->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+QualType Pointee = ParamTy->getPointeeType();
+if (Pointee.isConstQualified() || Pointee->isVoidType())
+  continue;
+if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+  Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee));
+  }
+}
+
+State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(),
+PSK_EscapeOnConservativeCall, );
+
+if (State == I->getState())
+  Dst.insert(I);
+else
+  B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent ,
@@ -670,8 +707,7 @@
 // Conservatively evaluate call by invalidating regions and binding
 // a conjured return value.
 void ExprEngine::conservativeEvalCall(const CallEvent , NodeBuilder ,
-  ExplodedNode *Pred,
-  ProgramStateRef State) {
+  ExplodedNode *Pred, ProgramStateRef State) {
   State = 

[PATCH] D71152: [analyzer] Keep track of escaped locals.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun updated this revision to Diff 233198.
xazax.hun added a comment.

- A first take on trying to reuse `processPointerEscapedOnBind`.


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

https://reviews.llvm.org/D71152

Files:
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SubEngine.h
  clang/lib/StaticAnalyzer/Checkers/AnalysisOrderChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/ponter-escape-on-conservative-calls.c

Index: clang/test/Analysis/ponter-escape-on-conservative-calls.c
===
--- /dev/null
+++ clang/test/Analysis/ponter-escape-on-conservative-calls.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=debug.AnalysisOrder -analyzer-config debug.AnalysisOrder:PointerEscape=true -analyzer-config debug.AnalysisOrder:PostCall=true %s 2>&1 | FileCheck %s
+
+
+void f(int *);
+
+int main() {
+int a;
+f();
+return 0;
+}
+
+// CHECK: PostCall
+// CHECK-NEXT: PointerEscape
Index: clang/test/Analysis/analyzer-config.c
===
--- clang/test/Analysis/analyzer-config.c
+++ clang/test/Analysis/analyzer-config.c
@@ -37,6 +37,7 @@
 // CHECK-NEXT: debug.AnalysisOrder:EndFunction = false
 // CHECK-NEXT: debug.AnalysisOrder:LiveSymbols = false
 // CHECK-NEXT: debug.AnalysisOrder:NewAllocator = false
+// CHECK-NEXT: debug.AnalysisOrder:PointerEscape = false
 // CHECK-NEXT: debug.AnalysisOrder:PostCall = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtArraySubscriptExpr = false
 // CHECK-NEXT: debug.AnalysisOrder:PostStmtCXXNewExpr = false
@@ -97,4 +98,4 @@
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 94
+// CHECK-NEXT: num-entries = 95
Index: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/AST/Decl.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "PrettyStackTraceLocationContext.h"
 #include "clang/AST/CXXInheritance.h"
@@ -592,9 +593,45 @@
   for (auto I : dstCallEvaluated)
 finishArgumentConstruction(dstArgumentCleanup, I, Call);
 
-  // Finally, run any post-call checks.
-  getCheckerManager().runCheckersForPostCall(Dst, dstArgumentCleanup,
+  ExplodedNodeSet dstPostCall;
+  getCheckerManager().runCheckersForPostCall(dstPostCall, dstArgumentCleanup,
  Call, *this);
+
+  // Escaping symbols conjured during invalidating the regions above.
+  // Note that, for inlined calls the nodes were put back into the worklist,
+  // so we can assume that every node belongs to a conservative call at this
+  // point.
+
+  // Run pointerEscape callback with the newly conjured symbols.
+  SmallVector, 8> Escaped;
+  for (auto I : dstPostCall) {
+NodeBuilder B(I, Dst, *currBldrCtx);
+ProgramStateRef State = I->getState();
+Escaped.clear();
+{
+  unsigned Arg = -1;
+  for (const ParmVarDecl *PVD : Call.parameters()) {
+++Arg;
+QualType ParamTy = PVD->getType();
+if (ParamTy.isNull() ||
+(!ParamTy->isPointerType() && !ParamTy->isReferenceType()))
+  continue;
+QualType Pointee = ParamTy->getPointeeType();
+if (Pointee.isConstQualified() || Pointee->isVoidType())
+  continue;
+if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+  Escaped.emplace_back(loc::MemRegionVal(MR), State->getSVal(MR, Pointee));
+  }
+}
+
+State = processPointerEscapedOnBind(State, Escaped, I->getLocationContext(),
+PSK_EscapeOnConservativeCall, );
+
+if (State == I->getState())
+  Dst.insert(I);
+else
+  B.generateNode(I->getLocation(), State, I);
+  }
 }
 
 ProgramStateRef ExprEngine::bindReturnValue(const CallEvent ,
@@ -670,8 +707,7 @@
 // Conservatively evaluate call by invalidating regions and binding
 // a conjured return value.
 void ExprEngine::conservativeEvalCall(const CallEvent , NodeBuilder ,
-  ExplodedNode *Pred,
-  ProgramStateRef State) {
+  ExplodedNode *Pred, ProgramStateRef State) {
   State = 

[PATCH] D71239: [clang-format] Fix ObjC keywords following try/catch getting split.

2019-12-10 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese marked an inline comment as done.
Bigcheese added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1849
+  } while (FormatTok->is(tok::comment));
+}
 if (!(FormatTok->isOneOf(tok::kw_catch, Keywords.kw___except,

MyDeveloperDay wrote:
> can you use `FormatTok->getNextNonComment()`?
No, `Next` has not been setup at this point, so it will always return `nullptr`.



Comment at: clang/unittests/Format/FormatTestObjC.cpp:200
+   "} @catch (NSException *e) {\n"
+   "}\n");
   verifyFormat("DEBUG({\n"

MyDeveloperDay wrote:
> Nit: Could you not keep the original test? just add a new testcase? I get 
> uncomfortable about changing tests no matter how trivial
Is there something special about clang-format that causes this concern? I'm all 
for testing separate things separately, but the additions to this test are 
testing the same leaf lines of code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71239



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

I think I found the main problem with the current model, at least for the 
FuchsiaHandleCheck.

Consider the following two snippets:

  zx_handle_t *get_handle_address();
  void escape_store_to_escaped_region01() {
zx_handle_t sb;
if (zx_channel_create(0, get_handle_address(), ))
  return;
zx_handle_close(sb);
  }



  void leak() {
zx_handle_t sa, sb;
if (zx_channel_create(0, , ))
  return;
zx_handle_close(sb);
  }

In the first one I want the first handle to be escaped in the second one I do 
not want it to be escaped.

With my current proposed changes the checker will receive a pointer escape 
callback for both but it does not have enough info to differentiate between the 
two cases.

If I do not act upon this kind of escape I end up reporting a false positive in 
the first case. If I act on this escape I end up missing a true positive in the 
second case.


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

https://reviews.llvm.org/D71224



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D71224#1778303 , @NoQ wrote:

> In D71224#1778284 , @xazax.hun wrote:
>
> > So I was wondering if we got the default right. Maybe a checker should do 
> > more work to get the escaping rather than more work preventing it?
>
>
> But that's exactly how it works right now(?) If you don't define 
> `checkPointerEscape` you get no escaping, if you do extra work of defining it 
> you get the exact amount of escaping that you want.


So basically what I am wonder/worrying about is the following:
The analyzer core will decide that the stack region is escaped and the checkers 
has no word about this. And from that time on the checkers have to do extra 
work each time there is a store or conservative call to find out if this escape 
corresponds to a region that was escaped earlier unwillingly (from the 
checker's point of view).


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

https://reviews.llvm.org/D71224



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D71224#1778284 , @xazax.hun wrote:

> So I was wondering if we got the default right. Maybe a checker should do 
> more work to get the escaping rather than more work preventing it?


But that's exactly how it works right now(?) If you don't define 
`checkPointerEscape` you get no escaping, if you do extra work of defining it 
you get the exact amount of escaping that you want.


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

https://reviews.llvm.org/D71224



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


[PATCH] D71300: [clangd] Deduplicate refs from index for cross-file rename.

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

Build result: pass - 60688 tests passed, 0 failed and 726 were skipped.

Log files: console-log.txt 
,
 CMakeCache.txt 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71300



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D71224#1778231 , @NoQ wrote:

> In D71224#1778204 , @xazax.hun wrote:
>
> > I don't think this is a good enough model currently. The problem is that, 
> > it does not play well with annotations. E.g. the checker can see a symbol 
> > escaping, but it does not have a whole lot of information how. For example, 
> > currently, there is no way to check if the output parameter through which 
> > the escape happened was annotated somehow.
>
>
> Hmm. If the function is annotated, it is hopefully "fully" annotated, or at 
> least the programmer doesn't mind adding more annotations to it. Given that 
> you have your `CallEvent` structure in `checkPointerEscape`, i hope you can 
> easily see if there are any annotations at all on the function, and if so, 
> suppress the current escape entirely. Or at least scan the annotated 
> parameters and suppress the escape for them.
>
> I guess it's still a problem if the *same* handle is also passed through a 
> parameter that *cannot* be annotated (eg., as part of a structure passed into 
> the call) and then actually getting released inside the call, but is it a 
> real problem for you?


Yeah, this was one of my idea as well. I think one of my main concerns is that 
I would except the majority of the escapes are simply being output parameters 
and only a minority are legitimate. So I was wondering if we got the default 
right. Maybe a checker should do more work to get the escaping rather than more 
work preventing it?


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

https://reviews.llvm.org/D71224



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


[PATCH] D71226: Also synthesize _cmd and self for properties

2019-12-10 Thread Liu Liu via Phabricator via cfe-commits
liuliu added a comment.

Thanks. Verified in our codebase this patch indeed fixed the crashes!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71226



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


[PATCH] D71301: [clang][IFS] Prevent Clang-IFS from Leaking symbols from inside a block.

2019-12-10 Thread Puyan Lotfi via Phabricator via cfe-commits
plotfi created this revision.
plotfi added reviewers: compnerd, cishida.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.

Built libdispatch with clang interface stubs. Ran into some block related 
issues. Basically VarDecl symbols can leak out because I wasn't checking the 
case where a VarDecl is contained inside a BlockDecl (versus a method or 
function).

  

This patch checks that a VarDecl is not a child decl of a BlockDecl.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71301

Files:
  clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
  clang/test/InterfaceStubs/blocks.c


Index: clang/test/InterfaceStubs/blocks.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/blocks.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-interface-stubs -fblocks -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT: ...
+static void (^f)(void*) = ^(void* data) { int i; };
Index: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -52,6 +52,11 @@
   if (!isVisible(ND))
 return true;
 
+  if (const VarDecl *VD = dyn_cast(ND))
+if (const auto *Parent = VD->getParentFunctionOrMethod())
+  if (const auto *BD = dyn_cast_or_null(Parent))
+return true;
+
   if (const VarDecl *VD = dyn_cast(ND))
 if ((VD->getStorageClass() == StorageClass::SC_Extern) ||
 (VD->getStorageClass() == StorageClass::SC_Static &&


Index: clang/test/InterfaceStubs/blocks.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/blocks.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-interface-stubs -fblocks -o - %s | FileCheck %s
+
+// CHECK: --- !experimental-ifs-v1
+// CHECK-NEXT: IfsVersion: 1.0
+// CHECK-NEXT: Triple:
+// CHECK-NEXT: ObjectFileFormat: ELF
+// CHECK-NEXT: Symbols:
+// CHECK-NEXT: ...
+static void (^f)(void*) = ^(void* data) { int i; };
Index: clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
===
--- clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
+++ clang/lib/Frontend/InterfaceStubFunctionsConsumer.cpp
@@ -52,6 +52,11 @@
   if (!isVisible(ND))
 return true;
 
+  if (const VarDecl *VD = dyn_cast(ND))
+if (const auto *Parent = VD->getParentFunctionOrMethod())
+  if (const auto *BD = dyn_cast_or_null(Parent))
+return true;
+
   if (const VarDecl *VD = dyn_cast(ND))
 if ((VD->getStorageClass() == StorageClass::SC_Extern) ||
 (VD->getStorageClass() == StorageClass::SC_Static &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71300: [clangd] Deduplicate refs from index for cross-file rename.

2019-12-10 Thread Haojian Wu via Phabricator via cfe-commits
hokein created this revision.
hokein added a reviewer: ilya-biryukov.
Herald added subscribers: usaxena95, kadircet, arphaman, mgrang, jkorous, 
MaskRay.
Herald added a project: clang.

If the index returns duplicated refs, it will trigger the assertion in
BuildRenameEdit (we expect the processing position is always larger the
the previous one, but it is not true if we have duplication), and also
breaks our heuristics.

This patch make the code robost enough to handle duplications, also
save some cost of redundnat llvm::sort.

Though clangd's index doesn't return duplications, our internal index
kythe will.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71300

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

Index: clang-tools-extra/clangd/unittests/RenameTests.cpp
===
--- clang-tools-extra/clangd/unittests/RenameTests.cpp
+++ clang-tools-extra/clangd/unittests/RenameTests.cpp
@@ -30,6 +30,20 @@
 using testing::UnorderedElementsAre;
 using testing::UnorderedElementsAreArray;
 
+// Covnert a Range to a Ref.
+std::pair toRef(const clangd::Range ,
+  llvm::StringRef Path) {
+  Ref Result;
+  Result.Kind = RefKind::Reference;
+  Result.Location.Start.setLine(Range.start.line);
+  Result.Location.Start.setColumn(Range.start.character);
+  Result.Location.End.setLine(Range.end.line);
+  Result.Location.End.setColumn(Range.end.character);
+  std::string Storage = URI::create(Path).toString();
+  Result.Location.FileURI = Storage.c_str();
+  return {Result, std::move(Storage)};
+}
+
 // Build a RefSlab from all marked ranges in the annotation. The ranges are
 // assumed to associate with the given SymbolName.
 std::unique_ptr buildRefSlab(const Annotations ,
@@ -41,15 +55,8 @@
   auto Symbols = TU.headerSymbols();
   const auto  = findSymbol(Symbols, SymbolName).ID;
   for (const auto  : Code.ranges()) {
-Ref R;
-R.Kind = RefKind::Reference;
-R.Location.Start.setLine(Range.start.line);
-R.Location.Start.setColumn(Range.start.character);
-R.Location.End.setLine(Range.end.line);
-R.Location.End.setColumn(Range.end.character);
-auto U = URI::create(Path).toString();
-R.Location.FileURI = U.c_str();
-Builder.insert(SymbolID, R);
+auto R = toRef(Range, Path);
+Builder.insert(SymbolID, R.first);
   }
 
   return std::make_unique(std::move(Builder).build());
@@ -664,6 +671,53 @@
   testing::HasSubstr("too many occurrences"));
 }
 
+TEST(CrossFileRenameTests, DeduplicateRefsFromIndex) {
+  auto MainCode = Annotations("int [[^x]] = 2;");
+  auto MainFilePath = testPath("main.cc");
+  auto BarCode = Annotations("int [[x]];");
+  auto BarPath = testPath("bar.cc");
+  auto TU = TestTU::withCode(MainCode.code());
+  // Set a file "bar.cc" on disk.
+  TU.AdditionalFiles["bar.cc"] = BarCode.code();
+  auto AST = TU.build();
+  auto XRefInBarCC = toRef(BarCode.range(), BarPath);
+  // The index will return duplicated refs, our code should be robost to handle
+  // it.
+  class DuplicatedXRefIndex : public SymbolIndex {
+  public:
+DuplicatedXRefIndex(const Ref ) : ReturnedRef(ReturnedRef) {}
+bool refs(const RefsRequest ,
+  llvm::function_ref Callback) const override {
+  // Return two duplicated refs.
+  Callback(ReturnedRef);
+  Callback(ReturnedRef);
+  return false;
+}
+
+bool fuzzyFind(const FuzzyFindRequest &,
+   llvm::function_ref) const override {
+  return false;
+}
+void lookup(const LookupRequest &,
+llvm::function_ref) const override {}
+
+void relations(const RelationsRequest &,
+   llvm::function_ref)
+const override {}
+size_t estimateMemoryUsage() const override { return 0; }
+Ref ReturnedRef;
+  } DIndex(XRefInBarCC.first);
+  llvm::StringRef NewName = "newName";
+  auto Results = rename({MainCode.point(), NewName, AST, MainFilePath, ,
+ /*CrossFile=*/true});
+  ASSERT_TRUE(bool(Results)) << Results.takeError();
+  EXPECT_THAT(
+  applyEdits(std::move(*Results)),
+  UnorderedElementsAre(
+  Pair(Eq(BarPath), Eq(expectedResult(BarCode, NewName))),
+  Pair(Eq(MainFilePath), Eq(expectedResult(MainCode, NewName);
+}
+
 TEST(CrossFileRenameTests, WithUpToDateIndex) {
   MockCompilationDatabase CDB;
   CDB.ExtraClangFlags = {"-xc++"};
Index: clang-tools-extra/clangd/refactor/Rename.h
===
--- clang-tools-extra/clangd/refactor/Rename.h
+++ clang-tools-extra/clangd/refactor/Rename.h
@@ -51,6 +51,7 @@
 /// Generates rename edits that replaces all given occurrences with the
 /// NewName.
 /// Exposed for testing only.
+/// REQUIRED: Occurrences is sorted and doesn't have duplicated ranges.
 

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In D71224#1778204 , @xazax.hun wrote:

> I don't think this is a good enough model currently. The problem is that, it 
> does not play well with annotations. E.g. the checker can see a symbol 
> escaping, but it does not have a whole lot of information how. For example, 
> currently, there is no way to check if the output parameter through which the 
> escape happened was annotated somehow.


Hmm. If the function is annotated, it is hopefully "fully" annotated, or at 
least the programmer doesn't mind adding more annotations to it. Given that you 
have your `CallEvent` structure in `checkPointerEscape`, i hope you can easily 
see if there are any annotations at all on the function, and if so, suppress 
the current escape entirely. Or at least scan the annotated parameters and 
suppress the escape for them.

I guess it's still a problem if the *same* handle is also passed through a 
parameter that *cannot* be annotated (eg., as part of a structure passed into 
the call) and then actually getting released inside the call, but is it a real 
problem for you?


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

https://reviews.llvm.org/D71224



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


[PATCH] D70613: Add method to ignore invisible AST nodes

2019-12-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire updated this revision to Diff 233183.
steveire added a comment.

Update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70613

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/Expr.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/Expr.cpp
  clang/unittests/AST/ASTTraverserTest.cpp
  clang/unittests/AST/CMakeLists.txt
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -1680,6 +1680,149 @@
  functionDecl(hasDescendant(Matcher)));
 }
 
+TEST(Traversal, traverseUnlessSpelledInSource) {
+
+  StringRef Code = R"cpp(
+
+struct A
+{
+};
+
+struct B
+{
+  B(int);
+  B(A const& a);
+  B();
+};
+
+struct C
+{
+  operator B();
+};
+
+B func1() {
+  return 42;
+}
+
+B func2() {
+  return B{42};
+}
+
+B func3() {
+  return B(42);
+}
+
+B func4() {
+  return B();
+}
+
+B func5() {
+  return B{};
+}
+
+B func6() {
+  return C();
+}
+
+B func7() {
+  return A();
+}
+
+B func8() {
+  return C{};
+}
+
+B func9() {
+  return A{};
+}
+
+B func10() {
+  A a;
+  return a;
+}
+
+B func11() {
+  B b;
+  return b;
+}
+
+B func12() {
+  C c;
+  return c;
+}
+
+)cpp";
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func1"))),
+hasReturnValue(integerLiteral(equals(42)));
+
+  EXPECT_TRUE(matches(
+  Code,
+  traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+   returnStmt(forFunction(functionDecl(hasName("func2"))),
+  hasReturnValue(cxxTemporaryObjectExpr(
+  hasArgument(0, integerLiteral(equals(42);
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func3"))),
+hasReturnValue(
+cxxFunctionalCastExpr(hasSourceExpression(
+integerLiteral(equals(42);
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func4"))),
+hasReturnValue(cxxTemporaryObjectExpr());
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func5"))),
+hasReturnValue(cxxTemporaryObjectExpr());
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func6"))),
+hasReturnValue(cxxTemporaryObjectExpr());
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func7"))),
+hasReturnValue(cxxTemporaryObjectExpr());
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func8"))),
+hasReturnValue(cxxFunctionalCastExpr(
+hasSourceExpression(initListExpr(;
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func9"))),
+hasReturnValue(cxxFunctionalCastExpr(
+hasSourceExpression(initListExpr(;
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func10"))),
+hasReturnValue(
+declRefExpr(to(varDecl(hasName("a");
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func11"))),
+hasReturnValue(
+declRefExpr(to(varDecl(hasName("b");
+
+  EXPECT_TRUE(matches(
+  Code, traverse(ast_type_traits::TK_IgnoreUnlessSpelledInSource,
+ returnStmt(forFunction(functionDecl(hasName("func12"))),
+hasReturnValue(
+

[PATCH] D70613: Add method to ignore invisible AST nodes

2019-12-10 Thread Stephen Kelly via Phabricator via cfe-commits
steveire marked 10 inline comments as done.
steveire added a comment.

It wasn't possible to add the `const` because of the return type.




Comment at: clang/include/clang/AST/Expr.h:769-770
+  /// implicit conversions.
+  Expr *IgnoreInvisible();
+  const Expr *IgnoreInvisible() const {
+return const_cast(this)->IgnoreInvisible();

aaron.ballman wrote:
> `Invisible` is a bit of an odd term because the AST nodes themselves are in 
> fact visible within the AST. I think this means "invisible" as in "not 
> corresponding directly to syntax the user wrote". Bikeshedding some other 
> names:
> 
> * IgnoreAllImplicitExprs
> * IgnoreAllImplicitNodes
> * IgnoreNodesNotWrittenByUser
> * IgnoreNotWrittenInSource
> 
> other suggestions welcome.
> 
Changed as discussed on IRC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70613



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


[clang] 3e1562e - Debug Info: Strengthen the synthesized-property-cleanup.mm test, NFC

2019-12-10 Thread Vedant Kumar via cfe-commits

Author: Vedant Kumar
Date: 2019-12-10T13:39:22-08:00
New Revision: 3e1562e83339bb1e18fb36a5a2fc629c75f5f239

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

LOG: Debug Info: Strengthen the synthesized-property-cleanup.mm test, NFC

After https://reviews.llvm.org/D71084, the line locations assigned when
emitting cleanups inside of property accessors changed. Update this test
to actually check that those locations are correct.

rdar://57796656

Added: 


Modified: 
clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm

Removed: 




diff  --git a/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm 
b/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
index 805766b23bdb..8740a4c279d7 100644
--- a/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
+++ b/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone 
-fobjc-arc \
+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone 
-fobjc-arc -fsanitize=nullability-return \
 // RUN:   %s -emit-llvm -o - | FileCheck %s
 
 @interface NSObject
@@ -10,16 +10,25 @@ @interface NSString : NSObject
 
 // CHECK: define {{.*}}@"\01-[MyData setData:]"
 // CHECK: [[DATA:%.*]] = alloca %struct.Data
-// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg 
[[LOC:![0-9]+]]
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg 
[[DATA_PROPERTY_LOC:![0-9]+]]
 // CHECK-NEXT: ret void
 
-// [[LOC]] = !DILocation(line: 0
+// CHECK: define {{.*}}@"\01-[MyData string]"
+// CHECK: call void @__ubsan_handle_nullability_return_v1_abort{{.*}}, !dbg 
[[STRING_PROPERTY_LOC:![0-9]+]]
+// CHECK: ret
 
 @interface MyData : NSObject
 struct Data {
 NSString *name;
 };
+
+// CHECK-DAG: [[DATA_PROPERTY_LOC]] = !DILocation(line: [[@LINE+1]]
 @property struct Data data;
+
+// CHECK-DAG: [[STRING_PROPERTY_LOC]] = !DILocation(line: [[@LINE+1]]
+@property (nonnull) NSString *string;
+
 @end
+
 @implementation MyData
 @end



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


[clang] 02d04d5 - [OPENMP50]Do not mark the function as used if referenced only in declare

2019-12-10 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2019-12-10T16:30:14-05:00
New Revision: 02d04d569edd39a17f27995565b7b65ac06afb35

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

LOG: [OPENMP50]Do not mark the function as used if referenced only in declare
variant directive.

If the function is used only in declare variant directive as a variant
function, it should not be marked as used to prevent emission of the
target-specific functions. Build the reference in the unevaluated
context.

Added: 


Modified: 
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/OpenMP/declare_variant_mixed_codegen.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 89f7f909b379..442d2ce0e0f5 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -1078,9 +1078,16 @@ void 
Parser::ParseOMPDeclareVariantClauses(Parser::DeclGroupPtrTy Ptr,
   SourceLocation RLoc;
   // Parse with IsAddressOfOperand set to true to parse methods as DeclRefExprs
   // instead of MemberExprs.
-  ExprResult AssociatedFunction =
-  ParseOpenMPParensExpr(getOpenMPDirectiveName(OMPD_declare_variant), RLoc,
-/*IsAddressOfOperand=*/true);
+  ExprResult AssociatedFunction;
+  {
+// Do not mark function as is used to prevent its emission if this is the
+// only place where it is used.
+EnterExpressionEvaluationContext Unevaluated(
+Actions, Sema::ExpressionEvaluationContext::Unevaluated);
+AssociatedFunction = ParseOpenMPParensExpr(
+getOpenMPDirectiveName(OMPD_declare_variant), RLoc,
+/*IsAddressOfOperand=*/true);
+  }
   if (!AssociatedFunction.isUsable()) {
 if (!Tok.is(tok::annot_pragma_openmp_end))
   while (!SkipUntil(tok::annot_pragma_openmp_end, StopBeforeMatch))

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index e9456ebda581..71399ff35908 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -385,8 +385,13 @@ static void instantiateOMPDeclareVariantAttr(
   };
 
   ExprResult VariantFuncRef;
-  if (Expr *E = Attr.getVariantFuncRef())
+  if (Expr *E = Attr.getVariantFuncRef()) {
+// Do not mark function as is used to prevent its emission if this is the
+// only place where it is used.
+EnterExpressionEvaluationContext Unevaluated(
+S, Sema::ExpressionEvaluationContext::ConstantEvaluated);
 VariantFuncRef = Subst(E);
+  }
 
   // Check function/variant ref.
   Optional> DeclVarData =

diff  --git a/clang/test/OpenMP/declare_variant_mixed_codegen.cpp 
b/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
index dea42d1c2170..0c13f5f2f120 100644
--- a/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
+++ b/clang/test/OpenMP/declare_variant_mixed_codegen.cpp
@@ -27,6 +27,7 @@
 // CHECK-DAG: ret i32 85
 // CHECK-DAG: ret i32 86
 // CHECK-DAG: ret i32 87
+// CHECK-DAG: ret i32 88
 // CHECK-NOT: ret i32 {{1|4|81|84}}
 
 #ifndef HEADER
@@ -45,8 +46,10 @@ int test();
 #pragma omp declare variant(test) match(implementation = {vendor(llvm)}, 
device={kind(cpu)})
 int call() { return 1; }
 
+static int stat_unused_no_emit() { return 1; }
 static int stat_unused_();
 #pragma omp declare variant(stat_unused_) match(implementation = 
{vendor(llvm)}, device={kind(cpu)})
+#pragma omp declare variant(stat_unused_no_emit) match(implementation = 
{vendor(xxx)}, device={kind(gpu)})
 static int stat_unused() { return 1; }
 
 static int stat_used_();
@@ -134,4 +137,12 @@ int fn_variant2() { return 1; }
 #pragma omp declare variant(fn_variant2) match(implementation = 
{vendor(llvm)}, device={kind(fpga)})
 int fn2() { return 87; }
 
+#pragma omp declare variant(stat_unused_no_emit) match(implementation = 
{vendor(xxx)}, device={kind(gpu)})
+template 
+static T stat_unused_T() { return 88; }
+
+int ba() {
+  return stat_unused_T();
+}
+
 #endif // HEADER



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added a comment.

In D71224#1778179 , @NoQ wrote:

> In any case, every checker is allowed to make their own decisions about 
> escaping. Escape on its own is not material, it's all about how the checker 
> reacts to escapes. Say, it's up to MallocChecker to decide whether the 
> function may or may not release memory that escapes on call.
>
> I think a valid approach would be to simply look up the function in your 
> `CallDescriptionMap` and then abort the `checkPointerEscape` callback when 
> it's found.
>
> Yet, it annoys me a bit that we didn't make everything magically work in an 
> "out of the box" manner. Can we eliminate the first pointer escape (that 
> happens before PostCall) but only keep the secondary escape?


I don't think this is a good enough model currently. The problem is that, it 
does not play well with annotations. E.g. the checker can see a symbol 
escaping, but it does not have a whole lot of information how. For example, 
currently, there is no way to check if the output parameter through which the 
escape happened was annotated somehow.


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

https://reviews.llvm.org/D71224



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


[PATCH] D68896: PR43080: Do not build context-sensitive expressions during name classification.

2019-12-10 Thread Kian Moniri via Phabricator via cfe-commits
kianm added a comment.

Hi, I am still seeing problems with this assertion. Could we please get a fix? 
I've posted the reduced test case and reproducible command on this Phabricator 
patch.

Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68896



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


[clang] 3bd7cbb - [Remarks][Docs] Enhance documentation for opt-remarks driver options

2019-12-10 Thread Francis Visoiu Mistrih via cfe-commits

Author: Francis Visoiu Mistrih
Date: 2019-12-10T13:19:49-08:00
New Revision: 3bd7cbb90cdb9cf2ca481107ec66a75d9c885782

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

LOG: [Remarks][Docs] Enhance documentation for opt-remarks driver options

Add better documentation about the naming scheme, add a few more
explicit descriptions and make the sphinx look better.

Added: 


Modified: 
clang/docs/ClangCommandLineReference.rst
clang/docs/UsersManual.rst
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/docs/ClangCommandLineReference.rst 
b/clang/docs/ClangCommandLineReference.rst
index 7047cd9bb7b3..5b8a96b61b3d 100644
--- a/clang/docs/ClangCommandLineReference.rst
+++ b/clang/docs/ClangCommandLineReference.rst
@@ -1699,9 +1699,14 @@ Emit OpenMP code only for SIMD-based constructs.
 
 .. option:: -foperator-arrow-depth=
 
-.. option:: -foptimization-record-file=
+.. option:: -foptimization-record-file=
 
-Specify the output name of the file containing the optimization remarks. 
Implies -fsave-optimization-record. On Darwin platforms, this cannot be used 
with multiple -arch  options.
+Implies -fsave-optimization-record. On Darwin platforms, this
+  cannot be used with multiple -arch  options.
+
+.. option:: -foptimization-record-passes=
+
+Only include passes which match a specified regular expression in the 
generated optimization record (by default, include all passes)
 
 .. option:: -foptimize-sibling-calls, -fno-optimize-sibling-calls
 
@@ -1834,6 +1839,12 @@ Turn on loop reroller
 
 Generate a YAML optimization record file
 
+.. program:: clang1
+.. option:: -fsave-optimization-record=
+.. program:: clang
+
+Generate an optimization record file in a specific format.
+
 .. option:: -fseh-exceptions
 
 Use SEH style exceptions

diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 62e2575c6b26..87434200e777 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -324,9 +324,10 @@ output format of the diagnostics that it generates.
 
 .. _opt_fsave-optimization-record:
 
-.. option:: -fsave-optimization-record[=]
+.. option:: -f[no-]save-optimization-record[=]
 
-   Write optimization remarks to a separate file.
+   Enable optimization remarks during compilation and write them to a separate
+   file.
 
This option, which defaults to off, controls whether Clang writes
optimization reports to a separate file. By recording diagnostics in a file,
@@ -345,20 +346,45 @@ output format of the diagnostics that it generates.
   ``-fsave-optimization-record=bitstream``: A binary format based on LLVM
   Bitstream.
 
+The output file is controlled by :ref:`-foptimization-record-file 
`.
+
+In the absence of an explicit output file, the file is chosen using the
+following scheme:
+
+``.opt.``
+
+where  is based on the output file of the compilation (whether
+it's explicitly specified through `-o` or not) when used with `-c` or `-S`.
+In other cases, it's based on the input file's stem. For example:
+
+* ``clang -fsave-optimization-record -c in.c -o out.o`` will generate
+  ``out.opt.yaml``
+
+* ``clang -fsave-optimization-record in.c -o out`` will generate
+  ``in.opt.yaml``
+
+Compiling for multiple architectures will use the following scheme:
+
+``-.opt.``
+
+Note that this is only allowed on Darwin platforms and is incompatible with
+passing multiple ``-arch `` options.
+
+When targeting (Thin)LTO, the base is derived from the output filename, and
+the extension is not dropped.
+
+When targeting ThinLTO, the following scheme is used:
+
+``.opt..thin..``
+
 .. _opt_foptimization-record-file:
 
 **-foptimization-record-file**
-   Control the file to which optimization reports are written.
-
-   When optimization reports are being output (see
-   :ref:`-fsave-optimization-record `), this
-   option controls the file to which those reports are written.
+   Control the file to which optimization reports are written. This implies
+   :ref:`-fsave-optimization-record `.
 
-   If this option is not used, optimization records are output to a file named
-   after the primary file being compiled. If that's "foo.c", for example,
-   optimization records are output to "foo.opt.yaml". If a specific
-   serialization format is specified, the file will be named
-   "foo.opt.".
+On Darwin platforms, this is incompatible with passing multiple
+``-arch `` options.
 
 .. _opt_foptimization-record-passes:
 

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 8965131b9001..6f404d1f2ea5 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1800,16 

[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Artem Dergachev via Phabricator via cfe-commits
NoQ added a comment.

In any case, every checker is allowed to make their own decisions about 
escaping. Escape on its own is not material, it's all about how the checker 
reacts to escapes. Say, it's up to MallocChecker to decide whether the function 
may or may not release memory that escapes on call.

I think a valid approach would be to simply look up the function in your 
`CallDescriptionMap` and then abort the `checkPointerEscape` callback when it's 
found.

Yet, it annoys me a bit that we didn't make everything magically work in an 
"out of the box" manner. Can we eliminate the first pointer escape (that 
happens before PostCall) but only keep the secondary escape?


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

https://reviews.llvm.org/D71224



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


[PATCH] D70613: Add method to ignore invisible AST nodes

2019-12-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Expr.h:769-770
+  /// implicit conversions.
+  Expr *IgnoreInvisible();
+  const Expr *IgnoreInvisible() const {
+return const_cast(this)->IgnoreInvisible();

`Invisible` is a bit of an odd term because the AST nodes themselves are in 
fact visible within the AST. I think this means "invisible" as in "not 
corresponding directly to syntax the user wrote". Bikeshedding some other names:

* IgnoreAllImplicitExprs
* IgnoreAllImplicitNodes
* IgnoreNodesNotWrittenByUser
* IgnoreNotWrittenInSource

other suggestions welcome.




Comment at: clang/lib/AST/Expr.cpp:3004-3006
+  Expr *s = this;
+
+  Expr *lasts = nullptr;

Can remove the spurious newline, and these should be named according to the 
usual naming conventions (I'd go with `E` and `LastE`, but feel free to pick 
less terrible names).



Comment at: clang/lib/AST/Expr.cpp:3012
+
+const auto SR = s->getSourceRange();
+

Drop top-level `const` (I wouldn't be sad if it also lost the `auto` at the 
same time).



Comment at: clang/lib/AST/Expr.cpp:3014
+
+if (auto C = dyn_cast(s)) {
+  if (C->getNumArgs() == 1) {

`const auto *`



Comment at: clang/lib/AST/Expr.cpp:3016
+  if (C->getNumArgs() == 1) {
+auto A = C->getArg(0);
+if (A->getSourceRange() == SR || !dyn_cast(C))

`const Expr *`



Comment at: clang/lib/AST/Expr.cpp:3017
+auto A = C->getArg(0);
+if (A->getSourceRange() == SR || !dyn_cast(C))
+  s = A;

`!isa(C)` since you're not using the returned value 
anyway.



Comment at: clang/lib/AST/Expr.cpp:3022
+
+if (auto C = dyn_cast(s)) {
+  auto *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();

`const auto *`



Comment at: clang/lib/AST/Expr.cpp:3023
+if (auto C = dyn_cast(s)) {
+  auto *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
+  if (ExprNode->getSourceRange() == SR) {

`const Expr *`

Are you sure you mean to skip parens here, as those are written in source?



Comment at: clang/lib/AST/Expr.cpp:3024
+  auto *ExprNode = C->getImplicitObjectArgument()->IgnoreParenImpCasts();
+  if (ExprNode->getSourceRange() == SR) {
+s = ExprNode;

Elide braces


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70613



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


[PATCH] D63640: [clang] Improve Serialization/Imporing/Dumping of APValues

2019-12-10 Thread Tyker via Phabricator via cfe-commits
Tyker added a comment.

now that the issue with uniqueness of expressions is solved. we should be able 
to keep going on that review @rsmith.
https://reviews.llvm.org/D63960 should be i think close to completion. so maybe 
for testing i could use immediate invocation as a source for ConstantExpr 
instead of the code i added to make constexpr variables emit ConstantExpr ?


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

https://reviews.llvm.org/D63640



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


[clang] 336ac71 - [cxx_status] Fix table layout.

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T13:03:12-08:00
New Revision: 336ac7197eeeb4ab083f7402ec4edbb9b7bb252c

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

LOG: [cxx_status] Fix table layout.

Added: 


Modified: 
clang/www/cxx_status.html

Removed: 




diff  --git a/clang/www/cxx_status.html b/clang/www/cxx_status.html
index 9df44f474e72..c32ee7f50bd2 100755
--- a/clang/www/cxx_status.html
+++ b/clang/www/cxx_status.html
@@ -924,7 +924,7 @@ C++2a implementation status
   Clang 8
 
 
-  Consistent comparison (operator=)
+  Consistent comparison (operator=)
   https://wg21.link/p0515r3;>P0515R3
   Partial
 



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


[clang] 68009c2 - [c++20] Return type deduction for defaulted three-way comparisons.

2019-12-10 Thread Richard Smith via cfe-commits

Author: Richard Smith
Date: 2019-12-10T13:03:12-08:00
New Revision: 68009c245dbe4c420ca06c0fea2a908f918137bb

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

LOG: [c++20] Return type deduction for defaulted three-way comparisons.

Added: 
clang/test/CXX/class/class.compare/class.spaceship/p2.cpp

Modified: 
clang/include/clang/AST/ComparisonCategories.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/ComparisonCategories.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ComparisonCategories.h 
b/clang/include/clang/AST/ComparisonCategories.h
index 3dd1db1f7e9b..e28d49963384 100644
--- a/clang/include/clang/AST/ComparisonCategories.h
+++ b/clang/include/clang/AST/ComparisonCategories.h
@@ -50,6 +50,20 @@ enum class ComparisonCategoryType : unsigned char {
   Last = StrongOrdering
 };
 
+/// Determine the common comparison type, as defined in C++2a
+/// [class.spaceship]p4.
+inline ComparisonCategoryType commonComparisonType(ComparisonCategoryType A,
+   ComparisonCategoryType B) {
+  if ((A == ComparisonCategoryType::StrongEquality) ^
+  (B == ComparisonCategoryType::StrongEquality))
+return ComparisonCategoryType::WeakEquality;
+  return A < B ? A : B;
+}
+
+/// Get the comparison category that should be used when comparing values of
+/// type \c T.
+Optional getComparisonCategoryForBuiltinCmp(QualType 
T);
+
 /// An enumeration representing the possible results of a three-way
 /// comparison. These values map onto instances of comparison category types
 /// defined in the standard library. e.g. 'std::strong_ordering::less'.

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 8a0ff1e8a697..aeeff2b9a76e 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8200,6 +8200,9 @@ def err_defaulted_comparison_non_const : Error<
 def err_defaulted_comparison_return_type_not_bool : Error<
   "return type for defaulted %sub{select_defaulted_comparison_kind}0 "
   "must be 'bool', not %1">;
+def err_defaulted_comparison_deduced_return_type_not_auto : Error<
+  "deduced return type for defaulted %sub{select_defaulted_comparison_kind}0 "
+  "must be 'auto', not %1">;
 def warn_defaulted_comparison_deleted : Warning<
   "explicitly defaulted %sub{select_defaulted_comparison_kind}0 is implicitly "
   "deleted">, InGroup;
@@ -8227,6 +8230,12 @@ def 
note_defaulted_comparison_no_viable_function_synthesized : Note<
 def note_defaulted_comparison_not_rewritten_callee : Note<
   "defaulted %0 is implicitly deleted because this non-rewritten comparison "
   "function would be the best match for the comparison">;
+def note_defaulted_comparison_cannot_deduce : Note<
+  "return type of defaulted 'operator<=>' cannot be deduced because "
+  "return type %2 of three-way comparison for %select{|member|base class}0 %1 "
+  "is not a standard comparison category type">;
+def note_defaulted_comparison_cannot_deduce_callee : Note<
+  "selected 'operator<=>' for %select{|member|base class}0 %1 declared here">;
 def err_incorrect_defaulted_comparison_constexpr : Error<
   "defaulted definition of %sub{select_defaulted_comparison_kind}0 "
   "cannot be declared %select{constexpr|consteval}1 because it invokes "

diff  --git a/clang/lib/AST/ComparisonCategories.cpp 
b/clang/lib/AST/ComparisonCategories.cpp
index 3fb500c580e8..9a07c2494ac2 100644
--- a/clang/lib/AST/ComparisonCategories.cpp
+++ b/clang/lib/AST/ComparisonCategories.cpp
@@ -19,6 +19,41 @@
 
 using namespace clang;
 
+Optional
+clang::getComparisonCategoryForBuiltinCmp(QualType T) {
+  using CCT = ComparisonCategoryType;
+
+  if (const ComplexType *CT = T->getAs()) {
+if (CT->getElementType()->hasFloatingRepresentation())
+  return CCT::WeakEquality;
+// FIXME: Remove this, consistent with P1959R0.
+return CCT::StrongEquality;
+  }
+
+  if (T->isIntegralOrEnumerationType())
+return CCT::StrongOrdering;
+
+  if (T->hasFloatingRepresentation())
+return CCT::PartialOrdering;
+
+  // C++2a [expr.spaceship]p7: If the composite pointer type is a function
+  // pointer type, a pointer-to-member type, or std::nullptr_t, the
+  // result is of type std::strong_equality
+  if (T->isFunctionPointerType() || T->isMemberPointerType() ||
+  T->isNullPtrType())
+// FIXME: This case was removed by P1959R0.
+return CCT::StrongEquality;
+
+  // C++2a [expr.spaceship]p8: If the composite pointer type is an object
+  // pointer type, p <=> q is of type std::strong_ordering.
+  // Note: this assumes neither operand is a null pointer constant.
+  if 

[PATCH] D71166: Deprecate the hasDefaultArgument matcher

2019-12-10 Thread Stephen Kelly via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e315ba2547c: Deprecate the hasDefaultArgument matcher 
(authored by stephenkelly).

Changed prior to commit:
  https://reviews.llvm.org/D71166?vs=232711=233177#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71166

Files:
  clang/include/clang/ASTMatchers/ASTMatchers.h


Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6544,6 +6544,20 @@
 /// void x(int val) {}
 /// void y(int val = 0) {}
 /// \endcode
+///
+/// Deprecated. Use hasInitializer() instead to be able to
+/// match on the contents of the default argument.  For example:
+///
+/// \code
+/// void x(int val = 7) {}
+/// void y(int val = 42) {}
+/// \endcode
+/// parmVarDecl(hasInitializer(integerLiteral(equals(42
+///   matches the parameter of y
+///
+/// A matcher such as
+///   parmVarDecl(hasInitializer(anything()))
+/// is equivalent to parmVarDecl(hasDefaultArgument()).
 AST_MATCHER(ParmVarDecl, hasDefaultArgument) {
   return Node.hasDefaultArg();
 }


Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6544,6 +6544,20 @@
 /// void x(int val) {}
 /// void y(int val = 0) {}
 /// \endcode
+///
+/// Deprecated. Use hasInitializer() instead to be able to
+/// match on the contents of the default argument.  For example:
+///
+/// \code
+/// void x(int val = 7) {}
+/// void y(int val = 42) {}
+/// \endcode
+/// parmVarDecl(hasInitializer(integerLiteral(equals(42
+///   matches the parameter of y
+///
+/// A matcher such as
+///   parmVarDecl(hasInitializer(anything()))
+/// is equivalent to parmVarDecl(hasDefaultArgument()).
 AST_MATCHER(ParmVarDecl, hasDefaultArgument) {
   return Node.hasDefaultArg();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 3e315ba - Deprecate the hasDefaultArgument matcher

2019-12-10 Thread Stephen Kelly via cfe-commits

Author: Stephen Kelly
Date: 2019-12-10T20:48:45Z
New Revision: 3e315ba2547cbbfd6055e38bbca03f4d11bacbea

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

LOG: Deprecate the hasDefaultArgument matcher

Summary:
It doesn't provide a way to match on the contents of the default
argumment.  Rather than give it that capability, make it deprecated and
recomment the use of hasInitializer instead.

Reviewers: aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 9c0aae2886fc..c3399db95b89 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -6544,6 +6544,20 @@ AST_MATCHER(NamedDecl, hasExternalFormalLinkage) {
 /// void x(int val) {}
 /// void y(int val = 0) {}
 /// \endcode
+///
+/// Deprecated. Use hasInitializer() instead to be able to
+/// match on the contents of the default argument.  For example:
+///
+/// \code
+/// void x(int val = 7) {}
+/// void y(int val = 42) {}
+/// \endcode
+/// parmVarDecl(hasInitializer(integerLiteral(equals(42
+///   matches the parameter of y
+///
+/// A matcher such as
+///   parmVarDecl(hasInitializer(anything()))
+/// is equivalent to parmVarDecl(hasDefaultArgument()).
 AST_MATCHER(ParmVarDecl, hasDefaultArgument) {
   return Node.hasDefaultArg();
 }



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


[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

2019-12-10 Thread Artem Belevich via Phabricator via cfe-commits
tra added a comment.

I wonder if this patch will help with this case:

https://godbolt.org/z/X4KdsV

  __device__ float fn(int) { return threadIdx.x; };
  __host__ float fn(float);
  
  float gvar1 = []()__device__ { return fn(1);} (); // This ends up calling 
fn(int) on *host*

We seem to happily let host code call __device__ function from a lambda 
function used as an initializer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227



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


[clang] ef94cd1 - [OPENMP50]Add if clause in target simd directive.

2019-12-10 Thread Alexey Bataev via cfe-commits

Author: Alexey Bataev
Date: 2019-12-10T15:33:18-05:00
New Revision: ef94cd1cf81d6e66838e1d7395fdaa6795ec7165

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

LOG: [OPENMP50]Add if clause in target simd directive.

According to OpenMP 5.0, if clause can be used in for simd directive. If
condition in the if clause if false, the non-vectorized version of the
loop must be executed.

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/target_simd_ast_print.cpp
clang/test/OpenMP/target_simd_codegen.cpp
clang/test/OpenMP/target_simd_if_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 1bb08b9bca48..3eaad2fd1536 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -4731,6 +4731,8 @@ StmtResult Sema::ActOnOpenMPExecutableDirective(
 Res = ActOnOpenMPTargetSimdDirective(ClausesWithImplicit, AStmt, StartLoc,
  EndLoc, VarsWithInheritedDSA);
 AllowedNameModifiers.push_back(OMPD_target);
+if (LangOpts.OpenMP >= 50)
+  AllowedNameModifiers.push_back(OMPD_simd);
 break;
   case OMPD_teams_distribute:
 Res = ActOnOpenMPTeamsDistributeDirective(
@@ -10761,13 +10763,17 @@ static OpenMPDirectiveKind 
getOpenMPCaptureRegionForClause(
   if (NameModifier == OMPD_unknown || NameModifier == OMPD_simd)
 CaptureRegion = OMPD_parallel;
   break;
+case OMPD_target_simd:
+  if (OpenMPVersion >= 50 &&
+  (NameModifier == OMPD_unknown || NameModifier == OMPD_simd))
+CaptureRegion = OMPD_target;
+  break;
 case OMPD_cancel:
 case OMPD_parallel:
 case OMPD_parallel_master:
 case OMPD_parallel_sections:
 case OMPD_parallel_for:
 case OMPD_target:
-case OMPD_target_simd:
 case OMPD_target_teams:
 case OMPD_target_teams_distribute:
 case OMPD_target_teams_distribute_simd:

diff  --git a/clang/test/OpenMP/target_simd_ast_print.cpp 
b/clang/test/OpenMP/target_simd_ast_print.cpp
index 8be9866cc27c..e754b957cfd6 100644
--- a/clang/test/OpenMP/target_simd_ast_print.cpp
+++ b/clang/test/OpenMP/target_simd_ast_print.cpp
@@ -1,10 +1,16 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -ast-print %s 
-Wno-openmp-mapping | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=45 -ast-print %s 
-Wno-openmp-mapping | FileCheck %s --check-prefix=CHECK --check-prefix=OMP45
 // RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -x c++ -std=c++11 -emit-pch -o 
%t %s -Wno-openmp-mapping
-// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -std=c++11 -include-pch %t 
-fsyntax-only -verify %s -ast-print -Wno-openmp-mapping | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=45 -std=c++11 -include-pch %t 
-fsyntax-only -verify %s -ast-print -Wno-openmp-mapping | FileCheck %s 
--check-prefix=CHECK --check-prefix=OMP45
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -DOMP5 -ast-print %s 
-Wno-openmp-mapping | FileCheck %s --check-prefix=CHECK --check-prefix=OMP50
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -DOMP5 -x c++ -std=c++11 
-emit-pch -o %t %s -Wno-openmp-mapping
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -DOMP5 -std=c++11 -include-pch 
%t -fsyntax-only -verify %s -ast-print -Wno-openmp-mapping | FileCheck %s 
--check-prefix=CHECK --check-prefix=OMP50
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=45 -ast-print %s 
-Wno-openmp-mapping | FileCheck %s
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=45 -ast-print %s 
-Wno-openmp-mapping | FileCheck %s --check-prefix=CHECK --check-prefix=OMP45
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -x c++ -std=c++11 
-emit-pch -o %t %s -Wno-openmp-mapping
-// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch 
%t -fsyntax-only -verify %s -ast-print -Wno-openmp-mapping | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=45 -std=c++11 -include-pch 
%t -fsyntax-only -verify %s -ast-print -Wno-openmp-mapping | FileCheck %s 
--check-prefix=CHECK --check-prefix=OMP45
+// RUN: %clang_cc1 -verify -fopenmp-simd -fopenmp-version=50 -DOMP5 -ast-print 
%s -Wno-openmp-mapping | FileCheck %s --check-prefix=CHECK --check-prefix=OMP50
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -DOMP5 -x c++ -std=c++11 
-emit-pch -o %t %s -Wno-openmp-mapping
+// RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -DOMP5 -std=c++11 
-include-pch %t -fsyntax-only -verify %s -ast-print -Wno-openmp-mapping | 
FileCheck %s --check-prefix=CHECK --check-prefix=OMP50
 // expected-no-diagnostics
 
 #ifndef HEADER
@@ -109,9 +115,14 @@ T tmain(T argc, T *argv) {
   // CHECK-NEXT: for (T i = 0; i < 2; ++i) {
   // CHECK-NEXT: }
 
+#ifdef OMP5
+#pragma 

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

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

In D71241#1777972 , @jdoerfert wrote:

> In D71241#109 , @ABataev wrote:
>
> > In D71241#1777661 , @jdoerfert 
> > wrote:
> >
> > > In D71241#1776798 , @ABataev 
> > > wrote:
> > >
> > > > You're merging different functions as multiversiin  variants. I don't 
> > > > think this right to overcomplicate the semantics of multiversion 
> > > > functions just because you want to do it.
> > >
> > >
> > > I am actually not doing that here.
> >
> >
> > You do this when tries to resolve the overloading though it is absolutely 
> > not required. You can easily implement it at the codegen phase (it is 
> > implemented already, actually). Because you don't need to resolve the 
> > overloads, it is resolved already by sema. hat you need to do is to select 
> > the correct version of the function and that's it. If you have global 
> > traits only, you emit alias. If you have local traits (like construct), you 
> > use the address of the best variant function directly. And no need to worry 
> > about templates, overloading resolution etc. Plus handling for the corner 
> > cases and future changes.
> >
> > In your solution, you're actually not using mutiversioning at all, you use 
> > just one feature from the multiversioning - handling of multiple 
> > definitions of the same function. Nothing else. I'm saying that it is 
> > better to modify slightly the codegen because there you have to deal with 
> > the C-like constrcuts, where you don't need to worry about most of the 
> > problematic c++ features. But you insist on moving of all this stuff to 
> > Sema and overcomplicate the things.
>
>
> There is no evidence that this is more complicated. By all measures, this is 
> less complicated (see also below). It is also actually doing the right thing 
> when it comes to code emission. Take https://godbolt.org/z/sJiP3B for 
> example. The calls are wrong and the definition of base is missing.


How did you measure it? I have a completely different opinion. Also, tried to 
reproduce the problem locally, could not reproduce. It seems to me, the output 
of the test misses several important things. You can check it yourself. 
`tgt_target_teams()` call uses `@.offload_maptypes` global var but it is not 
defined.

> 
> 
>>> What over complication do you mean exactly? Especially because this patch 
>>> does not touch multi-version functions at all I'm confused by your comment.
>> 
>> Handling of templates, for example. Plus, mixing different functions (with 
>> different names). You have it when you try to resolve overloadings though, 
>> actually, we don't need to do it, we can easily do this at the codegen.
> 
> Why is any of this complicated to you? The logic to do the overloading is 15 
> lines long and most of it is to determine the best of all versions. What 
> about that is more complicated than having multiple patch points during code 
> generation in which we try to modify existing IR but sometimes have to delay 
> it and hope it'll work at the end.

Without templates etc. Early resolution of the variant function leads to 
problems with AST at least. Plus, as I said, problems with handling complex 
features of C++.

> 
> 
>> Also, check how -ast-print works with your solution. It returns different 
>> result than expected because you're transform the code too early. It is 
>> incorrect behavior.
> 
> This is debatable. AST print does not print the input but the AST, thus what 
> is correct wrt. OpenMP declare variant is nowhere defined but by us.
>  Arguably, having it print the actually called function and not the base 
> function is preferable. Thus, the new way is actually more informative.

You're completely wrong here! We shall keep the original AST. This is used in 
several tools and you significantly modify the user code. I consider it a real 
issue here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71241



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


RE: [clang] d82b6ba - Revert "[DWARF5] Start emitting DW_AT_dwo_name when -gdwarf-5 is specified."

2019-12-10 Thread Tomar, Sourabh Singh via cfe-commits
[AMD Official Use Only - Internal Distribution Only]

Thank You! Eric for clarifying this. I wasn't sure about this.
I don't know why my git script missed that string in commit message, and 
allowed me to commit it.
Any way thanks again.

From: Eric Christopher 
Sent: Wednesday, December 11, 2019 1:24 AM
To: Tomar, Sourabh Singh ; Sourabh Singh Tomar 

Cc: Clang Commits 
Subject: Re: [clang] d82b6ba - Revert "[DWARF5] Start emitting DW_AT_dwo_name 
when -gdwarf-5 is specified."

[CAUTION: External Email]
You don't need to revert for a missing differential revision, just close the 
revision yourself :)

-eric

On Tue, Dec 10, 2019 at 11:51 AM Sourabh Singh Tomar via cfe-commits 
mailto:cfe-commits@lists.llvm.org>> wrote:

Author: Sourabh Singh Tomar
Date: 2019-12-11T01:20:40+05:30
New Revision: d82b6ba21b32ddf00af886b9160feef88211773e

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

LOG: Revert "[DWARF5] Start emitting DW_AT_dwo_name when -gdwarf-5 is 
specified."

This reverts commit 6ef01588f4d75ef43da4ed2a37ba7a8b8daab259.
Missing Differetial revision.

Added:


Modified:
clang/test/CodeGen/split-debug-output.c
clang/test/CodeGen/thinlto-split-dwarf.c
llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
llvm/test/DebugInfo/X86/debug_addr.ll
llvm/test/DebugInfo/X86/string-offsets-table.ll

Removed:




diff  --git a/clang/test/CodeGen/split-debug-output.c 
b/clang/test/CodeGen/split-debug-output.c
index 19569f8d574b..1507edd24849 100644
--- a/clang/test/CodeGen/split-debug-output.c
+++ b/clang/test/CodeGen/split-debug-output.c
@@ -1,11 +1,7 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -debug-info-kind=limited 
-dwarf-version=4 -split-dwarf-file foo.dwo -split-dwarf-output %t -emit-obj -o 
- %s | llvm-dwarfdump -debug-info - | FileCheck --check-prefix=DWARFv4 %s
-// RUN: llvm-dwarfdump -debug-info %t | FileCheck --check-prefix=DWARFv4 %s
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux -debug-info-kind=limited 
-dwarf-version=5 -split-dwarf-file foo.dwo -split-dwarf-output %t -emit-obj -o 
- %s | llvm-dwarfdump -debug-info - | FileCheck --check-prefix=DWARFv5 %s
-// RUN: llvm-dwarfdump -debug-info %t | FileCheck --check-prefix=DWARFv5 %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -debug-info-kind=limited 
-split-dwarf-file foo.dwo -split-dwarf-output %t -emit-obj -o - %s | 
llvm-dwarfdump -debug-info - | FileCheck %s
+// RUN: llvm-dwarfdump -debug-info %t | FileCheck %s

 int f() { return 0; }

-// DWARFv4: DW_AT_GNU_dwo_name ("foo.dwo")
-// DWARFv5: DW_AT_dwo_name ("foo.dwo")
+// CHECK: DW_AT_GNU_dwo_name ("foo.dwo")

diff  --git a/clang/test/CodeGen/thinlto-split-dwarf.c 
b/clang/test/CodeGen/thinlto-split-dwarf.c
index 419bd1320bb2..60649b0881bb 100644
--- a/clang/test/CodeGen/thinlto-split-dwarf.c
+++ b/clang/test/CodeGen/thinlto-split-dwarf.c
@@ -13,31 +13,11 @@
 // RUN:   -o %t.native.o -split-dwarf-file %t.file.dwo \
 // RUN:   -split-dwarf-output %t.output.dwo -x ir %t.o

-// RUN: llvm-dwarfdump %t.native.o | FileCheck --check-prefix=DWARFv4-O %s
-// RUN: llvm-dwarfdump %t.output.dwo | FileCheck --check-prefix=DWARFv4-DWO %s
+// RUN: llvm-dwarfdump %t.native.o | FileCheck --check-prefix=O %s
+// RUN: llvm-dwarfdump %t.output.dwo | FileCheck --check-prefix=DWO %s

-// DWARFv4-O: DW_AT_GNU_dwo_name ("{{.*}}.file.dwo")
-// DWARFv4-O-NOT: DW_TAG_subprogram
-// DWARFv4-DWO: DW_TAG_subprogram
-
-// RUN: %clang_cc1 -debug-info-kind=limited -dwarf-version=5 -triple 
x86_64-unknown-linux-gnu \
-// RUN:   -flto=thin -emit-llvm-bc \
-// RUN:   -o %t.o %s
-
-// RUN: llvm-lto2 run -thinlto-distributed-indexes %t.o \
-// RUN:   -o %t2.index \
-// RUN:   -r=%t.o,main,px
-
-// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu \
-// RUN:   -emit-obj -fthinlto-index=%t.o.thinlto.bc \
-// RUN:   -o %t.native.o -dwarf-version=5 -split-dwarf-file %t.file.dwo \
-// RUN:   -split-dwarf-output %t.output.dwo -x ir %t.o
-
-// RUN: llvm-dwarfdump %t.native.o | FileCheck --check-prefix=DWARFv5-O %s
-// RUN: llvm-dwarfdump 

[PATCH] D71227: [cuda][hip] Fix function overload resolution in the global initiailizer.

2019-12-10 Thread Artem Belevich via Phabricator via cfe-commits
tra added a subscriber: rsmith.
tra added a comment.

Looks good to me overall. I've pinged rsmith@ to double-check that we're 
covering all possibilities for non-local variable init.




Comment at: clang/include/clang/Sema/Sema.h:11037
+bool IgnoreImplicitHDAttr = false);
+  CUDAFunctionTarget IdentifyCUDATarget(const Decl *D,
+bool IgnoreImplicitHDAttr = false);

I'd add a comment describing that it's a wrapper which dispatches the call to 
one of more specific variants above.



Comment at: clang/include/clang/Sema/Sema.h:11061
   /// combination, based on their host/device attributes.
   /// \param Caller function which needs address of \p Callee.
   ///   nullptr in case of global context.

Comment needs updating.



Comment at: clang/include/clang/Sema/Sema.h:11073
+  void popCUDANonLocalVariable(const Decl *D);
+  const Decl *getCUDACurrentNonLocalVariable() const {
+return CUDANonLocalVariableStack.empty() ? nullptr

Nit: I'd add an empty line between delarations and the function. Jammed 
together they are hard to read.



Comment at: clang/include/clang/Sema/Sema.h:11096-11098
+const FunctionDecl *Caller = dyn_cast(CurContext);
+if (Kind == SkipImplicitCaller && Caller && Caller->isImplicit())
+  return true;

```
if (const FunctionDecl *Caller = dyn_cast(CurContext))
  if (Kind == SkipImplicitCaller && Caller->isImplicit())
  return true;
```



Comment at: clang/include/clang/Sema/Sema.h:11136
   void EraseUnwantedCUDAMatches(
-  const FunctionDecl *Caller,
+  const Decl *ContextDecl,
   SmallVectorImpl> );

Now that we always use getCUDAContextDecl() as the first argument, perhaps we 
can just always retrieve the context inside the function.



Comment at: clang/lib/Parse/ParseDecl.cpp:2336
 
+  Actions.pushCUDANonLocalVariable(ThisDecl);
+

@rsmith -- is this sufficient to catch all attempts to call an initializer for 
a global?
I wonder if there are other sneaky ways to call an initializer. 



Comment at: clang/test/SemaCUDA/function-overload.cu:428
+
+// Overload resolution should follow the same rule in the global
+// initialization.

I'd add more details here.
The problem is that here the overload set has both functions and the one with 
the integer argument wins, even though it's a __device__ function which we 
can't execute. We do handle similar cases during overload resolution in other 
places where we would prefer a callable function over a non-callable function 
with a better signature match.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71227



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


[PATCH] D71141: [Wdocumentation] Use C2x/C++14 deprecated attribute

2019-12-10 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Mordante marked 2 inline comments as done.
Closed by commit rGb6d386f6f996: [Wdocumentation] Use C2x/C++14 deprecated 
attribute (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D71141?vs=232695=233170#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71141

Files:
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation-fixits.c
  clang/test/Sema/warn-documentation-fixits.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
@@ -612,6 +612,12 @@
 /// \deprecated Bbb
 void test_deprecated_1(int a) __attribute__((deprecated));
 
+#if __cplusplus >= 201402L
+/// Aaa
+/// \deprecated Bbb
+[[deprecated]] void test_deprecated_no_warning_std14(int a);
+#endif
+
 // We don't want \deprecated to warn about empty paragraph.  It is fine to use
 // \deprecated by itself without explanations.
 
Index: clang/test/Sema/warn-documentation-fixits.cpp
===
--- clang/test/Sema/warn-documentation-fixits.cpp
+++ clang/test/Sema/warn-documentation-fixits.cpp
@@ -1,6 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -verify %s
-// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
-// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 %s
+// RUN  %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck -DATTRIBUTE="__attribute__((deprecated))" %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 -DATTRIBUTE="[[deprecated]]" %s
 
 // expected-warning@+1 {{parameter 'ZZ' not found in the function declaration}} expected-note@+1 {{did you mean 'a'?}}
 /// \param ZZ Blah blah.
@@ -96,6 +96,14 @@
 /// \deprecated
 void test_deprecated_9(int a);
 
+#if __cplusplus >= 201402L
+#define ATTRIBUTE_DEPRECATED [[deprecated]]
+
+// expected-warning@+1 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+/// \deprecated
+void test_deprecated_10(int a);
+#endif
+
 // rdar://12381408
 // expected-warning@+2  {{unknown command tag name 'retur'; did you mean 'return'?}}
 /// \brief testing fixit
@@ -119,16 +127,17 @@
 // CHECK: fix-it:"{{.*}}":{10:12-10:15}:"aaa"
 // CHECK: fix-it:"{{.*}}":{14:13-14:23}:"T"
 // CHECK: fix-it:"{{.*}}":{19:13-19:18}:"SomeTy"
-// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"__attribute__((deprecated)) "
-// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"__attribute__((deprecated)) "
-// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"__attribute__((deprecated)) "
+// CHECK: fix-it:"{{.*}}":{26:1-26:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{30:1-30:1}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{35:3-35:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{39:3-39:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{47:3-47:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{51:3-51:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{76:3-76:3}:"[[ATTRIBUTE]] "
+// CHECK: fix-it:"{{.*}}":{81:3-81:3}:"[[ATTRIBUTE]] "
+// CHECK14: fix-it:"{{.*}}":{87:3-87:3}:"[[ATTRIBUTE]] "
 // CHECK: fix-it:"{{.*}}":{97:1-97:1}:"MY_ATTR_DEPRECATED "
-// CHECK: fix-it:"{{.*}}":{102:6-102:11}:"return"
-// CHECK: fix-it:"{{.*}}":{106:6-106:11}:"foobar"
-// CHECK: fix-it:"{{.*}}":{115:6-115:12}:"endcode"
+// CHECK14: fix-it:"{{.*}}":{104:1-104:1}:"ATTRIBUTE_DEPRECATED "
+// CHECK: fix-it:"{{.*}}":{110:6-110:11}:"return"
+// CHECK: fix-it:"{{.*}}":{114:6-114:11}:"foobar"
+// CHECK: fix-it:"{{.*}}":{123:6-123:12}:"endcode"
Index: clang/test/Sema/warn-documentation-fixits.c
===
--- /dev/null
+++ clang/test/Sema/warn-documentation-fixits.c
@@ 

[PATCH] D71139: [Wdocumentation] Use the command marker

2019-12-10 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85fff898bb31: [Wdocumentation] Use the command marker. 
(authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71139

Files:
  clang/include/clang/Basic/DiagnosticCommentKinds.td
  clang/lib/AST/CommentSema.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
@@ -631,9 +631,9 @@
 /// \deprecated
 void test_deprecated_5(int a);
 
-// expected-warning@+2 {{declaration is marked with '\deprecated' command but 
does not have a deprecation attribute}} expected-note@+3 {{add a deprecation 
attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '@deprecated' command but 
does not have a deprecation attribute}} expected-note@+3 {{add a deprecation 
attribute to the declaration to silence this warning}}
 /// Aaa
-/// \deprecated
+/// @deprecated
 void test_deprecated_6(int a) {
 }
 
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -676,9 +676,8 @@
   D->hasAttr())
 return;
 
-  Diag(Command->getLocation(),
-   diag::warn_doc_deprecated_not_sync)
-<< Command->getSourceRange();
+  Diag(Command->getLocation(), diag::warn_doc_deprecated_not_sync)
+  << Command->getSourceRange() << Command->getCommandMarker();
 
   // Try to emit a fixit with a deprecation attribute.
   if (const FunctionDecl *FD = dyn_cast(D)) {
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -146,8 +146,8 @@
 // \deprecated command
 
 def warn_doc_deprecated_not_sync : Warning<
-  "declaration is marked with '\\deprecated' command but does not have "
-  "a deprecation attribute">,
+  "declaration is marked with '%select{\\|@}0deprecated' command but does "
+  "not have a deprecation attribute">,
   InGroup, DefaultIgnore;
 
 def note_add_deprecation_attr : Note<


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -631,9 +631,9 @@
 /// \deprecated
 void test_deprecated_5(int a);
 
-// expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '@deprecated' command but does not have a deprecation attribute}} expected-note@+3 {{add a deprecation attribute to the declaration to silence this warning}}
 /// Aaa
-/// \deprecated
+/// @deprecated
 void test_deprecated_6(int a) {
 }
 
Index: clang/lib/AST/CommentSema.cpp
===
--- clang/lib/AST/CommentSema.cpp
+++ clang/lib/AST/CommentSema.cpp
@@ -676,9 +676,8 @@
   D->hasAttr())
 return;
 
-  Diag(Command->getLocation(),
-   diag::warn_doc_deprecated_not_sync)
-<< Command->getSourceRange();
+  Diag(Command->getLocation(), diag::warn_doc_deprecated_not_sync)
+  << Command->getSourceRange() << Command->getCommandMarker();
 
   // Try to emit a fixit with a deprecation attribute.
   if (const FunctionDecl *FD = dyn_cast(D)) {
Index: clang/include/clang/Basic/DiagnosticCommentKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -146,8 +146,8 @@
 // \deprecated command
 
 def warn_doc_deprecated_not_sync : Warning<
-  "declaration is marked with '\\deprecated' command but does not have "
-  "a deprecation attribute">,
+  "declaration is marked with '%select{\\|@}0deprecated' command but does "
+  "not have a deprecation attribute">,
   InGroup, DefaultIgnore;
 
 def note_add_deprecation_attr : Note<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71140: [Wdocumentation] Properly place deprecated attribute

2019-12-10 Thread Mark de Wever via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb972f2d05e8b: [Wdocumentation] Properly place deprecated 
attribute (authored by Mordante).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71140

Files:
  clang/lib/AST/CommentSema.cpp
  clang/test/Sema/warn-documentation-fixits.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
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -verify %s
 
 // This file contains lots of corner cases, so ensure that XML we generate is not invalid.
 // RUN: c-index-test -test-load-source all -comments-xml-schema=%S/../../bindings/xml/comment-xml-schema.rng %s | FileCheck %s -check-prefix=WRONG
@@ -643,6 +644,44 @@
 template
 void test_deprecated_7(T aaa);
 
+class PR43753 {
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static void test_deprecated_static();
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static auto test_deprecated_static_trailing_return() -> int;
+
+#if __cplusplus >= 201402L
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static decltype(auto) test_deprecated_static_decltype_auto() { return 1; }
+#endif
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  void test_deprecated_const() const;
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  auto test_deprecated_trailing_return() -> int;
+
+#if __cplusplus >= 201402L
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  decltype(auto) test_deprecated_decltype_auto() const { return a; }
+
+private:
+  int a{0};
+#endif
+};
 
 // rdar://12397511
 // expected-note@+2 {{previous command '\headerfile' here}}
Index: clang/test/Sema/warn-documentation-fixits.cpp
===
--- clang/test/Sema/warn-documentation-fixits.cpp
+++ clang/test/Sema/warn-documentation-fixits.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -Wdocumentation -Wdocumentation-pedantic -fcomment-block-commands=foobar -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck --check-prefixes=CHECK,CHECK14 %s
 
 // expected-warning@+1 {{parameter 'ZZ' not found in the function declaration}} expected-note@+1 {{did you mean 'a'?}}
 /// \param ZZ Blah blah.
@@ -51,6 +52,44 @@
   }
 };
 
+class PR43753 {
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static void test_deprecated_static();
+
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // expected-note@+2 {{add a deprecation attribute to the declaration to silence this warning}}
+  /// \deprecated
+  static auto test_deprecated_static_trailing_return() -> int;
+
+#if __cplusplus >= 201402L
+  // expected-warning@+2 {{declaration is marked with '\deprecated' command but does not have a deprecation attribute}}
+  // 

[PATCH] D71294: [OpenMP] Emit warning message about undefined number of evaluation for atomic update/capture

2019-12-10 Thread Chi Chun Chen via Phabricator via cfe-commits
cchen planned changes to this revision.
cchen added a comment.

Don't know there is an analysis phase in Clang, I'll investigate later, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71294



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


[PATCH] D71224: [analyzer] Escape symbols stored into specific region after a conservative evalcall.

2019-12-10 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:624
+if (const MemRegion *MR = Call.getArgSVal(Arg).getAsRegion())
+  if (!MR->hasStackStorage())
+Escaped.push_back(State->getSVal(MR, Pointee));

xazax.hun wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > Ok, so this patch interacts with D71152 in a non-trivial manner. We 
> > > should re-use the logic that decides whether an escape on bind occurs.
> > Yeah, I just started to look into it and some skeletons fell out. 
> > 
> > Let's consider the following example:
> > ```
> > void leak() {
> >   zx_handle_t sa, sb;
> >   if (zx_channel_create(0, , ))
> > return;
> >   zx_handle_close(sb);
> > } 
> > ```
> > 
> > After using the same logic we can no longer detect the leak. The reason is, 
> > after `zx_channel_create` both `sa` and `sb` regions are escaped (and they 
> > become escaped during the conservative call) and then after the PostCall 
> > callback, when we attached the traits to our symbols, we will trigger 
> > pointer escape immediately, and lose the traits we just added.
> > 
> > I am not immediately sure what to do here. I feel like the main source of 
> > the problem is the fact that `zx_channel_create` should not escape anything 
> > and the checker has now way to communicate that to the analyzer core. Any 
> > ideas?
> Suddenly, I see four ways forward, none of which is optimal. Hopefully, you 
> can come up with something better.
> 1. Do not care about escaped local regions in this code path. So basically we 
> would tune the invalidation down a bit. 
> 2. Make it possible for modeling checkers to prevent escaping. It would be 
> great if there would be a way to do that without doing EvalCall.
> 3. Require the users to annotate out parameters. And we could assume that out 
> parameters do not escape.
> 4. Do some heuristics like if a checker just attached some info in a 
> PostCall, it is likely that we should not escape that symbol.
Or:
5. Since output arguments might be more common than escapes in practice, maybe 
we could just not escape local regions at all by default only if there is a 
special annotation or a modelling check asks for it. So users would get one 
more tool to suppress false positives but we would not overdo invalidation by 
default.  


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

https://reviews.llvm.org/D71224



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


[clang] 85fff89 - [Wdocumentation] Use the command marker.

2019-12-10 Thread Mark de Wever via cfe-commits

Author: Mark de Wever
Date: 2019-12-10T21:16:07+01:00
New Revision: 85fff898bb3175693e12536a837c8ad0ec8b2cdd

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

LOG: [Wdocumentation] Use the command marker.

Use the proper marker for -Wdocumentation-deprecated-sync instead of
hard-coded the backslash.

Discovered while looking at https://bugs.llvm.org/show_bug.cgi?id=43753

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticCommentKinds.td
clang/lib/AST/CommentSema.cpp
clang/test/Sema/warn-documentation.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticCommentKinds.td 
b/clang/include/clang/Basic/DiagnosticCommentKinds.td
index c577ac408539..ae63bb623ed3 100644
--- a/clang/include/clang/Basic/DiagnosticCommentKinds.td
+++ b/clang/include/clang/Basic/DiagnosticCommentKinds.td
@@ -146,8 +146,8 @@ def warn_doc_returns_attached_to_a_void_function : Warning<
 // \deprecated command
 
 def warn_doc_deprecated_not_sync : Warning<
-  "declaration is marked with '\\deprecated' command but does not have "
-  "a deprecation attribute">,
+  "declaration is marked with '%select{\\|@}0deprecated' command but does "
+  "not have a deprecation attribute">,
   InGroup, DefaultIgnore;
 
 def note_add_deprecation_attr : Note<

diff  --git a/clang/lib/AST/CommentSema.cpp b/clang/lib/AST/CommentSema.cpp
index 69d61dc55162..bef555d3ebe7 100644
--- a/clang/lib/AST/CommentSema.cpp
+++ b/clang/lib/AST/CommentSema.cpp
@@ -676,9 +676,8 @@ void Sema::checkDeprecatedCommand(const BlockCommandComment 
*Command) {
   D->hasAttr())
 return;
 
-  Diag(Command->getLocation(),
-   diag::warn_doc_deprecated_not_sync)
-<< Command->getSourceRange();
+  Diag(Command->getLocation(), diag::warn_doc_deprecated_not_sync)
+  << Command->getSourceRange() << Command->getCommandMarker();
 
   // Try to emit a fixit with a deprecation attribute.
   if (const FunctionDecl *FD = dyn_cast(D)) {

diff  --git a/clang/test/Sema/warn-documentation.cpp 
b/clang/test/Sema/warn-documentation.cpp
index 93273559a905..3d23acc1bebf 100644
--- a/clang/test/Sema/warn-documentation.cpp
+++ b/clang/test/Sema/warn-documentation.cpp
@@ -631,9 +631,9 @@ void test_deprecated_4(int a) __attribute__((unavailable));
 /// \deprecated
 void test_deprecated_5(int a);
 
-// expected-warning@+2 {{declaration is marked with '\deprecated' command but 
does not have a deprecation attribute}} expected-note@+3 {{add a deprecation 
attribute to the declaration to silence this warning}}
+// expected-warning@+2 {{declaration is marked with '@deprecated' command but 
does not have a deprecation attribute}} expected-note@+3 {{add a deprecation 
attribute to the declaration to silence this warning}}
 /// Aaa
-/// \deprecated
+/// @deprecated
 void test_deprecated_6(int a) {
 }
 



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


  1   2   3   >