[PATCH] D86743: [analyzer] Ignore VLASizeChecker case that could cause crash

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

A VLA in a loop may have different size on each iteration of the loop. This 
looks very much related to https://bugs.llvm.org/show_bug.cgi?id=28450.

> I do not know how these changes can appear

You know the node. Conditional breakpoint on the node and step-by-step 
debugging will give you all the answers.

>   // State may not be valid since constraints do not comprehend expressions
>   // used for VLAs.

That's not what the constraint manager does when it can't comprehend 
expressions. In such cases the original state would have been returned, not the 
null state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86743

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

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

[Dmitry Antipov via Phabricator via cfe-commits]

dmantipov updated this revision to Diff 293949.
dmantipov added a comment.

IIUC compiler driver is not intended to be multithreaded. But OK, here is the 
version with llvm::call_once(...).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87187

Files:
  clang/include/clang/Driver/Distro.h
  clang/lib/Driver/Distro.cpp

Index: clang/lib/Driver/Distro.cpp
===
--- clang/lib/Driver/Distro.cpp
+++ clang/lib/Driver/Distro.cpp
@@ -15,69 +15,100 @@
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Threading.h"
 
 using namespace clang::driver;
 using namespace clang;
 
-static Distro::DistroType DetectDistro(llvm::vfs::FileSystem &VFS,
-   const llvm::Triple &TargetOrHost) {
-  // If we don't target Linux, no need to check the distro. This saves a few
-  // OS calls.
-  if (!TargetOrHost.isOSLinux())
+static Distro::DistroType DetectOsRelease(llvm::vfs::FileSystem &VFS) {
+  llvm::ErrorOr> File =
+  VFS.getBufferForFile("/etc/os-release");
+  if (!File)
+File = VFS.getBufferForFile("/usr/lib/os-release");
+  if (!File)
 return Distro::UnknownDistro;
 
-  // If the host is not running Linux, and we're backed by a real file system,
-  // no need to check the distro. This is the case where someone is
-  // cross-compiling from BSD or Windows to Linux, and it would be meaningless
-  // to try to figure out the "distro" of the non-Linux host.
-  IntrusiveRefCntPtr RealFS =
-  llvm::vfs::getRealFileSystem();
-  llvm::Triple HostTriple(llvm::sys::getProcessTriple());
-  if (!HostTriple.isOSLinux() && &VFS == RealFS.get())
-return Distro::UnknownDistro;
+  SmallVector Lines;
+  File.get()->getBuffer().split(Lines, "\n");
+  Distro::DistroType Version = Distro::UnknownDistro;
+
+  // Obviously this can be improved a lot.
+  for (StringRef Line : Lines)
+if (Version == Distro::UnknownDistro && Line.startswith("ID="))
+  Version = llvm::StringSwitch(Line.substr(3))
+.Case("fedora", Distro::Fedora)
+.Case("gentoo", Distro::Gentoo)
+.Case("arch", Distro::ArchLinux)
+// On SLES, /etc/os-release was introduced in SLES 11.
+.Case("sles", Distro::OpenSUSE)
+.Case("opensuse", Distro::OpenSUSE)
+.Case("exherbo", Distro::Exherbo)
+.Default(Distro::UnknownDistro);
+  return Version;
+}
 
+static Distro::DistroType DetectLsbRelease(llvm::vfs::FileSystem &VFS) {
   llvm::ErrorOr> File =
   VFS.getBufferForFile("/etc/lsb-release");
-  if (File) {
-StringRef Data = File.get()->getBuffer();
-SmallVector Lines;
-Data.split(Lines, "\n");
-Distro::DistroType Version = Distro::UnknownDistro;
-for (StringRef Line : Lines)
-  if (Version == Distro::UnknownDistro && Line.startswith("DISTRIB_CODENAME="))
-Version = llvm::StringSwitch(Line.substr(17))
-  .Case("hardy", Distro::UbuntuHardy)
-  .Case("intrepid", Distro::UbuntuIntrepid)
-  .Case("jaunty", Distro::UbuntuJaunty)
-  .Case("karmic", Distro::UbuntuKarmic)
-  .Case("lucid", Distro::UbuntuLucid)
-  .Case("maverick", Distro::UbuntuMaverick)
-  .Case("natty", Distro::UbuntuNatty)
-  .Case("oneiric", Distro::UbuntuOneiric)
-  .Case("precise", Distro::UbuntuPrecise)
-  .Case("quantal", Distro::UbuntuQuantal)
-  .Case("raring", Distro::UbuntuRaring)
-  .Case("saucy", Distro::UbuntuSaucy)
-  .Case("trusty", Distro::UbuntuTrusty)
-  .Case("utopic", Distro::UbuntuUtopic)
-  .Case("vivid", Distro::UbuntuVivid)
-  .Case("wily", Distro::UbuntuWily)
-  .Case("xenial", Distro::UbuntuXenial)
-  .Case("yakkety", Distro::UbuntuYakkety)
-  .Case("zesty", Distro::UbuntuZesty)
-  .Case("artful", Distro::UbuntuArtful)
-  .Case("bionic", Distro::UbuntuBionic)
-  .Case("cosmic", Distro::UbuntuCosmic)
-  .Case("disco", Distro::UbuntuDisco)
-  .Case("eoan", Distro::UbuntuEoan)
-  .Case("focal", Distro::UbuntuFocal)
-  .Case("groovy", Distro::UbuntuGroovy)
-  .Default(Distro::UnknownDistro);
-if (Version != Distro::UnknownDistro)
-  return Version;
-  }
+  if (!File)
+return Distro::UnknownDistro;
+
+  SmallVector Lines;
+  File.get()->getBuffer().split(Lines, "\n");
+  Distro::DistroType Versi

[PATCH] D87901: [Driver] Filter out /gcc and /gcc-cross if they do not exists

[Dmitry Antipov via Phabricator via cfe-commits]

dmantipov updated this revision to Diff 293955.
dmantipov added a comment.

Prefer a bit more meaningful variable names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87901

Files:
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/Gnu.h


Index: clang/lib/Driver/ToolChains/Gnu.h
===
--- clang/lib/Driver/ToolChains/Gnu.h
+++ clang/lib/Driver/ToolChains/Gnu.h
@@ -215,6 +215,10 @@
 // Gentoo-specific toolchain configurations are stored here.
 const std::string GentooConfigDir = "/etc/env.d/gcc";
 
+// Internal flags used to filter out /gcc and /gcc-cross.
+bool GCCDirExists;
+bool GCCCrossDirExists;
+
   public:
 explicit GCCInstallationDetector(const Driver &D) : IsValid(false), D(D) {}
 void init(const llvm::Triple &TargetTriple, const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1945,6 +1945,9 @@
   const std::string LibDir = Prefix + Suffix.str();
   if (!D.getVFS().exists(LibDir))
 continue;
+  // Maybe filter out /gcc and /gcc-cross.
+  GCCDirExists = D.getVFS().exists(LibDir + "/gcc");
+  GCCCrossDirExists = D.getVFS().exists(LibDir + "/gcc-cross");
   // Try to match the exact target triple first.
   ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, TargetTriple.str());
   // Try rest of possible triples.
@@ -2463,12 +2466,13 @@
 // Whether this library suffix is relevant for the triple.
 bool Active;
   } Suffixes[] = {
-  // This is the normal place.
-  {"gcc/" + CandidateTriple.str(), "../..", true},
+  // This is the normal place if Clang is installed alongside with GCC,
+  // probably with the same prefix. But it's likely does not exists in
+  // case of standalone Clang install.
+  {"gcc/" + CandidateTriple.str(), "../..", GCCDirExists},
 
   // Debian puts cross-compilers in gcc-cross.
-  {"gcc-cross/" + CandidateTriple.str(), "../..",
-   TargetTriple.getOS() != llvm::Triple::Solaris},
+  {"gcc-cross/" + CandidateTriple.str(), "../..", GCCCrossDirExists},
 
   // The Freescale PPC SDK has the gcc libraries in
   // /usr/lib//x.y.z so have a look there as well. Only do


Index: clang/lib/Driver/ToolChains/Gnu.h
===
--- clang/lib/Driver/ToolChains/Gnu.h
+++ clang/lib/Driver/ToolChains/Gnu.h
@@ -215,6 +215,10 @@
 // Gentoo-specific toolchain configurations are stored here.
 const std::string GentooConfigDir = "/etc/env.d/gcc";
 
+// Internal flags used to filter out /gcc and /gcc-cross.
+bool GCCDirExists;
+bool GCCCrossDirExists;
+
   public:
 explicit GCCInstallationDetector(const Driver &D) : IsValid(false), D(D) {}
 void init(const llvm::Triple &TargetTriple, const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -1945,6 +1945,9 @@
   const std::string LibDir = Prefix + Suffix.str();
   if (!D.getVFS().exists(LibDir))
 continue;
+  // Maybe filter out /gcc and /gcc-cross.
+  GCCDirExists = D.getVFS().exists(LibDir + "/gcc");
+  GCCCrossDirExists = D.getVFS().exists(LibDir + "/gcc-cross");
   // Try to match the exact target triple first.
   ScanLibDirForGCCTriple(TargetTriple, Args, LibDir, TargetTriple.str());
   // Try rest of possible triples.
@@ -2463,12 +2466,13 @@
 // Whether this library suffix is relevant for the triple.
 bool Active;
   } Suffixes[] = {
-  // This is the normal place.
-  {"gcc/" + CandidateTriple.str(), "../..", true},
+  // This is the normal place if Clang is installed alongside with GCC,
+  // probably with the same prefix. But it's likely does not exists in
+  // case of standalone Clang install.
+  {"gcc/" + CandidateTriple.str(), "../..", GCCDirExists},
 
   // Debian puts cross-compilers in gcc-cross.
-  {"gcc-cross/" + CandidateTriple.str(), "../..",
-   TargetTriple.getOS() != llvm::Triple::Solaris},
+  {"gcc-cross/" + CandidateTriple.str(), "../..", GCCCrossDirExists},
 
   // The Freescale PPC SDK has the gcc libraries in
   // /usr/lib//x.y.z so have a look there as well. Only do
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Ten Tzen via Phabricator via cfe-commits]

tentzen added a comment.

In D80344#2291688 , @rjmccall wrote:

> In D80344#2291456 , @tentzen wrote:
>
>> Reordering is NOT okay for instructions 'directly' within a _try. 'directly' 
>> here means it's the code from source code originally, i.e., inlined code or 
>> the code in callees is not included. Disabling inline in general is good, 
>> but not necessary.
>> As said above, SEH semantic only applies to code "directly" under SEH scope 
>> for few obvious reasons (Ex, indirect callees or callees not in the same 
>> module wont be visible).  if faulty instructions are allowed to be reordered 
>> inside a callee, those instructions are allowed to do so even after they are 
>> inlined into _try.
>
> I feel almost certain that this is wrong.  Let me try to explain.  It would 
> be helpful to have a more precise statement of semantics than I can find 
> online, but we can start from the basics.
>
> "X and Y can be reordered" is how optimizer folks talk, but it's not 
> generally how semantics are specified.  As a general rule (ignoring sequence 
> points and concurrency for simplicity), C and C++ require operations to be 
> executed in a particular order on the abstract machine, and there are no 
> rules explicitly sanctioning reordering.  Instead, the "as if" rule gives the 
> implementation broad leeway to do things differently from how they're 
> specified on the abstract machine, as long as the difference cannot be 
> observed by a valid program.  Faults generally correspond to conditions that 
> have undefined behavior under the language, and so the implementation may 
> assume they do not happen in a valid program, and so the consequences of them 
> happening cannot be observed by a valid program, and so faults can be ignored 
> when deciding whether to reorder operations, which allows a lot of reordering.
>
> `_try` should be understood as fully defining the behavior of faulting 
> operations written within its scope so that they have the defined semantics 
> of resuming execution in the `_except` clause.  Because faults have the 
> defined behavior of ending execution of the `_try` block, whether the fault 
> occurred is observable, so the "as if" leeway stops applying.  Thus, other 
> operations with side-effects cannot be reordered with potentially faulty 
> operations written within the `_try`, no more than they could be reordered 
> with a `break` statement.  That applies whether those other operations are 
> written within the `_try` or not; it's just that potentially-faulty 
> operations written within the `_try` must always be treated as having 
> side-effects.
>
> `-EHa` more generally should be understand as partially defining the behavior 
> of faulting operations even if they are not written in a `_try`: if the 
> operation faults, the behavior is defined so that scopes are unwound and 
> control resumes at an `_except` clause if one is dynamically active, but this 
> may be observed at an  indeterminate point.  It is hard to describe these 
> semantics formally, but the intended rules for the implementation are pretty 
> clear: potentially faulty operations outside of a `_try` can be reordered 
> around each other or even moved to different scopes, as per normal 
> optimization rules, but whenever those operations are executed, if they fault 
> they must trigger an unwind and cause execution to resume at an `_except` 
> clause if one is active.
>
> So I think you cannot allow operations inlined from a call made within a 
> `_try` to be reordered with operations written within the `_try`, or to 
> happen outside the `_try`.  Since LLVM doesn't promise not to reorder 
> volatile and non-volatile stores to provably non-aliased locations, you 
> cannot safely inline non-volatile stores within a `_try`.  You can either 
> disable inlining or mark the calls in some way that will cause the inliner to 
> make any inlined stores volatile (and whatever else is necessary to ensure 
> they will not be reordered).

Oh, yes you are right.  I was talking about reordering between inlined faulty 
instructions is allowed.  Reordering inlined instruction with any 'direct' 
volatile instruction should be prohibited.  I overlooked LLVM doesn't promise 
not to reorder volatile and non-volatile. I will simply disable inlining into a 
_try scope.  thanks!

>> Volatile-ness here is primary for reordering constraint, not for data-flow. 
>> Per LLVM EH framework, a data-defined in _try will flow through explicit EH 
>> edge into the Handler.  In this SEH design, there will be a seh.try.end() at 
>> every exit of SEH region that ensures data defined in _try scope flow to the 
>> use in Handler.
>
> Where is the "explicit EH edge into the handler" from a faulting instruction? 
>  It seems like there's an EH edge to the innermost cleanup from the last 
> cleanup scope you entered, but that's not the same as from a faulting 
> instructio

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added a comment.

In D87891#2291760 , @kadircet wrote:

> The biggest problem I see is, this is not changing the fact that we are still 
> traversing the whole file:
>
> - You do one traversal over the whole file to find `FileLines`
> - Then possibly two more to find `OffsetMin` and `OffsetMax`

Seems you are right, but we do not compare strings during traversal to find 
`FileLines`.

> You can get rid of the first one by just using `getLocForEndOfFile` if 
> `positionToOffset` fails.
> For the latter, you can use `SourceManager::translateLineCol` instead, it 
> uses a cache and cheaper than the linear scan performed by `positionToOffset` 
> in case of multiple calls. The cache is already populated once we issue the 
> first `Cost` call, through `SM.getSpellingLineNumber`
>
> I was reluctant overall as the wins aren't clear. We are still going to 
> traverse the whole file at least once, and readability is hindered but I 
> suppose with the above mentioned two trics, runtime shouldn't be regressed.
> Also it is nice to get rid of 2 log calls for each token. Again all of these 
> feels like some micro optimizations that aren't justified.

Thanks for your reply, I will rethink this patch.




Comment at: clang-tools-extra/clangd/XRefs.cpp:568
 if (Line < WordLine)
-  return 2 + llvm::Log2_64(WordLine - Line);
+  return 2 + WordLine - Line;
 return 0;

sammccall wrote:
> this has changed the relative ordering, if we're dropping the log then +1 
> should now become multiplication by two.
> (Or was this intended?)
Yes, you are right, my fault here.

But should we penalize backwards so hard?



Comment at: clang-tools-extra/clangd/XRefs.cpp:580
   return false;
-// No point guessing the same location we started with.
-if (Tok.location() == Word.Location)

sammccall wrote:
> why can't this happen anymore?
As I understand, we call `findNearbyIdentifier()` only when the word is not an 
identifier. And `Tok` is always identifier here.

Btw, even if `findNearbyIdentifier()` could be called for an identifier, I 
could not get why it's bad to return current position here.

Am I wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

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

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D87891#2291838 , @ArcsinX wrote:

> Thanks for your reply, I will rethink this patch.

FWIW I think if we drop most of the math in favor of using SourceManager's line 
translation, this doesn't add much complexity and is probably a bit more direct 
and efficient.




Comment at: clang-tools-extra/clangd/XRefs.cpp:580
   return false;
-// No point guessing the same location we started with.
-if (Tok.location() == Word.Location)

ArcsinX wrote:
> sammccall wrote:
> > why can't this happen anymore?
> As I understand, we call `findNearbyIdentifier()` only when the word is not 
> an identifier. And `Tok` is always identifier here.
> 
> Btw, even if `findNearbyIdentifier()` could be called for an identifier, I 
> could not get why it's bad to return current position here.
> 
> Am I wrong?
> As I understand, we call findNearbyIdentifier() only when the word is not an 
> identifier

I'm not sure where you're getting that.
If you mean the bail-out inside the function itself:

```
  // Don't use heuristics if this is a real identifier.
  // Unlikely identifiers are OK if they were used as identifiers nearby.
  if (Word.ExpandedToken)
return nullptr;
```

then "real identifier" here means it's an identifier after preprocessing.
We're still getting here if it's e.g. part of a macro body, or code that's 
`#ifdef`'d out.
The spelled token in those cases is an identifier, there's just no 
corresponding expanded token.

(I'm surprised we don't have a test covering this though!)

> Btw, even if findNearbyIdentifier() could be called for an identifier, I 
> could not get why it's bad to return current position here.

The point of this function is that *this* occurrence of the word can't be 
resolved, so let's try another one nearby.
If we just return the same occurrence, then we're certainly not going to be 
able to resolve it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

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

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D71524#2284386 , @Szelethus wrote:

> I figured you're still working on this, sorry! I'd really like to chat about 
> my earlier comment D71524#1917251 , 
> as it kind of challenges the high level idea.

What about marking the `std::cin` object itself as tainted and any object 
created by `ifstream::ifstream(const char*)` or similar functions.
Then propagate taint via the extraction operator (`operator>>`) only if the 
stream was tainted.
This way we could reduce the false-positives of this crude heuristic. What do 
you think?


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

https://reviews.llvm.org/D71524

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  I think we're on the same page about representation now.  If you can 
come up with a good replacement for "eha" in the intrinsic names, I think this 
is pretty much ready to go.




Comment at: llvm/docs/LangRef.rst:11584
+These intrinsics make Block-level State computation possible in downstream
+LLVM code gen pass, even in the presence of ctor/dtor inlining.
+

This is a very emission-centric description of the intrinsic.  Maybe something 
like:

  LLVM's ordinary exception-handling representation associates EH cleanups and
  handlers only with ``invoke``s, which normally correspond only to call sites. 
 To
  support arbitrary faulting instructions, it must be possible to recover the 
current
  EH scope for any instruction.  Turning every operation in LLVM that could 
fault
  into an ``invoke`` of a new, potentially-throwing intrinsic would require 
adding a
  large number of intrinsics, impede optimization of those operations, and make
  compilation slower by introducing many extra basic blocks.  These intrinsics 
can
  be used instead to mark the region protected by a cleanup, such as for a local
  C++ object with a non-trivial destructor.  ``llvm.eha.scope.begin`` is used 
to mark
  the start of the region; it is aways called with ``invoke``, with the unwind 
block
  being the desired unwind destination for any potentially-throwing instructions
  within the region.  `llvm.eha.scope.end` is used to mark when the scope ends
  and the EH cleanup is no longer required (e.g. because the destructor is being
  called).

So, about that — how does this pairing actually work?  I think I understand how 
it's *meant* to work, by just scanning the structure of the CFG.  But Clang 
doesn't promise to emit blocks with that kind of structure, and in practice 
block structure gets pretty complicated when you have multiple edges out of a 
region and they have to go to destinations at different cleanup depths.  I 
don't really understand how your analysis looks through that sort of thing.

Example:

```
#include 
void test(int x, int &state) {
  state = 0;
  std::string a;
  state = 1;
  if (x > 0) {
state = 2;
std::string b;
state = 3;
if (x > 10) {
  state = 4;
  return;
}
state = 5;
std::string c;
state = 6;
  }
  state = 7;
}
```

IIRC, the way Clang will emit this is something like:

```
void test(int x, int &state) {
  int jumpdest;
  state = 0;
  std::string a;
  state = 1;
  if (x > 0) {
state = 2;
std::string b;
state = 3;
if (x > 10) {
  state = 4;
  jumpdest = 0;
  goto destroy_b;
}
state = 5;
std::string c;
state = 6;
c.~std::string();
jumpdest = 1;
  destroy_b:
b.~std::string();
switch (jumpdest) {
case 0: goto destroy_a;
case 1: goto fallthrough;
}
  fallthrough:
;
  }
  state = 7;
destroy_a:
  a.~std::string();
}
```

The point being that I'm not sure the stack-like relationship between these 
cleanup scopes that's present in the source actually reliably survives into IR. 
 That makes me concerned about the implicit pairing happening with these 
intrinsics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D52676: [clang-format] tweaked another case of lambda formatting

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I think if you have a proposal for changing the behavior lets see the patch and 
how it impacts the existing unit tests

something tells me these tests are going to change?

  // A lambda passed as arg0 is always pushed to the next line.
  verifyFormat("SomeFunction(\n"
   "[this] {\n"
   "  //\n"
   "},\n"
   "1);\n");
  
  
  // A multi-line lambda passed as arg1 forces arg0 to be pushed out, just like 
the arg0
  // case above.
  auto Style = getGoogleStyle();
  Style.BinPackArguments = false;
  verifyFormat("SomeFunction(\n"
   "a,\n"
   "[this] {\n"
   "  //\n"
   "},\n"
   "b);\n",
   Style);
  verifyFormat("SomeFunction(\n"
   "a,\n"
   "[this] {\n"
   "  //\n"
   "},\n"
   "b);\n");


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52676

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

[PATCH] D88088: WIP [clang] improve accuracy of ExprMutAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

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

- improve the expression matcher, to catch the expression in general 
ternary-operator cases, too
- considered unresolved operator calls to be a modification
- fix method calls with templates in some instances


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88088

Files:
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -19,9 +19,7 @@
 
 using namespace clang::ast_matchers;
 using ::testing::ElementsAre;
-using ::testing::IsEmpty;
 using ::testing::ResultOf;
-using ::testing::StartsWith;
 using ::testing::Values;
 
 namespace {
@@ -63,13 +61,18 @@
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
-std::string buffer;
+if (!By)
+  break;
+
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
-Chain.push_back(StringRef(stream.str()).trim().str());
+Chain.emplace_back(StringRef(stream.str()).trim().str());
 E = dyn_cast(By);
+buffer.clear();
   }
   return Chain;
 }
@@ -111,7 +114,13 @@
 
 class AssignmentTest : public ::testing::TestWithParam {};
 
+// This test is for the most basic and direct modification of a variable,
+// assignment to it (e.g. `x = 10;`).
+// It additionally tests, that reference to a variable are not only captured
+// directly, but expression that result in the variable are handled, too.
+// This includes the comma operator, parens and the ternary operator.
 TEST_P(AssignmentTest, AssignmentModifies) {
+  // Test the detection of the raw expression modifications.
   {
 const std::string ModExpr = "x " + GetParam() + " 10";
 const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -120,6 +129,7 @@
 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
   }
 
+  // Test the detection if the expression is surrounded by parens.
   {
 const std::string ModExpr = "(x) " + GetParam() + " 10";
 const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -127,6 +137,60 @@
 match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
   }
+
+  // Test the detection if the comma operator yields the expression as result.
+  {
+const std::string ModExpr = "x " + GetParam() + " 10";
+const auto AST = buildASTFromCodeWithArgs(
+"void f() { int x, y; y, " + ModExpr + "; }", {"-Wno-unused-value"});
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Ensure no detection if t he comma operator does not yield the expression as
+  // result.
+  {
+const std::string ModExpr = "y, x, y " + GetParam() + " 10";
+const auto AST = buildASTFromCodeWithArgs(
+"void f() { int x, y; " + ModExpr + "; }", {"-Wno-unused-value"});
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_FALSE(isMutated(Results, AST.get()));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression.
+  {
+const std::string ModExpr = "(y != 0 ? y : x) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression
+  // through multiple nesting of ternary operators.
+  {
+const std::string ModExpr =
+"(y != 0 ? (y > 5 ? y : x) : (y)) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression
+  // with additional parens.
+  {
+const std::string ModExpr = "(y != 0 ? (y) : ((x))) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const aut

[PATCH] D87891: [clangd] findNearbyIdentifier(): guaranteed to give up after 2^N lines

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:580
   return false;
-// No point guessing the same location we started with.
-if (Tok.location() == Word.Location)

sammccall wrote:
> ArcsinX wrote:
> > sammccall wrote:
> > > why can't this happen anymore?
> > As I understand, we call `findNearbyIdentifier()` only when the word is not 
> > an identifier. And `Tok` is always identifier here.
> > 
> > Btw, even if `findNearbyIdentifier()` could be called for an identifier, I 
> > could not get why it's bad to return current position here.
> > 
> > Am I wrong?
> > As I understand, we call findNearbyIdentifier() only when the word is not 
> > an identifier
> 
> I'm not sure where you're getting that.
> If you mean the bail-out inside the function itself:
> 
> ```
>   // Don't use heuristics if this is a real identifier.
>   // Unlikely identifiers are OK if they were used as identifiers nearby.
>   if (Word.ExpandedToken)
> return nullptr;
> ```
> 
> then "real identifier" here means it's an identifier after preprocessing.
> We're still getting here if it's e.g. part of a macro body, or code that's 
> `#ifdef`'d out.
> The spelled token in those cases is an identifier, there's just no 
> corresponding expanded token.
> 
> (I'm surprised we don't have a test covering this though!)
> 
> > Btw, even if findNearbyIdentifier() could be called for an identifier, I 
> > could not get why it's bad to return current position here.
> 
> The point of this function is that *this* occurrence of the word can't be 
> resolved, so let's try another one nearby.
> If we just return the same occurrence, then we're certainly not going to be 
> able to resolve it.
Thanks for your clarification. I will revert this change (and will try to add a 
test as well).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87891

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

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

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

  rebase to current exprmutanalyzer
  
  - remove spurious formatting change
  - fix include and typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/ConstCorrectnessCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-const-correctness.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-cxx17.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-pointer-as-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-transform-values.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-const-correctness-values.cpp
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -19,9 +19,7 @@
 
 using namespace clang::ast_matchers;
 using ::testing::ElementsAre;
-using ::testing::IsEmpty;
 using ::testing::ResultOf;
-using ::testing::StartsWith;
 using ::testing::Values;
 
 namespace {
@@ -63,13 +61,18 @@
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
-std::string buffer;
+if (!By)
+  break;
+
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
-Chain.push_back(StringRef(stream.str()).trim().str());
+Chain.emplace_back(StringRef(stream.str()).trim().str());
 E = dyn_cast(By);
+buffer.clear();
   }
   return Chain;
 }
@@ -111,7 +114,13 @@
 
 class AssignmentTest : public ::testing::TestWithParam {};
 
+// This test is for the most basic and direct modification of a variable,
+// assignment to it (e.g. `x = 10;`).
+// It additionally tests, that reference to a variable are not only captured
+// directly, but expression that result in the variable are handled, too.
+// This includes the comma operator, parens and the ternary operator.
 TEST_P(AssignmentTest, AssignmentModifies) {
+  // Test the detection of the raw expression modifications.
   {
 const std::string ModExpr = "x " + GetParam() + " 10";
 const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -120,6 +129,7 @@
 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
   }
 
+  // Test the detection if the expression is surrounded by parens.
   {
 const std::string ModExpr = "(x) " + GetParam() + " 10";
 const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -127,6 +137,60 @@
 match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
   }
+
+  // Test the detection if the comma operator yields the expression as result.
+  {
+const std::string ModExpr = "x " + GetParam() + " 10";
+const auto AST = buildASTFromCodeWithArgs(
+"void f() { int x, y; y, " + ModExpr + "; }", {"-Wno-unused-value"});
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Ensure no detection if t he comma operator does not yield the expression as
+  // result.
+  {
+const std::string ModExpr = "y, x, y " + GetParam() + " 10";
+const auto AST = buildASTFromCodeWithArgs(
+"void f() { int x, y; " + ModExpr + "; }", {"-Wno-unused-value"});
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_FALSE(isMutated(Results, AST.get()));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression.
+  {
+const std::string ModExpr = "(y != 0 ? y : x) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  /

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D77062#1976414 , @Szelethus wrote:

> I think what what be great to see here (and this seems to be the thing @NoQ 
> is fishing for) is not to change an if branch and avoid running into the 
> crash, but rather find out why `assumeZero` was ever called with a `nonloc` 
> value, which shouldn't really happen. We're treating the symptom, not curing 
> the illness, if you will. The `SVal` (if its `DefinedSVal`) is supposed to be 
> always `MemRegionVal` here, is that right? Maybe if we tried to add an assert 
> here, that could help nail where the actual issue is coming from. It's 
> probably in `evalStrcpyCommon`, judging from the bug report you linked in 
> your summary.

I would not accept this patch unless this investigation is done. However, I'm 
not inherently against this change.


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

https://reviews.llvm.org/D77062

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

[PATCH] D88019: [analyzer][solver] Fix issue with symbol non-equality tracking

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

What are our options mitigating anything similar happening in the future?

This way any change touching the `SymbolicRangeInferrer` and any related parts 
of the analyzer seems to be way too fragile.
Especially, since we might want to add support for comparing SymSyms, just like 
we try to do in D77792 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88019

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

[clang-tools-extra] 64168c6 - [clangd] Disable suffix matching fallback for C during include insertion

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-09-24T10:46:10+02:00
New Revision: 64168c6d996b6fdd017488785e0e9ce5ce05be54

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

LOG: [clangd] Disable suffix matching fallback for C during include insertion

Clangd currently doesn't respect language and breaks the builds with
include insertion for C. This patch aims to stop the bleeding by not mapping
back to CPP standard library headers.

Improves https://github.com/clangd/clangd/issues/376.

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

Added: 


Modified: 
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp 
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index a3b13347f23e..2822e359c0a5 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -772,7 +772,10 @@ void CanonicalIncludes::addSystemHeadersMapping(const 
LangOptions &Language) {
   MaxSuffixComponents;
  }) != SystemHeaderMap->keys().end());
 
-  StdSuffixHeaderMapping = SystemHeaderMap;
+  // FIXME: Suffix mapping contains invalid entries for C, so only enable it 
for
+  // CPP.
+  if (Language.CPlusPlus)
+StdSuffixHeaderMapping = SystemHeaderMap;
 }
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp 
b/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
index 8b613f56c07b..7969b638d3d3 100644
--- a/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -21,6 +21,10 @@ TEST(CanonicalIncludesTest, CStandardLibrary) {
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
+  // Suffix mapping isn't available for C, instead of mapping to ` we
+  // just leave the header as-is.
+  EXPECT_EQ("include/stdio.h",
+CI.mapHeader("include/stdio.h", "unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {



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

[PATCH] D88144: [clangd] Disable suffix matching fallback for C during include insertion

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D88144#2290830 , @sammccall wrote:

> (I do wonder whether it's safe to just drop the mapping table entirely now...)

As discussed offline;
It still helps with mapping symbols missing from our cppreference extraction 
back to umbrella headers. Not sure how often this happens in practice anymore, 
but it will likely be a regression.
So gonna drop it completely in a different patch, which can be reverted quickly 
if things go wrong. That way we'll learn about what's currently missing, rather 
than masking the bugs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88144

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

[PATCH] D88144: [clangd] Disable suffix matching fallback for C during include insertion

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG64168c6d996b: [clangd] Disable suffix matching fallback for 
C during include insertion (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88144

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp


Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -21,6 +21,10 @@
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
+  // Suffix mapping isn't available for C, instead of mapping to ` we
+  // just leave the header as-is.
+  EXPECT_EQ("include/stdio.h",
+CI.mapHeader("include/stdio.h", "unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -772,7 +772,10 @@
   MaxSuffixComponents;
  }) != SystemHeaderMap->keys().end());
 
-  StdSuffixHeaderMapping = SystemHeaderMap;
+  // FIXME: Suffix mapping contains invalid entries for C, so only enable it 
for
+  // CPP.
+  if (Language.CPlusPlus)
+StdSuffixHeaderMapping = SystemHeaderMap;
 }
 
 } // namespace clangd


Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -21,6 +21,10 @@
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
+  // Suffix mapping isn't available for C, instead of mapping to ` we
+  // just leave the header as-is.
+  EXPECT_EQ("include/stdio.h",
+CI.mapHeader("include/stdio.h", "unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -772,7 +772,10 @@
   MaxSuffixComponents;
  }) != SystemHeaderMap->keys().end());
 
-  StdSuffixHeaderMapping = SystemHeaderMap;
+  // FIXME: Suffix mapping contains invalid entries for C, so only enable it for
+  // CPP.
+  if (Language.CPlusPlus)
+StdSuffixHeaderMapping = SystemHeaderMap;
 }
 
 } // namespace clangd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 98756d8 - [clangd] Fix comment. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-09-24T10:49:39+02:00
New Revision: 98756d865b696bece858ff0832ed3687b6b8ba4b

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

LOG: [clangd] Fix comment. NFC

Added: 


Modified: 
clang-tools-extra/clangd/XRefs.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/XRefs.cpp 
b/clang-tools-extra/clangd/XRefs.cpp
index 67c857c378e1..973816253f42 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -445,8 +445,8 @@ locateSymbolTextually(const SpelledWord &Word, ParsedAST 
&AST,
   if ((Word.ExpandedToken && !isDependentName(NodeKind)) ||
   !Word.LikelyIdentifier || !Index)
 return {};
-  // We don't want to handle words in string literals. It'd be nice to include
-  // comments, but they're not retained in TokenBuffer.
+  // We don't want to handle words in string literals. (It'd be nice to list
+  // *allowed* token kinds explicitly, but comment Tokens aren't retained).
   if (Word.PartOfSpelledToken &&
   isStringLiteral(Word.PartOfSpelledToken->kind()))
 return {};
@@ -548,8 +548,8 @@ const syntax::Token *findNearbyIdentifier(const SpelledWord 
&Word,
   // Unlikely identifiers are OK if they were used as identifiers nearby.
   if (Word.ExpandedToken)
 return nullptr;
-  // We don't want to handle words in string literals. It'd be nice to include
-  // comments, but they're not retained in TokenBuffer.
+  // We don't want to handle words in string literals. (It'd be nice to list
+  // *allowed* token kinds explicitly, but comment Tokens aren't retained).
   if (Word.PartOfSpelledToken &&
   isStringLiteral(Word.PartOfSpelledToken->kind()))
 return {};



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

[PATCH] D86295: [analyzer] Reorder the layout of MemRegion and cache by hand for optimal size

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

Should I create more measurements?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86295

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

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@AlexanderLanin @0x8000- i created the branch `release-11-const` 
(https://github.com/JonasToth/llvm-project/tree/release-11-const) in my fork.

This branch is based on 11-rc3 and cherry picks the commits that correspond to 
this revision.
I hope this is of any use for you! I will probably forget to update this branch 
all the time, but if I feel like there has been actual progress made, i will 
update!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 293974.
sepavloff marked an inline comment as done.
sepavloff added a comment.

Updated patch

- improved Expr::getFPFeaturesInEffect,
- added tests for _Complex,
- added tests for constexpr,
- added chech in HandleFloatToFloatCast,
- implemented printing FPOptions in CompoundAssignOperator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87822

Files:
  clang/include/clang/AST/Expr.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/AST/const-fpfeatures-diag.c
  clang/test/AST/const-fpfeatures.c
  clang/test/AST/const-fpfeatures.cpp

Index: clang/test/AST/const-fpfeatures.cpp
===
--- /dev/null
+++ clang/test/AST/const-fpfeatures.cpp
@@ -0,0 +1,81 @@
+// RUN: %clang_cc1 -S -emit-llvm -triple i386-linux -std=c++2a -Wno-unknown-pragmas %s -o - | FileCheck %s
+
+// nextUp(1.F) == 0x1.02p0F
+
+constexpr float add_round_down(float x, float y) {
+  #pragma STDC FENV_ROUND FE_DOWNWARD
+  float res = x;
+  res += y;
+  return res;
+}
+
+constexpr float add_round_up(float x, float y) {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  float res = x;
+  res += y;
+  return res;
+}
+
+float V1 = add_round_down(1.0F, 0x0.01p0F);
+float V2 = add_round_up(1.0F, 0x0.01p0F);
+// CHECK: @V1 = {{.*}} float 1.00e+00
+// CHECK: @V2 = {{.*}} float 0x3FF02000
+
+constexpr float add_cast_round_down(float x, double y) {
+  #pragma STDC FENV_ROUND FE_DOWNWARD
+  float res = x;
+  res += y;
+  return res;
+}
+
+constexpr float add_cast_round_up(float x, double y) {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  float res = x;
+  res += y;
+  return res;
+}
+
+float V3 = add_cast_round_down(1.0F, 0x0.01p0F);
+float V4 = add_cast_round_up(1.0F, 0x0.01p0F);
+
+// CHECK: @V3 = {{.*}} float 1.00e+00
+// CHECK: @V4 = {{.*}} float 0x3FF02000
+
+// The next three variables use the same function as initializer, only rounding
+// modes differ.
+
+float V5 = []() -> float {
+  return [](float x, float y)->float {
+#pragma STDC FENV_ROUND FE_UPWARD
+return x + y;
+  }([](float x, float y) -> float {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  return x + y;
+}(1.0F, 0x0.01p0F),
+  0x0.01p0F);
+}();
+// CHECK: @V5 = {{.*}} float 0x3FF04000
+
+float V6 = []() -> float {
+  return [](float x, float y)->float {
+#pragma STDC FENV_ROUND FE_DOWNWARD
+return x + y;
+  }([](float x, float y) -> float {
+  #pragma STDC FENV_ROUND FE_UPWARD
+  return x + y;
+}(1.0F, 0x0.01p0F),
+  0x0.01p0F);
+}();
+// CHECK: @V6 = {{.*}} float 0x3FF02000
+
+float V7 = []() -> float {
+  return [](float x, float y)->float {
+#pragma STDC FENV_ROUND FE_DOWNWARD
+return x + y;
+  }([](float x, float y) -> float {
+  #pragma STDC FENV_ROUND FE_DOWNWARD
+  return x + y;
+}(1.0F, 0x0.01p0F),
+  0x0.01p0F);
+}();
+// CHECK: @V7 = {{.*}} float 1.00e+00
Index: clang/test/AST/const-fpfeatures.c
===
--- /dev/null
+++ clang/test/AST/const-fpfeatures.c
@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -S -emit-llvm -Wno-unknown-pragmas %s -o - | FileCheck %s
+
+// nextUp(1.F) == 0x1.02p0F
+
+const double _Complex C0 = 0x1.01p0 + 0x1.01p0I;
+
+#pragma STDC FENV_ROUND FE_UPWARD
+
+float F1u = 1.0F + 0x0.02p0F;
+float F2u = 1.0F + 0x0.01p0F;
+float F3u = 0x1.01p0;
+// CHECK: @F1u = {{.*}} float 0x3FF02000
+// CHECK: @F2u = {{.*}} float 0x3FF02000
+// CHECK: @F3u = {{.*}} float 0x3FF02000
+
+float _Complex C1u = C0;
+// CHECK: @C1u = {{.*}} { float, float } { float 0x3FF02000, float 0x3FF02000 }
+
+
+#pragma STDC FENV_ROUND FE_DOWNWARD
+
+float F1d = 1.0F + 0x0.02p0F;
+float F2d = 1.0F + 0x0.01p0F;
+float F3d = 0x1.01p0;
+
+// CHECK: @F1d = {{.*}} float 0x3FF02000
+// CHECK: @F2d = {{.*}} float 1.00e+00
+// CHECK: @F3d = {{.*}} float 1.00e+00
+
+float _Complex C1d = C0;
+// CHECK: @C1d = {{.*}} { float, float } { float 1.00e+00, float 1.00e+00 }
Index: clang/test/AST/const-fpfeatures-diag.c
===
--- /dev/null
+++ clang/test/AST/const-fpfeatures-diag.c
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -verify -ffp-exception-behavior=strict -Wno-unknown-pragmas %s
+
+#pragma STDC FENV_ROUND FE_DYNAMIC
+
+// nextUp(1.F) == 0x1.02p0F
+
+float F1 = 0x1.00p0F + 0x0.02p0F;
+float F2 = 0x1.00p0F + 0x0.01p0F; // expected-error{{initializer element is not a compile-time constant}}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -981,7 +981,9 @@
 void Sema::setRoundin

[PATCH] D87822: [FPEnv] Evaluate constant expressions under non-default rounding modes

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2427
+FPOptions FPFeatures = Cast->getFPFeaturesInEffect(Info.Ctx.getLangOpts());
+RM = FPFeatures.getRoundingMode();
+  }

rjmccall wrote:
> sepavloff wrote:
> > rjmccall wrote:
> > > I think the options really need to be passed in or else correctness is 
> > > somewhat doomed here.
> > > 
> > > For example, the call to CompoundAssignSubobjectHandler needs to 
> > > propagate this down from the operator expression.
> > It is guaranteed by the way AST is built, no?
> > 
> > As FP options may be changed only by pragmas and the pragmas can be 
> > specified only at file or block level, all sub-expression are evaluated at 
> > the same options.
> Yes, but you can't actually reliably recover those settings from E unless 
> you're sure E is one of a few kinds of expression.  The concern is that E 
> might end up being some closely-related expression that isn't actually the 
> expression that carries the current settings, and then we'll fall back on 
> using the global defaults.  It's much more correct-by-construction to pass 
> the settings down from the caller based on the caller's static knowledge of 
> which expression is under consideration, and I think you'll see that that's 
> pretty straightforward in practice.
This is a peculiarity of `CompoundAssignOperator`, which itself makes 
conversion, without `CastExpr`. I added `assert` to ensure that other nodes 
cannot appear here. Also some tests with conversion in `CompoundAssignOperator` 
were added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87822

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

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In D36836#2289639 , @lebedev.ri wrote:

> Rebased.
>
> There is a number of official open-source LGPL-3 implementations already:
>
> - https://github.com/SonarSource/SonarTS/pull/378
> - https://github.com/SonarSource/sonar-java/pull/1385
> - https://github.com/SonarSource/SonarJS/pull/449
> - https://github.com/SonarSource/sonar-php/pull/173
>
> There are other open-source LGPL-3 implementations already:
>
> - https://pypi.org/project/cognitive-complexity/ (MIT)
> - https://github.com/rossmacarthur/complexity (APACHE/MIT)
>
> There are other 3rd party implementations:
>
> - https://docs.codeclimate.com/docs/cognitive-complexity
>
> Quite honestly, i do not understand how did the license question arose.
> Would have it been fine if i based this on the open-source-licensed code?
> Would have it not been? Would same license question be raised?
> Somehow i don't think it would have been.
>
> Is this really just about `Copyright SonarSource S.A., 2018, Switzerland. All 
> content is copyright protected.` in 
> https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
> But that is only about the document, not the algorithm.
> But even if we enternain the idea that all of the implementations must bow to 
> that license,
> then surely this is not the first code in LLVM that is implicitly/explicitly 
> based on copyrighted doc.
>
> This is rather frustrating.

Given there are open source implementations, this is in my eyes completly 
acceptable to implement this in llvm.
The document is copyrighted, true. The algorithm itself is not, in european law 
it is even very hard to patent algorithms, software in general is not 
patentable.
The code is not copy&pasted, so there are no infringements on anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36836

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

[PATCH] D88204: [clangd] Drop path suffix mapping for std symbols

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, jfb, arphaman.
Herald added a project: clang.
kadircet requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

As mentioned in D88144 , this is only used as 
a fallback for symbols
missing in our stdsymbol mappings and preventing us from figuring out what's
broken.

After this patch we'll regress some cases, e.g. std::move, but those were
already broken for libc++ or C.

---

I am also happy with coming up with special casing sfor obvious regressions,
like std::move, before landing. The special casing I have in mind could either
be:

- having a more targeted version of the suffix mapping targeted at those 
symbols, so we can stop the bleeding as we figure out cases even if we can't 
invest in a more principled solution.
- just picking a single header in case of ambigious symbols, this might end up 
being annoying as clangd will insist on inserting the wrong header.
- disable include insertion completely for problematic symbols until we figure 
out a solution.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88204

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/index/CanonicalIncludes.h
  clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/FileIndexTests.cpp
===
--- clang-tools-extra/clangd/unittests/FileIndexTests.cpp
+++ clang-tools-extra/clangd/unittests/FileIndexTests.cpp
@@ -226,10 +226,11 @@
 
 TEST(FileIndexTest, HasSystemHeaderMappingsInPreamble) {
   TestTU TU;
-  TU.HeaderCode = "class Foo{};";
-  TU.HeaderFilename = "algorithm";
+  // std::max is mapped back to 
+  TU.HeaderCode = "namespace std { void max(); }";
+  TU.HeaderFilename = "ignored_header_name.h";
 
-  auto Symbols = runFuzzyFind(*TU.index(), "");
+  auto Symbols = runFuzzyFind(*TU.index(), "max");
   EXPECT_THAT(Symbols, ElementsAre(_));
   EXPECT_THAT(Symbols.begin()->IncludeHeaders.front().IncludeHeader,
   "");
Index: clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
===
--- clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
+++ clang-tools-extra/clangd/unittests/CanonicalIncludesTests.cpp
@@ -21,10 +21,6 @@
   CI.addSystemHeadersMapping(Language);
   // Usual standard library symbols are mapped correctly.
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "printf"));
-  // Suffix mapping isn't available for C, instead of mapping to ` we
-  // just leave the header as-is.
-  EXPECT_EQ("include/stdio.h",
-CI.mapHeader("include/stdio.h", "unknown_symbol"));
 }
 
 TEST(CanonicalIncludesTest, CXXStandardLibrary) {
@@ -37,14 +33,13 @@
   EXPECT_EQ("", CI.mapHeader("path/vector.h", "std::vector"));
   EXPECT_EQ("", CI.mapHeader("path/stdio.h", "std::printf"));
   // std::move is ambiguous, currently mapped only based on path
-  EXPECT_EQ("", CI.mapHeader("libstdc++/bits/move.h", "std::move"));
+  EXPECT_EQ("libstdc++/bits/move.h",
+CI.mapHeader("libstdc++/bits/move.h", "std::move"));
   EXPECT_EQ("path/utility.h", CI.mapHeader("path/utility.h", "std::move"));
   // Unknown std symbols aren't mapped.
   EXPECT_EQ("foo/bar.h", CI.mapHeader("foo/bar.h", "std::notathing"));
   // iosfwd declares some symbols it doesn't own.
   EXPECT_EQ("", CI.mapHeader("iosfwd", "std::ostream"));
-  // And (for now) we assume it owns the others.
-  EXPECT_EQ("", CI.mapHeader("iosfwd", "std::notwathing"));
 }
 
 TEST(CanonicalIncludesTest, PathMapping) {
@@ -77,8 +72,6 @@
 
   // We added a mapping from some/path to .
   ASSERT_EQ("", CI.mapHeader("some/path", ""));
-  // We should have a path from 'bits/stl_vector.h' to ''.
-  ASSERT_EQ("", CI.mapHeader("bits/stl_vector.h", ""));
   // We should also have a symbol mapping from 'std::map' to ''.
   ASSERT_EQ("", CI.mapHeader("some/header.h", "std::map"));
 
Index: clang-tools-extra/clangd/index/CanonicalIncludes.h
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.h
+++ clang-tools-extra/clangd/index/CanonicalIncludes.h
@@ -57,9 +57,6 @@
 private:
   /// A map from full include path to a canonical path.
   llvm::StringMap FullPathMapping;
-  /// A map from a suffix (one or components of a path) to a canonical path.
-  /// Used only for mapping standard headers.
-  const llvm::StringMap *StdSuffixHeaderMapping = nullptr;
   /// A map from fully qualified symbol names to header names.
   /// Used only for mapping standard symbols.
   const llvm::StringMap *StdSymbolMapping = nullptr;
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- cl

[PATCH] D77229: [Analyzer] Avoid handling of LazyCompundVals in IteratorModeling

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

baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/Iterator.cpp:330-336
+SVal getReturnIterator(const CallEvent &Call) {
+  Optional RetValUnderConstr = Call.getReturnValueUnderConstruction();
+  if (RetValUnderConstr.hasValue())
+return *RetValUnderConstr;
+
+  return Call.getReturnValue();
+}

NoQ wrote:
> baloghadamsoftware wrote:
> > baloghadamsoftware wrote:
> > > NoQ wrote:
> > > > baloghadamsoftware wrote:
> > > > > NoQ wrote:
> > > > > > NoQ wrote:
> > > > > > > NoQ wrote:
> > > > > > > > I still believe you have not addressed the problem while moving 
> > > > > > > > the functions from D81718 to this patch. The caller of this 
> > > > > > > > function has no way of knowing whether the return value is the 
> > > > > > > > prvalue of the iterator or the glvalue of the iterator.
> > > > > > > > 
> > > > > > > > Looks like most callers are safe because they expect the object 
> > > > > > > > of interest to also be already tracked. But it's quite possible 
> > > > > > > > that both are tracked, say:
> > > > > > > > 
> > > > > > > > ```lang=c++
> > > > > > > >   Container1 container1 = ...;
> > > > > > > >   Container2 container2 = { 
> > > > > > > > container1.begin() };
> > > > > > > >   container2.begin(); // ???
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > Suppose `Container1::iterator` is implemented as an object and 
> > > > > > > > `Container2::iterator` is implemented as a pointer. In this 
> > > > > > > > case `getIteratorPosition(getReturnIterator())` would yield the 
> > > > > > > > position of `container1.begin()` whereas the correct answer is 
> > > > > > > > the position of `container2.begin()`.
> > > > > > > > 
> > > > > > > > This problem may seem artificial but it is trivial to avoid if 
> > > > > > > > you simply stop defending your convoluted solution of looking 
> > > > > > > > at value classes instead of AST types.
> > > > > > > Ugh, the problem is much worse. D82185 is entirely broken for the 
> > > > > > > exact reason i described above and you only didn't notice it 
> > > > > > > because you wrote almost no tests.
> > > > > > > 
> > > > > > > Consider the test you've added in D82185:
> > > > > > > 
> > > > > > > ```lang=c++
> > > > > > > void begin_ptr_iterator(const cont_with_ptr_iterator &c) {
> > > > > > >   auto i = c.begin();
> > > > > > > 
> > > > > > >   clang_analyzer_eval(clang_analyzer_iterator_container(i) == 
> > > > > > > &c); // expected-warning{{TRUE}}
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > It breaks very easily if you modify it slightly:
> > > > > > > ```lang=c++
> > > > > > > void begin_ptr_iterator(const cont_with_ptr_iterator &c) {
> > > > > > >   auto i = c.begin();
> > > > > > >   ++i; // <==
> > > > > > > 
> > > > > > >   clang_analyzer_eval(clang_analyzer_iterator_container(i) == 
> > > > > > > &c); // Says FALSE!
> > > > > > > }
> > > > > > > ```
> > > > > > > The iterator obviously still points to the same container, so why 
> > > > > > > does the test fail? Because you're tracking the wrong iterator: 
> > > > > > > you treated your `&SymRegion{conj_$3}` as a glvalue whereas you 
> > > > > > > should have treated it as a prvalue. In other words, your checker 
> > > > > > > thinks that `&SymRegion{conj_$3}` is the location of an iterator 
> > > > > > > object rather than the iterator itself, and after you increment 
> > > > > > > the pointer it thinks that it's a completely unrelated iterator.
> > > > > > > 
> > > > > > > There's a separate concern about why does it say `FALSE` (should 
> > > > > > > be `UNKNOWN`) but you get the point.
> > > > > > The better way to test D82185 would be to make all existing tests 
> > > > > > with iterator objects pass with iterator pointers as well. Like, 
> > > > > > make existing container mocks use either iterator objects or 
> > > > > > iterator pointers depending on a macro and make two run-lines in 
> > > > > > each test file, one with `-D` and one without it. Most of the old 
> > > > > > tests should have worked out of the box if you did it right; the 
> > > > > > few that don't pass would be hidden under #ifdef for future 
> > > > > > investigation.
> > > > > Thank you for your review and especially for this tip! It is really a 
> > > > > good idea. I changed it now and it indeed shows the problem you 
> > > > > reported. It seems that my checker mixes up the region of the 
> > > > > pointer-typed variable (`&i` and `&j`) with the region they point to 
> > > > > (`&SymRegion{reg_$1 & 
> > > > > v>}._start>}` for `i` before the increment and 
> > > > > `&Element{SymRegion{reg_$1 > > > > std::vector & v>}._start>},1 S64b,int}` for both `i` and `j` 
> > > > > after the increment).
> > > > > 
> > > > > What I fail to see and I am asking you help in it is that the 
> > > > > relation between this problem and the `getReturnIterator()` function. 
> > > > > This function retrieves the object from the construction co

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D87774#2287927 , @sameeranjoshi 
wrote:

> Thanks for working on it.
> Few comments inline:
>
> 1. For an out-of-tree build, I see `check-flang` target failing with
>
>   /unittests/Frontend/CompilerInstanceTest.cpp:17:10: fatal error: 
> filesystem: No such file or directory
>#include 
> ^~~~
>
> I used gcc/g++ 7.5 version.
> I haven't checked in-tree still, and others/bots might have checked it.

I haven't been able to reproduce it, but note that `filesystem` is a C++17 
header. AFAIK, in GCC-7 you have to use `` instead of 
``. This suggests README.md should be updated: 
https://github.com/llvm/llvm-project/blob/master/flang/README.md#supported-c-compilers
 (we probably need an RFC for this). Since this `#include` was introduced 
elsewhere, I suggest that we move this discussion either to Slack or flang-dev.

> 2. Either the documentation comments are wrong or code.
>
> `README` mentions `DBUILD_FLANG_NEW_DRIVER` where as cmake ignores the flag 
> for me.
> Whereas, `CMakeLists.txt` mentions `FLANG_BUILD_NEW_DRIVER`.

Sorry, that's an unfortunate typo. Fixed here: 
https://github.com/llvm/llvm-project/commit/27da2875070ac00f6cd9f8040c6f994aca915406


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

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

[PATCH] D88164: [clang][Sema] Use enumerator instead of hard-coded constant

[Mikhail Maltsev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8cc842a95072: [clang][Sema] Use enumerator instead of 
hard-coded constant (authored by miyuki).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88164

Files:
  clang/lib/Sema/SemaDeclAttr.cpp


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5868,7 +5868,7 @@
 
   if (!F->hasWrittenPrototype()) {
 Diag(Loc, diag::warn_attribute_wrong_decl_type) << AL
-<< /* non-K&R-style functions */12;
+<< ExpectedFunctionWithProtoType;
 return false;
   }
 }


Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5868,7 +5868,7 @@
 
   if (!F->hasWrittenPrototype()) {
 Diag(Loc, diag::warn_attribute_wrong_decl_type) << AL
-<< /* non-K&R-style functions */12;
+<< ExpectedFunctionWithProtoType;
 return false;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 8cc842a - [clang][Sema] Use enumerator instead of hard-coded constant

[Mikhail Maltsev via cfe-commits]

Author: Mikhail Maltsev
Date: 2020-09-24T10:24:22+01:00
New Revision: 8cc842a95072aaa87b5067a12aa9ef5b3ac8e592

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

LOG: [clang][Sema] Use enumerator instead of hard-coded constant

Sema::DiagnoseSwiftName uses the constant 12 instead of the
corresponding enumerator ExpectedFunctionWithProtoType. This is
fragile and will fail if a new value gets added in the middle of the
enum.

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclAttr.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 3babbe05ca8f..d15ef232a5fb 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -5868,7 +5868,7 @@ bool Sema::DiagnoseSwiftName(Decl *D, StringRef Name, 
SourceLocation Loc,
 
   if (!F->hasWrittenPrototype()) {
 Diag(Loc, diag::warn_attribute_wrong_decl_type) << AL
-<< /* non-K&R-style functions */12;
+<< ExpectedFunctionWithProtoType;
 return false;
   }
 }



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

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added inline comments.



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:24
+
+TextDiagnosticPrinter::TextDiagnosticPrinter(
+raw_ostream &os, clang::DiagnosticOptions *diags)

awarzynski wrote:
> sameeranjoshi wrote:
> > A silly question from what I see usually in Flang coding style.
> > Why isn't it defined in header file?
> No such thing as silly questions! :)
> 
> AFAIK, there are no rules in the coding guidelines with regard to
> * where things should be defined, and 
> * where things should be declared. 
> I use this rather generic rule of thumb: declare in headers, define in source 
> files. In this particular case - I followed what was already in Clang. It 
> made sense to me.
> 
> Do you think that it would better to define this in a header file?
I think in flang the style is to declare the class in the source file if it is 
only used in that file. If it is used elsewhere then put it in the header. If 
it is used in another directory then move the header to include directory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

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

[clang-tools-extra] 00e05b1 - [clangd] Reorder a little bit of init code. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2020-09-24T11:57:31+02:00
New Revision: 00e05b12c76c396688cd8d4caac09a2e96851fd9

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

LOG: [clangd] Reorder a little bit of init code. NFC

This makes it possible to do something else (run checks) instead of
starting the server, with all config applied.

Added: 


Modified: 
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index cf74ded93632..9660f1bd76f7 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -761,35 +761,6 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
 Opts.ConfigProvider = Config.get();
   }
 
-  // Initialize and run ClangdLSPServer.
-  // Change stdin to binary to not lose \r\n on windows.
-  llvm::sys::ChangeStdinToBinary();
-
-  std::unique_ptr TransportLayer;
-  if (getenv("CLANGD_AS_XPC_SERVICE")) {
-#if CLANGD_BUILD_XPC
-log("Starting LSP over XPC service");
-TransportLayer = newXPCTransport();
-#else
-llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
-return (int)ErrorResultCode::CantRunAsXPCService;
-#endif
-  } else {
-log("Starting LSP over stdin/stdout");
-TransportLayer = newJSONTransport(
-stdin, llvm::outs(),
-InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
-PrettyPrint, InputStyle);
-  }
-  if (!PathMappingsArg.empty()) {
-auto Mappings = parsePathMappings(PathMappingsArg);
-if (!Mappings) {
-  elog("Invalid -path-mappings: {0}", Mappings.takeError());
-  return 1;
-}
-TransportLayer = createPathMappingTransport(std::move(TransportLayer),
-std::move(*Mappings));
-  }
   // Create an empty clang-tidy option.
   std::mutex ClangTidyOptMu;
   std::unique_ptr
@@ -816,9 +787,9 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
   return Opts;
 };
   }
+  Opts.AsyncPreambleBuilds = AsyncPreamble;
   Opts.SuggestMissingIncludes = SuggestMissingIncludes;
   Opts.QueryDriverGlobs = std::move(QueryDriverGlobs);
-
   Opts.TweakFilter = [&](const Tweak &T) {
 if (T.hidden() && !HiddenFeatures)
   return false;
@@ -834,7 +805,34 @@ clangd accepts flags on the commandline, and in the 
CLANGD_FLAGS environment var
   // Shall we allow to customize the file limit?
   RenameOpts.AllowCrossFile = CrossFileRename;
 
-  Opts.AsyncPreambleBuilds = AsyncPreamble;
+  // Initialize and run ClangdLSPServer.
+  // Change stdin to binary to not lose \r\n on windows.
+  llvm::sys::ChangeStdinToBinary();
+  std::unique_ptr TransportLayer;
+  if (getenv("CLANGD_AS_XPC_SERVICE")) {
+#if CLANGD_BUILD_XPC
+log("Starting LSP over XPC service");
+TransportLayer = newXPCTransport();
+#else
+llvm::errs() << "This clangd binary wasn't built with XPC support.\n";
+return (int)ErrorResultCode::CantRunAsXPCService;
+#endif
+  } else {
+log("Starting LSP over stdin/stdout");
+TransportLayer = newJSONTransport(
+stdin, llvm::outs(),
+InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
+PrettyPrint, InputStyle);
+  }
+  if (!PathMappingsArg.empty()) {
+auto Mappings = parsePathMappings(PathMappingsArg);
+if (!Mappings) {
+  elog("Invalid -path-mappings: {0}", Mappings.takeError());
+  return 1;
+}
+TransportLayer = createPathMappingTransport(std::move(TransportLayer),
+std::move(*Mappings));
+  }
 
   ClangdLSPServer LSPServer(
   *TransportLayer, TFS, CCOpts, RenameOpts, CompileCommandsDirPath,



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

[PATCH] D75574: RFC: Implement objc_direct_protocol attribute to remove protocol metadata

[David Chisnall via Phabricator via cfe-commits]

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

Looks good to me, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75574

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

[clang] 4e53490 - [NFC][Docs] fix clang-docs compilation

[Jonas Toth via cfe-commits]

Author: Jonas Toth
Date: 2020-09-24T13:13:38+02:00
New Revision: 4e534900476d2a5c620e74ddb9c9e7d321e6d443

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

LOG: [NFC][Docs] fix clang-docs compilation

Added: 


Modified: 
clang/docs/analyzer/checkers.rst
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/docs/analyzer/checkers.rst 
b/clang/docs/analyzer/checkers.rst
index dbbd49085dac..b47be97eef96 100644
--- a/clang/docs/analyzer/checkers.rst
+++ b/clang/docs/analyzer/checkers.rst
@@ -2545,6 +2545,7 @@ The goal of this rule is to make sure that any uncounted 
local variable is backe
 These are examples of cases that we consider safe:
 
   .. code-block:: cpp
+
 void foo1() {
   RefPtr counted;
   // The scope of uncounted is EMBEDDED in the scope of counted.

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 3e27924589e4..753c4dd235fa 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3509,7 +3509,7 @@ def SwiftBridgeDocs : Documentation {
 The ``swift_bridge`` attribute indicates that the declaration to which the
 attribute appertains is bridged to the named Swift type.
 
-  .. code-block:: c
+  .. code-block:: objc
 
 __attribute__((__objc_root__))
 @interface Base
@@ -3535,7 +3535,7 @@ the attribute appertains is imported into Swift, it 
should refer to the bridged
 Swift type (e.g. Swift's ``String``) rather than the Objective-C type as 
written
 (e.g. ``NSString``).
 
-  .. code-block:: c
+  .. code-block:: objc
 
 @interface NSString;
 typedef NSString *AliasedString __attribute__((__swift_bridged_typedef__));
@@ -3572,28 +3572,28 @@ the imported API. When calling the API, Swift will 
always pass a valid address
 initialized to a null pointer.
 
 * ``swift_error(none)`` means that the function should not be imported as
-throwing. The error parameter and result type will be imported normally.
+  throwing. The error parameter and result type will be imported normally.
 
 * ``swift_error(null_result)`` means that calls to the function should be
-considered to have thrown if they return a null value. The return type must be
-a pointer type, and it will be imported into Swift with a non-optional type.
-This is the default error convention for Objective-C methods that return
-pointers.
+  considered to have thrown if they return a null value. The return type must 
be
+  a pointer type, and it will be imported into Swift with a non-optional type.
+  This is the default error convention for Objective-C methods that return
+  pointers.
 
 * ``swift_error(zero_result)`` means that calls to the function should be
-considered to have thrown if they return a zero result. The return type must be
-an integral type. If the return type would have been imported as ``Bool``, it
-is instead imported as ``Void``. This is the default error convention for
-Objective-C methods that return a type that would be imported as ``Bool``.
+  considered to have thrown if they return a zero result. The return type must 
be
+  an integral type. If the return type would have been imported as ``Bool``, it
+  is instead imported as ``Void``. This is the default error convention for
+  Objective-C methods that return a type that would be imported as ``Bool``.
 
 * ``swift_error(nonzero_result)`` means that calls to the function should be
-considered to have thrown if they return a non-zero result. The return type 
must
-be an integral type. If the return type would have been imported as ``Bool``,
-it is instead imported as ``Void``.
+  considered to have thrown if they return a non-zero result. The return type 
must
+  be an integral type. If the return type would have been imported as ``Bool``,
+  it is instead imported as ``Void``.
 
 * ``swift_error(nonnull_error)`` means that calls to the function should be
-considered to have thrown if they leave a non-null error in the error 
parameter.
-The return type is left unmodified.
+  considered to have thrown if they leave a non-null error in the error 
parameter.
+  The return type is left unmodified.
 
   }];
 }
@@ -3611,7 +3611,7 @@ variable, or type. When renaming a function, the name may 
be a compound Swift
 name.  For a type, enum constant, property, or variable declaration, the name
 must be a simple or qualified identifier.
 
-  .. code-block:: c
+  .. code-block:: objc
 
 @interface URL
 - (void) initWithString:(NSString *)s 
__attribute__((__swift_name__("URL.init(_:)")))



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

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.

This clearly changes behaviour and should thereby not have the `[NFC]` tag.


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

https://reviews.llvm.org/D88105

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

[PATCH] D88216: [Analyzer] Fix handling of pointer-based iterator increments and decrements

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

baloghadamsoftware created this revision.
baloghadamsoftware added a reviewer: NoQ.
baloghadamsoftware added a project: clang.
Herald added subscribers: ASDenysPetrov, martong, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, 
szepet, xazax.hun, whisperity.
Herald added a reviewer: Szelethus.
baloghadamsoftware requested review of this revision.

The handling of iterators implemented as pointers were incorrectly handled in 
the increment and decrement operations because these operations did not handle 
the lvalue of the iterator correctly. This patch fixes that issue and extends 
the test coverage for such kind of iterators by adding an option to the 
simulated `std::vector` to implement its iterator as a pointer.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88216

Files:
  clang/lib/StaticAnalyzer/Checkers/IteratorModeling.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/diagnostics/explicit-suppression.cpp
  clang/test/Analysis/invalidated-iterator.cpp
  clang/test/Analysis/iterator-modeling.cpp
  clang/test/Analysis/iterator-range.cpp
  clang/test/Analysis/mismatched-iterator.cpp
  clang/test/Analysis/smart-ptr-text-output.cpp

Index: clang/test/Analysis/smart-ptr-text-output.cpp
===
--- clang/test/Analysis/smart-ptr-text-output.cpp
+++ clang/test/Analysis/smart-ptr-text-output.cpp
@@ -80,7 +80,7 @@
 void derefOnStdSwappedNullPtr() {
   std::unique_ptr P; // expected-note {{Default constructed smart pointer 'P' is null}}
   std::unique_ptr PNull; // expected-note {{Default constructed smart pointer 'PNull' is null}}
-  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:979 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
+  std::swap(P, PNull); // expected-note@Inputs/system-header-simulator-cxx.h:993 {{Swapped null smart pointer 'PNull' with smart pointer 'P'}}
   // expected-note@-1 {{Calling 'swap'}}
   // expected-note@-2 {{Returning from 'swap'}}
   P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
Index: clang/test/Analysis/mismatched-iterator.cpp
===
--- clang/test/Analysis/mismatched-iterator.cpp
+++ clang/test/Analysis/mismatched-iterator.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false %s -verify
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false -DSTD_VECTOR_ITERATOR_PTR %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.MismatchedIterator -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 -DSTD_VECTOR_ITERATOR_PTR %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
Index: clang/test/Analysis/iterator-range.cpp
===
--- clang/test/Analysis/iterator-range.cpp
+++ clang/test/Analysis/iterator-range.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false -analyzer-output=text %s -verify
 // RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 -analyzer-output=text %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=false -DSTD_VECTOR_ITERATOR_PTR -analyzer-output=text %s -verify
+// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,cplusplus,alpha.cplusplus.IteratorRange -analyzer-config aggressive-binary-operation-simplification=true -analyzer-config c++-container-inlining=true -DINLINE=1 -DSTD_VECTOR_ITERATOR_PTR -analyzer-output=text %s -verify
 
 #include "Inputs/system-header-simulator-cxx.h"
 
@@ -9,30 +11,30 @@
 
 void deref_begin(const std::vector &V) {
   auto i = V.begin();
-  *i; // no-warning
+  int n = *i; // no-warning
 }
 
 void deref_begind_begin(const std::vector &V) {
-  auto i = 

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

@chill ping.


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

https://reviews.llvm.org/D75044

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

[PATCH] D84962: [PowerPC] Correct cpsgn's behaviour on PowerPC to match that of the ABI

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

This is not what we want. The builtin behaves correctly. It is equivalent to 
the generic `__builtin_copysign` and it would be very surprising to a user if 
it reverses the operands depending on which version you use. What is 
implemented incorrectly is `vec_cpsgn` so that's what should change.


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

https://reviews.llvm.org/D84962

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

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

@sameeranjoshi Regarding ``, please see 
https://reviews.llvm.org/D88219. Sadly we missed that when adding that test, 
sorry!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

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

[PATCH] D54943: WIP [clang-tidy] implement const-transformation for cppcoreguidelines-const-correctness

[Florin Iucha via Phabricator via cfe-commits]

0x8000- added a comment.

In D54943#2291969 , @JonasToth wrote:

> @AlexanderLanin @0x8000- i created the branch `release-11-const` 
> (https://github.com/JonasToth/llvm-project/tree/release-11-const) in my fork.
>
> This branch is based on 11-rc3 and cherry picks the commits that correspond 
> to this revision.
> I hope this is of any use for you! I will probably forget to update this 
> branch all the time, but if I feel like there has been actual progress made, 
> i will update!

Thank you Jonas - will add this to our CI tool for the new project and try to 
find some time to run it on the legacy project.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D54943

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

[PATCH] D87774: [flang] Introduce DiagnosticConsumer classes in libflangFrontend

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 294023.
awarzynski added a comment.

Rebase on top of master


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87774

Files:
  clang/include/clang/Driver/Options.td
  flang/include/flang/Frontend/CompilerInvocation.h
  flang/include/flang/Frontend/TextDiagnostic.h
  flang/include/flang/Frontend/TextDiagnosticBuffer.h
  flang/include/flang/Frontend/TextDiagnosticPrinter.h
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/CompilerInstance.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/TextDiagnostic.cpp
  flang/lib/Frontend/TextDiagnosticBuffer.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/test/Flang-Driver/driver-error-cc1.c
  flang/test/Flang-Driver/driver-error-cc1.cpp
  flang/test/Flang-Driver/driver-help.f90
  flang/test/Flang-Driver/driver-version.f90
  flang/test/Flang-Driver/missing-input.f90
  flang/tools/flang-driver/driver.cpp
  flang/tools/flang-driver/fc1_main.cpp
  flang/unittests/Frontend/CompilerInstanceTest.cpp

Index: flang/unittests/Frontend/CompilerInstanceTest.cpp
===
--- flang/unittests/Frontend/CompilerInstanceTest.cpp
+++ flang/unittests/Frontend/CompilerInstanceTest.cpp
@@ -7,8 +7,8 @@
 //===--===//
 
 #include "flang/Frontend/CompilerInstance.h"
+#include "flang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Basic/DiagnosticOptions.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 
 #include "gtest/gtest.h"
 
@@ -21,7 +21,7 @@
   // 1. Set-up a basic DiagnosticConsumer
   std::string diagnosticOutput;
   llvm::raw_string_ostream diagnosticsOS(diagnosticOutput);
-  auto diagPrinter = std::make_unique(
+  auto diagPrinter = std::make_unique(
   diagnosticsOS, new clang::DiagnosticOptions());
 
   // 2. Create a CompilerInstance (to manage a DiagnosticEngine)
Index: flang/tools/flang-driver/fc1_main.cpp
===
--- flang/tools/flang-driver/fc1_main.cpp
+++ flang/tools/flang-driver/fc1_main.cpp
@@ -14,9 +14,9 @@
 
 #include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/CompilerInvocation.h"
+#include "flang/Frontend/TextDiagnosticBuffer.h"
 #include "flang/FrontendTool/Utils.h"
 #include "clang/Driver/DriverDiagnostic.h"
-#include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptTable.h"
@@ -34,18 +34,22 @@
   if (!flang->HasDiagnostics())
 return 1;
 
+  // We will buffer diagnostics from argument parsing so that we can output
+  // them using a well formed diagnostic object.
+  TextDiagnosticBuffer *diagsBuffer = new TextDiagnosticBuffer;
+
   // Create CompilerInvocation - use a dedicated instance of DiagnosticsEngine
   // for parsing the arguments
   llvm::IntrusiveRefCntPtr diagID(
   new clang::DiagnosticIDs());
   llvm::IntrusiveRefCntPtr diagOpts =
   new clang::DiagnosticOptions();
-  clang::TextDiagnosticBuffer *diagsBuffer = new clang::TextDiagnosticBuffer;
   clang::DiagnosticsEngine diags(diagID, &*diagOpts, diagsBuffer);
   bool success =
   CompilerInvocation::CreateFromArgs(flang->GetInvocation(), argv, diags);
 
   diagsBuffer->FlushDiagnostics(flang->getDiagnostics());
+
   if (!success)
 return 1;
 
Index: flang/tools/flang-driver/driver.cpp
===
--- flang/tools/flang-driver/driver.cpp
+++ flang/tools/flang-driver/driver.cpp
@@ -11,17 +11,21 @@
 //
 //===--===//
 #include "clang/Driver/Driver.h"
+#include "flang/Frontend/CompilerInvocation.h"
+#include "flang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "clang/Driver/Compilation.h"
-#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/VirtualFileSystem.h"
+#include 
+
+using llvm::StringRef;
 
 // main frontend method. Lives inside fc1_main.cpp
 extern int fc1_main(llvm::ArrayRef argv, const char *argv0);
@@ -37,6 +41,17 @@
 static clang::DiagnosticOptions *CreateAndPopulateDiagOpts(
 llvm::ArrayRef argv) {
   auto *diagOpts = new clang::DiagnosticOptions;
+
+  // Ignore missingArgCount and the return value of ParseDiagnosticArgs.
+  // Any errors that would be diagnosed here will also be diagnosed later,
+  // when the DiagnosticsEngine actually exists.
+  unsigned missingArgIndex, missingArgCount;
+  llvm::opt::InputArgList args = clang::driver::getDriverOptTable().ParseArgs(
+  argv.slice

[PATCH] D71199: [clang-tidy] New check cppcoreguidelines-prefer-member-initializer

[Lukas Rechberger via Phabricator via cfe-commits]

Rechi added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp:184
+bool AddComma = false;
+if (!Ctor->getNumCtorInitializers() && FirstToCtorInits) {
+  SourceLocation BodyPos = Ctor->getBody()->getBeginLoc();

The following example generates invalid fixes, if 
modernize-use-default-member-init check isn't enabled, because 
`Ctor->getNumCtorInitializers()` returns 1.

```lang=cpp
class Example
{
public:
  Example() { a = 0; };
  int a;
  std::string string;
};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71199

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

[PATCH] D88220: [C++20] Implement more implicit moves for return statements(Part of P1825R0)

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
nullptr.cpp requested review of this revision.

In P1825R0, the first parameter of overload resolution selected function does
not need to be an rvalue reference to the returned object's type.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88220

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaStmt.cpp
  clang/test/SemaCXX/implicit-move.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1201,6 +1201,11 @@
   https://wg21.link/p0593r6";>P0593R6 (DR)
   Clang 11
 
+
+More implicit moves
+https://wg21.link/p1825r0";>P1825R0 (DR)
+Clang 12 (partial)
+
 
 
 
Index: clang/test/SemaCXX/implicit-move.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/implicit-move.cpp
@@ -0,0 +1,80 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -fcxx-exceptions -DCXX20 -verify %s
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -fcxx-exceptions -DCXX17 -verify %s
+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -fcxx-exceptions -DCXX14 -verify %s
+
+class Error;
+
+class NeedRvalueRef {
+public:
+  NeedRvalueRef() {}
+  ~NeedRvalueRef() {}
+  NeedRvalueRef(const NeedRvalueRef &);
+  NeedRvalueRef(NeedRvalueRef &&);
+  NeedRvalueRef(Error &&);
+};
+
+class NoNeedRvalueRef {
+public:
+  NoNeedRvalueRef() {}
+  ~NoNeedRvalueRef() {}
+  NoNeedRvalueRef(const NoNeedRvalueRef &);
+  NoNeedRvalueRef(NoNeedRvalueRef &&);
+  NoNeedRvalueRef(Error);
+};
+
+#ifdef CXX20
+// expected-no-diagnostics
+class Error {
+public:
+  Error() {}
+  ~Error() {}
+  Error(Error &&);
+
+private:
+  Error(const Error &);
+};
+
+NoNeedRvalueRef test() {
+  Error Err;
+  return Err;
+}
+#endif
+
+#ifdef CXX17
+class Error {
+public:
+  Error() {}
+  ~Error() {}
+  Error(Error &&);
+
+private:
+  Error(const Error &); // expected-note {{declared private here}}
+};
+
+NoNeedRvalueRef test2() {
+  Error Err;
+  return Err; // expected-error {{calling a private constructor of class 'Error'}}
+}
+#endif
+
+#ifdef CXX14
+class Error {
+public:
+  Error() {}
+  ~Error() {}
+  Error(Error &&);
+
+private:
+  Error(const Error &); // expected-note {{declared private here}}
+};
+
+NoNeedRvalueRef test2() {
+  Error Err;
+  return Err; // expected-error {{calling a private constructor of class 'Error'}}
+}
+#endif
+
+NeedRvalueRef test_all_ok() {
+  Error Err;
+  return Err;
+}
\ No newline at end of file
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -3056,12 +3056,20 @@
   // variable will no longer be used.
   if (VD->hasAttr()) return false;
 
+  // ...non-volatile...
+  if (VD->getType().isVolatileQualified())
+return false;
+
+  // C++20 [class.copy.elision]p3:
+  // ...rvalue reference to a non-volatile...
+  if (VD->getType()->isRValueReferenceType() &&
+  (!(CESK & CES_AllowRValueReferenceType) ||
+   VD->getType().getNonReferenceType().isVolatileQualified()))
+return false;
+
   if (CESK & CES_AllowDifferentTypes)
 return true;
 
-  // ...non-volatile...
-  if (VD->getType().isVolatileQualified()) return false;
-
   // Variables with higher required alignment than their type's ABI
   // alignment cannot use NRVO.
   if (!VD->getType()->isDependentType() && VD->hasAttr() &&
@@ -3074,13 +3082,13 @@
 /// Try to perform the initialization of a potentially-movable value,
 /// which is the operand to a return or throw statement.
 ///
-/// This routine implements C++14 [class.copy]p32, which attempts to treat
-/// returned lvalues as rvalues in certain cases (to prefer move construction),
-/// then falls back to treating them as lvalues if that failed.
+/// This routine implements C++20 [class.copy.elision]p3:, which attempts to
+/// treat returned lvalues as rvalues in certain cases (to prefer move
+/// construction), then falls back to treating them as lvalues if that failed.
 ///
-/// \param ConvertingConstructorsOnly If true, follow [class.copy]p32 and reject
-/// resolutions that find non-constructors, such as derived-to-base conversions
-/// or `operator T()&&` member functions. If false, do consider such
+/// \param ConvertingConstructorsOnly If true, follow [class.copy.elision]p3 and
+/// reject resolutions that find non-constructors, such as derived-to-base
+/// conversions or `operator T()&&` member functions. If false, do consider such
 /// conversion sequences.
 ///
 /// \param Res We will fill this in if move-initialization was possible.
@@ -3114,10 +3122,14 @@
 
 FunctionDecl *FD = Step.Function.Function;
 if (ConvertingConstructorsOnly) {
-  if (isa(FD)) {
+  if (!isa(FD)) {
+continue;
+  }
+  if (!S.getLan

[clang] f5314d1 - [Support] On Unix, let the CrashRecoveryContext return the signal code

[Alexandre Ganea via cfe-commits]

Author: Alexandre Ganea
Date: 2020-09-24T08:21:43-04:00
New Revision: f5314d15af4f4514103ea12c74cb208538b8bef5

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

LOG: [Support] On Unix, let the CrashRecoveryContext return the signal code

Before this patch, the CrashRecoveryContext was returning -2 upon a signal, 
like ExecuteAndWait does. This didn't match the behavior on Windows, where the 
the exception code was returned.

We now return the signal's code, which optionally allows for re-throwing the 
signal later. Doing so requires all custom handlers to be removed first, 
through llvm::sys::unregisterHandlers() which we made a public API.

This is part of https://reviews.llvm.org/D70378

Added: 


Modified: 
clang/tools/driver/driver.cpp
llvm/include/llvm/Support/Signals.h
llvm/lib/Support/CrashRecoveryContext.cpp
llvm/lib/Support/Unix/Signals.inc
llvm/lib/Support/Windows/Signals.inc
llvm/unittests/Support/CrashRecoveryTest.cpp

Removed: 




diff  --git a/clang/tools/driver/driver.cpp b/clang/tools/driver/driver.cpp
index f24fd61e61a5..f67af6790fff 100644
--- a/clang/tools/driver/driver.cpp
+++ b/clang/tools/driver/driver.cpp
@@ -531,6 +531,13 @@ int main(int argc_, const char **argv_) {
   IsCrash = CommandRes < 0 || CommandRes == 70;
 #ifdef _WIN32
   IsCrash |= CommandRes == 3;
+#endif
+#if LLVM_ON_UNIX
+  // When running in integrated-cc1 mode, the CrashRecoveryContext returns
+  // the same codes as if the program crashed. See section "Exit Status for
+  // Commands":
+  // 
https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+  IsCrash |= CommandRes > 128;
 #endif
   if (IsCrash) {
 TheDriver.generateCompilationDiagnostics(*C, *FailingCommand);

diff  --git a/llvm/include/llvm/Support/Signals.h 
b/llvm/include/llvm/Support/Signals.h
index c5b94f5ac776..44f5a750ff5c 100644
--- a/llvm/include/llvm/Support/Signals.h
+++ b/llvm/include/llvm/Support/Signals.h
@@ -117,6 +117,8 @@ namespace sys {
   /// Context is a system-specific failure context: it is the signal type on
   /// Unix; the ExceptionContext on Windows.
   void CleanupOnSignal(uintptr_t Context);
+
+  void unregisterHandlers();
 } // End sys namespace
 } // End llvm namespace
 

diff  --git a/llvm/lib/Support/CrashRecoveryContext.cpp 
b/llvm/lib/Support/CrashRecoveryContext.cpp
index 2a41754c7786..7609f04cf68c 100644
--- a/llvm/lib/Support/CrashRecoveryContext.cpp
+++ b/llvm/lib/Support/CrashRecoveryContext.cpp
@@ -375,9 +375,10 @@ static void CrashRecoverySignalHandler(int Signal) {
   sigaddset(&SigMask, Signal);
   sigprocmask(SIG_UNBLOCK, &SigMask, nullptr);
 
-  // As per convention, -2 indicates a crash or timeout as opposed to failure 
to
-  // execute (see llvm/include/llvm/Support/Program.h)
-  int RetCode = -2;
+  // Return the same error code as if the program crashed, as mentioned in the
+  // section "Exit Status for Commands":
+  // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+  int RetCode = 128 + Signal;
 
   // Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp)
   if (Signal == SIGPIPE)

diff  --git a/llvm/lib/Support/Unix/Signals.inc 
b/llvm/lib/Support/Unix/Signals.inc
index 50b2ad4b5772..03181a713257 100644
--- a/llvm/lib/Support/Unix/Signals.inc
+++ b/llvm/lib/Support/Unix/Signals.inc
@@ -331,7 +331,7 @@ static void RegisterHandlers() { // Not signal-safe.
 registerHandler(S, SignalKind::IsInfo);
 }
 
-static void UnregisterHandlers() {
+void sys::unregisterHandlers() {
   // Restore all of the signal handlers to how they were before we showed up.
   for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
 sigaction(RegisteredSignalInfo[i].SigNo,
@@ -367,7 +367,7 @@ static RETSIGTYPE SignalHandler(int Sig) {
   // crashes when we return and the signal reissues.  This also ensures that if
   // we crash in our signal handler that the program will terminate immediately
   // instead of recursing in the signal handler.
-  UnregisterHandlers();
+  sys::unregisterHandlers();
 
   // Unmask all potentially blocked kill signals.
   sigset_t SigMask;

diff  --git a/llvm/lib/Support/Windows/Signals.inc 
b/llvm/lib/Support/Windows/Signals.inc
index 71dc6324e99f..3758582b35f7 100644
--- a/llvm/lib/Support/Windows/Signals.inc
+++ b/llvm/lib/Support/Windows/Signals.inc
@@ -869,3 +869,5 @@ static BOOL WINAPI LLVMConsoleCtrlHandler(DWORD dwCtrlType) 
{
  #pragma GCC diagnostic warning "-Wformat"
  #pragma GCC diagnostic warning "-Wformat-extra-args"
 #endif
+
+void sys::unregisterHandlers() {}

diff  --git a/llvm/unittests/Support/CrashRecoveryTest.cpp 
b/llvm/unittests/Support/CrashRecoveryTest.cpp
index a8040dc0110e..fc65bf6329b3 100644
--- a/llvm/unit

[PATCH] D88222: [AST] Use data-recursion when building ParentMap, avoid stack overflow.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
sammccall requested review of this revision.

The following crashes on my system before this patch, but not after:

  void foo(int i) {
switch (i) {
  case 1:
  case 2:
  ... 10 cases ...
;
}
  }
  
  clang-query -c="match stmt(hasAncestor(stmt()))" deep.c

I'm not sure it's actually a sane testcase to run though, it's pretty slow :-)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88222

Files:
  clang/lib/AST/ParentMapContext.cpp

Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -227,51 +227,57 @@
 
   bool shouldVisitImplicitCode() const { return true; }
 
+  template 
+  void MarkChild(MapNodeTy MapNode, MapTy *Parents) {
+if (ParentStack.empty())
+  return;
+
+// FIXME: Currently we add the same parent multiple times, but only
+// when no memoization data is available for the type.
+// For example when we visit all subexpressions of template
+// instantiations; this is suboptimal, but benign: the only way to
+// visit those is with hasAncestor / hasParent, and those do not create
+// new matches.
+// The plan is to enable DynTypedNode to be storable in a map or hash
+// map. The main problem there is to implement hash functions /
+// comparison operators for all types that DynTypedNode supports that
+// do not have pointer identity.
+auto &NodeOrVector = (*Parents)[MapNode];
+if (NodeOrVector.isNull()) {
+  if (const auto *D = ParentStack.back().get())
+NodeOrVector = D;
+  else if (const auto *S = ParentStack.back().get())
+NodeOrVector = S;
+  else
+NodeOrVector = new DynTypedNode(ParentStack.back());
+} else {
+  if (!NodeOrVector.template is()) {
+auto *Vector = new ParentVector(
+1, getSingleDynTypedNodeFromParentMap(NodeOrVector));
+delete NodeOrVector.template dyn_cast();
+NodeOrVector = Vector;
+  }
+
+  auto *Vector = NodeOrVector.template get();
+  // Skip duplicates for types that have memoization data.
+  // We must check that the type has memoization data before calling
+  // std::find() because DynTypedNode::operator== can't compare all
+  // types.
+  bool Found = ParentStack.back().getMemoizationData() &&
+   std::find(Vector->begin(), Vector->end(),
+ ParentStack.back()) != Vector->end();
+  if (!Found)
+Vector->push_back(ParentStack.back());
+}
+  }
+
   template 
   bool TraverseNode(T Node, MapNodeTy MapNode, BaseTraverseFn BaseTraverse,
 MapTy *Parents) {
 if (!Node)
   return true;
-if (ParentStack.size() > 0) {
-  // FIXME: Currently we add the same parent multiple times, but only
-  // when no memoization data is available for the type.
-  // For example when we visit all subexpressions of template
-  // instantiations; this is suboptimal, but benign: the only way to
-  // visit those is with hasAncestor / hasParent, and those do not create
-  // new matches.
-  // The plan is to enable DynTypedNode to be storable in a map or hash
-  // map. The main problem there is to implement hash functions /
-  // comparison operators for all types that DynTypedNode supports that
-  // do not have pointer identity.
-  auto &NodeOrVector = (*Parents)[MapNode];
-  if (NodeOrVector.isNull()) {
-if (const auto *D = ParentStack.back().get())
-  NodeOrVector = D;
-else if (const auto *S = ParentStack.back().get())
-  NodeOrVector = S;
-else
-  NodeOrVector = new DynTypedNode(ParentStack.back());
-  } else {
-if (!NodeOrVector.template is()) {
-  auto *Vector = new ParentVector(
-  1, getSingleDynTypedNodeFromParentMap(NodeOrVector));
-  delete NodeOrVector.template dyn_cast();
-  NodeOrVector = Vector;
-}
-
-auto *Vector = NodeOrVector.template get();
-// Skip duplicates for types that have memoization data.
-// We must check that the type has memoization data before calling
-// std::find() because DynTypedNode::operator== can't compare all
-// types.
-bool Found = ParentStack.back().getMemoizationData() &&
- std::find(Vector->begin(), Vector->end(),
-   ParentStack.back()) != Vector->end();
-if (!Found)
-  Vector->push_back(ParentStack.back());
-  }
-}
+MarkChild(MapNode, Parents);
 ParentStack.push_back(createDynTypedNode(Node));
 bool Result = BaseTraverse();
 ParentStack.pop_back();
@@ -284,12 +290,6 @@
 &Map.PointerParents

[clang-tools-extra] e39da8a - Recommit "[CUDA][HIP] Defer overloading resolution diagnostics for host device functions"

[Yaxun Liu via cfe-commits]

Author: Yaxun (Sam) Liu
Date: 2020-09-24T08:44:37-04:00
New Revision: e39da8ab6a286ac777d5fe7799f1eb782cf99938

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

LOG: Recommit "[CUDA][HIP] Defer overloading resolution diagnostics for host 
device functions"

This recommits 7f1f89ec8d9944559042bb6d3b1132eabe3409de and
40df06cdafc010002fc9cfe1dda73d689b7d27a6 after fixing memory
sanitizer failure.

Added: 
clang/test/SemaCUDA/deferred-oeverload.cu
clang/test/TableGen/deferred-diag.td

Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang/include/clang/Basic/Diagnostic.td
clang/include/clang/Basic/DiagnosticAST.h
clang/include/clang/Basic/DiagnosticAnalysis.h
clang/include/clang/Basic/DiagnosticComment.h
clang/include/clang/Basic/DiagnosticCrossTU.h
clang/include/clang/Basic/DiagnosticDriver.h
clang/include/clang/Basic/DiagnosticFrontend.h
clang/include/clang/Basic/DiagnosticIDs.h
clang/include/clang/Basic/DiagnosticLex.h
clang/include/clang/Basic/DiagnosticParse.h
clang/include/clang/Basic/DiagnosticRefactoring.h
clang/include/clang/Basic/DiagnosticSema.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/DiagnosticSerialization.h
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Sema/Sema.h
clang/lib/Basic/DiagnosticIDs.cpp
clang/lib/Driver/ToolChains/Cuda.cpp
clang/lib/Driver/ToolChains/HIP.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Sema/SemaCUDA.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExprObjC.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaSYCL.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/SemaStmtAsm.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/SemaTemplateVariadic.cpp
clang/lib/Sema/SemaType.cpp
clang/test/TableGen/DiagnosticBase.inc
clang/tools/diagtool/DiagnosticNames.cpp
clang/utils/TableGen/ClangDiagnosticsEmitter.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index afa72f9d4051..18ff96202e0a 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -43,7 +43,7 @@ namespace {
 const char *getDiagnosticCode(unsigned ID) {
   switch (ID) {
 #define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROPU, SFINAE, NOWERROR,  
\
- SHOWINSYSHEADER, CATEGORY)
\
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
   case clang::diag::ENUM:  
\
 return #ENUM;
 #include "clang/Basic/DiagnosticASTKinds.inc"

diff  --git a/clang/include/clang/Basic/Diagnostic.td 
b/clang/include/clang/Basic/Diagnostic.td
index 48ba8c0f469f..ab2c738a2ace 100644
--- a/clang/include/clang/Basic/Diagnostic.td
+++ b/clang/include/clang/Basic/Diagnostic.td
@@ -45,6 +45,7 @@ class TextSubstitution {
   // diagnostics
   string Component = "";
   string CategoryName = "";
+  bit Deferrable = 0;
 }
 
 // Diagnostic Categories.  These can be applied to groups or individual
@@ -83,6 +84,7 @@ class Diagnostic {
   bitAccessControl = 0;
   bitWarningNoWerror = 0;
   bitShowInSystemHeader = 0;
+  bitDeferrable = 0;
   Severity   DefaultSeverity = defaultmapping;
   DiagGroup  Group;
   string CategoryName = "";
@@ -106,6 +108,14 @@ class SuppressInSystemHeader {
   bit ShowInSystemHeader = 0;
 }
 
+class Deferrable {
+  bit Deferrable = 1;
+}
+
+class NonDeferrable {
+  bit Deferrable = 0;
+}
+
 // FIXME: ExtWarn and Extension should also be SFINAEFailure by default.
 class Error : Diagnostic, 
SFINAEFailure {
   bit ShowInSystemHeader = 1;

diff  --git a/clang/include/clang/Basic/DiagnosticAST.h 
b/clang/include/clang/Basic/DiagnosticAST.h
index afe5f62e2012..76c31ad9508e 100644
--- a/clang/include/clang/Basic/DiagnosticAST.h
+++ b/clang/include/clang/Basic/DiagnosticAST.h
@@ -15,7 +15,7 @@ namespace clang {
 namespace diag {
 enum {
 #define DIAG(ENUM, FLAGS, DEFAULT_MAPPING, DESC, GROUP, SFINAE, NOWERROR,  
\
- SHOWINSYSHEADER, CATEGORY)
\
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
   ENUM,
 #define ASTSTART
 #include "clang/Basic/DiagnosticASTKinds.inc"

diff  --git a/clang/include/clang/Basic/DiagnosticAnalysis.h 
b/clang/include/clang/Basic/DiagnosticA

[PATCH] D75044: [AArch64] __builtin_return_address for PAuth.

[Momchil Velikov via Phabricator via cfe-commits]

chill accepted this revision.
chill added a comment.

In D75044#2292302 , @danielkiss wrote:

> @chill ping.

Sorry, I thought about committing all PAC/BTI patches together, but there's no 
reason, is there?
So, let's go ahead and commit the two dealing with `__builtin-return_address` .


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

https://reviews.llvm.org/D75044

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

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA added a comment.

In D88105#2292266 , @nemanjai wrote:

> This clearly changes behaviour and should thereby not have the `[NFC]` tag.

Looks like it accidentally includes https://reviews.llvm.org/D88130.


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

https://reviews.llvm.org/D88105

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

[PATCH] D77062: [analyzer] Improve zero assumption in CStringChecke::assumeZero

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@steakhal

> I would not accept this patch unless this investigation is done. However, I'm 
> not inherently against this change.

Actually I've done the investigation. You can find it here 
https://reviews.llvm.org/D77062#1977613

The bug is somewhere deep in the core. I see two solutions:

1. Leave the crash presented in the clang and dig deep into the core trying to 
solve the root cause.
2. Accept the change with a TODO promt and dig deep into the core trying to 
solve the root cause.




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

https://reviews.llvm.org/D77062

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

[PATCH] D85984: [analyzer] Add a new checker alpha.cplusplus.CPlusPlus11Lock

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@NoQ




Comment at: clang/test/Analysis/Checkers/CPlusPlus11LockChecker.cpp:379-382
+void rm_bad1() {
+  rm1.lock(); // no-warning
+  rm1.lock(); // expected-warning{{This lock has already been acquired}}
+}

NoQ wrote:
> ASDenysPetrov wrote:
> > NoQ wrote:
> > > I repeat, this is a false positive. Recursive mutexes can be locked 
> > > twice, that's the whole point.
> > Yes, I remember. I think you've mixed up with my previous attempts of 
> > introducting a new checks for recursive mutexes.
> > This is not this case. This kind of checks already exists in 
> > PthreadLockChecker before.
> > I've just reused this check for all kind of STL mutexes.
> > 
> > If you look at `void bad1()` in clang\test\Analysis\pthreadlock.c you can 
> > find the same case.
> >  I've just reused this check for all kind of STL mutexes.
> 
> You're taking code for mushing potatoes and applying it to differential 
> calculus.
> 
> Different kinds of mutexes behave differently. This is not the same case. 
> That test is a true positive because that mutex is non-recursive, your test 
> is a false positive because your mutex is recursive. In that test the program 
> immediately hangs and there's no valid way to unhang it, in your test the 
> programmer simply has to unlock the mutex twice and they're good to go.
@Noq, sorry. This might be a simple misunderstanging. Previously there was a 
question in the summary of what if we will consider a twice lock for recursive 
mutex as a correct case:
```
recursive_mutex rm;
m.lock();
m.lock(); // OK
```
You've answered that recursive locks are much more complicated than that 
(https://reviews.llvm.org/D85984#2237242) and offered to make this separately. 
So I decided not to make any changes and handle recursive_mutexes as all the 
rest. But if you suggest to do this in a single patch, so I will do.


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

https://reviews.llvm.org/D85984

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

[PATCH] D69560: [clang-tidy] Add 'experimental-cppcoreguidelines-avoid-adjacent-arguments-of-the-same-type' check

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

I'd like to resurrect the discussion about this. Unfortunately, the concept of 
`experimental-` group (in D76545 ) has fallen 
silent, too. We will present the results of this analysis soon at IEEE SCAM 
(Source Code Analysis and Manipulation) 2020 . While 
a previous submission about this topic was rejected on the grounds of not being 
in line with the conference's usual, hyper-formalised nature, with one reviewer 
//especially// praising the paper for its nice touch with the empirical world 
and the fact that neither argument swaps (another checker worked on by a 
colleague) nor the interactions of this issue with the type system (this 
checker) was really investigated in the literature for C++ before, we've 
tailored both the paper and the implementation based on reviewer comments from 
the scientific world, and the comments here.
The reviews were mostly favourable, excl. comments about the lack of 
formalisation and the unfortunate lack of modelling for template 
instantiations. Such a cold cut, however, //only// introduces false negatives 
into the result set. Which is fine, as the users will never see those 
non-existent reports!

In D69560#1936574 , @aaron.ballman 
wrote:

> This is a check that is almost impossible to comply with when developing new 
> code because it is insufficiently clear on what constitutes "related" 
> parameters.

I agree the CppCG is rather vague about this. From an angle, it seems 
intentional... "relatedness" is hard to pin down formally, it might vary from 
one "module" to the next even in the same project. In a subsequent patch to 
this, I've given a few good examples as to what can be reasonably culled by 
relatedness heuristics. They filter a good chunk of the results, letting go of 
most of the trivially "valid (but maybe nonsense) if swapped" functions (min, 
max, find, etc.)

In D69560#1936574 , @aaron.ballman 
wrote:

> Two adjacent types in a function parameter list is an incredibly common 
> situation that arises naturally when writing code [...] and designing around 
> that from scratch is not always possible.

I see the reasoning behind this. However, this feels more like a motivation 
//against// the rule itself, and not the checker. We can debate the 
justification for the rule's existence, but (and correct me if I'm wrong) so 
far I have not seen any tool (that's publicly available, and is for C(++), 
etc...) that attempts to check your code satisfying this particular rule.

In D69560#1936574 , @aaron.ballman 
wrote:

> (especially for constructors, where your parameter order has to match the 
> declaration order within the class)

CMIIW, but I believe this statement is not true. Aggregate initialisation does 
not care about your constructors, and yes, takes arguments in the order of 
fields. However, once you have a user-defined constructor, you should be able 
to define your parameters in any order you like. The only thing you //should// 
match is that the //member-init-exprs// of the constructor are evaluated in 
order of field declarations, not in the order you wrote them. But trespassing 
that rule already emits a compiler warning, and in reality, the trespass only 
causes an issue if there is a data dependency between your fields. You 
//should// ensure your //member-init-exprs// are in the right order (to guard 
that a later change introducing a data dependency won't blow you up), but this 
has zero effect on the order of parameters of the constructor.

In D69560#1936574 , @aaron.ballman 
wrote:

> Retroactively applying this rule to an existing code base would be nearly 
> impossible for a project of any size.

It is hard in the general case (although there are some approaches and 
foundations that we could build upon with refactoring, I've taken up the mantle 
of spending some time with this research), but there are cases where 
"modernisation" efforts, especially tool-aided ones are already possible. I 
have been recently suggested reading this paper: //H. K. Wright: Incremental 
Type Migration Using Type Algebra // 
While I agree that the case study in the paper is rather domain-specific if we 
consider the example there: once a tool has identified a variable/expression to 
not be a `double` but instead `abseil::Duration`, from a function call where 
such a refactored argument is passed, you can (or, hopefully, need to) replace 
the type of the parameter. And one such "adjacent parameters of the same type" 
has been refactored.

My position with the paper, and this implementation, is that it is more useful 
for new projects than existing ones. It might definitely not be useful //for 
LLVM// (a humongous project!), and it might not be possible to fix //every// 

[PATCH] D87981: [X86] AMX programming model prototype.

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke updated this revision to Diff 294036.
LuoYuanke added a comment.

Updating D87981 : [X86] AMX programming model 
prototype.
 Rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87981

Files:
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/lib/Headers/amxintrin.h
  clang/test/CodeGen/AMX/amx_api.c
  llvm/include/llvm/CodeGen/LiveIntervalUnion.h
  llvm/include/llvm/CodeGen/LiveRegMatrix.h
  llvm/include/llvm/CodeGen/Passes.h
  llvm/include/llvm/CodeGen/TileShapeInfo.h
  llvm/include/llvm/CodeGen/VirtRegMap.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsX86.td
  llvm/lib/CodeGen/InlineSpiller.cpp
  llvm/lib/CodeGen/LiveIntervalUnion.cpp
  llvm/lib/CodeGen/LiveRegMatrix.cpp
  llvm/lib/CodeGen/VirtRegMap.cpp
  llvm/lib/IR/Function.cpp
  llvm/lib/Target/X86/CMakeLists.txt
  llvm/lib/Target/X86/X86.h
  llvm/lib/Target/X86/X86ExpandPseudo.cpp
  llvm/lib/Target/X86/X86ISelDAGToDAG.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86InstrAMX.td
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86LowerAMXType.cpp
  llvm/lib/Target/X86/X86RegisterInfo.cpp
  llvm/lib/Target/X86/X86RegisterInfo.h
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/lib/Target/X86/X86Subtarget.h
  llvm/lib/Target/X86/X86TargetMachine.cpp
  llvm/lib/Target/X86/X86TileConfig.cpp
  llvm/test/CodeGen/X86/AMX/amx-across-func.ll
  llvm/test/CodeGen/X86/AMX/amx-config.ll
  llvm/test/CodeGen/X86/AMX/amx-spill.ll
  llvm/test/CodeGen/X86/AMX/amx-type.ll
  llvm/test/CodeGen/X86/O0-pipeline.ll
  llvm/test/CodeGen/X86/opt-pipeline.ll
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -197,25 +197,25 @@
 enum IIT_Info {
   // Common values should be encoded with 0-15.
   IIT_Done = 0,
-  IIT_I1   = 1,
-  IIT_I8   = 2,
-  IIT_I16  = 3,
-  IIT_I32  = 4,
-  IIT_I64  = 5,
-  IIT_F16  = 6,
-  IIT_F32  = 7,
-  IIT_F64  = 8,
-  IIT_V2   = 9,
-  IIT_V4   = 10,
-  IIT_V8   = 11,
-  IIT_V16  = 12,
-  IIT_V32  = 13,
-  IIT_PTR  = 14,
-  IIT_ARG  = 15,
+  IIT_I1 = 1,
+  IIT_I8 = 2,
+  IIT_I16 = 3,
+  IIT_I32 = 4,
+  IIT_I64 = 5,
+  IIT_F16 = 6,
+  IIT_F32 = 7,
+  IIT_F64 = 8,
+  IIT_V2 = 9,
+  IIT_V4 = 10,
+  IIT_V8 = 11,
+  IIT_V16 = 12,
+  IIT_V32 = 13,
+  IIT_PTR = 14,
+  IIT_ARG = 15,
 
   // Values from 16+ are only encodable with the inefficient encoding.
-  IIT_V64  = 16,
-  IIT_MMX  = 17,
+  IIT_V64 = 16,
+  IIT_MMX = 17,
   IIT_TOKEN = 18,
   IIT_METADATA = 19,
   IIT_EMPTYSTRUCT = 20,
@@ -226,7 +226,7 @@
   IIT_EXTEND_ARG = 25,
   IIT_TRUNC_ARG = 26,
   IIT_ANYPTR = 27,
-  IIT_V1   = 28,
+  IIT_V1 = 28,
   IIT_VARARG = 29,
   IIT_HALF_VEC_ARG = 30,
   IIT_SAME_VEC_WIDTH_ARG = 31,
@@ -246,7 +246,8 @@
   IIT_SUBDIVIDE4_ARG = 45,
   IIT_VEC_OF_BITCASTS_TO_INT = 46,
   IIT_V128 = 47,
-  IIT_BF16 = 48
+  IIT_BF16 = 48,
+  IIT_V256 = 49
 };
 
 static void EncodeFixedValueType(MVT::SimpleValueType VT,
@@ -384,6 +385,9 @@
 case 32: Sig.push_back(IIT_V32); break;
 case 64: Sig.push_back(IIT_V64); break;
 case 128: Sig.push_back(IIT_V128); break;
+case 256:
+  Sig.push_back(IIT_V256);
+  break;
 case 512: Sig.push_back(IIT_V512); break;
 case 1024: Sig.push_back(IIT_V1024); break;
 }
Index: llvm/test/CodeGen/X86/opt-pipeline.ll
===
--- llvm/test/CodeGen/X86/opt-pipeline.ll
+++ llvm/test/CodeGen/X86/opt-pipeline.ll
@@ -24,6 +24,7 @@
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Expand Atomic instructions
+; CHECK-NEXT:   Lower AMX type for load/store
 ; CHECK-NEXT:   Module Verifier
 ; CHECK-NEXT:   Dominator Tree Construction
 ; CHECK-NEXT:   Basic Alias Analysis (stateless AA impl)
@@ -141,6 +142,7 @@
 ; CHECK-NEXT:   Lazy Machine Block Frequency Analysis
 ; CHECK-NEXT:   Machine Optimization Remark Emitter
 ; CHECK-NEXT:   Greedy Register Allocator
+; CHECK-NEXT:   Tile Register Configure
 ; CHECK-NEXT:   Virtual Register Rewriter
 ; CHECK-NEXT:   Stack Slot Coloring
 ; CHECK-NEXT:   Machine Copy Propagation Pass
Index: llvm/test/CodeGen/X86/O0-pipeline.ll
===
--- llvm/test/CodeGen/X86/O0-pipeline.ll
+++ llvm/test/CodeGen/X86/O0-pipeline.ll
@@ -18,6 +18,7 @@
 ; CHECK-NEXT: Pre-ISel Intrinsic Lowering
 ; CHECK-NEXT: FunctionPass Manager
 ; CHECK-NEXT:   Expand Atomic instructions
+; CHECK-NEXT:   Lower AMX type for load/store
 ; CHECK-NEXT:   Module Verifier
 ; CHECK-NEXT:   Lower Garbage Collection Instructions
 ; CHECK-NEXT:   Shadow Stack GC Lowering
Index: llvm/test/CodeGen/X86/AMX

[PATCH] D87187: [Driver] Perform Linux distribution detection just once

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

In D87187#2291806 , @dmantipov wrote:

> IIUC compiler driver is not intended to be multithreaded. But OK, here is the 
> version with llvm::call_once(...).

Thanks! There could be at least two cases I see, where the driver //could// be 
multi-threaded:

1. once `clang-cl /MP -fintegrated-cc1` is supported (D52193 
), I'm still planning to work on that, 
although in that case, it wouldn't reach this distro detection. However that 
patch could be extended to support `clang ... -j`.
2. Further, if the idea of llvm-buildozer ever lands (D86351 
) which in essence is a generalization of the 
above.

There's a slight issue with your last update, it now fails the tests (because 
several distros are checked one after another in the same process). You can 
perhaps leverage `EXPECT_EXIT(..., ::testing::ExitedWithCode(0))` in the distro 
tests to run each of them in a separate process (see 
`llvm/unittests/Support/ErrorTest.cpp` for an example).
Also please run `git clang-format`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87187

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

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D36836#2289639 , @lebedev.ri wrote:

> Rebased.
>
> There is a number of official open-source LGPL-3 implementations already:
>
> - https://github.com/SonarSource/SonarTS/pull/378
> - https://github.com/SonarSource/sonar-java/pull/1385
> - https://github.com/SonarSource/SonarJS/pull/449
> - https://github.com/SonarSource/sonar-php/pull/173
>
> There are other open-source LGPL-3 implementations already:
>
> - https://pypi.org/project/cognitive-complexity/ (MIT)
> - https://github.com/rossmacarthur/complexity (APACHE/MIT)
>
> There are other 3rd party implementations:
>
> - https://docs.codeclimate.com/docs/cognitive-complexity
>
> Quite honestly, i do not understand how did the license question arose.

It arose in a comment that I can't seem to get phab to show me the context for 
(which is a bit strange, I don't think I've run into that before): 
https://reviews.llvm.org/D36836#877636 Perhaps part of this was carrying 
discussion over from the IRC channel?

> Would have it been fine if i based this on the open-source-licensed code?

I believe that would require legal analysis to answer.

> Would have it not been? Would same license question be raised?

Likewise here (I suspect the answer would depend on what the license of the 
open source code is).

> Somehow i don't think it would have been.

I don't wish to speculate about legal licensing issues on the mailing lists.

> Is this really just about `Copyright SonarSource S.A., 2018, Switzerland. All 
> content is copyright protected.` in 
> https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
> But that is only about the document, not the algorithm.
> But even if we enternain the idea that all of the implementations must bow to 
> that license,
> then surely this is not the first code in LLVM that is implicitly/explicitly 
> based on copyrighted doc.
>
> This is rather frustrating.

I am sorry and I agree that it's frustrating. As far as I know, this captures 
the current state of affairs: https://reviews.llvm.org/D36836#1031600 and 
basically we're waiting for help from the foundation to clear the last hurdle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36836

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

[PATCH] D86819: [PowerPC][Power10] Implementation of 128-bit Binary Vector Rotate builtins

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

The remaining requests can be fulfilled when committing. I don't think this 
requires another review. Thanks.




Comment at: clang/lib/Headers/altivec.h:7865
+#endif
+  return __builtin_altivec_vrlqnm(__a, MaskAndShift);
+}

Please add explicit cast to `vector unsigned __int128` for `MaskAndShift`. 
Similarly below. I forgot to add that to my comment.



Comment at: clang/test/CodeGen/builtins-ppc-p10vector.c:996
+  // CHECK-COMMON-LABEL: @test_vec_rlnm_s128(
+  // CHECK-COMMON: call <1 x i128> @llvm.ppc.altivec.vrlqnm(<1 x i128>
+  // CHECK-COMMON-NEXT: ret <1 x i128>

nemanjai wrote:
> Please show the shift in the test case as well.
> Please show the shift in the test case as well.

This was still not addressed. Please show the shuffle in the checks.



Comment at: llvm/test/CodeGen/PowerPC/p10-vector-rotate.ll:59
+; Function Attrs: nounwind readnone
+define <1 x i128> @test_vrlqnm(<1 x i128> %a, <1 x i128> %b, <1 x i128> %c) {
+; CHECK-LABEL: test_vrlqnm:

Add a test case for this that was produced from `vec_rlnm` at -O2.


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

https://reviews.llvm.org/D86819

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

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: JakeMerdichAMD, MyDeveloperDay, jrtc27.
Herald added subscribers: cfe-commits, krytarowski, emaste.
Herald added a project: clang.
arichardson requested review of this revision.

Some projects (e.g. FreeBSD) align pointers to the right but expect a
space between the '*' and any pointer qualifiers such as const.

SpaceBeforePointerQualifiers = false (LLVM style):   void *const x = NULL;
SpaceBeforePointerQualifiers = true (FreeBSD style): void * const x = NULL;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88227

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12104,6 +12104,47 @@
NoSpaceStyle);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforePointerQualifiers) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.AttributeMacros.push_back("qualified");
+  verifyFormat("SomeType *const *a = NULL;", Spaces);
+  verifyFormat("SomeType *volatile *a = NULL;", Spaces);
+  verifyFormat("SomeType *__attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("SomeType *qualified *a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  Spaces.SpaceBeforePointerQualifiers = true;
+  verifyFormat("SomeType * const *a = NULL;", Spaces);
+  verifyFormat("SomeType * volatile *a = NULL;", Spaces);
+  verifyFormat("SomeType * __attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("SomeType * qualified *a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  // Check that setting SpaceBeforePointerQualifiers doesn't result in extra
+  // spaces for PAS_Left/PAS_Middle.
+  Spaces.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("SomeType* const* a = NULL;", Spaces);
+  verifyFormat("SomeType* volatile* a = NULL;", Spaces);
+  verifyFormat("SomeType* __attribute__((attr))* a = NULL;", Spaces);
+  verifyFormat("SomeType* qualified* a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  Spaces.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("SomeType * const * a = NULL;", Spaces);
+  verifyFormat("SomeType * volatile * a = NULL;", Spaces);
+  verifyFormat("SomeType * __attribute__((attr)) * a = NULL;", Spaces);
+  verifyFormat("SomeType * qualified * a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+}
+
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignConsecutiveAssignments = true;
@@ -14003,6 +14044,7 @@
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
+  CHECK_PARSE_BOOL(SpaceBeforePointerQualifiers);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
   CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
   CHECK_PARSE_BOOL(UseCRLF);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2871,7 +2871,10 @@
(Style.PointerAlignment != FormatStyle::PAS_Right &&
 !Line.IsMultiVariableDeclStmt)))
 return true;
-  if (Left.is(TT_PointerOrReference))
+  if (Left.is(TT_PointerOrReference)) {
+if (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier())
+  return true;
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||
@@ -2883,6 +2886,7 @@
 Left.Previous &&
 !Left.Previous->isOneOf(tok::l_paren, tok::coloncolon,
 tok::l_square));
+  }
   // Ensure right pointer alignement with ellipsis e.g. int *...P
   if (Left.is(tok::ellipsis) && Left.Previous &&
   Left.Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -598,6 +598,8 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceBeforePointerQualifiers",
+   Style.Spac

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889
+  if (Left.is(TT_PointerOrReference)) {
+if (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier())
+  return true;
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

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

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889
+  if (Left.is(TT_PointerOrReference)) {
+if (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier())
+  return true;
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||

jrtc27 wrote:
> 
I feel like moving it here could in theory miss some cases. Also the condition 
is already barely comprehensible (I didn't attempt to understand which special 
cases all these conditions are for) and I don't feel like making it more 
complex.

If clang-format has identified that this */& token is a pointer/reference, and 
the next token is something that can be paresed as a pointer qualifier, 
shouldn't we trust the parser and simply look at the format option? It also IMO 
makes the code slightly easier to understand.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

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

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889
+  if (Left.is(TT_PointerOrReference)) {
+if (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier())
+  return true;
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||

arichardson wrote:
> jrtc27 wrote:
> > 
> I feel like moving it here could in theory miss some cases. Also the 
> condition is already barely comprehensible (I didn't attempt to understand 
> which special cases all these conditions are for) and I don't feel like 
> making it more complex.
> 
> If clang-format has identified that this */& token is a pointer/reference, 
> and the next token is something that can be paresed as a pointer qualifier, 
> shouldn't we trust the parser and simply look at the format option? It also 
> IMO makes the code slightly easier to understand.
It's a series of `A || B || C || ...`, and you just added an `if (X) return 
true;` above, so changing it to `A || B || C || X || ...` is entirely 
equivalent, no need to understand what any of the other expressions in the 
giant OR are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

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

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889
+  if (Left.is(TT_PointerOrReference)) {
+if (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier())
+  return true;
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||

arichardson wrote:
> jrtc27 wrote:
> > 
> I feel like moving it here could in theory miss some cases. Also the 
> condition is already barely comprehensible (I didn't attempt to understand 
> which special cases all these conditions are for) and I don't feel like 
> making it more complex.
> 
> If clang-format has identified that this */& token is a pointer/reference, 
> and the next token is something that can be paresed as a pointer qualifier, 
> shouldn't we trust the parser and simply look at the format option? It also 
> IMO makes the code slightly easier to understand.
Never mind, I thought your suggestion it was inside nested parens. It's 
equivalent just avoids the braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

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

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:2874-2889
+  if (Left.is(TT_PointerOrReference)) {
+if (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier())
+  return true;
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||

arichardson wrote:
> jrtc27 wrote:
> > arichardson wrote:
> > > jrtc27 wrote:
> > > > 
> > > I feel like moving it here could in theory miss some cases. Also the 
> > > condition is already barely comprehensible (I didn't attempt to 
> > > understand which special cases all these conditions are for) and I don't 
> > > feel like making it more complex.
> > > 
> > > If clang-format has identified that this */& token is a 
> > > pointer/reference, and the next token is something that can be paresed as 
> > > a pointer qualifier, shouldn't we trust the parser and simply look at the 
> > > format option? It also IMO makes the code slightly easier to understand.
> > It's a series of `A || B || C || ...`, and you just added an `if (X) return 
> > true;` above, so changing it to `A || B || C || X || ...` is entirely 
> > equivalent, no need to understand what any of the other expressions in the 
> > giant OR are.
> Never mind, I thought your suggestion it was inside nested parens. It's 
> equivalent just avoids the braces.
Uh of course with `||` added on the end of that new sub-expression, managed to 
omit that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

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

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 294049.
arichardson marked 3 inline comments as done.
arichardson added a comment.

merge if with existing condition


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12104,6 +12104,47 @@
NoSpaceStyle);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforePointerQualifiers) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.AttributeMacros.push_back("qualified");
+  verifyFormat("SomeType *const *a = NULL;", Spaces);
+  verifyFormat("SomeType *volatile *a = NULL;", Spaces);
+  verifyFormat("SomeType *__attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("SomeType *qualified *a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  Spaces.SpaceBeforePointerQualifiers = true;
+  verifyFormat("SomeType * const *a = NULL;", Spaces);
+  verifyFormat("SomeType * volatile *a = NULL;", Spaces);
+  verifyFormat("SomeType * __attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("SomeType * qualified *a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  // Check that setting SpaceBeforePointerQualifiers doesn't result in extra
+  // spaces for PAS_Left/PAS_Middle.
+  Spaces.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("SomeType* const* a = NULL;", Spaces);
+  verifyFormat("SomeType* volatile* a = NULL;", Spaces);
+  verifyFormat("SomeType* __attribute__((attr))* a = NULL;", Spaces);
+  verifyFormat("SomeType* qualified* a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  Spaces.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("SomeType * const * a = NULL;", Spaces);
+  verifyFormat("SomeType * volatile * a = NULL;", Spaces);
+  verifyFormat("SomeType * __attribute__((attr)) * a = NULL;", Spaces);
+  verifyFormat("SomeType * qualified * a = NULL;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+}
+
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignConsecutiveAssignments = true;
@@ -14003,6 +14044,7 @@
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
+  CHECK_PARSE_BOOL(SpaceBeforePointerQualifiers);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
   CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
   CHECK_PARSE_BOOL(UseCRLF);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2873,6 +2873,8 @@
 return true;
   if (Left.is(TT_PointerOrReference))
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
+   (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier()) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||
(Right.is(tok::l_brace) && Right.is(BK_Block)) ||
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -598,6 +598,8 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceBeforePointerQualifiers",
+   Style.SpaceBeforePointerQualifiers);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
 IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock);
@@ -938,6 +940,7 @@
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  LLVMStyle.SpaceBeforePointerQualifiers = false;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/For

[PATCH] D88088: WIP [clang] improve accuracy of ExprMutAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:455
+  const auto HasAnyNonConstIterator =
+  anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst(,
+  unless(hasMethod(allOf(hasName("begin"), isConst(),

aaron.ballman wrote:
> Do we want to look for methods that end with `_?[Bb]egin` or `_?[Ee]nd` so 
> that this would catch patterns like `foo_begin()`/`foo_end()`, 
> `FooBegin()`/`FooEnd()`, or `Foo_Begin()`/`Foo_End()`?
This specific matcher is only applied in range-for contexts. There only the 
`begin(); end()` methods matter. I updated the comment above to clarify this.



Comment at: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp:65
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {

aaron.ballman wrote:
> Was there a reason you hoisted this out of the `for` loop?
Jup.
```
buffer.clear()
```
The current form does proper memory-recycling (i believe at least :D)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88088

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

[PATCH] D88088: WIP [clang] improve accuracy of ExprMutAnalyzer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 294050.
JonasToth marked 9 inline comments as done.
JonasToth added a comment.

- adress review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88088

Files:
  clang/lib/Analysis/ExprMutationAnalyzer.cpp
  clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp

Index: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
===
--- clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
+++ clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp
@@ -19,9 +19,7 @@
 
 using namespace clang::ast_matchers;
 using ::testing::ElementsAre;
-using ::testing::IsEmpty;
 using ::testing::ResultOf;
-using ::testing::StartsWith;
 using ::testing::Values;
 
 namespace {
@@ -63,13 +61,18 @@
   const auto *const S = selectFirst("stmt", Results);
   SmallVector Chain;
   ExprMutationAnalyzer Analyzer(*S, AST->getASTContext());
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {
 const Stmt *By = Analyzer.findMutation(E);
-std::string buffer;
+if (!By)
+  break;
+
 llvm::raw_string_ostream stream(buffer);
 By->printPretty(stream, nullptr, AST->getASTContext().getPrintingPolicy());
-Chain.push_back(StringRef(stream.str()).trim().str());
+Chain.emplace_back(StringRef(stream.str()).trim().str());
 E = dyn_cast(By);
+buffer.clear();
   }
   return Chain;
 }
@@ -111,7 +114,13 @@
 
 class AssignmentTest : public ::testing::TestWithParam {};
 
+// This test is for the most basic and direct modification of a variable,
+// assignment to it (e.g. `x = 10;`).
+// It additionally tests, that reference to a variable are not only captured
+// directly, but expression that result in the variable are handled, too.
+// This includes the comma operator, parens and the ternary operator.
 TEST_P(AssignmentTest, AssignmentModifies) {
+  // Test the detection of the raw expression modifications.
   {
 const std::string ModExpr = "x " + GetParam() + " 10";
 const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -120,6 +129,7 @@
 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
   }
 
+  // Test the detection if the expression is surrounded by parens.
   {
 const std::string ModExpr = "(x) " + GetParam() + " 10";
 const auto AST = buildASTFromCode("void f() { int x; " + ModExpr + "; }");
@@ -127,6 +137,60 @@
 match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
 EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
   }
+
+  // Test the detection if the comma operator yields the expression as result.
+  {
+const std::string ModExpr = "x " + GetParam() + " 10";
+const auto AST = buildASTFromCodeWithArgs(
+"void f() { int x, y; y, " + ModExpr + "; }", {"-Wno-unused-value"});
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Ensure no detection if t he comma operator does not yield the expression as
+  // result.
+  {
+const std::string ModExpr = "y, x, y " + GetParam() + " 10";
+const auto AST = buildASTFromCodeWithArgs(
+"void f() { int x, y; " + ModExpr + "; }", {"-Wno-unused-value"});
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_FALSE(isMutated(Results, AST.get()));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression.
+  {
+const std::string ModExpr = "(y != 0 ? y : x) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression
+  // through multiple nesting of ternary operators.
+  {
+const std::string ModExpr =
+"(y != 0 ? (y > 5 ? y : x) : (y)) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre(ModExpr));
+  }
+
+  // Test the detection if the a ternary operator can result in the expression
+  // with additional parens.
+  {
+const std::string ModExpr = "(y != 0 ? (y) : ((x))) " + GetParam() + " 10";
+const auto AST =
+buildASTFromCode("void f() { int y = 0, x; " + ModExpr + "; }");
+const auto Results =
+match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+EXPECT_THAT(mutatedBy(Results, AST.get()), Elem

[PATCH] D88195: Remove stale assert.

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/test/Modules/asm-goto.cpp:1
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x c++ -I%S/Inputs/asm-goto 
-emit-module %S/Inputs/asm-goto/module.modulemap -fmodule-name=a -o %t/a.pcm

void wrote:
> jyknight wrote:
> > New test seems to be failing on the autobuilders (yay pre-submit 
> > autobuilders!!) for 2 reasons:
> > 1. C++ function-name mangling on Windows is different.
> > 2. "$0 = callbr" != "%0 = callbr"
> > 
> Don't know why I did that. Fixed. :)
Still have problem 1, that @"?foo@@YAHXZ"() != @_Z3foov

Probably simplest to just do
extern "C" {
int foo() {...}
}

to avoid name mangling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88195

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

[PATCH] D88182: [clang][driver][AIX] Set compiler-rt as default rtlib

[David Tenty via Phabricator via cfe-commits]

daltenty updated this revision to Diff 294053.
daltenty added a comment.

- Fix formating
- Fix typos
- Replace pattern with windows-friendly directory seperators


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88182

Files:
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/lib/Driver/ToolChains/AIX.h
  clang/test/Driver/aix-ld.c
  clang/test/Driver/aix-rtlib.c

Index: clang/test/Driver/aix-rtlib.c
===
--- /dev/null
+++ clang/test/Driver/aix-rtlib.c
@@ -0,0 +1,10 @@
+// Check the default rtlib for AIX.
+// RUN: %clang -target powerpc-ibm-aix -print-libgcc-file-name -no-canonical-prefixes \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck -check-prefix=CHECK32 %s
+// RUN: %clang -target powerpc64-ibm-aix -print-libgcc-file-name -no-canonical-prefixes \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck -check-prefix=CHECK64 %s
+
+// CHECK32: resource_dir/lib/aix/libclang_rt.builtins-powerpc.a
+// CHECK64: resource_dir/lib/aix/libclang_rt.builtins-powerpc64.a
Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -3,11 +3,13 @@
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit.
 // RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
 // RUN:-target powerpc-ibm-aix7.1.0.0 \
 // RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32 %s
 // CHECK-LD32-NOT: warning:
 // CHECK-LD32: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD32: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD32: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-NOT: "-bnso"
@@ -17,15 +19,18 @@
 // CHECK-LD32-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD32: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-NOT: "-lc++"
+// CHECK-LD32: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit.
 // RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
 // RUN:-target powerpc64-ibm-aix7.1.0.0 \
 // RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD64 %s
 // CHECK-LD64-NOT: warning:
 // CHECK-LD64: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD64: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD64: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD64-NOT: "-bnso"
@@ -35,16 +40,19 @@
 // CHECK-LD64-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti_64.o"
 // CHECK-LD64: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD64-NOT: "-lc++"
+// CHECK-LD64: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64: "-lc"
 
 // Check powerpc-ibm-aix7.1.0.0, 32-bit. Enable POSIX thread support.
 // RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
 // RUN:-pthread \
 // RUN:-target powerpc-ibm-aix7.1.0.0 \
 // RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD32-PTHREAD %s
 // CHECK-LD32-PTHREAD-NOT: warning:
 // CHECK-LD32-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-LD32-PTHREAD: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD32-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD32-PTHREAD: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD32-PTHREAD-NOT: "-bnso"
@@ -54,17 +62,20 @@
 // CHECK-LD32-PTHREAD-NOT: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
 // CHECK-LD32-PTHREAD: "-L[[SYSROOT]]/usr/lib"
 // CHECK-LD32-PTHREAD-NOT: "-lc++"
+// CHECK-LD32-PTHREAD: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
 // CHECK-LD32-PTHREAD: "-lpthreads"
 // CHECK-LD32-PTHREAD: "-lc"
 
 // Check powerpc64-ibm-aix7.1.0.0, 64-bit. POSIX thread alias.
 // RUN: %clang -no-canonical-prefixes %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
 // RUN:-pthreads \
 // RUN:-target powerpc64-ibm-aix7.1.0.0 \
 // RUN:--sysroot %S/Inputs/aix_ppc_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-LD64-PTHREAD %s
 // CHECK-LD64-PTHREAD-NOT: warning:
 // CHECK-LD64-PTHREAD: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc64-ibm-aix7.1.0.0"
+// CHECK-LD64-PTHREAD: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
 // CHECK-LD64-PTHREAD: "-isysroot" "[[SYSROOT:[^"]+]]"
 // CHECK-LD64-PTHREAD: "{{.*}}ld{{(.exe)?}}"
 // CHECK-LD64-PTHREAD-NOT: "-bnso"
@@ -74,17 +85,20 @@
 // CHECK-LD64-PTHREAD

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D36836#2292541 , @aaron.ballman 
wrote:

> In D36836#2289639 , @lebedev.ri 
> wrote:
>
>> Rebased.
>>
>> There is a number of official open-source LGPL-3 implementations already:
>>
>> - https://github.com/SonarSource/SonarTS/pull/378
>> - https://github.com/SonarSource/sonar-java/pull/1385
>> - https://github.com/SonarSource/SonarJS/pull/449
>> - https://github.com/SonarSource/sonar-php/pull/173
>>
>> There are other open-source LGPL-3 implementations already:
>>
>> - https://pypi.org/project/cognitive-complexity/ (MIT)
>> - https://github.com/rossmacarthur/complexity (APACHE/MIT)
>>
>> There are other 3rd party implementations:
>>
>> - https://docs.codeclimate.com/docs/cognitive-complexity
>>
>> Quite honestly, i do not understand how did the license question arose.
>
> It arose in a comment that I can't seem to get phab to show me the context 
> for (which is a bit strange, I don't think I've run into that before): 
> https://reviews.llvm.org/D36836#877636 Perhaps part of this was carrying 
> discussion over from the IRC channel?
>
>> Would have it been fine if i based this on the open-source-licensed code?
>
> I believe that would require legal analysis to answer.
>
>> Would have it not been? Would same license question be raised?
>
> Likewise here (I suspect the answer would depend on what the license of the 
> open source code is).
>
>> Somehow i don't think it would have been.
>
> I don't wish to speculate about legal licensing issues on the mailing lists.
>
>> Is this really just about `Copyright SonarSource S.A., 2018, Switzerland. 
>> All content is copyright protected.` in 
>> https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
>> But that is only about the document, not the algorithm.
>> But even if we enternain the idea that all of the implementations must bow 
>> to that license,
>> then surely this is not the first code in LLVM that is implicitly/explicitly 
>> based on copyrighted doc.
>>
>> This is rather frustrating.
>
> I am sorry and I agree that it's frustrating.



> As far as I know, this captures the current state of affairs: 
> https://reviews.llvm.org/D36836#1031600

As far as I know, yes. Some further back&forth reiterated:

> @Roman so you know, none of the non-SonarSource implementations have an 
> official license from us.
> We put the spec out in the world and we're happy when someone uses it. And 
> that's all.
> I appreciate how frustrated you must be with your implementation caught 
> between a proverbial rock and a hard place.
> Unfortunately, we (the company) just aren't willing to do the paperwork.
> (G. Ann Campbell)

F13053113: reply.eml 

> and basically we're waiting for help from the foundation to clear the last 
> hurdle.

Is foundation even aware of this controversy/situation?
https://reviews.llvm.org/D36836#1021863, which is the last response i got, was 
2.5 years ago.
For all we/i know this has gone off their radar.
I understand that it is fully possible that they simply haven't gotten around 
to it,
but i think it would be important to check that it isn't the case of lost mail.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36836

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

[clang] 31a3c5f - [clang] use string tables for static diagnostic descriptions

[Nathan Froyd via cfe-commits]

Author: Nathan Froyd
Date: 2020-09-24T10:54:28-04:00
New Revision: 31a3c5fb45b78bdaa78d94ffcc9258e839002016

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

LOG: [clang] use string tables for static diagnostic descriptions

Using a pointer for the description string in StaticDiagInfoRec causes
several problems:

1. We don't need to use a whole pointer to represent the string;
2. The use of pointers incurs runtime relocations for those pointers;
   the relocations take up space on disk and represent runtime overhead;
3. The need to relocate data implies that, on some platforms, the entire
   array containing StaticDiagInfoRecs cannot be shared between processes.

This patch changes the storage scheme for the diagnostic descriptions to
avoid these problems.  We instead generate (effectively) one large
string and then StaticDiagInfoRec conceptually holds offsets into the
string.  We elected to also move the storage of those offsets into a
separate array to further reduce the space required.

On x86-64 Linux, this change removes about 120KB of relocations and
moves about 60KB from the non-shareable .data.rel.ro section to
shareable .rodata.  (The array is about 80KB before this, but we
eliminated 4 bytes/entry by using offsets rather than pointers.)  We
actually reap this benefit twice, because these tables show up in both
libclang.so and libclang-cpp.so and we get the reduction in both places.

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

Added: 


Modified: 
clang/lib/Basic/DiagnosticIDs.cpp

Removed: 




diff  --git a/clang/lib/Basic/DiagnosticIDs.cpp 
b/clang/lib/Basic/DiagnosticIDs.cpp
index 07e56fbcd611..b641077f1ac2 100644
--- a/clang/lib/Basic/DiagnosticIDs.cpp
+++ b/clang/lib/Basic/DiagnosticIDs.cpp
@@ -26,6 +26,78 @@ using namespace clang;
 
 namespace {
 
+struct StaticDiagInfoRec;
+
+// Store the descriptions in a separate table to avoid pointers that need to
+// be relocated, and also decrease the amount of data needed on 64-bit
+// platforms. See "How To Write Shared Libraries" by Ulrich Drepper.
+struct StaticDiagInfoDescriptionStringTable {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, 
\
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
+  char ENUM##_desc[sizeof(DESC)];
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
+const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, 
\
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
+  DESC,
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
+extern const StaticDiagInfoRec StaticDiagInfo[];
+
+// Stored separately from StaticDiagInfoRec to pack better.  Otherwise,
+// StaticDiagInfoRec would have extra padding on 64-bit platforms.
+const uint32_t StaticDiagInfoDescriptionOffsets[] = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, 
\
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)
\
+  offsetof(StaticDiagInfoDescriptionStringTable, ENUM##_desc),
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "cla

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Nathan Froyd via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG31a3c5fb45b7: [clang] use string tables for static 
diagnostic descriptions (authored by froydnj).

Changed prior to commit:
  https://reviews.llvm.org/D81865?vs=288628&id=294063#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

Files:
  clang/lib/Basic/DiagnosticIDs.cpp

Index: clang/lib/Basic/DiagnosticIDs.cpp
===
--- clang/lib/Basic/DiagnosticIDs.cpp
+++ clang/lib/Basic/DiagnosticIDs.cpp
@@ -26,6 +26,78 @@
 
 namespace {
 
+struct StaticDiagInfoRec;
+
+// Store the descriptions in a separate table to avoid pointers that need to
+// be relocated, and also decrease the amount of data needed on 64-bit
+// platforms. See "How To Write Shared Libraries" by Ulrich Drepper.
+struct StaticDiagInfoDescriptionStringTable {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)\
+  char ENUM##_desc[sizeof(DESC)];
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
+const StaticDiagInfoDescriptionStringTable StaticDiagInfoDescriptions = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)\
+  DESC,
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
+extern const StaticDiagInfoRec StaticDiagInfo[];
+
+// Stored separately from StaticDiagInfoRec to pack better.  Otherwise,
+// StaticDiagInfoRec would have extra padding on 64-bit platforms.
+const uint32_t StaticDiagInfoDescriptionOffsets[] = {
+#define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
+ SHOWINSYSHEADER, DEFERRABLE, CATEGORY)\
+  offsetof(StaticDiagInfoDescriptionStringTable, ENUM##_desc),
+  // clang-format off
+#include "clang/Basic/DiagnosticCommonKinds.inc"
+#include "clang/Basic/DiagnosticDriverKinds.inc"
+#include "clang/Basic/DiagnosticFrontendKinds.inc"
+#include "clang/Basic/DiagnosticSerializationKinds.inc"
+#include "clang/Basic/DiagnosticLexKinds.inc"
+#include "clang/Basic/DiagnosticParseKinds.inc"
+#include "clang/Basic/DiagnosticASTKinds.inc"
+#include "clang/Basic/DiagnosticCommentKinds.inc"
+#include "clang/Basic/DiagnosticCrossTUKinds.inc"
+#include "clang/Basic/DiagnosticSemaKinds.inc"
+#include "clang/Basic/DiagnosticAnalysisKinds.inc"
+#include "clang/Basic/DiagnosticRefactoringKinds.inc"
+  // clang-format on
+#undef DIAG
+};
+
 // Diagnostic classes.
 enum {
   CLASS_NOTE   = 0x01,
@@ -48,14 +120,16 @@
   uint16_t OptionGroupIndex;
 
   uint16_t DescriptionLen;
-  const char *DescriptionStr;
 
   unsigned getOptionGroupIndex() const {
 return OptionGroupIndex;
   }
 
   StringRef getDescription() const {
-return StringRef(DescriptionStr, DescriptionLen);
+size_t MyIndex = this - &StaticDiagInfo[0];
+uint32_t StringOffset = StaticDiagInfoDescriptionOffsets[MyIndex];
+const char* Table = reinterpret_cast(&StaticDiagInfoDescriptions);
+return StringRef(&Table[StringOffset], DescriptionLen);
   }
 
   diag::Flavor getFlavor() const {
@@ -93,14 +167,21 @@
 #undef VALIDATE_DIAG_SIZE
 #undef STRINGIFY_NAME
 
-} // namespace anonymous
-
-static const StaticDiagInfoRec StaticDiagInfo[] = {
+const StaticDiagInfoRec StaticDiagInfo[] = {
 #define DIAG(ENUM, CLASS, DEFAULT_SEVERITY, DESC, GROUP, SFINAE, NOWERROR, \
  SHOWINSYSHEADER, DEFERRABLE, CATEGORY)\
-  {diag::ENUM, DEFAULT_SEVERITY, CLASS,  DiagnosticIDs::SFINA

[PATCH] D87451: add new clang option -mno-xcoff-visibility

[Digger via Phabricator via cfe-commits]

DiggerLin marked 5 inline comments as done.
DiggerLin added inline comments.



Comment at: clang/lib/Frontend/CompilerInvocation.cpp:2768
   // The type-visibility mode defaults to the value-visibility mode.
   if (Arg *typeVisOpt = Args.getLastArg(OPT_ftype_visibility)) {
 Opts.setTypeVisibilityMode(parseVisibility(typeVisOpt, Args, Diags));

jasonliu wrote:
> Question: how should -mignore-xcoff-visiblity interact with -ftype-visibility?
the -ftype-visibility do not support in the clang,  so there is no interact.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87451

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

[PATCH] D88222: [AST] Use data-recursion when building ParentMap, avoid stack overflow.

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks for fixing this!




Comment at: clang/lib/AST/ParentMapContext.cpp:231
+  template 
+  void MarkChild(MapNodeTy MapNode, MapTy *Parents) {
+if (ParentStack.empty())

can we make some comments explaining what this method does?  I think I have a 
vague understanding, but would be helpful to have some documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88222

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

[PATCH] D87451: add new clang option -mno-xcoff-visibility

[Digger via Phabricator via cfe-commits]

DiggerLin updated this revision to Diff 294070.
DiggerLin marked an inline comment as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87451

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/aix-no-xcoff-visibility.cpp
  llvm/include/llvm/Target/TargetMachine.h
  llvm/include/llvm/Target/TargetOptions.h
  llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp

Index: llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
===
--- llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
+++ llvm/lib/Target/PowerPC/PPCAsmPrinter.cpp
@@ -1686,17 +1686,19 @@
   assert(LinkageAttr != MCSA_Invalid && "LinkageAttr should not MCSA_Invalid.");
 
   MCSymbolAttr VisibilityAttr = MCSA_Invalid;
-  switch (GV->getVisibility()) {
+  if (!TM.getNoXCOFFVisibility()) {
+switch (GV->getVisibility()) {
 
-  // TODO: "exported" and "internal" Visibility needs to go here.
-  case GlobalValue::DefaultVisibility:
-break;
-  case GlobalValue::HiddenVisibility:
-VisibilityAttr = MAI->getHiddenVisibilityAttr();
-break;
-  case GlobalValue::ProtectedVisibility:
-VisibilityAttr = MAI->getProtectedVisibilityAttr();
-break;
+// TODO: "exported" and "internal" Visibility needs to go here.
+case GlobalValue::DefaultVisibility:
+  break;
+case GlobalValue::HiddenVisibility:
+  VisibilityAttr = MAI->getHiddenVisibilityAttr();
+  break;
+case GlobalValue::ProtectedVisibility:
+  VisibilityAttr = MAI->getProtectedVisibilityAttr();
+  break;
+}
   }
 
   OutStreamer->emitXCOFFSymbolLinkageWithVisibility(GVSym, LinkageAttr,
Index: llvm/include/llvm/Target/TargetOptions.h
===
--- llvm/include/llvm/Target/TargetOptions.h
+++ llvm/include/llvm/Target/TargetOptions.h
@@ -123,9 +123,10 @@
   EnableFastISel(false), EnableGlobalISel(false), UseInitArray(false),
   DisableIntegratedAS(false), RelaxELFRelocations(false),
   FunctionSections(false), DataSections(false),
-  UniqueSectionNames(true), UniqueBasicBlockSectionNames(false),
-  TrapUnreachable(false), NoTrapAfterNoreturn(false), TLSSize(0),
-  EmulatedTLS(false), ExplicitEmulatedTLS(false), EnableIPRA(false),
+  NoXCOFFVisibility(false), UniqueSectionNames(true),
+  UniqueBasicBlockSectionNames(false), TrapUnreachable(false),
+  NoTrapAfterNoreturn(false), TLSSize(0), EmulatedTLS(false),
+  ExplicitEmulatedTLS(false), EnableIPRA(false),
   EmitStackSizeSection(false), EnableMachineOutliner(false),
   EnableMachineFunctionSplitter(false), SupportsDefaultOutlining(false),
   EmitAddrsig(false), EmitCallSiteInfo(false),
@@ -230,6 +231,9 @@
 /// Emit data into separate sections.
 unsigned DataSections : 1;
 
+/// Do not emit visibility attribute for xcoff.
+unsigned NoXCOFFVisibility : 1;
+
 unsigned UniqueSectionNames : 1;
 
 /// Use unique names for basic block sections.
Index: llvm/include/llvm/Target/TargetMachine.h
===
--- llvm/include/llvm/Target/TargetMachine.h
+++ llvm/include/llvm/Target/TargetMachine.h
@@ -260,6 +260,10 @@
 return Options.FunctionSections;
   }
 
+  /// Return true if visibility attribute should not be emitted in xcoff,
+  /// corresponding to -mno-xcoff-visibility.
+  bool getNoXCOFFVisibility() const { return Options.NoXCOFFVisibility; }
+
   /// If basic blocks should be emitted into their own section,
   /// corresponding to -fbasic-block-sections.
   llvm::BasicBlockSection getBBSectionsType() const {
Index: clang/test/CodeGen/aix-no-xcoff-visibility.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-no-xcoff-visibility.cpp
@@ -0,0 +1,110 @@
+// RUN: %clang -target powerpc-unknown-aix -emit-llvm -o - -S  %s  |\
+// RUN:   FileCheck --check-prefix=VISIBILITY-IR %s
+
+// RUN: %clang -target powerpc-unknown-aix -o - -S  %s  |\
+// RUN:   FileCheck --check-prefix=NOVISIBILITY-ASM %s
+
+// RUN: %clang -target powerpc-unknown-linux  -emit-llvm  -o - -S %s  | \
+// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
+
+// RUN: %clang -mno-xcoff-visibility -target powerpc-unknown-aix -emit-llvm -o - -S %s  | \
+// RUN: FileCheck -check-prefix=VISIBILITY-IR %s
+
+// RUN: %clang -mno-xcoff-visibility -target powerpc-unknown-aix -o - -S %s  | \
+// RUN: FileCheck -check-prefix=NOVISIBILITY-ASM %s
+
+// RUN: not %clang -mno-xcoff-visibility -target powerpc-unknown-linux -emit-llvm -o - -S %s  2>&1 | \
+// RUN: FileCheck -check-prefix=ERROR %s
+
+// RUN: %clang -fvisibilit

[PATCH] D77229: [Analyzer] Avoid handling of LazyCompundVals in IteratorModeling

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

baloghadamsoftware added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/Iterator.cpp:330-336
+SVal getReturnIterator(const CallEvent &Call) {
+  Optional RetValUnderConstr = Call.getReturnValueUnderConstruction();
+  if (RetValUnderConstr.hasValue())
+return *RetValUnderConstr;
+
+  return Call.getReturnValue();
+}

baloghadamsoftware wrote:
> NoQ wrote:
> > baloghadamsoftware wrote:
> > > baloghadamsoftware wrote:
> > > > NoQ wrote:
> > > > > baloghadamsoftware wrote:
> > > > > > NoQ wrote:
> > > > > > > NoQ wrote:
> > > > > > > > NoQ wrote:
> > > > > > > > > I still believe you have not addressed the problem while 
> > > > > > > > > moving the functions from D81718 to this patch. The caller of 
> > > > > > > > > this function has no way of knowing whether the return value 
> > > > > > > > > is the prvalue of the iterator or the glvalue of the iterator.
> > > > > > > > > 
> > > > > > > > > Looks like most callers are safe because they expect the 
> > > > > > > > > object of interest to also be already tracked. But it's quite 
> > > > > > > > > possible that both are tracked, say:
> > > > > > > > > 
> > > > > > > > > ```lang=c++
> > > > > > > > >   Container1 container1 = ...;
> > > > > > > > >   Container2 container2 = { 
> > > > > > > > > container1.begin() };
> > > > > > > > >   container2.begin(); // ???
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > Suppose `Container1::iterator` is implemented as an object 
> > > > > > > > > and `Container2::iterator` is implemented as a pointer. In 
> > > > > > > > > this case `getIteratorPosition(getReturnIterator())` would 
> > > > > > > > > yield the position of `container1.begin()` whereas the 
> > > > > > > > > correct answer is the position of `container2.begin()`.
> > > > > > > > > 
> > > > > > > > > This problem may seem artificial but it is trivial to avoid 
> > > > > > > > > if you simply stop defending your convoluted solution of 
> > > > > > > > > looking at value classes instead of AST types.
> > > > > > > > Ugh, the problem is much worse. D82185 is entirely broken for 
> > > > > > > > the exact reason i described above and you only didn't notice 
> > > > > > > > it because you wrote almost no tests.
> > > > > > > > 
> > > > > > > > Consider the test you've added in D82185:
> > > > > > > > 
> > > > > > > > ```lang=c++
> > > > > > > > void begin_ptr_iterator(const cont_with_ptr_iterator &c) {
> > > > > > > >   auto i = c.begin();
> > > > > > > > 
> > > > > > > >   clang_analyzer_eval(clang_analyzer_iterator_container(i) == 
> > > > > > > > &c); // expected-warning{{TRUE}}
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > It breaks very easily if you modify it slightly:
> > > > > > > > ```lang=c++
> > > > > > > > void begin_ptr_iterator(const cont_with_ptr_iterator &c) {
> > > > > > > >   auto i = c.begin();
> > > > > > > >   ++i; // <==
> > > > > > > > 
> > > > > > > >   clang_analyzer_eval(clang_analyzer_iterator_container(i) == 
> > > > > > > > &c); // Says FALSE!
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > The iterator obviously still points to the same container, so 
> > > > > > > > why does the test fail? Because you're tracking the wrong 
> > > > > > > > iterator: you treated your `&SymRegion{conj_$3}` as a glvalue 
> > > > > > > > whereas you should have treated it as a prvalue. In other 
> > > > > > > > words, your checker thinks that `&SymRegion{conj_$3}` is the 
> > > > > > > > location of an iterator object rather than the iterator itself, 
> > > > > > > > and after you increment the pointer it thinks that it's a 
> > > > > > > > completely unrelated iterator.
> > > > > > > > 
> > > > > > > > There's a separate concern about why does it say `FALSE` 
> > > > > > > > (should be `UNKNOWN`) but you get the point.
> > > > > > > The better way to test D82185 would be to make all existing tests 
> > > > > > > with iterator objects pass with iterator pointers as well. Like, 
> > > > > > > make existing container mocks use either iterator objects or 
> > > > > > > iterator pointers depending on a macro and make two run-lines in 
> > > > > > > each test file, one with `-D` and one without it. Most of the old 
> > > > > > > tests should have worked out of the box if you did it right; the 
> > > > > > > few that don't pass would be hidden under #ifdef for future 
> > > > > > > investigation.
> > > > > > Thank you for your review and especially for this tip! It is really 
> > > > > > a good idea. I changed it now and it indeed shows the problem you 
> > > > > > reported. It seems that my checker mixes up the region of the 
> > > > > > pointer-typed variable (`&i` and `&j`) with the region they point 
> > > > > > to (`&SymRegion{reg_$1 > > > > > std::vector & v>}._start>}` for `i` before the increment and 
> > > > > > `&Element{SymRegion{reg_$1 > > > > > std::vector & v>}._start>},1 S64b,int}` for both `i` and `j` 
> > > > > > after the increment).
> > > > > > 

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman edited reviewers, added: tonic, asl, hfinkel; removed: dberlin.
aaron.ballman added a comment.

In D36836#2292727 , @lebedev.ri wrote:

> In D36836#2292541 , @aaron.ballman 
> wrote:
>
>> In D36836#2289639 , @lebedev.ri 
>> wrote:
>>
>>> Rebased.
>>>
>>> There is a number of official open-source LGPL-3 implementations already:
>>>
>>> - https://github.com/SonarSource/SonarTS/pull/378
>>> - https://github.com/SonarSource/sonar-java/pull/1385
>>> - https://github.com/SonarSource/SonarJS/pull/449
>>> - https://github.com/SonarSource/sonar-php/pull/173
>>>
>>> There are other open-source LGPL-3 implementations already:
>>>
>>> - https://pypi.org/project/cognitive-complexity/ (MIT)
>>> - https://github.com/rossmacarthur/complexity (APACHE/MIT)
>>>
>>> There are other 3rd party implementations:
>>>
>>> - https://docs.codeclimate.com/docs/cognitive-complexity
>>>
>>> Quite honestly, i do not understand how did the license question arose.
>>
>> It arose in a comment that I can't seem to get phab to show me the context 
>> for (which is a bit strange, I don't think I've run into that before): 
>> https://reviews.llvm.org/D36836#877636 Perhaps part of this was carrying 
>> discussion over from the IRC channel?
>>
>>> Would have it been fine if i based this on the open-source-licensed code?
>>
>> I believe that would require legal analysis to answer.
>>
>>> Would have it not been? Would same license question be raised?
>>
>> Likewise here (I suspect the answer would depend on what the license of the 
>> open source code is).
>>
>>> Somehow i don't think it would have been.
>>
>> I don't wish to speculate about legal licensing issues on the mailing lists.
>>
>>> Is this really just about `Copyright SonarSource S.A., 2018, Switzerland. 
>>> All content is copyright protected.` in 
>>> https://www.sonarsource.com/docs/CognitiveComplexity.pdf ?
>>> But that is only about the document, not the algorithm.
>>> But even if we enternain the idea that all of the implementations must bow 
>>> to that license,
>>> then surely this is not the first code in LLVM that is 
>>> implicitly/explicitly based on copyrighted doc.
>>>
>>> This is rather frustrating.
>>
>> I am sorry and I agree that it's frustrating.
>
>
>
>> As far as I know, this captures the current state of affairs: 
>> https://reviews.llvm.org/D36836#1031600
>
> As far as I know, yes. Some further back&forth reiterated:
>
>> @Roman so you know, none of the non-SonarSource implementations have an 
>> official license from us.
>> We put the spec out in the world and we're happy when someone uses it. And 
>> that's all.
>> I appreciate how frustrated you must be with your implementation caught 
>> between a proverbial rock and a hard place.
>> Unfortunately, we (the company) just aren't willing to do the paperwork.
>> (G. Ann Campbell)
>
> F13053113: reply.eml 

Personally, I'm hopeful that this is sufficient to get us unstuck, but I'm not 
willing to make the determination simply because of the typical community view 
on making licensing decisions.

>> and basically we're waiting for help from the foundation to clear the last 
>> hurdle.
>
> Is foundation even aware of this controversy/situation?
> https://reviews.llvm.org/D36836#1021863, which is the last response i got, 
> was 2.5 years ago.
> For all we/i know this has gone off their radar.
> I understand that it is fully possible that they simply haven't gotten around 
> to it,
> but i think it would be important to check that it isn't the case of lost 
> mail.

I totally agree that we should double-check as it feels like this fell off the 
radar and I appreciate you pinging the review. @chandlerc is no longer on the 
foundation board and it looks like Danny is no longer on phabricator, so I've 
added some current LLVM foundation board members as reviewers to see if any of 
them can help us make progress.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36836

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

[PATCH] D88182: [clang][driver][AIX] Set compiler-rt as default rtlib

[Hubert Tong via Phabricator via cfe-commits]

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

LGTM; thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88182

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

[PATCH] D88233: [clang][aarch64] Address various fixed-length SVE vector operations

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes created this revision.
c-rhodes added reviewers: sdesmalen, efriedma, rsandifo-arm.
Herald added subscribers: kristof.beyls, tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
c-rhodes requested review of this revision.

This patch adds tests and support for operations on SVE vectors created
by the 'arm_sve_vector_bits' attribute, described by the Arm C Language
Extensions (ACLE, version 00bet5, section 3.7.3.3) for SVE [1].

This covers the following:

- VLSTs support the same forms of element-wise initialization as GNU vectors.
- VLSTs support the same built-in C and C++ operators as GNU vectors.
- Conditional and binary expressions containing GNU and SVE vectors (fixed or 
sizeless) are invalid since the ambiguity around the result type affects the 
ABI.

No functional changes were required to support vector initialization and
operators. The functional changes are to address unsupported conditional and
binary expressions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88233

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/attr-arm-sve-vector-bits.c

Index: clang/test/Sema/attr-arm-sve-vector-bits.c
===
--- clang/test/Sema/attr-arm-sve-vector-bits.c
+++ clang/test/Sema/attr-arm-sve-vector-bits.c
@@ -123,13 +123,38 @@
 void f(int c) {
   fixed_int8_t fs8;
   svint8_t ss8;
+  gnu_int8_t gs8;
 
+  // Check conditional expressions where the result is ambiguous are
+  // ill-formed.
   void *sel __attribute__((unused));
-  sel = c ? ss8 : fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = c ? fs8 : ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? ss8 : fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
 
-  sel = fs8 + ss8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
-  sel = ss8 + fs8; // expected-error {{cannot convert between a fixed-length and a sizeless vector}}
+  sel = c ? gs8 : ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? ss8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  sel = c ? gs8 : fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  sel = c ? fs8 : gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  // Check binary expressions where the result is ambiguous are ill-formed.
+  ss8 = ss8 + fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 = ss8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 = fs8 + ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 = fs8 + gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 = gs8 + ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 = gs8 + fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  ss8 += fs8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  ss8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  fs8 += ss8; // expected-error {{cannot combine fixed-length and sizeless SVE vectors in expression, result is ambiguous}}
+  fs8 += gs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+
+  gs8 += ss8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
+  gs8 += fs8; // expected-error {{cannot combine GNU and SVE vectors in expression, result is ambiguous}}
 }
 
 // --//
@@ -268,3 +293,78 @@
 TEST_CALL(int32)
 TEST_CALL(float64)
 TEST_CALL(bool)
+
+// --//
+// Vector initialization
+
+#if defined(__ARM_FEATURE_SVE_BITS) && __ARM_FEATURE_SVE_BITS == 256
+
+typedef svint32_t int32x8 __attribute__((arm_sve_vector_bits(N)));
+typedef svfloat64_t float64x4 __attribute__((arm_sve_vector_bits(N)));
+
+int32x8 foo = {1, 2, 3, 4, 5, 6, 7, 8};
+int32x8 foo2 = {1, 2, 3, 4, 5, 6, 7, 8, 9}; // expected-warning{{excess elements in vector initializer}}
+
+float64x4 bar = {1.0, 2.0, 3.0, 4.0};
+float64x4 bar2 = {1.0, 2.0, 3.0, 4.0, 5.0}; // e

[PATCH] D88105: [NFC] [PPC] Add PowerPC expected IR tests for C99 complex

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

Looks to me that this (since it is approved) completely replaces D88130 
. The patch description needs to be changed of 
course.
Not sure if the committer wants to try splitting out the non-AIX test as an NFC 
patch.


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

https://reviews.llvm.org/D88105

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

[PATCH] D88179: [OPENMP]PR47606: Do not update the lastprivate item if it was captured by reference as firstprivate data member.

[Mike Rice via Phabricator via cfe-commits]

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

LGTM.  Thanks Alexey.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88179

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

[clang] 296d883 - Sema: add support for `__attribute__((__swift_newtype__))`

[Saleem Abdulrasool via cfe-commits]

Author: Saleem Abdulrasool
Date: 2020-09-24T15:17:35Z
New Revision: 296d8832a3b5fe97725be62c5bbc721cc0e2cd20

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

LOG: Sema: add support for `__attribute__((__swift_newtype__))`

Add the `swift_newtype` attribute which allows a type definition to be
imported into Swift as a new type.  The imported type must be either an
enumerated type (enum) or an object type (struct).

This is based on the work of the original changes in
https://github.com/llvm/llvm-project-staging/commit/8afaf3aad2af43cfedca7a24cd817848c4e95c0c

Differential Revision: https://reviews.llvm.org/D87652
Reviewed By: Aaron Ballman

Added: 
clang/test/AST/attr-swift_newtype.m
clang/test/SemaObjC/attr-swift_newtype.m

Modified: 
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Misc/pragma-attribute-supported-attributes-list.test

Removed: 




diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index 7222f396089e..a9cee8c770f9 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2168,6 +2168,15 @@ def SwiftName : InheritableAttr {
   let Documentation = [SwiftNameDocs];
 }
 
+def SwiftNewType : InheritableAttr {
+  let Spellings = [GNU<"swift_newtype">, GNU<"swift_wrapper">];
+  let Args = [EnumArgument<"NewtypeKind", "NewtypeKind",
+   ["struct", "enum"], ["NK_Struct", "NK_Enum"]>];
+  let Subjects = SubjectList<[TypedefName], ErrorDiag>;
+  let Documentation = [SwiftNewTypeDocs];
+  let HasCustomParsing = 1;
+}
+
 def NoDeref : TypeAttr {
   let Spellings = [Clang<"noderef">];
   let Documentation = [NoDerefDocs];

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 753c4dd235fa..8930b61cced9 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3622,6 +3622,36 @@ must be a simple or qualified identifier.
   }];
 }
 
+def SwiftNewTypeDocs : Documentation {
+  let Category = SwiftDocs;
+  let Heading = "swift_newtype";
+  let Content = [{
+The ``swift_newtype`` attribute indicates that the typedef to which the
+attribute appertains is imported as a new Swift type of the typedef's name.
+Previously, the attribute was spelt ``swift_wrapper``.  While the behaviour of
+the attribute is identical with either spelling, ``swift_wrapper`` is
+deprecated, only exists for compatibility purposes, and should not be used in
+new code.
+
+* ``swift_newtype(struct)`` means that a Swift struct will be created for this
+typedef.
+
+* ``swift_newtype(enum)`` means that a Swift enum will be created for this
+ypedef.
+
+  .. code-block:: c
+
+// Import UIFontTextStyle as an enum type, with enumerated values being
+// constants.
+typedef NSString * UIFontTextStyle 
__attribute__((__swift_newtype__(enum)));
+
+// Import UIFontDescriptorFeatureKey as a structure type, with enumerated
+// values being members of the type structure.
+typedef NSString * UIFontDescriptorFeatureKey 
__attribute__((__swift_newtype__(struct)));
+
+  }];
+}
+
 def OMPDeclareSimdDocs : Documentation {
   let Category = DocCatFunction;
   let Heading = "#pragma omp declare simd";

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 211827e99de8..570f60fb9479 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2804,6 +2804,14 @@ class Parser : public CodeCompletionHandler {
SourceLocation ScopeLoc,
ParsedAttr::Syntax Syntax);
 
+  void ParseSwiftNewTypeAttribute(IdentifierInfo &AttrName,
+  SourceLocation AttrNameLoc,
+  ParsedAttributes &Attrs,
+  SourceLocation *EndLoc,
+  IdentifierInfo *ScopeName,
+  SourceLocation ScopeLoc,
+  ParsedAttr::Syntax Syntax);
+
   void ParseTypeTagForDatatypeAttribute(IdentifierInfo &AttrName,
 SourceLocation AttrNameLoc,
 ParsedAttributes &Attrs,

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 38df6be17efe..d6b2eb8a9877 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -23,6 +23,7 @@
 #include "clang/Sema/Lookup.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
+#include "clang/Sema/SemaDiagnostic.h"
 #include "

[PATCH] D36836: [clang-tidy] Implement sonarsource-function-cognitive-complexity check

[Tanya Lattner via Phabricator via cfe-commits]

tonic added a comment.

Licensing questions are best sent to the bo...@llvm.org mailing address. Can 
you reach out there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36836

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Melanie Blower via Phabricator via cfe-commits]

mibintc updated this revision to Diff 294074.
mibintc added a comment.

Responded to @rsmith's review


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

https://reviews.llvm.org/D87528

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/expr/expr.const/p2-0x.cpp
  clang/test/CodeGen/fp-floatcontrol-pragma.cpp
  clang/test/CodeGen/pragma-fenv_access.c
  clang/test/Parser/fp-floatcontrol-syntax.cpp
  clang/test/Parser/pragma-fenv_access.c
  clang/test/Preprocessor/pragma_unknown.c

Index: clang/test/Preprocessor/pragma_unknown.c
===
--- clang/test/Preprocessor/pragma_unknown.c
+++ clang/test/Preprocessor/pragma_unknown.c
@@ -16,15 +16,6 @@
 // CHECK: {{^}}#pragma STDC FP_CONTRACT DEFAULT{{$}}
 // CHECK: {{^}}#pragma STDC FP_CONTRACT IN_BETWEEN{{$}}
 
-#pragma STDC FENV_ACCESS ON  // expected-warning {{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS OFF
-#pragma STDC FENV_ACCESS DEFAULT
-#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS ON{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS OFF{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS DEFAULT{{$}}
-// CHECK: {{^}}#pragma STDC FENV_ACCESS IN_BETWEEN{{$}}
-
 #pragma STDC CX_LIMITED_RANGE ON
 #pragma STDC CX_LIMITED_RANGE OFF
 #pragma STDC CX_LIMITED_RANGE DEFAULT 
Index: clang/test/Parser/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/Parser/pragma-fenv_access.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -DSTRICT -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -DCPP -DSTRICT -ffp-exception-behavior=strict -fsyntax-only -verify %s
+#ifdef CPP
+#define CONST constexpr
+#else
+#define CONST const
+#endif
+
+#pragma STDC FENV_ACCESS IN_BETWEEN   // expected-warning {{expected 'ON' or 'OFF' or 'DEFAULT' in pragma}}
+
+#pragma STDC FENV_ACCESS OFF
+
+float func_04(int x, float y) {
+  if (x)
+return y + 2;
+  #pragma STDC FENV_ACCESS ON // expected-error{{'#pragma STDC FENV_ACCESS' can only appear at file scope or at the start of a compound statement}}
+  return x + y;
+}
+
+int main() {
+  CONST float one = 1.0F ;
+  CONST float three = 3.0F ;
+  CONST float four = 4.0F ;
+  CONST float frac_ok = one/four;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+3 {{constexpr variable 'frac' must be initialized by a constant expression}}
+//expected-note@+2 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+  CONST float frac = one/three; // rounding
+  CONST double d = one;
+  CONST int not_too_big = 255;
+  CONST float fnot_too_big = not_too_big;
+  CONST int too_big = 0x7ff0;
+#if defined(CPP) & defined(STRICT)
+//expected-error@+6 {{constexpr variable 'fbig' must be initialized by a constant expression}}
+//expected-note@+5 {{compile time floating point arithmetic suppressed in strict evaluation modes}}
+#endif
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
+  CONST float fbig = too_big; // inexact
+  if (one <= four)  return 0;
+  return -1;
+}
Index: clang/test/Parser/fp-floatcontrol-syntax.cpp
===
--- clang/test/Parser/fp-floatcontrol-syntax.cpp
+++ clang/test/Parser/fp-floatcontrol-syntax.cpp
@@ -26,19 +26,14 @@
 double a = 0.0;
 double b = 1.0;
 
-//FIXME At some point this warning will be removed, until then
-//  document the warning
-#ifdef FAST
-// expected-warning@+1{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#pragma STDC FENV_ACCESS ON
-#else
-#pragma STDC FENV_ACCESS ON // expected-warning{{pragma STDC FENV_ACCESS ON is not supported, ignoring pragma}}
-#endif
 #ifdef STRICT
 #pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when except is enabled}}
 #else
+#ifndef FAST
 // Currently FENV_ACCESS cannot be enabled by pragma, skip error check
-#pragma float_control(precise, off) // not-expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#pragma STDC FENV_ACCESS ON
+#pragma float_control(precise, off) // expected-error {{'#pragma float_control(precise, off)' is illegal when fenv_access is enabled}}
+#endif
 #endif
 
 #pragma float_control(precise, on)
Index: clang/test/CodeGen/pragma-fenv_access.c
===
--- /dev/null
+++ clang/test/CodeGen/pragma-fenv_access.c
@@ -0,

[PATCH] D87652: Sema: add support for `__attribute__((__swift_newtype__))`

[Saleem Abdulrasool via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG296d8832a3b5: Sema: add support for 
`__attribute__((__swift_newtype__))` (authored by compnerd).

Changed prior to commit:
  https://reviews.llvm.org/D87652?vs=293763&id=294078#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87652

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/attr-swift_newtype.m
  clang/test/Misc/pragma-attribute-supported-attributes-list.test
  clang/test/SemaObjC/attr-swift_newtype.m

Index: clang/test/SemaObjC/attr-swift_newtype.m
===
--- /dev/null
+++ clang/test/SemaObjC/attr-swift_newtype.m
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+typedef int Bad1 __attribute__((swift_newtype(invalid)));
+// expected-warning@-1 {{'swift_newtype' attribute argument not supported: 'invalid'}}
+typedef int Bad2 __attribute__((swift_newtype()));
+// expected-error@-1 {{argument required after attribute}}
+typedef int Bad3 __attribute__((swift_newtype(invalid, ignored)));
+// expected-error@-1 {{expected ')'}}
+// expected-note@-2 {{to match this '('}}
+// expected-warning@-3 {{'swift_newtype' attribute argument not supported: 'invalid'}}
+
+struct __attribute__((__swift_newtype__(struct))) Bad4 {};
+// expected-error@-1 {{'__swift_newtype__' attribute only applies to typedefs}}
Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -151,6 +151,7 @@
 // CHECK-NEXT: SwiftError (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: SwiftErrorResult (SubjectMatchRule_variable_is_parameter)
 // CHECK-NEXT: SwiftIndirectResult (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: SwiftNewType (SubjectMatchRule_type_alias)
 // CHECK-NEXT: SwiftObjCMembers (SubjectMatchRule_objc_interface)
 // CHECK-NEXT: TLSModel (SubjectMatchRule_variable_is_thread_local)
 // CHECK-NEXT: Target (SubjectMatchRule_function)
Index: clang/test/AST/attr-swift_newtype.m
===
--- /dev/null
+++ clang/test/AST/attr-swift_newtype.m
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+
+typedef int T1 __attribute__((__swift_newtype__(struct)));
+typedef int T2 __attribute__((__swift_newtype__(enum)));
+
+typedef int T3 __attribute__((__swift_wrapper__(struct)));
+typedef int T4 __attribute__((__swift_wrapper__(enum)));
+
+typedef int T5;
+typedef int T5 __attribute__((__swift_wrapper__(struct)));
+typedef int T5;
+// CHECK-LABEL: TypedefDecl {{.+}} T5 'int'
+// CHECK-NEXT: BuiltinType {{.+}} 'int'
+// CHECK-NEXT: TypedefDecl {{.+}} T5 'int'
+// CHECK-NEXT: BuiltinType {{.+}} 'int'
+// CHECK-NEXT: SwiftNewTypeAttr {{.+}} NK_Struct
+// CHECK-NEXT: TypedefDecl {{.+}} T5 'int'
+// CHECK-NEXT: BuiltinType {{.+}} 'int'
+// CHECK-NEXT: SwiftNewTypeAttr {{.+}} NK_Struct
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -5946,6 +5946,33 @@
   D->addAttr(::new (S.Context) SwiftNameAttr(S.Context, AL, Name));
 }
 
+static void handleSwiftNewType(Sema &S, Decl *D, const ParsedAttr &AL) {
+  // Make sure that there is an identifier as the annotation's single argument.
+  if (!checkAttributeNumArgs(S, AL, 1))
+return;
+
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIdentifier;
+return;
+  }
+
+  SwiftNewTypeAttr::NewtypeKind Kind;
+  IdentifierInfo *II = AL.getArgAsIdent(0)->Ident;
+  if (!SwiftNewTypeAttr::ConvertStrToNewtypeKind(II->getName(), Kind)) {
+S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << II;
+return;
+  }
+
+  if (!isa(D)) {
+S.Diag(AL.getLoc(), diag::warn_attribute_wrong_decl_type_str)
+<< AL << "typedefs";
+return;
+  }
+
+  D->addAttr(::new (S.Context) SwiftNewTypeAttr(S.Context, AL, Kind));
+}
+
 //===--===//
 // Microsoft specific attribute handlers.
 //===--===//
@@ -7871,6 +7898,9 @@
   case ParsedAttr::AT_SwiftName:
 handleSwiftName(S, D, AL);
 break;
+  case ParsedAttr::AT_SwiftNewType:
+handleSwiftNewType(S, D, AL);
+break;
   case ParsedAttr::AT_SwiftObjCMembers:
 handleSimpleAttribute(S, D, AL);
 break;
Index: clang/lib/Pars

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

Note, @sepavloff is working on overlapping concerns here, D87822 
: [FPEnv] Evaluate constant expressions under 
non-default rounding modes


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

https://reviews.llvm.org/D87528

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

[PATCH] D87528: Enable '#pragma STDC FENV_ACCESS' in frontend cf. D69272 - Work in Progress

[Melanie Blower via Phabricator via cfe-commits]

mibintc marked 2 inline comments as done.
mibintc added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:2683
   }
+  if (const BinaryOperator *BE = dyn_cast(E)) {
+if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {

rsmith wrote:
> `E` here is only intended for use in determining diagnostic locations, and 
> isn't intended to necessarily be the expression being evaluated. Instead of 
> assuming that this is the right expression to inspect, please either query 
> the FP features in the caller and pass them into here or change this function 
> to take a `const BinaryOperator*`.
OK I changed it to use BinaryOperator



Comment at: clang/lib/AST/ExprConstant.cpp:2685
+if (BE->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+  return false;

rsmith wrote:
> This should be an `FFDiag` not a `CCEDiag` because we want to suppress all 
> constant folding, not only treating the value as a core constant expression. 
> Also we should check this before checking for a NaN result, because if both 
> happen at once, this is the diagnostic we want to produce.
OK I made this change



Comment at: clang/lib/AST/ExprConstant.cpp:13283-13286
+if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+  Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+  return false;
+}

rsmith wrote:
> Hm, does `fabs` depend on the floating-point environment? Is the concern the 
> interaction with signaling NaNs?
You're right, "fabs(x) raises no floating-point exceptions, even if x is a 
signaling NaN. The returned value is independent of the current rounding 
direction mode.".  Thanks a lot for the careful reading, I really appreciate it



Comment at: clang/lib/AST/ExprConstant.cpp:13372-13376
+  if (E->getFPFeaturesInEffect(Info.Ctx.getLangOpts()).isFPConstrained()) {
+// In C lang ref, footnote, cast may raise inexact exception.
+Info.CCEDiag(E, diag::note_constexpr_float_arithmetic_strict);
+return false;
+  }

rsmith wrote:
> Is it feasible to only reject cases that would actually be inexact, rather 
> than disallowing all conversions to floating-point types? It would seem 
> preferable to allow at least widening FP conversions and complex -> real, 
> since they never depend on the FP environment (right?).
I changed it like you suggested, do the binary APFloat calculation and check 
the operation status to see if the result is inexact. I also added logic to 
check "is widening".  Is that OK (builtin kind <) or maybe I should rather 
check the precision bitwidth? 



Comment at: clang/test/CodeGen/pragma-fenv_access.c:1
+// xxx: %clang_cc1 -ffp-exception-behavior=strict -frounding-math -triple 
%itanium_abi_triple -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -ffp-exception-behavior=strict -triple %itanium_abi_triple 
-emit-llvm %s -o - | FileCheck %s

rsmith wrote:
> Is this RUN line intentionally disabled?
Oops thank you. I was working with an old revision of the patch and the test 
cause no longer worked because rounding setting was different. i just got rid 
of the run line that includes frounding-math


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

https://reviews.llvm.org/D87528

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

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

Ping. Can this go in? Still using this on a local fork, and still feels nice to 
be able to specify just a single tool.


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

https://reviews.llvm.org/D84176

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

[PATCH] D88236: [PR47636] Fix tryEmitPrivate to handle non-constantarraytypes

[Erich Keane via Phabricator via cfe-commits]

erichkeane created this revision.
erichkeane added reviewers: rjmccall, rsmith.
erichkeane requested review of this revision.

As mentioned in the bug report, tryEmitPrivate chokes on the
MaterializeTemporaryExpr in the reproducers, since it assumes that if
there are elements, than it must be a ConstantArrayType. However, the
MaterializeTemporaryExpr (which matches exactly the AST when it is NOT a
global/static) has an incomplete array type.

This changes the section where the number-of-elements is non-zero to
properly handle non-CAT types by just extracting it as an array type
(since all we needed was the element type out of it).


https://reviews.llvm.org/D88236

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/test/CodeGenCXX/pr47636.cpp


Index: clang/test/CodeGenCXX/pr47636.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr47636.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -o - -emit-llvm -triple x86_64-linux-pc %s | FileCheck %s
+int(&&intu_rvref)[] {1,2,3,4};
+// CHECK: @_ZGR10intu_rvref_ = internal global [4 x i32] [i32 1, i32 2, i32 3, 
i32 4]
+// CHECK: @intu_rvref = constant [4 x i32]* @_ZGR10intu_rvref_
+
+void foo() {
+  static const int(&&intu_rvref)[] {1,2,3,4};
+  // CHECK: @_ZZ3foovE10intu_rvref = internal constant [4 x i32]* 
@_ZGRZ3foovE10intu_rvref_
+  // CHECK: @_ZGRZ3foovE10intu_rvref_ = internal constant [4 x i32] [i32 1, 
i32 2, i32 3, i32 4]
+}
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -2108,8 +2108,7 @@
   case APValue::Union:
 return ConstStructBuilder::BuildStruct(*this, Value, DestType);
   case APValue::Array: {
-const ConstantArrayType *CAT =
-CGM.getContext().getAsConstantArrayType(DestType);
+const ArrayType *ArrayTy = CGM.getContext().getAsArrayType(DestType);
 unsigned NumElements = Value.getArraySize();
 unsigned NumInitElts = Value.getArrayInitializedElts();
 
@@ -2117,7 +2116,7 @@
 llvm::Constant *Filler = nullptr;
 if (Value.hasArrayFiller()) {
   Filler = tryEmitAbstractForMemory(Value.getArrayFiller(),
-CAT->getElementType());
+ArrayTy->getElementType());
   if (!Filler)
 return nullptr;
 }
@@ -2132,7 +2131,7 @@
 llvm::Type *CommonElementType = nullptr;
 for (unsigned I = 0; I < NumInitElts; ++I) {
   llvm::Constant *C = tryEmitPrivateForMemory(
-  Value.getArrayInitializedElt(I), CAT->getElementType());
+  Value.getArrayInitializedElt(I), ArrayTy->getElementType());
   if (!C) return nullptr;
 
   if (I == 0)
@@ -2144,7 +2143,8 @@
 
 // This means that the array type is probably "IncompleteType" or some
 // type that is not ConstantArray.
-if (CAT == nullptr && CommonElementType == nullptr && !NumInitElts) {
+if (!isa(ArrayTy) &&
+CommonElementType == nullptr && !NumInitElts) {
   const ArrayType *AT = CGM.getContext().getAsArrayType(DestType);
   CommonElementType = CGM.getTypes().ConvertType(AT->getElementType());
   llvm::ArrayType *AType = llvm::ArrayType::get(CommonElementType,


Index: clang/test/CodeGenCXX/pr47636.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/pr47636.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -o - -emit-llvm -triple x86_64-linux-pc %s | FileCheck %s
+int(&&intu_rvref)[] {1,2,3,4};
+// CHECK: @_ZGR10intu_rvref_ = internal global [4 x i32] [i32 1, i32 2, i32 3, i32 4]
+// CHECK: @intu_rvref = constant [4 x i32]* @_ZGR10intu_rvref_
+
+void foo() {
+  static const int(&&intu_rvref)[] {1,2,3,4};
+  // CHECK: @_ZZ3foovE10intu_rvref = internal constant [4 x i32]* @_ZGRZ3foovE10intu_rvref_
+  // CHECK: @_ZGRZ3foovE10intu_rvref_ = internal constant [4 x i32] [i32 1, i32 2, i32 3, i32 4]
+}
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -2108,8 +2108,7 @@
   case APValue::Union:
 return ConstStructBuilder::BuildStruct(*this, Value, DestType);
   case APValue::Array: {
-const ConstantArrayType *CAT =
-CGM.getContext().getAsConstantArrayType(DestType);
+const ArrayType *ArrayTy = CGM.getContext().getAsArrayType(DestType);
 unsigned NumElements = Value.getArraySize();
 unsigned NumInitElts = Value.getArrayInitializedElts();
 
@@ -2117,7 +2116,7 @@
 llvm::Constant *Filler = nullptr;
 if (Value.hasArrayFiller()) {
   Filler = tryEmitAbstractForMemory(Value.getArrayFiller(),
-CAT->getElementType());
+ArrayTy->getElementType());
   if (!Filler)
 return nullptr;
 }
@@ -2132,7 +2131,7 @@
 llvm

[PATCH] D88236: [PR47636] Fix tryEmitPrivate to handle non-constantarraytypes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Note: @rsmith and @rjmccall were the last ones in this section based on Blame 
(by a wide margin!), so added both as reviewers.


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

https://reviews.llvm.org/D88236

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

[PATCH] D88239: [clang-format] Fix spaces around */& in multi-variable declarations

[Alexander Richardson via Phabricator via cfe-commits]

arichardson created this revision.
arichardson added reviewers: JakeMerdichAMD, MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
arichardson requested review of this revision.

While writing tests for D88227 , I noticed 
that multi-variable declarations
such as `SomeType *qualified *a = NULL, *const *b;` were being formatted
weirdly for Left and Middle pointer alignment. This change attempts to make
this more consistent and ensures there is always a space between the */&
and the variable name.

Before (with PAS_Left):
SomeType * const *a = NULL, * const *b;
After:
SomeType* const* a = NULL, * const* b;


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88239

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -6629,14 +6629,30 @@
   Style.PointerAlignment = FormatStyle::PAS_Left;
   Style.DerivePointerAlignment = false;
   verifyFormat("aaa\n"
-   "*a = aaa,\n"
-   "*b = bbb;",
+   "* a = aaa,\n"
+   "  * b = bbb;",
Style);
-  verifyFormat("a *a = aaa, *b = bbb,\n"
-   "  *b = bbb, *d = ddd;",
+  verifyFormat("a* a = aaa, * b = bb,\n"
+   "   * b = bbb, * d = ;",
Style);
   verifyFormat("vector a, b;", Style);
+  verifyFormat("for (int* p, * q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char* x, * const* y = NULL;", Style);
+  verifyFormat("char** x, ** y = NULL;", Style);
+  verifyFormat("int x, * xp = &x, & xr = x, *& xpr = xp, * const& xcpr = xp;",
+   Style);
+  Style.PointerAlignment = FormatStyle::PAS_Right;
   verifyFormat("for (int *p, *q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char *x, *const *y = NULL;", Style);
+  verifyFormat("char **x, **y = NULL;", Style);
+  verifyFormat("int x, *xp = &x, &xr = x, *&xpr = xp, *const &xcpr = xp;",
+   Style);
+  Style.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("for (int * p, * q; p != q; p = p->next) {\n}", Style);
+  verifyFormat("char * x, * const * y = NULL;", Style);
+  verifyFormat("char ** x, ** y = NULL;", Style);
+  verifyFormat("int x, * xp = &x, & xr = x, *& xpr = xp, * const & xcpr = xp;",
+   Style);
 }
 
 TEST_F(FormatTest, ConditionalExpressionsInBrackets) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -914,13 +914,31 @@
   }
   break;
 case tok::pipe:
-case tok::amp:
   // | and & in declarations/type expressions represent union and
   // intersection types, respectively.
   if (Style.Language == FormatStyle::LK_JavaScript &&
   !Contexts.back().IsExpression)
 Tok->setType(TT_JsTypeOperator);
   break;
+case tok::amp:
+  if (Style.Language == FormatStyle::LK_JavaScript &&
+  !Contexts.back().IsExpression) {
+Tok->setType(TT_JsTypeOperator);
+break;
+  }
+  LLVM_FALLTHROUGH; // Handle multi-variable declarations of reference type
+case tok::star:
+  if (Style.isCpp() && Contexts.back().FirstStartOfName &&
+  Contexts.back().FirstStartOfName->PartOfMultiVariableDeclStmt) {
+// '*' or '&' after a comma is always a pointer or reference if
+// we are in a context declaring multiple variables.
+FormatToken *Prev = Tok->getPreviousNonComment();
+if (Prev && Prev->is(tok::comma)) {
+  Tok->setType(TT_PointerOrReference);
+  Line.IsMultiVariableDeclStmt = true;
+}
+  }
+  break;
 case tok::kw_if:
 case tok::kw_while:
   if (Tok->is(tok::kw_if) && CurrentToken &&
@@ -2859,12 +2877,16 @@
   if (!TokenBeforeMatchingParen || !Left.is(TT_TypeDeclarationParen))
 return true;
 }
-return (Left.Tok.isLiteral() ||
-(!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
- (Style.PointerAlignment != FormatStyle::PAS_Left ||
-  (Line.IsMultiVariableDeclStmt &&
-   (Left.NestingLevel == 0 ||
-(Left.NestingLevel == 1 && Line.First->is(tok::kw_for)));
+if (Left.Tok.isLiteral())
+  return true;
+// We group multiple '*'/'&' tokens for all pointer alignment settings.
+// TODO: why is the l_paren needed?
+if (Lef

[PATCH] D88227: [clang-format] Add a SpaceBeforePointerQualifiers style option

[Alexander Richardson via Phabricator via cfe-commits]

arichardson updated this revision to Diff 294090.
arichardson added a comment.

- Add a multi-variable-decl test (depends on D88239 
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88227

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12104,6 +12104,47 @@
NoSpaceStyle);
 }
 
+TEST_F(FormatTest, ConfigurableSpaceBeforePointerQualifiers) {
+  FormatStyle Spaces = getLLVMStyle();
+  Spaces.AttributeMacros.push_back("qualified");
+  verifyFormat("SomeType *const *a = NULL;", Spaces);
+  verifyFormat("SomeType *volatile *a = NULL;", Spaces);
+  verifyFormat("SomeType *__attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("SomeType *qualified *a = NULL, *const *b;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  Spaces.SpaceBeforePointerQualifiers = true;
+  verifyFormat("SomeType * const *a = NULL;", Spaces);
+  verifyFormat("SomeType * volatile *a = NULL;", Spaces);
+  verifyFormat("SomeType * __attribute__((attr)) *a = NULL;", Spaces);
+  verifyFormat("SomeType * qualified *a = NULL, * const *b;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  // Check that setting SpaceBeforePointerQualifiers doesn't result in extra
+  // spaces for PAS_Left/PAS_Middle.
+  Spaces.PointerAlignment = FormatStyle::PAS_Left;
+  verifyFormat("SomeType* const* a = NULL;", Spaces);
+  verifyFormat("SomeType* volatile* a = NULL;", Spaces);
+  verifyFormat("SomeType* __attribute__((attr))* a = NULL;", Spaces);
+  verifyFormat("SomeType* qualified* a = NULL, * const* b;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+
+  Spaces.PointerAlignment = FormatStyle::PAS_Middle;
+  verifyFormat("SomeType * const * a = NULL;", Spaces);
+  verifyFormat("SomeType * volatile * a = NULL;", Spaces);
+  verifyFormat("SomeType * __attribute__((attr)) * a = NULL;", Spaces);
+  verifyFormat("SomeType * qualified * a = NULL, * const * b;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+  verifyFormat("std::vector x;", Spaces);
+}
+
 TEST_F(FormatTest, AlignConsecutiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.AlignConsecutiveAssignments = true;
@@ -14003,6 +14044,7 @@
   CHECK_PARSE_BOOL(SpaceBeforeCpp11BracedList);
   CHECK_PARSE_BOOL(SpaceBeforeCtorInitializerColon);
   CHECK_PARSE_BOOL(SpaceBeforeInheritanceColon);
+  CHECK_PARSE_BOOL(SpaceBeforePointerQualifiers);
   CHECK_PARSE_BOOL(SpaceBeforeRangeBasedForLoopColon);
   CHECK_PARSE_BOOL(SpaceBeforeSquareBrackets);
   CHECK_PARSE_BOOL(UseCRLF);
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2873,6 +2873,8 @@
 return true;
   if (Left.is(TT_PointerOrReference))
 return Right.Tok.isLiteral() || Right.is(TT_BlockComment) ||
+   (Style.SpaceBeforePointerQualifiers &&
+Right.canBePointerOrReferenceQualifier()) ||
(Right.isOneOf(Keywords.kw_override, Keywords.kw_final) &&
 !Right.is(TT_StartOfName)) ||
(Right.is(tok::l_brace) && Right.is(BK_Block)) ||
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -598,6 +598,8 @@
 IO.mapOptional("SpaceBeforeInheritanceColon",
Style.SpaceBeforeInheritanceColon);
 IO.mapOptional("SpaceBeforeParens", Style.SpaceBeforeParens);
+IO.mapOptional("SpaceBeforePointerQualifiers",
+   Style.SpaceBeforePointerQualifiers);
 IO.mapOptional("SpaceBeforeRangeBasedForLoopColon",
Style.SpaceBeforeRangeBasedForLoopColon);
 IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock);
@@ -938,6 +940,7 @@
   LLVMStyle.SpaceBeforeCtorInitializerColon = true;
   LLVMStyle.SpaceBeforeInheritanceColon = true;
   LLVMStyle.SpaceBeforeParens = FormatStyle::SBPO_ControlStatements;
+  LLVMStyle.SpaceBeforePointerQualifiers = false;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;
   LLVMStyle.SpaceBeforeAssignmentOperators = true;
   LLVMStyle.SpaceBeforeCpp11BracedList = false;
Index: clang/include/clang/Format/Format.h
=

[PATCH] D88220: [C++20] Implement more implicit moves for return statements(Part of P1825R0)

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

I'd like to see some indication in the code comments, in the commit message, 
and/or in cxx_status.html, explaining //what// parts of P1825 
 are implemented and what parts remain 
unimplemented after this patch. (My ideal would be to hold this patch up until 
it completely implements P1825 . That way we 
minimize the number of minor divergences in implementation quality that people 
will have to memorize: Clang 10 does X, Clang 11 does Y, Clang 12 does Z... 
let's just jump straight from X to Z if at all humanly possible.)




Comment at: clang/lib/Sema/SemaStmt.cpp:3061
+  if (VD->getType().isVolatileQualified())
+return false;
+

This looks like a severable bugfix, right? Clang's current behavior diverges 
from GCC's, when the named return variable is volatile-qualified. 
https://godbolt.org/z/5EfK99
Personally I think this should be a separate bugfix patch, with regression 
test, fast-tracked separately from any of the C++20 stuff.



Comment at: clang/lib/Sema/SemaStmt.cpp:3085
 ///
-/// This routine implements C++14 [class.copy]p32, which attempts to treat
-/// returned lvalues as rvalues in certain cases (to prefer move construction),
-/// then falls back to treating them as lvalues if that failed.
+/// This routine implements C++20 [class.copy.elision]p3:, which attempts to
+/// treat returned lvalues as rvalues in certain cases (to prefer move

You changed "p32" to "p3:" — I think you meant "p3" without the colon.



Comment at: clang/lib/Sema/SemaStmt.cpp:3190
+///   not need to be an rvalue reference to the returned object's type.
+///   C++20
+

FWIW, I really like the idea of this table, but I don't know how to read it in 
this format. Are the papers/issues listed //below// the relevant standard, or 
//above// it?

My understanding is that CWG1579 is a defect report against C++11 (so, 
`-std=c++11` mode //includes// the new behavior added in CWG1579), but for 
example `-std=c++17` mode definitely does not include the new behavior for 
rvalue references added in P1825.

Could you change e.g. "C++11" to "-std=c++11 mode includes the following 
patches:", for clarity?



Comment at: clang/lib/Sema/SemaStmt.cpp:3210
 
+bool AffectedByCWG1579 = false;
 if (!NRVOCandidate) {

For the record, I am the person who added the diagnostic controlled by 
`AffectedByCWG1579`, and my opinion is that if removing that diagnostic would 
simplify the code, you should consider it. We should certainly remove it from 
Clang at //some// point — this year? next year? 2025? but it should 
//eventually// be removed.



Comment at: clang/lib/Sema/SemaStmt.cpp:3291
   // (again) now with the return value expression as written.
+  // TODO: C++20 considering the expression or operand as an lvalue
   if (Res.isInvalid())

FWIW, I don't understand this TODO comment. What's to do, here?



Comment at: clang/test/SemaCXX/implicit-move.cpp:21
+  NoNeedRvalueRef(const NoNeedRvalueRef &);
+  NoNeedRvalueRef(NoNeedRvalueRef &&);
+  NoNeedRvalueRef(Error);

if I understand correctly, you can (and should) remove the copy and move 
constructors from `NeedRvalueRef` and `NoNeedRvalueRef`.
Also, I strongly recommend renaming `NoNeedRvalueRef` to `NeedValue` (or 
something along those lines) — it's not that it doesn't need an rvalue ref, 
it's that it //does// need a //non//-ref.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D88088: WIP [clang] improve accuracy of ExprMutAnalyzer

[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 aside from a nit.




Comment at: clang/lib/Analysis/ExprMutationAnalyzer.cpp:455
+  const auto HasAnyNonConstIterator =
+  anyOf(allOf(hasMethod(allOf(hasName("begin"), unless(isConst(,
+  unless(hasMethod(allOf(hasName("begin"), isConst(),

JonasToth wrote:
> aaron.ballman wrote:
> > Do we want to look for methods that end with `_?[Bb]egin` or `_?[Ee]nd` so 
> > that this would catch patterns like `foo_begin()`/`foo_end()`, 
> > `FooBegin()`/`FooEnd()`, or `Foo_Begin()`/`Foo_End()`?
> This specific matcher is only applied in range-for contexts. There only the 
> `begin(); end()` methods matter. I updated the comment above to clarify this.
Ah, okay, that makes a lot more sense, thanks!



Comment at: clang/unittests/Analysis/ExprMutationAnalyzerTest.cpp:65
+
+  std::string buffer;
   for (const auto *E = selectFirst("expr", Results); E != nullptr;) {

JonasToth wrote:
> aaron.ballman wrote:
> > Was there a reason you hoisted this out of the `for` loop?
> Jup.
> ```
> buffer.clear()
> ```
> The current form does proper memory-recycling (i believe at least :D)
This smells like a micro-optimization to me (I think declaring the variable in 
the loop is more clear than clearing the buffer on each iteration).

I don't insist on changing it, but if you want to keep it, you should name it 
`Buffer` per our usual coding rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88088

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

[PATCH] D88240: [OPENMP]Fix PR47621: Variable used by task inside a template function is not made firstprivate by default

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: clang.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.

Need to fix a check for the variable if it is declared in the inner
OpenMP region to be able to firstprivatize it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88240

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/task_codegen.cpp

Index: clang/test/OpenMP/task_codegen.cpp
===
--- clang/test/OpenMP/task_codegen.cpp
+++ clang/test/OpenMP/task_codegen.cpp
@@ -389,4 +389,49 @@
 // CHECK-LABEL: taskinit
 // CHECK: call i8* @__kmpc_omp_task_alloc(
 
+template 
+void foobar() {
+  float a;
+#pragma omp parallel
+#pragma omp single
+  {
+double b;
+#pragma omp task
+a += b;
+  }
+}
+
+// CHECK: define void @{{.+}}{{.+}}()
+void () {
+  // CHECK: call void @{{.+}}foobar{{.+}}()
+  foobar();
+}
+// CHECK: define {{.*}}void @{{.+}}foobar{{.+}}()
+// CHECK: call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) @__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 1, void (i32*, i32*, ...)* bitcast (void (i32*, i32*, float*)* [[PAR_OUTLINED:@.+]] to void (i32*, i32*, ...)*), float* %{{.+}})
+
+// CHECK: define internal void [[PAR_OUTLINED]](i32* {{.+}}, i32* {{.+}}, float* {{.*}}[[A_ADDR:%.+]])
+// UNTIEDRT: [[A_ADDR_REF:%.+]] = alloca float*,
+// CHECK: [[B_ADDR:%.+]] = alloca double,
+// UNTIEDRT: [[A_ADDR:%.+]] = load float*, float** [[A_ADDR_REF]],
+
+// Copy `a` to the list of shared variables
+// CHECK: [[SHARED_A:%.+]] = getelementptr inbounds %{{.+}}, [[SHAREDS_TY:%.+]]* [[SHAREDS:%.+]], i32 0, i32 0
+// CHECK: store float* [[A_ADDR]], float** [[SHARED_A]],
+
+// Allocate task.
+// CHECK: [[RES:%.+]] = call i8* @__kmpc_omp_task_alloc(%struct.ident_t* {{.+}}, i32 {{.+}}, i32 1, i64 48, i64 8, i32 (i32, i8*)* bitcast (i32 (i32, [[T_TASK_TY:%.+]]*)* @{{.+}} to i32 (i32, i8*)*))
+// CHECK: [[TD:%.+]] = bitcast i8* [[RES]] to [[T_TASK_TY]]*
+// Copy shared vars.
+// CHECK: [[TD_TASK:%.+]] = getelementptr inbounds [[T_TASK_TY]], [[T_TASK_TY]]* [[TD]], i32 0, i32 0
+// CHECK: [[TD_TASK_SHARES_REF:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* [[TD_TASK]], i32 0, i32 0
+// CHECK: [[TD_TASK_SHARES:%.+]] = load i8*, i8** [[TD_TASK_SHARES_REF]],
+// CHECK: [[SHAREDS_BC:%.+]] = bitcast [[SHAREDS_TY]]* [[SHAREDS]] to i8*
+// CHECK: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 8 [[TD_TASK_SHARES]], i8* align 8 [[SHAREDS_BC]], i64 8, i1 false)
+
+// Copy firstprivate value of `b`.
+// CHECK: [[TD_TASK_PRIVS:%.+]] = getelementptr inbounds [[T_TASK_TY]], [[T_TASK_TY]]* [[TD]], i32 0, i32 1
+// CHECK: [[TD_TASK_PRIVS_B:%.+]] = getelementptr inbounds %{{.+}}, %{{.+}}* [[TD_TASK_PRIVS]], i32 0, i32 0
+// CHECK: [[B_VAL:%.+]] = load double, double* [[B_ADDR]],
+// CHECK: store double [[B_VAL]], double* [[TD_TASK_PRIVS_B]],
+
 #endif
Index: clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
===
--- clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
+++ clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm-bc %s -o %t-ppc-host.bc
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple nvptx64-unknown-unknown -aux-triple powerpc64le-unknown-unknown  -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -verify -fopenmp -fopenmp-cuda-mode -internal-isystem %S/../Headers/Inputs/include -internal-isystem %S/../../lib/Headers/openmp_wrappers -include __clang_openmp_device_functions.h -triple nvptx-unknown-unknown -aux-triple powerpc64le-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -o - | FileCheck %s
-// RUN: %clang_cc1 -x c++ -O1 -disable-llvm-optzns -

[PATCH] D87953: [xray] Function coverage groups

[Kyungwoo Lee via Phabricator via cfe-commits]

kyulee added a comment.

I think it looks good to me. @MaskRay Any further feedback on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

@froydnj The committed version rG31a3c5fb45b78bdaa78d94ffcc9258e839002016 
 appears 
to be very different from the review. I guess next time your probably can 
upload the diff again if it is very diffierent


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D81865#2293059 , @MaskRay wrote:

> @froydnj The committed version rG31a3c5fb45b78bdaa78d94ffcc9258e839002016 
>  appears 
> to be very different from the review. I guess next time your probably can 
> upload the diff again if it is very diffierent

Judging by a cursory glance at Phab's view of the delta ( 
https://reviews.llvm.org/rG4b64ce7428b66cacfe74dbd9dbc29aff6dfb84af ) it 
/looks/ like it wasn't too different. Mostly picking up upstream changes that 
added "DEFERRABLE"? (I think Phab uses light green for "this changed, but only 
because of upstream changes" and dark green is the actual patch changes?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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

[PATCH] D84176: [CMake][CTE] Add "check-clang-extra-..." targets to test only a particular Clang extra tool

[Roman Lebedev via Phabricator via cfe-commits]

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

LGTM, please feel free to land this as-is.


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

https://reviews.llvm.org/D84176

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

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D86671#2284078 , @dougpuob wrote:

> Hi @aaron.ballman
> About changing  `size_t nLength` to `cbLength`. I searched MSVC folders with 
> `size_t`, many names of variable start with `n`, or `i` in MFC related files. 
> So I prefer to keep it starts with `n`. Another side to name starts with 
> `cb`, I found variables like cbXxx are defined with ULONG, DWORD, or UCHAR 
> type.

I think the important point is that `cb` is used for APIs that specify a count 
of bytes, whereas `i`, `n`, and `l` are used for integers (there is no specific 
prefix for `size_t` that I'm aware of or that MSDN documents, so the lack of 
consistency there is not too surprising). That's why I mentioned that using a 
heuristic approach may allow you to identify the parameters that are intended 
to be a count of bytes so that you can use the more specific prefix if there's 
more specific contextual information available.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:236
+  PrefixStr = "fn"; // Function Pointer
+} else if (QT->isPointerType()) {
+  // clang-format off

dougpuob wrote:
> aaron.ballman wrote:
> > I'm not certain how valid it is to look at just the type and decide that 
> > it's a null-terminated string. For instance, the following is not an 
> > uncommon pattern: `void something(const char *buffer, size_t length);` and 
> > it would be a bit strange to change that into `szBuffer` as that would give 
> > an indication that the buffer is null terminated. You could look at 
> > surrounding code for additional information though, like other parameters 
> > in a function declaration. As an aside, this sort of heuristic search may 
> > also allow you to change `length` into `cbLength` instead of `nLength` for 
> > conventions like the Microsoft one.
> > 
> > However, for local variables, I don't see how you could even come up with a 
> > heuristic like you could with parameters, so I'm not certain what the right 
> > answer is here.
> OK (size_t nLength --> cbLength)
> 
> About the `void something(const char *buffer, size_t length)` pattern. `char` 
> is a special type which can express signed char and unsigned char.  One can 
> express C string(ASCII), another expresses general data buffer(in hex). I 
> think you are mentioning when it is a general data buffer. I agree with you 
> if it changed the name from `buffer` to `szBuffer` for general data buffer is 
> strange. I prefer to use `uint8_t` or `unsigned char` instead.
> 
> How about adding a new config option for it? like, `  - { key: 
> readability-identifier-naming.HungarainNotation.DontChangeCharPointer   , 
> value: 1 }` Users can make decision for their projects. For consistency with 
> HN, the default will still be changed to `szBuffer` in your case.
> 
> If I add the option, does it make sense to you from your side?
I'm not certain that approach would make sense because the decision needs to be 
made on a case-by-case basis. Taking the C standard library as an example set 
of APIs, a function like `char *strncpy(char * restrict s1, const char * 
restrict s2, size_t n);` is one where this functionality should not change `s1` 
or `s2` to be `szS1` or `szS2` because the `sz` modifier indicates a 
null-terminated string, but these strings do not have to be null terminated. 
However, an API like `char *strncat(char * restrict s1, const char * restrict 
s2, size_t n);` should change `s1` to be `szS1` because that string is required 
to be null terminated. In both cases, the declarations use `char`.

`uint8_t` and friends are newer datatypes (added in C99) and are not required 
to be present (so they're not portable to all implementations), which may 
account for buffer-based APIs still using `char`.

I think adding some heuristics to the check may help some of these situations 
pick the correct prefix, but I'm worried that cases like the two above are 
going to be almost intractable for the check. I don't have a good idea of how 
prevalent that kind of issue will be, though. One thing that would help me feel 
more comfortable would be to look at some data about how the check performs on 
some various code bases. e.g., pick some large or popular libraries that use C, 
C++, or both (Win32 SDK, LLVM headers, OpenSSL, etc), run the check over it, 
and check the results to see how often the prefix is right compared to how 
often it's wrong. If we find it's mostly right and only gets it wrong a small 
percentage of the time, then we've likely found a good compromise for the 
check's behavior. If we find it gets it wrong too often, then the kinds of 
tweaks needed may be more obvious.

Btw, the reason I think it's important to be careful about false positives for 
this particular check is because Hungarian notation is intended to convey 
semantic information in some cases (like whether a buffer is null terminated or 
not) an

[PATCH] D87953: [xray] Function coverage groups

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D87953#2293024 , @kyulee wrote:

> I think it looks good to me. @MaskRay Any further feedback on this?

This looks good from my viewpoint. One thing remains unanswered 
(https://reviews.llvm.org/D87953#2284071) is how the overhead is so large that 
you want to have multiple groups.
Deploying N versions of an executable can bring more challenges to the build 
system and deployment system.

I'd want to know how you weigh the tradeoff and decide to go this route.

This is a larger stuff that I want @dberris to sign off.




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:815
+if (FuncGroups > 1) {
+  const ArrayRef FuncName(CurFn->getName().bytes_begin(),
+   CurFn->getName().bytes_end());

You can use some variants of `makeArrayRef`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87953

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D81865#2293066 , @dblaikie wrote:

> In D81865#2293059 , @MaskRay wrote:
>
>> @froydnj The committed version rG31a3c5fb45b78bdaa78d94ffcc9258e839002016 
>>  
>> appears to be very different from the review. I guess next time your 
>> probably can upload the diff again if it is very diffierent
>
> Judging by a cursory glance at Phab's view of the delta ( 
> https://reviews.llvm.org/rG4b64ce7428b66cacfe74dbd9dbc29aff6dfb84af ) it 
> /looks/ like it wasn't too different. Mostly picking up upstream changes that 
> added "DEFERRABLE"? (I think Phab uses light green for "this changed, but 
> only because of upstream changes" and dark green is the actual patch changes?)

Sorry for the noise. What I saw previously was a mere difference in the DIAG 
macro and the new isDeferable... Maybe Phab presented the diff between two 
Diffs to me. The updated view seems good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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

[PATCH] D81865: [clang] Use string tables for static diagnostic descriptions

[Nathan Froyd via Phabricator via cfe-commits]

froydnj added a comment.

In D81865#2293146 , @MaskRay wrote:

> In D81865#2293066 , @dblaikie wrote:
>
>> In D81865#2293059 , @MaskRay wrote:
>>
>>> @froydnj The committed version rG31a3c5fb45b78bdaa78d94ffcc9258e839002016 
>>>  
>>> appears to be very different from the review. I guess next time your 
>>> probably can upload the diff again if it is very diffierent
>>
>> Judging by a cursory glance at Phab's view of the delta ( 
>> https://reviews.llvm.org/rG4b64ce7428b66cacfe74dbd9dbc29aff6dfb84af ) it 
>> /looks/ like it wasn't too different. Mostly picking up upstream changes 
>> that added "DEFERRABLE"? (I think Phab uses light green for "this changed, 
>> but only because of upstream changes" and dark green is the actual patch 
>> changes?)
>
> Sorry for the noise. What I saw previously was a mere difference in the DIAG 
> macro and the new isDeferable... Maybe Phab presented the diff between two 
> Diffs to me. The updated view seems good.

I assumed that "add another parameter to a macro due to rebasing" was not a 
significant enough change to warrant reposting...but as this is the first patch 
I was committing myself, I probably should have been a bit more explicit in 
what I was committing (even re-asking for review?  I'm not sure of the norms 
around rebasing in the LLVM project).  My mistake!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81865

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

[clang] a9fca98 - [OPENMP]PR47606: Do not update the lastprivate item if it was captured by reference as firstprivate data member.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-09-24T13:14:13-04:00
New Revision: a9fca98ee4f653278d84713caecd152fef8494f5

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

LOG: [OPENMP]PR47606: Do not update the lastprivate item if it was captured by 
reference as firstprivate data member.

No need to make final copy from the firsptrivate/lastprivate copy to the 
original item if the item is a data memeber.
Firstprivate copy creates a copy by reference and the original item gets
updated correctly when updating the lastprivate shared variable.

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/for_lastprivate_codegen.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index c5072b5563e4..7c89e07921be 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -14413,7 +14413,7 @@ OMPClause *Sema::ActOnOpenMPLastprivateClause(
 if (!isOpenMPCapturedDecl(D))
   ExprCaptures.push_back(Ref->getDecl());
   }
-  if (TopDVar.CKind == OMPC_firstprivate ||
+  if ((TopDVar.CKind == OMPC_firstprivate && !TopDVar.PrivateCopy) ||
   (!isOpenMPCapturedDecl(D) &&
Ref->getDecl()->hasAttr())) {
 ExprResult RefRes = DefaultLvalueConversion(Ref);

diff  --git a/clang/test/OpenMP/for_lastprivate_codegen.cpp 
b/clang/test/OpenMP/for_lastprivate_codegen.cpp
index 87f109e70e6e..9194a69cc49f 100644
--- a/clang/test/OpenMP/for_lastprivate_codegen.cpp
+++ b/clang/test/OpenMP/for_lastprivate_codegen.cpp
@@ -45,11 +45,12 @@ enum omp_allocator_handle_t {
 
 struct SS {
   int a;
+  char e[4];
   int b : 4;
   int &c;
   SS(int &d) : a(0), b(0), c(d) {
 #pragma omp parallel
-#pragma omp for lastprivate(a, b, c)
+#pragma omp for firstprivate(e) lastprivate(a, b, c, e)
 for (int i = 0; i < 2; ++i)
 #ifdef LAMBDA
   [&]() {
@@ -167,9 +168,9 @@ volatile int &g1 = g;
 float f;
 char cnt;
 
-// CHECK: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
-// LAMBDA: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
-// BLOCKS: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
+// CHECK: [[SS_TY:%.+]] = type { i{{[0-9]+}}, [4 x i8], i8
+// LAMBDA: [[SS_TY:%.+]] = type { i{{[0-9]+}}, [4 x i8], i8
+// BLOCKS: [[SS_TY:%.+]] = type { i{{[0-9]+}}, [4 x i8], i8
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[IMPLICIT_BARRIER_LOC:@.+]] = private unnamed_addr constant 
%{{.+}} { i32 0, i32 66, i32 0, i32 0, i8*
@@ -225,9 +226,9 @@ int main() {
 // LAMBDA: define {{.+}} @{{.+}}([[SS_TY]]*
 // LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, 
i32 0
 // LAMBDA: store i{{[0-9]+}} 0, i{{[0-9]+}}* %
-// LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, 
i32 1
-// LAMBDA: store i8
 // LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, 
i32 2
+// LAMBDA: store i8
+// LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, 
i32 3
 // LAMBDA: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, 
i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 1, 
void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, 
i{{[0-9]+}}*, [[SS_TY]]*)* [[SS_MICROTASK:@.+]] to void
 // LAMBDA: call void @__kmpc_for_static_init_4(
 // LAMBDA-NOT: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 
0, i32 0
@@ -237,14 +238,14 @@ int main() {
 
 // LAMBDA: define internal void [[SS_MICROTASK]](i{{[0-9]+}}* noalias 
[[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, [[SS_TY]]* %{{.+}})
 // LAMBDA: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 0
-// LAMBDA-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, 
i32 1
-// LAMBDA: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 2
+// LAMBDA-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, 
i32 2
+// LAMBDA: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 3
 // LAMBDA: call void @__kmpc_for_static_init_4(
 // LAMBDA-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]*
 // LAMBDA: call{{.*}} void [[SS_LAMBDA:@[^ ]+]]
 // LAMBDA: call void @__kmpc_for_static_fini(
 // LAMBDA: br i1
-// LAMBDA: [[B_REF:%.+]] = getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* 
%{{.*}}, i32 0, i32 1
+// LAMBDA: [[B_REF:%.+]] = getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* 
%{{.*}}, i32 0, i32 2
 // LAMBDA: store i8 %{{.+}}, i8* [[B_REF]],
 // LAMBDA: br label
 // LAMBDA: ret void
@@ -420,9 +421,9 @@ int main() {
 // BLOCKS: define {{.+}} @{{.+}}([[SS_TY]]*
 // BLOCKS: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 0
 // BLOCKS: store i{{[0

[PATCH] D88179: [OPENMP]PR47606: Do not update the lastprivate item if it was captured by reference as firstprivate data member.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa9fca98ee4f6: [OPENMP]PR47606: Do not update the lastprivate 
item if it was captured by… (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88179

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp

Index: clang/test/OpenMP/for_lastprivate_codegen.cpp
===
--- clang/test/OpenMP/for_lastprivate_codegen.cpp
+++ clang/test/OpenMP/for_lastprivate_codegen.cpp
@@ -45,11 +45,12 @@
 
 struct SS {
   int a;
+  char e[4];
   int b : 4;
   int &c;
   SS(int &d) : a(0), b(0), c(d) {
 #pragma omp parallel
-#pragma omp for lastprivate(a, b, c)
+#pragma omp for firstprivate(e) lastprivate(a, b, c, e)
 for (int i = 0; i < 2; ++i)
 #ifdef LAMBDA
   [&]() {
@@ -167,9 +168,9 @@
 float f;
 char cnt;
 
-// CHECK: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
-// LAMBDA: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
-// BLOCKS: [[SS_TY:%.+]] = type { i{{[0-9]+}}, i8
+// CHECK: [[SS_TY:%.+]] = type { i{{[0-9]+}}, [4 x i8], i8
+// LAMBDA: [[SS_TY:%.+]] = type { i{{[0-9]+}}, [4 x i8], i8
+// BLOCKS: [[SS_TY:%.+]] = type { i{{[0-9]+}}, [4 x i8], i8
 // CHECK: [[S_FLOAT_TY:%.+]] = type { float }
 // CHECK: [[S_INT_TY:%.+]] = type { i32 }
 // CHECK-DAG: [[IMPLICIT_BARRIER_LOC:@.+]] = private unnamed_addr constant %{{.+}} { i32 0, i32 66, i32 0, i32 0, i8*
@@ -225,9 +226,9 @@
 // LAMBDA: define {{.+}} @{{.+}}([[SS_TY]]*
 // LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 0
 // LAMBDA: store i{{[0-9]+}} 0, i{{[0-9]+}}* %
-// LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 1
-// LAMBDA: store i8
 // LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 2
+// LAMBDA: store i8
+// LAMBDA: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 3
 // LAMBDA: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 1, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, [[SS_TY]]*)* [[SS_MICROTASK:@.+]] to void
 // LAMBDA: call void @__kmpc_for_static_init_4(
 // LAMBDA-NOT: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 0
@@ -237,14 +238,14 @@
 
 // LAMBDA: define internal void [[SS_MICROTASK]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, [[SS_TY]]* %{{.+}})
 // LAMBDA: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 0
-// LAMBDA-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 1
-// LAMBDA: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 2
+// LAMBDA-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 2
+// LAMBDA: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 3
 // LAMBDA: call void @__kmpc_for_static_init_4(
 // LAMBDA-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]*
 // LAMBDA: call{{.*}} void [[SS_LAMBDA:@[^ ]+]]
 // LAMBDA: call void @__kmpc_for_static_fini(
 // LAMBDA: br i1
-// LAMBDA: [[B_REF:%.+]] = getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 1
+// LAMBDA: [[B_REF:%.+]] = getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 2
 // LAMBDA: store i8 %{{.+}}, i8* [[B_REF]],
 // LAMBDA: br label
 // LAMBDA: ret void
@@ -420,9 +421,9 @@
 // BLOCKS: define {{.+}} @{{.+}}([[SS_TY]]*
 // BLOCKS: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 0
 // BLOCKS: store i{{[0-9]+}} 0, i{{[0-9]+}}* %
-// BLOCKS: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 1
-// BLOCKS: store i8
 // BLOCKS: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 2
+// BLOCKS: store i8
+// BLOCKS: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 3
 // BLOCKS: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 1, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, [[SS_TY]]*)* [[SS_MICROTASK:@.+]] to void
 // BLOCKS: call void @__kmpc_for_static_init_4(
 // BLOCKS-NOT: getelementptr inbounds [[SS_TY]], [[SS_TY]]* %{{.+}}, i32 0, i32 0
@@ -432,14 +433,14 @@
 
 // BLOCKS: define internal void [[SS_MICROTASK]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, [[SS_TY]]* %{{.+}})
 // BLOCKS: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 0
-// BLOCKS-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 1
-// BLOCKS: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 2
+// BLOCKS-NOT: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 2
+// BLOCKS: getelementptr {{.*}}[[SS_TY]], [[SS_TY]]* %{{.*}}, i32 0, i32 3
 // BLOCKS: ca