[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

[Anna Zaks via Phabricator via cfe-commits]

zaks.anna added inline comments.



Comment at: lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:651-652
+  } else if (StoreSite->getLocation().getAs()) {
+os << "Reach the max loop limit.";
+os << " Assigning a conjured symbol";
+if (R->canPrintPretty()) {

MTC wrote:
> NoQ wrote:
> > This is user-facing text, and users shouldn't know about conjured symbols, 
> > and "max" shouldn't be shortened, and i'm not sure what else. I'd probably 
> > suggest something along the lines of "Contents of <...> are wiped", but 
> > this is still not good enough.
> > 
> > Also could you add a test that displays this note? I.e. with 
> > `-analyzer-output=text`.
> Thanks for your review. 
> 
> You are right, whether this information should be displayed to the user is a 
> question worth discussing.
I am not convinced that we need to print this information to the user. The 
problem here is that it leaks internal implementation details about the 
analyzer. The users should not know about "loop limits" and "invalidation" and 
most of the users would not even know what this means. I can see how this is 
useful to the analyzer developers for debugging the analyzer, but not to the 
end user.



https://reviews.llvm.org/D37187



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In https://reviews.llvm.org/D38680#901892, @dtzWill wrote:

> Wow, I've finally debugged some unwind failures to this on x86_64 Linux ...
>
> 1. Call-frame optimization introduces it
> 2. This fixes the cases I was investigating!!


That's good to hear! FWIW, I've talked to @joerg off-list and he said he'll try 
to look into it soon.


https://reviews.llvm.org/D38680



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

r316204 - Basic: restore {,u}intptr_t on NetBSD/ARM

[Saleem Abdulrasool via cfe-commits]

Author: compnerd
Date: Thu Oct 19 21:11:28 2017
New Revision: 316204

URL: http://llvm.org/viewvc/llvm-project?rev=316204&view=rev
Log:
Basic: restore {,u}intptr_t on NetBSD/ARM

NetBSD uses `long int` for `intptr_t` on ARM.  This was changed in SVN
r316046, referenced against other compilers.  However, NetBSD's
reference was incorrect as the current clang behaviour is more
up-to-date.  Restore the original behaviour for that target.

Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/test/Preprocessor/init.c

Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=316204&r1=316203&r2=316204&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Thu Oct 19 21:11:28 2017
@@ -236,9 +236,10 @@ ARMTargetInfo::ARMTargetInfo(const llvm:
 break;
   }
 
-  IntPtrType = (Triple.isOSDarwin() || Triple.getOS() == llvm::Triple::OpenBSD)
-   ? SignedLong
-   : SignedInt;
+  bool IsOpenBSD = Triple.getOS() == llvm::Triple::OpenBSD;
+  bool IsNetBSD = Triple.getOS() == llvm::Triple::NetBSD;
+  IntPtrType =
+  (Triple.isOSDarwin() || IsOpenBSD || IsNetBSD) ? SignedLong : SignedInt;
 
   // Cache arch related info.
   setArchInfo();

Modified: cfe/trunk/test/Preprocessor/init.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/init.c?rev=316204&r1=316203&r2=316204&view=diff
==
--- cfe/trunk/test/Preprocessor/init.c (original)
+++ cfe/trunk/test/Preprocessor/init.c Thu Oct 19 21:11:28 2017
@@ -2446,10 +2446,10 @@
 // ARM-NETBSD:#define __INTMAX_MAX__ 9223372036854775807LL
 // ARM-NETBSD:#define __INTMAX_TYPE__ long long int
 // ARM-NETBSD:#define __INTMAX_WIDTH__ 64
-// ARM-NETBSD:#define __INTPTR_FMTd__ "d"
-// ARM-NETBSD:#define __INTPTR_FMTi__ "i"
-// ARM-NETBSD:#define __INTPTR_MAX__ 2147483647
-// ARM-NETBSD:#define __INTPTR_TYPE__ int
+// ARM-NETBSD:#define __INTPTR_FMTd__ "ld"
+// ARM-NETBSD:#define __INTPTR_FMTi__ "li"
+// ARM-NETBSD:#define __INTPTR_MAX__ 2147483647L
+// ARM-NETBSD:#define __INTPTR_TYPE__ long int
 // ARM-NETBSD:#define __INTPTR_WIDTH__ 32
 // ARM-NETBSD:#define __INT_FAST16_FMTd__ "hd"
 // ARM-NETBSD:#define __INT_FAST16_FMTi__ "hi"
@@ -2541,8 +2541,8 @@
 // ARM-NETBSD:#define __UINTMAX_MAX__ 18446744073709551615ULL
 // ARM-NETBSD:#define __UINTMAX_TYPE__ long long unsigned int
 // ARM-NETBSD:#define __UINTMAX_WIDTH__ 64
-// ARM-NETBSD:#define __UINTPTR_MAX__ 4294967295U
-// ARM-NETBSD:#define __UINTPTR_TYPE__ unsigned int
+// ARM-NETBSD:#define __UINTPTR_MAX__ 4294967295UL
+// ARM-NETBSD:#define __UINTPTR_TYPE__ long unsigned int
 // ARM-NETBSD:#define __UINTPTR_WIDTH__ 32
 // ARM-NETBSD:#define __UINT_FAST16_MAX__ 65535
 // ARM-NETBSD:#define __UINT_FAST16_TYPE__ unsigned short


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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Peter Wu via Phabricator via cfe-commits]

Lekensteyn added a comment.

In https://reviews.llvm.org/D37954#901878, @rsmith wrote:

> In https://reviews.llvm.org/D37954#884029, @Lekensteyn wrote:
>
> > Any objection for merging these patches? I rebased today (no changes 
> > needed) and it still passes clang-tests.
>
>
> This is not the way that we do code review in LLVM and Clang. If you don't 
> get a response to a patch, you're expected to keep pinging it (typically once 
> a week) until you do get an answer.


There was no negative feedback and looking in the git logs also showed some 
examples without explicit review approval, so I (mistakenly) thought it was 
acceptable.

>   This past week in particular has been especially bad as we had the mailing 
> deadline for the C++ committee meeting on Monday and the LLVM developer 
> meeting yesterday and today.

Oh, where could I have learned about this? I am sporadically contributing and 
not following every day.

> I reverted this in r316195, because it breaks users using 
> `-fno-canonical-prefixes` to request that we do not do this kind of path 
> canonicalization.

I can propose a new patch and also add a test for this case. Thanks for your 
review and pointers!

In https://reviews.llvm.org/D37954#901884, @joerg wrote:

> The behavior of sometimes transforming the path and sometimes not is IMO 
> completely unacceptable. I don't care if GCC created that bug first.


Would it help if a compile-time option is added so you can disable that for 
OpenBSD? All I am trying to do is to fix a bug in Ninja that is triggered 
because Clang deviates from GCC here. And Ninja will apparently not be fixed, 
so that leaves only this option. Do you have an alternative proposal?




Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:967-968
 
+  if (Arg *A = Args.getLastArg(options::OPT_fno_canonical_system_headers,
+   options::OPT_fcanonical_system_headers)) {
+if (A->getOption().matches(options::OPT_fno_canonical_system_headers)) {

rsmith wrote:
> Use `hasFlag` instead.
> 
> Also, defaulting this on will break existing users who use 
> `-fno-canonical-prefixes` to turn off the (wrong in general) assumption that 
> using `realpath` is meaningful. Please default to the value of that flag 
> instead. I would guess (but haven't checked) that flag controls this behavior 
> in GCC.
Ok, I will apply the suggestions.

This patch does not seem to have any effect on clang, the path still starts 
with `/bin/../lib64/...` rather than `/usr/lib/...`

With GCC: just `-no-canonical-prefixes` does not have any effect.

`-fno-canonical-system-headers -no-canonical-prefixes` produces:
`/bin/../lib/gcc/x86_64-pc-linux-gnu/7.2.0/../../../../include/c++/7.2.0/cstdio`

`-fno-canonical-system-headers` produces:
`/usr/lib/gcc/x86_64-pc-linux-gnu/7.2.0/../../../../include/c++/7.2.0/cstdio`



Comment at: cfe/trunk/lib/Frontend/DependencyFile.cpp:296
+  if (CanonicalSystemHeaders && isSystem(FileType)) {
+StringRef RealPath = FE->tryGetRealPathName();
+if (!RealPath.empty() && RealPath.size() < Filename.size()) {

rsmith wrote:
> This is not an appropriate use of `tryGetRealPathName`. Its purpose is to get 
> the way the file name was stored in disk (particularly, preserving the 
> original case on a case-sensitive file system), not to resolve symlinks.
> 
> `FileManager::getCanonicalName` would be the right thing to call.
OK.


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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

[PATCH] D38680: [libunwind] Fix handling of DW_CFA_GNU_args_size

[Will Dietz via Phabricator via cfe-commits]

dtzWill added a comment.

Wow, I've finally debugged some unwind failures to this on x86_64 Linux ...

1. Call-frame optimization introduces it
2. This fixes the cases I was investigating!!

Thank you!


https://reviews.llvm.org/D38680



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

[PATCH] D26244: [Driver] Add CLANG_PREFER_GCC_LIBRARIES which can be disabled

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld abandoned this revision.
Hahnfeld added a comment.

Abandoning as I lost interest in this.


https://reviews.llvm.org/D26244



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

[PATCH] D36347: Add new script to launch lldb and set breakpoints for diagnostics all diagnostics seen.

[Zachary Turner via Phabricator via cfe-commits]

zturner added a subscriber: lldb-commits.
zturner added a comment.

One possible reason for why this never got any traction is that `lldb-commits` 
wasn't added as a subscriber.  While it's true that the tagged people should 
have chimed in, having the whole commits list will get some more visibility.  I 
never saw this come to my inbox.

I think this would be most suitable in the `lldb/examples` folder.

I can't really review this thoroughly, because it relies on a bash script, and 
I use Windows where we bash isn't really a thing.  My bash is rusty, but it 
looks like you're embedding a python script in the bash script?  It might be 
good if this were just an lldb script command.  Take a look at `command script 
add` in LLDB, and in the `examples` folder for some examples of existing 
commands that work this way.  The nice thing about doing it this way is that 
you could just be inside LLDB and write `(lldb) break-diag -Wcovered-switch`, 
for example, which would be a much tighter integration with the debugger.


https://reviews.llvm.org/D36347



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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

The behavior of sometimes transforming the path and sometimes not is IMO 
completely unacceptable. I don't care if GCC created that bug first.


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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

[PATCH] D27607: [ubsan] Treat ObjC's BOOL as if its range is always {0, 1}

[Francis Ricci via Phabricator via cfe-commits]

fjricci added a comment.

On platforms where `BOOL` == `signed char`, is it actually undefined behavior 
(or is it just bad programming practice) to store a value other than 0 or 1 in 
your `BOOL`? I can't find any language specs suggesting that it is, and given 
that it's just a typedef for a `signed char`, I don't see why it would be.

If it's not actually undefined behavior, could we make it controllable via a 
separate fsanitize switch (like we have for unsigned integer overflow, which is 
also potentially bad practice but not actually undefined behavior).


Repository:
  rL LLVM

https://reviews.llvm.org/D27607



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

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In https://reviews.llvm.org/D37954#884029, @Lekensteyn wrote:

> Any objection for merging these patches? I rebased today (no changes needed) 
> and it still passes clang-tests.


This is not the way that we do code review in LLVM and Clang. If you don't get 
a response to a patch, you're expected to keep pinging it (typically once a 
week) until you do get an answer. This past week in particular has been 
especially bad as we had the mailing deadline for the C++ committee meeting on 
Monday and the LLVM developer meeting yesterday and today.

I reverted this in r316195, because it breaks users using 
`-fno-canonical-prefixes` to request that we do not do this kind of path 
canonicalization.




Comment at: cfe/trunk/lib/Driver/ToolChains/Clang.cpp:967-968
 
+  if (Arg *A = Args.getLastArg(options::OPT_fno_canonical_system_headers,
+   options::OPT_fcanonical_system_headers)) {
+if (A->getOption().matches(options::OPT_fno_canonical_system_headers)) {

Use `hasFlag` instead.

Also, defaulting this on will break existing users who use 
`-fno-canonical-prefixes` to turn off the (wrong in general) assumption that 
using `realpath` is meaningful. Please default to the value of that flag 
instead. I would guess (but haven't checked) that flag controls this behavior 
in GCC.



Comment at: cfe/trunk/lib/Frontend/DependencyFile.cpp:296
+  if (CanonicalSystemHeaders && isSystem(FileType)) {
+StringRef RealPath = FE->tryGetRealPathName();
+if (!RealPath.empty() && RealPath.size() < Filename.size()) {

This is not an appropriate use of `tryGetRealPathName`. Its purpose is to get 
the way the file name was stored in disk (particularly, preserving the original 
case on a case-sensitive file system), not to resolve symlinks.

`FileManager::getCanonicalName` would be the right thing to call.


Repository:
  rL LLVM

https://reviews.llvm.org/D37954



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

r316195 - Revert r316193.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Oct 19 17:25:07 2017
New Revision: 316195

URL: http://llvm.org/viewvc/llvm-project?rev=316195&view=rev
Log:
Revert r316193.

This patch breaks users using -fno-canonical-prefixes, for whom resolving
symlinks is not acceptable.

Removed:
cfe/trunk/test/Preprocessor/dependencies-realpath.c
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
cfe/trunk/lib/Driver/Job.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp
cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=316195&r1=316194&r2=316195&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Oct 19 17:25:07 2017
@@ -384,11 +384,6 @@ def MT : JoinedOrSeparate<["-"], "MT">,
 HelpText<"Specify name of main file output in depfile">;
 def MV : Flag<["-"], "MV">, Group, Flags<[CC1Option]>,
 HelpText<"Use NMake/Jom format for the depfile">;
-def fno_canonical_system_headers : Flag<["-"], "fno-canonical-system-headers">,
-Group, Flags<[CC1Option]>,
-HelpText<"Do not shorten system header paths in depfiles">;
-def fcanonical_system_headers : Flag<["-"], "fcanonical-system-headers">,
-Group;
 def Mach : Flag<["-"], "Mach">, Group;
 def O0 : Flag<["-"], "O0">, Group, Flags<[CC1Option, HelpHidden]>;
 def O4 : Flag<["-"], "O4">, Group, Flags<[CC1Option, HelpHidden]>;

Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=316195&r1=316194&r2=316195&view=diff
==
--- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Thu Oct 19 
17:25:07 2017
@@ -30,8 +30,6 @@ public:
   unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency 
list
   unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info.
   unsigned IncludeModuleFiles : 1; ///< Include module file dependencies.
-  unsigned CanonicalSystemHeaders : 1; ///< Try to output a shorter path for
-   /// system header dependencies.
 
   /// The format for the dependency file.
   DependencyOutputFormat OutputFormat;
@@ -69,7 +67,6 @@ public:
 AddMissingHeaderDeps = 0;
 PrintShowIncludes = 0;
 IncludeModuleFiles = 0;
-CanonicalSystemHeaders = 1;
 OutputFormat = DependencyOutputFormat::Make;
   }
 };

Modified: cfe/trunk/lib/Driver/Job.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=316195&r1=316194&r2=316195&view=diff
==
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Thu Oct 19 17:25:07 2017
@@ -73,8 +73,8 @@ static bool skipArgs(const char *Flag, b
 
   // These flags are all of the form -Flag and have no second argument.
   ShouldSkip = llvm::StringSwitch(Flag)
-.Cases("-M", "-MM", "-MG", "-MP", "-MD", "-MMD", true)
-.Cases("-fno-canonical-system-headers", "-fcanonical-system-headers", true)
+.Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
+.Case("-MMD", true)
 .Default(false);
 
   // Match found.

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=316195&r1=316194&r2=316195&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Oct 19 17:25:07 2017
@@ -964,13 +964,6 @@ void Clang::AddPreprocessingOptions(Comp
   Args.AddLastArg(CmdArgs, options::OPT_C);
   Args.AddLastArg(CmdArgs, options::OPT_CC);
 
-  if (Arg *A = Args.getLastArg(options::OPT_fno_canonical_system_headers,
-   options::OPT_fcanonical_system_headers)) {
-if (A->getOption().matches(options::OPT_fno_canonical_system_headers)) {
-  CmdArgs.push_back("-fno-canonical-system-headers");
-}
-  }
-
   // Handle dependency file generation.
   if ((A = Args.getLastArg(options::OPT_M, options::OPT_MM)) ||
   (A = Args.getLastArg(options::OPT_MD)) ||

Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=316195&r1=316194&r2=316195&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
+++ cfe/trunk/lib/Frontend/C

[PATCH] D37954: Try to shorten system header paths when using -MD depfiles

[Peter Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316193: Try to shorten system header paths when using -MD 
depfiles (authored by Lekensteyn).

Changed prior to commit:
  https://reviews.llvm.org/D37954?vs=115946&id=119626#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D37954

Files:
  cfe/trunk/include/clang/Driver/Options.td
  cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
  cfe/trunk/lib/Driver/Job.cpp
  cfe/trunk/lib/Driver/ToolChains/Clang.cpp
  cfe/trunk/lib/Frontend/CompilerInvocation.cpp
  cfe/trunk/lib/Frontend/DependencyFile.cpp
  cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp
  cfe/trunk/test/Preprocessor/dependencies-realpath.c

Index: cfe/trunk/test/Preprocessor/dependencies-realpath.c
===
--- cfe/trunk/test/Preprocessor/dependencies-realpath.c
+++ cfe/trunk/test/Preprocessor/dependencies-realpath.c
@@ -0,0 +1,33 @@
+// RUN: mkdir -p %t/sub/dir
+// RUN: echo > %t/sub/empty.h
+
+// Test that system header paths are expanded
+//
+// RUN: %clang -fsyntax-only -MD -MF %t.d -MT foo %s -isystem %t/sub/dir/..
+// RUN: FileCheck -check-prefix=TEST1 %s < %t.d
+// TEST1: foo:
+// TEST1: sub{{/|\\}}empty.h
+
+// Test that system header paths are not expanded to a longer form
+//
+// RUN: cd %t && %clang -fsyntax-only -MD -MF %t.d -MT foo %s -isystem sub/dir/..
+// RUN: FileCheck -check-prefix=TEST2 %s < %t.d
+// TEST2: foo:
+// TEST2: sub/dir/..{{/|\\}}empty.h
+
+// Test that user header paths are not expanded
+//
+// RUN: %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %t/sub/dir/..
+// RUN: FileCheck -check-prefix=TEST3 %s < %t.d
+// TEST3: foo:
+// TEST3: sub/dir/..{{/|\\}}empty.h
+
+// Test that system header paths are not expanded with -fno-canonical-system-headers
+// (and also that the -fsystem-system-headers option is accepted)
+//
+// RUN: %clang -fsyntax-only -MD -MF %t.d -MT foo %s -I %t/sub/dir/.. -fcanonical-system-headers -fno-canonical-system-headers
+// RUN: FileCheck -check-prefix=TEST4 %s < %t.d
+// TEST4: foo:
+// TEST4: sub/dir/..{{/|\\}}empty.h
+
+#include 
Index: cfe/trunk/lib/Driver/Job.cpp
===
--- cfe/trunk/lib/Driver/Job.cpp
+++ cfe/trunk/lib/Driver/Job.cpp
@@ -73,8 +73,8 @@
 
   // These flags are all of the form -Flag and have no second argument.
   ShouldSkip = llvm::StringSwitch(Flag)
-.Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
-.Case("-MMD", true)
+.Cases("-M", "-MM", "-MG", "-MP", "-MD", "-MMD", true)
+.Cases("-fno-canonical-system-headers", "-fcanonical-system-headers", true)
 .Default(false);
 
   // Match found.
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -964,6 +964,13 @@
   Args.AddLastArg(CmdArgs, options::OPT_C);
   Args.AddLastArg(CmdArgs, options::OPT_CC);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fno_canonical_system_headers,
+   options::OPT_fcanonical_system_headers)) {
+if (A->getOption().matches(options::OPT_fno_canonical_system_headers)) {
+  CmdArgs.push_back("-fno-canonical-system-headers");
+}
+  }
+
   // Handle dependency file generation.
   if ((A = Args.getLastArg(options::OPT_M, options::OPT_MM)) ||
   (A = Args.getLastArg(options::OPT_MD)) ||
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1002,6 +1002,7 @@
   Opts.Targets = Args.getAllArgValues(OPT_MT);
   Opts.IncludeSystemHeaders = Args.hasArg(OPT_sys_header_deps);
   Opts.IncludeModuleFiles = Args.hasArg(OPT_module_file_deps);
+  Opts.CanonicalSystemHeaders = !Args.hasArg(OPT_fno_canonical_system_headers);
   Opts.UsePhonyTargets = Args.hasArg(OPT_MP);
   Opts.ShowHeaderIncludes = Args.hasArg(OPT_H);
   Opts.HeaderIncludeOutputFile = Args.getLastArgValue(OPT_header_include_file);
Index: cfe/trunk/lib/Frontend/DependencyFile.cpp
===
--- cfe/trunk/lib/Frontend/DependencyFile.cpp
+++ cfe/trunk/lib/Frontend/DependencyFile.cpp
@@ -161,6 +161,7 @@
   bool AddMissingHeaderDeps;
   bool SeenMissingHeader;
   bool IncludeModuleFiles;
+  bool CanonicalSystemHeaders;
   DependencyOutputFormat OutputFormat;
 
 private:
@@ -176,6 +177,7 @@
   AddMissingHeaderDeps(Opts.AddMissingHeaderDeps),
   SeenMissingHeader(false),
   IncludeModuleFiles(Opts.IncludeModuleFiles),
+  CanonicalSystemHeaders(Opts.CanonicalSystemHeaders),
   OutputFormat(Opts.OutputFormat) {
 for (const auto &ExtraDep : Opts.ExtraDeps) {
   AddFilename(ExtraDep);
@@ -288,6 +290,15 @@
   if (!FileMatchesDepCriteria(Filename.data(), FileType))
 return;
 
+ 

r316193 - Try to shorten system header paths when using -MD depfiles

[Peter Wu via cfe-commits]

Author: lekensteyn
Date: Thu Oct 19 16:53:27 2017
New Revision: 316193

URL: http://llvm.org/viewvc/llvm-project?rev=316193&view=rev
Log:
Try to shorten system header paths when using -MD depfiles

GCC tries to shorten system headers in depfiles using its real path
(resolving components like ".." and following symlinks). Mimic this
feature to ensure that the Ninja build tool detects the correct
dependencies when a symlink changes directory levels, see
https://github.com/ninja-build/ninja/issues/1330

An option to disable this feature is added in case "these changed header
paths may conflict with some compilation environments", see
https://gcc.gnu.org/ml/gcc-patches/2012-09/msg00287.html

Note that the original feature request for GCC
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52974) also included paths
preprocessed output (-E) and diagnostics. That is not implemented now
since I am not sure if it breaks something else.

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

Added:
cfe/trunk/test/Preprocessor/dependencies-realpath.c   (with props)
Modified:
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
cfe/trunk/lib/Driver/Job.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Frontend/DependencyFile.cpp
cfe/trunk/lib/Tooling/ArgumentsAdjusters.cpp

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=316193&r1=316192&r2=316193&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Oct 19 16:53:27 2017
@@ -384,6 +384,11 @@ def MT : JoinedOrSeparate<["-"], "MT">,
 HelpText<"Specify name of main file output in depfile">;
 def MV : Flag<["-"], "MV">, Group, Flags<[CC1Option]>,
 HelpText<"Use NMake/Jom format for the depfile">;
+def fno_canonical_system_headers : Flag<["-"], "fno-canonical-system-headers">,
+Group, Flags<[CC1Option]>,
+HelpText<"Do not shorten system header paths in depfiles">;
+def fcanonical_system_headers : Flag<["-"], "fcanonical-system-headers">,
+Group;
 def Mach : Flag<["-"], "Mach">, Group;
 def O0 : Flag<["-"], "O0">, Group, Flags<[CC1Option, HelpHidden]>;
 def O4 : Flag<["-"], "O4">, Group, Flags<[CC1Option, HelpHidden]>;

Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=316193&r1=316192&r2=316193&view=diff
==
--- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original)
+++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Thu Oct 19 
16:53:27 2017
@@ -30,6 +30,8 @@ public:
   unsigned AddMissingHeaderDeps : 1; ///< Add missing headers to dependency 
list
   unsigned PrintShowIncludes : 1; ///< Print cl.exe style /showIncludes info.
   unsigned IncludeModuleFiles : 1; ///< Include module file dependencies.
+  unsigned CanonicalSystemHeaders : 1; ///< Try to output a shorter path for
+   /// system header dependencies.
 
   /// The format for the dependency file.
   DependencyOutputFormat OutputFormat;
@@ -67,6 +69,7 @@ public:
 AddMissingHeaderDeps = 0;
 PrintShowIncludes = 0;
 IncludeModuleFiles = 0;
+CanonicalSystemHeaders = 1;
 OutputFormat = DependencyOutputFormat::Make;
   }
 };

Modified: cfe/trunk/lib/Driver/Job.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Job.cpp?rev=316193&r1=316192&r2=316193&view=diff
==
--- cfe/trunk/lib/Driver/Job.cpp (original)
+++ cfe/trunk/lib/Driver/Job.cpp Thu Oct 19 16:53:27 2017
@@ -73,8 +73,8 @@ static bool skipArgs(const char *Flag, b
 
   // These flags are all of the form -Flag and have no second argument.
   ShouldSkip = llvm::StringSwitch(Flag)
-.Cases("-M", "-MM", "-MG", "-MP", "-MD", true)
-.Case("-MMD", true)
+.Cases("-M", "-MM", "-MG", "-MP", "-MD", "-MMD", true)
+.Cases("-fno-canonical-system-headers", "-fcanonical-system-headers", true)
 .Default(false);
 
   // Match found.

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=316193&r1=316192&r2=316193&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Oct 19 16:53:27 2017
@@ -964,6 +964,13 @@ void Clang::AddPreprocessingOptions(Comp
   Args.AddLastArg(CmdArgs, options::OPT_C);
   Args.AddLastArg(CmdArgs, options::OPT_CC);
 
+  if (Arg *A = Args.getLastArg(options::OPT_fno_canonical

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

[Jiří Zárevúcky via Phabricator via cfe-commits]

jzr added a comment.

I happen to need this functionality as well. What's the progress? Is there 
anything I can do to help it along?


Repository:
  rL LLVM

https://reviews.llvm.org/D28462



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

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

[Jiří Zárevúcky via Phabricator via cfe-commits]

jzr added a comment.

On a related note, would it be difficult to add support for custom spacing 
between the macro name and definition?
In the HelenOS project, we have definitions aligned two spaces after the 
longest name.


Repository:
  rL LLVM

https://reviews.llvm.org/D28462



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

[libcxx] r316191 - Fix an unsigned integer overflow in regex that lead to a bad memory access. Found by OSS-Fuzz

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu Oct 19 15:10:41 2017
New Revision: 316191

URL: http://llvm.org/viewvc/llvm-project?rev=316191&view=rev
Log:
Fix an unsigned integer overflow in regex that lead to a bad memory access. 
Found by OSS-Fuzz

Modified:
libcxx/trunk/include/regex
libcxx/trunk/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp

Modified: libcxx/trunk/include/regex
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/regex?rev=316191&r1=316190&r2=316191&view=diff
==
--- libcxx/trunk/include/regex (original)
+++ libcxx/trunk/include/regex Thu Oct 19 15:10:41 2017
@@ -4327,8 +4327,12 @@ basic_regex<_CharT, _Traits>::__parse_de
 unsigned __v = *__first - '0';
 for (++__first;
 __first != __last && '0' <= *__first && *__first <= '9'; 
++__first)
+{
+if (__v >= std::numeric_limits::max() / 10)
+__throw_regex_error();
 __v = 10 * __v + *__first - '0';
-if (__v > mark_count())
+}
+if (__v == 0 || __v > mark_count())
 __throw_regex_error();
 __push_back_ref(__v);
 }
@@ -5455,15 +5459,17 @@ match_results<_BidirectionalIterator, _A
 if ('0' <= __fmt_first[1] && __fmt_first[1] <= '9')
 {
 ++__fmt_first;
-size_t __i = *__fmt_first - '0';
+size_t __idx = *__fmt_first - '0';
 if (__fmt_first + 1 != __fmt_last &&
 '0' <= __fmt_first[1] && __fmt_first[1] <= '9')
 {
 ++__fmt_first;
-__i = 10 * __i + *__fmt_first - '0';
+if (__idx >= std::numeric_limits::max() / 
10)
+
__throw_regex_error();
+__idx = 10 * __idx + *__fmt_first - '0';
 }
-__output = _VSTD::copy((*this)[__i].first,
-(*this)[__i].second, __output);
+__output = _VSTD::copy((*this)[__idx].first,
+(*this)[__idx].second, __output);
 }
 else
 {

Modified: 
libcxx/trunk/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp?rev=316191&r1=316190&r2=316191&view=diff
==
--- libcxx/trunk/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp 
(original)
+++ libcxx/trunk/test/std/re/re.regex/re.regex.construct/bad_backref.pass.cpp 
Thu Oct 19 15:10:41 2017
@@ -34,6 +34,7 @@ int main()
 {
 assert(error_badbackref_thrown("\\1abc"));  // no references
 assert(error_badbackref_thrown("ab(c)\\2def")); // only one reference
+assert(error_badbackref_thrown("\\80")); // 
overflows
 
 //  this should NOT throw, because we only should look at the '1'
 //  See https://bugs.llvm.org/show_bug.cgi?id=31387


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

r316186 - These attributes are not supported by GCC and should not be in the gnu namespace. Switching from the GCC spelling to the GNU spelling so that they are only supported with __attribute__(()).

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Thu Oct 19 14:20:28 2017
New Revision: 316186

URL: http://llvm.org/viewvc/llvm-project?rev=316186&view=rev
Log:
These attributes are not supported by GCC and should not be in the gnu 
namespace. Switching from the GCC spelling to the GNU spelling so that they are 
only supported with __attribute__(()).

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=316186&r1=316185&r2=316186&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Oct 19 14:20:28 2017
@@ -1758,23 +1758,23 @@ def StdCall : InheritableAttr {
 }
 
 def SwiftCall : InheritableAttr {
-  let Spellings = [GCC<"swiftcall">];
+  let Spellings = [GNU<"swiftcall">];
 //  let Subjects = SubjectList<[Function]>;
   let Documentation = [SwiftCallDocs];
 }
 
 def SwiftContext : ParameterABIAttr {
-  let Spellings = [GCC<"swift_context">];
+  let Spellings = [GNU<"swift_context">];
   let Documentation = [SwiftContextDocs];
 }
 
 def SwiftErrorResult : ParameterABIAttr {
-  let Spellings = [GCC<"swift_error_result">];
+  let Spellings = [GNU<"swift_error_result">];
   let Documentation = [SwiftErrorResultDocs];
 }
 
 def SwiftIndirectResult : ParameterABIAttr {
-  let Spellings = [GCC<"swift_indirect_result">];
+  let Spellings = [GNU<"swift_indirect_result">];
   let Documentation = [SwiftIndirectResultDocs];
 }
 


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

r316184 - These attributes are supported by GCC with the gnu vendor namespace for C++11-style attributes. Enabling the gnu namespace by switching to the GCC spelling.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Thu Oct 19 14:09:39 2017
New Revision: 316184

URL: http://llvm.org/viewvc/llvm-project?rev=316184&view=rev
Log:
These attributes are supported by GCC with the gnu vendor namespace for 
C++11-style attributes. Enabling the gnu namespace by switching to the GCC 
spelling.

Modified:
cfe/trunk/include/clang/Basic/Attr.td

Modified: cfe/trunk/include/clang/Basic/Attr.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=316184&r1=316183&r2=316184&view=diff
==
--- cfe/trunk/include/clang/Basic/Attr.td (original)
+++ cfe/trunk/include/clang/Basic/Attr.td Thu Oct 19 14:09:39 2017
@@ -611,7 +611,7 @@ def Annotate : InheritableParamAttr {
 def ARMInterrupt : InheritableAttr, TargetSpecificAttr {
   // NOTE: If you add any additional spellings, MSP430Interrupt's,
   // MipsInterrupt's and AnyX86Interrupt's spellings must match.
-  let Spellings = [GNU<"interrupt">];
+  let Spellings = [GCC<"interrupt">];
   let Args = [EnumArgument<"Interrupt", "InterruptType",
["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", ""],
["IRQ", "FIQ", "SWI", "ABORT", "UNDEF", "Generic"],
@@ -622,14 +622,14 @@ def ARMInterrupt : InheritableAttr, Targ
 }
 
 def AVRInterrupt : InheritableAttr, TargetSpecificAttr {
-  let Spellings = [GNU<"interrupt">];
+  let Spellings = [GCC<"interrupt">];
   let Subjects = SubjectList<[Function]>;
   let ParseKind = "Interrupt";
   let Documentation = [AVRInterruptDocs];
 }
 
 def AVRSignal : InheritableAttr, TargetSpecificAttr {
-  let Spellings = [GNU<"signal">];
+  let Spellings = [GCC<"signal">];
   let Subjects = SubjectList<[Function]>;
   let Documentation = [AVRSignalDocs];
 }
@@ -1158,7 +1158,7 @@ def MSABI : InheritableAttr {
 def MSP430Interrupt : InheritableAttr, TargetSpecificAttr {
   // NOTE: If you add any additional spellings, ARMInterrupt's, MipsInterrupt's
   // and AnyX86Interrupt's spellings must match.
-  let Spellings = [GNU<"interrupt">];
+  let Spellings = [GCC<"interrupt">];
   let Args = [UnsignedArgument<"Number">];
   let ParseKind = "Interrupt";
   let HasCustomParsing = 1;
@@ -1174,7 +1174,7 @@ def Mips16 : InheritableAttr, TargetSpec
 def MipsInterrupt : InheritableAttr, TargetSpecificAttr {
   // NOTE: If you add any additional spellings, ARMInterrupt's,
   // MSP430Interrupt's and AnyX86Interrupt's spellings must match.
-  let Spellings = [GNU<"interrupt">];
+  let Spellings = [GCC<"interrupt">];
   let Subjects = SubjectList<[Function]>;
   let Args = [EnumArgument<"Interrupt", "InterruptType",
["vector=sw0", "vector=sw1", "vector=hw0",
@@ -1692,7 +1692,7 @@ def WorkGroupSizeHint :  InheritableAttr
 }
 
 def InitPriority : InheritableAttr {
-  let Spellings = [GNU<"init_priority">];
+  let Spellings = [GCC<"init_priority">];
   let Args = [UnsignedArgument<"Priority">];
   let Subjects = SubjectList<[Var], ErrorDiag>;
   let Documentation = [Undocumented];
@@ -1999,7 +1999,7 @@ def VecReturn : InheritableAttr {
 }
 
 def WarnUnused : InheritableAttr {
-  let Spellings = [GNU<"warn_unused">];
+  let Spellings = [GCC<"warn_unused">];
   let Subjects = SubjectList<[Record]>;
   let Documentation = [Undocumented];
 }
@@ -2041,7 +2041,7 @@ def LTOVisibilityPublic : InheritableAtt
 def AnyX86Interrupt : InheritableAttr, TargetSpecificAttr {
   // NOTE: If you add any additional spellings, ARMInterrupt's,
   // MSP430Interrupt's and MipsInterrupt's spellings must match.
-  let Spellings = [GNU<"interrupt">];
+  let Spellings = [GCC<"interrupt">];
   let Subjects = SubjectList<[HasFunctionProto]>;
   let ParseKind = "Interrupt";
   let HasCustomParsing = 1;


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

r316179 - [CGExprScalar] Add missing types in function GetIntrinsic

[Guozhi Wei via cfe-commits]

Author: carrot
Date: Thu Oct 19 13:11:23 2017
New Revision: 316179

URL: http://llvm.org/viewvc/llvm-project?rev=316179&view=rev
Log:
[CGExprScalar] Add missing types in function GetIntrinsic

In function GetIntrinsic, not all types are covered. Types double and long long 
are missed, type long is wrongly treated same as int, it should be same as long 
long. These problems cause compiler crashes when compiling code in PR31161. 
This patch fixed the problem.

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


Modified:
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/test/CodeGen/ppc-vector-compare.cc

Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=316179&r1=316178&r2=316179&view=diff
==
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Thu Oct 19 13:11:23 2017
@@ -3120,16 +3120,25 @@ static llvm::Intrinsic::ID GetIntrinsic(
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequh_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtsh_p;
   case BuiltinType::UInt:
-  case BuiltinType::ULong:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtuw_p;
   case BuiltinType::Int:
-  case BuiltinType::Long:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtsw_p;
+  case BuiltinType::ULong:
+  case BuiltinType::ULongLong:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequd_p :
+llvm::Intrinsic::ppc_altivec_vcmpgtud_p;
+  case BuiltinType::Long:
+  case BuiltinType::LongLong:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequd_p :
+llvm::Intrinsic::ppc_altivec_vcmpgtsd_p;
   case BuiltinType::Float:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpeqfp_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtfp_p;
+  case BuiltinType::Double:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_vsx_xvcmpeqdp_p :
+llvm::Intrinsic::ppc_vsx_xvcmpgtdp_p;
   }
 }
 

Modified: cfe/trunk/test/CodeGen/ppc-vector-compare.cc
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ppc-vector-compare.cc?rev=316179&r1=316178&r2=316179&view=diff
==
--- cfe/trunk/test/CodeGen/ppc-vector-compare.cc (original)
+++ cfe/trunk/test/CodeGen/ppc-vector-compare.cc Thu Oct 19 13:11:23 2017
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -target-feature +altivec -triple powerpc64-unknown-unknown 
-emit-llvm %s \
+// RUN: %clang_cc1 -target-feature +vsx -triple powerpc64-unknown-unknown 
-emit-llvm %s \
 // RUN:-o - | FileCheck %s
 
 #include 
@@ -9,3 +9,26 @@ bool test1(vector unsigned short v1, vec
   return v1 == v2;
 }
 
+// CHECK-LABEL: @_Z5test2Dv2_mS_Dv2_lS0_Dv2_yS1_Dv2_xS2_Dv2_dS3_
+bool test2(vector unsigned long v1, vector unsigned long v2,
+   vector long v3, vector long v4,
+   vector unsigned long long v5, vector unsigned long long v6,
+   vector long long v7, vector long long v8,
+   vector double v9, vector double v10) {
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  bool res = v1 == v2;
+
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  res |= v3 == v4;
+
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  res |= v5 == v6;
+
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  res |= v7 == v8;
+
+  // CHECK: @llvm.ppc.vsx.xvcmpeqdp.p
+  res |= v9 == v10;
+  return res;
+}
+


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

[PATCH] D38667: AMDGPU: Parse r600 CPU name early and expose FMAF capability

[Jan Vesely via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316181: AMDGPU: Parse r600 CPU name early and expose FMAF 
capability (authored by jvesely).

Changed prior to commit:
  https://reviews.llvm.org/D38667?vs=118136&id=119614#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38667

Files:
  cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
  cfe/trunk/test/Preprocessor/predefined-arch-macros.c


Index: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
===
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c
@@ -2378,10 +2378,20 @@
 // RUN: -target amdgcn-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_AMDGCN
 // CHECK_AMDGCN: #define __AMDGCN__ 1
+// CHECK_AMDGCN: #define __HAS_FMAF__ 1
+// CHECK_AMDGCN: #define __HAS_FP64__ 1
+// CHECK_AMDGCN: #define __HAS_LDEXPF__ 1
 
 // Begin r600 tests 
 //
 // RUN: %clang -march=amdgcn -E -dM %s -o - 2>&1 \
 // RUN: -target r600-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_R600
 // CHECK_R600: #define __R600__ 1
+// CHECK_R600-NOT: #define __HAS_FMAF__ 1
+
+// RUN: %clang -march=amdgcn -mcpu=cypress -E -dM %s -o - 2>&1 \
+// RUN: -target r600-unknown-unknown \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_R600_FP64
+// CHECK_R600_FP64-DAG: #define __R600__ 1
+// CHECK_R600_FP64-DAG: #define __HAS_FMAF__ 1
Index: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
===
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
@@ -308,14 +308,20 @@
 
 AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
-: TargetInfo(Triple), GPU(isAMDGCN(Triple) ? GK_GFX6 : GK_R600),
+: TargetInfo(Triple),
+  GPU(isAMDGCN(Triple) ? GK_GFX6 : parseR600Name(Opts.CPU)),
   hasFP64(false), hasFMAF(false), hasLDEXPF(false),
   AS(isGenericZero(Triple)) {
   if (getTriple().getArch() == llvm::Triple::amdgcn) {
 hasFP64 = true;
 hasFMAF = true;
 hasLDEXPF = true;
   }
+  if (getTriple().getArch() == llvm::Triple::r600) {
+if (GPU == GK_EVERGREEN_DOUBLE_OPS || GPU == GK_CAYMAN) {
+  hasFMAF = true;
+}
+  }
   auto IsGenericZero = isGenericZero(Triple);
   resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn
   ? (IsGenericZero ? DataLayoutStringSIGenericIsZero


Index: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
===
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c
@@ -2378,10 +2378,20 @@
 // RUN: -target amdgcn-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_AMDGCN
 // CHECK_AMDGCN: #define __AMDGCN__ 1
+// CHECK_AMDGCN: #define __HAS_FMAF__ 1
+// CHECK_AMDGCN: #define __HAS_FP64__ 1
+// CHECK_AMDGCN: #define __HAS_LDEXPF__ 1
 
 // Begin r600 tests 
 //
 // RUN: %clang -march=amdgcn -E -dM %s -o - 2>&1 \
 // RUN: -target r600-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_R600
 // CHECK_R600: #define __R600__ 1
+// CHECK_R600-NOT: #define __HAS_FMAF__ 1
+
+// RUN: %clang -march=amdgcn -mcpu=cypress -E -dM %s -o - 2>&1 \
+// RUN: -target r600-unknown-unknown \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_R600_FP64
+// CHECK_R600_FP64-DAG: #define __R600__ 1
+// CHECK_R600_FP64-DAG: #define __HAS_FMAF__ 1
Index: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
===
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
@@ -308,14 +308,20 @@
 
 AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
-: TargetInfo(Triple), GPU(isAMDGCN(Triple) ? GK_GFX6 : GK_R600),
+: TargetInfo(Triple),
+  GPU(isAMDGCN(Triple) ? GK_GFX6 : parseR600Name(Opts.CPU)),
   hasFP64(false), hasFMAF(false), hasLDEXPF(false),
   AS(isGenericZero(Triple)) {
   if (getTriple().getArch() == llvm::Triple::amdgcn) {
 hasFP64 = true;
 hasFMAF = true;
 hasLDEXPF = true;
   }
+  if (getTriple().getArch() == llvm::Triple::r600) {
+if (GPU == GK_EVERGREEN_DOUBLE_OPS || GPU == GK_CAYMAN) {
+  hasFMAF = true;
+}
+  }
   auto IsGenericZero = isGenericZero(Triple);
   resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn
   ? (IsGenericZero ? DataLayoutStringSIGenericIsZero
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316181 - AMDGPU: Parse r600 CPU name early and expose FMAF capability

[Jan Vesely via cfe-commits]

Author: jvesely
Date: Thu Oct 19 13:40:13 2017
New Revision: 316181

URL: http://llvm.org/viewvc/llvm-project?rev=316181&view=rev
Log:
AMDGPU: Parse r600 CPU name early and expose FMAF capability

Improve amdgcn macro test
Differential Revision: https://reviews.llvm.org/D38667

Modified:
cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/lib/Basic/Targets/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/AMDGPU.cpp?rev=316181&r1=316180&r2=316181&view=diff
==
--- cfe/trunk/lib/Basic/Targets/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/AMDGPU.cpp Thu Oct 19 13:40:13 2017
@@ -308,7 +308,8 @@ void AMDGPUTargetInfo::setAddressSpaceMa
 
 AMDGPUTargetInfo::AMDGPUTargetInfo(const llvm::Triple &Triple,
const TargetOptions &Opts)
-: TargetInfo(Triple), GPU(isAMDGCN(Triple) ? GK_GFX6 : GK_R600),
+: TargetInfo(Triple),
+  GPU(isAMDGCN(Triple) ? GK_GFX6 : parseR600Name(Opts.CPU)),
   hasFP64(false), hasFMAF(false), hasLDEXPF(false),
   AS(isGenericZero(Triple)) {
   if (getTriple().getArch() == llvm::Triple::amdgcn) {
@@ -316,6 +317,11 @@ AMDGPUTargetInfo::AMDGPUTargetInfo(const
 hasFMAF = true;
 hasLDEXPF = true;
   }
+  if (getTriple().getArch() == llvm::Triple::r600) {
+if (GPU == GK_EVERGREEN_DOUBLE_OPS || GPU == GK_CAYMAN) {
+  hasFMAF = true;
+}
+  }
   auto IsGenericZero = isGenericZero(Triple);
   resetDataLayout(getTriple().getArch() == llvm::Triple::amdgcn
   ? (IsGenericZero ? DataLayoutStringSIGenericIsZero

Modified: cfe/trunk/test/Preprocessor/predefined-arch-macros.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/predefined-arch-macros.c?rev=316181&r1=316180&r2=316181&view=diff
==
--- cfe/trunk/test/Preprocessor/predefined-arch-macros.c (original)
+++ cfe/trunk/test/Preprocessor/predefined-arch-macros.c Thu Oct 19 13:40:13 
2017
@@ -2378,6 +2378,9 @@
 // RUN: -target amdgcn-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_AMDGCN
 // CHECK_AMDGCN: #define __AMDGCN__ 1
+// CHECK_AMDGCN: #define __HAS_FMAF__ 1
+// CHECK_AMDGCN: #define __HAS_FP64__ 1
+// CHECK_AMDGCN: #define __HAS_LDEXPF__ 1
 
 // Begin r600 tests 
 //
@@ -2385,3 +2388,10 @@
 // RUN: -target r600-unknown-unknown \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_R600
 // CHECK_R600: #define __R600__ 1
+// CHECK_R600-NOT: #define __HAS_FMAF__ 1
+
+// RUN: %clang -march=amdgcn -mcpu=cypress -E -dM %s -o - 2>&1 \
+// RUN: -target r600-unknown-unknown \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_R600_FP64
+// CHECK_R600_FP64-DAG: #define __R600__ 1
+// CHECK_R600_FP64-DAG: #define __HAS_FMAF__ 1


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

[PATCH] D38820: [CGExprScalar] Add missing types in function GetIntrinsic

[Guozhi Wei via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316179: [CGExprScalar] Add missing types in function 
GetIntrinsic (authored by Carrot).

Changed prior to commit:
  https://reviews.llvm.org/D38820?vs=118680&id=119613#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D38820

Files:
  cfe/trunk/lib/CodeGen/CGExprScalar.cpp
  cfe/trunk/test/CodeGen/ppc-vector-compare.cc


Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -3120,16 +3120,25 @@
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequh_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtsh_p;
   case BuiltinType::UInt:
-  case BuiltinType::ULong:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtuw_p;
   case BuiltinType::Int:
-  case BuiltinType::Long:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtsw_p;
+  case BuiltinType::ULong:
+  case BuiltinType::ULongLong:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequd_p :
+llvm::Intrinsic::ppc_altivec_vcmpgtud_p;
+  case BuiltinType::Long:
+  case BuiltinType::LongLong:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequd_p :
+llvm::Intrinsic::ppc_altivec_vcmpgtsd_p;
   case BuiltinType::Float:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpeqfp_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtfp_p;
+  case BuiltinType::Double:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_vsx_xvcmpeqdp_p :
+llvm::Intrinsic::ppc_vsx_xvcmpgtdp_p;
   }
 }
 
Index: cfe/trunk/test/CodeGen/ppc-vector-compare.cc
===
--- cfe/trunk/test/CodeGen/ppc-vector-compare.cc
+++ cfe/trunk/test/CodeGen/ppc-vector-compare.cc
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -target-feature +altivec -triple powerpc64-unknown-unknown 
-emit-llvm %s \
+// RUN: %clang_cc1 -target-feature +vsx -triple powerpc64-unknown-unknown 
-emit-llvm %s \
 // RUN:-o - | FileCheck %s
 
 #include 
@@ -9,3 +9,26 @@
   return v1 == v2;
 }
 
+// CHECK-LABEL: @_Z5test2Dv2_mS_Dv2_lS0_Dv2_yS1_Dv2_xS2_Dv2_dS3_
+bool test2(vector unsigned long v1, vector unsigned long v2,
+   vector long v3, vector long v4,
+   vector unsigned long long v5, vector unsigned long long v6,
+   vector long long v7, vector long long v8,
+   vector double v9, vector double v10) {
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  bool res = v1 == v2;
+
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  res |= v3 == v4;
+
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  res |= v5 == v6;
+
+  // CHECK: @llvm.ppc.altivec.vcmpequd.p
+  res |= v7 == v8;
+
+  // CHECK: @llvm.ppc.vsx.xvcmpeqdp.p
+  res |= v9 == v10;
+  return res;
+}
+


Index: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
===
--- cfe/trunk/lib/CodeGen/CGExprScalar.cpp
+++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp
@@ -3120,16 +3120,25 @@
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequh_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtsh_p;
   case BuiltinType::UInt:
-  case BuiltinType::ULong:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtuw_p;
   case BuiltinType::Int:
-  case BuiltinType::Long:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequw_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtsw_p;
+  case BuiltinType::ULong:
+  case BuiltinType::ULongLong:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequd_p :
+llvm::Intrinsic::ppc_altivec_vcmpgtud_p;
+  case BuiltinType::Long:
+  case BuiltinType::LongLong:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpequd_p :
+llvm::Intrinsic::ppc_altivec_vcmpgtsd_p;
   case BuiltinType::Float:
 return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_altivec_vcmpeqfp_p :
 llvm::Intrinsic::ppc_altivec_vcmpgtfp_p;
+  case BuiltinType::Double:
+return (IT == VCMPEQ) ? llvm::Intrinsic::ppc_vsx_xvcmpeqdp_p :
+llvm::Intrinsic::ppc_vsx_xvcmpgtdp_p;
   }
 }
 
Index: cfe/trunk/test/CodeGen/ppc-vector-compare.cc
===
--- cfe/trunk/test/CodeGen/ppc-vector-compare.cc
+++ cfe/trunk/test/CodeGen/ppc-vector-compare.cc
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -target-feature +altivec -triple powerpc64-unknown-unknown -emit-llvm %s \
+// RUN: %clang_cc1 -target-feature +vsx -trip

[PATCH] D39105: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors)

[Antoni via Phabricator via cfe-commits]

antoyo created this revision.

Update clang-tidy to return 1 when there's an error.


https://reviews.llvm.org/D39105

Files:
  clang-tidy/tool/ClangTidyMain.cpp


Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -449,6 +449,10 @@
 return WErrorCount;
   }
 
+  if (FoundErrors) {
+  return 1;
+  }
+
   return 0;
 }
 


Index: clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-tidy/tool/ClangTidyMain.cpp
+++ clang-tidy/tool/ClangTidyMain.cpp
@@ -449,6 +449,10 @@
 return WErrorCount;
   }
 
+  if (FoundErrors) {
+  return 1;
+  }
+
   return 0;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39104: Allow /showIncludes with /P

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.

r213589 was checked in as a solution to
https://bugs.llvm.org/show_bug.cgi?id=20336.

However, it is possible to use /EP with /P
to suppress #line directives AND output to
a file.  There is no reason in that case to
suppress /showIncludes.

This was reported here: 
https://bugs.llvm.org/show_bug.cgi?id=34997


https://reviews.llvm.org/D39104

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


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4873,7 +4873,9 @@
 
   // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
   // would produce interleaved output, so ignore /showIncludes in such cases.
-  if (!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP))
+  if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
+  (Args.hasArg(options::OPT__SLASH_P) &&
+   Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
 if (Arg *A = Args.getLastArg(options::OPT_show_includes))
   A->render(Args, CmdArgs);
 
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -197,8 +197,12 @@
 
 // RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck 
-check-prefix=showIncludes_E %s
 // RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck 
-check-prefix=showIncludes_E %s
+// RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck 
-check-prefix=showIncludes_E %s
 // showIncludes_E: warning: argument unused during compilation: 
'--show-includes'
 
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck 
-check-prefix=showIncludes_E_And_P %s
+// showIncludes_E_And_P-NOT: warning: argument unused during compilation: 
'--show-includes'
+
 // /source-charset: should warn on everything except UTF-8.
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck 
-check-prefix=source-charset-utf-16 %s
 // source-charset-utf-16: invalid value 'utf-16'


Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -4873,7 +4873,9 @@
 
   // Both /showIncludes and /E (and /EP) write to stdout. Allowing both
   // would produce interleaved output, so ignore /showIncludes in such cases.
-  if (!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP))
+  if ((!Args.hasArg(options::OPT_E) && !Args.hasArg(options::OPT__SLASH_EP)) ||
+  (Args.hasArg(options::OPT__SLASH_P) &&
+   Args.hasArg(options::OPT__SLASH_EP) && !Args.hasArg(options::OPT_E)))
 if (Arg *A = Args.getLastArg(options::OPT_show_includes))
   A->render(Args, CmdArgs);
 
Index: test/Driver/cl-options.c
===
--- test/Driver/cl-options.c
+++ test/Driver/cl-options.c
@@ -197,8 +197,12 @@
 
 // RUN: %clang_cl /E /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // RUN: %clang_cl /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
+// RUN: %clang_cl /E /EP /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E %s
 // showIncludes_E: warning: argument unused during compilation: '--show-includes'
 
+// RUN: %clang_cl /EP /P /showIncludes -### -- %s 2>&1 | FileCheck -check-prefix=showIncludes_E_And_P %s
+// showIncludes_E_And_P-NOT: warning: argument unused during compilation: '--show-includes'
+
 // /source-charset: should warn on everything except UTF-8.
 // RUN: %clang_cl /source-charset:utf-16 -### -- %s 2>&1 | FileCheck -check-prefix=source-charset-utf-16 %s
 // source-charset-utf-16: invalid value 'utf-16'
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r315811 - Re-land r315787, "[Sema] Warn about unused variables if we can constant evaluate the initializer."

[Benjamin Kramer via cfe-commits]

Fixed in r316177.

On Thu, Oct 19, 2017 at 5:57 PM, Benjamin Kramer  wrote:
> We should check VD->getInit()->isValueDependent() before we call
> evaluateValue. I wasn't able to come up with a test case that triggers
> the assert though :(
>
> On Thu, Oct 19, 2017 at 5:45 PM, Alexander Kornienko  
> wrote:
>>
>>
>> On Sat, Oct 14, 2017 at 5:59 PM, Benjamin Kramer via cfe-commits
>>  wrote:
>>>
>>> Author: d0k
>>> Date: Sat Oct 14 08:59:34 2017
>>> New Revision: 315811
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=315811&view=rev
>>> Log:
>>> Re-land r315787, "[Sema] Warn about unused variables if we can constant
>>> evaluate the initializer."
>>>
>>> The warnings in libc++ tests were fixed in the meantime.
>>>
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaDecl.cpp
>>> cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=315811&r1=315810&r2=315811&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Oct 14 08:59:34 2017
>>> @@ -1723,7 +1723,8 @@ static bool ShouldDiagnoseUnusedDecl(con
>>>  dyn_cast(Init);
>>>if (Construct && !Construct->isElidable()) {
>>>  CXXConstructorDecl *CD = Construct->getConstructor();
>>> -if (!CD->isTrivial() && !RD->hasAttr())
>>> +if (!CD->isTrivial() && !RD->hasAttr() &&
>>> +!VD->evaluateValue())
>>
>>
>> The evaluateValue call above causes an assertion failure on
>> instantiation-dependent values:
>> llvm/tools/clang/lib/AST/Decl.cpp:2196 in clang::APValue
>> *clang::VarDecl::evaluateValue(SmallVectorImpl
>> &) const: !Init->isValueDependent()
>>
>> I'm not sure why evaluateValue uses an assertion instead of outputting a
>> note and returning nullptr, but the assertion can be avoided on the caller
>> site as well.
>>
>> Working on a reduced test case...
>>
>>>
>>>return false;
>>>}
>>>  }
>>>
>>> Modified: cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-variables.cpp?rev=315811&r1=315810&r2=315811&view=diff
>>>
>>> ==
>>> --- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
>>> +++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Sat Oct 14 08:59:34
>>> 2017
>>> @@ -1,4 +1,5 @@
>>>  // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
>>> -Wno-c++1y-extensions -verify %s
>>> +// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
>>> -Wno-c++1y-extensions -verify -std=c++11 %s
>>>  template void f() {
>>>T t;
>>>t = 17;
>>> @@ -194,3 +195,35 @@ void test() {
>>>  }
>>>
>>>  }
>>> +
>>> +#if __cplusplus >= 201103L
>>> +namespace with_constexpr {
>>> +template 
>>> +struct Literal {
>>> +  T i;
>>> +  Literal() = default;
>>> +  constexpr Literal(T i) : i(i) {}
>>> +};
>>> +
>>> +struct NoLiteral {
>>> +  int i;
>>> +  NoLiteral() = default;
>>> +  constexpr NoLiteral(int i) : i(i) {}
>>> +  ~NoLiteral() {}
>>> +};
>>> +
>>> +static Literal gl1;  // expected-warning {{unused variable
>>> 'gl1'}}
>>> +static Literal gl2(1);   // expected-warning {{unused variable
>>> 'gl2'}}
>>> +static const Literal gl3(0); // expected-warning {{unused variable
>>> 'gl3'}}
>>> +
>>> +template 
>>> +void test(int i) {
>>> +  Literal l1; // expected-warning {{unused variable 'l1'}}
>>> +  Literal l2(42); // expected-warning {{unused variable 'l2'}}
>>> +  Literal l3(i);  // no-warning
>>> +  Literal l4(0);// no-warning
>>> +  NoLiteral nl1;   // no-warning
>>> +  NoLiteral nl2(42);   // no-warning
>>> +}
>>> +}
>>> +#endif
>>>
>>>
>>> ___
>>> cfe-commits mailing list
>>> cfe-commits@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316177 - [Sema] Fix assertion failure when checking for unused variables in a dependent context.

[Benjamin Kramer via cfe-commits]

Author: d0k
Date: Thu Oct 19 12:07:13 2017
New Revision: 316177

URL: http://llvm.org/viewvc/llvm-project?rev=316177&view=rev
Log:
[Sema] Fix assertion failure when checking for unused variables in a dependent 
context.

Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaCXX/warn-unused-variables.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=316177&r1=316176&r2=316177&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Thu Oct 19 12:07:13 2017
@@ -1724,7 +1724,7 @@ static bool ShouldDiagnoseUnusedDecl(con
   if (Construct && !Construct->isElidable()) {
 CXXConstructorDecl *CD = Construct->getConstructor();
 if (!CD->isTrivial() && !RD->hasAttr() &&
-!VD->evaluateValue())
+(VD->getInit()->isValueDependent() || !VD->evaluateValue()))
   return false;
   }
 }

Modified: cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-variables.cpp?rev=316177&r1=316176&r2=316177&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Thu Oct 19 12:07:13 2017
@@ -226,4 +226,14 @@ void test(int i) {
   NoLiteral nl2(42);   // no-warning
 }
 }
+
+namespace crash {
+struct a {
+  a(const char *);
+};
+template 
+void c() {
+  a d(b::e ? "" : "");
+}
+}
 #endif


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

Re: Patch bug 27628 (clang-tidy should exit with a failure code when there are errors)

[Alexander Kornienko via cfe-commits]

The patch was lost. Could you re-attach it or even better upload to
reviews.llvm.org as described in http://llvm.org/docs/Phabricator.html?
Please make sure the patch applies cleanly to the current HEAD.

Thanks!

On Tue, Oct 17, 2017 at 9:10 PM, Friedman, Eli 
wrote:

> Sometimes people just lose track; "pinging" is normal (see
> http://llvm.org/docs/DeveloperPolicy.html#code-reviews).  And sometimes
> it's helpful to CC reviewers in the area; adding Alexander Kornienko.
>
> -Eli
>
>
> On 10/17/2017 12:03 PM, Antoni Boucher wrote:
>
>> Since the patch was redirected to the right mailing list and the subject
>> was edited, I believe everything is okay, now?
>> Is there any news?
>> Thanks.
>>
>> On Thu, Sep 28, 2017 at 06:56:57PM +, Friedman, Eli wrote:
>>
>> When you're submitting a patch, please make sure you're sending it to the
>>> right list, and the "subject" line actually describes what the patch
>>> changes; otherwise, reviewers won't notice the patch.  Optionally, you can
>>> submit a patch using Phabricator, a tool which which makes patch reviews a
>>> little easier.  See http://llvm.org/docs/Developer
>>> Policy.html#code-reviews
>>>
>>> -Eli
>>>
>>> On 9/28/2017 10:22 AM, Antoni Boucher via cfe-commits wrote:
>>>
>>> Any news on this?

 Aaand the patch itself...
>
> -K
>
> On 9/8/2017 10:32 AM, Krzysztof Parzyszek via cfe-commits wrote:
>
> This should to to cfe-commits. Redirecting.
>>
>> -Krzysztof
>>
>> On 9/8/2017 10:25 AM, Antoni Boucher via llvm-commits wrote:
>>
>> Hello. I've fixed the bug 27628: https://bugs.llvm.org/show_bug
>>> .cgi?id=27628
>>>
>>> I attached the patch.
>>>
>>> Thanks.
>>>
>>>
>>> ___ llvm-commits
>>> mailing list llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
>>
>
> -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> hosted by The Linux Foundation -- next part -- An
> embedded and charset-unspecified text was scrubbed... Name:
> clang-tidy-error-code.diff URL:  il/cfe-commits/attachments/20170908/8a2 98102/attachment.ksh>
>

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

>>>
>>>
>>> -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation
>>> Center, Inc. is a member of Code Aurora Forum, a Linux Foundation
>>> Collaborative Project
>>>
>>>
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r315811 - Re-land r315787, "[Sema] Warn about unused variables if we can constant evaluate the initializer."

[Alexander Kornienko via cfe-commits]

A reduced test case:

struct a {
  a(const char *);
};
template 
void c() {
  a d(b::e ? "" : "");
}

On Thu, Oct 19, 2017 at 5:57 PM, Benjamin Kramer 
wrote:

> We should check VD->getInit()->isValueDependent() before we call
> evaluateValue. I wasn't able to come up with a test case that triggers
> the assert though :(
>
> On Thu, Oct 19, 2017 at 5:45 PM, Alexander Kornienko 
> wrote:
> >
> >
> > On Sat, Oct 14, 2017 at 5:59 PM, Benjamin Kramer via cfe-commits
> >  wrote:
> >>
> >> Author: d0k
> >> Date: Sat Oct 14 08:59:34 2017
> >> New Revision: 315811
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=315811&view=rev
> >> Log:
> >> Re-land r315787, "[Sema] Warn about unused variables if we can constant
> >> evaluate the initializer."
> >>
> >> The warnings in libc++ tests were fixed in the meantime.
> >>
> >> Modified:
> >> cfe/trunk/lib/Sema/SemaDecl.cpp
> >> cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
> >>
> >> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=315811&r1=315810&r2=315811&view=diff
> >>
> >> 
> ==
> >> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> >> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Oct 14 08:59:34 2017
> >> @@ -1723,7 +1723,8 @@ static bool ShouldDiagnoseUnusedDecl(con
> >>  dyn_cast(Init);
> >>if (Construct && !Construct->isElidable()) {
> >>  CXXConstructorDecl *CD = Construct->getConstructor();
> >> -if (!CD->isTrivial() && !RD->hasAttr())
> >> +if (!CD->isTrivial() && !RD->hasAttr() &&
> >> +!VD->evaluateValue())
> >
> >
> > The evaluateValue call above causes an assertion failure on
> > instantiation-dependent values:
> > llvm/tools/clang/lib/AST/Decl.cpp:2196 in clang::APValue
> > *clang::VarDecl::evaluateValue(SmallVectorImpl<
> clang::PartialDiagnosticAt>
> > &) const: !Init->isValueDependent()
> >
> > I'm not sure why evaluateValue uses an assertion instead of outputting a
> > note and returning nullptr, but the assertion can be avoided on the
> caller
> > site as well.
> >
> > Working on a reduced test case...
> >
> >>
> >>return false;
> >>}
> >>  }
> >>
> >> Modified: cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
> >> URL:
> >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/warn-unused-variables.cpp?rev=315811&r1=315810&r2=315811&view=diff
> >>
> >> 
> ==
> >> --- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
> >> +++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Sat Oct 14
> 08:59:34
> >> 2017
> >> @@ -1,4 +1,5 @@
> >>  // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
> >> -Wno-c++1y-extensions -verify %s
> >> +// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
> >> -Wno-c++1y-extensions -verify -std=c++11 %s
> >>  template void f() {
> >>T t;
> >>t = 17;
> >> @@ -194,3 +195,35 @@ void test() {
> >>  }
> >>
> >>  }
> >> +
> >> +#if __cplusplus >= 201103L
> >> +namespace with_constexpr {
> >> +template 
> >> +struct Literal {
> >> +  T i;
> >> +  Literal() = default;
> >> +  constexpr Literal(T i) : i(i) {}
> >> +};
> >> +
> >> +struct NoLiteral {
> >> +  int i;
> >> +  NoLiteral() = default;
> >> +  constexpr NoLiteral(int i) : i(i) {}
> >> +  ~NoLiteral() {}
> >> +};
> >> +
> >> +static Literal gl1;  // expected-warning {{unused variable
> >> 'gl1'}}
> >> +static Literal gl2(1);   // expected-warning {{unused variable
> >> 'gl2'}}
> >> +static const Literal gl3(0); // expected-warning {{unused variable
> >> 'gl3'}}
> >> +
> >> +template 
> >> +void test(int i) {
> >> +  Literal l1; // expected-warning {{unused variable 'l1'}}
> >> +  Literal l2(42); // expected-warning {{unused variable 'l2'}}
> >> +  Literal l3(i);  // no-warning
> >> +  Literal l4(0);// no-warning
> >> +  NoLiteral nl1;   // no-warning
> >> +  NoLiteral nl2(42);   // no-warning
> >> +}
> >> +}
> >> +#endif
> >>
> >>
> >> ___
> >> cfe-commits mailing list
> >> cfe-commits@lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> >
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38578: [preamble] Also record the "skipping" state of the preprocessor

[Cameron via Phabricator via cfe-commits]

cameron314 added a comment.

@nik, pretty sure this fix  I submitted ages 
ago fixes that. Been using it in production a couple years. Would be nice if 
someone reviewed it so we could finally upstream it.


https://reviews.llvm.org/D38578



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

[PATCH] D39066: [libcxx, regex] Fix signed overflow when constructing integers from brace expressions.

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists closed this revision.
mclow.lists added a comment.

Committed as revision 316172.


https://reviews.llvm.org/D39066



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

[libcxx] r316172 - Fix UB - signed integer overflow in regex. Thanks to Tim Shen for the patch. Reviewed as https://reviews.llvm.org/D39066

[Marshall Clow via cfe-commits]

Author: marshall
Date: Thu Oct 19 10:39:16 2017
New Revision: 316172

URL: http://llvm.org/viewvc/llvm-project?rev=316172&view=rev
Log:
Fix UB - signed integer overflow in regex. Thanks to Tim Shen for the patch. 
Reviewed as https://reviews.llvm.org/D39066

Added:
libcxx/trunk/test/std/re/re.grammar/excessive_brace_count.pass.cpp
Modified:
libcxx/trunk/include/regex

Modified: libcxx/trunk/include/regex
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/regex?rev=316172&r1=316171&r2=316172&view=diff
==
--- libcxx/trunk/include/regex (original)
+++ libcxx/trunk/include/regex Thu Oct 19 10:39:16 2017
@@ -4064,6 +4064,8 @@ basic_regex<_CharT, _Traits>::__parse_DU
  __first != __last && ( __val = __traits_.value(*__first, 10)) 
!= -1;
  ++__first)
 {
+if (__c >= std::numeric_limits::max() / 10)
+__throw_regex_error();
 __c *= 10;
 __c += __val;
 }

Added: libcxx/trunk/test/std/re/re.grammar/excessive_brace_count.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/re/re.grammar/excessive_brace_count.pass.cpp?rev=316172&view=auto
==
--- libcxx/trunk/test/std/re/re.grammar/excessive_brace_count.pass.cpp (added)
+++ libcxx/trunk/test/std/re/re.grammar/excessive_brace_count.pass.cpp Thu Oct 
19 10:39:16 2017
@@ -0,0 +1,40 @@
+//===--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===--===//
+
+// 
+// UNSUPPORTED: libcpp-no-exceptions
+// UNSUPPORTED: c++03
+
+// the "n" in `a{n}` should be within the numeric limits.
+
+#include 
+#include 
+
+int main() {
+  for (std::regex_constants::syntax_option_type op :
+   {std::regex::basic, std::regex::grep}) {
+try {
+  (void)std::regex("a\\{10\\}", op);
+  assert(false);
+} catch (const std::regex_error &e) {
+  assert(e.code() == std::regex_constants::error_badbrace);
+}
+  }
+  for (std::regex_constants::syntax_option_type op :
+   {std::regex::ECMAScript, std::regex::extended, std::regex::egrep,
+std::regex::awk}) {
+try {
+  (void)std::regex("a{10}", op);
+  assert(false);
+} catch (const std::regex_error &e) {
+  assert(e.code() == std::regex_constants::error_badbrace);
+}
+  }
+  return 0;
+}


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

[PATCH] D39066: [libcxx, regex] Fix signed overflow when constructing integers from brace expressions.

[Marshall Clow via Phabricator via cfe-commits]

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

LGTM.


https://reviews.llvm.org/D39066



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

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D38101#901709, @materi wrote:

> Hi!


Hi.

> After this patch I started to see warnings:

Thank you for the report!

>   e.c:8:23: warning: integer constant not in range of enumerated type 'enum 
> E' [-Wassign-enum]
> enum E {a = 7,} e = 1000;
> ^
>   e.c:10:12: warning: comparison 'enum E' > 7 is always false 
> [-Wtautological-constant-compare]
> return e > 7;
> 
> 
> Isn't the the "always false" message misleading? It's only "always false" if 
> e was initialized with an in-range value. Maybe the tautology check should be 
> on the enum's underlying type instead?

I agree, please open a bug, i'll look into this.


Repository:
  rL LLVM

https://reviews.llvm.org/D38101



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

[PATCH] D38101: [Sema] Diagnose tautological comparison with type's min/max values

[mattias.v.eriks...@ericsson.com via Phabricator via cfe-commits]

materi added a comment.

Hi!

After this patch I started to see warnings:

  e.c:8:23: warning: integer constant not in range of enumerated type 'enum E' 
[-Wassign-enum]
enum E {a = 7,} e = 1000;
^
  e.c:10:12: warning: comparison 'enum E' > 7 is always false 
[-Wtautological-constant-compare]
return e > 7;

Isn't the the "always false" message misleading? It's only "always false" if e 
was initialized with an in-range value. Maybe the tautology check should be on 
the enum's underlying type instead?


Repository:
  rL LLVM

https://reviews.llvm.org/D38101



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

[PATCH] D39092: [clang-refactor] Add "-Inplace" option to the commandline tool.

[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


https://reviews.llvm.org/D39092



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

Re: r316152 - Fix a few nits in RenamingAction.

[Alex L via cfe-commits]

Thanks!

On 19 October 2017 at 01:20, Haojian Wu via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: hokein
> Date: Thu Oct 19 01:20:55 2017
> New Revision: 316152
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316152&view=rev
> Log:
> Fix a few nits in RenamingAction.
>
> * Add missing override keyword.
> * avoid unnecessary copy of std::string.
>
> Modified:
> cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
>
> Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/
> Refactoring/Rename/RenamingAction.cpp?rev=316152&
> r1=316151&r2=316152&view=diff
> 
> ==
> --- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp (original)
> +++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp Thu Oct
> 19 01:20:55 2017
> @@ -77,10 +77,10 @@ private:
>  class RenameOccurrences final : public SourceChangeRefactoringRule {
>  public:
>RenameOccurrences(const NamedDecl *ND, std::string NewName)
> -  : Finder(ND), NewName(NewName) {}
> +  : Finder(ND), NewName(std::move(NewName)) {}
>
>Expected
> -  createSourceReplacements(RefactoringRuleContext &Context) {
> +  createSourceReplacements(RefactoringRuleContext &Context) override {
>  Expected Occurrences =
>  Finder.findSymbolOccurrences(Context);
>  if (!Occurrences)
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30489: [analyzer] catch out of bounds for VLA

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki updated this revision to Diff 119590.
danielmarjamaki added a comment.
Herald added a subscriber: szepet.

As suggested, use a ProgramState trait to detect VLA overflows.

I did not yet manage to get a SubRegion from the DeclStmt that matches the 
location SubRegion. Therefore I am using VariableArrayType in the trait for now.


Repository:
  rL LLVM

https://reviews.llvm.org/D30489

Files:
  lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
  test/Analysis/out-of-bounds.c

Index: test/Analysis/out-of-bounds.c
===
--- test/Analysis/out-of-bounds.c
+++ test/Analysis/out-of-bounds.c
@@ -174,3 +174,7 @@
   clang_analyzer_eval(x <= 99); // expected-warning{{TRUE}}
 }
 
+void vla(int X) {
+  char buf[X];
+  buf[X] = 0; // expected-warning {{Out of bound memory access (access exceeds upper limit of memory block)}}
+}
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -27,17 +27,18 @@
 using namespace ento;
 
 namespace {
-class ArrayBoundCheckerV2 :
-public Checker {
+class ArrayBoundCheckerV2
+: public Checker> {
   mutable std::unique_ptr BT;
 
   enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted };
 
   void reportOOB(CheckerContext &C, ProgramStateRef errorState,
  OOB_Kind kind) const;
 
 public:
-  void checkLocation(SVal l, bool isLoad, const Stmt*S,
+  void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
+  void checkLocation(SVal l, bool isLoad, const Stmt *S,
  CheckerContext &C) const;
 };
 
@@ -64,7 +65,10 @@
   void dump() const;
   void dumpToStream(raw_ostream &os) const;
 };
-}
+} // namespace
+
+REGISTER_MAP_WITH_PROGRAMSTATE(VariableLengthArrayExtent,
+   const VariableArrayType *, NonLoc);
 
 static SVal computeExtentBegin(SValBuilder &svalBuilder,
const MemRegion *region) {
@@ -111,8 +115,46 @@
   return std::pair(offset, extent);
 }
 
+void ArrayBoundCheckerV2::checkPreStmt(const DeclStmt *DS,
+   CheckerContext &checkerContext) const {
+  const VarDecl *VD = dyn_cast(DS->getSingleDecl());
+  if (!VD)
+return;
+
+  ASTContext &Ctx = checkerContext.getASTContext();
+  const VariableArrayType *VLA = Ctx.getAsVariableArrayType(VD->getType());
+  if (!VLA)
+return;
+
+  SVal VLASize = checkerContext.getSVal(VLA->getSizeExpr());
+
+  Optional Sz = VLASize.getAs();
+  if (!Sz)
+return;
+
+  ProgramStateRef state = checkerContext.getState();
+  checkerContext.addTransition(
+  state->set(VLA, Sz.getValue()));
+}
+
+static const VariableArrayType *getVLAFromExtent(DefinedOrUnknownSVal extentVal,
+ ASTContext &Ctx) {
+  if (extentVal.getSubKind() != nonloc::SymbolValKind)
+return nullptr;
+
+  SymbolRef SR = extentVal.castAs().getSymbol();
+  const SymbolExtent *SE = dyn_cast(SR);
+  const MemRegion *SEMR = SE->getRegion();
+  if (SEMR->getKind() != MemRegion::VarRegionKind)
+return nullptr;
+
+  const VarRegion *VR = cast(SEMR);
+  QualType T = VR->getDecl()->getType();
+  return Ctx.getAsVariableArrayType(T);
+}
+
 void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
-const Stmt* LoadS,
+const Stmt *LoadS,
 CheckerContext &checkerContext) const {
 
   // NOTE: Instead of using ProgramState::assumeInBound(), we are prototyping
@@ -175,13 +217,21 @@
   }
 
   do {
+
 // CHECK UPPER BOUND: Is byteOffset >= extent(baseRegion)?  If so,
 // we are doing a load/store after the last valid offset.
 DefinedOrUnknownSVal extentVal =
-  rawOffset.getRegion()->getExtent(svalBuilder);
+rawOffset.getRegion()->getExtent(svalBuilder);
 if (!extentVal.getAs())
   break;
 
+if (const VariableArrayType *VLA =
+getVLAFromExtent(extentVal, checkerContext.getASTContext())) {
+  const NonLoc *V = state->get(VLA);
+  if (V)
+extentVal = *V;
+}
+
 if (extentVal.getAs()) {
   std::pair simplifiedOffsets =
   getSimplifiedOffsets(rawOffset.getByteOffset(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39083: [CodeGen] Fix generation of TBAA info for array-to-pointer conversions

[Hal Finkel via Phabricator via cfe-commits]

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

LGTM


Repository:
  rL LLVM

https://reviews.llvm.org/D39083



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

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

Still LGTM.


https://reviews.llvm.org/D39042



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

[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added inline comments.



Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:139
 
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
-   bool RefersToEnclosingVariableOrCapture,
-   bool GetNonReferenceType) {
-  auto Type = D->getType();
-  if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,

this looks strange, did clang-format do this?


Repository:
  rL LLVM

https://reviews.llvm.org/D39015



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

[PATCH] D38986: [Analyzer] Better unreachable message in enumeration

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.

> I think it is much better when the assert failure tells the developer _what_ 
> value is failing, rather than saying "oops we are dead".

yes of course, more informative assert messages is better.


https://reviews.llvm.org/D38986



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

r316166 - Fix nodiscard for volatile references

[Erich Keane via cfe-commits]

Author: erichkeane
Date: Thu Oct 19 08:58:58 2017
New Revision: 316166

URL: http://llvm.org/viewvc/llvm-project?rev=316166&view=rev
Log:
Fix nodiscard for volatile references

As reported here https://bugs.llvm.org/show_bug.cgi?id=34988
[[nodiscard]] warnings were not being suppressed for
volatile-ref return values.

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

Modified:
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=316166&r1=316165&r2=316166&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Thu Oct 19 08:58:58 2017
@@ -2298,7 +2298,8 @@ bool Expr::isUnusedResultAWarning(const
 const DeclRefExpr *DRE =
 dyn_cast(CE->getSubExpr()->IgnoreParens());
 if (!(DRE && isa(DRE->getDecl()) &&
-  cast(DRE->getDecl())->hasLocalStorage())) {
+  cast(DRE->getDecl())->hasLocalStorage()) &&
+!isa(CE->getSubExpr()->IgnoreParens())) {
   return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
   R1, R2, Ctx);
 }

Modified: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp?rev=316166&r1=316165&r2=316166&view=diff
==
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp (original)
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp Thu Oct 19 
08:58:58 2017
@@ -9,21 +9,33 @@ enum [[nodiscard]] E {};
 E get_e();
 
 [[nodiscard]] int get_i();
+[[nodiscard]] volatile int &get_vi();
 
 void f() {
   get_s(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
   get_i(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
+  get_vi(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
   get_e(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
 
   // Okay, warnings are not encouraged
   get_s_ref();
   (void)get_s();
   (void)get_i();
+  (void)get_vi();
   (void)get_e();
 }
 
+[[nodiscard]] volatile char &(*fp)();
+void g() {
+  fp(); // expected-warning {{ignoring return value of function declared with 
'nodiscard' attribute}}
+
+  // OK, warning suppressed.
+  (void)fp();
+}
 #ifdef EXT
 // expected-warning@4 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@8 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@11 {{use of the 'nodiscard' attribute is a C++17 
extension}}
+// expected-warning@12 {{use of the 'nodiscard' attribute is a C++17 
extension}}
+// expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 
extension}}
 #endif


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

Re: r315811 - Re-land r315787, "[Sema] Warn about unused variables if we can constant evaluate the initializer."

[Benjamin Kramer via cfe-commits]

We should check VD->getInit()->isValueDependent() before we call
evaluateValue. I wasn't able to come up with a test case that triggers
the assert though :(

On Thu, Oct 19, 2017 at 5:45 PM, Alexander Kornienko  wrote:
>
>
> On Sat, Oct 14, 2017 at 5:59 PM, Benjamin Kramer via cfe-commits
>  wrote:
>>
>> Author: d0k
>> Date: Sat Oct 14 08:59:34 2017
>> New Revision: 315811
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=315811&view=rev
>> Log:
>> Re-land r315787, "[Sema] Warn about unused variables if we can constant
>> evaluate the initializer."
>>
>> The warnings in libc++ tests were fixed in the meantime.
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=315811&r1=315810&r2=315811&view=diff
>>
>> ==
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Oct 14 08:59:34 2017
>> @@ -1723,7 +1723,8 @@ static bool ShouldDiagnoseUnusedDecl(con
>>  dyn_cast(Init);
>>if (Construct && !Construct->isElidable()) {
>>  CXXConstructorDecl *CD = Construct->getConstructor();
>> -if (!CD->isTrivial() && !RD->hasAttr())
>> +if (!CD->isTrivial() && !RD->hasAttr() &&
>> +!VD->evaluateValue())
>
>
> The evaluateValue call above causes an assertion failure on
> instantiation-dependent values:
> llvm/tools/clang/lib/AST/Decl.cpp:2196 in clang::APValue
> *clang::VarDecl::evaluateValue(SmallVectorImpl
> &) const: !Init->isValueDependent()
>
> I'm not sure why evaluateValue uses an assertion instead of outputting a
> note and returning nullptr, but the assertion can be avoided on the caller
> site as well.
>
> Working on a reduced test case...
>
>>
>>return false;
>>}
>>  }
>>
>> Modified: cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-variables.cpp?rev=315811&r1=315810&r2=315811&view=diff
>>
>> ==
>> --- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Sat Oct 14 08:59:34
>> 2017
>> @@ -1,4 +1,5 @@
>>  // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
>> -Wno-c++1y-extensions -verify %s
>> +// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
>> -Wno-c++1y-extensions -verify -std=c++11 %s
>>  template void f() {
>>T t;
>>t = 17;
>> @@ -194,3 +195,35 @@ void test() {
>>  }
>>
>>  }
>> +
>> +#if __cplusplus >= 201103L
>> +namespace with_constexpr {
>> +template 
>> +struct Literal {
>> +  T i;
>> +  Literal() = default;
>> +  constexpr Literal(T i) : i(i) {}
>> +};
>> +
>> +struct NoLiteral {
>> +  int i;
>> +  NoLiteral() = default;
>> +  constexpr NoLiteral(int i) : i(i) {}
>> +  ~NoLiteral() {}
>> +};
>> +
>> +static Literal gl1;  // expected-warning {{unused variable
>> 'gl1'}}
>> +static Literal gl2(1);   // expected-warning {{unused variable
>> 'gl2'}}
>> +static const Literal gl3(0); // expected-warning {{unused variable
>> 'gl3'}}
>> +
>> +template 
>> +void test(int i) {
>> +  Literal l1; // expected-warning {{unused variable 'l1'}}
>> +  Literal l2(42); // expected-warning {{unused variable 'l2'}}
>> +  Literal l3(i);  // no-warning
>> +  Literal l4(0);// no-warning
>> +  NoLiteral nl1;   // no-warning
>> +  NoLiteral nl2(42);   // no-warning
>> +}
>> +}
>> +#endif
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316165 - [AMDGPU] Fix bug in enqueued block codegen due to an extra line

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Oct 19 08:56:13 2017
New Revision: 316165

URL: http://llvm.org/viewvc/llvm-project?rev=316165&view=rev
Log:
[AMDGPU] Fix bug in enqueued block codegen due to an extra line

Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp
cfe/trunk/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=316165&r1=316164&r2=316165&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Thu Oct 19 08:56:13 2017
@@ -8985,7 +8985,6 @@ llvm::Function *AMDGPUTargetCodeGenInfo:
   ArgNames.push_back(llvm::MDString::get(C, "block_literal"));
   for (unsigned I = 1, E = InvokeFT->getNumParams(); I < E; ++I) {
 ArgTys.push_back(InvokeFT->getParamType(I));
-ArgTys.push_back(BlockTy);
 ArgTypeNames.push_back(llvm::MDString::get(C, "void*"));
 AddressQuals.push_back(llvm::ConstantAsMetadata::get(Builder.getInt32(3)));
 AccessQuals.push_back(llvm::MDString::get(C, "none"));

Modified: cfe/trunk/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl?rev=316165&r1=316164&r2=316165&view=diff
==
--- cfe/trunk/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl (original)
+++ cfe/trunk/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl Thu Oct 19 08:56:13 
2017
@@ -18,6 +18,12 @@ kernel void test(global char *a, char b,
  a[0] = b;
  c[0] = d;
  });
+  enqueue_kernel(default_queue, flags, ndrange,
+ ^(local void *lp) {
+ a[0] = b;
+ c[0] = d;
+ ((local int*)lp)[0] = 1;
+ }, 100);
 }
 
 // CHECK-LABEL: define internal amdgpu_kernel void 
@__test_block_invoke_kernel(<{ i32, i32, i8 addrspace(4)*, i8 addrspace(1)*, i8 
}>)
@@ -33,4 +39,7 @@ kernel void test(global char *a, char b,
 // CHECK-LABEL: define internal amdgpu_kernel void 
@__test_block_invoke_2_kernel(<{ i32, i32, i8 addrspace(4)*, i8 addrspace(1)*, 
i64 addrspace(1)*, i64, i8 }>)
 // CHECK-SAME: #[[ATTR]] !kernel_arg_addr_space !{{.*}} 
!kernel_arg_access_qual !{{.*}} !kernel_arg_type !{{.*}} !kernel_arg_base_type 
!{{.*}} !kernel_arg_type_qual !{{.*}}
 
+// CHECK-LABEL: define internal amdgpu_kernel void 
@__test_block_invoke_3_kernel(<{ i32, i32, i8 addrspace(4)*, i8 addrspace(1)*, 
i64 addrspace(1)*, i64, i8 }>, i8 addrspace(3)*)
+// CHECK-SAME: #[[ATTR]] !kernel_arg_addr_space !{{.*}} 
!kernel_arg_access_qual !{{.*}} !kernel_arg_type !{{.*}} !kernel_arg_base_type 
!{{.*}} !kernel_arg_type_qual !{{.*}}
+
 // CHECK: attributes #[[ATTR]] = { nounwind "enqueued-block" }


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

[PATCH] D39075: Fix nodiscard for volatile references

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316166: Fix nodiscard for volatile references (authored by 
erichkeane).

Changed prior to commit:
  https://reviews.llvm.org/D39075?vs=119535&id=119588#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39075

Files:
  cfe/trunk/lib/AST/Expr.cpp
  cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp


Index: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
===
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
@@ -9,21 +9,33 @@
 E get_e();
 
 [[nodiscard]] int get_i();
+[[nodiscard]] volatile int &get_vi();
 
 void f() {
   get_s(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
   get_i(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
+  get_vi(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
   get_e(); // expected-warning {{ignoring return value of function declared 
with 'nodiscard' attribute}}
 
   // Okay, warnings are not encouraged
   get_s_ref();
   (void)get_s();
   (void)get_i();
+  (void)get_vi();
   (void)get_e();
 }
 
+[[nodiscard]] volatile char &(*fp)();
+void g() {
+  fp(); // expected-warning {{ignoring return value of function declared with 
'nodiscard' attribute}}
+
+  // OK, warning suppressed.
+  (void)fp();
+}
 #ifdef EXT
 // expected-warning@4 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@8 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@11 {{use of the 'nodiscard' attribute is a C++17 
extension}}
+// expected-warning@12 {{use of the 'nodiscard' attribute is a C++17 
extension}}
+// expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 
extension}}
 #endif
Index: cfe/trunk/lib/AST/Expr.cpp
===
--- cfe/trunk/lib/AST/Expr.cpp
+++ cfe/trunk/lib/AST/Expr.cpp
@@ -2298,7 +2298,8 @@
 const DeclRefExpr *DRE =
 dyn_cast(CE->getSubExpr()->IgnoreParens());
 if (!(DRE && isa(DRE->getDecl()) &&
-  cast(DRE->getDecl())->hasLocalStorage())) {
+  cast(DRE->getDecl())->hasLocalStorage()) &&
+!isa(CE->getSubExpr()->IgnoreParens())) {
   return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
   R1, R2, Ctx);
 }


Index: cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
===
--- cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
+++ cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
@@ -9,21 +9,33 @@
 E get_e();
 
 [[nodiscard]] int get_i();
+[[nodiscard]] volatile int &get_vi();
 
 void f() {
   get_s(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
   get_i(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  get_vi(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
   get_e(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 
   // Okay, warnings are not encouraged
   get_s_ref();
   (void)get_s();
   (void)get_i();
+  (void)get_vi();
   (void)get_e();
 }
 
+[[nodiscard]] volatile char &(*fp)();
+void g() {
+  fp(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  // OK, warning suppressed.
+  (void)fp();
+}
 #ifdef EXT
 // expected-warning@4 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@8 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@11 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// expected-warning@12 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 extension}}
 #endif
Index: cfe/trunk/lib/AST/Expr.cpp
===
--- cfe/trunk/lib/AST/Expr.cpp
+++ cfe/trunk/lib/AST/Expr.cpp
@@ -2298,7 +2298,8 @@
 const DeclRefExpr *DRE =
 dyn_cast(CE->getSubExpr()->IgnoreParens());
 if (!(DRE && isa(DRE->getDecl()) &&
-  cast(DRE->getDecl())->hasLocalStorage())) {
+  cast(DRE->getDecl())->hasLocalStorage()) &&
+!isa(CE->getSubExpr()->IgnoreParens())) {
   return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
   R1, R2, Ctx);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a comment.

I like this patch overall.. here are some stylistic nits.




Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:610
+} else {
+  if (*lInt >= *rInt) {
+newRhsExt = lInt->getExtValue() - rInt->getExtValue();

you can use `else if` here



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:595
+
+  if (origWidth < 128) {
+auto newWidth = std::max(2 * origWidth, (uint32_t) 8);

I would like that "128" is rewritten somehow. Using expression instead.



Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:596
+  if (origWidth < 128) {
+auto newWidth = std::max(2 * origWidth, (uint32_t) 8);
+auto newAPSIntType = APSIntType(newWidth, false);

Is `origWidth < 4` possible?

I wonder about "8". Is that CHAR_BIT hardcoded?


https://reviews.llvm.org/D35109



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

Re: r315811 - Re-land r315787, "[Sema] Warn about unused variables if we can constant evaluate the initializer."

[Alexander Kornienko via cfe-commits]

On Sat, Oct 14, 2017 at 5:59 PM, Benjamin Kramer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: d0k
> Date: Sat Oct 14 08:59:34 2017
> New Revision: 315811
>
> URL: http://llvm.org/viewvc/llvm-project?rev=315811&view=rev
> Log:
> Re-land r315787, "[Sema] Warn about unused variables if we can constant
> evaluate the initializer."
>
> The warnings in libc++ tests were fixed in the meantime.
>
> Modified:
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
> SemaDecl.cpp?rev=315811&r1=315810&r2=315811&view=diff
> 
> ==
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Sat Oct 14 08:59:34 2017
> @@ -1723,7 +1723,8 @@ static bool ShouldDiagnoseUnusedDecl(con
>  dyn_cast(Init);
>if (Construct && !Construct->isElidable()) {
>  CXXConstructorDecl *CD = Construct->getConstructor();
> -if (!CD->isTrivial() && !RD->hasAttr())
> +if (!CD->isTrivial() && !RD->hasAttr() &&
> +!VD->evaluateValue())
>

The evaluateValue call above causes an assertion failure on
instantiation-dependent values:
llvm/tools/clang/lib/AST/Decl.cpp:2196 in clang::APValue
*clang::VarDecl::evaluateValue(SmallVectorImpl
&) const: !Init->isValueDependent()

I'm not sure why evaluateValue uses an assertion instead of outputting a
note and returning nullptr, but the assertion can be avoided on the caller
site as well.

Working on a reduced test case...


>return false;
>}
>  }
>
> Modified: cfe/trunk/test/SemaCXX/warn-unused-variables.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> SemaCXX/warn-unused-variables.cpp?rev=315811&r1=315810&r2=315811&view=diff
> 
> ==
> --- cfe/trunk/test/SemaCXX/warn-unused-variables.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-unused-variables.cpp Sat Oct 14 08:59:34
> 2017
> @@ -1,4 +1,5 @@
>  // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
> -Wno-c++1y-extensions -verify %s
> +// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label
> -Wno-c++1y-extensions -verify -std=c++11 %s
>  template void f() {
>T t;
>t = 17;
> @@ -194,3 +195,35 @@ void test() {
>  }
>
>  }
> +
> +#if __cplusplus >= 201103L
> +namespace with_constexpr {
> +template 
> +struct Literal {
> +  T i;
> +  Literal() = default;
> +  constexpr Literal(T i) : i(i) {}
> +};
> +
> +struct NoLiteral {
> +  int i;
> +  NoLiteral() = default;
> +  constexpr NoLiteral(int i) : i(i) {}
> +  ~NoLiteral() {}
> +};
> +
> +static Literal gl1;  // expected-warning {{unused variable
> 'gl1'}}
> +static Literal gl2(1);   // expected-warning {{unused variable
> 'gl2'}}
> +static const Literal gl3(0); // expected-warning {{unused variable
> 'gl3'}}
> +
> +template 
> +void test(int i) {
> +  Literal l1; // expected-warning {{unused variable 'l1'}}
> +  Literal l2(42); // expected-warning {{unused variable 'l2'}}
> +  Literal l3(i);  // no-warning
> +  Literal l4(0);// no-warning
> +  NoLiteral nl1;   // no-warning
> +  NoLiteral nl2(42);   // no-warning
> +}
> +}
> +#endif
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39031: [Analyzer] Correctly handle parameters passed by reference when bodyfarming std::call_once

[Daniel Marjamäki via Phabricator via cfe-commits]

danielmarjamaki added a reviewer: danielmarjamaki.
danielmarjamaki added a comment.

Stylistically this looks pretty good to me. Just a minor nit.




Comment at: lib/Analysis/BodyFarm.cpp:389
+  for (unsigned int i = 2; i < D->getNumParams(); i++) {
+
+const ParmVarDecl *PDecl = D->getParamDecl(i);

ehm.. I would remove this blank


https://reviews.llvm.org/D39031



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

[PATCH] D39075: Fix nodiscard for volatile references

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


https://reviews.llvm.org/D39075



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

[PATCH] D39042: [Tooling] Add a factory method for CommonOptionsParser

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 119583.
ioeric added a comment.

- Make the factory method return an expected object instead of unique_ptr.
- clang-format code.


https://reviews.llvm.org/D39042

Files:
  include/clang/Tooling/CommonOptionsParser.h
  lib/Tooling/CommonOptionsParser.cpp

Index: lib/Tooling/CommonOptionsParser.cpp
===
--- lib/Tooling/CommonOptionsParser.cpp
+++ lib/Tooling/CommonOptionsParser.cpp
@@ -24,9 +24,9 @@
 //
 //===--===//
 
-#include "llvm/Support/CommandLine.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
 
 using namespace clang::tooling;
 using namespace llvm;
@@ -81,7 +81,7 @@
   return Commands;
 }
 
-CommonOptionsParser::CommonOptionsParser(
+llvm::Error CommonOptionsParser::init(
 int &argc, const char **argv, cl::OptionCategory &Category,
 llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
   static cl::opt Help("h", cl::desc("Alias for -help"), cl::Hidden,
@@ -105,20 +105,30 @@
   cl::desc("Additional argument to prepend to the compiler command line"),
   cl::cat(Category), cl::sub(*cl::AllSubCommands));
 
+  cl::ResetAllOptionOccurrences();
+
   cl::HideUnrelatedOptions(Category);
 
   std::string ErrorMessage;
   Compilations =
   FixedCompilationDatabase::loadFromCommandLine(argc, argv, ErrorMessage);
-  if (!Compilations && !ErrorMessage.empty())
-llvm::errs() << ErrorMessage;
-  cl::ParseCommandLineOptions(argc, argv, Overview);
+  if (!ErrorMessage.empty())
+ErrorMessage.append("\n");
+  llvm::raw_string_ostream OS(ErrorMessage);
+  // Stop initializing if command-line option parsing failed.
+  if (!cl::ParseCommandLineOptions(argc, argv, Overview, &OS)) {
+OS.flush();
+return llvm::make_error("[CommonOptionsParser]: " +
+   ErrorMessage,
+   llvm::inconvertibleErrorCode());
+  }
+
   cl::PrintOptionValues();
 
   SourcePathList = SourcePaths;
   if ((OccurrencesFlag == cl::ZeroOrMore || OccurrencesFlag == cl::Optional) &&
   SourcePathList.empty())
-return;
+return llvm::Error::success();
   if (!Compilations) {
 if (!BuildPath.empty()) {
   Compilations =
@@ -142,4 +152,27 @@
   AdjustingCompilations->appendArgumentsAdjuster(
   getInsertArgumentAdjuster(ArgsAfter, ArgumentInsertPosition::END));
   Compilations = std::move(AdjustingCompilations);
+  return llvm::Error::success();
+}
+
+llvm::Expected CommonOptionsParser::create(
+int &argc, const char **argv, llvm::cl::OptionCategory &Category,
+llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
+  CommonOptionsParser Parser;
+  llvm::Error Err =
+  Parser.init(argc, argv, Category, OccurrencesFlag, Overview);
+  if (Err)
+return std::move(Err);
+  return std::move(Parser);
+}
+
+CommonOptionsParser::CommonOptionsParser(
+int &argc, const char **argv, cl::OptionCategory &Category,
+llvm::cl::NumOccurrencesFlag OccurrencesFlag, const char *Overview) {
+  llvm::Error Err = init(argc, argv, Category, OccurrencesFlag, Overview);
+  if (Err) {
+llvm::report_fatal_error(
+"CommonOptionsParser: failed to parse command-line arguments. " +
+llvm::toString(std::move(Err)));
+  }
 }
Index: include/clang/Tooling/CommonOptionsParser.h
===
--- include/clang/Tooling/CommonOptionsParser.h
+++ include/clang/Tooling/CommonOptionsParser.h
@@ -30,6 +30,7 @@
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
 
 namespace clang {
 namespace tooling {
@@ -86,13 +87,18 @@
   /// All options not belonging to \p Category become hidden.
   ///
   /// It also allows calls to set the required number of positional parameters.
-  ///
-  /// This constructor exits program in case of error.
   CommonOptionsParser(int &argc, const char **argv,
   llvm::cl::OptionCategory &Category,
   llvm::cl::NumOccurrencesFlag OccurrencesFlag,
   const char *Overview = nullptr);
 
+  /// \brief A factory method that is similar to the above constructor, except
+  /// this returns an error instead exiting the program on error.
+  static llvm::Expected
+  create(int &argc, const char **argv, llvm::cl::OptionCategory &Category,
+ llvm::cl::NumOccurrencesFlag OccurrencesFlag,
+ const char *Overview = nullptr);
+
   /// Returns a reference to the loaded compilations database.
   CompilationDatabase &getCompilations() {
 return *Compilations;
@@ -106,6 +112,13 @@
   static const char *const HelpMessage;
 
 private:
+  CommonOptionsParser() = default;
+
+  llvm::Error init(i

[PATCH] D34272: [Tooling] A new framework for executing clang frontend actions.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 119580.
ioeric added a comment.

- Remove unused variable.
- Narrow supported actions to FrontendActionFactory from ToolAction.


https://reviews.llvm.org/D34272

Files:
  include/clang/Tooling/CommonOptionsParser.h
  include/clang/Tooling/Execution.h
  include/clang/Tooling/StandaloneExecution.h
  include/clang/Tooling/ToolExecutorPluginRegistry.h
  include/clang/Tooling/Tooling.h
  lib/Tooling/CMakeLists.txt
  lib/Tooling/CommonOptionsParser.cpp
  lib/Tooling/Execution.cpp
  lib/Tooling/StandaloneExecution.cpp
  lib/Tooling/Tooling.cpp
  unittests/Tooling/CMakeLists.txt
  unittests/Tooling/ExecutionTest.cpp

Index: unittests/Tooling/ExecutionTest.cpp
===
--- /dev/null
+++ unittests/Tooling/ExecutionTest.cpp
@@ -0,0 +1,250 @@
+//===- unittest/Tooling/ExecutionTest.cpp - Tool execution tests. ===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "clang/AST/ASTConsumer.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Frontend/ASTUnit.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Execution.h"
+#include "clang/Tooling/StandaloneExecution.h"
+#include "clang/Tooling/ToolExecutorPluginRegistry.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace tooling {
+
+namespace {
+
+// This traverses the AST and outputs function name as key and "1" as value for
+// each function declaration.
+class ASTConsumerWithResult
+: public ASTConsumer,
+  public RecursiveASTVisitor {
+public:
+  using ASTVisitor = RecursiveASTVisitor;
+
+  explicit ASTConsumerWithResult(ExecutionContext *Context) : Context(Context) {
+assert(Context != nullptr);
+  }
+
+  void HandleTranslationUnit(clang::ASTContext &Context) override {
+TraverseDecl(Context.getTranslationUnitDecl());
+  }
+
+  bool TraverseFunctionDecl(clang::FunctionDecl *Decl) {
+Context->getToolResults()->addResult(Decl->getNameAsString(), "1");
+return ASTVisitor::TraverseFunctionDecl(Decl);
+  }
+
+private:
+  ExecutionContext *const Context;
+};
+
+class ReportResultAction : public ASTFrontendAction {
+public:
+  explicit ReportResultAction(ExecutionContext *Context) : Context(Context) {
+assert(Context != nullptr);
+  }
+
+protected:
+  std::unique_ptr
+  CreateASTConsumer(clang::CompilerInstance &compiler,
+StringRef /* dummy */) override {
+std::unique_ptr ast_consumer{
+new ASTConsumerWithResult(Context)};
+return ast_consumer;
+  }
+
+private:
+  ExecutionContext *const Context;
+};
+
+class ReportResultActionFactory : public FrontendActionFactory {
+public:
+  ReportResultActionFactory(ExecutionContext *Context) : Context(Context) {}
+  FrontendAction *create() override { return new ReportResultAction(Context); }
+
+private:
+  ExecutionContext *const Context;
+};
+
+inline llvm::Error make_string_error(const llvm::Twine &Message) {
+  return llvm::make_error(Message,
+ llvm::inconvertibleErrorCode());
+}
+
+} // namespace
+
+class TestToolExecutor : public ToolExecutor {
+public:
+  static const char *ExecutorName;
+
+  TestToolExecutor(std::unique_ptr Options)
+  : OptionsParser(std::move(Options)) {}
+
+  StringRef getExecutorName() const override { return ExecutorName; }
+
+  llvm::Error execute(const ExecutionConfig &) override {
+return llvm::Error::success();
+  }
+
+  ExecutionContext *getExecutionContext() override { return nullptr; };
+
+  llvm::ArrayRef getSourcePaths() const {
+return OptionsParser->getSourcePathList();
+  }
+
+  void mapVirtualFile(StringRef FilePath, StringRef Content) override {
+VFS[FilePath] = Content;
+  }
+
+private:
+  std::unique_ptr OptionsParser;
+  std::string SourcePaths;
+  std::map VFS;
+};
+
+const char *TestToolExecutor::ExecutorName = "TestToolExecutor";
+
+class TestToolExecutorPlugin : public ToolExecutorPlugin {
+public:
+  llvm::Expected>
+  create(int &argc, const char **argv,
+ llvm::cl::OptionCategory &Category) override {
+// Depending on the test runner, test cases might run in different threads
+// in the same process, so we put test options in the stack and manually
+// remove it from the global registry later so that tests do not interfere
+// with each other.
+llvm::cl::opt TestExecutor("test_executor",
+ llvm::cl::desc("Use TestToolExecutor"));
+auto OptionsParser = CommonOptionsParser::create(
+argc, argv, Category, llvm::cl::OneOrMore, /*Overview=*/

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

[Peter Szecsi via Phabricator via cfe-commits]

szepet updated this revision to Diff 119579.
szepet added a comment.

Checking for importing base updated to (!Imported && From) style.


https://reviews.llvm.org/D38843

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


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -535,5 +535,21 @@
  binaryOperator(has(cxxUnresolvedConstructExpr()));
 }
 
+const internal::VariadicDynCastAllOfMatcher
+cxxPseudoDestructorExpr;
+
+TEST(ImportExpr, ImportCXXPseudoDestructorExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+  testImport("typedef int T;"
+ "void declToImport(int *p) {"
+ "p->T::~T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(has(compoundStmt(has(
+ callExpr(has(cxxPseudoDestructorExpr();
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -278,6 +278,7 @@
 Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
 Expr *VisitCXXThisExpr(CXXThisExpr *E);
 Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
+Expr *VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E);
 Expr *VisitMemberExpr(MemberExpr *E);
 Expr *VisitCallExpr(CallExpr *E);
 Expr *VisitInitListExpr(InitListExpr *E);
@@ -5540,6 +5541,38 @@
   E->isOverloaded(), ToDecls.begin(), ToDecls.end());
 }
 
+
+Expr *ASTNodeImporter::VisitCXXPseudoDestructorExpr(
+CXXPseudoDestructorExpr *E) {
+
+  Expr *BaseE = Importer.Import(E->getBase());
+  if (!BaseE && E->getBase())
+return nullptr;
+
+  TypeSourceInfo *ScopeInfo = Importer.Import(E->getScopeTypeInfo());
+
+  PseudoDestructorTypeStorage Storage;
+  if (IdentifierInfo *FromII = E->getDestroyedTypeIdentifier()) {
+IdentifierInfo *ToII = Importer.Import(FromII);
+if (!ToII)
+  return nullptr;
+Storage = PseudoDestructorTypeStorage(
+  ToII, Importer.Import(E->getDestroyedTypeLoc()));
+  } else {
+TypeSourceInfo *TI = Importer.Import(E->getDestroyedTypeInfo());
+if (!TI)
+  return nullptr;
+Storage = PseudoDestructorTypeStorage(TI);
+  }
+
+  return new (Importer.getToContext()) CXXPseudoDestructorExpr(
+Importer.getToContext(), BaseE, E->isArrow(),
+Importer.Import(E->getOperatorLoc()),
+Importer.Import(E->getQualifierLoc()),
+ScopeInfo, Importer.Import(E->getColonColonLoc()),
+Importer.Import(E->getTildeLoc()), Storage);
+}
+
 Expr *ASTNodeImporter::VisitCallExpr(CallExpr *E) {
   QualType T = Importer.Import(E->getType());
   if (T.isNull())


Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -535,5 +535,21 @@
  binaryOperator(has(cxxUnresolvedConstructExpr()));
 }
 
+const internal::VariadicDynCastAllOfMatcher
+cxxPseudoDestructorExpr;
+
+TEST(ImportExpr, ImportCXXPseudoDestructorExpr) {
+  MatchVerifier Verifier;
+  EXPECT_TRUE(
+  testImport("typedef int T;"
+ "void declToImport(int *p) {"
+ "p->T::~T();"
+ "}",
+ Lang_CXX, "", Lang_CXX, Verifier,
+ functionDecl(has(compoundStmt(has(
+ callExpr(has(cxxPseudoDestructorExpr();
+}
+
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/AST/ASTImporter.cpp
===
--- lib/AST/ASTImporter.cpp
+++ lib/AST/ASTImporter.cpp
@@ -278,6 +278,7 @@
 Expr *VisitExprWithCleanups(ExprWithCleanups *EWC);
 Expr *VisitCXXThisExpr(CXXThisExpr *E);
 Expr *VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E);
+Expr *VisitCXXPseudoDestructorExpr(CXXPseudoDestructorExpr *E);
 Expr *VisitMemberExpr(MemberExpr *E);
 Expr *VisitCallExpr(CallExpr *E);
 Expr *VisitInitListExpr(InitListExpr *E);
@@ -5540,6 +5541,38 @@
   E->isOverloaded(), ToDecls.begin(), ToDecls.end());
 }
 
+
+Expr *ASTNodeImporter::VisitCXXPseudoDestructorExpr(
+CXXPseudoDestructorExpr *E) {
+
+  Expr *BaseE = Importer.Import(E->getBase());
+  if (!BaseE && E->getBase())
+return nullptr;
+
+  TypeSourceInfo *ScopeInfo = Importer.Import(E->getScopeTypeInfo());
+
+  PseudoDestructorTypeStorage Storage;
+  if (IdentifierInfo *FromII = E->getDestroyedTypeIdentifier()) {
+IdentifierInfo *ToII = Importer.Import(FromII);
+if (!ToII)
+  return nullptr;
+Storage = PseudoDestructorTypeStorage(
+  ToII, Importer.Import(E->getDestroyedTypeLoc()));
+  } else {
+TypeSourceInfo *TI = Importer.Import(E->g

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

[Peter Szecsi via Phabricator via cfe-commits]

szepet added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5549
+  Expr *BaseE = Importer.Import(E->getBase());
+  if (!BaseE)
+return nullptr;

xazax.hun wrote:
> Does `E->getBase()` guaranteed to return non-null? What happens when this 
> node was constructed using EmptyShell? Shouldn't we check for that somehow? 
> When can that happen?
The import process of ArraySubscriptExpr and UnaryOperator (and probably more 
other classes) are not prepared for this case as well. Not sure if this can be 
encountered in a complete AST.
However, I think a lazy evaluated && operator won't hurt the performance and at 
least we are going to be prepared for this case.


https://reviews.llvm.org/D38843



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

[PATCH] D39092: [clang-refactor] Add "-Inplace" option to the commandline tool.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.

Change clang-refactor default behavior to print the new code after refactoring
(instead of editing the source files), which would make it easier to use
and debug the refactoring action.


https://reviews.llvm.org/D39092

Files:
  test/Refactor/tool-apply-replacements.cpp
  tools/clang-refactor/ClangRefactor.cpp


Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefactor.cpp
+++ tools/clang-refactor/ClangRefactor.cpp
@@ -40,6 +40,11 @@
 static cl::opt Verbose("v", cl::desc("Use verbose output"),
  cl::cat(cl::GeneralCategory),
  cl::sub(*cl::AllSubCommands));
+
+static cl::opt Inplace("i", cl::desc("Inplace edit s"),
+ cl::cat(cl::GeneralCategory),
+ cl::sub(*cl::AllSubCommands));
+
 } // end namespace opts
 
 namespace {
@@ -436,13 +441,18 @@
 return true;
   }
 
-  std::error_code EC;
-  llvm::raw_fd_ostream OS(File, EC, llvm::sys::fs::F_Text);
-  if (EC) {
-llvm::errs() << EC.message() << "\n";
-return true;
+  if (opts::Inplace) {
+std::error_code EC;
+llvm::raw_fd_ostream OS(File, EC, llvm::sys::fs::F_Text);
+if (EC) {
+  llvm::errs() << EC.message() << "\n";
+  return true;
+}
+OS << *Result;
+continue;
   }
-  OS << *Result;
+
+  llvm::outs() << *Result;
 }
 return false;
   }
Index: test/Refactor/tool-apply-replacements.cpp
===
--- test/Refactor/tool-apply-replacements.cpp
+++ test/Refactor/tool-apply-replacements.cpp
@@ -1,10 +1,8 @@
-// RUN: rm -f %t.cp.cpp
-// RUN: cp %s %t.cp.cpp
-// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7 -new-name=test 
%t.cp.cpp --
-// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
-// RUN: cp %s %t.cp.cpp
-// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7-9:15 
-new-name=test %t.cp.cpp --
-// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-refactor local-rename -selection=%t.cpp:7:7 -new-name=test 
%t.cpp -- | FileCheck %s
+// RUN: clang-refactor local-rename -selection=%t.cpp:7:7-7:15 -new-name=test 
%t.cpp -- | FileCheck %s
+// RUN: clang-refactor local-rename -i -selection=%t.cpp:7:7 -new-name=test 
%t.cpp --
+// RUN: FileCheck -input-file=%t.cpp %s
 
 class RenameMe {
 // CHECK: class test {


Index: tools/clang-refactor/ClangRefactor.cpp
===
--- tools/clang-refactor/ClangRefactor.cpp
+++ tools/clang-refactor/ClangRefactor.cpp
@@ -40,6 +40,11 @@
 static cl::opt Verbose("v", cl::desc("Use verbose output"),
  cl::cat(cl::GeneralCategory),
  cl::sub(*cl::AllSubCommands));
+
+static cl::opt Inplace("i", cl::desc("Inplace edit s"),
+ cl::cat(cl::GeneralCategory),
+ cl::sub(*cl::AllSubCommands));
+
 } // end namespace opts
 
 namespace {
@@ -436,13 +441,18 @@
 return true;
   }
 
-  std::error_code EC;
-  llvm::raw_fd_ostream OS(File, EC, llvm::sys::fs::F_Text);
-  if (EC) {
-llvm::errs() << EC.message() << "\n";
-return true;
+  if (opts::Inplace) {
+std::error_code EC;
+llvm::raw_fd_ostream OS(File, EC, llvm::sys::fs::F_Text);
+if (EC) {
+  llvm::errs() << EC.message() << "\n";
+  return true;
+}
+OS << *Result;
+continue;
   }
-  OS << *Result;
+
+  llvm::outs() << *Result;
 }
 return false;
   }
Index: test/Refactor/tool-apply-replacements.cpp
===
--- test/Refactor/tool-apply-replacements.cpp
+++ test/Refactor/tool-apply-replacements.cpp
@@ -1,10 +1,8 @@
-// RUN: rm -f %t.cp.cpp
-// RUN: cp %s %t.cp.cpp
-// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7 -new-name=test %t.cp.cpp --
-// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
-// RUN: cp %s %t.cp.cpp
-// RUN: clang-refactor local-rename -selection=%t.cp.cpp:9:7-9:15 -new-name=test %t.cp.cpp --
-// RUN: grep -v CHECK %t.cp.cpp | FileCheck %t.cp.cpp
+// RUN: sed -e 's#//.*$##' %s > %t.cpp
+// RUN: clang-refactor local-rename -selection=%t.cpp:7:7 -new-name=test %t.cpp -- | FileCheck %s
+// RUN: clang-refactor local-rename -selection=%t.cpp:7:7-7:15 -new-name=test %t.cpp -- | FileCheck %s
+// RUN: clang-refactor local-rename -i -selection=%t.cpp:7:7 -new-name=test %t.cpp --
+// RUN: FileCheck -input-file=%t.cpp %s
 
 class RenameMe {
 // CHECK: class test {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement

[Balogh , Ádám via Phabricator via cfe-commits]

baloghadamsoftware updated this revision to Diff 119576.
baloghadamsoftware added a comment.
Herald added a subscriber: szepet.

I think it is the final attempt. If Symbols are different, the type is 
extended, so we store a correct (but extended) range. However, if the Symbols 
are the same, we do not change the type so checks like `assumeInbound()` are 
not affected.

Anna, Devin, Artem, please review it whether this version is functionally 
correct. (Style is another thing.) If not, then we should go back to the local 
solution in the iterator checkers. However, we must continue the review there, 
it is standing still for half a year.


https://reviews.llvm.org/D35109

Files:
  lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  test/Analysis/std-c-library-functions.c
  test/Analysis/svalbuilder-rearrange-comparisons.c

Index: test/Analysis/svalbuilder-rearrange-comparisons.c
===
--- /dev/null
+++ test/Analysis/svalbuilder-rearrange-comparisons.c
@@ -0,0 +1,913 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -verify %s
+
+void clang_analyzer_dump(int x);
+void clang_analyzer_eval(int x);
+void clang_analyzer_printState();
+
+int f();
+
+void compare_different_symbol_equal() {
+  int x = f(), y = f();
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 0}}
+}
+
+void compare_different_symbol_plus_left_int_equal() {
+  int x = f()+1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 1}}
+}
+
+void compare_different_symbol_minus_left_int_equal() {
+  int x = f()-1, y = f();
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$5{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 1}}
+}
+
+void compare_different_symbol_plus_right_int_equal() {
+  int x = f(), y = f()+2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 2}}
+}
+
+void compare_different_symbol_minus_right_int_equal() {
+  int x = f(), y = f()-2;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 2}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 2}}
+}
+
+void compare_different_symbol_plus_left_plus_right_int_equal() {
+  int x = f()+2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 1}}
+}
+
+void compare_different_symbol_plus_left_minus_right_int_equal() {
+  int x = f()+2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$5{int})) - ((long) (conj_$2{int}))) == 3}}
+}
+
+void compare_different_symbol_minus_left_plus_right_int_equal() {
+  int x = f()-2, y = f()+1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) + 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 3}}
+}
+
+void compare_different_symbol_minus_left_minus_right_int_equal() {
+  int x = f()-2, y = f()-1;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 2}}
+  clang_analyzer_dump(y); // expected-warning{{(conj_$5{int}) - 1}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{(((long) (conj_$2{int})) - ((long) (conj_$5{int}))) == 1}}
+}
+
+void compare_same_symbol_equal() {
+  int x = f(), y = x;
+  clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{1 S32b}}
+}
+
+void compare_same_symbol_plus_left_int_equal() {
+  int x = f(), y = x;
+  ++x;
+  clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
+  clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
+  clang_analyzer_dump(x == y);
+  // expected-warning@-1{{0 S32b}}
+}
+
+void compare_same_symbol_minus_left_int_equal() {
+  int x = f(), y = x;
+  --x;
+  clang_analyzer_dump(x); // expected-

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5500
+
+  TemplateArgumentListInfo ToTAInfo;
+  TemplateArgumentListInfo *ResInfo = nullptr;

szepet wrote:
> xazax.hun wrote:
> > According to phabricator this code is very similar to a snippet starting 
> > from line 4524 and some code bellow. Maybe it would be worth to have a 
> > function instead of duplicating?
> Good point, I would do it in a separate patch and add it as a dependency if 
> it is OK.
Sure, that would be awesome. :)


https://reviews.llvm.org/D38845



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

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Peter Szecsi via Phabricator via cfe-commits]

szepet added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5500
+
+  TemplateArgumentListInfo ToTAInfo;
+  TemplateArgumentListInfo *ResInfo = nullptr;

xazax.hun wrote:
> According to phabricator this code is very similar to a snippet starting from 
> line 4524 and some code bellow. Maybe it would be worth to have a function 
> instead of duplicating?
Good point, I would do it in a separate patch and add it as a dependency if it 
is OK.


https://reviews.llvm.org/D38845



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

[PATCH] D39086: Performance tracing facility for clangd.

[Eric Liu via Phabricator via cfe-commits]

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

lgtm


https://reviews.llvm.org/D39086



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

[PATCH] D39048: Dump signed integers in SymIntExpr and IntSymExpr correctly

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL316157: [analyzer] Dump signed integers in SymIntExpr and 
IntSymExpr correctly (authored by xazax).

Changed prior to commit:
  https://reviews.llvm.org/D39048?vs=119466&id=119569#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D39048

Files:
  cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
  cfe/trunk/test/Analysis/expr-inspection.c


Index: cfe/trunk/test/Analysis/expr-inspection.c
===
--- cfe/trunk/test/Analysis/expr-inspection.c
+++ cfe/trunk/test/Analysis/expr-inspection.c
@@ -8,6 +8,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
   int y = 1;
   clang_analyzer_printState();
   for (; y < 3; ++y)
Index: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -31,14 +31,20 @@
   os << '(';
   getLHS()->dumpToStream(os);
   os << ") "
- << BinaryOperator::getOpcodeStr(getOpcode()) << ' '
- << getRHS().getZExtValue();
+ << BinaryOperator::getOpcodeStr(getOpcode()) << ' ';
+  if (getRHS().isUnsigned())
+os << getRHS().getZExtValue();
+  else
+os << getRHS().getSExtValue();
   if (getRHS().isUnsigned())
 os << 'U';
 }
 
 void IntSymExpr::dumpToStream(raw_ostream &os) const {
-  os << getLHS().getZExtValue();
+  if (getLHS().isUnsigned())
+os << getLHS().getZExtValue();
+  else
+os << getLHS().getSExtValue();
   if (getLHS().isUnsigned())
 os << 'U';
   os << ' '


Index: cfe/trunk/test/Analysis/expr-inspection.c
===
--- cfe/trunk/test/Analysis/expr-inspection.c
+++ cfe/trunk/test/Analysis/expr-inspection.c
@@ -8,6 +8,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
   int y = 1;
   clang_analyzer_printState();
   for (; y < 3; ++y)
Index: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -31,14 +31,20 @@
   os << '(';
   getLHS()->dumpToStream(os);
   os << ") "
- << BinaryOperator::getOpcodeStr(getOpcode()) << ' '
- << getRHS().getZExtValue();
+ << BinaryOperator::getOpcodeStr(getOpcode()) << ' ';
+  if (getRHS().isUnsigned())
+os << getRHS().getZExtValue();
+  else
+os << getRHS().getSExtValue();
   if (getRHS().isUnsigned())
 os << 'U';
 }
 
 void IntSymExpr::dumpToStream(raw_ostream &os) const {
-  os << getLHS().getZExtValue();
+  if (getLHS().isUnsigned())
+os << getLHS().getZExtValue();
+  else
+os << getLHS().getSExtValue();
   if (getLHS().isUnsigned())
 os << 'U';
   os << ' '
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316157 - [analyzer] Dump signed integers in SymIntExpr and IntSymExpr correctly

[Gabor Horvath via cfe-commits]

Author: xazax
Date: Thu Oct 19 04:58:21 2017
New Revision: 316157

URL: http://llvm.org/viewvc/llvm-project?rev=316157&view=rev
Log:
[analyzer] Dump signed integers in SymIntExpr and IntSymExpr correctly

Patch by: Adam Balogh!

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
cfe/trunk/test/Analysis/expr-inspection.c

Modified: cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp?rev=316157&r1=316156&r2=316157&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/SymbolManager.cpp Thu Oct 19 04:58:21 2017
@@ -31,14 +31,20 @@ void SymIntExpr::dumpToStream(raw_ostrea
   os << '(';
   getLHS()->dumpToStream(os);
   os << ") "
- << BinaryOperator::getOpcodeStr(getOpcode()) << ' '
- << getRHS().getZExtValue();
+ << BinaryOperator::getOpcodeStr(getOpcode()) << ' ';
+  if (getRHS().isUnsigned())
+os << getRHS().getZExtValue();
+  else
+os << getRHS().getSExtValue();
   if (getRHS().isUnsigned())
 os << 'U';
 }
 
 void IntSymExpr::dumpToStream(raw_ostream &os) const {
-  os << getLHS().getZExtValue();
+  if (getLHS().isUnsigned())
+os << getLHS().getZExtValue();
+  else
+os << getLHS().getSExtValue();
   if (getLHS().isUnsigned())
 os << 'U';
   os << ' '

Modified: cfe/trunk/test/Analysis/expr-inspection.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/expr-inspection.c?rev=316157&r1=316156&r2=316157&view=diff
==
--- cfe/trunk/test/Analysis/expr-inspection.c (original)
+++ cfe/trunk/test/Analysis/expr-inspection.c Thu Oct 19 04:58:21 2017
@@ -8,6 +8,7 @@ void clang_analyzer_numTimesReached();
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
   int y = 1;
   clang_analyzer_printState();
   for (; y < 3; ++y)


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

[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a subscriber: NoQ.
xazax.hun added a comment.

I think this change is very useful but it is also important to get these 
changes right.
I think one of the main reason you did not get review comments yet is that it 
is not easy to verify that these changes are sound.

In general, there are false positives in the analyzer due to limits in the 
constraint manager (or missing parts in modeling the language). But in general, 
we try to avoid having false positives due to unsound assumptions (apart from 
some cases like assuming const methods will not change the fields of a class).

While the change you introduced is indeed very useful the soundness probably 
depends on the details of how promotions, conversions, and other corner cases 
are handled.
In order to introduce a change like this, we need to have those cases covered 
to ensure that we have the soundness we want and this needs to be verified with 
test cases.
Also once the solution is sound it would be great to measure the performance to 
ensure that we did not regress too much.

I understand that you do not want to work on something that might not get 
accepted but also with the available information it might be hard to decide 
whether this is a good approach to the problem or not. 
But of course, I am just guessing here, @dcoughlin, @zaks.anna, @NoQ  might 
have a different opinion.

A bit more technical comment: did you consider using `SValBuilder`'s 
`evalBinOpNN`? I believe it already handles at least some of the conversions 
you did not cover here.


Repository:
  rL LLVM

https://reviews.llvm.org/D36471



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

[PATCH] D39086: Performance tracing facility for clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 119562.
sammccall marked 3 inline comments as done.
sammccall added a comment.

Address review comments.


https://reviews.llvm.org/D39086

Files:
  clangd/CMakeLists.txt
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/JSONRPCDispatcher.cpp
  clangd/ProtocolHandlers.cpp
  clangd/Trace.cpp
  clangd/Trace.h
  clangd/tool/ClangdMain.cpp
  test/clangd/trace.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- /dev/null
+++ unittests/clangd/TraceTests.cpp
@@ -0,0 +1,117 @@
+//===-- TraceTests.cpp - Tracing unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Trace.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/YAMLParser.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using namespace llvm;
+
+MATCHER_P(StringNode, Val, "") {
+  if (arg->getType() != yaml::Node::NK_Scalar) {
+*result_listener << "is a " << arg->getVerbatimTag();
+return false;
+  }
+  SmallString<32> S;
+  return Val == static_cast(arg)->getValue(S);
+}
+
+// Checks that N is a Mapping (JS object) with the expected scalar properties.
+// The object must have all the Expected properties, but may have others.
+bool VerifyObject(yaml::Node &N, std::map Expected) {
+  auto* M = dyn_cast(&N);
+  if (!M) {
+ADD_FAILURE() << "Not an object";
+return false;
+  }
+  bool Match = true;
+  SmallString<32> Tmp;
+  for (auto Prop : *M) {
+auto* K = dyn_cast_or_null(Prop.getKey());
+if (!K) continue;
+std::string KS = K->getValue(Tmp).str();
+auto I = Expected.find(KS);
+if (I == Expected.end()) continue;  // Ignore properties with no assertion.
+
+auto* V = dyn_cast_or_null(Prop.getValue());
+if (!V) {
+  ADD_FAILURE() << KS << " is not a string";
+  Match = false;
+}
+std::string VS = V->getValue(Tmp).str();
+if (VS != I->second) {
+  ADD_FAILURE() << KS << " expected " << I->second << " but actual " << VS;
+  Match = false;
+}
+Expected.erase(I);
+  }
+  for (const auto& P : Expected) {
+ADD_FAILURE() << P.first << " missing, expected " << P.second;
+Match = false;
+  }
+  return Match;
+}
+
+TEST(TraceTest, SmokeTest) {
+  // Capture some events.
+  std::string JSON;
+  raw_string_ostream OS(JSON);
+  trace::start(OS);
+  {
+trace::Span S("A");
+trace::log("B");
+  }
+  trace::stop();
+
+  // Get the root JSON object using the YAML parser.
+  SourceMgr SM;
+  yaml::Stream Stream(JSON, SM);
+  auto Doc = Stream.begin();
+  ASSERT_NE(Doc, Stream.end());
+  auto* Root = dyn_cast_or_null(Doc->getRoot());
+  ASSERT_NE(Root, nullptr) << "Root should be an object";
+
+  // We expect in order:
+  //   displayTimeUnit: "ns"
+  //   traceEvents: [process name, thread name, start span, log, end span]
+  // (The order doesn't matter, but the YAML parser is awkward to use otherwise)
+  auto Prop = Root->begin();
+  ASSERT_NE(Prop, Root->end()) << "Expected displayTimeUnit property";
+  ASSERT_THAT(Prop->getKey(), StringNode("displayTimeUnit"));
+  EXPECT_THAT(Prop->getValue(), StringNode("ns"));
+  ASSERT_NE(++Prop, Root->end()) << "Expected traceEvents property";
+  EXPECT_THAT(Prop->getKey(), StringNode("traceEvents"));
+  auto* Events = dyn_cast_or_null(Prop->getValue());
+  ASSERT_NE(Events, nullptr) << "traceEvents should be an array";
+  auto Event = Events->begin();
+  ASSERT_NE(Event, Events->end()) << "Expected process name";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "M"}, {"name", "process_name"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected thread name";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "M"}, {"name", "thread_name"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected span start";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "B"}, {"name", "A"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected log message";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "i"}, {"name", "B"}}));
+  ASSERT_NE(++Event, Events->end()) << "Expected span end";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "E"}}));
+  ASSERT_EQ(++Event, Events->end());
+  ASSERT_EQ(++Prop, Root->end());
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang
Index: unittests/clangd/CMakeLists.txt
===
--- unittests/clangd/CMakeLists.txt
+++ unittests/clangd/CMakeLists.txt
@@ -10,6 +10,7 @@
 
 add_extra_unittest(ClangdTests
   ClangdTests.cpp
+  TraceTests.cpp
   )
 
 target_link_libraries(ClangdTests
Index: test/clangd

[PATCH] D39086: Performance tracing facility for clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/JSONRPCDispatcher.cpp:230
 
-  // Finally, execute the action for this JSON message.
-  if (!Dispatcher.call(JSONRef, Out))
-Out.log("JSON dispatch failed!\n");
+  {
+// Finally, execute the action for this JSON message.

ioeric wrote:
> Why this empty block?
Oops, was tracing here but it didn't turn out to be useful. Reverted.



Comment at: clangd/Trace.cpp:90
+
+void start(raw_ostream &OS) {
+  assert(!T && "start() called twice without stop()");

ioeric wrote:
> Would it make sense to have a helper class which handles starting/stopping 
> in, say, constructor/destructor? ... so that we don't need to worry about 
> forgetting to stop?
I didn't do this for a couple of reasons:

 - it obscures the static-stateful nature of this code, making it look like 
e.g. you could dynamically turn on tracing while clangd is running. (Which 
would be cool, but wouldn't currently work)
 - it's some more boilerplate, and isn't actually very convenient to ClangdMain 
(since `start()` is conditional)



Comment at: clangd/tool/ClangdMain.cpp:98
+  llvm::errs() << "Error while opening trace file: " << EC.message();
+}
+trace::start(*TraceStream);

ioeric wrote:
> Is this intended? Start tracing even if there is an error?
Oops, thanks!


https://reviews.llvm.org/D39086



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

[PATCH] D39086: Performance tracing facility for clangd.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Looks good in general. A few nits.




Comment at: clangd/JSONRPCDispatcher.cpp:230
 
-  // Finally, execute the action for this JSON message.
-  if (!Dispatcher.call(JSONRef, Out))
-Out.log("JSON dispatch failed!\n");
+  {
+// Finally, execute the action for this JSON message.

Why this empty block?



Comment at: clangd/Trace.cpp:90
+
+void start(raw_ostream &OS) {
+  assert(!T && "start() called twice without stop()");

Would it make sense to have a helper class which handles starting/stopping in, 
say, constructor/destructor? ... so that we don't need to worry about 
forgetting to stop?



Comment at: clangd/tool/ClangdMain.cpp:98
+  llvm::errs() << "Error while opening trace file: " << EC.message();
+}
+trace::start(*TraceStream);

Is this intended? Start tracing even if there is an error?



Comment at: unittests/clangd/TraceTests.cpp:47
+auto I = Expected.find(KS);
+if (I == Expected.end()) continue;
+

Add a comment on why we skip unexpected keys?


https://reviews.llvm.org/D39086



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

[PATCH] D37187: [Analyzer] Fix Bug 25609 - Assertion UNREACHABLE: 'Unexpected ProgramPoint' with widen-loops=true

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

LGTM!


https://reviews.llvm.org/D37187



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

[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

I checked what happens:

The checker would like to solve the following (I inspect the branch when x == 0 
):
`((reg_$1) + 1) * 4   <= 0`
The `getSimplifiedOffsets` function kicks in and simplifies the expression 
above to the following:
`(reg_$1)   <= -1`

The analyzer also know that the value of `y` is within `[1,98]`.

The source of the problem is that the simplified expression is evaluated after 
the right-hand side is converted to an unsigned value which will be greater 
than the max value of `y`.

I think we did not regress after omitting some of the computation because the 
analyzer's default constraint manager handles the case when there is a constant 
addition/subtraction next to the symbol. So if we lose something with this 
modification, we could probably observe that using multidimensional arrays.

Do you mind writing some tests with multidimensional arrays to check what do we 
lose if we remove that code?
Also, how hard would it be to correct the calculation for unsigned values?


Repository:
  rL LLVM

https://reviews.llvm.org/D39049



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

[PATCH] D38171: Implement clang-tidy check aliases.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

One problem to think about when we add all clang-diagnostic as "first or 
second" class citizen, `checkes=*` might now enable all the warnings which make 
no sense and might be surprising to the users. What do you think?


https://reviews.llvm.org/D38171



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

[PATCH] D38694: [ASTImporter] Support importing CXXUnresolvedConstructExpr and UnresolvedLookupExpr

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5476
+
+  for (unsigned ai = 0, ae = NumArgs; ai != ae; ++ai) {
+Expr *FromArg = CE->getArg(ai);

Use uppercase variable names. 



Comment at: lib/AST/ASTImporter.cpp:5477
+  for (unsigned ai = 0, ae = NumArgs; ai != ae; ++ai) {
+Expr *FromArg = CE->getArg(ai);
+Expr *ToArg = Importer.Import(FromArg);

I would eliminate this local variable.


https://reviews.llvm.org/D38694



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

[PATCH] D38843: [ASTImporter] Support importing CXXPseudoDestructorExpr

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5549
+  Expr *BaseE = Importer.Import(E->getBase());
+  if (!BaseE)
+return nullptr;

Does `E->getBase()` guaranteed to return non-null? What happens when this node 
was constructed using EmptyShell? Shouldn't we check for that somehow? When can 
that happen?


https://reviews.llvm.org/D38843



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

[PATCH] D38845: [ASTImporter] Support importing UnresolvedMemberExpr, DependentNameType, DependentScopeDeclRefExpr

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5500
+
+  TemplateArgumentListInfo ToTAInfo;
+  TemplateArgumentListInfo *ResInfo = nullptr;

According to phabricator this code is very similar to a snippet starting from 
line 4524 and some code bellow. Maybe it would be worth to have a function 
instead of duplicating?


https://reviews.llvm.org/D38845



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

[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: lib/StaticAnalyzer/Core/LoopUnrolling.cpp:100
 declRefExpr(to(varDecl(VarNodeMatcher)),
   binaryOperator(anyOf(hasOperatorName("="), hasOperatorName("+="),
hasOperatorName("/="), hasOperatorName("*="),

Maybe instead of enumerating all the assignment operators here it would be 
better to have a matcher that checks `isAssignmentOp` for 
CXXOperatorCalls/BinaryOperators? This would be less error prone and more 
future proof.


https://reviews.llvm.org/D38921



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

[PATCH] D39086: Performance tracing facility for clangd.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added a subscriber: mgorny.

This lets you visualize clangd's activity on different threads over time,
and understand critical paths of requests and object lifetimes.
The data produced can be visualized in Chrome (at chrome://tracing), or
in a standalone copy of catapult (http://github.com/catapult-project/catapult)

This patch consists of:

- a command line flag "-trace" that causes clangd to emit JSON trace data
- an API (in Trace.h) allowing clangd code to easily add events to the stream
- several initial uses of this API to capture JSON-RPC requests, builds, logs

Example result: https://photos.app.goo.gl/12L9swaz5REGQ1rm1

Caveats:

- JSON serialization is ad-hoc (isn't it everywhere?) so the API is limited to 
naming events rather than attaching arbitrary metadata. I'd like to fix this (I 
think we could use a JSON-object abstraction).
- The recording is very naive: events are written immediately by locking a 
mutex. Contention on the mutex might disturb performance.
- For now it just traces instants or spans on the current thread. There are 
other things that make sense to show (cross-thread flows, non-thread resources 
such as ASTs). But we have to start somewhere.


https://reviews.llvm.org/D39086

Files:
  clangd/CMakeLists.txt
  clangd/ClangdServer.cpp
  clangd/ClangdUnit.cpp
  clangd/JSONRPCDispatcher.cpp
  clangd/ProtocolHandlers.cpp
  clangd/Trace.cpp
  clangd/Trace.h
  clangd/tool/ClangdMain.cpp
  test/clangd/trace.test
  unittests/clangd/CMakeLists.txt
  unittests/clangd/TraceTests.cpp

Index: unittests/clangd/TraceTests.cpp
===
--- /dev/null
+++ unittests/clangd/TraceTests.cpp
@@ -0,0 +1,116 @@
+//===-- TraceTests.cpp - Tracing unit tests -*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "Trace.h"
+
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/Support/SourceMgr.h"
+#include "llvm/Support/YAMLParser.h"
+#include "gtest/gtest.h"
+#include "gmock/gmock.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+using namespace llvm;
+
+MATCHER_P(StringNode, Val, "") {
+  if (arg->getType() != yaml::Node::NK_Scalar) {
+*result_listener << "is a " << arg->getVerbatimTag();
+return false;
+  }
+  SmallString<32> S;
+  return Val == static_cast(arg)->getValue(S);
+}
+
+// Checks that N is a Mapping (JS object) with the expected scalar properties.
+bool VerifyObject(yaml::Node &N, std::map Expected) {
+  auto* M = dyn_cast(&N);
+  if (!M) {
+ADD_FAILURE() << "Not an object";
+return false;
+  }
+  bool Match = true;
+  SmallString<32> Tmp;
+  for (auto Prop : *M) {
+auto* K = dyn_cast_or_null(Prop.getKey());
+if (!K) continue;
+std::string KS = K->getValue(Tmp).str();
+auto I = Expected.find(KS);
+if (I == Expected.end()) continue;
+
+auto* V = dyn_cast_or_null(Prop.getValue());
+if (!V) {
+  ADD_FAILURE() << KS << " is not a string";
+  Match = false;
+}
+std::string VS = V->getValue(Tmp).str();
+if (VS != I->second) {
+  ADD_FAILURE() << KS << " expected " << I->second << " but actual " << VS;
+  Match = false;
+}
+Expected.erase(I);
+  }
+  for (const auto& P : Expected) {
+ADD_FAILURE() << P.first << " missing, expected " << P.second;
+Match = false;
+  }
+  return Match;
+}
+
+TEST(TraceTest, SmokeTest) {
+  // Capture some events.
+  std::string JSON;
+  raw_string_ostream OS(JSON);
+  trace::start(OS);
+  {
+trace::Span S("A");
+trace::log("B");
+  }
+  trace::stop();
+
+  // Get the root JSON object using the YAML parser.
+  SourceMgr SM;
+  yaml::Stream Stream(JSON, SM);
+  auto Doc = Stream.begin();
+  ASSERT_NE(Doc, Stream.end());
+  auto* Root = dyn_cast_or_null(Doc->getRoot());
+  ASSERT_NE(Root, nullptr) << "Root should be an object";
+
+  // We expect in order:
+  //   displayTimeUnit: "ns"
+  //   traceEvents: [process name, thread name, start span, log, end span]
+  // (The order doesn't matter, but the YAML parser is awkward to use otherwise)
+  auto Prop = Root->begin();
+  ASSERT_NE(Prop, Root->end()) << "Expected displayTimeUnit property";
+  ASSERT_THAT(Prop->getKey(), StringNode("displayTimeUnit"));
+  EXPECT_THAT(Prop->getValue(), StringNode("ns"));
+  ASSERT_NE(++Prop, Root->end()) << "Expected traceEvents property";
+  EXPECT_THAT(Prop->getKey(), StringNode("traceEvents"));
+  auto* Events = dyn_cast_or_null(Prop->getValue());
+  ASSERT_NE(Events, nullptr) << "traceEvents should be an array";
+  auto Event = Events->begin();
+  ASSERT_NE(Event, Events->end()) << "Expected process name";
+  EXPECT_TRUE(VerifyObject(*Event, {{"ph", "M"}, {"name", "process_name"}}));
+  ASSERT_NE(++

[PATCH] D39083: [CodeGen] Fix generation of TBAA info for array-to-pointer conversions

[Ivan Kosarev via Phabricator via cfe-commits]

kosarev created this revision.
kosarev added a project: clang.

Resolves:
Fatal error: Offset not zero at the point of scalar access.
http://llvm.org/PR34992


Repository:
  rL LLVM

https://reviews.llvm.org/D39083

Files:
  lib/CodeGen/CGExpr.cpp
  test/CodeGen/tbaa-array.cpp


Index: test/CodeGen/tbaa-array.cpp
===
--- test/CodeGen/tbaa-array.cpp
+++ test/CodeGen/tbaa-array.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
+// RUN: -emit-llvm -o - | FileCheck %s
+//
+// Check that we generate correct TBAA information for accesses to array
+// elements.
+
+struct A { int i; };
+struct B { A a[1]; };
+
+int foo(B *b) {
+// CHECK-LABEL: _Z3fooP1B
+// CHECK: load i32, {{.*}}, !tbaa [[TAG_A_i:!.*]]
+  return b->a->i;
+}
+
+// CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0}
+// CHECK-DAG: [[TYPE_A]] = !{!"_ZTS1A", !{{.*}}, i64 0}
+// CHECK-DAG: [[TYPE_int]] = !{!"int", !{{.*}}, i64 0}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3072,8 +3072,6 @@
   // Expressions of array type can't be bitfields or vector elements.
   LValue LV = EmitLValue(E);
   Address Addr = LV.getAddress();
-  if (BaseInfo) *BaseInfo = LV.getBaseInfo();
-  if (TBAAInfo) *TBAAInfo = LV.getTBAAInfo();
 
   // If the array type was an incomplete type, we need to make sure
   // the decay ends up being the right type.
@@ -3088,7 +3086,15 @@
 Addr = Builder.CreateStructGEP(Addr, 0, CharUnits::Zero(), "arraydecay");
   }
 
+  // The result of this decay conversion points to an array element within the
+  // base lvalue. However, since TBAA currently does not support representing
+  // accesses to elements of member arrays, we conservatively represent 
accesses
+  // to the pointee object as if it had no any base lvalue specified.
+  // TODO: Support TBAA for member arrays.
   QualType EltType = E->getType()->castAsArrayTypeUnsafe()->getElementType();
+  if (BaseInfo) *BaseInfo = LV.getBaseInfo();
+  if (TBAAInfo) *TBAAInfo = CGM.getTBAAAccessInfo(EltType);
+
   return Builder.CreateElementBitCast(Addr, ConvertTypeForMem(EltType));
 }
 


Index: test/CodeGen/tbaa-array.cpp
===
--- test/CodeGen/tbaa-array.cpp
+++ test/CodeGen/tbaa-array.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -triple x86_64-linux -O1 -disable-llvm-passes %s \
+// RUN: -emit-llvm -o - | FileCheck %s
+//
+// Check that we generate correct TBAA information for accesses to array
+// elements.
+
+struct A { int i; };
+struct B { A a[1]; };
+
+int foo(B *b) {
+// CHECK-LABEL: _Z3fooP1B
+// CHECK: load i32, {{.*}}, !tbaa [[TAG_A_i:!.*]]
+  return b->a->i;
+}
+
+// CHECK-DAG: [[TAG_A_i]] = !{[[TYPE_A:!.*]], [[TYPE_int:!.*]], i64 0}
+// CHECK-DAG: [[TYPE_A]] = !{!"_ZTS1A", !{{.*}}, i64 0}
+// CHECK-DAG: [[TYPE_int]] = !{!"int", !{{.*}}, i64 0}
Index: lib/CodeGen/CGExpr.cpp
===
--- lib/CodeGen/CGExpr.cpp
+++ lib/CodeGen/CGExpr.cpp
@@ -3072,8 +3072,6 @@
   // Expressions of array type can't be bitfields or vector elements.
   LValue LV = EmitLValue(E);
   Address Addr = LV.getAddress();
-  if (BaseInfo) *BaseInfo = LV.getBaseInfo();
-  if (TBAAInfo) *TBAAInfo = LV.getTBAAInfo();
 
   // If the array type was an incomplete type, we need to make sure
   // the decay ends up being the right type.
@@ -3088,7 +3086,15 @@
 Addr = Builder.CreateStructGEP(Addr, 0, CharUnits::Zero(), "arraydecay");
   }
 
+  // The result of this decay conversion points to an array element within the
+  // base lvalue. However, since TBAA currently does not support representing
+  // accesses to elements of member arrays, we conservatively represent accesses
+  // to the pointee object as if it had no any base lvalue specified.
+  // TODO: Support TBAA for member arrays.
   QualType EltType = E->getType()->castAsArrayTypeUnsafe()->getElementType();
+  if (BaseInfo) *BaseInfo = LV.getBaseInfo();
+  if (TBAAInfo) *TBAAInfo = CGM.getTBAAAccessInfo(EltType);
+
   return Builder.CreateElementBitCast(Addr, ConvertTypeForMem(EltType));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r316152 - Fix a few nits in RenamingAction.

[Haojian Wu via cfe-commits]

Author: hokein
Date: Thu Oct 19 01:20:55 2017
New Revision: 316152

URL: http://llvm.org/viewvc/llvm-project?rev=316152&view=rev
Log:
Fix a few nits in RenamingAction.

* Add missing override keyword.
* avoid unnecessary copy of std::string.

Modified:
cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp

Modified: cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp?rev=316152&r1=316151&r2=316152&view=diff
==
--- cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp (original)
+++ cfe/trunk/lib/Tooling/Refactoring/Rename/RenamingAction.cpp Thu Oct 19 
01:20:55 2017
@@ -77,10 +77,10 @@ private:
 class RenameOccurrences final : public SourceChangeRefactoringRule {
 public:
   RenameOccurrences(const NamedDecl *ND, std::string NewName)
-  : Finder(ND), NewName(NewName) {}
+  : Finder(ND), NewName(std::move(NewName)) {}
 
   Expected
-  createSourceReplacements(RefactoringRuleContext &Context) {
+  createSourceReplacements(RefactoringRuleContext &Context) override {
 Expected Occurrences =
 Finder.findSymbolOccurrences(Context);
 if (!Occurrences)


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

[PATCH] D37808: [clang-tidy] Add new hicpp-multiway-paths-covered check for missing branches

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 119553.
JonasToth added a comment.

- Improve docs, grammar


https://reviews.llvm.org/D37808

Files:
  clang-tidy/hicpp/CMakeLists.txt
  clang-tidy/hicpp/HICPPTidyModule.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/hicpp-multiway-paths-covered.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/hicpp-multiway-paths-covered-else.cpp
  test/clang-tidy/hicpp-multiway-paths-covered.cpp

Index: test/clang-tidy/hicpp-multiway-paths-covered.cpp
===
--- /dev/null
+++ test/clang-tidy/hicpp-multiway-paths-covered.cpp
@@ -0,0 +1,445 @@
+// RUN: %check_clang_tidy %s hicpp-multiway-paths-covered %t
+
+enum OS { Mac,
+  Windows,
+  Linux };
+
+struct Bitfields {
+  unsigned UInt : 3;
+  int SInt : 1;
+};
+
+int return_integer() { return 42; }
+
+void bad_switch(int i) {
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use if statement
+  case 0:
+break;
+  }
+  // No default in this switch
+  switch (i) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+break;
+  case 1:
+break;
+  case 2:
+break;
+  }
+
+  // degenerate, maybe even warning
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch without labels
+  }
+
+  switch (int j = return_integer()) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+  case 2:
+break;
+  }
+
+  // Degenerated, only default case.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: degenerated switch with default label only
+  default:
+break;
+  }
+
+  // Degenerated, only one case label and default case -> Better as if-stmt.
+  switch (i) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch could be better written as if/else statement
+  case 0:
+break;
+  default:
+break;
+  }
+
+  unsigned long long BigNumber = 0;
+  switch (BigNumber) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+break;
+  }
+
+  const int &IntRef = i;
+  switch (IntRef) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+break;
+  }
+
+  char C = 'A';
+  switch (C) {
+  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 'A':
+break;
+  case 'B':
+break;
+  }
+
+  Bitfields Bf;
+  // UInt has 3 bits size.
+  switch (Bf.UInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: potential uncovered codepath; add a default case
+  case 0:
+  case 1:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.UInt) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+break;
+  }
+  // SInt has 1 bit size, so this is somewhat degenerated.
+  switch (Bf.SInt) {
+// CHECK-MESSAGES: [[@LINE-1]]:3: warning: switch with only one case; use if statement
+  case 0:
+break;
+  }
+  // All paths explicitly covered.
+  switch (Bf.SInt) {
+  case 0:
+  case 1:
+break;
+  }
+}
+
+void unproblematic_switch(unsigned char c) {
+  switch (c) {
+  case 0:
+  case 1:
+  case 2:
+  case 3:
+  case 4:
+  case 5:
+  case 6:
+  case 7:
+  case 8:
+  case 9:
+  case 10:
+  case 11:
+  case 12:
+  case 13:
+  case 14:
+  case 15:
+  case 16:
+  case 17:
+  case 18:
+  case 19:
+  case 20:
+  case 21:
+  case 22:
+  case 23:
+  case 24:
+  case 25:
+  case 26:
+  case 27:
+  case 28:
+  case 29:
+  case 30:
+  case 31:
+  case 32:
+  case 33:
+  case 34:
+  case 35:
+  case 36:
+  case 37:
+  case 38:
+  case 39:
+  case 40:
+  case 41:
+  case 42:
+  case 43:
+  case 44:
+  case 45:
+  case 46:
+  case 47:
+  case 48:
+  case 49:
+  case 50:
+  case 51:
+  case 52:
+  case 53:
+  case 54:
+  case 55:
+  case 56:
+  case 57:
+  case 58:
+  case 59:
+  case 60:
+  case 61:
+  case 62:
+  case 63:
+  case 64:
+  case 65:
+  case 66:
+  case 67:
+  case 68:
+  case 69:
+  case 70:
+  case 71:
+  case 72:
+  case 73:
+  case 74:
+  case 75:
+  case 76:
+  case 77:
+  case 78:
+  case 79:
+  case 80:
+  case 81:
+  case 82:
+  case 83:
+  case 84:
+  case 85:
+  case 86:
+  case 87:
+  case 88:
+  case 89:
+  case 90:
+  case 91:
+  case 92:
+  case 93:
+  case 94:
+  case 95:
+  case 96:
+  case 97:
+  case 98:
+  case 99:
+  case 100:
+  case 101:
+  case 102:
+  case 103:
+  case 104:
+  case 105:
+  case 106:
+  case 107:
+  case 108:
+  case 109:
+  case 110:
+  case 111:
+  case 112:
+  case 113:
+  case 114:
+  case 115:
+  case 116:
+  case 117:
+  case 118:
+  case 119:
+  case 120:
+  case 121:
+  case 122:
+  case 123:
+  case 124:
+  case 125:
+  case 126:
+  case 127:
+  case 128:
+  case 129:
+  case 130:
+  cas

r316149 - [Headers] Fix typoed __ARM_DWARF_EH__ ifdefs

[Martin Storsjo via cfe-commits]

Author: mstorsjo
Date: Thu Oct 19 00:40:45 2017
New Revision: 316149

URL: http://llvm.org/viewvc/llvm-project?rev=316149&view=rev
Log:
[Headers] Fix typoed __ARM_DWARF_EH__ ifdefs

These typos appeared in SVN r309226 and r309327.

Modified:
cfe/trunk/lib/Headers/unwind.h

Modified: cfe/trunk/lib/Headers/unwind.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Headers/unwind.h?rev=316149&r1=316148&r2=316149&view=diff
==
--- cfe/trunk/lib/Headers/unwind.h (original)
+++ cfe/trunk/lib/Headers/unwind.h Thu Oct 19 00:40:45 2017
@@ -76,7 +76,7 @@ typedef intptr_t _sleb128_t;
 typedef uintptr_t _uleb128_t;
 
 struct _Unwind_Context;
-#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || 
defined(__ARM_DWARF_EH___))
+#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || 
defined(__ARM_DWARF_EH__))
 struct _Unwind_Control_Block;
 typedef struct _Unwind_Control_Block _Unwind_Exception; /* Alias */
 #else
@@ -117,7 +117,7 @@ typedef enum {
 typedef void (*_Unwind_Exception_Cleanup_Fn)(_Unwind_Reason_Code,
  _Unwind_Exception *);
 
-#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || 
defined(__ARM_DWARF_EH___))
+#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || 
defined(__ARM_DWARF_EH__))
 typedef struct _Unwind_Control_Block _Unwind_Control_Block;
 typedef uint32_t _Unwind_EHT_Header;
 
@@ -177,7 +177,7 @@ typedef _Unwind_Personality_Fn __persona
 typedef _Unwind_Reason_Code (*_Unwind_Trace_Fn)(struct _Unwind_Context *,
 void *);
 
-#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || 
defined(__ARM_DWARF_EH___))
+#if defined(__arm__) && !(defined(__USING_SJLJ_EXCEPTIONS__) || 
defined(__ARM_DWARF_EH__))
 typedef enum {
   _UVRSC_CORE = 0,/* integer register */
   _UVRSC_VFP = 1, /* vfp */


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